Skip to content

Commit

Permalink
fix: allow @PlanningId to be defined on interfaces (#1060)
Browse files Browse the repository at this point in the history
Previously, `getAllMembers` excluded members defined in interfaces.
Presumably, this was because methods would have implementations in the
implementing class.
However, there is a flaw in that logic: implementations of a method do
not inherit the annotations of the declared method. As a result, when
LookupManager looks up the accessor of a class whose @PlanningId is
defined in an interface,
it finds none.

`getAllMembers` is changed to add all interface methods to the member
list, thus allowing interface members to be used in lookups.
  • Loading branch information
Christopher-Chianelli committed Aug 30, 2024
1 parent 274dadd commit 96c3da0
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -369,15 +369,19 @@ public static List<Member> getAllMembers(Class<?> baseClass, Class<? extends Ann
Stream<Member> memberStream = Stream.empty();
while (clazz != null) {
var fieldStream = Stream.of(clazz.getDeclaredFields())
.filter(field -> field.isAnnotationPresent(annotationClass) && !field.isSynthetic())
.sorted(alphabeticMemberComparator);
var methodStream = Stream.of(clazz.getDeclaredMethods())
.filter(method -> method.isAnnotationPresent(annotationClass) && !method.isSynthetic())
.sorted(alphabeticMemberComparator);
.filter(field -> field.isAnnotationPresent(annotationClass) && !field.isSynthetic());

// Implementations of an interface method do not inherit the annotations of the declared method in
// the interface, so we need to also add the interface's methods to the stream.
var methodStream = Stream.concat(
Stream.of(clazz.getDeclaredMethods()),
Arrays.stream(clazz.getInterfaces())
.flatMap(implementedInterface -> Arrays.stream(implementedInterface.getMethods())))
.filter(method -> method.isAnnotationPresent(annotationClass) && !method.isSynthetic());
memberStream = Stream.concat(memberStream, Stream.concat(fieldStream, methodStream));
clazz = clazz.getSuperclass();
}
return memberStream.collect(Collectors.toList());
return memberStream.distinct().sorted(alphabeticMemberComparator).collect(Collectors.toList());
}

@SafeVarargs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

import ai.timefold.solver.core.api.domain.lookup.LookUpStrategyType;
import ai.timefold.solver.core.impl.testdata.domain.clone.lookup.TestdataObjectIntegerId;
import ai.timefold.solver.core.impl.testdata.domain.interface_domain.TestdataInterfaceEntity;
import ai.timefold.solver.core.impl.testdata.domain.interface_domain.TestdataInterfaceValue;

import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -36,4 +38,43 @@ void resetWorkingObjects() {
lookUpManager.removeWorkingObject(p);
}

public static class InterfaceEntity implements TestdataInterfaceEntity {
final String id;

public InterfaceEntity(String id) {
this.id = id;
}

@Override
public String getId() {
return id;
}

@Override
public TestdataInterfaceValue getValue() {
return null;
}

@Override
public void setValue(TestdataInterfaceValue value) {

}
}

@Test
void lookupInterfaceEntity() {
var o = new InterfaceEntity("0");
var p = new InterfaceEntity("1");
// The objects should be added during the reset
lookUpManager.reset();
for (Object fact : Arrays.asList(o, p)) {
lookUpManager.addWorkingObject(fact);
}
// So it's possible to look up and remove them
assertThat(lookUpManager.lookUpWorkingObject(new InterfaceEntity("0"))).isSameAs(o);
assertThat(lookUpManager.lookUpWorkingObject(new InterfaceEntity("1"))).isSameAs(p);
lookUpManager.removeWorkingObject(o);
lookUpManager.removeWorkingObject(p);
}

}
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package ai.timefold.solver.core.impl.testdata.domain.interface_domain;

import ai.timefold.solver.core.api.domain.entity.PlanningEntity;
import ai.timefold.solver.core.api.domain.lookup.PlanningId;
import ai.timefold.solver.core.api.domain.variable.PlanningVariable;

@PlanningEntity
public interface TestdataInterfaceEntity {
@PlanningId
String getId();

@PlanningVariable
TestdataInterfaceValue getValue();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ public void setScore(SimpleScore score) {
new TestdataInterfaceEntity() {
private TestdataInterfaceValue value;

@Override
public String getId() {
return "entity";
}

@Override
public TestdataInterfaceValue getValue() {
return value;
Expand Down

0 comments on commit 96c3da0

Please sign in to comment.