]
Matteo Mortari updated DROOLS-579:
----------------------------------
Attachment: 20140826.drools6test-missingquery.zip
attaching reproducer / code example as per JIRA record description.
Wrong doc for RuleRuntime.getQueryResults - throwing RuntimeException
instead of empty results
----------------------------------------------------------------------------------------------
Key: DROOLS-579
URL:
https://issues.jboss.org/browse/DROOLS-579
Project: Drools
Issue Type: Bug
Security Level: Public(Everyone can see)
Affects Versions: 6.1.0.Final
Reporter: Matteo Mortari
Assignee: Mark Proctor
Attachments: 20140826.drools6test-missingquery.zip
The
[
javadoc|http://docs.jboss.org/drools/release/6.1.0.Final/kie-api-javadoc/...,
java.lang.Object...)] for public api {{RuleRuntime.getQueryResults}} mentions: _If the
query does not exist or no results match the query it is empty_ ; however if query does
not exist, actually RuntimeException is thrown instead. Details follows. I'm reporting
this as I assume this will become even more relevant for the execution server, ref 2nd
test below.
h5. Proposals
My proposals are that either:
* javadoc shall be at least corrected - I know there is no obligation to document
unchecked exceptions, but [here the doc is explicitly saying something
different|http://docs.jboss.org/drools/release/6.1.0.Final/kie-api-javado...,
java.lang.Object...)] . I will attach pull request accordingly.
* the public api method shall reflect the doc and {{QueryResult}} returned should
resemble Java 8's Optional then - if I understood some good api design styles to
mitigate this scenario from Mario, instead of throwing the RuntimeException, the
{{QueryResult}} returned could stand in place of Java 8's Optional. Without dependency
on Java 8 API, the {{QueryResult}} in case 'query does not exist' could be empty,
to guarantee retro-compatibility, and have specific methods to distinguish root cause of
being empty between query-not-found or no-results-match-criteria.
... or possibly ack the former option for the time being, and welcome the latter option
as a suggestion for enhancement on the new releases, please? Thanks
h5. Details
If you consider the following code
{code}
@Test
public void missingQueryViaApi() {
String drl =
"query \"my query A\"\n" +
" $r : String()\n" +
"end";
KieHelper helper = new KieHelper();
helper.addContent( drl, ResourceType.DRL );
KieSession ksession = helper.build().newKieSession();
// If ... no results match the query it is empty
QueryResults validQueryNameResults = ksession.getQueryResults("my query A");
assertNotNull("The query exists so result should NOT be null",
validQueryNameResults);
assertEquals("The query exists and result should be empty", 0,
validQueryNameResults.size());
// If the query does not exist ... it is empty
boolean hasThrownRTE = false;
try {
ksession.getQueryResults("an non-existent query");
} catch (RuntimeException e) {
hasThrownRTE = true;
}
assertFalse("Should not throw RTE, should be sort-of 'empty' by doc",
hasThrownRTE);
}
{code}
The test fails on the last assert, as accordingly to
[
javadoc|http://docs.jboss.org/drools/release/6.1.0.Final/kie-api-javadoc/...,
java.lang.Object...)], but instead it actually throws the RuntimeException.
This becomes even more of an issue if you consider invoking via Commands:
{code}
@Test
public void missingQueryViaCommands() {
String drl =
"query \"my query A\"\n" +
" $r : String()\n" +
"end";
String someText = "This is some text";
KieHelper helper = new KieHelper();
helper.addContent( drl, ResourceType.DRL );
KieSession ksession = helper.build().newKieSession();
List<Command<?>> cmds = new ArrayList<Command<?>>();
cmds.add( CommandFactory.newInsert( someText ));
cmds.add( CommandFactory.newQuery( "results A", "my query A" ));
cmds.add( CommandFactory.newQuery( "results NE", "an non-existent
query" ));
ExecutionResults results = ksession.execute( CommandFactory.newBatchExecution( cmds )
);
QueryResults queryResults = ( QueryResults ) results.getValue( "results A" );
assertEquals(someText, queryResults.iterator().next().get("$r") );
}
{code}
The test fails on the {{ksession.execute()}} and that's why I wanted to report this I
spotted, because I'm afraid might have more deep implications for the drools execution
server.