Bug 1317026 - Deletion of logically inserted Fact fails in 6.2
Summary: Deletion of logically inserted Fact fails in 6.2
Keywords:
Status: CLOSED EOL
Alias: None
Product: JBoss BRMS Platform 6
Classification: Retired
Component: BRE
Version: 6.2.0
Hardware: All
OS: All
urgent
high
Target Milestone: ER2
: 6.3.0
Assignee: Mario Fusco
QA Contact: Tibor Zimanyi
URL:
Whiteboard:
Depends On:
Blocks: 1317058
TreeView+ depends on / blocked
 
Reported: 2016-03-11 18:02 UTC by Jean Abraham
Modified: 2020-03-27 19:13 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 1317058 (view as bug list)
Environment:
Last Closed: 2020-03-27 19:13:10 UTC
Type: Bug


Attachments (Terms of Use)
Reproducer (63.43 KB, application/zip)
2016-03-11 18:02 UTC, Jean Abraham
no flags Details

Description Jean Abraham 2016-03-11 18:02:21 UTC
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.

Comment 2 Alessandro Lazarotti 2016-03-11 19:55:38 UTC
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".

Comment 4 Mario Fusco 2016-03-14 10:00:00 UTC
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());
    }

Comment 5 Kris Verlaenen 2016-03-16 15:22:52 UTC
Setting blocker flag for 6.3 as this has also been requested for 6.2.3

Comment 6 Mario Fusco 2016-03-18 09:17:47 UTC
Fixed on master by:

droolsjbpm-knowledge:
https://github.com/droolsjbpm/droolsjbpm-knowledge/commit/2a201147d

drools:
https://github.com/droolsjbpm/drools/commit/7a62f35a9

Comment 8 Tibor Zimanyi 2016-04-01 11:37:26 UTC
Verified by running tests from PR in different environments.


Note You need to log in before you can comment on or make changes to this bug.