fix(MethodContainer): the data source container based on single param…

…eter method cannot correctly handle the set of input key values (Gitee #I9C5GA)
opengoofy · Mar 27, 2024 · da9b38f · da9b38f
1 parent e841c22
commit da9b38f
Show file tree

Hide file tree

Showing 6 changed files with 95 additions and 73 deletions.
diff --git a/crane4j-core/src/main/java/cn/crane4j/core/container/MethodInvokerContainer.java b/crane4j-core/src/main/java/cn/crane4j/core/container/MethodInvokerContainer.java
@@ -36,7 +36,7 @@
  * @see MappingType
  */
 @RequiredArgsConstructor
-public class MethodInvokerContainer implements Container<Object> {
+public abstract class MethodInvokerContainer implements Container<Object> {
 
     /**
      * Namespace of method.
@@ -54,7 +54,7 @@ public class MethodInvokerContainer implements Container<Object> {
      * if method is static, this field can be null.
      */
     @Nullable
-    private final Object target;
+    protected final Object target;
 
     /**
      * Create a standard method data source container.
@@ -69,7 +69,7 @@ public class MethodInvokerContainer implements Container<Object> {
     public static MethodInvokerContainer create(
         String namespace, MethodInvoker methodInvoker, @Nullable Object target, boolean isMapped) {
         return isMapped ? new NoMapping(namespace, methodInvoker, target)
-            : new MethodInvokerContainer(namespace, methodInvoker, target);
+            : new StandardMethodInvokerContainer(namespace, methodInvoker, target);
     }
 
     /**
@@ -120,53 +120,68 @@ public static MethodInvokerContainer oneToMany(
     }
 
     /**
-     * Enter a batch of key values to return data source objects grouped by key values.
+     * The container for method with single parameter.
      *
-     * @param keys keys
-     * @return data source objects grouped by key value
+     * @author huangchengxing
      */
-    @Override
-    public Map<Object, ?> get(Collection<Object> keys) {
-        Object[] arguments = resolveArguments(keys);
-        Object result = methodInvoker.invoke(target, arguments);
-        if (Objects.isNull(result)) {
-            return Collections.emptyMap();
+    protected static class SingleKey extends MethodInvokerContainer {
+
+        public SingleKey(String namespace, MethodInvoker methodInvoker, @Nullable Object target) {
+            super(namespace, methodInvoker, target);
         }
-        return resolveResult(keys, result);
-    }
 
-    /**
-     * Resolve arguments.
-     *
-     * @param keys keys
-     * @return arguments
-     */
-    protected Object[] resolveArguments(Collection<Object> keys) {
-        return new Object[] {keys};
+        /**
+         * Enter a batch of key values to return data source objects grouped by key values.
+         *
+         * @param keys keys
+         * @return data source objects grouped by key value
+         */
+        @Override
+        public Map<Object, ?> get(Collection<Object> keys) {
+            Map<Object, Object> results = new HashMap<>(keys.size());
+            keys.forEach(key -> {
+                Object result = methodInvoker.invoke(target, key);
+                results.put(key, result);
+            });
+            return results;
+        }
     }
 
     /**
-     * Resolve result to map.
+     * Standard method data source container.
      *
-     * @param keys    keys
-     * @param result result
-     * @return map
+     * @author huangchengxing
      */
-    protected Map<Object, ?> resolveResult(Collection<Object> keys, Object result) {
-        Collection<?> results = CollectionUtils.adaptObjectToCollection(result);
-        Map<Object, Object> resultMap = new HashMap<>(keys.size());
-        Iterator<?> valueIterator = results.iterator();
-        for (Object key : keys) {
-            Object value = valueIterator.hasNext() ? valueIterator.next() : null;
-            resultMap.put(key, value);
+    protected static class StandardMethodInvokerContainer extends MethodInvokerContainer {
+
+        public StandardMethodInvokerContainer(String namespace, MethodInvoker methodInvoker, @Nullable Object target) {
+            super(namespace, methodInvoker, target);
         }
-        return resultMap;
-    }
 
-    protected static class SingleKey extends MethodInvokerContainer {
+        /**
+         * Enter a batch of key values to return data source objects grouped by key values.
+         *
+         * @param keys keys
+         * @return data source objects grouped by key value
+         */
+        @Override
+        public Map<Object, ?> get(Collection<Object> keys) {
+            Object[] arguments = resolveArguments(keys);
+            Object result = methodInvoker.invoke(target, arguments);
+            if (Objects.isNull(result)) {
+                return Collections.emptyMap();
+            }
+            return resolveResult(keys, result);
+        }
 
-        public SingleKey(String namespace, MethodInvoker methodInvoker, @Nullable Object target) {
-            super(namespace, methodInvoker, target);
+        /**
+         * Resolve arguments.
+         *
+         * @param keys keys
+         * @return arguments
+         */
+        protected Object[] resolveArguments(Collection<Object> keys) {
+            return new Object[] {keys};
         }
 
         /**
@@ -176,9 +191,15 @@ public SingleKey(String namespace, MethodInvoker methodInvoker, @Nullable Object
          * @param result result
          * @return map
          */
-        @Override
         protected Map<Object, ?> resolveResult(Collection<Object> keys, Object result) {
-            return Collections.singletonMap(CollectionUtils.getFirstNotNull(keys), result);
+            Collection<?> results = CollectionUtils.adaptObjectToCollection(result);
+            Map<Object, Object> resultMap = new HashMap<>(keys.size());
+            Iterator<?> valueIterator = results.iterator();
+            for (Object key : keys) {
+                Object value = valueIterator.hasNext() ? valueIterator.next() : null;
+                resultMap.put(key, value);
+            }
+            return resultMap;
         }
     }
 
@@ -187,7 +208,7 @@ public SingleKey(String namespace, MethodInvoker methodInvoker, @Nullable Object
      *
      * @since 2.4.0
      */
-    protected static class NoMapping extends MethodInvokerContainer {
+    protected static class NoMapping extends StandardMethodInvokerContainer {
 
         public NoMapping(String namespace, MethodInvoker methodInvoker, @Nullable Object target) {
             super(namespace, methodInvoker, target);
@@ -212,7 +233,7 @@ public NoMapping(String namespace, MethodInvoker methodInvoker, @Nullable Object
      *
      * @since 2.4.0
      */
-    protected static class OneToOne extends MethodInvokerContainer {
+    protected static class OneToOne extends StandardMethodInvokerContainer {
 
         protected final KeyExtractor keyExtractor;
         private final DuplicateStrategy duplicateStrategy;
@@ -249,7 +270,7 @@ public OneToOne(
      *
      * @since 2.4.0
      */
-    protected static class OneToMany extends MethodInvokerContainer {
+    protected static class OneToMany extends StandardMethodInvokerContainer {
 
         private final KeyExtractor keyExtractor;
 

diff --git a/...j-core/src/main/java/cn/crane4j/core/support/container/MethodInvokerContainerCreator.java b/...j-core/src/main/java/cn/crane4j/core/support/container/MethodInvokerContainerCreator.java
@@ -9,10 +9,8 @@
 import cn.crane4j.core.support.converter.ParameterConvertibleMethodInvoker;
 import cn.crane4j.core.support.reflect.PropertyOperator;
 import cn.crane4j.core.support.reflect.ReflectiveMethodInvoker;
-import cn.crane4j.core.util.ArrayUtils;
 import cn.crane4j.core.util.Asserts;
 import cn.crane4j.core.util.ClassUtils;
-import cn.crane4j.core.util.CollectionUtils;
 import cn.crane4j.core.util.StringUtils;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
@@ -86,7 +84,6 @@ private MethodInvokerContainer doCreateContainer(
         @Nullable Object target, MethodInvoker methodInvoker, @Nullable Method method, MappingType mappingType,
         @Nullable String namespace, Class<?> resultType, String resultKey, DuplicateStrategy duplicateStrategy) {
 
-        methodInvoker = wrapInvokerIfNecessary(methodInvoker, method);
         namespace = getNamespace(method, namespace);
         MethodInvokerContainer container;
         if (mappingType == MappingType.NO_MAPPING || mappingType == MappingType.MAPPED) {
@@ -115,19 +112,6 @@ private MethodInvokerContainer doCreateContainer(
         return container;
     }
 
-    protected MethodInvoker wrapInvokerIfNecessary(
-        MethodInvoker methodInvoker, @Nullable Method method) {
-        if (Objects.nonNull(method)) {
-            // if the method has only one parameter and the parameter is not a collection,
-            // we will take the first element of the collection as the parameter to call the method
-            boolean isSingleParameterMethod = isSingleParameterMethod(method);
-            if (isSingleParameterMethod) {
-                methodInvoker = new SingleParameterMethodInvoker(methodInvoker);
-            }
-        }
-        return methodInvoker;
-    }
-
     private static boolean isSingleParameterMethod(@NonNull Method method) {
         return method.getParameterCount() == 1
             && !Collection.class.isAssignableFrom(method.getParameterTypes()[0]);
@@ -245,14 +229,4 @@ protected MethodInvoker findKeyGetter(Class<?> resultType, String resultKey) {
         Asserts.isNotNull(keyGetter, "cannot find getter method [{}] on [{}]", resultKey, resultType);
         return keyGetter;
     }
-
-    @RequiredArgsConstructor
-    protected static class SingleParameterMethodInvoker implements MethodInvoker {
-        private final MethodInvoker delegate;
-        @Override
-        public Object invoke(Object target, Object... args) {
-            Collection<?> firstArg = CollectionUtils.adaptObjectToCollection(ArrayUtils.get(args, 0));
-            return delegate.invoke(target, CollectionUtils.getFirstNotNull(firstArg));
-        }
-    }
 }
diff --git a/crane4j-core/src/test/java/cn/crane4j/core/container/MethodInvokerContainerTest.java b/crane4j-core/src/test/java/cn/crane4j/core/container/MethodInvokerContainerTest.java
@@ -79,6 +79,22 @@ public void getWhenOneToMany() {
         Assert.assertTrue(data.isEmpty());
     }
 
+    @Test
+    public void getWhenSingle() {
+        MethodInvokerContainer container = MethodInvokerContainer.singleKey(
+            MethodInvokerContainer.class.getSimpleName(),
+            (t, arg) -> service.singleMethod((String) arg[0]),
+            service
+        );
+        Assert.assertEquals(MethodInvokerContainer.class.getSimpleName(), container.getNamespace());
+
+        Map<Object, Foo> data = (Map<Object, Foo>) container.get(Arrays.asList("1", "2", "3", "4"));
+        Assert.assertEquals("1", data.get("1").getKey());
+        Assert.assertEquals("2", data.get("2").getKey());
+        Assert.assertEquals("3", data.get("3").getKey());
+        Assert.assertEquals("4", data.get("4").getKey());
+    }
+
     @Test
     @SuppressWarnings("unchecked")
     public void getWhenNotKeyExtractor() {
@@ -102,6 +118,9 @@ private static class Foo {
     }
 
     private static class Service {
+        public Foo singleMethod(String key) {
+            return new Foo(key, key);
+        }
         public Map<String, Foo> mappedMethod(Collection<String> key) {
             return Objects.isNull(key) ? null : Stream.of(foo1, foo2).collect(Collectors.toMap(Foo::getKey, Function.identity()));
         }

diff --git a/...re/src/test/java/cn/crane4j/core/support/container/DefaultMethodContainerFactoryTest.java b/...re/src/test/java/cn/crane4j/core/support/container/DefaultMethodContainerFactoryTest.java
@@ -110,9 +110,9 @@ public void getWhenNoneMappedMethod() {
         Assert.assertEquals("noneMappedMethod", container.getNamespace());
 
         Map<Object, ?> values = container.get(Arrays.asList("1", "2"));
-        Assert.assertEquals(1, values.size());
+        Assert.assertEquals(2, values.size());
         Assert.assertEquals("1", values.get("1"));
-        Assert.assertNull(values.get("2"));
+        Assert.assertEquals("2", values.get("2"));
     }
 
     @Test

diff --git a/pom.xml b/pom.xml
@@ -44,7 +44,7 @@
 
   <properties>
     <!--revision-->
-    <revision>2.6.1</revision>
+    <revision>2.6.2</revision>
     <!--build-->
     <java.version>1.8</java.version>
     <server.id>maven-central</server.id>

diff --git a/website/docs/other/changelog.md b/website/docs/other/changelog.md
@@ -360,4 +360,12 @@
 
 **Fix**
 
-- [当作为数据源容器的方法接受单个参数并返回集合类型结果时，只会填充集合中的首个元素](https://gitee.com/opengoofy/crane4j/issues/I97R7E)；
+- [当作为数据源容器的方法接受单个参数并返回集合类型结果时，只会填充集合中的首个元素](https://gitee.com/opengoofy/crane4j/issues/I97R7E)；
+
+## 2.6.2 (2024-03-27)
+
+这是一个 Bug 修复版本，请尽快升级。
+
+**Fix**
+
+- [将接受单个参数并返回单个对象的方法作为数据源容器时，填充出现问题](https://gitee.com/opengoofy/crane4j/issues/I9C5GA)；