-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
Add rule for coordination between services of different contexts #348
Add rule for coordination between services of different contexts #348
Conversation
add mappings between bounded contexts
- Remove 'sagaOrchestrator' - Refactor step '.' to '::'
- Fix wrong reference to validator file in generator - Update validation logic and messages - Remove scope reduction
- Add sketchminer generator - Add generator tests
Codecov Report
@@ Coverage Diff @@
## master #348 +/- ##
==========================================
- Coverage 93.0% 92.6% -0.4%
- Complexity 2354 2406 +52
==========================================
Files 204 209 +5
Lines 6344 6490 +146
Branches 851 875 +24
==========================================
+ Hits 5902 6016 +114
- Misses 214 225 +11
- Partials 228 249 +21
|
Thanks for creating the PR @mLeveIST ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mLeveIST
I have a few small comments (only code-level details), but overall this is very good work 👌 Looking forward to merge it 😊
Can you maybe have a look at the comments? If something is not clear I can help or we discuss it in a short meeting. Another thing we have to prepare is documentation on the website (repo: https://github.com/ContextMapper/contextmapper.github.io).
Best regards,
Stefan
...org/contextmapper/dsl/ide/tests/quickfixes/OpenCoordinationInBPMNSketchMinerActionTest.xtend
Outdated
Show resolved
Hide resolved
|
||
@Test | ||
@DisabledOnOs(OS.WINDOWS) | ||
public void canCreateSketchMinerLinkForCoordinationModelUnix() throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also add these tests for Windows. Do you have a Windows machine to test it? Otherwise I can do that...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately I could not test it on a Windows :(
org.contextmapper.dsl.tests/src/org/contextmapper/dsl/ApplicationLayerDSLParsingTest.xtend
Outdated
Show resolved
Hide resolved
org.contextmapper.dsl/src/org/contextmapper/dsl/cml/CMLModelObjectsResolvingHelper.java
Show resolved
Hide resolved
...pper.dsl/src/org/contextmapper/dsl/validation/ApplicationCoordinationSemanticsValidator.java
Outdated
Show resolved
Hide resolved
Hello Stefan, Thank you for the review! Best regards, |
Hello @stefan-ka, I added most of the changes you requested, just missing the test for the Windows machine... Regarding the documentation, can this be added to the Application and Process Layer page? If you have the time this or next week, I wouldn't mind having a small meeting to discuss the results of Coordinations as is, and if you think it is worthwhile to expand upon it in the future (having the type of coordination, the synchronicity of the steps, if they can relate to flows). Best regards, |
Hi @mLeveIST Thank you! I will try to do the tests on Windows... I will come back to you regarding a meeting. Probably next week earliest. Best regards, |
Sorry @mLeveIST, I forgot to answer your question regarding documentation. I would say, yes. Or create a new page but then we have to link it on the application layer page. Best regards, |
Hi @mLeveIST I just realized that we have many other issues with unit tests on Windows 🫣 From my point of view we can then merge it. But lets give @socadk a chance to test the new feature as well. We'll come back to you. Best regards, |
Hi @mLeveIST We tested your feature and it looks good from our side, so I merged it right now :) Were you already able to do something regarding documentation? ;) I can also put it on my own TODO-list, but I think it would be good if you could do it; also to provide proper background information. Best regards, |
Hi @stefan-ka, Thank you for the update! That's great to know. Best regards, |
Perfect, thanks @mLeveIST! |
Overview
This change provides a way to represent coordination between services of different contexts, with the addition of a Coordination rule in the Application and Process Layer of Bounded Contexts. Example in CML:
A Coordination can be visualised in a BPMN diagram with Sketch Miner, where each context will be represented as a different actor, and each operation as a task. The example above generates the following code:
Motivation
The main drive for this rule is to be able to model in CML processes that span multiple Bounded Contexts, like Sagas. CML does already provides a Flow rule to model processes, but these processes are restricted to a single Bounded Context and the rule has a focus on modelling Event/Command flows for Event Storming.
Technical Details
The following sections specify details on the implementation of the rule.
Validation
Since the steps of a coordination are composed of references to other rules, the following verifications are done to avoid ambiguity when pinpointing the step operation:
IDE integration
The following features were implemented for the IDE when writing in CML: