Bugzilla – Full Text Bug Listing
| Summary: | [Scripting usability] Expressions in report design should use a new syntax to differentiate them from static strings | ||
|---|---|---|---|
| Product: | z_Archived | Reporter: | David Michonneau <dmichonneau> |
| Component: | BIRT | Assignee: | Xiaoxiao Wu <xwu> |
| Status: | VERIFIED FIXED | QA Contact: | Maggie Shen <lshen> |
| Severity: | normal | ||
| Priority: | P3 | CC: | bluesoldier, jasonweathersby, jouyang, lzhu, qwang, rkanguri, rlu, scottr, wenfeng.fwd, whe, wyan, xxue, yli, zqian |
| Version: | 2.2.0 | Keywords: | plan |
| Target Milestone: | 2.5.0 RC3 | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | Autoed,G | ||
| Bug Depends on: | 260309, 269923, 275273 | ||
| Bug Blocks: | 264273 | ||
|
Description
David Michonneau
+1 for this enhancement See related discussions at Bug 190422. The current proposal is that we should have a script-type property associated with expressions. It will start with Constant and Javascript types. It will also be desirable for us to introduce extension points to support external script types and calculation engines. Suggest to support constant, JavaScript, and structure expressions in 2.5.0. Structure expression would be evaulated by BIRT's data engine, it might be less flexible but could have much higher performance for simple expression such as +, -, *, /, for calculated field or sum for aggregation, etc.. (In reply to comment #3) > Suggest to support constant, JavaScript, and structure expressions in 2.5.0. > Structure expression would be evaulated by BIRT's data engine, it might be less > flexible but could have much higher performance for simple expression such as > +, -, *, /, for calculated field or sum for aggregation, etc.. > For the old design file, the design file parser cannot distinguish JavaScript and structure expression. So, the parser with the backward compatibility cannot set the correct type. Or, we should set all expressions to JavaScript type? (In reply to comment #4) > (In reply to comment #3) > > Suggest to support constant, JavaScript, and structure expressions in 2.5.0. > > Structure expression would be evaulated by BIRT's data engine, it might be less > > flexible but could have much higher performance for simple expression such as > > +, -, *, /, for calculated field or sum for aggregation, etc.. > > > For the old design file, the design file parser cannot distinguish JavaScript > and structure expression. So, the parser with the backward compatibility cannot > set the correct type. Or, we should set all expressions to JavaScript type? Can we use the design version to maintain backward comp? that is, all design files that are in older version, the expressions are JS. For newer version, can we use either 1) an attribute to indicate the exp type for the property value. or 2) prefix for the property value to indicate the exp type, for example, no '=' prefix or =constant:<value> as a constant value =js:<expression> as a JavaScript expression =<expression> or =birt:<expression> as a birt structure expression Approach (1) is clean but could be too verbose in the XML design file. (2) could be a pratical solution if the parser performance is not significantly degraded due to prefix processing for each property value. prefer for the 1st solution as the 2nd solution means: 1. The expression is not a valid javascript expression. The user must learn an extended syntax for BIRT. 2. it is hard for user to maintain the program as backward comptatability. Although the API setProperty(String expr), String getProperty() are not changed, the parameter value and return value is changed. (In reply to comment #6) > prefer for the 1st solution as the 2nd solution means: > 1. The expression is not a valid javascript expression. The user must learn an > extended syntax for BIRT. > 2. it is hard for user to maintain the program as backward comptatability. > Although the API setProperty(String expr), String getProperty() are not > changed, the parameter value and return value is changed. With solution (2), at API level, I think getValue should return the constant or expression without the prefix, and has a separate call to return the value type. The choices between (1) and (2) would be inside ROM how it persists the values. (In reply to comment #7) > (In reply to comment #6) > > With solution (2), at API level, I think getValue should return the constant or > expression without the prefix, and has a separate call to return the value > type. The choices between (1) and (2) would be inside ROM how it persists the > values. > From Model view, both solutions are OK to implement. They will not have much performance degradation here. The solution 1 adds another property to parser and solution 2 need to parse the string in the memory. We needs to provide getExpressionType/setExpressionType methods. However, if the user creates the design on Model APIs without calling new methods, the preview result may be error. So, in default, the expression type is javascript. This case applies to the old API user or the user changes the XML manually. (In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #6) > > > > With solution (2), at API level, I think getValue should return the constant or > > expression without the prefix, and has a separate call to return the value > > type. The choices between (1) and (2) would be inside ROM how it persists the > > values. > > > From Model view, both solutions are OK to implement. They will not have much > performance degradation here. The solution 1 adds another property to parser > and solution 2 need to parse the string in the memory. > We needs to provide getExpressionType/setExpressionType methods. However, if > the user creates the design on Model APIs without calling new methods, the > preview result may be error. So, in default, the expression type is javascript. > This case applies to the old API user or the user changes the XML manually. Is it correct that some of the properties only take constant values currently? In that case, the default of a property type could either be constant or javascript expression depends on the ROM metadata? Do we want to make it consitent in 2.5.0 that all defaults are constants? or we want to maintain backward compatibility to have the detaulf to follow current ROM metadata? Please check how much larger and verbose it becomes if all property values have to include a type. Maybe we only need to specify a type if the value is not a constant? The conversion from the old design to the new one: 1. ROM expression ==> ROM expression, with the type is JavaScript 2. ROM string ==> ROM expression, with the type is Constant Currently, ROM only has case 1. So, in default, the expression type is JavaScript for case 1. The "abc" is treated as JavaScript. The case 2 is for some enhancements like ScalarParameter.defaultValue ( it is string type now). Only expressions need the type, right? This change won't affect integer, string type ,etc. (In reply to comment #10) > The conversion from the old design to the new one: > 1. ROM expression ==> ROM expression, with the type is JavaScript > 2. ROM string ==> ROM expression, with the type is Constant > Currently, ROM only has case 1. So, in default, the expression type is > JavaScript for case 1. The "abc" is treated as JavaScript. > The case 2 is for some enhancements like ScalarParameter.defaultValue ( it is > string type now). > Only expressions need the type, right? This change won't affect integer, string > type ,etc. Does it make sense to allow all properties of ROM can be evaluated from an expression? I think a ROM property value shall have two types: one is the value type which can be integer, string, date, datetime, etc. another tpye is expression type, which can be constant, js expression, birt expression, etc.. The value type is the data type of the value, while the expression type is about how the value is/should be calculated. I think it is not clean to have "expression" and "integer", "string" in the same type system domain. They are of different aspect of a property value. By separating them into different attributes of the property value, we can allow type checking of an expression's returning value type and the defined property value type. The question is shall we make the default expression type of all properties to be constant to make the new API easier to use? or some defaults being constant and some other defaults being js expression for backward compatibility of the API? Need model support this feature first. Need more time to do this change. I am having a bit of difficulty understanding the scope of this API change. In the PDF it references that only getAllTocs, and getAllBookmarks as changing. My sense is that the changes will be more far reaching than that.
The way that I read this is that anywhere someone is doing a
String expr = (String)romObj.getPropert("ExprProp");
is going to have to change.
Is this correct?
Can you provide a bit more detail as to the scope of the change and how this will impact developers that have written DEAPI code?
(In reply to comment #14) > I am having a bit of difficulty understanding the scope of this API change. In > the PDF it references that only getAllTocs, and getAllBookmarks as changing. > My sense is that the changes will be more far reaching than that. > > The way that I read this is that anywhere someone is doing a > > String expr = (String)romObj.getPropert("ExprProp"); > > is going to have to change. > > Is this correct? > > Can you provide a bit more detail as to the scope of the change and how this > will impact developers that have written DEAPI code? > Yes, String expr = (String)romObj.getPropert("ExprProp") must be changed. Otherwise, it will have ClassCastException. The suggestion is to use String expr = romObj.getStringPropert("ExprProp"). This has been talked about on page 3 of the PDF. Maybe I should highlight this one... I don't think we can make this change. We have too many people that will be upset if they have to change all of their code.
Would it not be better to add a function:
romObj.getExpressionProperty("exprProperty");
If you want to use the 'new' function you use that, but it allows the legacy code to continue to work.
sr
(In reply to comment #16) > I don't think we can make this change. We have too many people that will be > upset if they have to change all of their code. > > Would it not be better to add a function: > > romObj.getExpressionProperty("exprProperty"); > > If you want to use the 'new' function you use that, but it allows the legacy > code to continue to work. > > sr > OK. We won't break the return value. element.getExpressionProperty/setExpressionProperty should be used instead. ROM part has been finished. Each expression has the value and the type. Reassign to UI team. Fixed the designer part, now reassign it to dte part. DTE should replace the old exprssion button with the new expression button in the dataset editor and dataset parameter dialog. As it is closed to M7, we will fix problem in RC1. 1. In the Property Binding page within the Data Set Editor, each property is binding to an ExpressionButton which has both "Constant" input and "criptExpression" building functions. The former expression builder buttons are discarded here. 2. In the data set parameter's New/Edit dialog within the Data Set Editor, the expression builder button working for the default value's input is replaced with an ExpressionButton. DtE part has been fixed. Same should be done in property binding page of data source editor. Reopen the bug. Suggest to fix the problem in RC3. For the data source property binding page, all the expression builder buttons are replaced with new-style constant/expression button. And to make the expression and the constant value work correctly, the adapter logic from model to DtE has also been modified. So mark this bug as FIXED. verified on build 2.5.0 v20090602-0630 and BRDpro090529. constant is supported for the followings: Image Builder,Flash builder,HTML button builder(no fixed),data set editor,data source editor -property binding,report parameter - default value,data set parameter - default value, hyperlink builder - URI location. |