Ignore non-XML files in data store directories

To prevent errors when trying to read all entries in a data entry store, this filters for XML files only and ignored other files or directories in the data directory.

Co-authored-by: Eduard Heimbuch<eduard.heimbuch@cloudogu.com>
Pushed-by: Rene Pfeuffer<rene.pfeuffer@cloudogu.com>
Pushed-by: Eduard Heimbuch<eduard.heimbuch@cloudogu.com>
Committed-by: René Pfeuffer<rene.pfeuffer@cloudogu.com>

gradle/changelog/read_only_xml_files_in_data_store.yaml

@@ -0,0 +1,2 @@
+- type: fixed
+  description: Ignore non-XML files in data store directories

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

@@ -75,7 +75,7 @@ public class JAXBConfigurationStore<T> extends AbstractStore<T> {
     return compute(
       () -> {
         if (configFile.exists()) {
-          return context.unmarshall(configFile);
+          return context.unmarshal(configFile);
         }
         return null;
       }

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

@@ -35,6 +35,7 @@ import javax.xml.bind.JAXBException;
 import javax.xml.bind.Marshaller;
 import java.io.File;
 import java.util.Map;
+import java.util.Objects;
 
 import static sonia.scm.store.CopyOnWrite.compute;
 
@@ -101,8 +102,10 @@ public class JAXBDataStore<T> extends FileBasedStore<T> implements DataStore<T>
 
     Builder<String, T> builder = ImmutableMap.builder();
 
-    for (File file : directory.listFiles()) {
-      builder.put(getId(file), read(file));
+    for (File file : Objects.requireNonNull(directory.listFiles())) {
+      if (file.isFile() && file.getName().endsWith(StoreConstants.FILE_EXTENSION)) {
+        builder.put(getId(file), read(file));
+      }
     }
 
     return builder.build();
@@ -120,7 +123,7 @@ public class JAXBDataStore<T> extends FileBasedStore<T> implements DataStore<T>
       compute(() -> {
         if (file.exists()) {
           LOG.trace("try to read {}", file);
-          return context.unmarshall(file);
+          return context.unmarshal(file);
         }
         return null;
       }).withLockedFileForRead(file)

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

@@ -24,6 +24,7 @@
 
 package sonia.scm.store;
 
+import lombok.extern.slf4j.Slf4j;
 import sonia.scm.xml.XmlStreams;
 
 import javax.xml.bind.JAXBContext;
@@ -37,6 +38,7 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicReference;
 
+@Slf4j
 final class TypedStoreContext<T> {
 
   private final JAXBContext jaxbContext;
@@ -65,7 +67,8 @@ final class TypedStoreContext<T> {
     }
   }
 
-  T unmarshall(File file) {
+  T unmarshal(File file) {
+    log.trace("unmarshal file {}", file);
     AtomicReference<T> ref = new AtomicReference<>();
     withUnmarshaller(unmarshaller -> {
       T value = parameters.getType().cast(unmarshaller.unmarshal(file));
@@ -75,6 +78,7 @@ final class TypedStoreContext<T> {
   }
 
   void marshal(Object object, File file) {
+    log.trace("marshal file {}", file);
     withMarshaller(marshaller -> marshaller.marshal(object, XmlStreams.createWriter(file)));
   }
 

scm-dao-xml/src/test/java/sonia/scm/store/JAXBDataStoreTest.java

@@ -25,11 +25,14 @@
 package sonia.scm.store;
 
 import org.junit.Test;
-import sonia.scm.cache.MapCache;
 import sonia.scm.repository.Repository;
 import sonia.scm.repository.RepositoryReadOnlyChecker;
 import sonia.scm.security.UUIDKeyGenerator;
 
+import java.io.File;
+import java.io.IOException;
+import java.util.Map;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.mockito.Mockito.mock;
@@ -88,4 +91,17 @@ public class JAXBDataStoreTest extends DataStoreTestBase {
     when(readOnlyChecker.isReadOnly(repository.getId())).thenReturn(true);
     getDataStore(StoreObject.class, repository).put("abc", new StoreObject("abc_value"));
   }
+
+  @Test
+  public void testGetAllWithNonXmlFile() throws IOException {
+    StoreObject obj1 = new StoreObject("test-1");
+
+    store.put("1", obj1);
+
+    new File(getTempDirectory(), "var/data/test/no-xml").createNewFile();
+
+    Map<String, StoreObject> map = store.getAll();
+
+    assertEquals(obj1, map.get("1"));
+  }
 }

scm-dao-xml/src/test/java/sonia/scm/store/TypedStoreContextTest.java

@@ -55,7 +55,7 @@ class TypedStoreContextTest {
 
     File file = tempDir.resolve("test.xml").toFile();
     context.marshal(new Sample("awesome"), file);
-    Sample sample = context.unmarshall(file);
+    Sample sample = context.unmarshal(file);
 
     assertThat(sample.value).isEqualTo("awesome");
   }
@@ -109,7 +109,7 @@ class TypedStoreContextTest {
 
     File file = tempDir.resolve("test.xml").toFile();
     context.marshal(new SampleWithAdapter("awesome"), file);
-    SampleWithAdapter sample = context.unmarshall(file);
+    SampleWithAdapter sample = context.unmarshal(file);
 
     // one ! should be added for marshal and one for unmarshal
     assertThat(sample.value).isEqualTo("awesome!!");

scm-test/src/main/java/sonia/scm/AbstractTestBase.java

@@ -243,4 +243,7 @@ public class AbstractTestBase
     subjectThreadState.bind();
   }
 
+  protected File getTempDirectory() {
+    return tempDirectory;
+  }
 }