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.

Sign up for GitHub

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

Jump to bottom

[PoC] Import decisions and BKMs from another DMN #222

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

[PoC] Import decisions and BKMs from another DMN #222

wants to merge 9 commits into from

Conversation

saig0
Copy link
Member

@saig0 saig0 commented Jun 27, 2023 •
edited
Loading

Description

This is a proof-of-concept (PoC) to see how we could import decisions and BKMs from another DMN file.

The general idea:

  • Store parsed decisions/BKMs in a repository.
  • Ignore imported decisions/BKMs during the parsing.
  • On evaluation, load the imported decisions/BKMs from the repository.

Open to-do's:

  • Move the loading of imported decisions/BKMs from the parser to the evaluation logic. The import is part of the evaluation.
  • Handle decisions/BKMs with namespaces natively (i.e. qualified names). Don't modify the name.
  • Evaluate imported decisions/BKMs differently by wrapping them into a FEEL context. The evaluation interprets the qualified name as a path expression.
  • Handle the case if an imported decision/BKM is not present. Don't throw exceptions.
  • Document the new public API.
  • Write more test cases to cover edge cases.
  • Fix the InvocationTest.scala. Out of scope for this PR because the test was not really correct before.

Related issues

related to #71

This was referenced Jun 27, 2023
Closed
Open
@tracy-hires tracy-hires mentioned this pull request Aug 28, 2023
Merged
7 tasks
saig0 and others added 9 commits October 6, 2023 13:44
@saig0
test: add TCK test case for BKM import
c0c6ea3 
Replace whitespaces. Set decision id.
@saig0
wip: evaluate decision with imported BKM
551a62d
@saig0
test: ignore invocation test case
db85f16 
The invocation test fails now because all BKMs are parsed. Instead of parsing only BKMs that are referenced by a decision.

However, the behavior was also not fully correct before. If another decision referenced the BKM before then the invocation would invoke the BKM.
@saig0
test: modify DMN for unique results
dbdb927 
test: add TCK test case for imported decisions

Replace whitespaces. Replace decision id.

Modify the literal expressions of the decisions to return a different result.
@saig0
wip: evaluate decision with imported decision
b6f3bd9
@tracyhires @saig0
late-loading of imported models
41190bb
@tracyhires @saig0
cleanup code
5b193a5
@tracyhires @saig0
more code cleanup
0adecd2
@saig0
refactor: move decision/BKM loading to evaluation
d9dc857 
Remove the resolve() function of the references. Instead, load imported models during the evaluation.

Handle missing models during the evaluation by returning a failure.

Refine and simplify the parsed types.
@saig0
Copy link
Member Author

saig0 commented Oct 6, 2023

I merged #225 into the branch and rebased it on main. 🏗️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@saig0 @tracyhires