[2.x] Enable `IntegrationBroker` dispatch events regardless of registration order of subscribing and publishing BCs #1408

yevhenii-nadtochii · 2021-10-28T14:45:33Z

It's a migration of #1402 to the 2.x branch.

Additionally, the code has been migrated to the latest McJavaOptions. The versions of the dependencies were set to the latest available.

…s registration

codecov · 2021-10-28T15:30:00Z

Codecov Report

Merging #1408 (419a37e) into master (6e40708) will increase coverage by 0.27%.
The diff coverage is 89.11%.

@@             Coverage Diff              @@
##             master    #1408      +/-   ##
============================================
+ Coverage     90.83%   91.10%   +0.27%     
- Complexity     4884     4907      +23     
============================================
  Files           624      626       +2     
  Lines         15326    15342      +16     
  Branches        891      890       -1     
============================================
+ Hits          13921    13978      +57     
+ Misses         1104     1063      -41     
  Partials        301      301

server/src/main/java/io/spine/server/integration/AbstractChannelObserver.java

server/src/main/java/io/spine/server/integration/ExternalMessages.java

alexander-yevsyukov

Please see my comments.

alexander-yevsyukov · 2021-11-02T14:57:34Z

server/src/main/java/io/spine/server/integration/IntegrationBroker.java

