From f72223f2f8eaeb82e506efb08be28b8517f6afba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Fri, 30 May 2025 11:55:05 +0200 Subject: [PATCH 1/5] feat: new SolutionUpdatePolicy to force update of shadow vars --- .../core/api/solver/SolutionManager.java | 4 +- .../core/api/solver/SolutionUpdatePolicy.java | 17 +++++++ .../descriptor/SolutionDescriptor.java | 10 ++++ .../variable/ShadowVariableUpdateHelper.java | 22 ++++---- .../score/director/AbstractScoreDirector.java | 50 ++++++++++++++++++- .../score/director/InnerScoreDirector.java | 5 +- .../impl/solver/DefaultSolutionManager.java | 2 +- .../variable/ListVariableListenerTest.java | 2 +- .../custom/CustomVariableListenerTest.java | 2 +- ...VariableAwareMultiConstraintAssertion.java | 3 +- ...ariableAwareSingleConstraintAssertion.java | 3 +- 11 files changed, 98 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/api/solver/SolutionManager.java b/core/src/main/java/ai/timefold/solver/core/api/solver/SolutionManager.java index 13712380b7..9a960422df 100644 --- a/core/src/main/java/ai/timefold/solver/core/api/solver/SolutionManager.java +++ b/core/src/main/java/ai/timefold/solver/core/api/solver/SolutionManager.java @@ -111,8 +111,8 @@ static void updateShadowVariables(@NonNull Class 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 */ diff --git a/core/src/main/java/ai/timefold/solver/core/api/solver/SolutionUpdatePolicy.java b/core/src/main/java/ai/timefold/solver/core/api/solver/SolutionUpdatePolicy.java index 8b4d8d8d97..abf7b000ca 100644 --- a/core/src/main/java/ai/timefold/solver/core/api/solver/SolutionUpdatePolicy.java +++ b/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 an uninitialized solution; + * for solutions where some shadow variables are already filled in, + * use {@link #RESET_SHADOW_VARIABLES_ONLY} instead. + *

* 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; + } + } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java index 08d00e6f65..d9116b78e0 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java @@ -99,6 +99,16 @@ public class SolutionDescriptor { PlanningEntityCollectionProperty.class, PlanningScore.class }; + @SuppressWarnings("unchecked") + public static SolutionDescriptor buildSolutionDescriptorFromSolution(Solution_ solution) { + var enabledPreviewFeatures = EnumSet.of(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES); + var solutionClass = (Class) solution.getClass(); + var initialSolutionDescriptor = buildSolutionDescriptor(enabledPreviewFeatures, solutionClass); + var entityClassSet = new LinkedHashSet>(); + initialSolutionDescriptor.visitAllEntities(solution, e -> entityClassSet.add(e.getClass())); + return buildSolutionDescriptor(enabledPreviewFeatures, solutionClass, entityClassSet.toArray(new Class[0])); + } + public static SolutionDescriptor buildSolutionDescriptor(Class solutionClass, Class... entityClasses) { return buildSolutionDescriptor(solutionClass, Arrays.asList(entityClasses)); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java index f2c8d6e72e..ed5d33a6a5 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/ShadowVariableUpdateHelper.java @@ -52,6 +52,13 @@ */ public final class ShadowVariableUpdateHelper { + /** + * 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, SolutionDescriptor> SOLUTION_DESCRIPTOR_CACHE = new IdentityHashMap<>(); + private static final EnumSet SUPPORTED_TYPES = EnumSet.of(BASIC, CUSTOM_LISTENER, CASCADING_UPDATE, DECLARATIVE); @@ -74,21 +81,12 @@ private ShadowVariableUpdateHelper(EnumSet supportedShadowVa @SuppressWarnings("unchecked") public void updateShadowVariables(Solution_ solution) { - var enabledPreviewFeatures = EnumSet.of(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES); - var solutionClass = (Class) 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_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); } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java index eb37224e72..7852d035a5 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java @@ -2,6 +2,7 @@ import static java.util.Objects.requireNonNull; +import java.util.Collection; import java.util.Collections; import java.util.IdentityHashMap; import java.util.LinkedHashSet; @@ -27,6 +28,7 @@ import ai.timefold.solver.core.impl.domain.variable.ListVariableStateSupply; import ai.timefold.solver.core.impl.domain.variable.descriptor.ListVariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor; +import ai.timefold.solver.core.impl.domain.variable.inverserelation.InverseRelationShadowVariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.listener.support.VariableListenerSupport; import ai.timefold.solver.core.impl.domain.variable.listener.support.violation.SolutionTracker; import ai.timefold.solver.core.impl.domain.variable.supply.SupplyManager; @@ -358,8 +360,52 @@ protected void clearVariableListenerEvents() { } @Override - public void forceTriggerVariableListeners() { - variableListenerSupport.forceTriggerAllVariableListeners(getWorkingSolution()); + public void forceTriggerVariableListeners(boolean resetShadowVariablesFirst) { + var solution = getWorkingSolution(); + if (resetShadowVariablesFirst) { + var solutionDescriptor = getSolutionDescriptor(); + solutionDescriptor.visitAllEntities(solution, entity -> { + var entityDescriptor = solutionDescriptor.findEntityDescriptor(entity.getClass()); + var shadowVariableDescriptors = entityDescriptor.getShadowVariableDescriptors(); + if (shadowVariableDescriptors.isEmpty()) { + return; + } + for (var shadowVariableDescriptor : shadowVariableDescriptors) { + var value = shadowVariableDescriptor.getValue(entity); + if (value == null) { + continue; + } + if (shadowVariableDescriptor instanceof InverseRelationShadowVariableDescriptor + && value instanceof Collection collection) { + // Inverse collection must never be null, so just empty it. + collection.clear(); + continue; + } + var propertyType = shadowVariableDescriptor.getVariablePropertyType(); + if (propertyType.isPrimitive()) { + if (propertyType == boolean.class) { + shadowVariableDescriptor.setValue(entity, false); + } else if (propertyType == byte.class) { + shadowVariableDescriptor.setValue(entity, (byte) 0); + } else if (propertyType == char.class) { + shadowVariableDescriptor.setValue(entity, '\u0000'); + } else if (propertyType == short.class) { + shadowVariableDescriptor.setValue(entity, (short) 0); + } else if (propertyType == int.class) { + shadowVariableDescriptor.setValue(entity, 0); + } else if (propertyType == long.class) { + shadowVariableDescriptor.setValue(entity, 0L); + } else { + throw new IllegalStateException( + "Impossible state: unknown primitive type %s".formatted(propertyType)); + } + } else { + shadowVariableDescriptor.setValue(entity, null); + } + } + }); + } + variableListenerSupport.forceTriggerAllVariableListeners(solution); } protected void setCalculatedScore(Score_ score) { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java index a7c3d7b033..248c007933 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java @@ -349,8 +349,11 @@ void assertExpectedUndoMoveScore(Move move, InnerScore before * Unlike {@link #triggerVariableListeners()} which only triggers notifications already in the queue, * this triggers every variable listener on every genuine variable. * This is useful in {@link SolutionManager#update(Object)} to fill in shadow variable values. + * + * @param resetShadowVariablesFirst true if the shadow variables should be reset first, + * slowing down the operation but preventing stale dependencies. */ - void forceTriggerVariableListeners(); + void forceTriggerVariableListeners(boolean resetShadowVariablesFirst); /** * A derived score director is created from a root score director. diff --git a/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolutionManager.java b/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolutionManager.java index 73bb531043..5e9fd85c5d 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolutionManager.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolutionManager.java @@ -78,7 +78,7 @@ private Result_ callScoreDirector(Solution_ solution, Maybe use Constraint Streams instead of Easy or Incremental score calculator?"""); } if (isShadowVariableUpdateEnabled) { - scoreDirector.forceTriggerVariableListeners(); + scoreDirector.forceTriggerVariableListeners(solutionUpdatePolicy.isShadowVariableUpdateForced()); } if (solutionUpdatePolicy.isScoreUpdateEnabled()) { scoreDirector.calculateScore(); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/ListVariableListenerTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/ListVariableListenerTest.java index 176bc7eced..21f73ed34f 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/ListVariableListenerTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/ListVariableListenerTest.java @@ -122,7 +122,7 @@ void addAndRemoveEntity() { var ann = new TestdataListEntityWithShadowHistory("Ann", a, b, c); scoreDirector.setWorkingSolution(buildSolution(ann)); - scoreDirector.forceTriggerVariableListeners(); + scoreDirector.forceTriggerVariableListeners(false); // Assert inverse entity. assertEntityHistory(a, ann); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/custom/CustomVariableListenerTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/custom/CustomVariableListenerTest.java index 2cb2d38de1..126d86645e 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/custom/CustomVariableListenerTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/variable/custom/CustomVariableListenerTest.java @@ -185,7 +185,7 @@ void manyToManyRequiresUniqueEntityEvents() { solution.setEntityList(new ArrayList<>()); solution.setValueList(List.of(val1)); scoreDirector.setWorkingSolution(solution); - scoreDirector.forceTriggerVariableListeners(); + scoreDirector.forceTriggerVariableListeners(true); scoreDirector.beforeEntityAdded(b); scoreDirector.getWorkingSolution().getEntityList().add(b); diff --git a/test/src/main/java/ai/timefold/solver/test/impl/score/stream/DefaultShadowVariableAwareMultiConstraintAssertion.java b/test/src/main/java/ai/timefold/solver/test/impl/score/stream/DefaultShadowVariableAwareMultiConstraintAssertion.java index 601c2df04d..84b132ff0f 100644 --- a/test/src/main/java/ai/timefold/solver/test/impl/score/stream/DefaultShadowVariableAwareMultiConstraintAssertion.java +++ b/test/src/main/java/ai/timefold/solver/test/impl/score/stream/DefaultShadowVariableAwareMultiConstraintAssertion.java @@ -6,6 +6,7 @@ import ai.timefold.solver.core.api.score.Score; import ai.timefold.solver.core.api.score.stream.ConstraintProvider; +import ai.timefold.solver.core.api.solver.SolutionManager; import ai.timefold.solver.core.impl.score.constraint.ConstraintMatchPolicy; import ai.timefold.solver.core.impl.score.stream.common.AbstractConstraintStreamScoreDirectorFactory; import ai.timefold.solver.test.api.score.stream.MultiConstraintAssertion; @@ -28,11 +29,11 @@ public final class DefaultShadowVariableAwareMultiConstraintAssertion Date: Mon, 9 Jun 2025 12:46:42 +0200 Subject: [PATCH 2/5] Refactor and add test coverage # Conflicts: # core/src/main/java/ai/timefold/solver/core/impl/domain/entity/descriptor/EntityDescriptor.java --- .../entity/descriptor/EntityDescriptor.java | 57 +++++----------- .../support/VariableListenerSupport.java | 66 +++++++++++++++++-- .../score/director/AbstractScoreDirector.java | 54 ++------------- .../stream/bavet/BavetConstraintFactory.java | 2 +- .../core/api/solver/SolutionManagerTest.java | 39 +++++++++++ ...ldProcessorGeneratedGizmoSupplierTest.java | 4 ++ 6 files changed, 127 insertions(+), 95 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/entity/descriptor/EntityDescriptor.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/entity/descriptor/EntityDescriptor.java index 62664e3535..47ba0f663e 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/entity/descriptor/EntityDescriptor.java +++ b/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 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 getGenuineListVariableDescriptor() { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java index a3f5870594..606a66cff2 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java @@ -425,7 +425,7 @@ private void cascadeUnassignedValues( var snapshot = ShadowVariablesAssert.takeSnapshot(scoreDirector.getSolutionDescriptor(), workingSolution); - forceTriggerAllVariableListeners(workingSolution); + forceTriggerAllVariableListeners(workingSolution, false); return snapshot.createShadowVariablesViolationMessage(SHADOW_VARIABLE_VIOLATION_DISPLAY_LIMIT); } @@ -439,9 +439,11 @@ private void cascadeUnassignedValues( * triggering listeners at this point must not change any shadow variables either. * * @param workingSolution working solution + * @param resetShadowVariables true to set all shadow variable values to null */ - public void forceTriggerAllVariableListeners(Solution_ workingSolution) { - scoreDirector.getSolutionDescriptor().visitAllEntities(workingSolution, this::simulateGenuineVariableChange); + public void forceTriggerAllVariableListeners(Solution_ workingSolution, boolean resetShadowVariables) { + scoreDirector.getSolutionDescriptor().visitAllEntities(workingSolution, + e -> simulateGenuineVariableChange(e, resetShadowVariables)); triggerVariableListenersInNotificationQueues(); } @@ -454,12 +456,13 @@ public void clearAllVariableListenerEvents() { notificationQueuesAreEmpty = true; } - private void simulateGenuineVariableChange(Object entity) { - var entityDescriptor = scoreDirector.getSolutionDescriptor() - .findEntityDescriptorOrFail(entity.getClass()); - if (!entityDescriptor.isGenuine()) { + private void simulateGenuineVariableChange(Object entity, boolean resetShadowVariables) { + var solutionDescriptor = scoreDirector.getSolutionDescriptor(); + var entityDescriptor = solutionDescriptor.findEntityDescriptorOrFail(entity.getClass()); + if (!entityDescriptor.isGenuine() && !resetShadowVariables) { return; } + for (var variableDescriptor : entityDescriptor.getGenuineVariableDescriptorList()) { if (variableDescriptor.isListVariable()) { var descriptor = (ListVariableDescriptor) variableDescriptor; @@ -471,6 +474,55 @@ private void simulateGenuineVariableChange(Object entity) { beforeVariableChanged(variableDescriptor, entity); } } + + if (!resetShadowVariables) { + return; + } + + var shadowVariableDescriptors = entityDescriptor.getShadowVariableDescriptors(); + if (shadowVariableDescriptors.isEmpty()) { + return; + } + + for (var shadowVariableDescriptor : shadowVariableDescriptors) { + clearShadowVariable(shadowVariableDescriptor, entity); + } + } + + private static void clearShadowVariable(ShadowVariableDescriptor shadowVariableDescriptor, Object entity) { + var value = shadowVariableDescriptor.getValue(entity); + if (value == null) { + return; + } + + if (shadowVariableDescriptor instanceof InverseRelationShadowVariableDescriptor + && value instanceof Collection collection) { + // Inverse collection must never be null, so just empty it. + collection.clear(); + return; + } + + var propertyType = shadowVariableDescriptor.getVariablePropertyType(); + if (propertyType.isPrimitive()) { + if (propertyType == boolean.class) { + shadowVariableDescriptor.setValue(entity, false); + } else if (propertyType == byte.class) { + shadowVariableDescriptor.setValue(entity, (byte) 0); + } else if (propertyType == char.class) { + shadowVariableDescriptor.setValue(entity, '\u0000'); + } else if (propertyType == short.class) { + shadowVariableDescriptor.setValue(entity, (short) 0); + } else if (propertyType == int.class) { + shadowVariableDescriptor.setValue(entity, 0); + } else if (propertyType == long.class) { + shadowVariableDescriptor.setValue(entity, 0L); + } else { + throw new IllegalStateException( + "Impossible state: unknown primitive type %s".formatted(propertyType)); + } + } else { + shadowVariableDescriptor.setValue(entity, null); + } } public void assertNotificationQueuesAreEmpty() { diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java index 7852d035a5..0f76907d4b 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java @@ -2,7 +2,6 @@ import static java.util.Objects.requireNonNull; -import java.util.Collection; import java.util.Collections; import java.util.IdentityHashMap; import java.util.LinkedHashSet; @@ -28,7 +27,6 @@ import ai.timefold.solver.core.impl.domain.variable.ListVariableStateSupply; import ai.timefold.solver.core.impl.domain.variable.descriptor.ListVariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor; -import ai.timefold.solver.core.impl.domain.variable.inverserelation.InverseRelationShadowVariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.listener.support.VariableListenerSupport; import ai.timefold.solver.core.impl.domain.variable.listener.support.violation.SolutionTracker; import ai.timefold.solver.core.impl.domain.variable.supply.SupplyManager; @@ -360,52 +358,14 @@ protected void clearVariableListenerEvents() { } @Override - public void forceTriggerVariableListeners(boolean resetShadowVariablesFirst) { + public void forceTriggerVariableListeners(boolean resetShadowVariables) { var solution = getWorkingSolution(); - if (resetShadowVariablesFirst) { + if (resetShadowVariables) { var solutionDescriptor = getSolutionDescriptor(); solutionDescriptor.visitAllEntities(solution, entity -> { - var entityDescriptor = solutionDescriptor.findEntityDescriptor(entity.getClass()); - var shadowVariableDescriptors = entityDescriptor.getShadowVariableDescriptors(); - if (shadowVariableDescriptors.isEmpty()) { - return; - } - for (var shadowVariableDescriptor : shadowVariableDescriptors) { - var value = shadowVariableDescriptor.getValue(entity); - if (value == null) { - continue; - } - if (shadowVariableDescriptor instanceof InverseRelationShadowVariableDescriptor - && value instanceof Collection collection) { - // Inverse collection must never be null, so just empty it. - collection.clear(); - continue; - } - var propertyType = shadowVariableDescriptor.getVariablePropertyType(); - if (propertyType.isPrimitive()) { - if (propertyType == boolean.class) { - shadowVariableDescriptor.setValue(entity, false); - } else if (propertyType == byte.class) { - shadowVariableDescriptor.setValue(entity, (byte) 0); - } else if (propertyType == char.class) { - shadowVariableDescriptor.setValue(entity, '\u0000'); - } else if (propertyType == short.class) { - shadowVariableDescriptor.setValue(entity, (short) 0); - } else if (propertyType == int.class) { - shadowVariableDescriptor.setValue(entity, 0); - } else if (propertyType == long.class) { - shadowVariableDescriptor.setValue(entity, 0L); - } else { - throw new IllegalStateException( - "Impossible state: unknown primitive type %s".formatted(propertyType)); - } - } else { - shadowVariableDescriptor.setValue(entity, null); - } - } }); } - variableListenerSupport.forceTriggerAllVariableListeners(solution); + variableListenerSupport.forceTriggerAllVariableListeners(solution, resetShadowVariables); } protected void setCalculatedScore(Score_ score) { @@ -778,13 +738,13 @@ public void assertExpectedUndoMoveScore(Move move, InnerScore // We cannot set all shadow variables to null, since some variable listeners // may expect them to be non-null. // Instead, we just simulate a change to all genuine variables. - variableListenerSupport.forceTriggerAllVariableListeners(workingSolution); + variableListenerSupport.forceTriggerAllVariableListeners(workingSolution, false); solutionTracker.setUndoFromScratchSolution(workingSolution); // Also calculate from scratch for the before solution, since it might // have been corrupted but was only detected now solutionTracker.restoreBeforeSolution(); - variableListenerSupport.forceTriggerAllVariableListeners(workingSolution); + variableListenerSupport.forceTriggerAllVariableListeners(workingSolution, false); solutionTracker.setBeforeFromScratchSolution(workingSolution); corruptionDiagnosis = solutionTracker.buildScoreCorruptionMessage(); @@ -843,13 +803,13 @@ public SolutionTracker.SolutionCorruptionResult getSolutionCorruptionAfterUndo(M // We cannot set all shadow variables to null, since some variable listeners // may expect them to be non-null. // Instead, we just simulate a change to all genuine variables. - variableListenerSupport.forceTriggerAllVariableListeners(workingSolution); + variableListenerSupport.forceTriggerAllVariableListeners(workingSolution, false); solutionTracker.setUndoFromScratchSolution(workingSolution); // Also calculate from scratch for the before solution, since it might // have been corrupted but was only detected now solutionTracker.restoreBeforeSolution(); - variableListenerSupport.forceTriggerAllVariableListeners(workingSolution); + variableListenerSupport.forceTriggerAllVariableListeners(workingSolution, false); solutionTracker.setBeforeFromScratchSolution(workingSolution); return solutionTracker.buildSolutionCorruptionResult(); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/stream/bavet/BavetConstraintFactory.java b/core/src/main/java/ai/timefold/solver/core/impl/score/stream/bavet/BavetConstraintFactory.java index 6acb8eaa7d..22a39a4fc9 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/stream/bavet/BavetConstraintFactory.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/stream/bavet/BavetConstraintFactory.java @@ -135,7 +135,7 @@ public > Stream_ share( public @NonNull UniConstraintStream from(@NonNull Class fromClass) { assertValidFromType(fromClass); var entityDescriptor = solutionDescriptor.findEntityDescriptor(fromClass); - if (entityDescriptor != null && entityDescriptor.hasAnyGenuineVariables()) { + if (entityDescriptor != null && entityDescriptor.isGenuine()) { var predicate = (Predicate) entityDescriptor.getIsInitializedPredicate(); return share(new BavetForEachUniConstraintStream<>(this, fromClass, predicate, RetrievalSemantics.LEGACY)); } else { diff --git a/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java b/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java index dfa7e87909..ecfa70eb4b 100644 --- a/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java +++ b/core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java @@ -1,5 +1,6 @@ package ai.timefold.solver.core.api.solver; +import static ai.timefold.solver.core.api.solver.SolutionUpdatePolicy.RESET_SHADOW_VARIABLES_ONLY; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.SoftAssertions.assertSoftly; @@ -118,6 +119,14 @@ void updateEverything(SolutionManagerSource SolutionManagerSource) { softly.assertThat(solution.getScore()).isNotNull(); softly.assertThat(solution.getEntityList().get(0).getFirstShadow()).isNotNull(); }); + + var oldScore = solution.getScore(); + solutionManager.update(solution, RESET_SHADOW_VARIABLES_ONLY); + + assertSoftly(softly -> { + softly.assertThat(solution.getScore()).isSameAs(oldScore); + softly.assertThat(solution.getEntityList().get(0).getFirstShadow()).isNotNull(); + }); } @ParameterizedTest @@ -162,6 +171,22 @@ void updateEverythingChained(SolutionManagerSource SolutionManagerSource) { softly.assertThat(b2.getNextEntity()).isNull(); softly.assertThat(c0.getNextEntity()).isNull(); }); + + var oldScore = solution.getScore(); + solutionManager.update(solution, RESET_SHADOW_VARIABLES_ONLY); + + assertSoftly(softly -> { + softly.assertThat(solution.getScore()).isSameAs(oldScore); + softly.assertThat(a0.getNextEntity()).isEqualTo(a1); + softly.assertThat(a1.getAnchor()).isEqualTo(a0); + softly.assertThat(a1.getNextEntity()).isNull(); + softly.assertThat(b0.getNextEntity()).isEqualTo(b1); + softly.assertThat(b1.getAnchor()).isEqualTo(b0); + softly.assertThat(b1.getNextEntity()).isEqualTo(b2); + softly.assertThat(b2.getAnchor()).isEqualTo(b0); + softly.assertThat(b2.getNextEntity()).isNull(); + softly.assertThat(c0.getNextEntity()).isNull(); + }); } @ParameterizedTest @@ -204,6 +229,20 @@ void updateEverythingList(SolutionManagerSource SolutionManagerSource) { assertShadowedListValue(softly, b2, b, 2, b1, null); assertShadowedListValue(softly, c0, c, 0, null, null); }); + + var oldScore = solution.getScore(); + solutionManager.update(solution, RESET_SHADOW_VARIABLES_ONLY); + + assertSoftly(softly -> { + softly.assertThat(solution.getScore()).isSameAs(oldScore); + assertShadowedListValue(softly, a0, a, 0, null, a1); + assertShadowedListValue(softly, a1, a, 1, a0, null); + assertShadowedListValue(softly, b0, b, 0, null, b1); + assertShadowedListValue(softly, b1, b, 1, b0, b2); + assertShadowedListValue(softly, b2, b, 2, b1, null); + assertShadowedListValue(softly, c0, c, 0, null, null); + }); + } private void assertShadowedListValueAllNull(SoftAssertions softly, TestdataListValueWithShadowHistory current) { diff --git a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorGeneratedGizmoSupplierTest.java b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorGeneratedGizmoSupplierTest.java index 183737cb41..8609b2f24f 100644 --- a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorGeneratedGizmoSupplierTest.java +++ b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorGeneratedGizmoSupplierTest.java @@ -99,6 +99,8 @@ public interface DummyInterfaceEntity { @ShadowVariable(variableListenerClass = DummyVariableListener.class, sourceEntityClass = TestdataEntity.class, sourceVariableName = "value") Integer getLength(); + + void setLength(Integer length); } @PlanningEntity @@ -106,6 +108,8 @@ public abstract static class DummyAbstractEntity { @ShadowVariable(variableListenerClass = DummyVariableListener.class, sourceEntityClass = TestdataEntity.class, sourceVariableName = "value") abstract Integer getLength(); + + abstract void setLength(Integer length); } public static class DummySolutionPartitioner implements SolutionPartitioner { From 287a1934b8b325919a576fd03518c63afb40df1a Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Mon, 9 Jun 2025 14:49:33 -0400 Subject: [PATCH 3/5] chore: enfore getter match setter visibility for variables and score --- .../impl/domain/common/ReflectionHelper.java | 17 +++++-- .../ReflectionBeanPropertyMemberAccessor.java | 47 ++++++++++++++++++- ...lectionBeanPropertyMemberAccessorTest.java | 20 ++++++++ ...DifferentGetterSetterVisibilityEntity.java | 30 ++++++++++++ ...stdataBendableBigDecimalScoreSolution.java | 2 +- .../TestdataBendableLongScoreSolution.java | 2 +- .../score/TestdataBendableScoreSolution.java | 2 +- ...HardMediumSoftBigDecimalScoreSolution.java | 2 +- ...stdataHardMediumSoftLongScoreSolution.java | 2 +- .../TestdataHardMediumSoftScoreSolution.java | 2 +- ...stdataHardSoftBigDecimalScoreSolution.java | 2 +- .../TestdataHardSoftLongScoreSolution.java | 2 +- .../score/TestdataHardSoftScoreSolution.java | 2 +- ...ldProcessorGeneratedGizmoSupplierTest.java | 4 +- .../gizmo/TestDataKitchenSinkEntity.java | 2 +- 15 files changed, 122 insertions(+), 16 deletions(-) create mode 100644 core/src/test/java/ai/timefold/solver/core/testdomain/invalid/gettersettervisibility/TestdataDifferentGetterSetterVisibilityEntity.java diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/common/ReflectionHelper.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/common/ReflectionHelper.java index 766d4e2c37..c6483795fc 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/common/ReflectionHelper.java +++ b/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 diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/common/accessor/ReflectionBeanPropertyMemberAccessor.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/common/accessor/ReflectionBeanPropertyMemberAccessor.java index e17878cc25..f6ae0b398e 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/common/accessor/ReflectionBeanPropertyMemberAccessor.java +++ b/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; @@ -49,8 +51,16 @@ public ReflectionBeanPropertyMemberAccessor(Method getterMethod, boolean getterO 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) @@ -69,6 +79,41 @@ 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; + } + } + + private int getAccessLevel(Method method) { + int mask = Modifier.PUBLIC | Modifier.PROTECTED | Modifier.PRIVATE; + return method.getModifiers() & mask; + } + @Override public Class getDeclaringClass() { return getterMethod.getDeclaringClass(); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/common/accessor/ReflectionBeanPropertyMemberAccessorTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/common/accessor/ReflectionBeanPropertyMemberAccessorTest.java index 96688aac15..a32e17ccd7 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/common/accessor/ReflectionBeanPropertyMemberAccessorTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/common/accessor/ReflectionBeanPropertyMemberAccessorTest.java @@ -1,10 +1,12 @@ package ai.timefold.solver.core.impl.domain.common.accessor; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import ai.timefold.solver.core.api.domain.variable.PlanningVariable; import ai.timefold.solver.core.testdomain.TestdataEntity; import ai.timefold.solver.core.testdomain.TestdataValue; +import ai.timefold.solver.core.testdomain.invalid.gettersettervisibility.TestdataDifferentGetterSetterVisibilityEntity; import org.junit.jupiter.api.Test; @@ -26,4 +28,22 @@ void methodAnnotatedEntity() throws NoSuchMethodException { assertThat(e1.getValue()).isSameAs(v2); } + @Test + void getterSetterVisibilityDoesNotMatch() { + assertThatCode(() -> new ReflectionBeanPropertyMemberAccessor( + TestdataDifferentGetterSetterVisibilityEntity.class.getDeclaredMethod("getValue1"))) + .hasMessageContainingAll("getter method (getValue1)", + "has access modifier (public)", + "not match the setter method's (setValue1)", + "access modifier (private)", + "on class (TestdataDifferentGetterSetterVisibilityEntity)"); + assertThatCode(() -> new ReflectionBeanPropertyMemberAccessor( + TestdataDifferentGetterSetterVisibilityEntity.class.getDeclaredMethod("getValue2"))) + .hasMessageContainingAll("getter method (getValue2)", + "has access modifier (package-private)", + "not match the setter method's (setValue2)", + "access modifier (protected)", + "on class (TestdataDifferentGetterSetterVisibilityEntity)"); + } + } diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/invalid/gettersettervisibility/TestdataDifferentGetterSetterVisibilityEntity.java b/core/src/test/java/ai/timefold/solver/core/testdomain/invalid/gettersettervisibility/TestdataDifferentGetterSetterVisibilityEntity.java new file mode 100644 index 0000000000..d4f5205f0a --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/invalid/gettersettervisibility/TestdataDifferentGetterSetterVisibilityEntity.java @@ -0,0 +1,30 @@ +package ai.timefold.solver.core.testdomain.invalid.gettersettervisibility; + +import ai.timefold.solver.core.api.domain.entity.PlanningEntity; +import ai.timefold.solver.core.api.domain.variable.PlanningVariable; +import ai.timefold.solver.core.testdomain.TestdataValue; + +@PlanningEntity +public class TestdataDifferentGetterSetterVisibilityEntity { + private TestdataValue value1; + private TestdataValue value2; + + @PlanningVariable + public TestdataValue getValue1() { + return value1; + } + + private void setValue1(TestdataValue value1) { + this.value1 = value1; + } + + @PlanningVariable + TestdataValue getValue2() { + return value2; + } + + @PlanningVariable + protected TestdataValue setValue2(TestdataValue value2) { + return value2; + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataBendableBigDecimalScoreSolution.java b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataBendableBigDecimalScoreSolution.java index 9c6f61a20f..66c4defd62 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataBendableBigDecimalScoreSolution.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataBendableBigDecimalScoreSolution.java @@ -75,7 +75,7 @@ public void setEntityList(List entityList) { } @PlanningScore(bendableHardLevelsSize = 1, bendableSoftLevelsSize = 2) - BendableBigDecimalScore getScore() { + public BendableBigDecimalScore getScore() { return score; } diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataBendableLongScoreSolution.java b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataBendableLongScoreSolution.java index b1baa467ac..aeba0ee4c4 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataBendableLongScoreSolution.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataBendableLongScoreSolution.java @@ -75,7 +75,7 @@ public void setEntityList(List entityList) { } @PlanningScore(bendableHardLevelsSize = 1, bendableSoftLevelsSize = 2) - BendableLongScore getScore() { + public BendableLongScore getScore() { return score; } diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataBendableScoreSolution.java b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataBendableScoreSolution.java index af4d3943ec..98c72b687e 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataBendableScoreSolution.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataBendableScoreSolution.java @@ -75,7 +75,7 @@ public void setEntityList(List entityList) { } @PlanningScore(bendableHardLevelsSize = 1, bendableSoftLevelsSize = 2) - BendableScore getScore() { + public BendableScore getScore() { return score; } diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardMediumSoftBigDecimalScoreSolution.java b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardMediumSoftBigDecimalScoreSolution.java index 2489b1dc77..ba2639804a 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardMediumSoftBigDecimalScoreSolution.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardMediumSoftBigDecimalScoreSolution.java @@ -77,7 +77,7 @@ public void setEntityList(List entityList) { } @PlanningScore - HardMediumSoftBigDecimalScore getScore() { + public HardMediumSoftBigDecimalScore getScore() { return score; } diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardMediumSoftLongScoreSolution.java b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardMediumSoftLongScoreSolution.java index 79e837713c..37c22847f1 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardMediumSoftLongScoreSolution.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardMediumSoftLongScoreSolution.java @@ -75,7 +75,7 @@ public void setEntityList(List entityList) { } @PlanningScore - HardMediumSoftLongScore getScore() { + public HardMediumSoftLongScore getScore() { return score; } diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardMediumSoftScoreSolution.java b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardMediumSoftScoreSolution.java index 53bc066ba1..2687af2e6a 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardMediumSoftScoreSolution.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardMediumSoftScoreSolution.java @@ -75,7 +75,7 @@ public void setEntityList(List entityList) { } @PlanningScore - HardMediumSoftScore getScore() { + public HardMediumSoftScore getScore() { return score; } diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardSoftBigDecimalScoreSolution.java b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardSoftBigDecimalScoreSolution.java index 7d259be1a8..4a148c012b 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardSoftBigDecimalScoreSolution.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardSoftBigDecimalScoreSolution.java @@ -75,7 +75,7 @@ public void setEntityList(List entityList) { } @PlanningScore - HardSoftBigDecimalScore getScore() { + public HardSoftBigDecimalScore getScore() { return score; } diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardSoftLongScoreSolution.java b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardSoftLongScoreSolution.java index 942be69083..1a3ca68357 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardSoftLongScoreSolution.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardSoftLongScoreSolution.java @@ -75,7 +75,7 @@ public void setEntityList(List entityList) { } @PlanningScore - HardSoftLongScore getScore() { + public HardSoftLongScore getScore() { return score; } diff --git a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardSoftScoreSolution.java b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardSoftScoreSolution.java index a0182f5aae..b2f739bcb6 100644 --- a/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardSoftScoreSolution.java +++ b/core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardSoftScoreSolution.java @@ -75,7 +75,7 @@ public void setEntityList(List entityList) { } @PlanningScore - HardSoftScore getScore() { + public HardSoftScore getScore() { return score; } diff --git a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorGeneratedGizmoSupplierTest.java b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorGeneratedGizmoSupplierTest.java index 8609b2f24f..70ff03754f 100644 --- a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorGeneratedGizmoSupplierTest.java +++ b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/TimefoldProcessorGeneratedGizmoSupplierTest.java @@ -107,9 +107,9 @@ public interface DummyInterfaceEntity { public abstract static class DummyAbstractEntity { @ShadowVariable(variableListenerClass = DummyVariableListener.class, sourceEntityClass = TestdataEntity.class, sourceVariableName = "value") - abstract Integer getLength(); + public abstract Integer getLength(); - abstract void setLength(Integer length); + public abstract void setLength(Integer length); } public static class DummySolutionPartitioner implements SolutionPartitioner { diff --git a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/gizmo/TestDataKitchenSinkEntity.java b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/gizmo/TestDataKitchenSinkEntity.java index 5d0d2a3729..6b5c1bfd18 100644 --- a/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/gizmo/TestDataKitchenSinkEntity.java +++ b/quarkus-integration/quarkus/deployment/src/test/java/ai/timefold/solver/quarkus/testdomain/gizmo/TestDataKitchenSinkEntity.java @@ -52,7 +52,7 @@ public class TestDataKitchenSinkEntity { private boolean isPinned; @PlanningVariable(valueRangeProviderRefs = { "ints" }) - private Integer getIntVariable() { + public Integer getIntVariable() { return intVariable; } From 77867f326c534549d3533b555c9ca803fdf7311c Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Mon, 9 Jun 2025 15:15:28 -0400 Subject: [PATCH 4/5] docs: update javadoc for UPDATE_SHADOW_VARIABLES_ONLY --- .../timefold/solver/core/api/solver/SolutionUpdatePolicy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/ai/timefold/solver/core/api/solver/SolutionUpdatePolicy.java b/core/src/main/java/ai/timefold/solver/core/api/solver/SolutionUpdatePolicy.java index abf7b000ca..ef57efba2b 100644 --- a/core/src/main/java/ai/timefold/solver/core/api/solver/SolutionUpdatePolicy.java +++ b/core/src/main/java/ai/timefold/solver/core/api/solver/SolutionUpdatePolicy.java @@ -40,7 +40,7 @@ public enum SolutionUpdatePolicy { /** * Runs variable listeners on all planning entities and problem facts, * updates shadow variables. - * Assumes an uninitialized solution; + * Assumes all shadow variables are unset; * for solutions where some shadow variables are already filled in, * use {@link #RESET_SHADOW_VARIABLES_ONLY} instead. *

From 974f4f82d5ab37f4bb2d3dfbdb668ac1fec94f13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Tue, 10 Jun 2025 07:54:57 +0200 Subject: [PATCH 5/5] Sonar --- .../ReflectionBeanPropertyMemberAccessor.java | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/common/accessor/ReflectionBeanPropertyMemberAccessor.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/common/accessor/ReflectionBeanPropertyMemberAccessor.java index f6ae0b398e..e915c35d96 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/common/accessor/ReflectionBeanPropertyMemberAccessor.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/common/accessor/ReflectionBeanPropertyMemberAccessor.java @@ -36,14 +36,13 @@ 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); @@ -68,9 +67,7 @@ public ReflectionBeanPropertyMemberAccessor(Method getterMethod, boolean getterO } catch (IllegalAccessException e) { throw new IllegalStateException(""" Impossible state: method (%s) not accessible. - %s - """ - .strip() + %s""" .formatted(setterMethod, MemberAccessorFactory.CLASSLOADER_NUDGE_MESSAGE), e); } } else { @@ -109,11 +106,6 @@ public String toString() { } } - private int getAccessLevel(Method method) { - int mask = Modifier.PUBLIC | Modifier.PROTECTED | Modifier.PRIVATE; - return method.getModifiers() & mask; - } - @Override public Class getDeclaringClass() { return getterMethod.getDeclaringClass();