[XBEAN-453] try to explicit the parameter types matched instead of us…

[rmannibucau]

…ing ...

[rzo1]

I have tested the change with the other change from @jungm in TomEE and encountered some additional failing tests due to the changes for finding a (static) factory method.

org.apache.xbean.recipe.MissingFactoryMethodException: Instance factory method is static: public static org.apache.geronimo.transaction.manager.GeronimoTransactionManager org.apache.openejb.resource.GeronimoTransactionManagerFactory.create(java.lang.Integer,org.apache.openejb.util.Duration,boolean,byte[],java.lang.String,int,boolean,boolean,java.lang.Integer,org.apache.openejb.util.Duration,java.lang.String,java.lang.String,java.lang.String,int,int,int,int,int) throws java.lang.Exception

	at org.apache.xbean.recipe.ReflectionUtil.findInstanceFactory(ReflectionUtil.java:879)
	at org.apache.xbean.recipe.ObjectRecipe.internalCreate(ObjectRecipe.java:305)
	at org.apache.xbean.recipe.AbstractRecipe.create(AbstractRecipe.java:96)
	at org.apache.xbean.recipe.AbstractRecipe.create(AbstractRecipe.java:61)
	at org.apache.xbean.recipe.AbstractRecipe.create(AbstractRecipe.java:49)
	at org.apache.openejb.assembler.classic.Assembler.createTransactionManager(Assembler.java:3587)
	at org.apache.openejb.config.XmlOverridesTest.test(XmlOverridesTest.java:69)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

Test to reproduce would be

package org.apache.xbean.recipe;

import org.junit.Test;

import java.time.Duration;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;

import static org.junit.Assert.assertNotNull;

public class XBean353FactoryTest {

    @Test
    public void shouldFindStaticFactoryForStaticFactory() {
        // given
        Class<?> typeClass = InterfaceTypeFactory.class;

        String factoryMethod = "create";

        List<String> parameterNames = Arrays.asList(
                "defaultTransactionTimeoutSeconds",
                "defaultTransactionTimeout",
                "TxRecovery",
                "tmId",
                "bufferClassName",
                "bufferSizeKb",
                "checksumEnabled",
                "adler32Checksum",
                "flushSleepTimeMilliseconds",
                "flushSleepTime",
                "logFileDir",
                "logFileExt",
                "logFileName",
                "maxBlocksPerFile",
                "maxBuffers",
                "maxLogFiles",
                "minBuffers",
                "threadsWaitingForceThreshold"
        );

        // Explicitly null
        List<Class<?>> parameterTypes = null;

        Set<String> allProperties = new LinkedHashSet<>(Arrays.asList(
                "defaultTransactionTimeout",
                "TxRecovery",
                "bufferSizeKb",
                "checksumEnabled",
                "adler32Checksum",
                "flushSleepTime",
                "logFileDir",
                "logFileExt",
                "logFileName",
                "maxBlocksPerFile",
                "maxBuffers",
                "maxLogFiles",
                "minBuffers",
                "threadsWaitingForceThreshold"
        ));

        EnumSet<Option> options = EnumSet.of(
                Option.PRIVATE_PROPERTIES,
                Option.FIELD_INJECTION,
                Option.IGNORE_MISSING_PROPERTIES,
                Option.CASE_INSENSITIVE_PROPERTIES
        );

        // when
        ReflectionUtil.StaticFactory staticFactory = ReflectionUtil.findStaticFactory(
                typeClass,
                factoryMethod,
                parameterNames,
                parameterTypes,
                allProperties,
                options
        );

        // then
        assertNotNull(staticFactory);
    }

    public interface InterfaceType {
    }

    public static class InterfaceTypeFactory {
        public static InterfaceType create(Integer defaultTransactionTimeoutSeconds,
                                           final Duration defaultTransactionTimeout,
                                           final boolean txRecovery,
                                           final byte[] tmId,
                                           final String bufferClassName,
                                           final int bufferSizeKb,
                                           final boolean checksumEnabled,
                                           final boolean adler32Checksum,
                                           Integer flushSleepTimeMilliseconds, // Deprecated, use flushSleepTime
                                           final Duration flushSleepTime,
                                           final String logFileDir,
                                           final String logFileExt,
                                           final String logFileName,
                                           final int maxBlocksPerFile,
                                           final int maxBuffers,
                                           final int maxLogFiles,
                                           final int minBuffers,
                                           final int threadsWaitingForceThreshold) throws Exception {
            return null;
        }
    }
}

I think the issue is that we (again) run into implicit selection and since we have

   parameterNames = getParameterNames(method);
                if (parameterNames == null || !allProperties.containsAll(parameterNames)) {
                    continue;
                }
                

we are ignoring case insensitive properties (and missing properties) and run into the issue with not finding the correct (static) factory.

[rzo1]

caseInsesnitive typo was missed here.

[rzo1]

Good catch! I think the same happens for findStaticFactory(...) too, so we want to fix it too.