Autocloseable streams in XML DB (#1868)

Introduce autocloseable streams for file handling in xml database module.

gradle/changelog/windows_stream_handling.yaml

@@ -0,0 +1,4 @@
+- type: fixed
+  description: Closing of file streams ([#1857](https://github.com/scm-manager/scm-manager/pull/1857) and [#1868](https://github.com/scm-manager/scm-manager/pull/1868))
+- type: fixed
+  description: Exit of retry loop for deletion of files ([#1857](https://github.com/scm-manager/scm-manager/pull/1857)  and [#1868](https://github.com/scm-manager/scm-manager/pull/1868))

scm-core/src/test/java/sonia/scm/BaseDirectoryTest.java

@@ -28,7 +28,6 @@ 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;

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

@@ -29,16 +29,15 @@ import org.slf4j.LoggerFactory;
 import sonia.scm.ContextEntry;
 import sonia.scm.repository.InternalRepositoryException;
 import sonia.scm.store.CopyOnWrite;
-import sonia.scm.xml.IndentXMLStreamWriter;
 import sonia.scm.xml.XmlStreams;
+import sonia.scm.xml.XmlStreams.AutoCloseableXMLReader;
+import sonia.scm.xml.XmlStreams.AutoCloseableXMLWriter;
 
+import javax.xml.stream.XMLStreamConstants;
 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;
@@ -70,24 +69,22 @@ class PathDatabase {
 
     CopyOnWrite.withTemporaryFile(
       temp -> {
-        try (Writer ioWriter = Files.newBufferedWriter(temp, StandardCharsets.UTF_8)) {
-          try (IndentXMLStreamWriter writer = XmlStreams.createWriter(ioWriter)) {
-            writer.writeStartDocument(ENCODING, VERSION);
+        try (AutoCloseableXMLWriter writer = XmlStreams.createWriter(temp)) {
+          writer.writeStartDocument(ENCODING, VERSION);
 
-            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
-            );
+          writeRepositoriesStart(writer, creationTime, lastModified);
+          for (Map.Entry<String, Path> e : pathDatabase.entrySet()) {
+            writeRepository(writer, e.getKey(), e.getValue());
           }
+          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
@@ -125,14 +122,12 @@ class PathDatabase {
 
   void read(OnRepositories onRepositories, OnRepository onRepository) {
     LOG.trace("read repository path database from {}", storePath);
-    XMLStreamReader reader = null;
-    try (Reader inputReader = Files.newBufferedReader(storePath, StandardCharsets.UTF_8)) {
-      reader = XmlStreams.createReader(inputReader);
+    try (AutoCloseableXMLReader reader = XmlStreams.createReader(storePath)) {
 
       while (reader.hasNext()) {
         int eventType = reader.next();
 
-        if (eventType == XMLStreamReader.START_ELEMENT) {
+        if (eventType == XMLStreamConstants.START_ELEMENT) {
           String element = reader.getLocalName();
           if (ELEMENT_REPOSITORIES.equals(element)) {
             readRepositories(reader, onRepositories);
@@ -147,8 +142,6 @@ class PathDatabase {
         "failed to read repository path database",
         ex
       );
-    } finally {
-      XmlStreams.close(reader);
     }
   }
 

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

@@ -30,13 +30,11 @@ import sonia.scm.repository.Repository;
 import sonia.scm.repository.RepositoryLocationResolver;
 import sonia.scm.repository.api.ExportFailedException;
 import sonia.scm.xml.XmlStreams;
+import sonia.scm.xml.XmlStreams.AutoCloseableXMLReader;
 
 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;
@@ -136,9 +134,7 @@ public class FileStoreExporter implements StoreExporter {
   }
 
   private StoreType determineConfigType(Path storePath) {
-    XMLStreamReader reader = null;
-    try (Reader inputReader = Files.newBufferedReader(storePath, StandardCharsets.UTF_8)) {
-      reader = XmlStreams.createReader(inputReader);
+    try (AutoCloseableXMLReader reader = XmlStreams.createReader(storePath)) {
       reader.nextTag();
       if (
         "configuration".equals(reader.getLocalName())
@@ -150,8 +146,6 @@ public class FileStoreExporter implements StoreExporter {
       }
     } catch (XMLStreamException | IOException e) {
       throw new ExportFailedException(noContext(), "Failed to read store file " + storePath, e);
-    } finally {
-      XmlStreams.close(reader);
     }
   }
 }

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

@@ -29,18 +29,14 @@ import com.google.common.collect.Maps;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import sonia.scm.security.KeyGenerator;
-import sonia.scm.xml.IndentXMLStreamWriter;
 import sonia.scm.xml.XmlStreams;
+import sonia.scm.xml.XmlStreams.AutoCloseableXMLReader;
+import sonia.scm.xml.XmlStreams.AutoCloseableXMLWriter;
 
 import javax.xml.bind.JAXBElement;
 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;
@@ -141,10 +137,7 @@ public class JAXBConfigurationEntryStore<V> implements ConfigurationEntryStore<V
     LOG.debug("load configuration from {}", file);
 
     context.withUnmarshaller(u -> {
-      XMLStreamReader reader = null;
-
-      try (Reader inputReader = Files.newBufferedReader(file.toPath(), StandardCharsets.UTF_8)) {
-        reader = XmlStreams.createReader(inputReader);
+      try (AutoCloseableXMLReader reader = XmlStreams.createReader(file)) {
 
         // configuration
         reader.nextTag();
@@ -181,8 +174,6 @@ public class JAXBConfigurationEntryStore<V> implements ConfigurationEntryStore<V
             reader.nextTag();
           }
         }
-      } finally {
-        XmlStreams.close(reader);
       }
     });
   }
@@ -191,12 +182,11 @@ 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 (Writer ioWriter = Files.newBufferedWriter(temp, StandardCharsets.UTF_8);
-               IndentXMLStreamWriter writer = XmlStreams.createWriter(ioWriter)) {
+          try (AutoCloseableXMLWriter writer = XmlStreams.createWriter(temp)) {
             writer.writeStartDocument();
 
             // configuration start
@@ -217,7 +207,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);
 

scm-dao-xml/src/main/java/sonia/scm/xml/XmlStreams.java

@@ -27,13 +27,21 @@ package sonia.scm.xml;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.xml.namespace.NamespaceContext;
 import javax.xml.stream.XMLInputFactory;
 import javax.xml.stream.XMLOutputFactory;
 import javax.xml.stream.XMLStreamException;
 import javax.xml.stream.XMLStreamReader;
 import javax.xml.stream.XMLStreamWriter;
+import javax.xml.stream.util.StreamReaderDelegate;
+import java.io.Closeable;
+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 {
 
@@ -62,13 +70,226 @@ public final class XmlStreams {
     }
   }
 
-  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 AutoCloseableXMLReader createReader(Path path) throws IOException, XMLStreamException {
+    return createReader(Files.newBufferedReader(path, StandardCharsets.UTF_8));
   }
 
-  public static IndentXMLStreamWriter createWriter(Writer writer) throws XMLStreamException {
-    return new IndentXMLStreamWriter(XMLOutputFactory.newFactory().createXMLStreamWriter(writer));
+  public static AutoCloseableXMLReader createReader(File file) throws IOException, XMLStreamException {
+    return createReader(file.toPath());
+  }
+
+  private static AutoCloseableXMLReader createReader(Reader reader) throws XMLStreamException {
+    XMLInputFactory factory = XMLInputFactory.newInstance();
+    factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE);
+    XMLStreamReader xmlStreamReader = factory.createXMLStreamReader(reader);
+    return new AutoCloseableXMLReader(xmlStreamReader, reader);
+  }
+
+  public static AutoCloseableXMLWriter createWriter(Path path) throws IOException, XMLStreamException {
+    return createWriter(Files.newBufferedWriter(path, StandardCharsets.UTF_8));
+  }
+
+  public static AutoCloseableXMLWriter createWriter(File file) throws IOException, XMLStreamException {
+    return createWriter(file.toPath());
+  }
+
+  private static AutoCloseableXMLWriter createWriter(Writer writer) throws XMLStreamException {
+    IndentXMLStreamWriter indentXMLStreamWriter = new IndentXMLStreamWriter(XMLOutputFactory.newFactory().createXMLStreamWriter(writer));
+    return new AutoCloseableXMLWriter(indentXMLStreamWriter, writer);
+  }
+
+  public static final class AutoCloseableXMLReader extends StreamReaderDelegate implements AutoCloseable {
+
+    private final Closeable closeable;
+
+    public AutoCloseableXMLReader(XMLStreamReader delegate, Closeable closeable) {
+      super(delegate);
+      this.closeable = closeable;
+    }
+
+    @Override
+    public void close() throws XMLStreamException {
+      super.close();
+      try {
+        closeable.close();
+      } catch (IOException e) {
+        throw new XMLStreamException("failed to close nested reader", e);
+      }
+    }
+  }
+
+  public static final class AutoCloseableXMLWriter implements XMLStreamWriter, AutoCloseable {
+    private final XMLStreamWriter delegate;
+    private final Closeable closeable;
+
+    public AutoCloseableXMLWriter(XMLStreamWriter delegate, Closeable closeable) {
+      this.delegate = delegate;
+      this.closeable = closeable;
+    }
+
+    @Override
+    public void writeStartElement(String localName) throws XMLStreamException {
+      delegate.writeStartElement(localName);
+    }
+
+    @Override
+    public void writeStartElement(String namespaceURI, String localName) throws XMLStreamException {
+      delegate.writeStartElement(namespaceURI, localName);
+    }
+
+    @Override
+    public void writeStartElement(String prefix, String localName, String namespaceURI) throws XMLStreamException {
+      delegate.writeStartElement(prefix, localName, namespaceURI);
+    }
+
+    @Override
+    public void writeEmptyElement(String namespaceURI, String localName) throws XMLStreamException {
+      delegate.writeEmptyElement(namespaceURI, localName);
+    }
+
+    @Override
+    public void writeEmptyElement(String prefix, String localName, String namespaceURI) throws XMLStreamException {
+      delegate.writeEmptyElement(prefix, localName, namespaceURI);
+    }
+
+    @Override
+    public void writeEmptyElement(String localName) throws XMLStreamException {
+      delegate.writeEmptyElement(localName);
+    }
+
+    @Override
+    public void writeEndElement() throws XMLStreamException {
+      delegate.writeEndElement();
+    }
+
+    @Override
+    public void writeEndDocument() throws XMLStreamException {
+      delegate.writeEndDocument();
+    }
+
+    @Override
+    public void close() throws XMLStreamException {
+      delegate.close();
+      try {
+        closeable.close();
+      } catch (IOException e) {
+        throw new XMLStreamException("failed to close nested writer", e);
+      }
+    }
+
+    @Override
+    public void flush() throws XMLStreamException {
+      delegate.flush();
+    }
+
+    @Override
+    public void writeAttribute(String localName, String value) throws XMLStreamException {
+      delegate.writeAttribute(localName, value);
+    }
+
+    @Override
+    public void writeAttribute(String prefix, String namespaceURI, String localName, String value) throws XMLStreamException {
+      delegate.writeAttribute(prefix, namespaceURI, localName, value);
+    }
+
+    @Override
+    public void writeAttribute(String namespaceURI, String localName, String value) throws XMLStreamException {
+      delegate.writeAttribute(namespaceURI, localName, value);
+    }
+
+    @Override
+    public void writeNamespace(String prefix, String namespaceURI) throws XMLStreamException {
+      delegate.writeNamespace(prefix, namespaceURI);
+    }
+
+    @Override
+    public void writeDefaultNamespace(String namespaceURI) throws XMLStreamException {
+      delegate.writeDefaultNamespace(namespaceURI);
+    }
+
+    @Override
+    public void writeComment(String data) throws XMLStreamException {
+      delegate.writeComment(data);
+    }
+
+    @Override
+    public void writeProcessingInstruction(String target) throws XMLStreamException {
+      delegate.writeProcessingInstruction(target);
+    }
+
+    @Override
+    public void writeProcessingInstruction(String target, String data) throws XMLStreamException {
+      delegate.writeProcessingInstruction(target, data);
+    }
+
+    @Override
+    public void writeCData(String data) throws XMLStreamException {
+      delegate.writeCData(data);
+    }
+
+    @Override
+    public void writeDTD(String dtd) throws XMLStreamException {
+      delegate.writeDTD(dtd);
+    }
+
+    @Override
+    public void writeEntityRef(String name) throws XMLStreamException {
+      delegate.writeEntityRef(name);
+    }
+
+    @Override
+    public void writeStartDocument() throws XMLStreamException {
+      delegate.writeStartDocument();
+    }
+
+    @Override
+    public void writeStartDocument(String version) throws XMLStreamException {
+      delegate.writeStartDocument(version);
+    }
+
+    @Override
+    public void writeStartDocument(String encoding, String version) throws XMLStreamException {
+      delegate.writeStartDocument(encoding, version);
+    }
+
+    @Override
+    public void writeCharacters(String text) throws XMLStreamException {
+      delegate.writeCharacters(text);
+    }
+
+    @Override
+    public void writeCharacters(char[] text, int start, int len) throws XMLStreamException {
+      delegate.writeCharacters(text, start, len);
+    }
+
+    @Override
+    public String getPrefix(String uri) throws XMLStreamException {
+      return delegate.getPrefix(uri);
+    }
+
+    @Override
+    public void setPrefix(String prefix, String uri) throws XMLStreamException {
+      delegate.setPrefix(prefix, uri);
+    }
+
+    @Override
+    public void setDefaultNamespace(String uri) throws XMLStreamException {
+      delegate.setDefaultNamespace(uri);
+    }
+
+    @Override
+    public void setNamespaceContext(NamespaceContext context) throws XMLStreamException {
+      delegate.setNamespaceContext(context);
+    }
+
+    @Override
+    public NamespaceContext getNamespaceContext() {
+      return delegate.getNamespaceContext();
+    }
+
+    @Override
+    public Object getProperty(String name) throws IllegalArgumentException {
+      return delegate.getProperty(name);
+    }
   }
 }

scm-webapp/src/main/java/sonia/scm/update/store/DifferentiateBetweenConfigAndConfigEntryUpdateStep.java

@@ -32,12 +32,12 @@ import sonia.scm.migration.UpdateException;
 import sonia.scm.store.CopyOnWrite;
 import sonia.scm.version.Version;
 import sonia.scm.xml.XmlStreams;
+import sonia.scm.xml.XmlStreams.AutoCloseableXMLReader;
 
 import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.stream.XMLStreamException;
-import javax.xml.stream.XMLStreamReader;
 import javax.xml.transform.TransformerException;
 import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.dom.DOMSource;
@@ -45,8 +45,6 @@ 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;
@@ -75,9 +73,7 @@ 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 inputReader = Files.newBufferedReader(potentialFile, StandardCharsets.UTF_8)) {
-      reader = XmlStreams.createReader(inputReader);
+    try (AutoCloseableXMLReader reader = XmlStreams.createReader(potentialFile)) {
       reader.nextTag();
       if ("configuration".equals(reader.getLocalName())) {
         reader.nextTag();
@@ -87,8 +83,6 @@ abstract class DifferentiateBetweenConfigAndConfigEntryUpdateStep {
       }
     } catch (XMLStreamException | IOException e) {
       throw new UpdateException("Error reading file " + potentialFile, e);
-    } finally {
-      XmlStreams.close(reader);
     }
     return false;
   }
@@ -106,12 +100,11 @@ abstract class DifferentiateBetweenConfigAndConfigEntryUpdateStep {
   }
 
   private void writeXmlDocument(Document configEntryDocument, Path temporaryFile) throws TransformerException {
+    TransformerFactory factory = TransformerFactory.newInstance();
+    factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+    factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
+    DOMSource domSource = new DOMSource(configEntryDocument);
     try (OutputStream os = Files.newOutputStream(temporaryFile)) {
-      final TransformerFactory factory = TransformerFactory.newInstance();
-      factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
-      factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
-      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);