Skip to content

Clarify the command request rejection callback API#151

Merged
dmitrykuzmin merged 8 commits intomasterfrom
rename-filter-rejection-callback
Sep 1, 2020
Merged

Clarify the command request rejection callback API#151
dmitrykuzmin merged 8 commits intomasterfrom
rename-filter-rejection-callback

Conversation

@dmitrykuzmin
Copy link
Contributor

@dmitrykuzmin dmitrykuzmin commented Aug 31, 2020

This PR resolves #6.

With the current imlementation, it seems that the onRejection callback for a command request will be run once the command leads to a business rejection. But in reality that's not the case.

The callback will be invoked if and only if the passed command cannot bypass the command bus filters due to a business rejection (created and thrown by a filter). It is a valid use-case in our system for e.g. security.

This PR thus renames the callback to onImmediateRejection and adjusts the corresponding doc.

The "ordinary" (thrown by handler methods) rejections can be tracked using the observe(...) method of CommandRequest.

Library version advances to 1.5.25.

@dmitrykuzmin dmitrykuzmin self-assigned this Aug 31, 2020
@dmitrykuzmin dmitrykuzmin requested a review from armiol August 31, 2020 20:38
@dmitrykuzmin
Copy link
Contributor Author

dmitrykuzmin commented Aug 31, 2020

@armiol PTAL.

I couldn't come up with an obviously better term than onImmediateRejection. I can propose something like onFilterRejection, onExecutionDenied, onDeskRejection (a term with similar meaning but it sounds a bit strange in this context).

The build will pass once core-java#1295 is merged.

Copy link
Contributor

@armiol armiol left a comment

Choose a reason for hiding this comment

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

@dmitrykuzmin LGTM with a single comment to address.

@codecov
Copy link

codecov bot commented Sep 1, 2020

Codecov Report

Merging #151 into master will decrease coverage by 0.36%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master     #151      +/-   ##
============================================
- Coverage     62.16%   61.79%   -0.37%     
  Complexity      178      178              
============================================
  Files            87       88       +1     
  Lines          2175     2188      +13     
  Branches         38       40       +2     
============================================
  Hits           1352     1352              
- Misses          814      827      +13     
  Partials          9        9              

@dmitrykuzmin
Copy link
Contributor Author

The missing coverage are either lines fully covered by the integration tests, or the integration tests themselves. So I'm merging the PR.

@dmitrykuzmin dmitrykuzmin merged commit a90e36e into master Sep 1, 2020
@dmitrykuzmin dmitrykuzmin deleted the rename-filter-rejection-callback branch September 1, 2020 12:08
@dmitrykuzmin dmitrykuzmin mentioned this pull request Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BackendClient.sendCommand method's rejection callback is not invoked

2 participants