Bug 724772 (BRMS-610)

Link: Added: This issue depends JBRULES-2993

This problem is fixed, but please note:

Due to the need of supporting backward compatible metadata that could be written without "", the current behavior is adopted:

1. The system will try to resolve the annotation value as an MVEL expression. 
1.1. If it succeeds, the result value is returned.
1.2. If it does not succeed, the original content will pass-through as a String.
2. As of 5.2, the old non-quoted syntax for annotation values is deprecated. Needs documentation.
3. As of 5.2, multi-valued annotations for rules are not supported yet, but this is a desirable feature for 5.3

Examples:


    @Test
    public void testRuleAnnotation() {
        String drl = "package org.drools\n" +
                     "rule X\n" +
                     "    @author(\"John Doe\")\n" +
                     "    @output(Hello World!)\n" + // backward compatibility
                     "    @value( 10 + 10 )\n" +
                     "    @alt( \"Hello \"+\"World!\" )\n" +
                     "when\n" +
                     "    Person()\n" +
                     "then\n" +
                     "end";
        KnowledgeBaseConfiguration conf = KnowledgeBaseFactory.newKnowledgeBaseConfiguration();
        conf.setOption( EventProcessingOption.STREAM );
        conf.setOption( MBeansOption.ENABLED );

        KnowledgeBase kbase = loadKnowledgeBase( "kb1",
                                                 drl,
                                                 conf );

        Rule rule = kbase.getRule( "org.drools",
                                   "X" );

        Assert.assertEquals( "John Doe",
                             rule.getMetaData().get( "author" ) );
        Assert.assertEquals( "Hello World!",
                             rule.getMetaData().get( "output" ) );
        Assert.assertEquals( 20,
                             ((Number)rule.getMetaData().get( "value" )).intValue() );
        Assert.assertEquals( "Hello World!",
                             rule.getMetaData().get( "alt" ) );

    }

Applied:

https://github.com/droolsjbpm/drools/commit/625bd3ce8b3b53884137a82ee54129676c85474f

https://github.com/droolsjbpm/drools/commit/007827a5829dc86d874be3119ffe81bd0b07bb2d

Created attachment 520347 [details]
Simple example

Since the new quoted way is preferred the quotes surrounding the valu should no be counted to resulting string.

@alt(" \"<- these are supposed to be the only quotes ->\" ")

results (when printed to console) to
" "<- these are supposed to be the only quotes ->" "

where I'd expect
"<- these are supposed to be the only quotes ->"

Tomas,

This is very tricky because of the backward compatibility issue. The reason the above was not working was because Drools parser was unescaping strings and then MVEL was doing it again during execution of the expression, what was failing and falling back to the legacy mode. So it would require a number of \ for each \... very ugly.

I changed the code again so that the above works as you mentioned, but I am becoming more and more convinced that it is dangerous to support silent backward compatibility as I was trying to. I am wondering if we should add a configuration to explicit work in the 5.1 compatibility mode (i.e., no expression support whatsoever) or in the 5.2+ support (i.e., only expression support, no backward compatibility for invalid expressions).

Anyway, please take a look and check if all your tests pass or please let me know otherwise.

Edson

Committed:

https://github.com/droolsjbpm/drools/commit/8b3d0d656030fb74fc9ed351a87310407ec5f628

This bug was fixed in BRMS-5.2.0.ER4