Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@Aggregate type property ignored #1090

Closed
hatzlj opened this issue May 13, 2019 · 6 comments

Comments

Projects
None yet
4 participants
@hatzlj
Copy link

commented May 13, 2019

Hi there,

maybe it's just a kind of a misunderstandig but I think the type property of the @Aggregate annotation to define an Aggregate class is not working as expected. Using Axon 4.0.2 we refactored our Aggregate Root class from former called class Project to the following Aggregate:

@Aggregate(type = "Project")
public class ProjectAggregate extends AbstractOrganizationAggregate<String> {

  // command handlers go here:
  // ...

  // event handlers go here:
 @EventSourcingHandler
  private void on(ProjectCreatedEvent event, @Timestamp Instant eventTime) {
        this.guid = event.guid;
        this.owner = event.userId;
        this.organizationId = event.organizationId;
        this.name = event.name;
        setCreated(eventTime);
  }
  
  // ...
}

In combination with axoniq/axonserver:4.0 we get the following entries when querying the ProjectCreated event (note the aggregateType ProjectAggregate in column 5)

4324 | abf64c12-8873-444d-ac9d-1188eb8f3e53 | 5f181c83-ce9b-4135-b7f9-75e64ad97f80 | 0 | ProjectAggregate | my.domain.event.ProjectCreatedEvent | 1.2.0-SNAPSHOT | {"guid":"5f181c83-ce9b-4135-b7f9-75e64ad97f80","userId":"376f4c46-4f86-4831-9461-2b2b814a51a1","organizationId":"4583cdd9-13cb-4590-9dc6-f45557a6e196","name":"prj1"} | 2019-04-08T14:01:01.135Z | {traceId=3674719d-6549-4667-826f-c3539d3d1056, correlationId=3674719d-6549-4667-826f-c3539d3d1056}

Before refactoring the ProjectAggregate class from Project the events in the Axon Server looked like this:

2687 | 031ba966-8c55-4499-bff1-60a203b9d5be | f6010b34-03fd-4afb-9498-c9145b10a675 | 0 | Project | my.domain.event.ProjectCreatedEvent | 1.1.4-SNAPSHOT | {"guid":"f6010b34-03fd-4afb-9498-c9145b10a675","userId":"55214754-e81d-4100-91c2-85fdc9b19388","organizationId":"8f030006-7cce-43bb-9491-41c2a5fd1118","name":"test"} | 2019-04-04T15:05:51.443Z | {traceId=3c8ed574-90d7-4e05-9284-dfcade792a6e, correlationId=3c8ed574-90d7-4e05-9284-dfcade792a6e}

I would have expected that the typeattribute of the @Aggregate Annotation defines the aggregate name used in the event store (AxonServer in this case).

I would be grateful if you could clarify if this is a bug or if I interpreted the type property in the wrong way.

Thanks,
Jakob

@hatzlj

This comment has been minimized.

Copy link
Author

commented May 15, 2019

Considering the following discussion in the axon google group, is it right that the aggregate identifier is considered globally unique for AxonServer and thus the aggregate type isn't taken into account when applying events on aggregates?
If true, then it may still be helpful to adjust the javadoc of the @Aggregate annotation to clarify the use of the type property.

see: https://groups.google.com/forum/#!searchin/axonframework/filter$20event%7Csort:date/axonframework/dIw3HuKfg3Q/PVQcWBzeAgAJ

@smcvb

This comment has been minimized.

Copy link
Member

commented May 15, 2019

Hi @hatzlj,

I checked out whether your statement would make sense given the current implementation, and sadly, I haven't been able to figure out how you end up in the situation you're in.

The type field of the AggregateRoot and Aggregate annotation, will be set as the type of an Aggregate Meta Model (used internally to make everything spin).

The class in charge of this, is the AnnotatedAggregateMetaModelFactory, which will call the AnnotatedAggregateMetaModelFactory#createModel(Class<A>) method on line 123 to initialize an AnnotatedAggregateModel. In that method, the model is initialized, which will call the inspectAggregateType() method, which in turn performs the following operation on lines 180-183:

aggregateType = AnnotationUtils.findAnnotationAttributes(inspectedType, AggregateRoot.class)
                    .map(map -> (String) map.get("type"))
                    .filter(i -> i.length() > 0)
                    .orElse(inspectedType.getSimpleName());

