Skip to content

UNOMI-492 Make rules engine more robust when some rules are invalid (null actions)#314

Merged
sergehuber merged 2 commits intomasterfrom
UNOMI-492-fix-rule-null-actions
Jun 28, 2021
Merged

UNOMI-492 Make rules engine more robust when some rules are invalid (null actions)#314
sergehuber merged 2 commits intomasterfrom
UNOMI-492-fix-rule-null-actions

Conversation

@sergehuber
Copy link
Copy Markdown
Contributor

Here's what was achieved in this PR:

  • Make sure we handle null actions gracefully instead of generating a NPE and blocking loading of further rules
  • Add an integration test to test the case of null actions in rule
  • Improve logging when rules have null or empty conditions and actions

…null actions)

- Make sure we handle null actions gracefully instead of generating a NPE and blocking loading of further rules
- Add an integration test to test the case of null actions in rule
- Improve logging when rules have null or empty conditions and actions
lastRuleCount = ruleMetadatas.size();
loopCount++;
}
assertEquals("Rule not properly saved", initialRuleCount + 1, lastRuleCount);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add a call to ruleService.getAllRules when it is registered.
Because the NPE from UNOMI-492 was generated at this moment:

java.lang.NullPointerException: null
        at org.apache.unomi.services.impl.ParserHelper.resolveActionTypes(ParserHelper.java:101) ~[!/:?]
        at org.apache.unomi.services.impl.rules.RulesServiceImpl.getAllRules(RulesServiceImpl.java:252) ~[!/:?]

ruleService.getRuleMetadatas() doesnt process the condition types and the actions.
ruleService.getRule(), ruleService.getAllRules() does

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes you are right I need to correct that. Thanks for catching it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I've modified the test to simply use getRule

@sergehuber sergehuber merged commit 0fa7cc1 into master Jun 28, 2021
@sergehuber sergehuber deleted the UNOMI-492-fix-rule-null-actions branch June 28, 2021 11:04
asfgit pushed a commit that referenced this pull request Jun 28, 2021
…null actions) (#314)

* UNOMI-492 Make rules engine more robust when some rules are invalid (null actions)
- Make sure we handle null actions gracefully instead of generating a NPE and blocking loading of further rules
- Add an integration test to test the case of null actions in rule
- Improve logging when rules have null or empty conditions and actions

* UNOMI-492 Simplify and improve integration test

(cherry picked from commit 0fa7cc1)
anatol-sialitski pushed a commit that referenced this pull request Sep 1, 2021
…null actions) (#314)

* UNOMI-492 Make rules engine more robust when some rules are invalid (null actions)
- Make sure we handle null actions gracefully instead of generating a NPE and blocking loading of further rules
- Add an integration test to test the case of null actions in rule
- Improve logging when rules have null or empty conditions and actions

* UNOMI-492 Simplify and improve integration test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants