SolutionDescriptor: more test on invalid combinations + fixes for som…

…e of those + move all invalid testdata's into a subpackage invalid (subpackage of the functional package)

optaplanner-core/src/main/java/org/optaplanner/core/config/util/ConfigUtils.java

@@ -239,8 +239,12 @@ public static MemberAccessor buildMemberAccessor(Member member, MemberAccessorTy
             MemberAccessor memberAccessor;
             switch (memberAccessorType) {
                 case FIELD_OR_READ_METHOD:
-                    ReflectionHelper.assertReadMethod(method, annotationClass);
-                    memberAccessor = new MethodMemberAccessor(method);
+                    if (ReflectionHelper.isGetterMethod(method)) {
+                        memberAccessor = new BeanPropertyMemberAccessor(method);
+                    } else {
+                        ReflectionHelper.assertReadMethod(method, annotationClass);
+                        memberAccessor = new MethodMemberAccessor(method);
+                    }
                     break;
                 case FIELD_OR_GETTER_METHOD:
                 case FIELD_OR_GETTER_METHOD_WITH_SETTER:

optaplanner-core/src/main/java/org/optaplanner/core/impl/domain/solution/descriptor/SolutionDescriptor.java

@@ -34,6 +34,7 @@
 import com.google.common.collect.Iterators;
 import org.apache.commons.lang3.tuple.MutablePair;
 import org.apache.commons.lang3.tuple.Pair;
+import org.optaplanner.core.api.domain.entity.PlanningEntity;
 import org.optaplanner.core.api.domain.solution.PlanningEntityCollectionProperty;
 import org.optaplanner.core.api.domain.solution.PlanningEntityProperty;
 import org.optaplanner.core.api.domain.solution.PlanningScore;
@@ -48,7 +49,6 @@
 import org.optaplanner.core.config.util.ConfigUtils;
 import org.optaplanner.core.impl.domain.common.accessor.BeanPropertyMemberAccessor;
 import org.optaplanner.core.impl.domain.common.accessor.MemberAccessor;
-import org.optaplanner.core.impl.domain.common.accessor.MethodMemberAccessor;
 import org.optaplanner.core.impl.domain.entity.descriptor.EntityDescriptor;
 import org.optaplanner.core.impl.domain.policy.DescriptorPolicy;
 import org.optaplanner.core.impl.domain.solution.cloner.FieldAccessingSolutionCloner;
@@ -196,8 +196,9 @@ private void processLegacySolution(DescriptorPolicy descriptorPolicy) {
         }
         try {
             Method getProblemFactsMethod = solutionClass.getMethod("getProblemFacts");
-            registerProblemFactPropertyAccessor(ProblemFactCollectionProperty.class,
-                    new MethodMemberAccessor(getProblemFactsMethod));
+            MemberAccessor problemFactsMemberAccessor = new BeanPropertyMemberAccessor(getProblemFactsMethod);
+            problemFactCollectionMemberAccessorMap.put(
+                    problemFactsMemberAccessor.getName(), problemFactsMemberAccessor);
         } catch (NoSuchMethodException e) {
             throw new IllegalStateException("Impossible situation: the solutionClass (" + solutionClass
                     + ") which implements the legacy interface " + Solution.class.getSimpleName()
@@ -268,25 +269,77 @@ private void processValueRangeProviderAnnotation(DescriptorPolicy descriptorPoli
     }
 
     private void processProblemFactPropertyAnnotation(DescriptorPolicy descriptorPolicy, Member member) {
-        Class<? extends Annotation> factPropertyAnnotationClass = ConfigUtils.extractAnnotationClass(member,
+        Class<? extends Annotation> annotationClass = ConfigUtils.extractAnnotationClass(member,
                 ProblemFactProperty.class, ProblemFactCollectionProperty.class);
-        if (factPropertyAnnotationClass != null) {
+        if (annotationClass != null) {
             MemberAccessor memberAccessor = ConfigUtils.buildMemberAccessor(
-                    member, FIELD_OR_READ_METHOD, factPropertyAnnotationClass);
-            registerProblemFactPropertyAccessor(factPropertyAnnotationClass, memberAccessor);
+                    member, FIELD_OR_READ_METHOD, annotationClass);
+            assertUnexistingProblemFactOrPlanningEntityProperty(memberAccessor, annotationClass);
+            if (annotationClass == ProblemFactProperty.class) {
+                problemFactMemberAccessorMap.put(memberAccessor.getName(), memberAccessor);
+            } else if (annotationClass == ProblemFactCollectionProperty.class) {
+                if (!Collection.class.isAssignableFrom(memberAccessor.getType())) {
+                    throw new IllegalStateException("The solutionClass (" + solutionClass
+                            + ") has a " + ProblemFactCollectionProperty.class.getSimpleName()
+                            + " annotated member (" + member + ") that does not return a "
+                            + Collection.class.getSimpleName() + ".");
+                }
+                problemFactCollectionMemberAccessorMap.put(memberAccessor.getName(), memberAccessor);
+            }
         }
     }
 
     private void processPlanningEntityPropertyAnnotation(DescriptorPolicy descriptorPolicy, Member member) {
-        Class<? extends Annotation> entityPropertyAnnotationClass = ConfigUtils.extractAnnotationClass(member,
+        Class<? extends Annotation> annotationClass = ConfigUtils.extractAnnotationClass(member,
                 PlanningEntityProperty.class, PlanningEntityCollectionProperty.class);
-        if (entityPropertyAnnotationClass != null) {
+        if (annotationClass != null) {
             MemberAccessor memberAccessor = ConfigUtils.buildMemberAccessor(
-                    member, FIELD_OR_GETTER_METHOD, entityPropertyAnnotationClass);
-            registerPlanningEntityPropertyAccessor(entityPropertyAnnotationClass, memberAccessor);
+                    member, FIELD_OR_GETTER_METHOD, annotationClass);
+            assertUnexistingProblemFactOrPlanningEntityProperty(memberAccessor, annotationClass);
+            if (annotationClass == PlanningEntityProperty.class) {
+                entityMemberAccessorMap.put(memberAccessor.getName(), memberAccessor);
+            } else if (annotationClass == PlanningEntityCollectionProperty.class) {
+                if (!Collection.class.isAssignableFrom(memberAccessor.getType())) {
+                    throw new IllegalStateException("The solutionClass (" + solutionClass
+                            + ") has a " + PlanningEntityCollectionProperty.class.getSimpleName()
+                            + " annotated member (" + member + ") that does not return a "
+                            + Collection.class.getSimpleName() + ".");
+                }
+                entityCollectionMemberAccessorMap.put(memberAccessor.getName(), memberAccessor);
+            }
         }
     }
 
+    private void assertUnexistingProblemFactOrPlanningEntityProperty(
+            MemberAccessor memberAccessor, Class<? extends Annotation> annotationClass) {
+        MemberAccessor duplicate;
+        Class<? extends Annotation> otherAnnotationClass;
+        String memberName = memberAccessor.getName();
+        if (problemFactMemberAccessorMap.containsKey(memberName)) {
+            duplicate = problemFactMemberAccessorMap.get(memberName);
+            otherAnnotationClass = ProblemFactProperty.class;
+        } else if (problemFactCollectionMemberAccessorMap.containsKey(memberName)) {
+            duplicate = problemFactCollectionMemberAccessorMap.get(memberName);
+            otherAnnotationClass = ProblemFactCollectionProperty.class;
+        } else if (entityMemberAccessorMap.containsKey(memberName)) {
+            duplicate = entityMemberAccessorMap.get(memberName);
+            otherAnnotationClass = PlanningEntityProperty.class;
+        } else if (entityCollectionMemberAccessorMap.containsKey(memberName)) {
+            duplicate = entityCollectionMemberAccessorMap.get(memberName);
+            otherAnnotationClass = PlanningEntityCollectionProperty.class;
+        } else {
+            return;
+        }
+        throw new IllegalStateException("The solutionClass (" + solutionClass
+                + ") has a " + annotationClass.getSimpleName()
+                + " annotated member (" + memberAccessor
+                + ") that is duplicated by a " + otherAnnotationClass.getSimpleName()
+                + " annotated member (" + duplicate + ").\n"
+                + (annotationClass.equals(otherAnnotationClass)
+                ? "Maybe the annotation is defined on both the field and its getter."
+                : "Maybe 2 mutually exclusive annotations are configured."));
+    }
+
     private void processScoreAnnotation(DescriptorPolicy descriptorPolicy, Member member) {
         if (((AnnotatedElement) member).isAnnotationPresent(PlanningScore.class)) {
             MemberAccessor memberAccessor = ConfigUtils.buildMemberAccessor(
@@ -306,51 +359,6 @@ private void processScoreAnnotation(DescriptorPolicy descriptorPolicy, Member me
         }
     }
 
-    private void registerPlanningEntityPropertyAccessor(Class<? extends Annotation> entityPropertyAnnotationClass,
-            MemberAccessor memberAccessor) {
-        registerPropertyAccessor(entityPropertyAnnotationClass, memberAccessor, entityMemberAccessorMap,
-                entityCollectionMemberAccessorMap, PlanningEntityProperty.class,
-                PlanningEntityCollectionProperty.class);
-    }
-
-    private void registerProblemFactPropertyAccessor(Class<? extends Annotation> factPropertyAnnotationClass,
-            MemberAccessor memberAccessor) {
-        registerPropertyAccessor(factPropertyAnnotationClass, memberAccessor, problemFactMemberAccessorMap,
-                problemFactCollectionMemberAccessorMap, ProblemFactProperty.class, ProblemFactCollectionProperty.class);
-    }
-
-    private void registerPropertyAccessor(Class<? extends Annotation> annotationClass,
-            MemberAccessor memberAccessor,
-            Map<String, MemberAccessor> propertyAccessorMap,
-            Map<String, MemberAccessor> collectionPropertyAccessorMap,
-            Class<? extends Annotation> propertyAnnotationClass,
-            Class<? extends Annotation> collectionPropertyAnnotationClass) {
-        String memberName = memberAccessor.getName();
-        if (propertyAccessorMap.containsKey(memberName)
-                || collectionPropertyAccessorMap.containsKey(memberName)) {
-            MemberAccessor duplicate = propertyAccessorMap.get(memberName);
-            if (duplicate == null) {
-                duplicate = collectionPropertyAccessorMap.get(memberName);
-            }
-            throw new IllegalStateException("The solutionClass (" + solutionClass
-                    + ") has a " + propertyAnnotationClass.getSimpleName()
-                    + " annotated member (" + memberAccessor
-                    + ") that is duplicated by another member (" + duplicate + ").\n"
-                    + "  Verify that the annotation is not defined on both the field and its getter.");
-        }
-        if (annotationClass.equals(propertyAnnotationClass)) {
-            propertyAccessorMap.put(memberName, memberAccessor);
-        } else if (annotationClass.equals(collectionPropertyAnnotationClass)) {
-            if (!Collection.class.isAssignableFrom(memberAccessor.getType())) {
-                throw new IllegalStateException("The solutionClass (" + solutionClass
-                        + ") has a " + collectionPropertyAnnotationClass.getSimpleName()
-                        + " annotated member (" + memberName + ") that does not return a "
-                        + Collection.class.getSimpleName() + ".");
-            }
-            collectionPropertyAccessorMap.put(memberName, memberAccessor);
-        }
-    }
-
     public void afterAnnotationsProcessed(DescriptorPolicy descriptorPolicy) {
         for (EntityDescriptor<Solution_> entityDescriptor : entityDescriptorMap.values()) {
             entityDescriptor.linkInheritedEntityDescriptors(descriptorPolicy);

optaplanner-core/src/test/java/org/optaplanner/core/impl/domain/solution/descriptor/SolutionDescriptorTest.java

@@ -17,16 +17,85 @@
 
 import org.junit.Assert;
 import org.junit.Test;
+import org.optaplanner.core.api.solver.SolverFactory;
+import org.optaplanner.core.impl.testdata.domain.TestdataEntity;
 import org.optaplanner.core.impl.testdata.domain.extended.abstractsolution.TestdataExtendedAbstractSolution;
-import org.optaplanner.core.impl.testdata.domain.problemfacts.TestdataInvalidProblemFactCollectionPropertySolution;
+import org.optaplanner.core.impl.testdata.domain.solutionproperties.TestdataProblemFactPropertySolution;
+import org.optaplanner.core.impl.testdata.domain.solutionproperties.TestdataReadMethodProblemFactCollectionPropertySolution;
+import org.optaplanner.core.impl.testdata.domain.solutionproperties.invalid.TestdataDuplicatePlanningEntityCollectionPropertySolution;
+import org.optaplanner.core.impl.testdata.domain.solutionproperties.invalid.TestdataDuplicateProblemFactCollectionPropertySolution;
+import org.optaplanner.core.impl.testdata.domain.solutionproperties.TestdataNoProblemFactPropertySolution;
+import org.optaplanner.core.impl.testdata.domain.solutionproperties.invalid.TestdataProblemFactIsPlanningEntityCollectionPropertySolution;
+import org.optaplanner.core.impl.testdata.domain.solutionproperties.invalid.TestdataProblemFactCollectionPropertyWithArgumentSolution;
+import org.optaplanner.core.impl.testdata.util.PlannerTestUtils;
+
+import static org.optaplanner.core.impl.testdata.util.PlannerAssert.*;
 
 public class SolutionDescriptorTest {
 
+    // ************************************************************************
+    // Problem fact and planning entity properties
+    // ************************************************************************
+
+    @Test
+    public void problemFactProperty() {
+        SolutionDescriptor<TestdataProblemFactPropertySolution> solutionDescriptor
+                = TestdataProblemFactPropertySolution.buildSolutionDescriptor();
+        assertMapContainsKeysExactly(solutionDescriptor.getProblemFactMemberAccessorMap(),
+                "extraObject");
+        assertMapContainsKeysExactly(solutionDescriptor.getProblemFactCollectionMemberAccessorMap(),
+                "valueList", "otherProblemFactList");
+    }
+
+    @Test
+    public void readMethodProblemFactCollectionProperty() {
+        SolutionDescriptor<TestdataReadMethodProblemFactCollectionPropertySolution> solutionDescriptor
+                = TestdataReadMethodProblemFactCollectionPropertySolution.buildSolutionDescriptor();
+        assertMapContainsKeysExactly(solutionDescriptor.getProblemFactMemberAccessorMap());
+        assertMapContainsKeysExactly(solutionDescriptor.getProblemFactCollectionMemberAccessorMap(),
+                "valueList", "createProblemFacts");
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void problemFactCollectionPropertyWithArgument() {
+        TestdataProblemFactCollectionPropertyWithArgumentSolution.buildSolutionDescriptor();
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void duplicateProblemFactCollectionProperty() {
+        TestdataDuplicateProblemFactCollectionPropertySolution.buildSolutionDescriptor();
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void duplicatePlanningEntityCollectionProperty() {
+        TestdataDuplicatePlanningEntityCollectionPropertySolution.buildSolutionDescriptor();
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void problemFactIsPlanningEntityCollectionProperty() {
+        TestdataProblemFactIsPlanningEntityCollectionPropertySolution.buildSolutionDescriptor();
+    }
+
+    @Test
+    public void noProblemFactPropertyWithEasyScoreCalculation() {
+        SolverFactory<TestdataNoProblemFactPropertySolution> solverFactory
+                = PlannerTestUtils.buildSolverFactoryWithEasyScoreDirector(
+                        TestdataNoProblemFactPropertySolution.class, TestdataEntity.class);
+        solverFactory.buildSolver();
+    }
+
     @Test(expected = IllegalStateException.class)
-    public void invalidProblemFactCollectionProperty() {
-        TestdataInvalidProblemFactCollectionPropertySolution.buildSolutionDescriptor();
+    public void noProblemFactPropertyWithDroolsScoreCalculation() {
+        SolverFactory<TestdataNoProblemFactPropertySolution> solverFactory
+                = PlannerTestUtils.buildSolverFactoryWithDroolsScoreDirector(
+                        TestdataNoProblemFactPropertySolution.class, TestdataEntity.class);
+        solverFactory.buildSolver();
     }
 
+    // ************************************************************************
+    // Others
+    // ************************************************************************
+
     @Test
     public void extendedAbstractSolution() {
         SolutionDescriptor<TestdataExtendedAbstractSolution> descriptor

optaplanner-core/src/test/java/org/optaplanner/core/impl/domain/variable/custom/CustomVariableListenerTest.java

@@ -24,9 +24,9 @@
 import org.optaplanner.core.impl.domain.variable.descriptor.GenuineVariableDescriptor;
 import org.optaplanner.core.impl.score.director.InnerScoreDirector;
 import org.optaplanner.core.impl.testdata.domain.TestdataValue;
-import org.optaplanner.core.impl.testdata.domain.shadow.cyclic.TestdataCyclicShadowedSolution;
-import org.optaplanner.core.impl.testdata.domain.shadow.cyclic.reference.TestdataCyclicReferencedShadowedSolution;
-import org.optaplanner.core.impl.testdata.domain.shadow.cyclic.seven.TestdataSevenNonCyclicShadowedSolution;
+import org.optaplanner.core.impl.testdata.domain.shadow.cyclic.invalid.TestdataCyclicShadowedSolution;
+import org.optaplanner.core.impl.testdata.domain.shadow.cyclic.invalid.TestdataCyclicReferencedShadowedSolution;
+import org.optaplanner.core.impl.testdata.domain.shadow.cyclic.TestdataSevenNonCyclicShadowedSolution;
 import org.optaplanner.core.impl.testdata.domain.shadow.extended.TestdataExtendedShadowedChildEntity;
 import org.optaplanner.core.impl.testdata.domain.shadow.extended.TestdataExtendedShadowedParentEntity;
 import org.optaplanner.core.impl.testdata.domain.shadow.extended.TestdataExtendedShadowedSolution;

optaplanner-core/src/test/java/org/optaplanner/core/impl/heuristic/selector/common/decorator/ComparatorSelectionSorterTest.java

@@ -19,7 +19,6 @@
 import org.junit.Test;
 import org.optaplanner.core.config.heuristic.selector.common.decorator.SelectionSorterOrder;
 import org.optaplanner.core.impl.testdata.domain.TestdataSolution;
-import org.optaplanner.core.impl.testdata.domain.shadow.cyclic.seven.TestdataSevenNonCyclicShadowedSolution;
 
 import java.util.ArrayList;
 import java.util.Collections;

optaplanner-core/src/test/java/org/optaplanner/core/impl/score/DummySimpleScoreEasyScoreCalculator.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright 2016 Red Hat, Inc. and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.optaplanner.core.impl.score;
+
+import org.optaplanner.core.api.score.Score;
+import org.optaplanner.core.api.score.buildin.simple.SimpleScore;
+import org.optaplanner.core.impl.score.director.easy.EasyScoreCalculator;
+
+public class DummySimpleScoreEasyScoreCalculator<Solution_> implements EasyScoreCalculator<Solution_> {
+
+    @Override
+    public Score calculateScore(Solution_ solution_) {
+        return SimpleScore.valueOf(0);
+    }
+
+}