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

Implement aggregate lifecycle helper functions #72

Merged

Conversation

zambrovski
Copy link
Collaborator

fix #71

Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is provided looks fine, but I am uncertain why you haven't added the other static operations from the AggregateLifecycle class with it. So, things like isLive, getVersion and markDeleted. Especially the last one is pretty important if the life cycle of an aggregate actually ends.

@zambrovski
Copy link
Collaborator Author

zambrovski commented Oct 14, 2020

What is provided looks fine, but I am uncertain why you haven't added the other static operations from the AggregateLifecycle class with it. So, things like isLive, getVersion and markDeleted. Especially the last one is pretty important if the life cycle of an aggregate actually ends.

Since the methods isLife() and getVersion() and markDeleted() are static methods of AggregateLifecycle class, they can still be called from Kotlin by providing a static import and they neither collide with any Kotlin functions nor creating aliases would add any value (no reified classes, no functional support).

So in Kotlin code it will look like:

import org.axonframework.modelling.command.AggregateLifecycle.*

class MyAggregate {

    @CommandHandler
    fun someHandler(cmd: SomeCommand) {
        
        if (isLive()) {
            val version = getVersion()
            // do something
        }
    }
    
    @EventSourcingHandler
    fun on(evt: SomeEvent) {
        // process event
        markDeleted()
    }
}

I see no value in providing the same functions and just change the import.

@sonarcloud
Copy link

sonarcloud bot commented Oct 14, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@smcvb
Copy link
Member

smcvb commented Oct 14, 2020

Aaaah gotcha, I completely missed the point of collision in your original issue description. Could be me, could be the description; in the end it doesn't matter, as I feel that what you've done is fine as is. Thanks for clarifying @zambrovski.

Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concerns have been addressed, hence approving.

Copy link
Member

@sandjelkovic sandjelkovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@sandjelkovic sandjelkovic merged commit 10aef26 into AxonFramework:master Oct 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide improved methods for AggregateLifecycle
3 participants