Nemcio/SCM-Manager
1
0
Fork 0
mirror of https://github.com/scm-manager/scm-manager.git synced 2025-11-17 02:31:14 +01:00
Code Issues Packages Projects Releases Activity

Handle resources, never left left opened file handler on windows (#1857)

Browse Source 
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.
This commit is contained in:
StNekroman
2021-11-22 11:22:46 +02:00
committed by GitHub
parent 47842cc071
commit b09284f1f5
12 changed files with 150 additions and 147 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
scm-dao-xml/src/main/java/sonia/scm/repository/xml/PathDatabase.java
View File

@@ -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<String, Path> e : pathDatabase.entrySet()) {
writeRepository(writer, e.getKey(), e.getValue());
writeRepositoriesStart(writer, creationTime, lastModified);
for (Map.Entry<String, Path> 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();

6
scm-dao-xml/src/main/java/sonia/scm/store/FileStoreExporter.java
View File

@@ -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())

16
scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationEntryStore.java
View File

@@ -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<V> implements ConfigurationEntryStore<V
context.withUnmarshaller(u -> {
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<V> implements ConfigurationEntryStore<V
LOG.debug("store configuration to {}", file);
context.withMarshaller(m -> {
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<V> implements ConfigurationEntryStore<V
// value
JAXBElement<V> je = new JAXBElement<>(QName.valueOf(TAG_VALUE), type,
e.getValue());
e.getValue());
m.marshal(je, writer);

27
scm-dao-xml/src/main/java/sonia/scm/xml/XmlStreams.java
View File

@@ -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));
}
}

59
scm-dao-xml/src/test/java/sonia/scm/store/CopyOnWriteTest.java
View File

@@ -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);
}