@Timestamp results in different values for the same event when testing

[Paul Dubs]

Hello Everyone.

I have just started using Axon, and maybe I don't understand it correctly. As I understand it the @Timestamp annotation or .getTimestamp() on a Message should contain the Timestamp of when the event has been originally created. I want to save this time with an aggregate using an @EventSourcingHandler, but when I try it, I get the following error:

org.axonframework.test.AxonAssertionError: Illegal state change detected! Property "com.example.domain.model.Letter.imported" has different value when sourcing events.
Working aggregate value:     <2014-10-22T12:57:08.572+02:00>
Value after applying events: <2014-10-22T12:57:10.670+02:00>

The test case that creates this error looks like this:

    @Test
    public void lettersCanBeCreated(){
        final LetterId letterId = new LetterId(UUID.randomUUID().toString());
        final LetterFileRef letterRef = mock(LetterFileRef.class);


        fixture.given()
               .when(new CreateLetterCommand(letterId, letterRef))
               .expectReturnValue(letterId)
               .expectEvents(new LetterImportedEvent(letterId, letterRef));

    }

With the Handling Code for it being the following:

public class Letter extends AbstractAnnotatedAggregateRoot {
    @AggregateIdentifier
    private LetterId id;
    @Version
    private Long version;

    private LetterFileRef fileRef;
    private DateTime imported = null;


    // constructor needed for reconstruction
    protected Letter() {
    }


    @CommandHandler
    public Letter(@NotNull CreateLetterCommand command) {
        apply(new LetterImportedEvent(command.getLetterId(), command.getFileRef()));
    }


   @EventSourcingHandler
    private void handleCreation(@NotNull LetterImportedEvent event, @Timestamp eventTime) {
        this.id = event.getLetterId();
        this.fileRef = event.getFileRef();
        this.imported = eventTime;
    }
}

As far as I can tell the apply method in AbstractEventSourcedAggregateRoot creates a new GenericDomainEventMessage when the event is replayed, which in turn sets a new timestamp. But while stepping through it in the debugger I noticed that inReplay is not set to true. 

Now I'm confused as to what I did wrong, can anybody help me to find it?

Cheers,

Paul

[Paul Dubs]

It seems that the reason for this problem are lines 113 and 114 in AbstractAggregateRoot:

handleRecursively(new GenericDomainEventMessage<Object>(null, 0, eventPayload, metaData));
registerEvent(metaData, eventPayload)

registerEvent calls addEvent on the current eventContainer, which itself then creates a new GenericDomainEventMessage. Because of this the event that is registered is different from the event that is replayed. This seems like a bug to me, or is that intentional?

Cheers,

Paul

[Paul Dubs]

(in the previous post I meant to say lines 113 and 114 of AbstractEventSourcedAggregateRoot.java)

Ok, some more investigation and a thorough reading of the code around those lines has brought the comment some lines above them:

workaround for aggregates that set the aggregate identifier in an Event Handler

I thought that setting the Identifier in an Event Handler is how it is supposed to be, as the documentation shows it like that:

 public class MyAggregateRoot extends AbstractAnnotatedAggregateRoot {


    @AggregateIdentifier
    private String aggregateIdentifier;
    private String someProperty;


    public MyAggregateRoot(String id) {
        apply(new MyAggregateCreatedEvent(id));

    }


    // constructor needed for reconstruction

    protected MyAggregateRoot() {
    }


    @EventSourcingHandler
    private void handleMyAggregateCreatedEvent(MyAggregateCreatedEvent event) {
        // make sure identifier is always initialized properly
        this.aggregateIdentifier = event.getMyAggregateIdentifier();
        // do something with someProperty
    }
}

Now if I set the identifier in its constructor (i.e. change state in a command handler!) my test case works fine. From a conceptual perspective it seems wrong to have such a workaround, just to work around the workaround (as this sentence clearly demonstrates).

But I can't think of a right way of letting the (general) handler know what the right identifier is. Maybe some kind of annotation on the event could be used (may be repurpose the TargetAggregateIdentifier or AggregateIdentifier), or maybe there is already something and I'm missing it.

So what is (or would be) the idiomatic way to do it?