This will thus set the aggregateType of the AnnotatedAggregateModel to the type attribute of the AggregateRoot, but also annotation like @Aggregate which are meta-annotated with AggregateRoot every meta-annotated.

This behavior is tested in the AnnotatedAggregateMetaModelFactoryTest#testFindIdentifierInSuperClass() on line 236.

To further ensure this works as expected, I added a test similar to the testFindIdentifierInSuperClass which uses the @Aggregate annotation instead, which gave me the same result. Thus, the type attribute was used as the type of the Aggregate.

Lastly, I checked whether it might be an issue when running this together with Axon Server, together with a basic test suite we have in place. In the test suite, I adjusted the type field of the @Aggregate annotation and issued some commands to get events into Axon Server. The aggregateType column was adjusted accordingly in my test case.

This brings me in a predicament for your situation, as I seem to be unable to reproduce the problem.
My sole guess is that it might have something to do with the AbstractOrganizationAggregate you extend from in your ProjectAggregate. This is however a wild guess which I haven't tested.

If you're up to the task, you might be able to debug your own application using the class names and line numbers I've provided on the AnnotatedAggregateMetaModelFactory. I'd assume that if something is wrong with your set up, it's because the type attribute isn't retrieved as expected.

I'll keep this issue upon for you to respond on it. Hopefully you're able to verify the correct creation of the Aggregate Meta Model. If you have any other thoughts how this might occur, of course feel free to comment on this issue.

Cheers, Steven

@j-egger

This comment has been minimized.

Copy link

commented May 20, 2019

Hello @smcvb,

I am working with @hatzlj on this issue and managed to find out what was happening. Our annotations for the ProjectAggregate looked like this:

@AggregateRoot
@Aggregate(type = "Project")
public class ProjectAggregate extends AbstractOrganizationAggregate<String> { ... }

Seems like if there is no type-parameter specified in the @AggregateRoot annotation, the type-parameter of the @Aggregate annotation is overridden and the class name is used.

We removed the unnecessary @AggregateRoot annotation and kept the (type = "Project") for the @Aggregate annotation.

Now it looks like this and is working the way we were hoping it would:

@Aggregate(type = "Project")
public class ProjectAggregate extends AbstractOrganizationAggregate<String> { ... }
@smcvb

This comment has been minimized.

Copy link
Member

commented May 20, 2019

Hi @j-egger and @hatzlj,

Great news that you've solved the problem!
Given the set up with both annotations in place I would indeed have suggested the same course of action. Thus, just remove the @AggregateRoot annotation.

Based on your last response, I'll assume that the issue has been resolved.
As such, I'll be closing this issue.

If you find any possible bugs in the future, feel free to file another issue.
If you might have usage questions regarding Axon Framework and Axon Server, I'd like to refer to the user group or Stack Overflow, using the axon tag.

@hatzlj

This comment has been minimized.

Copy link
Author

commented May 22, 2019

Hi @smcvb ,

the problem is indeed solved, thanks for your thorough response.
A last comment on this: maybe it is possible to let the @AggregateRoot(type) default to @Aggregate(type) if it's not set explicitly (i.e. null or empty, assuming that an empty Aggregate Name is not intended) and/or give a Runtime Warning in the 'AnnotatedAggregateMetaModelFactory'.

I'd be happy to have a look at this and prepare a PR if you think it's a good idea.

@abuijze

This comment has been minimized.

Copy link
Member

commented May 22, 2019

Hi @hatzlj,

I agree that it "would be nice" to have some warnings or defaults. In this case, the @Aggregate annotation is meta-annotated with the @AggregateRoot annotation. By using that annotation explicitly on your class, you basically override the defaults of the @Aggregate annotation.

If you have time to submit a PR, that would be great. It would be nice to have the meta-annotation strategy of Axon consider "defaults" before overriding property values blindly. Currently, your explicit value would be overridden by the value from the @AggregateRoot annotation, even though the property itself isn't explicitly provided.

In other words, the AnnotationUtils#findAnnotationAttributes(java.lang.reflect.AnnotatedElement, java.lang.String) method should not override attribute values with default values if a non-default was already present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.