Import third-party events #1152
Conversation
# Conflicts: # license-report.md
Codecov Report
@@ Coverage Diff @@
## master #1152 +/- ##
============================================
+ Coverage 91.43% 91.61% +0.18%
- Complexity 4208 4227 +19
============================================
Files 544 545 +1
Lines 13248 13309 +61
Branches 778 782 +4
============================================
+ Hits 12113 12193 +80
+ Misses 880 859 -21
- Partials 255 257 +2 |
# Conflicts: # license-report.md # server/src/main/java/io/spine/server/model/HandlerMap.java
|
@alexander-yevsyukov, @dmitrykuzmin, PTAL again. |
server/src/main/java/io/spine/server/integration/ThirdPartyContext.java
Outdated
Show resolved
Hide resolved
| * Creates a new single-tenant instance of {@code ThirdPartyContext} with the given name. | ||
| * | ||
| * @param name | ||
| * name of the third-party system |
There was a problem hiding this comment.
What we create is our representation of a part of a 3rd party system, as if it was a Bounded Context. So, I'd say this: “A name of a Context which represents a part of a third-party system.“ Anyway, it should say that it's not the whole system.
| * name of the third-party system | ||
| */ | ||
| public static ThirdPartyContext singleTenant(String name) { | ||
| checkNotNull(name); |
There was a problem hiding this comment.
Can we check for an empty string too, please?
| * name of the third-party system | ||
| */ | ||
| public static ThirdPartyContext multitenant(String name) { | ||
| checkNotNull(name); |
There was a problem hiding this comment.
Can we check for an empty string too, please?
| private static ThirdPartyContext newContext(String name, boolean multitenant) { | ||
| BoundedContext context = notStoringEvents(name, multitenant).build(); | ||
| context.integrationBus() | ||
| .notifyOfCurrentNeeds(); |
There was a problem hiding this comment.
We need to name this method so that it tells about the fact that Contexts need to speak to each other, maybe again, because a new context appeared in the whole picture of the system.
| checkNotNull(eventMessage); | ||
| checkTenant(actorContext, eventMessage); | ||
|
|
||
| EventId id = EventId |
There was a problem hiding this comment.
Why Events.generateId() does not work here?
| .newBuilder() | ||
| .setValue(newUuid()) | ||
| .build(); | ||
| EventContext eventContext = EventContext |
There was a problem hiding this comment.
Please have EventFactory to avoid generating event IDs and assembling Event instances. We need to encapsulate these details of creating events even.
| .get(); | ||
| return result; | ||
| if (count != 1) { | ||
| logWrongMethodCount(count, targetType); |
There was a problem hiding this comment.
Please test this branch.
|
@alexander-yevsyukov, PTAL again. |
| @@ -32,12 +33,13 @@ | |||
| /** | |||
| * The signature of {@link EventReactorMethod}. | |||
| */ | |||
| final class EventReactorSignature extends EventAcceptingSignature<EventReactorMethod> { | |||
| @Internal | |||
| public final class EventReactorSignature extends EventAcceptingSignature<EventReactorMethod> { | |||
There was a problem hiding this comment.
Can we hide this class back?
| @@ -89,7 +89,7 @@ public void handle(ExternalMessage value) { | |||
| clearStaleSubscriptions(request.getRequestedMessageTypeList(), origin); | |||
| if (!knownContexts.contains(origin)) { | |||
| knownContexts.add(origin); | |||
| integrationBus.notifyOfCurrentNeeds(); | |||
| integrationBus.introduceSelf(); | |||
There was a problem hiding this comment.
This doesn't sound right. The bus is already introduced to the system. It's the new context which introduces itself. Or, if you want the bus to do the job the method should be named differently.
| */ | ||
| _error().log( | ||
| "There are %d handler methods found for the type `%s`." + | ||
| "Expected only one. Model is corrupt.", |
There was a problem hiding this comment.
Model is corrupt
This doesn't tell the reader much of info. It just scares him. Tell the reader what to do about the problem.
Please include method names into diagnostics.
| return handler; | ||
| } | ||
|
|
||
| private IllegalStateException wrongMethodCount(Collection<H> handlers, M targetType) { |
There was a problem hiding this comment.
Please make this a state-check method and name it checkOnlyOne().
|
@alexander-yevsyukov, PTAL again. |
In this PR we introduce new API for posting events into a Bounded Context manually.
Mitivation
In real-life projects, often there are upstream third-party systems to integrate with. Such systems may emit events. However, constructing an instance of
io.spine.core.Eventand posting it into the correntEventBusmay be cumbersome, as well as error-prone, since theEventtype is the internal API of the framework.API
ThirdPartyContextis an adapter for external systems.ThirdPartyContextrepresents an external system as if it was just another Bounded Context. The adapter posts submitted by the user events to other Bounded Contexts via the common transport.Typical usage looks as follows:
Further Improvements
The next step for this feature is to allow events going through the
IntegrationBusbe posted into the domesticEventBusso that they are stored in the domesticEventStorefor catch up and audit.Other Changes
This PR also re-enabled the 3 disabled tests in the
servermodule. 2 of them used to fail due to unimplemented functionality and the third one is just too slow. All the tests now pass. The slow one is marked asSlowTestso that it can be skipped locally by runningfastTestGradle task instead oftest.