issue #495: require toString() for types used as aggregate identifier

Aggregate field annotated with @EntityID or @AggregateIdentifier should override Object.toString()\ - StubAggregate classes now use UUID or String as identifier type - java.lang.Object is no longer supported as identifier type
AxonFramework · Feb 8, 2018 · ff2e341 · ff2e341
1 parent 4d98198
commit ff2e341
Show file tree

Hide file tree

Showing 10 changed files with 114 additions and 32 deletions.
diff --git a/...rg/axonframework/commandhandling/model/inspection/AnnotatedAggregateMetaModelFactory.java b/...rg/axonframework/commandhandling/model/inspection/AnnotatedAggregateMetaModelFactory.java
@@ -20,6 +20,8 @@
 import org.axonframework.commandhandling.model.AggregateRoot;
 import org.axonframework.commandhandling.model.AggregateVersion;
 import org.axonframework.commandhandling.model.EntityId;
+import org.axonframework.common.AxonConfigurationException;
+import org.axonframework.common.IdentifierValidator;
 import org.axonframework.common.ReflectionUtils;
 import org.axonframework.common.annotation.AnnotationUtils;
 import org.axonframework.eventhandling.EventMessage;
@@ -119,7 +121,7 @@ private class AnnotatedAggregateModel<T> implements AggregateModel<T> {
         private Field versionField;
         private String routingKey;
 
-        public AnnotatedAggregateModel(Class<? extends T> aggregateType, AnnotatedHandlerInspector<T> handlerInspector) {
+        AnnotatedAggregateModel(Class<? extends T> aggregateType, AnnotatedHandlerInspector<T> handlerInspector) {
             this.inspectedType = aggregateType;
             this.commandHandlers = new HashMap<>();
             this.eventHandlers = new ArrayList<>();
@@ -147,7 +149,7 @@ private void prepareHandlers() {
 
         private void inspectAggregateType() {
             aggregateType = AnnotationUtils.findAnnotationAttributes(inspectedType, AggregateRoot.class)
-                                           .map(map -> (String) map.get("type")).filter(i -> i.length() > 0).orElse(inspectedType.getSimpleName());
+                    .map(map -> (String) map.get("type")).filter(i -> i.length() > 0).orElse(inspectedType.getSimpleName());
         }
 
         private void inspectFields() {
@@ -173,8 +175,14 @@ private void inspectFields() {
                         routingKey = field.getName();
                     });
                 }
+                if (identifierField != null) {
+                    final Class<?> idClazz = identifierField.getType();
+                    if (!IdentifierValidator.getInstance().isValidIdentifier(idClazz)) {
+                        throw new AxonConfigurationException(format("Aggregate identifier type [%s] should override Object.toString()", idClazz.getName()));
+                    }
+                }
                 AnnotationUtils.findAnnotationAttributes(field, AggregateVersion.class)
-                               .ifPresent(attributes -> versionField = field);
+                        .ifPresent(attributes -> versionField = field);
             }
         }
 
@@ -247,7 +255,7 @@ public Long getVersion(T target) {
          * @return the handler of the message if present on the model
          */
         @SuppressWarnings("unchecked")
-        protected Optional<MessageHandlingMember<? super T>> getHandler(Message<?> message) {
+        private Optional<MessageHandlingMember<? super T>> getHandler(Message<?> message) {
             for (MessageHandlingMember<? super T> handler : eventHandlers) {
                 if (handler.canHandle(message)) {
                     return Optional.of(handler);

diff --git a/core/src/test/java/org/axonframework/commandhandling/StubAggregate.java b/core/src/test/java/org/axonframework/commandhandling/StubAggregate.java
@@ -33,16 +33,16 @@
 public class StubAggregate {
 
     @AggregateIdentifier
-    private Object identifier;
+    private String identifier;
 
     private int invocationCount;
 
     public StubAggregate() {
-        identifier = UUID.randomUUID();
+        identifier = UUID.randomUUID().toString();
     }
 
     public StubAggregate(Object identifier) {
-        this.identifier = identifier;
+        this.identifier = identifier.toString();
     }
 
     public void doSomething() {

diff --git a/...xonframework/commandhandling/model/inspection/AnnotatedAggregateMetaModelFactoryTest.java b/...xonframework/commandhandling/model/inspection/AnnotatedAggregateMetaModelFactoryTest.java
@@ -144,7 +144,7 @@ public void testLinkedListIsModifiedDuringIterationInRecursiveHierarchy() {
     public void testHashSetIsModifiedDuringIterationInRecursiveHierarchy() {
         testCollectionIsModifiedDuringIterationInRecursiveHierarchy(HashSet::new);
     }
-    
+
     @Test
     public void testCopyOnWriteArrayListIsModifiedDuringIterationInRecursiveHierarchy() {
         testCollectionIsModifiedDuringIterationInRecursiveHierarchy(CopyOnWriteArrayList::new);
@@ -154,7 +154,7 @@ public void testCopyOnWriteArrayListIsModifiedDuringIterationInRecursiveHierarch
     public void testConcurrentLinkedQueueIsModifiedDuringIterationInRecursiveHierarchy() {
         testCollectionIsModifiedDuringIterationInRecursiveHierarchy(ConcurrentLinkedQueue::new);
     }
-    
+
     private void testCollectionIsModifiedDuringIterationInRecursiveHierarchy(Supplier<Collection<SomeRecursiveEntity>> supplier) {
         // Note that if the inspector does not support recursive entities this will throw an StackOverflowError.
         AggregateModel<SomeRecursiveEntity> inspector = AnnotatedAggregateMetaModelFactory.inspectAggregate(SomeRecursiveEntity.class);
@@ -178,14 +178,14 @@ private void testCollectionIsModifiedDuringIterationInRecursiveHierarchy(Supplie
 
         // Assert child1: it should have 1 child
         assertEquals(1, child1.children.size());
-        
+
         // Now move child3 up one level so it is a child of child1.
         // The resulting hierarchy will look as follows:
         // root 
         //      child1
         //              child2
         //              child3
-        
+
         // Note that if the inspector does not use copy-iterators this will throw an ConcurrentModificationException.
         inspector.publish(asEventMessage(new MoveChildUp(childId2, childId3)), root);
         assertEquals(2, child1.children.size());
@@ -241,6 +241,13 @@ public void testIllegalFactoryMethodThrowsExceptionClass() throws Exception {
         AnnotatedAggregateMetaModelFactory.inspectAggregate(SomeIllegalAnnotatedFactoryMethodClass.class);
     }
 
+    @Test(expected = AxonConfigurationException.class)
+    public void typedAggregateIdentifier() {
+        AggregateModel<TypedIdentifierAggregate> inspector = AnnotatedAggregateMetaModelFactory.inspectAggregate(TypedIdentifierAggregate.class);
+
+        assertNotNull(inspector.getIdentifier(new TypedIdentifierAggregate()));
+    }
+
     private static class JavaxPersistenceAnnotatedHandlers {
 
         @Id
@@ -291,6 +298,27 @@ public void handle(AtomicLong value) {
 
     }
 
+    private static class CustomIdentifier {
+    }
+
+    @AggregateRoot
+    private static class TypedIdentifierAggregate {
+
+        @AggregateIdentifier
+        private CustomIdentifier aggregateIdentifier = new CustomIdentifier();
+
+        @CommandHandler
+        public boolean handleInSubclass(String test) {
+            return test.contains("sub");
+        }
+
+        @EventHandler
+        public void handle(AtomicLong value) {
+            value.incrementAndGet();
+        }
+
+    }
+
     private static class SomeOtherEntity {
 
         @CommandHandler
@@ -303,7 +331,7 @@ public void handle(AtomicLong value) {
             value.incrementAndGet();
         }
     }
-    
+
     private static class CreateChild {
         private final String parentId;
         private final String childId;
@@ -339,10 +367,10 @@ public SomeRecursiveEntity(Supplier<Collection<SomeRecursiveEntity>> supplier, S
             this.entityId = entityId;
             this.children = new SomeIterable<>(supplier.get());
         }
-        
+
         public SomeRecursiveEntity getChild(String childId) {
-            for(SomeRecursiveEntity c : children) {
+            for (SomeRecursiveEntity c : children) {
-                if(Objects.equals(c.entityId, childId)) {
+                if (Objects.equals(c.entityId, childId)) {
                     return c;
                 }
             }

diff --git a/core/src/test/java/org/axonframework/common/IdentifierValidatorTest.java b/core/src/test/java/org/axonframework/common/IdentifierValidatorTest.java
@@ -0,0 +1,44 @@
+package org.axonframework.common;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class IdentifierValidatorTest {
+
+    private final IdentifierValidator validator = IdentifierValidator.getInstance();
+
+    @Test
+    public void boxedPrimitivesAreValidIdentifiers() {
+        assertTrue(validator.isValidIdentifier(Long.class));
+        assertTrue(validator.isValidIdentifier(Integer.class));
+        assertTrue(validator.isValidIdentifier(Double.class));
+        assertTrue(validator.isValidIdentifier(Short.class));
+    }
+
+    @Test
+    public void stringIsValidIdentifier() {
+        assertTrue(validator.isValidIdentifier(CharSequence.class));
+    }
+
+    @Test
+    public void typeWithoutToStringIsNotAccepted() {
+        assertFalse(validator.isValidIdentifier(CustomType.class));
+    }
+
+    @Test
+    public void typeWithOverriddenToString() {
+        assertTrue(validator.isValidIdentifier(CustomType2.class));
+    }
+
+    private static class CustomType {
+    }
+
+    private static class CustomType2 {
+        @Override
+        public String toString() {
+            return "ok";
+        }
+    }
+}
diff --git a/core/src/test/java/org/axonframework/eventsourcing/AggregateSnapshotterTest.java b/core/src/test/java/org/axonframework/eventsourcing/AggregateSnapshotterTest.java
@@ -96,7 +96,7 @@ public void testCreateSnapshot_AggregateMarkedDeletedWillNotGenerateSnapshot() {
         DomainEventMessage firstEvent = new GenericDomainEventMessage<>("type", aggregateIdentifier, (long) 0,
                                                                         "Mock contents", MetaData.emptyInstance());
         DomainEventMessage secondEvent = new GenericDomainEventMessage<>("type", aggregateIdentifier, (long) 0,
-                                                                        "deleted", MetaData.emptyInstance());
+                                                                         "deleted", MetaData.emptyInstance());
         DomainEventStream eventStream = DomainEventStream.of(firstEvent, secondEvent);
         StubAggregate aggregate = new StubAggregate(aggregateIdentifier);
         when(mockAggregateFactory.createAggregateRoot(aggregateIdentifier, firstEvent)).thenReturn(aggregate);
@@ -111,14 +111,14 @@ public void testCreateSnapshot_AggregateMarkedDeletedWillNotGenerateSnapshot() {
     public static class StubAggregate {
 
         @AggregateIdentifier
-        private Object identifier;
+        private String identifier;
 
         public StubAggregate() {
-            identifier = UUID.randomUUID();
+            identifier = UUID.randomUUID().toString();
         }
 
         public StubAggregate(Object identifier) {
-            this.identifier = identifier;
+            this.identifier = identifier.toString();
         }
 
         public void doSomething() {
@@ -134,7 +134,7 @@ protected void handle(EventMessage event) {
             identifier = ((DomainEventMessage) event).getAggregateIdentifier();
             // See Issue #
             if ("Mock contents".equals(event.getPayload().toString())) {
-                    apply("Another");
+                apply("Another");
             }
             if ("deleted".equals(event.getPayload().toString())) {
                 markDeleted();

diff --git a/...tests/src/test/java/org/axonframework/integrationtests/commandhandling/StubAggregate.java b/...tests/src/test/java/org/axonframework/integrationtests/commandhandling/StubAggregate.java
@@ -29,7 +29,7 @@ public class StubAggregate {
     private int changeCounter;
 
     @AggregateIdentifier
-    private Object identifier;
+    private String identifier;
 
     public StubAggregate(Object aggregateId) {
         apply(new StubAggregateCreatedEvent(aggregateId));
@@ -48,7 +48,7 @@ public void causeTrouble() {
 
     @EventSourcingHandler
     private void onCreated(StubAggregateCreatedEvent event) {
-        this.identifier = event.getAggregateIdentifier();
+        this.identifier = event.getAggregateIdentifier().toString();
         changeCounter = 0;
     }
 

diff --git a/spring/src/test/java/org/axonframework/spring/config/annotation/StubAggregate.java b/spring/src/test/java/org/axonframework/spring/config/annotation/StubAggregate.java
@@ -36,14 +36,14 @@ public class StubAggregate {
     private int invocationCount;
 
     @AggregateIdentifier
-    private Object identifier;
+    private String identifier;
 
     public StubAggregate() {
-        identifier = UUID.randomUUID();
+        identifier = UUID.randomUUID().toString();
     }
 
     public StubAggregate(Object identifier) {
-        this.identifier = identifier;
+        this.identifier = identifier.toString();
     }
 
     public void doSomething() {

diff --git a/test/src/test/java/org/axonframework/test/aggregate/AnnotatedAggregate.java b/test/src/test/java/org/axonframework/test/aggregate/AnnotatedAggregate.java
@@ -37,11 +37,11 @@ class AnnotatedAggregate implements AnnotatedAggregateInterface {
     private transient int counter;
     private Integer lastNumber;
     @AggregateIdentifier
-    private Object identifier;
+    private String identifier;
     private MyEntity entity;
 
     public AnnotatedAggregate(Object identifier) {
-        this.identifier = identifier;
+        this.identifier = identifier.toString();
     }
 
     public AnnotatedAggregate() {
@@ -71,7 +71,7 @@ public void doSomethingIllegal(IllegalStateChangeCommand command) {
 
     @EventSourcingHandler
     public void handleMyEvent(MyEvent event) {
-        identifier = event.getAggregateIdentifier();
+        identifier = event.getAggregateIdentifier() == null ? null : event.getAggregateIdentifier().toString();
         lastNumber = event.getSomeValue();
         if (entity == null) {
             entity = new MyEntity();

diff --git a/test/src/test/java/org/axonframework/test/aggregate/FixtureTest_CommandInterceptors.java b/test/src/test/java/org/axonframework/test/aggregate/FixtureTest_CommandInterceptors.java
@@ -203,13 +203,14 @@ public static class InterceptorAggregate {
         private transient int counter;
         private Integer lastNumber;
         @AggregateIdentifier
-        private Object identifier;
+        private String identifier;
         private MyEntity entity;
 
         public InterceptorAggregate() {
         }
 
         public InterceptorAggregate(Object aggregateIdentifier) {
+            identifier = aggregateIdentifier.toString();
         }
 
         @CommandHandler
@@ -225,7 +226,7 @@ public void handle(TestCommand command, MetaData metaData) {
 
         @EventHandler
         public void handle(StandardAggregateCreatedEvent event) {
-            this.identifier = event.getAggregateIdentifier();
+            this.identifier = event.getAggregateIdentifier().toString();
         }
 
     }

diff --git a/test/src/test/java/org/axonframework/test/aggregate/StandardAggregate.java b/test/src/test/java/org/axonframework/test/aggregate/StandardAggregate.java
@@ -35,10 +35,11 @@ class StandardAggregate {
     private transient int counter;
     private Integer lastNumber;
     @AggregateIdentifier
-    private Object identifier;
+    private String identifier;
     private MyEntity entity;
 
     public StandardAggregate(Object aggregateIdentifier) {
+        identifier = aggregateIdentifier.toString();
     }
 
     public StandardAggregate(int initialValue, Object aggregateIdentifier) {
@@ -59,7 +60,7 @@ public void doSomethingIllegal(Integer newIllegalValue) {
 
     @EventSourcingHandler
     public void handleMyEvent(MyEvent event) {
-        identifier = event.getAggregateIdentifier();
+        identifier = event.getAggregateIdentifier().toString();
         lastNumber = event.getSomeValue();
         if (entity == null) {
             entity = new MyEntity();