[jboss-jira] [JBoss JIRA] (DROOLS-579) Wrong doc for RuleRuntime.getQueryResults - throwing RuntimeException instead of empty results

[Matteo Mortari (JIRA)]

     [ https://issues.jboss.org/browse/DROOLS-579?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

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/org/kie/api/runtime/rule/RuleRuntime.html#getQueryResults(java.lang.String, 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-javadoc/org/kie/api/runtime/rule/RuleRuntime.html#getQueryResults(java.lang.String, 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/org/kie/api/runtime/rule/RuleRuntime.html#getQueryResults(java.lang.String, 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. 



--
This message was sent by Atlassian JIRA
(v6.3.1#6329)