Hide Forgot
Created attachment 1135319 [details] Reproducer Description of problem: When the user tries to delete a Fact that was inserted using the 'insertLogical()' operation, the deletion fails with the below exception: ---------- Exception ---------- java.lang.IllegalArgumentException: The FactHandle did not originate from WM : [fact 0:1:1990421361:1990421361:1:DEFAULT:NON_TRAIT:com.company.support.DecisionTableFactValue:com.company.support.DecisionTableFactValue@76a36b71] at org.drools.core.common.NamedEntryPoint.delete(NamedEntryPoint.java:474) at org.drools.core.impl.StatefulKnowledgeSessionImpl.delete(StatefulKnowledgeSessionImpl.java:1505) at org.drools.core.impl.StatefulKnowledgeSessionImpl.delete(StatefulKnowledgeSessionImpl.java:1496) at com.company.support.DecisionTableRuleTest.factInsertedInWorkingMemoryCannotBeDeleted(DecisionTableRuleTest.java:51) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:86) at org.testng.internal.Invoker.invokeMethod(Invoker.java:643) at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:820) at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1128) at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129) at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112) .... ---------- Exception ---------- Version-Release number of selected component (if applicable): 6.2.0 How reproducible: Everytime delete() is executed on a logically inserted Fact. Steps to Reproduce: 1. Logically insert some fact. 2. Retrieve FactHandle for inserted fact (either via query or KieSession.getFactHandles()). 3. Delete fact using ksession.delete() Actual results: Fact deleted. Expected results: Exception thrown. Additional info: Reproducer attached. Please unzip and unit test DecisionTableRuleTest to see the exception reproduced.
Mario see that it works for 6.2.x branch, but not for 6.3.x, so could be a regression. If it is not a regression, but a new check added to 6.3.x so: - Should be incremented the community doc first (to be consumed for product doc) in the TMS chapeter, mentioning that such operation should not be allowed. - The exception should be clear. An Illegal Operation Exception can be thrown reporting as message something like "Deleting a fact inserted logically is not supported currently".
I reproduced the reported issue with the simple test case that I'm pasting below. That exception is indeed thrown by this check https://github.com/droolsjbpm/drools/commit/3132b8febc5c316d5cc1b366df7af68db10c80c4#diff-7290855911f5856e5432eb94c66e5ac8R500 that was not present on branch 6.2.x. I'm pretty sure that this is a wanted (even if not well documented) behaviour, even because we have a test that is explicitly checking that when the user tries to programmatically delete a logically insert fact that exception is thrown: https://github.com/droolsjbpm/drools/blob/master/drools-compiler/src/test/java/org/drools/compiler/integrationtests/TruthMaintenanceTest.java#L1483 Alessandro, under my point of view the current error message "The FactHandle did not originate from WM" is not technically wrong, even if I admit that could be not immediately understandable for an end user. The message you're proposing is indeed more straightforward, but in my opinion has 2 minor issues. In particular saying that: "Deleting a fact inserted logically is not supported currently" 1. Can be even more misleading in some circumstances. I'm not sure if that exception could be thrown only when attempting the deletion of logically inserted facts. In particular I suspect it could be also thrown when using a mutable hashcode, so we should at least include this possibility. 2. Implies that this is a feature that we don't have at moment, but we want to support in future, and I think this is not true. Please email me or ping me on IRC, so we could agree on a better message. ---- @Test public void testDeleteLogicalInsertion() { // BZ-1317026 String drl = "rule R when\n" + "then\n" + " insertLogical(\"test\");\n" + "end\n"; KieSession ksession = new KieHelper().addContent( drl, ResourceType.DRL ) .build() .newKieSession(); ksession.fireAllRules(); Collection<FactHandle> factHandles = ksession.getFactHandles( new org.kie.api.runtime.ClassObjectFilter( String.class) ); assertEquals(1, factHandles.size()); ksession.delete(factHandles.iterator().next()); factHandles = ksession.getFactHandles( new org.kie.api.runtime.ClassObjectFilter( String.class) ); assertEquals(0, factHandles.size()); }
Setting blocker flag for 6.3 as this has also been requested for 6.2.3
Fixed on master by: droolsjbpm-knowledge: https://github.com/droolsjbpm/droolsjbpm-knowledge/commit/2a201147d drools: https://github.com/droolsjbpm/drools/commit/7a62f35a9
Documented by https://github.com/droolsjbpm/kie-docs/commit/d7b2d437e8e428fceca21680f3a0256b1d82d4b6
Verified by running tests from PR in different environments.