Make Multi streams compatible with Reactive Streams #1260
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
danielkec
commented
Dec 27, 2019
•
danielkec
commented
- Changes to common reactive module needed for MP Reactive Messaging and Reactive Streams support #1206
danielkec
commented
Dec 27, 2019
common/reactive/src/main/java/io/helidon/common/reactive/MultiMapProcessor.java
Outdated
Show resolved
Hide resolved
danielkec
commented
Dec 27, 2019
common/reactive/src/test/java/io/helidon/common/reactive/BaseProcessorTest.java
Outdated
Show resolved
Hide resolved
39 tasks
common/reactive/src/main/java/io/helidon/common/reactive/Multi.java
Outdated
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/Multi.java
Outdated
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/Multi.java
Outdated
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/Multi.java
Outdated
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/OfPublisher.java
Outdated
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/FailedPublisher.java
Outdated
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/Multi.java
Outdated
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/MultiFlatMapProcessor.java
Show resolved
Hide resolved
romain-grecourt
previously approved these changes
Jan 6, 2020
There was a problem hiding this comment.
@spericas @tomas-langer Can you please review this PR, thanks!
tomas-langer
requested changes
Jan 6, 2020
There was a problem hiding this comment.
There are a lot of new public classes with public constructors.
If the intention is for these classes to be public, please change constructors to private and create a public static factory method (default name should be create); otherwise change classes and constructors to package local
common/reactive/src/main/java/io/helidon/common/reactive/ConcatPublisher.java
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/ConcatPublisher.java
Outdated
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/FailedPublisher.java
Outdated
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/FailedPublisher.java
Outdated
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/MultiCoupledProcessor.java
Outdated
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/MultiTakeWhileProcessor.java
Outdated
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/OfPublisher.java
Outdated
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/OfPublisher.java
Outdated
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/StreamValidationUtils.java
Outdated
Show resolved
Hide resolved
common/reactive/src/main/java/io/helidon/common/reactive/SubscriberReference.java
Outdated
Show resolved
Hide resolved
|
@danielkec Can you please remove the valve stuff in a separate PR. |
danielkec
force-pushed
the
reactive-streams-compatible-operators
branch
from
5afe499
to
1da5ba1
Compare
Sure #1279 |
There was a problem hiding this comment.
Looks good to me.
You may want to remove the copyright changes in the valve related files to avoid a merge conflict when the PR that removes the valve stuff gets merged.
Also, can you look at the code coverage ? You can use mvn test -Pcoverage to generate a jacoco report under the target directory. Right now the coverage is pretty high, it would be nice to keep it that way.
Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
* breaks FlatMapStageVerification$InnerSubscriberVerification.required_spec209_mustBePreparedToReceiveAnOnCompleteSignalWithoutPrecedingRequestCall Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
* Unusual method name fix * Copyright year Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
* Build fix after cherry-pick Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
danielkec
force-pushed
the
reactive-streams-compatible-operators
branch
from
1da5ba1
to
38bf1b3
Compare
Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
|
@romain-grecourt Coverage aligned to 84% before and after, there is lot of additional tests in rs module(future PR) for the same processors + tck test suite. I would rather not duplicate test scenarios more than necessary. |
romain-grecourt
previously approved these changes
Jan 8, 2020
Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
tomas-langer
approved these changes
Jan 9, 2020
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.