From 52386b21c4a6537fb721db20dd4344ff3bd59d29 Mon Sep 17 00:00:00 2001
From: Earle Nietzel <earle.nietzel@gmail.com>
Date: Wed, 30 May 2018 17:06:06 -0400
Subject: [PATCH] SAK-34049 SakaiContextLoader with improved Spring integration

---
 .../component/impl/SakaiContextLoader.java    | 45 +++++++------------
 .../util/SakaiContextLoaderListener.java      | 33 ++++++++++++--
 master/pom.xml                                |  2 +-
 3 files changed, 46 insertions(+), 34 deletions(-)

diff --git a/kernel/component-manager/src/main/java/org/sakaiproject/component/impl/SakaiContextLoader.java b/kernel/component-manager/src/main/java/org/sakaiproject/component/impl/SakaiContextLoader.java
index 9ac4163ccf73..826ba68ff9dd 100644
--- a/kernel/component-manager/src/main/java/org/sakaiproject/component/impl/SakaiContextLoader.java
+++ b/kernel/component-manager/src/main/java/org/sakaiproject/component/impl/SakaiContextLoader.java
@@ -36,11 +36,9 @@
 import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ConfigurableApplicationContext;
-import org.springframework.context.annotation.AnnotationConfigUtils;
 import org.springframework.core.io.FileSystemResource;
 import org.springframework.web.context.ConfigurableWebApplicationContext;
 import org.springframework.web.context.ContextLoader;
-import org.springframework.web.context.WebApplicationContext;
 
 import org.sakaiproject.component.cover.ComponentManager;
 import org.sakaiproject.component.cover.ServerConfigurationService;
