Refactoring : Parse pending elements in Configuration

Author: harawata

src/main/java/org/apache/ibatis/builder/annotation/MapperAnnotationBuilder.java

@@ -1,5 +1,5 @@
 /*
- *    Copyright 2009-2023 the original author or authors.
+ *    Copyright 2009-2024 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -27,7 +27,6 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -135,29 +134,14 @@ public void parse() {
         }
       }
     }
-    parsePendingMethods();
+    configuration.parsePendingMethods(false);
   }
 
   private static boolean canHaveStatement(Method method) {
     // issue #237
     return !method.isBridge() && !method.isDefault();
   }
 
-  private void parsePendingMethods() {
-    Collection<MethodResolver> incompleteMethods = configuration.getIncompleteMethods();
-    synchronized (incompleteMethods) {
-      Iterator<MethodResolver> iter = incompleteMethods.iterator();
-      while (iter.hasNext()) {
-        try {
-          iter.next().resolve();
-          iter.remove();
-        } catch (IncompleteElementException e) {
-          // This method is still missing a resource
-        }
-      }
-    }
-  }
-
   private void loadXmlResource() {
     // Spring may not know the real resource name so we check a flag
     // to prevent loading again a resource twice

src/main/java/org/apache/ibatis/builder/xml/XMLMapperBuilder.java

@@ -1,5 +1,5 @@
 /*
- *    Copyright 2009-2023 the original author or authors.
+ *    Copyright 2009-2024 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -19,10 +19,8 @@
 import java.io.Reader;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
@@ -101,9 +99,9 @@ public void parse() {
       configuration.addLoadedResource(resource);
       bindMapperForNamespace();
     }
-    parsePendingResultMaps();
-    parsePendingCacheRefs();
-    parsePendingStatements();
+    configuration.parsePendingResultMaps(false);
+    configuration.parsePendingCacheRefs(false);
+    configuration.parsePendingStatements(false);
   }
 
   public XNode getSqlFragment(String refid) {
@@ -147,51 +145,6 @@ private void buildStatementFromContext(List<XNode> list, String requiredDatabase
     }
   }
 
-  private void parsePendingResultMaps() {
-    Collection<ResultMapResolver> incompleteResultMaps = configuration.getIncompleteResultMaps();
-    synchronized (incompleteResultMaps) {
-      Iterator<ResultMapResolver> iter = incompleteResultMaps.iterator();
-      while (iter.hasNext()) {
-        try {
-          iter.next().resolve();
-          iter.remove();
-        } catch (IncompleteElementException e) {
-          // ResultMap is still missing a resource...
-        }
-      }
-    }
-  }
-
-  private void parsePendingCacheRefs() {
-    Collection<CacheRefResolver> incompleteCacheRefs = configuration.getIncompleteCacheRefs();
-    synchronized (incompleteCacheRefs) {
-      Iterator<CacheRefResolver> iter = incompleteCacheRefs.iterator();
-      while (iter.hasNext()) {
-        try {
-          iter.next().resolveCacheRef();
-          iter.remove();
-        } catch (IncompleteElementException e) {
-          // Cache ref is still missing a resource...
-        }
-      }
-    }
-  }
-
-  private void parsePendingStatements() {
-    Collection<XMLStatementBuilder> incompleteStatements = configuration.getIncompleteStatements();
-    synchronized (incompleteStatements) {
-      Iterator<XMLStatementBuilder> iter = incompleteStatements.iterator();
-      while (iter.hasNext()) {
-        try {
-          iter.next().parseStatementNode();
-          iter.remove();
-        } catch (IncompleteElementException e) {
-          // Statement is still missing a resource...
-        }
-      }
-    }
-  }
-
   private void cacheRefElement(XNode context) {
     if (context != null) {
       configuration.addCacheRef(builderAssistant.getCurrentNamespace(), context.getStringAttribute("namespace"));

src/main/java/org/apache/ibatis/session/Configuration.java

@@ -1,5 +1,5 @@
 /*
- *    Copyright 2009-2023 the original author or authors.
+ *    Copyright 2009-2024 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -26,6 +26,7 @@
 import java.util.Properties;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.BiFunction;
 
 import org.apache.ibatis.binding.MapperRegistry;
@@ -95,7 +96,6 @@
 import org.apache.ibatis.type.TypeAliasRegistry;
 import org.apache.ibatis.type.TypeHandler;
 import org.apache.ibatis.type.TypeHandlerRegistry;
-import org.apache.ibatis.util.LockKit;
 
 /**
  * @author Clinton Begin
@@ -172,6 +172,11 @@ public class Configuration {
   protected final Collection<ResultMapResolver> incompleteResultMaps = new LinkedList<>();
   protected final Collection<MethodResolver> incompleteMethods = new LinkedList<>();
 
+  private final ReentrantLock incompleteResultMapsLock = new ReentrantLock();
+  private final ReentrantLock incompleteCacheRefsLock = new ReentrantLock();
+  private final ReentrantLock incompleteStatementsLock = new ReentrantLock();
+  private final ReentrantLock incompleteMethodsLock = new ReentrantLock();
+
   /*
    * A map holds cache-ref relationship. The key is the namespace that references a cache bound to another namespace and
    * the value is the namespace which the actual cache is bound to.
@@ -832,34 +837,70 @@ public Collection<MappedStatement> getMappedStatements() {
     return mappedStatements.values();
   }
 
+  /**
+   * @deprecated call {@link #parsePendingStatements(boolean)}
+   */
+  @Deprecated
   public Collection<XMLStatementBuilder> getIncompleteStatements() {
     return incompleteStatements;
   }
 
   public void addIncompleteStatement(XMLStatementBuilder incompleteStatement) {
-    incompleteStatements.add(incompleteStatement);
+    incompleteStatementsLock.lock();
+    try {
+      incompleteStatements.add(incompleteStatement);
+    } finally {
+      incompleteStatementsLock.unlock();
+    }
   }
 
