Fix the issue with MyBatis-Spring integration where MyBatis reloading…

… is not functioning properly and results in a NullPointerException. (#557) * Fix the issue in MyBatis-Spring mode where a NullPointerException (NPE) occurs during reload, and also differentiate between MyBatis and MyBatis-Spring modes * Update MyBatisTransformers.java Fix the issue where using the insertAt method might fail due to changes in the processPropertyPlaceHolders method body. Add a list variable at the beginning of the method, include the factory in the list after factory initialization, and finally execute the removal logic at the end of the method. Why not execute the removal right after initialization? I tried that, but it fails during reload. --------- Co-authored-by: hejiajin <Homejim5>
HotswapProjects · Aug 2, 2024 · d0b1759 · d0b1759
1 parent 73dd3f5
commit d0b1759
Show file tree

Hide file tree

Showing 5 changed files with 122 additions and 11 deletions.
diff --git a/...mybatis-plugin/src/main/java/org/hotswap/agent/plugin/mybatis/MyBatisRefreshCommands.java b/...mybatis-plugin/src/main/java/org/hotswap/agent/plugin/mybatis/MyBatisRefreshCommands.java
@@ -42,9 +42,17 @@ public class MyBatisRefreshCommands {
 
     public static void reloadConfiguration() {
         LOGGER.debug("Refreshing MyBatis configuration.");
-        ConfigurationProxy.refreshProxiedConfigurations();
-        SpringMybatisConfigurationProxy.refreshProxiedConfigurations();
-        LOGGER.reload("MyBatis configuration refreshed.");
+        /**
+         * If running in MyBatis-Spring mode, then during reload, we only need to use SpringMybatisConfigurationProxy
+         * for reloading. Refreshing the ConfigurationProxy is not meaningful.The same applies in the opposite case.
+         */
+        if (SpringMybatisConfigurationProxy.runningBySpringMybatis()) {
+            SpringMybatisConfigurationProxy.refreshProxiedConfigurations();
+            LOGGER.debug("MyBatis Spring configuration refreshed.");
+        } else {
+            ConfigurationProxy.refreshProxiedConfigurations();
+            LOGGER.reload("MyBatis configuration refreshed.");
+        }
         reloadFlag = false;
     }
 }
diff --git a/...batis-plugin/src/main/java/org/hotswap/agent/plugin/mybatis/proxy/ConfigurationProxy.java b/...batis-plugin/src/main/java/org/hotswap/agent/plugin/mybatis/proxy/ConfigurationProxy.java
@@ -27,6 +27,7 @@
 import org.apache.ibatis.session.Configuration;
 import org.hotswap.agent.javassist.util.proxy.MethodHandler;
 import org.hotswap.agent.javassist.util.proxy.ProxyFactory;
+import org.hotswap.agent.logging.AgentLogger;
 import org.hotswap.agent.plugin.mybatis.transformers.MyBatisTransformers;
 import org.hotswap.agent.util.ReflectionHelper;
 
@@ -36,9 +37,21 @@
  * @author Vladimir Dvorak
  */
 public class ConfigurationProxy {
+
+    private static AgentLogger LOGGER = AgentLogger.getLogger(ConfigurationProxy.class);
+
     private static Map<XMLConfigBuilder, ConfigurationProxy> proxiedConfigurations = new HashMap<>();
 
     public static ConfigurationProxy getWrapper(XMLConfigBuilder configBuilder) {
+        /*
+         * MyBatis runs in MyBatis-Spring mode, so there is no need to cache configuration-related data.
+         * The related reload operations are handled by SpringMybatisConfigurationProxy
+         */
+        if (SpringMybatisConfigurationProxy.runningBySpringMybatis()) {
+            LOGGER.debug("MyBatis runs in MyBatis-Spring mode, so there is no need to cache configuration-related data");
+            return new ConfigurationProxy(configBuilder);
+        }
+
         if (!proxiedConfigurations.containsKey(configBuilder)) {
             proxiedConfigurations.put(configBuilder, new ConfigurationProxy(configBuilder));
         }

diff --git a/...src/main/java/org/hotswap/agent/plugin/mybatis/proxy/SpringMybatisConfigurationProxy.java b/...src/main/java/org/hotswap/agent/plugin/mybatis/proxy/SpringMybatisConfigurationProxy.java
@@ -3,6 +3,7 @@
 import org.apache.ibatis.session.Configuration;
 import org.hotswap.agent.javassist.util.proxy.MethodHandler;
 import org.hotswap.agent.javassist.util.proxy.ProxyFactory;
+import org.hotswap.agent.plugin.mybatis.transformers.ConfigurationCaller;
 import org.hotswap.agent.util.ReflectionHelper;
 
 import java.lang.reflect.Method;
@@ -24,10 +25,16 @@ public static SpringMybatisConfigurationProxy getWrapper(Object sqlSessionFactor
         return proxiedConfigurations.get(sqlSessionFactoryBean);
     }
 
+    public static boolean runningBySpringMybatis() {
+        return !proxiedConfigurations.isEmpty();
+    }
+
     public static void refreshProxiedConfigurations() {
         for (SpringMybatisConfigurationProxy wrapper : proxiedConfigurations.values())
             try {
+                ConfigurationCaller.setInReload(wrapper.configuration, true);
                 wrapper.refreshProxiedConfiguration();
+                ConfigurationCaller.setInReload(wrapper.configuration, false);
             } catch (Exception e) {
                 e.printStackTrace();
             }

diff --git a/...ugin/src/main/java/org/hotswap/agent/plugin/mybatis/transformers/ConfigurationCaller.java b/...ugin/src/main/java/org/hotswap/agent/plugin/mybatis/transformers/ConfigurationCaller.java
@@ -0,0 +1,19 @@
+package org.hotswap.agent.plugin.mybatis.transformers;
+
+import org.apache.ibatis.session.Configuration;
+import org.hotswap.agent.util.ReflectionHelper;
+
+public class ConfigurationCaller {
+    /**
+     * Sets the value of the $$ha$inReload field in the given MyBatis configuration.
+     * This field is used to determine if the configuration is in reload status.
+     *
+     * @param configuration The MyBatis configuration object to be modified.
+     * @param val           The value to set for the $$ha$inReload field. If true,
+     *                      the configuration will be marked as being in reload status.
+     */
+    public static void setInReload(Configuration configuration, boolean val) {
+        // Use ReflectionHelper to set the value of the $$ha$inReload field in the configuration object.
+        ReflectionHelper.set(configuration, MyBatisTransformers.IN_RELOAD_FIELD, val);
+    }
+}
diff --git a/...ugin/src/main/java/org/hotswap/agent/plugin/mybatis/transformers/MyBatisTransformers.java b/...ugin/src/main/java/org/hotswap/agent/plugin/mybatis/transformers/MyBatisTransformers.java
@@ -20,14 +20,9 @@
 
 import org.apache.ibatis.javassist.bytecode.AccessFlag;
 import org.hotswap.agent.annotation.OnClassLoadEvent;
-import org.hotswap.agent.javassist.CannotCompileException;
-import org.hotswap.agent.javassist.ClassPool;
-import org.hotswap.agent.javassist.CtClass;
-import org.hotswap.agent.javassist.CtConstructor;
-import org.hotswap.agent.javassist.CtField;
-import org.hotswap.agent.javassist.CtMethod;
-import org.hotswap.agent.javassist.CtNewMethod;
-import org.hotswap.agent.javassist.NotFoundException;
+import org.hotswap.agent.javassist.*;
+import org.hotswap.agent.javassist.expr.ExprEditor;
+import org.hotswap.agent.javassist.expr.NewExpr;
 import org.hotswap.agent.logging.AgentLogger;
 import org.hotswap.agent.plugin.mybatis.MyBatisPlugin;
 import org.hotswap.agent.plugin.mybatis.proxy.ConfigurationProxy;
@@ -46,6 +41,7 @@ public class MyBatisTransformers {
     public static final String SRC_FILE_NAME_FIELD = "$$ha$srcFileName";
     public static final String REFRESH_DOCUMENT_METHOD = "$$ha$refreshDocument";
     public static final String REFRESH_METHOD = "$$ha$refresh";
+    public static final String IN_RELOAD_FIELD = "$$ha$inReload";
 
     private static final String INITIALIZED_FIELD = "$$ha$initialized";
     private static final String FACTORYBEAN_FIELD = "$$ha$factoryBean";
@@ -183,4 +179,72 @@ public static void patchSqlSessionFactoryBean(CtClass ctClass, ClassPool classPo
         ctClass.addMethod(proxyMethod);
         LOGGER.debug("org.mybatis.spring.SqlSessionFactoryBean patched.");
     }
+
+    @OnClassLoadEvent(classNameRegexp = "org.mybatis.spring.mapper.MapperScannerConfigurer")
+    public static void patchMapperScannerConfigurer(CtClass ctClass, ClassPool classPool)
+            throws NotFoundException, CannotCompileException {
+        /*
+         * In org.mybatis.spring.mapper.MapperScannerConfigurer#processPropertyPlaceHolders,
+         * a BeanFactory is created using new DefaultListableBeanFactory() that contains only
+         * this mapper scanner and processes the factory. This is not needed;
+         * it should be removed from SpringChangedAgent after processing.
+         */
+        CtMethod processPropertyPlaceHolderM = ctClass.getDeclaredMethod("processPropertyPlaceHolders");
+        processPropertyPlaceHolderM.addLocalVariable("tmpFactoryList", classPool.get("java.util.List"));
+        processPropertyPlaceHolderM.insertBefore("{ tmpFactoryList = new java.util.ArrayList(); }");
+
+        // Add the instance of DefaultListableBeanFactory to a list after its creation so that it can be removed later
+        processPropertyPlaceHolderM.instrument(new ExprEditor() {
+            @Override
+            public void edit(NewExpr e) throws CannotCompileException {
+                if (e.getClassName().equals("org.springframework.beans.factory.support.DefaultListableBeanFactory")) {
+                    e.replace("{ $_ = $proceed($$); tmpFactoryList.add($_); }");
+                }
+            }
+        });
+
+        processPropertyPlaceHolderM.insertAfter("{ " +
+                "    for (java.util.Iterator it = tmpFactoryList.iterator(); it.hasNext(); ) {\n" +
+                "        org.springframework.beans.factory.support.DefaultListableBeanFactory factory = (org.springframework.beans.factory.support.DefaultListableBeanFactory) it.next();\n" +
+                "        org.hotswap.agent.plugin.spring.reload.SpringChangedAgent.destroyBeanFactory(factory);\n" +
+                "    }\n" +
+                " }");
+
+        LOGGER.debug("org.mybatis.spring.mapper.MapperScannerConfigurer patched.", new Object[0]);
+    }
+
+    @OnClassLoadEvent(classNameRegexp = "org.apache.ibatis.session.Configuration")
+    public static void patchConfiguration(CtClass ctClass, ClassPool classPool)
+            throws NotFoundException, CannotCompileException {
+        try {
+            CtClass booleanClass = classPool.get(boolean.class.getName());
+            CtField onReloadField = new CtField(booleanClass, IN_RELOAD_FIELD, ctClass);
+            onReloadField.setModifiers(Modifier.PUBLIC);
+            ctClass.addField(onReloadField);
+
+            // If $$ha$inReload is true, then we need to remove the old entry.
+            CtMethod isResourceLoadedMethod = ctClass.getDeclaredMethod("isResourceLoaded", new CtClass[]{
+                    classPool.get(String.class.getName())
+            });
+
+            isResourceLoadedMethod.insertBefore("{\n" +
+                    "if(" + IN_RELOAD_FIELD + "){\n" +
+                    "this.loadedResources.remove($1);" +
+                    "}\n" +
+                    "}");
+        } catch (Exception e) {
+            LOGGER.warning("mybatis class enhance error:", e);
+        }
+    }
+
+    @OnClassLoadEvent(classNameRegexp = "org.apache.ibatis.session.Configuration\\$StrictMap")
+    public static void patchStrictMap(CtClass ctClass, ClassPool classPool)
+            throws NotFoundException, CannotCompileException {
+
+        // To avoid xxx collection already contains value for xxx.
+        CtMethod method = ctClass.getDeclaredMethod("put", new CtClass[]{
+                classPool.get(String.class.getName()), classPool.get(Object.class.getName())
+        });
+        method.insertBefore("if(containsKey($1)){remove($1);}");
+    }
 }