jkevan commented on a change in pull request #365:
URL: https://github.com/apache/unomi/pull/365#discussion_r754275841
##########
File path:
services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java
##########
@@ -277,10 +278,23 @@ public void refreshRules() {
private List<Rule> getAllRules() {
List<Rule> rules = persistenceService.getAllItems(Rule.class, 0, -1,
"priority").getList();
+ List<Rule> rulesToSwitch = new ArrayList<>();
for (Rule rule : rules) {
- ParserHelper.resolveConditionType(definitionsService,
rule.getCondition(), "rule " + rule.getItemId());
- ParserHelper.resolveActionTypes(definitionsService, rule);
+ // Check rule integrity
+ boolean isValid =
ParserHelper.resolveConditionType(definitionsService, rule.getCondition(),
"rule " + rule.getItemId());
+ isValid = isValid &&
ParserHelper.resolveActionTypes(definitionsService, rule);
+ // check if rule status has changed
+ if (rule.getMetadata().isEnabled() != isValid) {
+ rule.getMetadata().setEnabled(isValid);
+ rulesToSwitch.add(rule);
+ }
Review comment:
Something is wrong here, in case:
- rule enabled: false
- rule valid: true
Then it will force enable the rule and save it as enabled, soometimes users
just want to create draft rule that could be valid but not enabled.
##########
File path:
services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java
##########
@@ -454,6 +470,8 @@ public void setRule(Rule rule) {
}
public void removeRule(String ruleId) {
+ Set<Rule> newRules = allRules.stream().filter(rule ->
!rule.getItemId().equals(ruleId)).collect(Collectors.toSet());
+ allRules = new ArrayList<>(newRules);
Review comment:
Don't we risk concurrency issue in case of multiple update of this list ?
in case there is the job running in // or multiple threads using the list ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
