fix: properly detect value ranges with wildcard types (#838)

TimefoldAI · May 13, 2024 · 6c4e2e6 · 6c4e2e6
1 parent 610b1a8
commit 6c4e2e6
Show file tree

Hide file tree

Showing 8 changed files with 107 additions and 146 deletions.
diff --git a/core/src/main/java/ai/timefold/solver/core/config/util/ConfigUtils.java b/core/src/main/java/ai/timefold/solver/core/config/util/ConfigUtils.java
diff --git a/...main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java b/...main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java
@@ -331,9 +331,7 @@ private Class<? extends Annotation> extractFactEntityOrScoreAnnotationClassOrAut
                                     + "Maybe the member (" + memberName + ") should return a typed "
                                     + Collection.class.getSimpleName() + ".");
                         }
-                        elementType = ConfigUtils.extractCollectionGenericTypeParameterLeniently(
-                                "solutionClass", solutionClass,
-                                type, genericType,
+                        elementType = ConfigUtils.extractGenericTypeParameter("solutionClass", solutionClass, type, genericType,
                                 null, member.getName()).orElse(Object.class);
                     } else {
                         elementType = type.getComponentType();
@@ -588,10 +586,10 @@ private Set<Class<?>> collectEntityAndProblemFactClasses() {
         Stream<Class<?>> problemFactOrEntityClassStream = concat(entityClassStream, factClassStream);
         Stream<Class<?>> factCollectionClassStream = problemFactCollectionMemberAccessorMap.values()
                 .stream()
-                .map(accessor -> ConfigUtils.extractCollectionGenericTypeParameterLeniently(
-                        "solutionClass", getSolutionClass(),
-                        accessor.getType(), accessor.getGenericType(), ProblemFactCollectionProperty.class,
-                        accessor.getName()).orElse(Object.class));
+                .map(accessor -> ConfigUtils
+                        .extractGenericTypeParameter("solutionClass", getSolutionClass(), accessor.getType(),
+                                accessor.getGenericType(), ProblemFactCollectionProperty.class, accessor.getName())
+                        .orElse(Object.class));
         problemFactOrEntityClassStream = concat(problemFactOrEntityClassStream, factCollectionClassStream);
         // Add constraint configuration, if configured.
         if (constraintConfigurationDescriptor != null) {
@@ -932,12 +930,9 @@ public void visitEntitiesByEntityClass(Solution_ solution, Class<?> entityClass,
             }
         }
         for (MemberAccessor entityCollectionMemberAccessor : entityCollectionMemberAccessorMap.values()) {
-            Optional<Class<?>> optionalTypeParameter = ConfigUtils.extractCollectionGenericTypeParameterLeniently(
-                    "solutionClass", entityCollectionMemberAccessor.getDeclaringClass(),
-                    entityCollectionMemberAccessor.getType(),
-                    entityCollectionMemberAccessor.getGenericType(),
-                    null,
-                    entityCollectionMemberAccessor.getName());
+            Optional<Class<?>> optionalTypeParameter = ConfigUtils.extractGenericTypeParameter("solutionClass",
+                    entityCollectionMemberAccessor.getDeclaringClass(), entityCollectionMemberAccessor.getType(),
+                    entityCollectionMemberAccessor.getGenericType(), null, entityCollectionMemberAccessor.getName());
             boolean collectionGuaranteedToContainOnlyGivenEntityType = optionalTypeParameter
                     .map(entityClass::isAssignableFrom)
                     .orElse(false);

diff --git a/...lver/core/impl/domain/valuerange/descriptor/AbstractFromPropertyValueRangeDescriptor.java b/...lver/core/impl/domain/valuerange/descriptor/AbstractFromPropertyValueRangeDescriptor.java
@@ -63,9 +63,8 @@ private void processValueRangeProviderAnnotation(ValueRangeProvider valueRangePr
                     + ", an array or a " + ValueRange.class.getSimpleName() + ".");
         }
         if (collectionWrapping) {
-            Class<?> collectionElementClass = ConfigUtils.extractCollectionGenericTypeParameterStrictly(
-                    "solutionClass or entityClass", memberAccessor.getDeclaringClass(),
-                    memberAccessor.getType(), memberAccessor.getGenericType(),
+            Class<?> collectionElementClass = ConfigUtils.extractGenericTypeParameterOrFail("solutionClass or entityClass",
+                    memberAccessor.getDeclaringClass(), memberAccessor.getType(), memberAccessor.getGenericType(),
                     ValueRangeProvider.class, memberAccessor.getName());
             if (!variableDescriptor.acceptsValueType(collectionElementClass)) {
                 throw new IllegalArgumentException("The entityClass (" + entityDescriptor.getEntityClass()

diff --git a/...va/ai/timefold/solver/core/impl/domain/variable/descriptor/GenuineVariableDescriptor.java b/...va/ai/timefold/solver/core/impl/domain/variable/descriptor/GenuineVariableDescriptor.java
@@ -1,15 +1,14 @@
 package ai.timefold.solver.core.impl.domain.variable.descriptor;
 
+import static ai.timefold.solver.core.config.util.ConfigUtils.newInstance;
+
 import java.lang.reflect.ParameterizedType;
-import java.lang.reflect.Type;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Comparator;
 import java.util.stream.Stream;
 
 import ai.timefold.solver.core.api.domain.solution.PlanningSolution;
-import ai.timefold.solver.core.api.domain.valuerange.ValueRange;
 import ai.timefold.solver.core.api.domain.valuerange.ValueRangeProvider;
 import ai.timefold.solver.core.api.domain.variable.PlanningListVariable;
 import ai.timefold.solver.core.api.domain.variable.PlanningVariable;
@@ -91,8 +90,8 @@ The entityClass (%s) has a @%s annotated property (%s) that has no valueRangePro
     }
 
     private MemberAccessor[] findAnonymousValueRangeMemberAccessors(DescriptorPolicy descriptorPolicy) {
-        boolean supportsValueRangeProviderFromEntity = !isListVariable();
-        Stream<MemberAccessor> applicableValueRangeProviderAccessors =
+        var supportsValueRangeProviderFromEntity = !isListVariable();
+        var applicableValueRangeProviderAccessors =
                 supportsValueRangeProviderFromEntity ? Stream.concat(
                         descriptorPolicy.getAnonymousFromEntityValueRangeProviderSet().stream(),
                         descriptorPolicy.getAnonymousFromSolutionValueRangeProviderSet().stream())
@@ -103,26 +102,23 @@ private MemberAccessor[] findAnonymousValueRangeMemberAccessors(DescriptorPolicy
                      * For basic variable, the type is the type of the variable.
                      * For list variable, the type is List<X>, and we need to know X.
                      */
-                    Class<?> variableType =
+                    var variableType =
                             isListVariable() ? (Class<?>) ((ParameterizedType) variableMemberAccessor.getGenericType())
                                     .getActualTypeArguments()[0] : variableMemberAccessor.getType();
                     // We expect either ValueRange, Collection or an array.
-                    Type valueRangeType = valueRangeProviderAccessor.getGenericType();
+                    var valueRangeType = valueRangeProviderAccessor.getGenericType();
                     if (valueRangeType instanceof ParameterizedType parameterizedValueRangeType) {
-                        Class<?> rawType = (Class<?>) parameterizedValueRangeType.getRawType();
-                        if (!ValueRange.class.isAssignableFrom(rawType) && !Collection.class.isAssignableFrom(rawType)) {
-                            return false;
-                        }
-                        Type[] generics = parameterizedValueRangeType.getActualTypeArguments();
-                        if (generics.length != 1) {
-                            return false;
-                        }
-                        Class<?> valueRangeGenericType = (Class<?>) generics[0];
-                        return variableType.isAssignableFrom(valueRangeGenericType);
+                        return ConfigUtils
+                                .extractGenericTypeParameter("solutionClass",
+                                        entityDescriptor.getSolutionDescriptor().getSolutionClass(),
+                                        valueRangeProviderAccessor.getType(), parameterizedValueRangeType,
+                                        ValueRangeProvider.class, valueRangeProviderAccessor.getName())
+                                .map(variableType::isAssignableFrom)
+                                .orElse(false);
                     } else {
-                        Class<?> clz = (Class<?>) valueRangeType;
+                        var clz = (Class<?>) valueRangeType;
                         if (clz.isArray()) {
-                            Class<?> componentType = clz.getComponentType();
+                            var componentType = clz.getComponentType();
                             return variableType.isAssignableFrom(componentType);
                         }
                         return false;
@@ -137,7 +133,7 @@ private MemberAccessor findValueRangeMemberAccessor(DescriptorPolicy descriptorP
         } else if (descriptorPolicy.hasFromEntityValueRangeProvider(valueRangeProviderRef)) {
             return descriptorPolicy.getFromEntityValueRangeProvider(valueRangeProviderRef);
         } else {
-            Collection<String> providerIds = descriptorPolicy.getValueRangeProviderIds();
+            var providerIds = descriptorPolicy.getValueRangeProviderIds();
             throw new IllegalArgumentException("The entityClass (" + entityDescriptor.getEntityClass()
                     + ") has a @" + PlanningVariable.class.getSimpleName()
                     + " annotated property (" + variableMemberAccessor.getName()
@@ -183,15 +179,15 @@ protected void processStrength(Class<? extends Comparator> strengthComparatorCla
                     + ") at the same time.");
         }
         if (strengthComparatorClass != null) {
-            Comparator<Object> strengthComparator = ConfigUtils.newInstance(this::toString,
+            Comparator<Object> strengthComparator = newInstance(this::toString,
                     "strengthComparatorClass", strengthComparatorClass);
             increasingStrengthSorter = new ComparatorSelectionSorter<>(strengthComparator,
                     SelectionSorterOrder.ASCENDING);
             decreasingStrengthSorter = new ComparatorSelectionSorter<>(strengthComparator,
                     SelectionSorterOrder.DESCENDING);
         }
         if (strengthWeightFactoryClass != null) {
-            SelectionSorterWeightFactory<Solution_, Object> strengthWeightFactory = ConfigUtils.newInstance(this::toString,
+            SelectionSorterWeightFactory<Solution_, Object> strengthWeightFactory = newInstance(this::toString,
                     "strengthWeightFactoryClass", strengthWeightFactoryClass);
             increasingStrengthSorter = new WeightFactorySelectionSorter<>(strengthWeightFactory,
                     SelectionSorterOrder.ASCENDING);
@@ -241,7 +237,7 @@ public boolean isValueRangeEntityIndependent() {
      * is reinitializable if its value is {@code null}.
      */
     public boolean isReinitializable(Object entity) {
-        Object value = getValue(entity);
+        var value = getValue(entity);
         return value == null;
     }
 

diff --git a/.../java/ai/timefold/solver/core/impl/domain/variable/descriptor/ListVariableDescriptor.java b/.../java/ai/timefold/solver/core/impl/domain/variable/descriptor/ListVariableDescriptor.java
@@ -93,10 +93,9 @@ public boolean isInitialized(Object entity) {
     }
 
     public Class<?> getElementType() {
-        return ConfigUtils.extractCollectionGenericTypeParameterStrictly(
-                "entityClass", entityDescriptor.getEntityClass(),
-                variableMemberAccessor.getType(), variableMemberAccessor.getGenericType(),
-                PlanningListVariable.class, variableMemberAccessor.getName());
+        return ConfigUtils.extractGenericTypeParameterOrFail("entityClass", entityDescriptor.getEntityClass(),
+                variableMemberAccessor.getType(), variableMemberAccessor.getGenericType(), PlanningListVariable.class,
+                variableMemberAccessor.getName());
     }
 
     public int countUnassigned(Solution_ solution) {

diff --git a/...er/core/impl/domain/variable/inverserelation/InverseRelationShadowVariableDescriptor.java b/...er/core/impl/domain/variable/inverserelation/InverseRelationShadowVariableDescriptor.java
@@ -60,10 +60,10 @@ private void linkShadowSources(DescriptorPolicy descriptorPolicy) {
         Class<?> sourceClass;
         if (Collection.class.isAssignableFrom(variablePropertyType)) {
             Type genericType = variableMemberAccessor.getGenericType();
-            sourceClass = ConfigUtils.extractCollectionGenericTypeParameterLeniently(
-                    "entityClass", entityDescriptor.getEntityClass(),
-                    variablePropertyType, genericType,
-                    InverseRelationShadowVariable.class, variableMemberAccessor.getName()).orElse(Object.class);
+            sourceClass = ConfigUtils
+                    .extractGenericTypeParameter("entityClass", entityDescriptor.getEntityClass(), variablePropertyType,
+                            genericType, InverseRelationShadowVariable.class, variableMemberAccessor.getName())
+                    .orElse(Object.class);
             singleton = false;
         } else {
             sourceClass = variablePropertyType;

diff --git a/...st/java/ai/timefold/solver/core/api/domain/valuerange/AnonymousValueRangeFactoryTest.java b/...st/java/ai/timefold/solver/core/api/domain/valuerange/AnonymousValueRangeFactoryTest.java
@@ -77,7 +77,7 @@ void solveList() {
         assertThat(solution).isNotNull();
     }
 
-    private void assertEntity(SoftAssertions softly, TestdataAnonymousValueRangeEntity entity) {
+    private static void assertEntity(SoftAssertions softly, TestdataAnonymousValueRangeEntity entity) {
         softly.assertThat(entity.getNumberValue()).isNotNull();
         softly.assertThat(entity.getIntegerValue()).isNotNull();
         softly.assertThat(entity.getLongValue()).isNotNull();

diff --git a/.../solver/core/impl/testdata/domain/valuerange/anonymous/TestdataAnonymousListSolution.java b/.../solver/core/impl/testdata/domain/valuerange/anonymous/TestdataAnonymousListSolution.java
@@ -64,7 +64,7 @@ public List<Long> createLongList() {
     }
 
     @ValueRangeProvider
-    public List<Number> createNumberList() {
+    public List<? super Number> createNumberList() { // Test the wildcards too.
         return List.of(0, BigInteger.TEN);
     }