feat: new SolutionUpdatePolicy to force update of shadow vars

core/src/main/java/ai/timefold/solver/core/api/solver/SolutionManager.java

@@ -111,8 +111,8 @@ static <Solution_> void updateShadowVariables(@NonNull Class<Solution_> solution
     }
 
     /**
-     * Same as {@link #updateShadowVariables(Class, Object...)},
-     * this method accepts a solution rather than a list of entities.
+     * Equivalent to {@link SolutionManager#update(Object, SolutionUpdatePolicy)}
+     * with {@link SolutionUpdatePolicy#RESET_SHADOW_VARIABLES_ONLY}.
      *
      * @param solution the solution
      */

core/src/main/java/ai/timefold/solver/core/api/solver/SolutionUpdatePolicy.java

@@ -40,11 +40,23 @@ public enum SolutionUpdatePolicy {
     /**
      * Runs variable listeners on all planning entities and problem facts,
      * updates shadow variables.
+     * Assumes all shadow variables are unset;
+     * for solutions where some shadow variables are already filled in,
+     * use {@link #RESET_SHADOW_VARIABLES_ONLY} instead.
+     * <p>
      * Does not update score;
      * the solution will keep the current score, even if it is stale or null.
      * To avoid this, use {@link #UPDATE_ALL} instead.
      */
     UPDATE_SHADOW_VARIABLES_ONLY(false, true),
+    /**
+     * A specialized variant of {@link #UPDATE_SHADOW_VARIABLES_ONLY}, which clears the existing shadow variables first.
+     * This is significantly slower than the former,
+     * but it avoids an issue with dependent shadow variables not being recomputed
+     * if their source value did not change.
+     * This is only useful for solutions where some shadow variables are already filled in.
+     */
+    RESET_SHADOW_VARIABLES_ONLY(false, true),
     /**
      * Does not run anything.
      * Improves performance during {@link SolutionManager#analyze(Object, ScoreAnalysisFetchPolicy, SolutionUpdatePolicy)}
@@ -75,4 +87,9 @@ public boolean isScoreUpdateEnabled() {
     public boolean isShadowVariableUpdateEnabled() {
         return shadowVariableUpdateEnabled;
     }
+
+    public boolean isShadowVariableUpdateForced() {
+        return isShadowVariableUpdateEnabled() && this == RESET_SHADOW_VARIABLES_ONLY;
+    }
+
 }

core/src/main/java/ai/timefold/solver/core/impl/domain/common/ReflectionHelper.java

@@ -136,7 +136,7 @@ public static Method getDeclaredGetterMethod(Class<?> containingClass, String pr
         var baseClass = containingClass;
         while (baseClass != null) {
             try {
-                return containingClass.getDeclaredMethod(getterName);
+                return baseClass.getDeclaredMethod(getterName);
             } catch (NoSuchMethodException e) {
                 baseClass = baseClass.getSuperclass();
             }
@@ -158,12 +158,12 @@ public static Method getDeclaredGetterMethod(Class<?> containingClass, String pr
      * @param methodName never null
      * @return sometimes null
      */
-    public static Method getDeclaredMethod(Class<?> containingClass, String methodName) {
+    public static Method getDeclaredMethod(Class<?> containingClass, String methodName, Class<?>... parameterTypes) {
         var baseClass = containingClass;
 
         while (baseClass != null) {
             try {
-                return containingClass.getDeclaredMethod(methodName);
+                return baseClass.getDeclaredMethod(methodName, parameterTypes);
             } catch (NoSuchMethodException e) {
                 baseClass = baseClass.getSuperclass();
             }
@@ -233,6 +233,17 @@ public static Method getSetterMethod(Class<?> containingClass, Class<?> property
         }
     }
 
+    /**
+     * @param containingClass never null
+     * @param propertyType never null
+     * @param propertyName never null
+     * @return null if it doesn't exist
+     */
+    public static Method getDeclaredSetterMethod(Class<?> containingClass, Class<?> propertyType, String propertyName) {
+        String setterName = PROPERTY_MUTATOR_PREFIX + capitalizePropertyName(propertyName);
+        return getDeclaredMethod(containingClass, setterName, propertyType);
+    }
+
     /**
      * @param containingClass never null
      * @param propertyName never null

core/src/main/java/ai/timefold/solver/core/impl/domain/common/accessor/ReflectionBeanPropertyMemberAccessor.java

@@ -4,7 +4,9 @@
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.lang.reflect.Type;
+import java.util.function.IntPredicate;
 
 import ai.timefold.solver.core.impl.domain.common.ReflectionHelper;
 
@@ -34,33 +36,38 @@ public ReflectionBeanPropertyMemberAccessor(Method getterMethod, boolean getterO
         } catch (IllegalAccessException e) {
             throw new IllegalStateException("""
                     Impossible state: method (%s) not accessible.
-                    %s
-                    """
-                    .strip()
+                    %s"""
                     .formatted(getterMethod, MemberAccessorFactory.CLASSLOADER_NUDGE_MESSAGE), e);
         }
         Class<?> declaringClass = getterMethod.getDeclaringClass();
         if (!ReflectionHelper.isGetterMethod(getterMethod)) {
-            throw new IllegalArgumentException("The getterMethod (" + getterMethod + ") is not a valid getter.");
+            throw new IllegalArgumentException("The getterMethod (%s) is not a valid getter."
+                    .formatted(getterMethod));
         }
         propertyType = getterMethod.getReturnType();
         propertyName = ReflectionHelper.getGetterPropertyName(getterMethod);
         if (getterOnly) {
             setterMethod = null;
             setterMethodHandle = null;
         } else {
-            setterMethod = ReflectionHelper.getSetterMethod(declaringClass, getterMethod.getReturnType(), propertyName);
+            setterMethod = ReflectionHelper.getDeclaredSetterMethod(declaringClass, getterMethod.getReturnType(), propertyName);
             if (setterMethod != null) {
+                var getterAccess = AccessModifier.forMethod(getterMethod);
+                var setterAccess = AccessModifier.forMethod(setterMethod);
+                if (getterAccess != setterAccess) {
+                    throw new IllegalArgumentException(
+                            "The getter method (%s) has access modifier (%s) which does not match the setter method's (%s) access modifier (%s) on class (%s)."
+                                    .formatted(getterMethod.getName(), getterAccess, setterMethod.getName(), setterAccess,
+                                            getterMethod.getDeclaringClass().getSimpleName()));
+                }
                 try {
                     setterMethod.setAccessible(true); // Performance hack by avoiding security checks
                     this.setterMethodHandle = lookup.unreflect(setterMethod)
                             .asFixedArity();
                 } catch (IllegalAccessException e) {
                     throw new IllegalStateException("""
                             Impossible state: method (%s) not accessible.
-                            %s
-                            """
-                            .strip()
+                            %s"""
                             .formatted(setterMethod, MemberAccessorFactory.CLASSLOADER_NUDGE_MESSAGE), e);
                 }
             } else {
@@ -69,6 +76,36 @@ public ReflectionBeanPropertyMemberAccessor(Method getterMethod, boolean getterO
         }
     }
 
+    private enum AccessModifier {
+        PUBLIC("public", Modifier::isPublic),
+        PROTECTED("protected", Modifier::isProtected),
+        PACKAGE_PRIVATE("package-private", modifier -> false),
+        PRIVATE("private", Modifier::isPrivate);
+
+        final String name;
+        final IntPredicate predicate;
+
+        AccessModifier(String name, IntPredicate predicate) {
+            this.name = name;
+            this.predicate = predicate;
+        }
+
+        public static AccessModifier forMethod(Method method) {
+            var modifiers = method.getModifiers();
+            for (var accessModifier : AccessModifier.values()) {
+                if (accessModifier.predicate.test(modifiers)) {
+                    return accessModifier;
+                }
+            }
+            return PACKAGE_PRIVATE;
+        }
+
+        @Override
+        public String toString() {
+            return name;
+        }
+    }
+
     @Override
     public Class<?> getDeclaringClass() {
         return getterMethod.getDeclaringClass();

core/src/main/java/ai/timefold/solver/core/impl/domain/entity/descriptor/EntityDescriptor.java

@@ -1,6 +1,5 @@
 package ai.timefold.solver.core.impl.domain.entity.descriptor;
 
-import static ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessorFactory.MemberAccessorType.FIELD_OR_GETTER_METHOD;
 import static ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessorFactory.MemberAccessorType.FIELD_OR_GETTER_METHOD_WITH_SETTER;
 import static ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessorFactory.MemberAccessorType.FIELD_OR_READ_METHOD;
 import static ai.timefold.solver.core.impl.domain.entity.descriptor.EntityDescriptorValidator.assertNotMixedInheritance;
@@ -46,7 +45,6 @@
 import ai.timefold.solver.core.config.util.ConfigUtils;
 import ai.timefold.solver.core.impl.domain.common.ReflectionHelper;
 import ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessor;
-import ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessorFactory;
 import ai.timefold.solver.core.impl.domain.policy.DescriptorPolicy;
 import ai.timefold.solver.core.impl.domain.solution.descriptor.ProblemScaleTracker;
 import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor;
@@ -323,44 +321,27 @@ private void processValueRangeProviderAnnotation(DescriptorPolicy descriptorPoli
         }
     }
 
+    @SuppressWarnings("unchecked")
     private void processPlanningVariableAnnotation(MutableInt variableDescriptorCounter, DescriptorPolicy descriptorPolicy,
             Member member) {
-        var variableAnnotationClass = ConfigUtils.extractAnnotationClass(
-                member, VARIABLE_ANNOTATION_CLASSES);
+        var variableAnnotationClass = ConfigUtils.extractAnnotationClass(member, VARIABLE_ANNOTATION_CLASSES);
         if (variableAnnotationClass != null) {
-            MemberAccessorFactory.MemberAccessorType memberAccessorType;
-            if (variableAnnotationClass.equals(ShadowVariable.class)) {
-                // Need to check only the single annotation version,
-                // since supplier variable can only be used with a single
-                // annotation.
-                ShadowVariable annotation;
-                if (member instanceof Field field) {
-                    annotation = field.getAnnotation(ShadowVariable.class);
-                } else if (member instanceof Method method) {
-                    annotation = method.getAnnotation(ShadowVariable.class);
-                } else {
-                    throw new IllegalStateException(
-                            "Member must be a field or a method, but was (%s).".formatted(member.getClass().getSimpleName()));
-                }
-                if (annotation == null) {
-                    throw new IllegalStateException("Impossible state: cannot get %s annotation on a %s annotated member (%s)."
-                            .formatted(ShadowVariable.class.getSimpleName(), ShadowVariable.class.getSimpleName(), member));
-                }
-                if (!annotation.supplierName().isEmpty()) {
-                    memberAccessorType = FIELD_OR_GETTER_METHOD_WITH_SETTER;
-                } else {
-                    memberAccessorType = FIELD_OR_GETTER_METHOD;
-                }
-            } else if (variableAnnotationClass.equals(CustomShadowVariable.class)
-                    || variableAnnotationClass.equals(ShadowVariable.List.class)
-                    || variableAnnotationClass.equals(PiggybackShadowVariable.class)
-                    || variableAnnotationClass.equals(CascadingUpdateShadowVariable.class)) {
-                memberAccessorType = FIELD_OR_GETTER_METHOD;
+            Object annotation;
+            if (member instanceof Field field) {
+                annotation = field.getAnnotation(variableAnnotationClass);
+            } else if (member instanceof Method method) {
+                annotation = method.getAnnotation(variableAnnotationClass);
             } else {
-                memberAccessorType = FIELD_OR_GETTER_METHOD_WITH_SETTER;
+                throw new IllegalStateException("Member must be a field or a method, but was (%s)."
+                        .formatted(member.getClass().getSimpleName()));
             }
-            var memberAccessor = descriptorPolicy.getMemberAccessorFactory().buildAndCacheMemberAccessor(member,
-                    memberAccessorType, variableAnnotationClass, descriptorPolicy.getDomainAccessType());
+            if (annotation == null) {
+                throw new IllegalStateException("Impossible state: cannot get annotation on a %s-annotated member (%s)."
+                        .formatted(variableAnnotationClass, member));
+            }
+            var memberAccessor = descriptorPolicy.getMemberAccessorFactory()
+                    .buildAndCacheMemberAccessor(member, FIELD_OR_GETTER_METHOD_WITH_SETTER, variableAnnotationClass,
+                            descriptorPolicy.getDomainAccessType());
             registerVariableAccessor(variableDescriptorCounter.intValue(), variableAnnotationClass, memberAccessor);
             variableDescriptorCounter.increment();
         }
@@ -695,10 +676,6 @@ public GenuineVariableDescriptor<Solution_> getGenuineVariableDescriptor(String
         return shadowVariableLoopedDescriptor;
     }
 
-    public boolean hasAnyGenuineVariables() {
-        return !effectiveGenuineVariableDescriptorMap.isEmpty();
-    }
-
     public boolean hasBothGenuineListAndBasicVariables() {
         if (!isGenuine()) {
             return false;
@@ -732,7 +709,7 @@ public boolean hasAnyGenuineListVariables() {
     }
 
     public boolean isGenuine() {
-        return hasAnyGenuineVariables();
+        return !effectiveGenuineVariableDescriptorMap.isEmpty();
     }
 
     public ListVariableDescriptor<Solution_> getGenuineListVariableDescriptor() {

core/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java

@@ -99,6 +99,16 @@ public class SolutionDescriptor<Solution_> {
             PlanningEntityCollectionProperty.class,
             PlanningScore.class };
 
+    @SuppressWarnings("unchecked")
+    public static <Solution_> SolutionDescriptor<Solution_> buildSolutionDescriptorFromSolution(Solution_ solution) {
+        var enabledPreviewFeatures = EnumSet.of(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES);
+        var solutionClass = (Class<Solution_>) solution.getClass();
+        var initialSolutionDescriptor = buildSolutionDescriptor(enabledPreviewFeatures, solutionClass);
+        var entityClassSet = new LinkedHashSet<Class<?>>();
+        initialSolutionDescriptor.visitAllEntities(solution, e -> entityClassSet.add(e.getClass()));
+        return buildSolutionDescriptor(enabledPreviewFeatures, solutionClass, entityClassSet.toArray(new Class<?>[0]));
+    }
+
     public static <Solution_> SolutionDescriptor<Solution_> buildSolutionDescriptor(Class<Solution_> solutionClass,
             Class<?>... entityClasses) {
         return buildSolutionDescriptor(solutionClass, Arrays.asList(entityClasses));

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java

@@ -52,6 +52,13 @@
  */
 public final class ShadowVariableUpdateHelper<Solution_> {
 
+    /**
+     * Deducing the solution descriptors from the solution instance is very expensive.
+     * Cache the solution descriptors to avoid this,
+     * even at the unlikely expense of keeping solution descriptors in memory for solutions which are no longer used.
+     */
+    private static final IdentityHashMap<Class<?>, SolutionDescriptor<?>> SOLUTION_DESCRIPTOR_CACHE = new IdentityHashMap<>();
+
     private static final EnumSet<ShadowVariableType> SUPPORTED_TYPES =
             EnumSet.of(BASIC, CUSTOM_LISTENER, CASCADING_UPDATE, DECLARATIVE);
 
@@ -74,21 +81,12 @@ private ShadowVariableUpdateHelper(EnumSet<ShadowVariableType> supportedShadowVa
 
     @SuppressWarnings("unchecked")
     public void updateShadowVariables(Solution_ solution) {
-        var enabledPreviewFeatures = EnumSet.of(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES);
-        var solutionClass = (Class<Solution_>) solution.getClass();
-        var initialSolutionDescriptor = SolutionDescriptor.buildSolutionDescriptor(
-                enabledPreviewFeatures, solutionClass);
-        var entityClassArray = initialSolutionDescriptor.getAllEntitiesAndProblemFacts(solution)
-                .stream()
-                .map(Object::getClass)
-                .distinct()
-                .toArray(Class[]::new);
-        var solutionDescriptor = SolutionDescriptor.buildSolutionDescriptor(enabledPreviewFeatures, solutionClass,
-                entityClassArray);
+        var solutionDescriptor = (SolutionDescriptor<Solution_>) SOLUTION_DESCRIPTOR_CACHE.computeIfAbsent(solution.getClass(),
+                solutionClass -> SolutionDescriptor.buildSolutionDescriptorFromSolution(solution));
         try (var scoreDirector = new InternalScoreDirector<>(solutionDescriptor)) {
             // When we have a solution, we can reuse the logic from VariableListenerSupport to update all variable types
             scoreDirector.setWorkingSolution(solution);
-            scoreDirector.forceTriggerVariableListeners();
+            scoreDirector.forceTriggerVariableListeners(true);
         }
     }