Merge pull request #1710 from drewwills/XmlUtilitiesImpl-race-condition

fix(startup): fix a race condition with XmlUtilitiesImpl that has become a problem lately
uPortal-Project · May 24, 2019 · bdb51c2 · bdb51c2
2 parents 6071686 + c2faba1
commit bdb51c2
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 22 deletions.
diff --git a/uPortal-io/uPortal-io-core/src/main/java/org/apereo/portal/io/xml/XsltDataUpgrader.java b/uPortal-io/uPortal-io-core/src/main/java/org/apereo/portal/io/xml/XsltDataUpgrader.java
@@ -24,17 +24,17 @@
 import javax.xml.transform.TransformerConfigurationException;
 import javax.xml.transform.TransformerException;
 import org.apereo.portal.xml.XmlUtilities;
-import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.core.io.Resource;
 
 /** */
-public class XsltDataUpgrader implements IDataUpgrader, InitializingBean {
+public class XsltDataUpgrader implements IDataUpgrader {
     private Set<PortalDataKey> portalDataKeys;
     private Resource xslResource;
     private XmlUtilities xmlUtilities;
 
-    private Templates upgradeTemplates;
+    // volatile because it's initialized in a synchronized method
+    private volatile Templates upgradeTemplates;
 
     public void setPortalDataKey(PortalDataKey portalDataKey) {
         this.portalDataKeys = Collections.singleton(portalDataKey);
@@ -53,41 +53,48 @@ public void setXmlUtilities(XmlUtilities xmlUtilities) {
         this.xmlUtilities = xmlUtilities;
     }
 
-    @Override
-    public void afterPropertiesSet() throws Exception {
-        this.upgradeTemplates = this.xmlUtilities.getTemplates(xslResource);
-    }
-
-    /* (non-Javadoc)
-     * @see org.apereo.portal.io.xml.IDataUpgrader#getSourceDataType()
-     */
     @Override
     public Set<PortalDataKey> getSourceDataTypes() {
-        return this.portalDataKeys;
+        return portalDataKeys;
     }
 
-    /* (non-Javadoc)
-     * @see org.apereo.portal.io.xml.IDataUpgrader#upgradeData(javax.xml.transform.Source, javax.xml.transform.Result)
-     */
     @Override
     public boolean upgradeData(Source source, Result result) {
+        if (upgradeTemplates == null) {
+            initializeUpgradeTemplates();
+        }
+
         final Transformer transformer;
         try {
-            transformer = this.upgradeTemplates.newTransformer();
+            transformer = upgradeTemplates.newTransformer();
         } catch (TransformerConfigurationException e) {
             throw new RuntimeException(
                     "Failed to create new Transformer from the configured templates: "
-                            + this.xslResource,
+                            + xslResource,
                     e);
         }
 
         try {
             transformer.transform(source, result);
         } catch (TransformerException e) {
             throw new RuntimeException(
-                    "Failed to upgrade data using XSLT transformation: " + this.xslResource, e);
+                    "Failed to upgrade data using XSLT transformation: " + xslResource, e);
         }
 
         return true;
     }
+
+    private synchronized void initializeUpgradeTemplates() {
+        /*
+         * Lazy-initialize the upgradeTemplates because of a potential race condition in the Spring
+         * application context.
+         */
+        if (upgradeTemplates == null) {
+            try {
+                upgradeTemplates = xmlUtilities.getTemplates(xslResource);
+            } catch (Exception e) {
+                throw new RuntimeException("Failed to initialize upgrade templates", e);
+            }
+        }
+    }
 }
diff --git a/uPortal-utils/uPortal-utils-core/src/main/java/org/apereo/portal/xml/XmlUtilitiesImpl.java b/uPortal-utils/uPortal-utils-core/src/main/java/org/apereo/portal/xml/XmlUtilitiesImpl.java
@@ -82,7 +82,7 @@ public void setTemplatesBuilder(TemplatesBuilder templatesBuilder) {
     @Override
     public Templates getTemplates(Resource stylesheet)
             throws TransformerConfigurationException, IOException {
-        final CachedResource<Templates> templates = this.getStylesheetCachedResource(stylesheet);
+        final CachedResource<Templates> templates = getStylesheetCachedResource(stylesheet);
         return templates.getCachedResource();
     }
 
@@ -216,7 +216,7 @@ private int getElementIndex(Node node) {
 
     private CachedResource<Templates> getStylesheetCachedResource(Resource stylesheet)
             throws IOException {
-        return this.cachingResourceLoader.getResource(stylesheet, this.templatesBuilder);
+        return cachingResourceLoader.getResource(stylesheet, templatesBuilder);
     }
 
     public static String getElementText(Element e) {

diff --git a/uPortal-webapp/src/test/java/org/apereo/portal/io/xml/BaseXsltDataUpgraderTest.java b/uPortal-webapp/src/test/java/org/apereo/portal/io/xml/BaseXsltDataUpgraderTest.java
@@ -82,7 +82,6 @@ public Templates getTemplates(Resource stylesheet)
         xsltDataUpgrader.setPortalDataKey(dataKey);
         xsltDataUpgrader.setXslResource(xslResource);
         xsltDataUpgrader.setXmlUtilities(xmlUtilities);
-        xsltDataUpgrader.afterPropertiesSet();
 
         // Create XmlEventReader (what the JaxbPortalDataHandlerService has)
         final XMLInputFactory xmlInputFactory = xmlUtilities.getXmlInputFactory();

diff --git a/uPortal-webapp/src/test/java/org/apereo/portal/io/xml/JaxbPortalDataHandlerServiceTest.java b/uPortal-webapp/src/test/java/org/apereo/portal/io/xml/JaxbPortalDataHandlerServiceTest.java
@@ -348,7 +348,6 @@ protected IDataUpgrader createXsltDataUpgrader(
         xsltDataUpgrader.setPortalDataKey(dataKey);
         xsltDataUpgrader.setXslResource(xslResource);
         xsltDataUpgrader.setXmlUtilities(xmlUtilities);
-        xsltDataUpgrader.afterPropertiesSet();
 
         return xsltDataUpgrader;
     }