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

Allow subscribing on same rejection with different causes #1205

Merged
merged 29 commits into from
Dec 4, 2019

Conversation

yuri-sergiichuk
Copy link
Contributor

This PR fixes #1202.

The EventHandlerMethod now shares the DispatchKey logic with the EventSubscriberMethod and creates a DispatchKey for rejections in the very same way.

@yuri-sergiichuk yuri-sergiichuk self-assigned this Nov 27, 2019
@codecov
Copy link

codecov bot commented Nov 27, 2019

Codecov Report

Merging #1205 into master will increase coverage by 0.03%.
The diff coverage is 96%.

@@             Coverage Diff              @@
##             master    #1205      +/-   ##
============================================
+ Coverage     91.49%   91.53%   +0.03%     
- Complexity     4269     4278       +9     
============================================
  Files           547      549       +2     
  Lines         13346    13355       +9     
  Branches        759      759              
============================================
+ Hits          12211    12224      +13     
+ Misses          894      892       -2     
+ Partials        241      239       -2

The getDeclaredMethods may behave differently in IDE and during the Gradle build process.
@yuri-sergiichuk yuri-sergiichuk marked this pull request as ready for review November 28, 2019 09:50
@yuri-sergiichuk
Copy link
Contributor Author

@armiol PTAL.

Copy link
Contributor

@alexander-yevsyukov alexander-yevsyukov left a comment

Choose a reason for hiding this comment

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

Added one minor request for extending EventEnvelope.


@Override
public DispatchKey key() {
if (parameterSpec().acceptsCommand()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's discuss this one in person. I am not sure I understand.

Copy link
Contributor

@alexander-yevsyukov alexander-yevsyukov left a comment

Choose a reason for hiding this comment

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

Please see my comment.

yuri-sergiichuk and others added 2 commits December 2, 2019 10:36
…atchKeys.java

Co-Authored-By: Alexander Yevsyukov <alexander.yevsyukov@teamdev.com>
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.

@yuri-sergiichuk please see my comments.

Class<? extends CommandMessage> commandMessageClass = toCommandMessage(parameters[1]);
Class<?> secondParameter = parameters[1];
checkArgument(CommandMessage.class.isAssignableFrom(secondParameter),
"The method `%s` should have the second parameter assignable from CommandMessage, but has `%s`.",
Copy link
Contributor

Choose a reason for hiding this comment

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

`CommandMessage`

import io.spine.type.MessageClass;

/**
* A handler method that handles rejections.
Copy link
Contributor

Choose a reason for hiding this comment

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

that is capable of handling rejection messages.

It's not that it always handles those. It just may handle.

/**
* {@inheritDoc}
*
* <p>If a handler method spec is for a rejection — creates rejection dispatch key.
Copy link
Contributor

Choose a reason for hiding this comment

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

In case this method is declared to handle a rejection, a rejection-specific dispatch key is created.

* <p>If a handler method spec is for a rejection — creates rejection dispatch key.
*/
@Override
default DispatchKey key() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I also think, @OverridingMethodsMustInvokeSuper.

@@ -139,6 +139,9 @@ default void ensureExternalMatch(boolean expectedValue) throws SignalOriginMisma
}
}

/**
* Creates a handler method dispatch key out of the {@code message class}.
Copy link
Contributor

Choose a reason for hiding this comment

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

...out of the {@linkplain #messageClass() message class}.

@yuri-sergiichuk
Copy link
Contributor Author

@armiol PTAL again.

@yuri-sergiichuk yuri-sergiichuk dismissed alexander-yevsyukov’s stale review December 4, 2019 09:44

Alex already approved the PR.

@yuri-sergiichuk yuri-sergiichuk merged commit 1b846bb into master Dec 4, 2019
@yuri-sergiichuk yuri-sergiichuk deleted the fix-1202 branch December 4, 2019 09:46
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.

It's not possible to subscribe on the same rejection with different causes
3 participants