+    private static final ChannelId MESSAGE_SOURCES_CHANNEL_ID = channelIdFor(
+            TypeUrl.of(ExternalMessagesSourceAvailable.class)
+    );
+    private static final ChannelId NEEDS_EXCHANGE_CHANNEL_ID = channelIdFor(


In the spirit of Ubiquitous Language, let's not use ID suffix, unless it's absolutely necessary. Since things in modern DDD are referenced by type-safe IDs (and not references to a memory block with an object), having the suffix does not have much meaning.

armiol · 2021-11-11T18:34:24Z

@yevhenii-nadtochii FYI. For some reason, I am not able to request a review from you. Probably, because you are the original author. However, should you have any comments/questions, let's discuss those vocally.

alexander-yevsyukov

Please see some minor comments. Will continue reviewing tomorrow.

alexander-yevsyukov · 2021-11-11T20:42:24Z

server/src/main/java/io/spine/server/BoundedContextBuilder.java

@@ -90,7 +89,7 @@

    /**
     * Event dispatchers to be registered with the context {@link EventBus} and/or
-     * {@link IntegrationBroker} after the Bounded Context creation.
+     * {@link io.spine.server.integration.IntegrationBroker} after the Bounded Context creation.


Can we have a short name of the class following the fully-qualified name in the link, please?

alexander-yevsyukov · 2021-11-11T20:54:04Z

server/src/main/java/io/spine/server/integration/ConfigExchange.java

+     */
+    private static ExternalEventType typeOfTransmittedEvents(ChannelId channel) {
+        return ExternalEventType
+                .newBuilder()


Please have .newBuilder() on the previous line.

alexander-yevsyukov · 2021-11-11T20:55:04Z

server/src/main/java/io/spine/server/integration/ConfigExchange.java

+     */
+    @Override
+    public void close() throws Exception {
+        for (ObserveWantedEvents observer : observers) {


Can we have forEach() here, please?

alexander-yevsyukov · 2021-11-11T20:55:53Z

server/src/main/java/io/spine/server/integration/EventsExchange.java

+ * <p>Receives the domain events from other Bounded Contexts and posts them to the domestic
+ * bus treating them as {@code external}.
+ */
+class EventsExchange extends AbstractExchange {


alexander-yevsyukov

LGTM, with few minor comments to address

alexander-yevsyukov · 2021-11-12T10:22:06Z

server/src/main/java/io/spine/server/integration/EventsExchange.java

+     * @param event
+     *         event to publish
+     */
+    @Internal


Do we really need this annotation, assuming that the class is package-private?

alexander-yevsyukov · 2021-11-12T10:25:36Z

server/src/main/java/io/spine/server/integration/EventsExchange.java

+     * Creates an ID of the channel that will transmit the events of the given class.
+     */
+    private static ChannelId toChannelId(EventClass cls) {
+        checkNotNull(cls);


This null check looks as excessive, assuming we're doing it in private static method, and the next line calls a method of the passed object, anyway.

yevhenii-nadtochii

Please see my questions

server/src/main/java/io/spine/server/integration/AbstractChannelObserver.java

server/src/main/java/io/spine/server/integration/AbstractExchange.java

server/src/main/java/io/spine/server/integration/ConfigExchange.java

server/src/main/java/io/spine/server/integration/IntegrationBroker.java

server/src/main/java/io/spine/server/integration/TransportLink.java

yevhenii-nadtochii added 10 commits October 28, 2021 15:56

Make BoundedContext.Internal register*() methods chainable

272da03

Fix caught typos

44bb506

Create ExternalMessagesSourceAvailable proto message

92494ee

Make BlackBox implement Closeable

d7aded9

Make entity type of EmptyAggregate to AGGREGATE

58ecd0e

Implement new proto messages for IntegrationBrokerTest

7dcf2d1

Delete no longer used proto messages of IntegrationBrokerTest

d56b47b

Re-write tests of IntegrationBroker with BlackBox

6d17094

Make IntegrationBroker emit ExternalMessagesSourceAvailable on it…

fac9724

…s registration

Bump the version

59e5291

yevhenii-nadtochii marked this pull request as ready for review October 28, 2021 14:59

Fix EOF

bdc52dd

armiol assigned yevhenii-nadtochii Nov 1, 2021

armiol self-requested a review November 1, 2021 13:36

alexander-yevsyukov reviewed Nov 1, 2021

View reviewed changes

server/src/main/java/io/spine/server/integration/AbstractChannelObserver.java Outdated Show resolved Hide resolved

alexander-yevsyukov reviewed Nov 1, 2021

View reviewed changes

server/src/main/java/io/spine/server/integration/ExternalMessages.java Outdated Show resolved Hide resolved

yevhenii-nadtochii added 2 commits November 1, 2021 21:03

Use StringValue.of() instead of builder

1be6cb6

Fix @linkplain

5f18ff4

yevhenii-nadtochii requested a review from alexander-yevsyukov November 1, 2021 20:22

armiol added 6 commits November 2, 2021 16:21

Remove the dead code.

7205587

Reformat the code to fit the max page width.

cd27c01

Shorten the method name.

cc24abb

Shorten the expression.

3809a93

Extract and document the anonymous type.

27dacbe

Fix the indentation in the Javadoc section.

d6a4277

armiol marked this pull request as draft November 2, 2021 14:41

armiol removed the request for review from alexander-yevsyukov November 2, 2021 14:41

armiol self-assigned this Nov 2, 2021

alexander-yevsyukov requested changes Nov 2, 2021

View reviewed changes

armiol added 4 commits November 11, 2021 19:15

Bump own version. Use the latest mc-java as well.

87a1894

Migrate to the new API of McJavaOptions.

79a2e04

Migrate test-artifacts.

ae3e9d8

Update the license reports.

ce2912a

armiol unassigned yevhenii-nadtochii Nov 11, 2021

armiol removed their request for review November 11, 2021 18:31

armiol marked this pull request as ready for review November 11, 2021 18:34

armiol requested a review from alexander-yevsyukov November 11, 2021 18:34

alexander-yevsyukov reviewed Nov 11, 2021

View reviewed changes

alexander-yevsyukov approved these changes Nov 12, 2021

View reviewed changes

armiol added 6 commits November 12, 2021 13:41

Add the alt text to links.

7e46e7e

Adjust the code layout.

99f744b

Use forEach.

35a23b4

Mark the final class as such.

0a921d5

Avoid using @Internal for the package-private API.

9fd15f6

Remove the excessive check.

a81e5f3

yevhenii-nadtochii commented Nov 12, 2021

View reviewed changes

armiol added 7 commits November 12, 2021 14:34

Fix a typo.

f4ba93b

Shorten the description.

9e9c995

Reduce the visibility.

8bbf1e7

Hide the utility as it's now never used from outside.

78e8df8

Use a proper tag for the alt text.

5844f4a

Suppress the warning.

ff88d9a

Kill the unused exception.

419a37e

armiol merged commit a3fe6f8 into master Nov 12, 2021

armiol deleted the integration-broker-feature branch November 12, 2021 13:25

armiol changed the title ~~Enable IntegrationBroker dispatch events regardless of registration order of subscribing and publishing BCs~~ [2.x] Enable IntegrationBroker dispatch events regardless of registration order of subscribing and publishing BCs Nov 16, 2021

armiol changed the title ~~[2.x] Enable IntegrationBroker dispatch events regardless of registration order of subscribing and publishing BCs~~ [2.x] Enable IntegrationBroker dispatch events regardless of registration order of subscribing and publishing BCs Nov 16, 2021

armiol mentioned this pull request Nov 16, 2021

Context initialization order matters when one context reacts to other's events #1392

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2.x] Enable `IntegrationBroker` dispatch events regardless of registration order of subscribing and publishing BCs #1408

[2.x] Enable `IntegrationBroker` dispatch events regardless of registration order of subscribing and publishing BCs #1408

yevhenii-nadtochii commented Oct 28, 2021 •

edited by armiol

Loading

codecov bot commented Oct 28, 2021 •

edited

Loading

alexander-yevsyukov left a comment

alexander-yevsyukov Nov 2, 2021

armiol commented Nov 11, 2021

alexander-yevsyukov left a comment

alexander-yevsyukov Nov 11, 2021

alexander-yevsyukov Nov 11, 2021

alexander-yevsyukov Nov 11, 2021

alexander-yevsyukov Nov 11, 2021

alexander-yevsyukov left a comment

alexander-yevsyukov Nov 12, 2021

alexander-yevsyukov Nov 12, 2021

yevhenii-nadtochii left a comment

[2.x] Enable IntegrationBroker dispatch events regardless of registration order of subscribing and publishing BCs #1408

[2.x] Enable IntegrationBroker dispatch events regardless of registration order of subscribing and publishing BCs #1408

Conversation

yevhenii-nadtochii commented Oct 28, 2021 • edited by armiol Loading

codecov bot commented Oct 28, 2021 • edited Loading

Codecov Report

alexander-yevsyukov left a comment

Choose a reason for hiding this comment

alexander-yevsyukov Nov 2, 2021

Choose a reason for hiding this comment

armiol commented Nov 11, 2021

alexander-yevsyukov left a comment

Choose a reason for hiding this comment

alexander-yevsyukov Nov 11, 2021

Choose a reason for hiding this comment

alexander-yevsyukov Nov 11, 2021

Choose a reason for hiding this comment

alexander-yevsyukov Nov 11, 2021

Choose a reason for hiding this comment

alexander-yevsyukov Nov 11, 2021

Choose a reason for hiding this comment

alexander-yevsyukov left a comment

Choose a reason for hiding this comment

alexander-yevsyukov Nov 12, 2021

Choose a reason for hiding this comment

alexander-yevsyukov Nov 12, 2021

Choose a reason for hiding this comment

yevhenii-nadtochii left a comment

Choose a reason for hiding this comment

[2.x] Enable `IntegrationBroker` dispatch events regardless of registration order of subscribing and publishing BCs #1408

[2.x] Enable `IntegrationBroker` dispatch events regardless of registration order of subscribing and publishing BCs #1408

yevhenii-nadtochii commented Oct 28, 2021 •

edited by armiol

Loading

codecov bot commented Oct 28, 2021 •

edited

Loading