Copilot commented on code in PR #7080:
URL: https://github.com/apache/incubator-kie/pull/7080#discussion_r3911907649
##########
drools-ruleunits/drools-ruleunits-dsl/src/main/java/org/drools/ruleunits/dsl/RuleUnitProviderForDSL.java:
##########
@@ -74,7 +76,9 @@ public ModelRuleUnit(Class<T> type, Model model,
UnitGlobalsResolver unitGlobals
@Override
public RuleUnitInstance<T> internalCreateInstance(T data, RuleConfig
ruleConfig) {
- ReteEvaluator reteEvaluator = new RuleUnitExecutorImpl(ruleBase);
+ SessionConfiguration sessionConfiguration =
ruleBase.getSessionConfiguration().as(SessionConfiguration.KEY);
+ ((RuleConfigImpl)
ruleConfig).mergeSessionConfiguration(sessionConfiguration);
Review Comment:
`getSessionConfiguration()` returns the rule base's mutable configuration,
and this `ModelRuleUnit` is cached and reused. Merging `PSEUDO` here therefore
permanently changes the shared default: a later instance created without an
explicit clock (for which `mergeSessionConfiguration` is a no-op) also receives
a pseudo clock, while concurrent creations with different clocks can race.
Create a per-instance session configuration copied from the rule-base defaults
before applying `RuleConfig`, and add a pseudo-then-default regression case.
##########
drools-ruleunits/drools-ruleunits-dsl/src/main/java/org/drools/ruleunits/dsl/RuleUnitProviderForDSL.java:
##########
@@ -74,7 +76,9 @@ public ModelRuleUnit(Class<T> type, Model model,
UnitGlobalsResolver unitGlobals
@Override
public RuleUnitInstance<T> internalCreateInstance(T data, RuleConfig
ruleConfig) {
- ReteEvaluator reteEvaluator = new RuleUnitExecutorImpl(ruleBase);
+ SessionConfiguration sessionConfiguration =
ruleBase.getSessionConfiguration().as(SessionConfiguration.KEY);
+ ((RuleConfigImpl)
ruleConfig).mergeSessionConfiguration(sessionConfiguration);
Review Comment:
The public creation API accepts the `RuleConfig` interface, so this
implementation cast introduces a `ClassCastException` for any caller-provided
implementation that worked on the previous DSL path. Apply
`ruleConfig.getClockType()` through the interface (or expose the merge
operation through an API-level abstraction) rather than requiring
`RuleConfigImpl`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
