tkobayas opened a new issue, #7137:
URL: https://github.com/apache/incubator-kie/issues/7137
**Describe the bug**
With the executable model, `@propertyChangeSupport` on a DRL type
declaration has no effect. The engine does not register itself as a
`PropertyChangeListener` on inserted facts, so setters that fire
`PropertyChangeEvent` do not trigger an update.
It works with the non-executable model (DRL compiled by `drools-compiler`).
Related: #7132. That issue deprecates `insert(Object object, boolean
dynamic)` and points users to `@propertyChangeSupport`. Today, executable model
users have no working replacement.
**Expected behavior**
Same as the non-executable model: after `ksession.insert(fact)`, the engine
registers a listener on the fact, and a property change event causes an update
of the fact.
**Actual behavior**
No listener is registered (`getPropertyChangeListeners().length == 0` after
insert). The property change is not seen by the engine, and rules that depend
on it do not fire.
**How to Reproduce?**
A JavaBean fact:
```java
public class DynamicFact {
private final PropertyChangeSupport support = new
PropertyChangeSupport(this);
private String name;
private String value;
public String getName() { return name; }
public void setName(String name) {
String old = this.name;
this.name = name;
support.firePropertyChange("name", old, name);
}
public String getValue() { return value; }
public void setValue(String value) {
String old = this.value;
this.value = value;
support.firePropertyChange("value", old, value);
}
public void addPropertyChangeListener(PropertyChangeListener l) {
support.addPropertyChangeListener(l); }
public void removePropertyChangeListener(PropertyChangeListener l) {
support.removePropertyChangeListener(l); }
}
```
DRL:
```drl
import org.example.DynamicFact;
declare DynamicFact
@propertyChangeSupport
end
rule rule1 when
$f : DynamicFact( name == "user1" )
then
$f.setName("user2"); // no modify: the property change event should
trigger the update
end
rule rule2 when
$f : DynamicFact( name == "user2" )
then
$f.setValue("VAL1");
end
```
```java
DynamicFact fact = new DynamicFact();
fact.setName("user1");
ksession.insert(fact);
ksession.fireAllRules();
// expected: fact.getValue() == "VAL1"
```
Result with a `BaseModelTest` in `drools-model-codegen`:
| Case | STANDARD_FROM_DRL | PATTERN_DSL (executable model) |
|---|---|---|
| `declare ... @propertyChangeSupport end` + `ksession.insert(fact)` | OK
(listeners after insert: 1, `value = "VAL1"`) | **NG** (listeners after insert:
0, `value = null`) |
| RHS `insert(fact, true)` (deprecated) | OK | OK |
**Additional information**
Cause:
- In the non-executable model, `TypeDeclarationFactory.processAnnotations()`
sets `TypeDeclaration.setDynamic(true)`:
https://github.com/apache/incubator-kie/blob/a0b09cc64fa53b67770b4db69fe935df9bfc2220/drools-compiler/src/main/java/org/drools/compiler/builder/impl/TypeDeclarationFactory.java#L105
At runtime, `NamedEntryPoint` registers the listener when
`typeConf.isDynamic()` is true:
https://github.com/apache/incubator-kie/blob/a0b09cc64fa53b67770b4db69fe935df9bfc2220/drools-kiesession/src/main/java/org/drools/kiesession/entrypoints/NamedEntryPoint.java#L230
- In the executable model, codegen (`POJOGenerator.processTypeMetadata()`)
emits the annotation as `TypeMetaData`. But
`TypeDeclarationUtil.wireMetaTypeAnnotations()` has no case for
`propertyChangeSupport`, and it is not in `KNOWN_ANNOTATIONS`. It is only
stored as custom metadata, and `setDynamic(true)` is never called:
https://github.com/apache/incubator-kie/blob/a0b09cc64fa53b67770b4db69fe935df9bfc2220/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/util/TypeDeclarationUtil.java#L53-L120
The existing test `PropertyReactivityTest.testPropertyChangeSupportNewAPI`
runs with the executable model too, but it does not catch this. Its assertions
(the rule does not re-fire, and no listener is left after `dispose()`) are also
true when no listener was ever registered:
https://github.com/apache/incubator-kie/blob/a0b09cc64fa53b67770b4db69fe935df9bfc2220/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/mvel/integrationtests/PropertyReactivityTest.java#L1693
Proposed fix:
1. In `TypeDeclarationUtil`, add `"propertyChangeSupport"` to
`KNOWN_ANNOTATIONS` and add a case in `wireMetaTypeAnnotations()`:
```java
case "propertyChangeSupport":
typeDeclaration.setDynamic( true );
break;
```
No runtime change is needed.
2. Add an executable model test (`BaseModelTest`) with the scenario above:
automatic update, and listener count after insert / delete / dispose.
3. Make `testPropertyChangeSupportNewAPI` assert that a listener is
registered after insert.
Open questions:
- The non-executable model also accepts the capitalized form
`@PropertyChangeSupport` in DRL. Should the executable model accept it too?
Other annotations in `wireMetaTypeAnnotations()` (`role`, `expires`, ...) are
matched by the exact lowercase name as well.
- The Java annotation `org.kie.api.definition.type.PropertyChangeSupport` on
a Java class is ignored in both models
(`TypeDeclaration.processTypeAnnotations()` does not read it). Should it be
supported, here or in a separate issue?
Version: main (`999-SNAPSHOT`, a0b09cc64fa), Java 17.
--
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]
