[jboss-jira] [JBoss JIRA] (DROOLS-1541) newInsert.outIdentifier/getValue.identifier don't refer via fact handle (as per doc.), get only original, not replacement, fact object instances

Tue May 9 08:23:00 EDT 2017

    [ https://issues.jboss.org/browse/DROOLS-1541?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13403549#comment-13403549 ] 

Matteo Mortari edited comment on DROOLS-1541 at 5/9/17 8:22 AM:
----------------------------------------------------------------

I don't believe there is an error in the documentation, neither there is an error in the code.

I believe the real problem here is mixing two api approaches, using the Commands Vs calling operations directly on the KieSession. Therefore, if one had to use only the Commands API, none of the mentioned issue would manifest.

Anyway, given the Commands API does not support the case of calling "update" and replacing the actual object instance, there could be an edge-case of mixing the two approaches.

But even in the case of mixing Commands API and calling operations directly on the KieSession, it is (still) *incorrect* to reuse a ExecutionResults after subsequent operations have been performed directly on the KieSession.

This is because ExecutionResults represents a "snapshot" of the results of the execution of the Commands, contextually to the KieSession.execute(Command c).
Once the execute returns a given ExecutionResults instance, the KieSession does NOT keep a reference to it, and that given instance will not be updated following subsequent KieSession API call. If the user want an updated ExecutionResults, it will require a new execute of Commands, which will return a consistent (new, up-to-date) ExecutionResults instance.

I want to stress the fact the ExecutionResults represent the result contextually to the Commands API call.

As an example see below, although I would NOT recommend it as it mixes Commands API and calling operations directly on the KieSession.

{code:java}
        // Step1
        ExecutionResults resultsA = session.execute(
            CommandFactory.newBatchExecution( Arrays.asList(new Command[]{
                CommandFactory.newInsert( new Measurement("color", "red"), "my_out_ID" ),
                CommandFactory.newFireAllRules()
            }))
        );
        FactHandle redFH = (FactHandle) resultsA.getFactHandle("my_out_ID");

        // Step2
        Object replacedInstance = new Measurement("color", "reddish");
        session.update(redFH, replacedInstance );
        session.fireAllRules();

        // Step3
        ExecutionResults resultsB = session.execute(
            CommandFactory.newBatchExecution( Arrays.asList(new Command[]{
                CommandFactory.newGetObject(redFH, "my_out_ID")
            }))
        );

        assertEquals( replacedInstance, resultsB.getValue("my_out_ID") );
{code}

At step2, it would be INCORRECT to re-use the ExecutionResults coming from step1. The more correct approach in this case would be to retrieve an up-to-date ExecutionResults instance by calling a new execute Commands, i.e.: step3, notice the final assert statement

I hope this clarifies the misunderstanding.

If I incorrectly interpreted your original description, kindly let us know, please (eventually re-opening this jira)

Thank you,
MM

was (Author: tari_manga):
I don't believe there is an error in the documentation, neither there is an error in the code.

I believe the real problem here is mixing two api approaches, using the Commands Vs calling operations directly on the KieSession. Therefore, if one had to use only the Commands API, none of the mentioned issue would manifest.

Anyway, given the Commands API does not support the case of calling "update" and replacing the actual object instance, there could be an edge-case of mixing the two approaches.

But even in the case of mixing Commands API and calling operations directly on the KieSession, it is (still) *incorrect* to reuse a ExecutionResults after subsequent operations have been performed directly on the KieSession.

This is because ExecutionResults represents a "snapshot" of the results of the execution of the Commands, contextually to the KieSession.execute(Command c).
Once the execute returns a given ExecutionResults instance, the KieSession does NOT keep a reference to it, and that given instance will not be updated following subsequent KieSession API call. If the user want an updated ExecutionResults, it will require a new execute of Commands, which will return a consistent (new, up-to-date) ExecutionResults instance.

I want to stress the fact the ExecutionResults represent the result contextually to the Commands API call.

As an example see below, although I would NOT recommend it as it mixes Commands API and calling operations directly on the KieSession.

{code:java}
        // Step1
        ExecutionResults resultsA = session.execute(
            CommandFactory.newBatchExecution( Arrays.asList(new Command[]{
                CommandFactory.newInsert( new Measurement("color", "red"), "my_out_ID" ),
                CommandFactory.newFireAllRules()
            }))
        );
        FactHandle redFH = (FactHandle) resultsA.getFactHandle("my_out_ID");

        // Step2
        Object replacedInstance = new Measurement("color", "reddish");
        session.update(redFH, replacedInstance );
        session.fireAllRules();

        // Step3
        ExecutionResults resultsB = session.execute(
            CommandFactory.newBatchExecution( Arrays.asList(new Command[]{
                CommandFactory.newGetObject(redFH, "my_out_ID")
            }))
        );

        assertEquals( replacedInstance, resultsB.getValue("my_out_ID") );
{code}

At step2, it would be INCORRECT to re-use the ExecutionResults coming from step1. The more correct approach in this case would be to retrieve an up-to-date ExecutionResults instance by calling a new execute Commands, i.e.: step3.

I hope this clarifies the misunderstanding.

If I incorrectly interpreted your original description, kindly let us know, please (eventually re-opening this jira)

Thank you,
MM

> newInsert.outIdentifier/getValue.identifier don't refer via fact handle (as per doc.), get only original, not replacement, fact object instances
> ------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: DROOLS-1541
>                 URL: https://issues.jboss.org/browse/DROOLS-1541
>             Project: Drools
>          Issue Type: Bug
>          Components: core engine, kie server
>    Affects Versions: 6.3.0.Final, 6.5.0.Final
>            Reporter: Daniel B.
>            Assignee: Mario Fusco
>
> The identifier passed to the {{KieCommands.newInsert}} method's {{outIdentifier}} parameter and to the {{ExecutionResults.getValue}} method's {{identifier}} parameter doesn't actually refer to the fact object _via the fact handle_ as described in the documentation of {{InsertObjectCommand}}).
> The documentation says:
> {quote}
> 11.2.2. InsertObjectCommand
> ...
> outIdentifier	Id to identify the FactHandle created in the object insertion and added to the execution results
> {quote}
> Although the Drools code in method {{InsertObjectCommand.execute}} does map the given identifier both to the original object and to the fact handle, the code in method {{ExecutionResultImpl.getValue}} retrieves the _original_ object instead of retrieving the object _currently associated with the fact handle_.
> This means that if the original fact object instance is replaced with a different instance (e.g., with {{update(kcontext.getKieRuntime().getFactHandle($oldObj), newObj);}} in the rules), then {{ExecutionResults.getValue}} will return the _original_ fact object, not the _current_ value of the fact object (the object instance currently associated with the fact handle created in the {{newInsert}} call). 
> That in turn means that immutable fact object instances cannot be used with {{ExecutionResults.getValue}}.
> (It's not 100% clear that it's the code that is wrong (relative to the documentation) rather than it being documentation that's wrong (relative to the code).  However, the behavior described by the documentation seems more useful than the behavior exhibited by the code.)

--
This message was sent by Atlassian JIRA
(v7.2.3#72005)