+  /**
+   * @deprecated call {@link #parsePendingCacheRefs(boolean)}
+   */
+  @Deprecated
   public Collection<CacheRefResolver> getIncompleteCacheRefs() {
     return incompleteCacheRefs;
   }
 
   public void addIncompleteCacheRef(CacheRefResolver incompleteCacheRef) {
-    incompleteCacheRefs.add(incompleteCacheRef);
+    incompleteCacheRefsLock.lock();
+    try {
+      incompleteCacheRefs.add(incompleteCacheRef);
+    } finally {
+      incompleteCacheRefsLock.unlock();
+    }
   }
 
+  /**
+   * @deprecated call {@link #parsePendingResultMaps(boolean)}
+   */
+  @Deprecated
   public Collection<ResultMapResolver> getIncompleteResultMaps() {
     return incompleteResultMaps;
   }
 
   public void addIncompleteResultMap(ResultMapResolver resultMapResolver) {
-    incompleteResultMaps.add(resultMapResolver);
+    incompleteResultMapsLock.lock();
+    try {
+      incompleteResultMaps.add(resultMapResolver);
+    } finally {
+      incompleteResultMapsLock.unlock();
+    }
   }
 
   public void addIncompleteMethod(MethodResolver builder) {
-    incompleteMethods.add(builder);
+    incompleteMethodsLock.lock();
+    try {
+      incompleteMethods.add(builder);
+    } finally {
+      incompleteMethodsLock.unlock();
+    }
   }
 
+  /**
+   * @deprecated call {@link #parsePendingMethods(boolean)}
+   */
+  @Deprecated
   public Collection<MethodResolver> getIncompleteMethods() {
     return incompleteMethods;
   }
@@ -923,48 +964,71 @@ public void addCacheRef(String namespace, String referencedNamespace) {
    * are added as it provides fail-fast statement validation.
    */
   protected void buildAllStatements() {
-    parsePendingResultMaps();
-    if (!incompleteCacheRefs.isEmpty()) {
-      LockKit.ReentrantLock lock = LockKit.obtainLock(incompleteCacheRefs);
-      lock.lock();
-      try {
-        incompleteCacheRefs.removeIf(x -> x.resolveCacheRef() != null);
-      } finally {
-        lock.unlock();
+    parsePendingResultMaps(true);
+    parsePendingCacheRefs(true);
+    parsePendingStatements(true);
+    parsePendingMethods(true);
+  }
+
+  public void parsePendingMethods(boolean reportUnresolved) {
+    if (incompleteMethods.isEmpty()) {
+      return;
+    }
+    incompleteMethodsLock.lock();
+    try {
+      incompleteMethods.removeIf(x -> {
+        x.resolve();
+        return true;
+      });
+    } catch (IncompleteElementException e) {
+      if (reportUnresolved) {
+        throw e;
       }
+    } finally {
+      incompleteMethodsLock.unlock();
     }
-    if (!incompleteStatements.isEmpty()) {
-      LockKit.ReentrantLock lock = LockKit.obtainLock(incompleteStatements);
-      lock.lock();
-      try {
-        incompleteStatements.removeIf(x -> {
-          x.parseStatementNode();
-          return true;
-        });
-      } finally {
-        lock.unlock();
+  }
+
+  public void parsePendingStatements(boolean reportUnresolved) {
+    if (incompleteStatements.isEmpty()) {
+      return;
+    }
+    incompleteStatementsLock.lock();
+    try {
+      incompleteStatements.removeIf(x -> {
+        x.parseStatementNode();
+        return true;
+      });
+    } catch (IncompleteElementException e) {
+      if (reportUnresolved) {
+        throw e;
       }
+    } finally {
+      incompleteStatementsLock.unlock();
+    }
+  }
+
+  public void parsePendingCacheRefs(boolean reportUnresolved) {
+    if (incompleteCacheRefs.isEmpty()) {
+      return;
     }
-    if (!incompleteMethods.isEmpty()) {
-      LockKit.ReentrantLock lock = LockKit.obtainLock(incompleteMethods);
-      lock.lock();
-      try {
-        incompleteMethods.removeIf(x -> {
-          x.resolve();
-          return true;
-        });
-      } finally {
-        lock.unlock();
+    incompleteCacheRefsLock.lock();
+    try {
+      incompleteCacheRefs.removeIf(x -> x.resolveCacheRef() != null);
+    } catch (IncompleteElementException e) {
+      if (reportUnresolved) {
+        throw e;
       }
+    } finally {
+      incompleteCacheRefsLock.unlock();
     }
   }
 
-  private void parsePendingResultMaps() {
+  public void parsePendingResultMaps(boolean reportUnresolved) {
     if (incompleteResultMaps.isEmpty()) {
       return;
     }
-    LockKit.ReentrantLock lock = LockKit.obtainLock(incompleteResultMaps);
-    lock.lock();
+    incompleteResultMapsLock.lock();
     try {
       boolean resolved;
       IncompleteElementException ex = null;
@@ -981,12 +1045,12 @@ private void parsePendingResultMaps() {
           }
         }
       } while (resolved);
-      if (!incompleteResultMaps.isEmpty() && ex != null) {
+      if (reportUnresolved && !incompleteResultMaps.isEmpty() && ex != null) {
         // At least one result map is unresolvable.
         throw ex;
       }
     } finally {
-      lock.unlock();
+      incompleteResultMapsLock.unlock();
     }
   }