From e98a6ff0930cb0a7433db3d8929a22d13a83d9e8 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Mon, 26 Jan 2026 15:51:32 +0000 Subject: [PATCH] Include filename of corrupt file in store exception --- gradle/changelog/error.yaml | 2 + .../file/JAXBConfigurationEntryStore.java | 137 +++++++++--------- .../scm/store/file/TypedStoreContext.java | 27 ++-- .../scm/store/file/TypedStoreContextTest.java | 26 ++-- 4 files changed, 105 insertions(+), 87 deletions(-) create mode 100644 gradle/changelog/error.yaml diff --git a/gradle/changelog/error.yaml b/gradle/changelog/error.yaml new file mode 100644 index 0000000000..bf58b8ca4e --- /dev/null +++ b/gradle/changelog/error.yaml @@ -0,0 +1,2 @@ +- type: fixed + description: Enhanced error message including filename of corrupt file diff --git a/scm-persistence/src/main/java/sonia/scm/store/file/JAXBConfigurationEntryStore.java b/scm-persistence/src/main/java/sonia/scm/store/file/JAXBConfigurationEntryStore.java index 9a246941b7..d025738a5a 100644 --- a/scm-persistence/src/main/java/sonia/scm/store/file/JAXBConfigurationEntryStore.java +++ b/scm-persistence/src/main/java/sonia/scm/store/file/JAXBConfigurationEntryStore.java @@ -123,92 +123,99 @@ class JAXBConfigurationEntryStore implements ConfigurationEntryStore { private void load() { LOG.debug("load configuration from {}", file); execute(() -> - context.withUnmarshaller(u -> { - try (AutoCloseableXMLReader reader = XmlStreams.createReader(file)) { + context.withUnmarshaller( + u -> { + try (AutoCloseableXMLReader reader = XmlStreams.createReader(file)) { - // configuration - reader.nextTag(); - - // entry start - reader.nextTag(); - - while (reader.isStartElement() && reader.getLocalName().equals(TAG_ENTRY)) { - - // read key + // configuration reader.nextTag(); - String key = reader.getElementText(); - - // read value + // entry start reader.nextTag(); - JAXBElement element = u.unmarshal(reader, type); + while (reader.isStartElement() && reader.getLocalName().equals(TAG_ENTRY)) { - if (!element.isNil()) { - V v = element.getValue(); - - LOG.trace("add element {} to configuration entry store", v); - - entries.put(key, v); - } else { - LOG.warn("could not unmarshall object of entry store"); - } - - // closed or new entry tag - if (reader.nextTag() == END_ELEMENT) { - - // fixed format, start new entry + // read key reader.nextTag(); + + String key = reader.getElementText(); + + // read value + reader.nextTag(); + + JAXBElement element = u.unmarshal(reader, type); + + if (!element.isNil()) { + V v = element.getValue(); + + LOG.trace("add element {} to configuration entry store", v); + + entries.put(key, v); + } else { + LOG.warn("could not unmarshall object of entry store"); + } + + // closed or new entry tag + if (reader.nextTag() == END_ELEMENT) { + + // fixed format, start new entry + reader.nextTag(); + } } } - } - })).withLockedFileForRead(file); + }, + file.getAbsoluteFile() + ) + ).withLockedFileForRead(file); } private void store() { LOG.debug("store configuration to {}", file); - context.withMarshaller(m -> { - m.setProperty(Marshaller.JAXB_FRAGMENT, Boolean.TRUE); + context.withMarshaller( + m -> { + m.setProperty(Marshaller.JAXB_FRAGMENT, Boolean.TRUE); - CopyOnWrite.withTemporaryFile( - temp -> { - try (AutoCloseableXMLWriter writer = XmlStreams.createWriter(temp)) { - writer.writeStartDocument(); + CopyOnWrite.withTemporaryFile( + temp -> { + try (AutoCloseableXMLWriter writer = XmlStreams.createWriter(temp)) { + writer.writeStartDocument(); - // configuration start - writer.writeStartElement(TAG_CONFIGURATION); - writer.writeAttribute("type", "config-entry"); + // configuration start + writer.writeStartElement(TAG_CONFIGURATION); + writer.writeAttribute("type", "config-entry"); - for (Entry e : entries.entrySet()) { + for (Entry e : entries.entrySet()) { - // entry start - writer.writeStartElement(TAG_ENTRY); + // entry start + writer.writeStartElement(TAG_ENTRY); - // key start - writer.writeStartElement(TAG_KEY); - writer.writeCharacters(e.getKey()); + // key start + writer.writeStartElement(TAG_KEY); + writer.writeCharacters(e.getKey()); - // key end - writer.writeEndElement(); - - // value - JAXBElement je = new JAXBElement<>(QName.valueOf(TAG_VALUE), type, - e.getValue()); - - m.marshal(je, writer); - - // entry end + // key end + writer.writeEndElement(); + + // value + JAXBElement je = new JAXBElement<>(QName.valueOf(TAG_VALUE), type, + e.getValue()); + + m.marshal(je, writer); + + // entry end + writer.writeEndElement(); + } + + // configuration end writer.writeEndElement(); + writer.writeEndDocument(); } - - // configuration end - writer.writeEndElement(); - writer.writeEndDocument(); - } - }, - file.toPath() - ); - }); + }, + file.toPath() + ); + }, + file.getAbsoluteFile() + ); } } diff --git a/scm-persistence/src/main/java/sonia/scm/store/file/TypedStoreContext.java b/scm-persistence/src/main/java/sonia/scm/store/file/TypedStoreContext.java index 3be80db167..36dfa3c0d8 100644 --- a/scm-persistence/src/main/java/sonia/scm/store/file/TypedStoreContext.java +++ b/scm-persistence/src/main/java/sonia/scm/store/file/TypedStoreContext.java @@ -64,33 +64,35 @@ final class TypedStoreContext { T unmarshal(File file) { log.trace("unmarshal file {}", file); AtomicReference ref = new AtomicReference<>(); - withUnmarshaller(unmarshaller -> { - T value = parameters.getType().cast(unmarshaller.unmarshal(file)); - ref.set(value); - }); + withUnmarshaller( + unmarshaller -> { + T value = parameters.getType().cast(unmarshaller.unmarshal(file)); + ref.set(value); + }, + file.getAbsoluteFile()); return ref.get(); } void marshal(Object object, File file) { log.trace("marshal file {}", file); - withMarshaller(marshaller -> marshaller.marshal(object, XmlStreams.createWriter(file))); + withMarshaller(marshaller -> marshaller.marshal(object, XmlStreams.createWriter(file)), file.getAbsoluteFile()); } - void withMarshaller(ThrowingConsumer consumer) { + void withMarshaller(ThrowingConsumer consumer, Object context) { Marshaller marshaller = createMarshaller(); - withClassLoader(consumer, marshaller); + withClassLoader(consumer, marshaller, context); } - void withUnmarshaller(ThrowingConsumer consumer) { + void withUnmarshaller(ThrowingConsumer consumer, Object context) { Unmarshaller unmarshaller = createUnmarshaller(); - withClassLoader(consumer, unmarshaller); + withClassLoader(consumer, unmarshaller, context); } Class getType() { return parameters.getType(); } - private void withClassLoader(ThrowingConsumer consumer, C consume) { + private void withClassLoader(ThrowingConsumer consumer, C consume, Object context) { ClassLoader contextClassLoader = null; Optional classLoader = parameters.getClassLoader(); if (classLoader.isPresent()) { @@ -100,7 +102,7 @@ final class TypedStoreContext { try { consumer.consume(consume); } catch (Exception e) { - throw new StoreException("failure during marshalling/unmarshalling", e); + throw new StoreException("failure during marshalling/unmarshalling '" + context + "'", e); } finally { if (contextClassLoader != null) { Thread.currentThread().setContextClassLoader(contextClassLoader); @@ -135,7 +137,8 @@ final class TypedStoreContext { @FunctionalInterface interface ThrowingConsumer { - @SuppressWarnings("java:S112") // we need to throw Exception here + @SuppressWarnings("java:S112") + // we need to throw Exception here void consume(T item) throws Exception; } diff --git a/scm-persistence/src/test/java/sonia/scm/store/file/TypedStoreContextTest.java b/scm-persistence/src/test/java/sonia/scm/store/file/TypedStoreContextTest.java index 1acfe370ce..27dd8cf1fb 100644 --- a/scm-persistence/src/test/java/sonia/scm/store/file/TypedStoreContextTest.java +++ b/scm-persistence/src/test/java/sonia/scm/store/file/TypedStoreContextTest.java @@ -59,16 +59,19 @@ class TypedStoreContextTest { File file = tempDir.resolve("test.xml").toFile(); - context.withMarshaller(marshaller -> { - marshaller.marshal(new Sample("wow"), file); - }); + context.withMarshaller( + marshaller -> { + marshaller.marshal(new Sample("wow"), file); + }, + file); AtomicReference ref = new AtomicReference<>(); - context.withUnmarshaller(unmarshaller -> { - Sample sample = (Sample) unmarshaller.unmarshal(file); - ref.set(sample); - }); + context.withUnmarshaller( + unmarshaller -> { + Sample sample = (Sample) unmarshaller.unmarshal(file); + ref.set(sample); + }, file); assertThat(ref.get().value).isEqualTo("wow"); } @@ -85,9 +88,12 @@ class TypedStoreContextTest { TypedStoreContext context = TypedStoreContext.of(params); AtomicReference ref = new AtomicReference<>(); - context.withMarshaller(marshaller -> { - ref.set(Thread.currentThread().getContextClassLoader()); - }); + context.withMarshaller( + marshaller -> { + ref.set(Thread.currentThread().getContextClassLoader()); + }, + "nothing" + ); assertThat(ref.get()).isSameAs(classLoader); assertThat(Thread.currentThread().getContextClassLoader()).isSameAs(contextClassLoader);