Copilot commented on code in PR #6520:
URL:
https://github.com/apache/incubator-kie-drools/pull/6520#discussion_r2545478168
##########
drools-ruleunits/drools-ruleunits-impl/src/main/java/org/drools/ruleunits/impl/RuleUnitProviderImpl.java:
##########
@@ -66,21 +68,29 @@ public class RuleUnitProviderImpl implements
RuleUnitProvider {
private static final boolean USE_EXEC_MODEL = true;
- private final Map<String, RuleUnit> ruleUnitMap;
+ private final ConcurrentMap<String, RuleUnit> ruleUnitMap;
+ private final Object generationLock = new Object();
public RuleUnitProviderImpl() {
- this.ruleUnitMap =
loadRuleUnits(Thread.currentThread().getContextClassLoader());
+ this.ruleUnitMap = new
ConcurrentHashMap<>(loadRuleUnits(Thread.currentThread().getContextClassLoader()));
}
@Override
public <T extends RuleUnitData> RuleUnit<T> getRuleUnit(T ruleUnitData) {
String ruleUnitName = getRuleUnitName(ruleUnitData);
- RuleUnit<T> ruleUnit = ruleUnitMap.get(ruleUnitName);
+ RuleUnit<T> ruleUnit = (RuleUnit<T>) ruleUnitMap.get(ruleUnitName);
Review Comment:
[nitpick] Unchecked cast from `RuleUnit` to `RuleUnit<T>` occurs twice in
the method (lines 81 and 86). Consider extracting this into a helper method or
using `@SuppressWarnings("unchecked")` with clear documentation explaining why
this cast is safe. The cast is safe because the map only contains RuleUnits
that match their corresponding RuleUnitData types, but this invariant should be
documented.
##########
drools-ruleunits/pom.xml:
##########
@@ -59,4 +59,32 @@
<scope>test</scope>
</dependency>
</dependencies>
+
+ <profiles>
+ <profile>
+ <id>runTurtleTests</id>
+ <activation>
+ <property>
+ <name>runTurtleTests</name>
+ </property>
+ </activation>
+ <build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <systemProperties>
+ <property>
+ <name>runTurtleTests</name>
+ <value>true</value>
+ </property>
+ </systemProperties>
+ </configuration>
+ </plugin>
+ </plugins>
+ </pluginManagement>
Review Comment:
The `runTurtleTests` profile configuration is inconsistent with other
similar profiles in the codebase. In `drools-compiler/pom.xml`, the profile
sets `<excludedGroups/>` property instead of configuring surefire with
systemProperties. However, in
`drools-test-coverage/test-compiler-integration/pom.xml` and `kie-ci/pom.xml`,
the pattern matches this implementation. The configuration here is wrapped in
`<pluginManagement>` which may not apply the settings as expected. Consider
using `<plugins>` directly in `<build>` section instead of
`<pluginManagement>`, as seen in the test-compiler-integration module.
```suggestion
<plugins>
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<systemProperties>
<property>
<name>runTurtleTests</name>
<value>true</value>
</property>
</systemProperties>
</configuration>
</plugin>
</plugins>
```
--
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]
