FasterXML / jackson-datatypes-collections

Jackson project that contains various collection-oriented datatype libraries: Eclipse Collections, Guava, HPPC, PCollections
Apache License 2.0
79 stars 53 forks source link

There maybe a misusage in GuavaMultimapDeserializer.findTransformer method #106

Closed magical-l closed 1 year ago

magical-l commented 1 year ago

CN: 在com.fasterxml.jackson.datatype.guava.deser.multimap.GuavaMultimapDeserializer#findTransformer方法中,两个for循环完全一样。 按照上下文的注释,第一个循环应该要在rawType自己定义的方法里寻找,所以应该用rawType.getDeclaredMethod方法。 可能是个笔误。 EN: there are two for loop in the method com.fasterxml.jackson.datatype.guava.deser.multimap.GuavaMultimapDeserializer#findTransformer, and they are exactly the same. according to the comments from context, the first loop is to find the method in the methods declared by rawType itself, so the reflect-method to use should be rawType.getDeclaredMethod. it maybe a typo.

    private static Method findTransformer(Class<?> rawType) {
        // Very first thing: if it's a "standard multi-map type", can avoid copying
        if (rawType == LinkedListMultimap.class || rawType == ListMultimap.class || rawType ==
                Multimap.class) {
            return null;
        }

        // First, check type itself for matching methods
        for (String methodName : METHOD_NAMES) {
            try {
                Method m = rawType.getMethod(methodName, Multimap.class); // should be rawType.getDeclaredMethod(methodName, Multimap.class);
                if (m != null) {
                    return m;
                }
            } catch (NoSuchMethodException e) {
            }
            // pass SecurityExceptions as-is:
            // } catch (SecurityException e) { }
        }

        // If not working, possibly super types too (should we?)
        for (String methodName : METHOD_NAMES) {
            try {
                Method m = rawType.getMethod(methodName, Multimap.class);
                if (m != null) {
                    return m;
                }
            } catch (NoSuchMethodException e) {
            }
            // pass SecurityExceptions as-is:
            // } catch (SecurityException e) { }
        }

        return null;
    }
cowtowncoder commented 1 year ago

Sounds reasonable to me.