diff --git a/gradle/changelog/read_only_xml_files_in_data_store.yaml b/gradle/changelog/read_only_xml_files_in_data_store.yaml new file mode 100644 index 0000000000..8990fb17ab --- /dev/null +++ b/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 diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationStore.java b/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationStore.java index 2fdbcd370d..389d2297bd 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationStore.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationStore.java @@ -75,7 +75,7 @@ public class JAXBConfigurationStore extends AbstractStore { return compute( () -> { if (configFile.exists()) { - return context.unmarshall(configFile); + return context.unmarshal(configFile); } return null; } diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBDataStore.java b/scm-dao-xml/src/main/java/sonia/scm/store/JAXBDataStore.java index b0df8fd5a9..5c4159f68b 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBDataStore.java +++ b/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 extends FileBasedStore implements DataStore Builder 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 extends FileBasedStore implements DataStore compute(() -> { if (file.exists()) { LOG.trace("try to read {}", file); - return context.unmarshall(file); + return context.unmarshal(file); } return null; }).withLockedFileForRead(file) diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/TypedStoreContext.java b/scm-dao-xml/src/main/java/sonia/scm/store/TypedStoreContext.java index fe4306ff5d..05f8644f01 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/TypedStoreContext.java +++ b/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 { private final JAXBContext jaxbContext; @@ -65,7 +67,8 @@ final class TypedStoreContext { } } - T unmarshall(File file) { + T unmarshal(File file) { + log.trace("unmarshal file {}", file); AtomicReference ref = new AtomicReference<>(); withUnmarshaller(unmarshaller -> { T value = parameters.getType().cast(unmarshaller.unmarshal(file)); @@ -75,6 +78,7 @@ final class TypedStoreContext { } void marshal(Object object, File file) { + log.trace("marshal file {}", file); withMarshaller(marshaller -> marshaller.marshal(object, XmlStreams.createWriter(file))); } diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBDataStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBDataStoreTest.java index ced640a628..08d2194dc0 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBDataStoreTest.java +++ b/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 map = store.getAll(); + + assertEquals(obj1, map.get("1")); + } } diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/TypedStoreContextTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/TypedStoreContextTest.java index 0829d7af69..d937a88c32 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/TypedStoreContextTest.java +++ b/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!!"); diff --git a/scm-test/src/main/java/sonia/scm/AbstractTestBase.java b/scm-test/src/main/java/sonia/scm/AbstractTestBase.java index e6b5e1a305..4dfa974189 100644 --- a/scm-test/src/main/java/sonia/scm/AbstractTestBase.java +++ b/scm-test/src/main/java/sonia/scm/AbstractTestBase.java @@ -243,4 +243,7 @@ public class AbstractTestBase subjectThreadState.bind(); } + protected File getTempDirectory() { + return tempDirectory; + } }