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.

Sign up for GitHub

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

Jump to bottom

[#2640] Support @ExceptionHandler and @MessageHandlerInterceptor annotated methods in Sagas #2656

Merged
merged 4 commits into from May 9, 2023
Merged

[#2640] Support @ExceptionHandler and @MessageHandlerInterceptor annotated methods in Sagas #2656

merged 4 commits into from May 9, 2023

Conversation

gklijs
Copy link
Contributor

@gklijs gklijs commented Mar 22, 2023

Fixes #2640
It does 'break' the InspectedSagaModel I don't think this is a problem, but we can create an overloaded constructor setting a NoMoreInterceptors instance for the interceptorMemberChain if neccesary.

@gklijs gklijs added Type: Feature Use to signal an issue is completely new to the project. Priority 2: Should High priority. Ideally, these issues are part of the release they’re assigned to. Status: In Progress Use to signal this issue is actively worked on. labels Mar 22, 2023
@gklijs gklijs added this to the Release 4.8.0 milestone Mar 22, 2023
@gklijs gklijs self-assigned this Mar 22, 2023
@gklijs gklijs force-pushed the make_exception_handler_work_for_sagas branch from 33fe1b8 to ff40cfd Compare March 22, 2023 15:51
@smcvb smcvb changed the title Support @ExceptionHandler annotated methods in Sagas [#2640] Support @ExceptionHandler annotated methods in Sagas Apr 12, 2023
smcvb
smcvb requested changes Apr 12, 2023
View reviewed changes
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.

I've got some reservations about the placement of the chainedInterceptor() method we need to talk through.

modelling/src/main/java/org/axonframework/modelling/saga/metamodel/SagaModel.java Outdated Show resolved Hide resolved
@gklijs gklijs force-pushed the make_exception_handler_work_for_sagas branch from a7ac1a9 to b9c87f0 Compare April 21, 2023 08:51
@gklijs gklijs force-pushed the make_exception_handler_work_for_sagas branch from b9c87f0 to dad71f7 Compare April 28, 2023 09:04
@CodeDrivenMitch CodeDrivenMitch requested a review from smcvb May 4, 2023 13:52
@gklijs gklijs force-pushed the make_exception_handler_work_for_sagas branch from dad71f7 to 6d0ba9f Compare May 8, 2023 08:01
smcvb
smcvb approved these changes May 8, 2023
View reviewed changes
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.

I have some minor concerns with the current changes, but nothing major. I trust in the fact you'll account for my pointers. Hence I am approving preemptively.

modelling/src/main/java/org/axonframework/modelling/saga/metamodel/SagaMetaModelFactory.java Outdated Show resolved Hide resolved
modelling/src/main/java/org/axonframework/modelling/saga/AnnotatedSaga.java Show resolved Hide resolved
...lling/src/main/java/org/axonframework/modelling/saga/repository/AnnotatedSagaRepository.java Outdated Show resolved Hide resolved
...lling/src/main/java/org/axonframework/modelling/saga/repository/AnnotatedSagaRepository.java Show resolved Hide resolved
@gklijs gklijs changed the title [#2640] Support @ExceptionHandler annotated methods in Sagas [#2640] Support @ExceptionHandler and @MessageHandlerInterceptor annotated methods in Sagas May 8, 2023
@gklijs gklijs force-pushed the make_exception_handler_work_for_sagas branch from 6d0ba9f to 0d4cdd4 Compare May 9, 2023 09:20
@gklijs gklijs force-pushed the make_exception_handler_work_for_sagas branch from 0d4cdd4 to 38a656f Compare May 9, 2023 10:04
@gklijs gklijs merged commit 7bd83a5 into master May 9, 2023
5 of 6 checks passed
@gklijs gklijs deleted the make_exception_handler_work_for_sagas branch May 9, 2023 10:06
@gklijs gklijs added Status: Resolved Use to signal that work on this issue is done. and removed Status: In Progress Use to signal this issue is actively worked on. labels May 9, 2023
@sonarcloud
Copy link

sonarcloud bot commented May 9, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

87.5% 87.5% Coverage
1.9% 1.9% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smcvb smcvb smcvb approved these changes

@CodeDrivenMitch CodeDrivenMitch Awaiting requested review from CodeDrivenMitch

Assignees

@gklijs gklijs

Labels
Priority 2: Should High priority. Ideally, these issues are part of the release they’re assigned to. Status: Resolved Use to signal that work on this issue is done. Type: Feature Use to signal an issue is completely new to the project.
Projects
None yet
Milestone
Release 4.8.0
Development

Successfully merging this pull request may close these issues.

Support @ExceptionHandler annotated methods in Sagas
2 participants
@gklijs @smcvb