From b09284f1f538d688f8bfdb87cd2cca338c08d0f4 Mon Sep 17 00:00:00 2001 From: StNekroman Date: Mon, 22 Nov 2021 11:22:46 +0200 Subject: [PATCH] Handle resources, never left left opened file handler on windows (#1857) On windows unit tests are failing because junit checks if all @tempdir directries are empty and can be deleted after test run. But due to opened file handles (not closed resource streams) Windows keeps files, which are "in use". Linux is less strict in this area. Additionally I want highlight that XMLStreamReaderImpl/XMLStreamWriterImpl from apache.xerces library (in OpenJDK11 at least) which are picked at runtime as xml parser implementation - they don't close associated resources. BTW, I thing that relying on some runtime (sometimes - unpredictable) dependencies - is bad practice, but this it up to separate topic. Additional fix: in IOUtil is file is locked (due to permissions or opened handle) - it will undlessly try-and-retry to delete it until end of the world, on windows. --- .../src/main/java/sonia/scm/util/IOUtil.java | 21 +++---- .../java/sonia/scm/BaseDirectoryTest.java | 14 +++-- .../java/sonia/scm/plugin/SmpArchiveTest.java | 15 ++--- .../scm/repository/xml/PathDatabase.java | 37 +++++++----- .../sonia/scm/store/FileStoreExporter.java | 6 +- .../store/JAXBConfigurationEntryStore.java | 16 +++-- .../main/java/sonia/scm/xml/XmlStreams.java | 27 +-------- .../java/sonia/scm/store/CopyOnWriteTest.java | 59 ++++++++++++------- .../importexport/RepositoryImportStep.java | 4 +- ...BetweenConfigAndConfigEntryUpdateStep.java | 33 ++++++----- .../repository/CopyMigrationStrategyTest.java | 16 ++--- ...eenConfigAndConfigEntryUpdateStepTest.java | 49 ++++++++------- 12 files changed, 150 insertions(+), 147 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/util/IOUtil.java b/scm-core/src/main/java/sonia/scm/util/IOUtil.java index 037f8e07ff..5df6e4968b 100644 --- a/scm-core/src/main/java/sonia/scm/util/IOUtil.java +++ b/scm-core/src/main/java/sonia/scm/util/IOUtil.java @@ -391,20 +391,17 @@ public final class IOUtil } } - for (int i = 20; !file.delete(); i--) + for (int i = 20; !file.delete() && i > 0; i--) { - if (i <= 20) - { - String message = "could not delete file ".concat(file.getPath()); + String message = "could not delete file ".concat(file.getPath()); - if (silent) - { - logger.error(message); - } - else - { - throw new IOException(message); - } + if (silent) + { + logger.error(message); + } + else + { + throw new IOException(message); } try diff --git a/scm-core/src/test/java/sonia/scm/BaseDirectoryTest.java b/scm-core/src/test/java/sonia/scm/BaseDirectoryTest.java index b9342db32d..6d8e63a9ed 100644 --- a/scm-core/src/test/java/sonia/scm/BaseDirectoryTest.java +++ b/scm-core/src/test/java/sonia/scm/BaseDirectoryTest.java @@ -28,6 +28,8 @@ import com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import java.io.File; +import java.nio.file.Paths; import java.util.Map; import java.util.Properties; @@ -38,19 +40,19 @@ class BaseDirectoryTest { @Test void shouldGetFromClassPathResource() { BaseDirectory directory = builder().withClassPathResource("/sonia/scm/basedirectory.properties").create(); - assertThat(directory.find().toString()).isEqualTo("/tmp/scm_home"); + assertThat(directory.find()).isEqualTo(Paths.get("/tmp/scm_home")); } @Test void shouldGetFromSystemProperty() { BaseDirectory directory = builder().withSystemProperty(BaseDirectory.SYSTEM_PROPERTY, "/tmp/scm_home").create(); - assertThat(directory.find().toString()).isEqualTo("/tmp/scm_home"); + assertThat(directory.find()).isEqualTo(Paths.get("/tmp/scm_home")); } @Test void shouldGetFromEnvironmentVariable() { BaseDirectory directory = builder().withEnvironment(BaseDirectory.ENVIRONMENT_VARIABLE, "/tmp/scm_home").create(); - assertThat(directory.find().toString()).isEqualTo("/tmp/scm_home"); + assertThat(directory.find()).isEqualTo(Paths.get("/tmp/scm_home")); } @Nested @@ -59,19 +61,19 @@ class BaseDirectoryTest { @Test void linux() { BaseDirectory directory = builder().withSystemProperty("user.home", "/tmp").create(); - assertThat(directory.find().toString()).isEqualTo("/tmp/.scm"); + assertThat(directory.find()).isEqualTo(Paths.get("/tmp/.scm")); } @Test void osx() { BaseDirectory directory = builder().withOsx().withSystemProperty("user.home", "/tmp").create(); - assertThat(directory.find().toString()).isEqualTo("/tmp/Library/Application Support/SCM-Manager"); + assertThat(directory.find()).isEqualTo(Paths.get("/tmp/Library/Application Support/SCM-Manager")); } @Test void windows() { BaseDirectory directory = builder().withWindows().withEnvironment("APPDATA", "/tmp").create(); - assertThat(directory.find().toString()).isEqualTo("/tmp\\SCM-Manager"); + assertThat(directory.find()).isEqualTo(Paths.get("/tmp\\SCM-Manager")); } } diff --git a/scm-core/src/test/java/sonia/scm/plugin/SmpArchiveTest.java b/scm-core/src/test/java/sonia/scm/plugin/SmpArchiveTest.java index c732f5c313..4153328bec 100644 --- a/scm-core/src/test/java/sonia/scm/plugin/SmpArchiveTest.java +++ b/scm-core/src/test/java/sonia/scm/plugin/SmpArchiveTest.java @@ -43,6 +43,7 @@ import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.nio.file.Path; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; @@ -137,17 +138,14 @@ class SmpArchiveTest { return archiveFile; } - private XMLStreamWriter createStreamWriter(File file) throws IOException, XMLStreamException { - return XMLOutputFactory.newFactory().createXMLStreamWriter(new FileOutputStream(file)); - } - private void writeDescriptor(File descriptor, String name, String version) throws IOException, XMLStreamException { IOUtil.mkdirs(descriptor.getParentFile()); XMLStreamWriter writer = null; - try { - writer = createStreamWriter(descriptor); + try (OutputStream os = new FileOutputStream(descriptor)) { + writer = XMLOutputFactory.newFactory().createXMLStreamWriter(os); + writer.writeStartDocument(); writer.writeStartElement("plugin"); writer.writeStartElement("information"); @@ -157,10 +155,7 @@ class SmpArchiveTest { writer.writeEndElement(); writer.writeEndElement(); writer.writeEndDocument(); - } finally { - if (writer != null) { - writer.close(); - } + writer.close(); } } diff --git a/scm-dao-xml/src/main/java/sonia/scm/repository/xml/PathDatabase.java b/scm-dao-xml/src/main/java/sonia/scm/repository/xml/PathDatabase.java index 4ef7898bec..a807ed35f8 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/repository/xml/PathDatabase.java +++ b/scm-dao-xml/src/main/java/sonia/scm/repository/xml/PathDatabase.java @@ -36,6 +36,9 @@ import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; import javax.xml.stream.XMLStreamWriter; import java.io.IOException; +import java.io.Reader; +import java.io.Writer; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -67,22 +70,24 @@ class PathDatabase { CopyOnWrite.withTemporaryFile( temp -> { - try (IndentXMLStreamWriter writer = XmlStreams.createWriter(temp)) { - writer.writeStartDocument(ENCODING, VERSION); + try (Writer ioWriter = Files.newBufferedWriter(temp, StandardCharsets.UTF_8)) { + try (IndentXMLStreamWriter writer = XmlStreams.createWriter(ioWriter)) { + writer.writeStartDocument(ENCODING, VERSION); - writeRepositoriesStart(writer, creationTime, lastModified); - for (Map.Entry e : pathDatabase.entrySet()) { - writeRepository(writer, e.getKey(), e.getValue()); + writeRepositoriesStart(writer, creationTime, lastModified); + for (Map.Entry e : pathDatabase.entrySet()) { + writeRepository(writer, e.getKey(), e.getValue()); + } + writer.writeEndElement(); + + writer.writeEndDocument(); + } catch (XMLStreamException ex) { + throw new InternalRepositoryException( + ContextEntry.ContextBuilder.entity(Path.class, storePath.toString()).build(), + "failed to write repository path database", + ex + ); } - writer.writeEndElement(); - - writer.writeEndDocument(); - } catch (XMLStreamException | IOException ex) { - throw new InternalRepositoryException( - ContextEntry.ContextBuilder.entity(Path.class, storePath.toString()).build(), - "failed to write repository path database", - ex - ); } }, storePath @@ -121,8 +126,8 @@ class PathDatabase { void read(OnRepositories onRepositories, OnRepository onRepository) { LOG.trace("read repository path database from {}", storePath); XMLStreamReader reader = null; - try { - reader = XmlStreams.createReader(storePath); + try (Reader inputReader = Files.newBufferedReader(storePath, StandardCharsets.UTF_8)) { + reader = XmlStreams.createReader(inputReader); while (reader.hasNext()) { int eventType = reader.next(); diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/FileStoreExporter.java b/scm-dao-xml/src/main/java/sonia/scm/store/FileStoreExporter.java index 437dbc8336..654101f131 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/FileStoreExporter.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/FileStoreExporter.java @@ -35,6 +35,8 @@ import javax.inject.Inject; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; import java.io.IOException; +import java.io.Reader; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; @@ -135,8 +137,8 @@ public class FileStoreExporter implements StoreExporter { private StoreType determineConfigType(Path storePath) { XMLStreamReader reader = null; - try { - reader = XmlStreams.createReader(storePath); + try (Reader inputReader = Files.newBufferedReader(storePath, StandardCharsets.UTF_8)) { + reader = XmlStreams.createReader(inputReader); reader.nextTag(); if ( "configuration".equals(reader.getLocalName()) diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationEntryStore.java b/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationEntryStore.java index 714067036d..33f6792447 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationEntryStore.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationEntryStore.java @@ -37,6 +37,10 @@ import javax.xml.bind.Marshaller; import javax.xml.namespace.QName; import javax.xml.stream.XMLStreamReader; import java.io.File; +import java.io.Reader; +import java.io.Writer; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.util.Collection; import java.util.Collections; import java.util.Map; @@ -138,8 +142,9 @@ public class JAXBConfigurationEntryStore implements ConfigurationEntryStore { XMLStreamReader reader = null; - try { - reader = XmlStreams.createReader(file); + + try (Reader inputReader = Files.newBufferedReader(file.toPath(), StandardCharsets.UTF_8)) { + reader = XmlStreams.createReader(inputReader); // configuration reader.nextTag(); @@ -186,11 +191,12 @@ public class JAXBConfigurationEntryStore implements ConfigurationEntryStore { - m.setProperty(Marshaller.JAXB_FRAGMENT, Boolean.TRUE); + m.setProperty(Marshaller.JAXB_FRAGMENT, Boolean.TRUE); CopyOnWrite.withTemporaryFile( temp -> { - try (IndentXMLStreamWriter writer = XmlStreams.createWriter(temp)) { + try (Writer ioWriter = Files.newBufferedWriter(temp, StandardCharsets.UTF_8); + IndentXMLStreamWriter writer = XmlStreams.createWriter(ioWriter)) { writer.writeStartDocument(); // configuration start @@ -211,7 +217,7 @@ public class JAXBConfigurationEntryStore implements ConfigurationEntryStore je = new JAXBElement<>(QName.valueOf(TAG_VALUE), type, - e.getValue()); + e.getValue()); m.marshal(je, writer); diff --git a/scm-dao-xml/src/main/java/sonia/scm/xml/XmlStreams.java b/scm-dao-xml/src/main/java/sonia/scm/xml/XmlStreams.java index 9ccbcc17b9..c15b770952 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/xml/XmlStreams.java +++ b/scm-dao-xml/src/main/java/sonia/scm/xml/XmlStreams.java @@ -32,13 +32,8 @@ import javax.xml.stream.XMLOutputFactory; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; import javax.xml.stream.XMLStreamWriter; -import java.io.File; -import java.io.IOException; import java.io.Reader; import java.io.Writer; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; public final class XmlStreams { @@ -67,31 +62,13 @@ public final class XmlStreams { } } - public static XMLStreamReader createReader(Path path) throws IOException, XMLStreamException { - return createReader(Files.newBufferedReader(path, StandardCharsets.UTF_8)); - } - - public static XMLStreamReader createReader(File file) throws IOException, XMLStreamException { - return createReader(file.toPath()); - } - - private static XMLStreamReader createReader(Reader reader) throws XMLStreamException { + public static XMLStreamReader createReader(Reader reader) throws XMLStreamException { XMLInputFactory factory = XMLInputFactory.newInstance(); factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE); return factory.createXMLStreamReader(reader); } - - public static IndentXMLStreamWriter createWriter(Path path) throws IOException, XMLStreamException { - return createWriter(Files.newBufferedWriter(path, StandardCharsets.UTF_8)); - } - - public static IndentXMLStreamWriter createWriter(File file) throws IOException, XMLStreamException { - return createWriter(file.toPath()); - } - - private static IndentXMLStreamWriter createWriter(Writer writer) throws XMLStreamException { + public static IndentXMLStreamWriter createWriter(Writer writer) throws XMLStreamException { return new IndentXMLStreamWriter(XMLOutputFactory.newFactory().createXMLStreamWriter(writer)); } - } diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/CopyOnWriteTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/CopyOnWriteTest.java index b2d9143655..3abecdcf73 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/CopyOnWriteTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/CopyOnWriteTest.java @@ -30,6 +30,7 @@ import org.junit.jupiter.api.io.TempDir; import java.io.FileOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -43,9 +44,11 @@ class CopyOnWriteTest { void shouldCreateNewFile(@TempDir Path tempDir) { Path expectedFile = tempDir.resolve("toBeCreated.txt"); - withTemporaryFile( - file -> new FileOutputStream(file.toFile()).write("great success".getBytes()), - expectedFile); + withTemporaryFile(file -> { + try (OutputStream os = new FileOutputStream(file.toFile())) { + os.write("great success".getBytes()); + } + }, expectedFile); Assertions.assertThat(expectedFile).hasContent("great success"); } @@ -55,34 +58,40 @@ class CopyOnWriteTest { Path expectedFile = tempDir.resolve("toBeOverwritten.txt"); Files.createFile(expectedFile); - withTemporaryFile( - file -> new FileOutputStream(file.toFile()).write("great success".getBytes()), - expectedFile); + withTemporaryFile(file -> { + try (OutputStream os = new FileOutputStream(file.toFile())) { + os.write("great success".getBytes()); + } + }, expectedFile); Assertions.assertThat(expectedFile).hasContent("great success"); } @Test void shouldFailForDirectory(@TempDir Path tempDir) { - assertThrows(IllegalArgumentException.class, - () -> withTemporaryFile( - file -> new FileOutputStream(file.toFile()).write("should not be written".getBytes()), - tempDir)); + assertThrows(IllegalArgumentException.class, () -> withTemporaryFile(file -> { + try (OutputStream os = new FileOutputStream(file.toFile())) { + os.write("should not be written".getBytes()); + } + }, tempDir)); } @Test void shouldFailForMissingDirectory() { - assertThrows( - IllegalArgumentException.class, - () -> withTemporaryFile( - file -> new FileOutputStream(file.toFile()).write("should not be written".getBytes()), - Paths.get("someFile"))); + assertThrows(IllegalArgumentException.class, () -> withTemporaryFile(file -> { + try (OutputStream os = new FileOutputStream(file.toFile())) { + os.write("should not be written".getBytes()); + } + }, Paths.get("someFile"))); } @Test void shouldKeepBackupIfTemporaryFileCouldNotBeWritten(@TempDir Path tempDir) throws IOException { Path unchangedOriginalFile = tempDir.resolve("notToBeDeleted.txt"); - new FileOutputStream(unchangedOriginalFile.toFile()).write("this should be kept".getBytes()); + + try (OutputStream unchangedOriginalOs = new FileOutputStream(unchangedOriginalFile.toFile())) { + unchangedOriginalOs.write("this should be kept".getBytes()); + } assertThrows( StoreException.class, @@ -111,7 +120,10 @@ class CopyOnWriteTest { @Test void shouldKeepBackupIfTemporaryFileIsMissing(@TempDir Path tempDir) throws IOException { Path backedUpFile = tempDir.resolve("notToBeDeleted.txt"); - new FileOutputStream(backedUpFile.toFile()).write("this should be kept".getBytes()); + + try (OutputStream backedUpOs = new FileOutputStream(backedUpFile.toFile())) { + backedUpOs.write("this should be kept".getBytes()); + } assertThrows( StoreException.class, @@ -125,11 +137,16 @@ class CopyOnWriteTest { @Test void shouldDeleteExistingFile(@TempDir Path tempDir) throws IOException { Path expectedFile = tempDir.resolve("toBeReplaced.txt"); - new FileOutputStream(expectedFile.toFile()).write("this should be removed".getBytes()); - withTemporaryFile( - file -> new FileOutputStream(file.toFile()).write("overwritten".getBytes()), - expectedFile); + try (OutputStream expectedOs = new FileOutputStream(expectedFile.toFile())) { + expectedOs.write("this should be removed".getBytes()); + } + + withTemporaryFile(file -> { + try (OutputStream os = new FileOutputStream(file.toFile())) { + os.write("overwritten".getBytes()) ; + } + }, expectedFile); Assertions.assertThat(Files.list(tempDir)).hasSize(1); } diff --git a/scm-webapp/src/main/java/sonia/scm/importexport/RepositoryImportStep.java b/scm-webapp/src/main/java/sonia/scm/importexport/RepositoryImportStep.java index 73d0517716..3647572eac 100644 --- a/scm-webapp/src/main/java/sonia/scm/importexport/RepositoryImportStep.java +++ b/scm-webapp/src/main/java/sonia/scm/importexport/RepositoryImportStep.java @@ -93,8 +93,8 @@ class RepositoryImportStep implements ImportStep { private void importFromTemporaryPath(ImportState state, Path path) { LOG.debug("Importing repository from temporary location in work dir"); state.getLogger().step("importing repository from temporary location"); - try { - unbundleRepository(state, Files.newInputStream(path)); + try (InputStream is = Files.newInputStream(path)) { + unbundleRepository(state, is); } catch (IOException e) { throw new ImportFailedException( entity(state.getRepository()).build(), diff --git a/scm-webapp/src/main/java/sonia/scm/update/store/DifferentiateBetweenConfigAndConfigEntryUpdateStep.java b/scm-webapp/src/main/java/sonia/scm/update/store/DifferentiateBetweenConfigAndConfigEntryUpdateStep.java index 2c2c457bbe..942aeb6d35 100644 --- a/scm-webapp/src/main/java/sonia/scm/update/store/DifferentiateBetweenConfigAndConfigEntryUpdateStep.java +++ b/scm-webapp/src/main/java/sonia/scm/update/store/DifferentiateBetweenConfigAndConfigEntryUpdateStep.java @@ -34,17 +34,19 @@ import sonia.scm.version.Version; import sonia.scm.xml.XmlStreams; import javax.xml.XMLConstants; -import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; -import javax.xml.transform.Transformer; import javax.xml.transform.TransformerException; import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.Reader; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.stream.Stream; @@ -74,8 +76,8 @@ abstract class DifferentiateBetweenConfigAndConfigEntryUpdateStep { private boolean isConfigEntryFile(Path potentialFile) { LOG.trace("Testing whether file is config entry file without mark: {}", potentialFile); XMLStreamReader reader = null; - try { - reader = XmlStreams.createReader(potentialFile); + try (Reader inputReader = Files.newBufferedReader(potentialFile, StandardCharsets.UTF_8)) { + reader = XmlStreams.createReader(inputReader); reader.nextTag(); if ("configuration".equals(reader.getLocalName())) { reader.nextTag(); @@ -104,26 +106,25 @@ abstract class DifferentiateBetweenConfigAndConfigEntryUpdateStep { } private void writeXmlDocument(Document configEntryDocument, Path temporaryFile) throws TransformerException { - try { - TransformerFactory factory = TransformerFactory.newInstance(); + try (OutputStream os = Files.newOutputStream(temporaryFile)) { + final TransformerFactory factory = TransformerFactory.newInstance(); factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); - Transformer transformer = factory.newTransformer(); - DOMSource domSource = new DOMSource(configEntryDocument); - StreamResult streamResult = new StreamResult(Files.newOutputStream(temporaryFile)); - transformer.transform(domSource, streamResult); + final DOMSource domSource = new DOMSource(configEntryDocument); + + factory.newTransformer().transform(domSource, new StreamResult(os)); } catch (IOException e) { throw new UpdateException("Could not write modified config entry file", e); } } private Document readAsXmlDocument(Path configFile) { - try { - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); - factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); - DocumentBuilder builder = factory.newDocumentBuilder(); - return builder.parse(Files.newInputStream(configFile)); + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + + try (InputStream is = Files.newInputStream(configFile)) { + return factory.newDocumentBuilder().parse(is); } catch (ParserConfigurationException | SAXException | IOException e) { throw new UpdateException("Could not read config entry file", e); } diff --git a/scm-webapp/src/test/java/sonia/scm/update/repository/CopyMigrationStrategyTest.java b/scm-webapp/src/test/java/sonia/scm/update/repository/CopyMigrationStrategyTest.java index 1766c288f9..d0d72f967e 100644 --- a/scm-webapp/src/test/java/sonia/scm/update/repository/CopyMigrationStrategyTest.java +++ b/scm-webapp/src/test/java/sonia/scm/update/repository/CopyMigrationStrategyTest.java @@ -91,14 +91,8 @@ class CopyMigrationStrategyTest { } private void assertDirectoriesEqual(Path targetDataDir, Path originalDataDir) { - Stream list = null; - try { - list = Files.list(originalDataDir); - } catch (IOException e) { - fail("could not read original directory", e); - } - list.forEach( - original -> { + try (Stream list = Files.list(originalDataDir)) { + list.forEach(original -> { Path expectedTarget = targetDataDir.resolve(original.getFileName()); assertThat(expectedTarget).exists(); if (Files.isDirectory(original)) { @@ -106,7 +100,9 @@ class CopyMigrationStrategyTest { } else { assertThat(expectedTarget).hasSameContentAs(original); } - } - ); + }); + } catch (IOException e) { + fail("could not read original directory", e); + } } } diff --git a/scm-webapp/src/test/java/sonia/scm/update/store/DifferentiateBetweenConfigAndConfigEntryUpdateStepTest.java b/scm-webapp/src/test/java/sonia/scm/update/store/DifferentiateBetweenConfigAndConfigEntryUpdateStepTest.java index 0eea1f959e..6ef4d0c629 100644 --- a/scm-webapp/src/test/java/sonia/scm/update/store/DifferentiateBetweenConfigAndConfigEntryUpdateStepTest.java +++ b/scm-webapp/src/test/java/sonia/scm/update/store/DifferentiateBetweenConfigAndConfigEntryUpdateStepTest.java @@ -29,6 +29,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import java.io.IOException; +import java.io.OutputStream; +import java.net.URISyntaxException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -43,15 +45,16 @@ import static org.assertj.core.api.Assertions.assertThat; class DifferentiateBetweenConfigAndConfigEntryUpdateStepTest { @Test - void shouldNotModifyConfigFile(@TempDir Path temp) throws IOException { + void shouldNotModifyConfigFile(@TempDir Path temp) throws IOException, URISyntaxException { Path configFile = temp.resolve("some.store.xml"); - copy( - getResource("sonia/scm/update/store/config_file.xml.content"), - newOutputStream(configFile)); - new DifferentiateBetweenConfigAndConfigEntryUpdateStep() {}.updateAllInDirectory(temp); + try (OutputStream os = newOutputStream(configFile)) { + copy(getResource("sonia/scm/update/store/config_file.xml.content"), os); - assertContent(configFile, "sonia/scm/update/store/config_file.xml.content"); + new DifferentiateBetweenConfigAndConfigEntryUpdateStep() {}.updateAllInDirectory(temp); + + assertContent(configFile, "sonia/scm/update/store/config_file.xml.content"); + } } @Test @@ -77,33 +80,35 @@ class DifferentiateBetweenConfigAndConfigEntryUpdateStepTest { } @Test - void shouldIgnoreFilesWithoutXmlSuffix(@TempDir Path temp) throws IOException { + void shouldIgnoreFilesWithoutXmlSuffix(@TempDir Path temp) throws IOException, URISyntaxException { Path otherFile = temp.resolve("some.other.file"); - copy( - getResource("sonia/scm/update/store/config_entry_file_without_mark.xml.content"), - newOutputStream(otherFile)); - new DifferentiateBetweenConfigAndConfigEntryUpdateStep() {}.updateAllInDirectory(temp); + try (OutputStream os = newOutputStream(otherFile)) { + copy(getResource("sonia/scm/update/store/config_entry_file_without_mark.xml.content"), os); - assertContent(otherFile, "sonia/scm/update/store/config_entry_file_without_mark.xml.content"); + new DifferentiateBetweenConfigAndConfigEntryUpdateStep() {}.updateAllInDirectory(temp); + + assertContent(otherFile, "sonia/scm/update/store/config_entry_file_without_mark.xml.content"); + } } @Test - void shouldHandleConfigEntryFile(@TempDir Path temp) throws IOException { + void shouldHandleConfigEntryFile(@TempDir Path temp) throws IOException, URISyntaxException { Path configFile = temp.resolve("some.store.xml"); - copy( - getResource("sonia/scm/update/store/config_entry_file_without_mark.xml.content"), - newOutputStream(configFile)); - new DifferentiateBetweenConfigAndConfigEntryUpdateStep() {}.updateAllInDirectory(temp); + try (OutputStream os = newOutputStream(configFile)) { + copy(getResource("sonia/scm/update/store/config_entry_file_without_mark.xml.content"), os); - assertThat(Files.readAllLines(configFile)) - .areAtLeastOne(new Condition<>(line -> line.contains(""), "line containing start tag with attribute")) - .endsWith(Files.readAllLines(Paths.get(getResource("sonia/scm/update/store/config_entry_file.xml.content").getFile())).toArray(new String[9])); + new DifferentiateBetweenConfigAndConfigEntryUpdateStep() {}.updateAllInDirectory(temp); + + assertThat(Files.readAllLines(configFile)) + .areAtLeastOne(new Condition<>(line -> line.contains(""), "line containing start tag with attribute")) + .endsWith(Files.readAllLines(Paths.get(getResource("sonia/scm/update/store/config_entry_file.xml.content").toURI())).toArray(new String[9])); + } } - private void assertContent(Path configFile, String expectedContentResource) { + private void assertContent(Path configFile, String expectedContentResource) throws URISyntaxException { assertThat(configFile) - .hasSameTextualContentAs(Paths.get(getResource(expectedContentResource).getFile())); + .hasSameTextualContentAs(Paths.get(getResource(expectedContentResource).toURI())); } }