[
https://issues.apache.org/jira/browse/OOZIE-580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13134467#comment-13134467
]
[email protected] commented on OOZIE-580:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2427/#review2803
-----------------------------------------------------------
The code needs documentation. There is a lot of code where the developer is
expected to spend time figuring out the purpose of the methods and the logic
inside the methods.
/trunk/core/src/main/java/org/apache/oozie/DagELFunctions.java
<https://reviews.apache.org/r/2427/#comment6267>
Interesting. Your comment indicates that it should not happen. It could
happen and the code will try to alleviate this problem by escaping special
characters. The escaping may or may not solve the problem since the
JDOMException could be for a range of issues.
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java
<https://reviews.apache.org/r/2427/#comment6268>
What is the intention of this test case? Could you please add a top level
comment for the test case and comments in the test case to describe the logic
and the criteria being evaluated?
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java
<https://reviews.apache.org/r/2427/#comment6271>
What is the purpose of this utility method? Is it something that can be
used more generally, i.e., across test cases
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java
<https://reviews.apache.org/r/2427/#comment6269>
If this is a real host name, then it should be anonymized.
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java
<https://reviews.apache.org/r/2427/#comment6270>
The host name should be anonymized if its a real host name
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java
<https://reviews.apache.org/r/2427/#comment6272>
Will this line of code be executed? I was not sure about the semantics of
fail.
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java
<https://reviews.apache.org/r/2427/#comment6274>
Can you please document the purpose of this method?
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java
<https://reviews.apache.org/r/2427/#comment6273>
Will this line of code be executed? I was not sure about the semantics of
fail.
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java
<https://reviews.apache.org/r/2427/#comment6275>
Will this line of code be executed? I was not sure about the semantics of
fail.
- Santhosh
On 2011-10-24 20:02:05, Angelo K. Huang wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2427/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-24 20:02:05)
bq.
bq.
bq. Review request for oozie.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. try {
bq. String valueElem = "<value>"+value+"</value>";
bq. XmlUtils.parseXml(valueElem);
bq. }
bq. catch (JDOMException ex) {
bq. // It should not happen, so escape the characters for xml
bq. value = XmlUtils.escapeCharsForXML(value);
bq. }
bq.
bq. With above check, for element CDATA can be avoided for escaping, EX:
bq.
bq. For these two elements, first one has to convert because of '&', however
second one can be avoided.
bq.
bq. <property>
bq. <name>test.ampsign</name>
bq.
<value>http://app1.soln-stage.nova.cp.vip.ne1.yahoo.com/nova-webservices?urlSigner=signUrl&namespace=nova.proxy</value>
bq. </property>
bq. <property>
bq. <name>test.cdata</name>
bq.
<value><![CDATA[?redirect=http%3A%2F%2Fapp1.soln-stage.nova.cp.vip.ne1.yahoo.com%3A4080%2Fnova-webservices%2Fv1%2FurlSigner%2FsignUrl&namespace=nova.proxy&keyDBHash=Vsy6n_C7K6NG0z4R2eBlKg--]]></value>
bq. </property>
bq.
bq.
bq. *** & has to convert to & ***
bq. *** <![CDATA[]] does not need to convert. ***
bq.
bq. <property>
bq. <name>test.ampsign</name>
bq.
<value>http://app1.soln-stage.nova.cp.vip.ne1.yahoo.com/nova-webservices?urlSigner=signUrl&namespace=nova.proxy</value>
bq. </property>
bq. <property>
bq. <name>test.cdata</name>
bq.
<value><![CDATA[?redirect=http%3A%2F%2Fapp1.soln-stage.nova.cp.vip.ne1.yahoo.com%3A4080%2Fnova-webservices%2Fv1%2FurlSigner%2FsignUrl&namespace=nova.proxy&keyDBHash=Vsy6n_C7K6NG0z4R2eBlKg--]]></value>
bq. </property>
bq.
bq.
bq. This addresses bug OOZIE-580.
bq. https://issues.apache.org/jira/browse/OOZIE-580
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. /trunk/core/src/main/java/org/apache/oozie/DagELFunctions.java 1185461
bq.
/trunk/core/src/main/java/org/apache/oozie/command/wf/ActionStartXCommand.java
1185461
bq.
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java
1185461
bq. /trunk/release-log.txt 1185461
bq.
bq. Diff: https://reviews.apache.org/r/2427/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Angelo K.
bq.
bq.
> use xml element to handle string escape when configure evaluator
> ----------------------------------------------------------------
>
> Key: OOZIE-580
> URL: https://issues.apache.org/jira/browse/OOZIE-580
> Project: Oozie
> Issue Type: Improvement
> Reporter: Angelo K. Huang
> Assignee: Angelo K. Huang
>
> Instead of using string value to do escape, xml element is able to do it and
> also avoid escaping legit character at XML element, such as <![CDATA[]]>.
> public static void configureEvaluator(ELEvaluator evaluator, WorkflowJobBean
> workflow, WorkflowActionBean action) {
> evaluator.setVariable(WORKFLOW, workflow);
> evaluator.setVariable(ACTION, action);
> for (Map.Entry<String, String> entry :
> workflow.getWorkflowInstance().getConf()) {
> if (ParamChecker.isValidIdentifier(entry.getKey())) {
> String value = entry.getValue().trim();
> // escape the characters for xml
> value = XmlUtils.escapeCharsForXML(value);
> evaluator.setVariable(entry.getKey().trim(), value);
> }
> }
> try {
> evaluator.setVariable(ACTION_PROTO_CONF,
> new XConfiguration(new
> StringReader(workflow.getProtoActionConf())));
> }
> catch (IOException ex) {
> throw new RuntimeException("It should not happen", ex);
> }
> }
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