[Allard Buijze]

Hi Paul,

the error is generated by the test fixture, as it tried to detect changes made outside of an event handler. To do so, it will reapply all generated events to a new aggregate instance, and compare the state of the newly generated instance with the one used in the test scenario. If the state diverges, it means there is an issue.

The problem does seem to be in the Event that is applied during the creation of the aggregate. That one is handled slightly differently, as Axon cannot create an instance of the definitive event right away. It simply doesn't have an identifier yet. However, it seems that this code should be modified so that the timestamp (and most likely also the identifier) should remain identical.

In other words, you're not doing anything wrong.

To work around the issue, you can do two things.

1. Fix the time for the test. Axon uses JodaTime. You can use DateTimeUtils.setCurrentMillisFixed(...) to fix time. In a tear down method (e.g. @After), you can use DateTimeUtils.setCurrentMillisSystem() to set the time back to normal.

2. Disable illegal state change detection for that specific test. Just use fixture.setReportIllegalStateChange(false) to disable it. Obvisouly, your events will still be validated and the test will still fail if there is a functional issue.

Hope this helps.

Cheers,

Allard

--
You received this message because you are subscribed to the Google Groups "Axon Framework Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to axonframewor...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Paul Dubs]

Hello Allard,

Thanks again for the explanation of what happens, seems like understood that code right. Also thanks for the workaround to make the Test pass.

As you say the actual problem is

The problem does seem to be in the Event that is applied during the creation of the aggregate. That one is handled slightly differently, as Axon cannot create an instance of the definitive event right away. It simply doesn't have an identifier yet. However, it seems that this code should be modified so that the timestamp (and most likely also the identifier) should remain identical.

Do you have any idea how this could be fixed? For the last hours I have tried to come up with a solution for this, as it will introduce inconsistencies between Aggregates that are created from scratch and those created through replaying events - because the event applied for the first instantiation differs from the one that is actually saved.

I understand that this difference is at least somewhat necessary because the event doesn't know yet which identifier the aggregate will have, but can't we use a "not decided yet" placeholder in its place?

If we assume that an EventContainer and a GenericDomainEventMessage could both hold an identifier that is deferred, then it should be possible to get the value just as when it is needed, and throw an exception if it wasn't set until then, that way the workaround for event application during aggregate creation wouldn't be needed. 

I have tried to implement something that could work this way, but didn't have any success yet, as it opened up another can of worms (the original event has a wrapped identifier, while the event sourced one has the actual value).

Cheers,

Paul

[Paul Dubs]

Please take a look at the following commit:

https://github.com/treo/AxonFramework/commit/c29466dfdb38a5facc02f98ba4fcce7681de7a70

It works in my case, and all of the Axon Tests seem to still work right. Do you think something like this would be a good solution?

Cheers,

Paul

[bui...@gmail.com]

Hi Paul,

looks like it could work. I was thinking of a different approach myself, though.

Instead of registerEvent(metaData, payload), we could overload the registerEvent method (if it doesn't yet exist) to accept an EventMessage. GenericEventMessage has a constructor in which we can pass the event identifier, timestamp etc. This information can be copied from the event that had been created just before. 

I think this might be a simpler solution. 

Cheers,

Allard

--

[Paul Dubs]

Hi Allard,

so you were thinking more along the lines of something like this?

https://github.com/treo/AxonFramework/commit/bb36fd010bee720141f4fa524428de11118c9c65

Earlier today I had tried a similar approach and it didn't work out, now it seems to work as expected, probably missed something trivial then.

Anyway, this approach also works well, for both the Axon tests and my own tests, and as the resulting code is simpler I also prefer it.

Cheers,

Paul

[Allard Buijze]

Hi Paul,

looks good. Could you make a pull request for it?

For slightly improved performance, you only need to create a new GenericDomainEvent instance if the aggregateIdentifier is null. Otherwise the original instance would work fine. 

Cheers,

Allard

--

[Paul Dubs]

Hi Allard,

I have added your suggestion and created a pull request.

Cheers,

Paul

[Allard Buijze]

Thanks a lot! I will merge it as soon as I get the chance.