@@ -56,37 +54,27 @@ public class SakaiContextLoader extends ContextLoader
 	public static final String SPRING_CONTEXT_SUFFIX = "-context.xml";
 
 	/**
-     * Allows loading/override of custom bean definitions from sakai.home
-     *
-     * <p>The pattern is the 'servlet_name-context.xml'</p>
-     *
-     * @param servletContext current servlet context
-     * @return the new WebApplicationContext
-     * @throws org.springframework.beans.BeansException
-     *          if the context couldn't be initialized
-     */
-    @Override
-    public WebApplicationContext initWebApplicationContext(ServletContext servletContext) throws BeansException {
-
-        ConfigurableWebApplicationContext cwac = null;
-        if (servletContext.getAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE) == null) {
-            cwac = (ConfigurableWebApplicationContext) super.initWebApplicationContext(servletContext);
-        } else {
-            log.debug("Servlet {} contains attribute ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE with value {} ", servletContext.getServletContextName(), servletContext.getAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE));
-            cwac = (ConfigurableWebApplicationContext) createWebApplicationContext(servletContext);
-        }
-        // optionally look in sakai home for additional bean deifinitions to load
-		if (cwac != null) {
-			final String servletName = servletContext.getServletContextName(); 
+	 * Allows loading/override of custom bean definitions from sakai.home
+	 *
+	 * <p>The pattern is the 'servlet_name-context.xml'</p>
+	 *
+	 * @param sc current servlet context
+	 * @param wac the new WebApplicationContext
+	 */
+	@Override
+	protected void customizeContext(ServletContext sc, ConfigurableWebApplicationContext wac) {
+		super.customizeContext(sc, wac);
+		if (wac != null) {
+			final String servletName = sc.getServletContextName();
 			String location = getHomeBeanDefinitionIfExists(servletName);
 			if (StringUtils.isNotBlank(location)) {
 				log.debug("Servlet " + servletName + " is attempting to load bean definition [" + location + "]");
-				XmlBeanDefinitionReader reader = new XmlBeanDefinitionReader((BeanDefinitionRegistry) cwac.getBeanFactory());
+				XmlBeanDefinitionReader reader = new XmlBeanDefinitionReader((BeanDefinitionRegistry) wac.getBeanFactory());
 				try {
 					int loaded = reader.loadBeanDefinitions(new FileSystemResource(location));
 					log.info("Servlet " + servletName + " loaded " + loaded + " beans from [" + location + "]");
-					AnnotationConfigUtils.registerAnnotationConfigProcessors(reader.getRegistry());
-					cwac.getBeanFactory().preInstantiateSingletons();
+					// AnnotationConfigUtils.registerAnnotationConfigProcessors(reader.getRegistry());
+					// wac.getBeanFactory().preInstantiateSingletons();
 				} catch (BeanDefinitionStoreException bdse) {
 					log.warn("Failure loading beans from [" + location + "]", bdse);
 				} catch (BeanCreationException bce) {
@@ -94,8 +82,7 @@ public WebApplicationContext initWebApplicationContext(ServletContext servletCon
 				}
 			}
 		}
-        return cwac;
-    }
+	}
 
 	/**
 	 * Spring allows a parent ApplicationContext to be set during the creation of a new ApplicationContext
diff --git a/kernel/component-manager/src/main/java/org/sakaiproject/util/SakaiContextLoaderListener.java b/kernel/component-manager/src/main/java/org/sakaiproject/util/SakaiContextLoaderListener.java
index 3f38984175ae..252700c500bc 100644
--- a/kernel/component-manager/src/main/java/org/sakaiproject/util/SakaiContextLoaderListener.java
+++ b/kernel/component-manager/src/main/java/org/sakaiproject/util/SakaiContextLoaderListener.java
@@ -21,14 +21,16 @@
 
 package org.sakaiproject.util;
 
+import java.util.Enumeration;
+import javax.servlet.ServletContext;
 import javax.servlet.ServletContextEvent;
 import javax.servlet.ServletContextListener;
 
 import lombok.extern.slf4j.Slf4j;
-
 import org.sakaiproject.component.cover.ComponentManager;
 import org.sakaiproject.component.impl.SakaiContextLoader;
 import org.sakaiproject.component.impl.SpringCompMgr;
+import org.springframework.beans.factory.DisposableBean;
 
 /**
  * <p>
@@ -57,11 +59,34 @@ public void contextInitialized(ServletContextEvent event)
 	public void contextDestroyed(ServletContextEvent event)
 	{
 		closeWebApplicationContext(event.getServletContext());
-		//ContextCleanupListener.cleanupAttributes(event.getServletContext());
-		
-		log.info("Destroying Components in "+event.getServletContext().getServletContextName());
+		cleanupAttributes(event.getServletContext());
+
+		log.debug("Destroying Components in {}", event.getServletContext().getServletContextName());
 
 		// decrement the count of children for the component manager
 		((SpringCompMgr) ComponentManager.getInstance()).removeChildAc();
 	}
+
+	/**
+	 * Find all ServletContext attributes which implement {@link DisposableBean}
+	 * and destroy them, removing all affected ServletContext attributes eventually.
+	 * @param sc the ServletContext to check
+	 */
+	static void cleanupAttributes(ServletContext sc) {
+		Enumeration<String> attrNames = sc.getAttributeNames();
+		while (attrNames.hasMoreElements()) {
+			String attrName = attrNames.nextElement();
+			if (attrName.startsWith("org.springframework.")) {
+				Object attrValue = sc.getAttribute(attrName);
+				if (attrValue instanceof DisposableBean) {
+					try {
+						((DisposableBean) attrValue).destroy();
+					}
+					catch (Throwable ex) {
+						log.error("Couldn't invoke destroy method of attribute with name '" + attrName + "'", ex);
+					}
+				}
+			}
+		}
+	}
 }
diff --git a/master/pom.xml b/master/pom.xml
index ae2a7dd7748b..6c79daa4b2c5 100644
--- a/master/pom.xml
+++ b/master/pom.xml
@@ -764,7 +764,7 @@
       <dependency>
         <groupId>org.hibernate</groupId>
         <artifactId>hibernate-validator</artifactId>
-        <version>5.3.4.Final</version>
+        <version>5.3.5.Final</version>
         <scope>provided</scope>
       </dependency>
       <dependency>