Merge pull request #586 from focus-shift/improve-xmlutil-performane

XML unmarshal: performance improvements - static XMLUtil with static converters
focus-shift · Dec 3, 2024 · d547336 · d547336
2 parents 0d0f6ca + 4789983
commit d547336
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 124 deletions.
diff --git a/...ay-jackson/src/main/java/de/focus_shift/jollyday/jackson/JacksonConfigurationService.java b/...ay-jackson/src/main/java/de/focus_shift/jollyday/jackson/JacksonConfigurationService.java
@@ -10,7 +10,7 @@
 
 public class JacksonConfigurationService implements ConfigurationService {
 
-  private final XMLUtil xmlUtil = new XMLUtil();
+  private static final XMLUtil xmlUtil = new XMLUtil();
 
   @Override
   public Configuration getConfiguration(ManagerParameter parameter) {

diff --git a/jollyday-jackson/src/main/java/de/focus_shift/jollyday/jackson/XMLUtil.java b/jollyday-jackson/src/main/java/de/focus_shift/jollyday/jackson/XMLUtil.java
@@ -4,14 +4,12 @@
 import com.fasterxml.jackson.dataformat.xml.XmlMapper;
 import de.focus_shift.jollyday.jackson.mapping.Configuration;
 import de.focus_shift.jollyday.jackson.mapping.Month;
-import de.focus_shift.jollyday.jackson.mapping.Weekday;
 
 import java.io.InputStream;
-import java.time.DayOfWeek;
 
 public class XMLUtil {
 
-  private final JacksonMapperCreator mapper = new JacksonMapperCreator();
+  private static final XmlMapper mapper = new JacksonMapperCreator().create();
 
   /**
    * Unmarshalls the configuration from the stream. Uses <code>jackson</code> for
@@ -22,22 +20,12 @@ public class XMLUtil {
    */
   public Configuration unmarshallConfiguration(InputStream stream) {
     try {
-      return mapper.create().readValue(stream, Configuration.class);
+      return mapper.readValue(stream, Configuration.class);
     } catch (Exception e) {
       throw new IllegalStateException("Cannot parse holidays XML file.", e);
     }
   }
 
-  /**
-   * Returns the {@link DayOfWeek} equivalent for the given weekday.
-   *
-   * @param weekday a {@link Weekday} object.
-   * @return a DayOfWeek instance.
-   */
-  public final DayOfWeek getWeekday(Weekday weekday) {
-    return DayOfWeek.valueOf(weekday.value());
-  }
-
   /**
    * Returns the value for the given month.
    *
@@ -48,8 +36,8 @@ public int getMonth(Month month) {
     return month.ordinal() + 1;
   }
 
-  public static class JacksonMapperCreator {
-    public XmlMapper create() {
+  private static class JacksonMapperCreator {
+    private XmlMapper create() {
       final XmlMapper mapper = new XmlMapper();
       mapper.setPropertyNamingStrategy(PropertyNamingStrategies.UPPER_CAMEL_CASE);
       return mapper;

diff --git a/jollyday-jackson/src/test/java/de/focus_shift/jollyday/jackson/test/XMLUtilTest.java b/jollyday-jackson/src/test/java/de/focus_shift/jollyday/jackson/test/XMLUtilTest.java
@@ -1,17 +1,10 @@
 package de.focus_shift.jollyday.jackson.test;
 
-import com.fasterxml.jackson.databind.PropertyNamingStrategies;
-import com.fasterxml.jackson.dataformat.xml.XmlMapper;
 import de.focus_shift.jollyday.jackson.XMLUtil;
 import de.focus_shift.jollyday.jackson.mapping.Configuration;
-import de.focus_shift.jollyday.jackson.mapping.Fixed;
-import de.focus_shift.jollyday.jackson.mapping.Holidays;
-import de.focus_shift.jollyday.jackson.mapping.Month;
-import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
 
-import java.io.ByteArrayInputStream;
 import java.io.InputStream;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -21,32 +14,9 @@ class XMLUtilTest {
   @ParameterizedTest
   @ValueSource(strings = {"Holidays_at.xml", "Holidays_de.xml", "Holidays_gb.xml", "Holidays_ua.xml", "Holidays_tr.xml", "Holidays_za.xml"})
   void unmarshalRealResource(String holidayFileName) {
-    final XMLUtil xmlUtil = new XMLUtil();
+    final XMLUtil sut = new XMLUtil();
     final InputStream inputStream = getClass().getClassLoader().getResourceAsStream("holidays/" + holidayFileName);
-    final Configuration configuration = xmlUtil.unmarshallConfiguration(inputStream);
+    final Configuration configuration = sut.unmarshallConfiguration(inputStream);
     assertThat(configuration.getHolidays()).isNotNull();
   }
-
-  @Test
-  void remarshalExampleResource() throws Exception {
-
-    final Configuration configuration = new Configuration();
-    configuration.setHolidays(new Holidays());
-
-    final Fixed fixed = new Fixed();
-    fixed.setValidFrom(1234);
-    fixed.setValidTo(4444);
-    fixed.setDay(4);
-    fixed.setMonth(Month.JANUARY);
-    configuration.getHolidays().getFixed().add(fixed);
-
-    final XmlMapper mapper = new XmlMapper();
-    mapper.setPropertyNamingStrategy(PropertyNamingStrategies.UPPER_CAMEL_CASE);
-    final String xml = mapper.writeValueAsString(configuration);
-
-    final XMLUtil xmlUtil = new XMLUtil();
-    final Configuration newConfiguration = xmlUtil.unmarshallConfiguration(new ByteArrayInputStream(xml.getBytes()));
-    assertThat(newConfiguration.getHolidays()).isNotNull();
-    assertThat(newConfiguration.getHolidays().getFixed()).isNotEmpty();
-  }
 }
diff --git a/jollyday-jaxb/src/main/java/de/focus_shift/jollyday/jaxb/JaxbConfigurationService.java b/jollyday-jaxb/src/main/java/de/focus_shift/jollyday/jaxb/JaxbConfigurationService.java
@@ -10,7 +10,7 @@
 
 public class JaxbConfigurationService implements ConfigurationService {
 
-  private final XMLUtil xmlUtil = new XMLUtil();
+  private static final XMLUtil xmlUtil = new XMLUtil();
 
   @Override
   public Configuration getConfiguration(ManagerParameter parameter) {

diff --git a/jollyday-jaxb/src/main/java/de/focus_shift/jollyday/jaxb/XMLUtil.java b/jollyday-jaxb/src/main/java/de/focus_shift/jollyday/jaxb/XMLUtil.java
@@ -4,7 +4,6 @@
 import de.focus_shift.jollyday.jaxb.mapping.Configuration;
 import de.focus_shift.jollyday.jaxb.mapping.Month;
 import de.focus_shift.jollyday.jaxb.mapping.ObjectFactory;
-import de.focus_shift.jollyday.jaxb.mapping.Weekday;
 import jakarta.xml.bind.JAXBContext;
 import jakarta.xml.bind.JAXBElement;
 import jakarta.xml.bind.JAXBException;
@@ -13,7 +12,6 @@
 import org.slf4j.LoggerFactory;
 
 import java.io.InputStream;
-import java.time.DayOfWeek;
 
 public class XMLUtil {
 
@@ -24,7 +22,7 @@ public class XMLUtil {
 
   private static final Logger LOG = LoggerFactory.getLogger(XMLUtil.class);
 
-  private JAXBContextCreator contextCreator = new JAXBContextCreator();
+  private static final JAXBContext jaxbContext = new JAXBContextCreator().create();
 
   /**
    * Unmarshalls the configuration from the stream. Uses <code>JAXB</code> for
@@ -39,39 +37,14 @@ public Configuration unmarshallConfiguration(InputStream stream) {
     }
 
     try {
-      final JAXBContext ctx = createJAXBContext();
-      final Unmarshaller um = ctx.createUnmarshaller();
+      final Unmarshaller um = jaxbContext.createUnmarshaller();
       @SuppressWarnings("unchecked") final JAXBElement<Configuration> jaxbElement = (JAXBElement<Configuration>) um.unmarshal(stream);
       return jaxbElement.getValue();
     } catch (JAXBException exception) {
       throw new IllegalStateException("Cannot parse holidays XML file.", exception);
     }
   }
 
-  private JAXBContext createJAXBContext() throws JAXBException {
-    JAXBContext ctx = null;
-    try {
-      ctx = contextCreator.create(XMLUtil.PACKAGE, ClassLoadingUtil.getClassloader());
-    } catch (JAXBException e) {
-      LOG.warn("Could not create JAXB context using the current threads context classloader. Falling back to ObjectFactory class classloader.");
-    }
-
-    if (ctx == null) {
-      ctx = contextCreator.create(XMLUtil.PACKAGE, ObjectFactory.class.getClassLoader());
-    }
-    return ctx;
-  }
-
-  /**
-   * Returns the {@link DayOfWeek} equivalent for the given weekday.
-   *
-   * @param weekday a {@link Weekday} object.
-   * @return a DayOfWeek instance.
-   */
-  public final DayOfWeek getWeekday(Weekday weekday) {
-    return DayOfWeek.valueOf(weekday.value());
-  }
-
   /**
    * Returns the value for the given month.
    *
@@ -82,9 +55,28 @@ public int getMonth(Month month) {
     return month.ordinal() + 1;
   }
 
-  public static class JAXBContextCreator {
-    public JAXBContext create(String packageName, ClassLoader classLoader) throws JAXBException {
-      return JAXBContext.newInstance(packageName, classLoader);
+  private static class JAXBContextCreator {
+    private JAXBContext create() {
+      return createJAXBContext();
+    }
+
+    private static JAXBContext createJAXBContext() {
+      JAXBContext ctx = null;
+      try {
+        ctx = JAXBContext.newInstance(XMLUtil.PACKAGE, ClassLoadingUtil.getClassloader());
+      } catch (JAXBException e) {
+        LOG.warn("Could not create JAXB context using the current classloader from ClassLoadingUtil. Falling back to ObjectFactory class classloader.");
+      }
+
+      if (ctx == null) {
+        try {
+          ctx = JAXBContext.newInstance(XMLUtil.PACKAGE, ObjectFactory.class.getClassLoader());
+        } catch (JAXBException exception) {
+          throw new IllegalStateException("Could not create JAXB context using ObjectFactory classloader.", exception);
+        }
+      }
+
+      return ctx;
     }
   }
 }
diff --git a/jollyday-jaxb/src/test/java/de/focus_shift/jollyday/jaxb/test/XMLUtilTest.java b/jollyday-jaxb/src/test/java/de/focus_shift/jollyday/jaxb/test/XMLUtilTest.java
@@ -1,60 +1,30 @@
 package de.focus_shift.jollyday.jaxb.test;
 
-import de.focus_shift.jollyday.core.spi.Configuration;
 import de.focus_shift.jollyday.jaxb.XMLUtil;
-import jakarta.xml.bind.JAXBContext;
-import jakarta.xml.bind.JAXBElement;
-import jakarta.xml.bind.JAXBException;
-import jakarta.xml.bind.Unmarshaller;
+import de.focus_shift.jollyday.jaxb.mapping.Configuration;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.ExtendWith;
-import org.mockito.InjectMocks;
-import org.mockito.Mock;
-import org.mockito.junit.jupiter.MockitoExtension;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
-import java.io.IOException;
 import java.io.InputStream;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static org.mockito.Mockito.any;
-import static org.mockito.Mockito.eq;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 
-@ExtendWith(MockitoExtension.class)
 class XMLUtilTest {
 
-  @Mock
-  private XMLUtil.JAXBContextCreator contextCreator;
-  @Mock
-  private InputStream inputStream;
-
-  @InjectMocks
-  private final XMLUtil xmlUtil = new XMLUtil();
+  private final XMLUtil sut = new XMLUtil();
 
   @Test
   void testUnmarshallConfigurationNullCheck() {
-    assertThatThrownBy(() -> xmlUtil.unmarshallConfiguration(null)).isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> sut.unmarshallConfiguration(null)).isInstanceOf(IllegalArgumentException.class);
   }
 
-  @Test
-  void testUnmarshallConfigurationException() throws IOException, JAXBException {
-    when(contextCreator.create(eq("de.focus_shift.jollyday.jaxb.mapping"), any(ClassLoader.class))).thenThrow(new JAXBException("")).thenThrow(new JAXBException(""));
-    assertThatThrownBy(() -> xmlUtil.unmarshallConfiguration(inputStream)).isInstanceOf(IllegalStateException.class);
-    verify(inputStream, never()).close();
-  }
-
-  @Test
-  void testUnmarshallConfiguration() throws JAXBException {
-    final JAXBContext ctx = mock(JAXBContext.class);
-    final Unmarshaller unmarshaller = mock(Unmarshaller.class);
-    @SuppressWarnings("unchecked") final JAXBElement<Configuration> element = mock(JAXBElement.class);
-    when(contextCreator.create(eq("de.focus_shift.jollyday.jaxb.mapping"), any(ClassLoader.class))).thenReturn(null).thenReturn(ctx);
-    when(ctx.createUnmarshaller()).thenReturn(unmarshaller);
-    when(unmarshaller.unmarshal(inputStream)).thenReturn(element);
-    xmlUtil.unmarshallConfiguration(inputStream);
-    verify(element).getValue();
+  @ParameterizedTest
+  @ValueSource(strings = {"Holidays_at.xml", "Holidays_de.xml", "Holidays_gb.xml", "Holidays_ua.xml", "Holidays_tr.xml", "Holidays_za.xml"})
+  void unmarshalRealResource(String holidayFileName) {
+    final InputStream inputStream = getClass().getClassLoader().getResourceAsStream("holidays/" + holidayFileName);
+    final Configuration configuration = sut.unmarshallConfiguration(inputStream);
+    assertThat(configuration.getHolidays()).isNotNull();
   }
 }