[
https://issues.apache.org/jira/browse/OOZIE-580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13134643#comment-13134643
]
[email protected] commented on OOZIE-580:
-----------------------------------------------------
bq. On 2011-10-24 20:47:37, Santhosh Srinivasan wrote:
bq. >
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java,
line 369
bq. > <https://reviews.apache.org/r/2427/diff/3/?file=52892#file52892line369>
bq. >
bq. > What is the purpose of this utility method? Is it something that can
be used more generally, i.e., across test cases
There is addRecordToWfJobTable() at XDataTestcase. This one is mainly for this
test case.
bq. On 2011-10-24 20:47:37, Santhosh Srinivasan wrote:
bq. >
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java,
line 379
bq. > <https://reviews.apache.org/r/2427/diff/3/?file=52892#file52892line379>
bq. >
bq. > If this is a real host name, then it should be anonymized.
fixed it.
bq. On 2011-10-24 20:47:37, Santhosh Srinivasan wrote:
bq. >
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java,
line 380
bq. > <https://reviews.apache.org/r/2427/diff/3/?file=52892#file52892line380>
bq. >
bq. > The host name should be anonymized if its a real host name
fixed it.
bq. On 2011-10-24 20:47:37, Santhosh Srinivasan wrote:
bq. >
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java,
line 393
bq. > <https://reviews.apache.org/r/2427/diff/3/?file=52892#file52892line393>
bq. >
bq. > Will this line of code be executed? I was not sure about the
semantics of fail.
fixed it.
bq. On 2011-10-24 20:47:37, Santhosh Srinivasan wrote:
bq. >
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java,
line 411
bq. > <https://reviews.apache.org/r/2427/diff/3/?file=52892#file52892line411>
bq. >
bq. > Will this line of code be executed? I was not sure about the
semantics of fail.
fixed it.
bq. On 2011-10-24 20:47:37, Santhosh Srinivasan wrote:
bq. >
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java,
line 416
bq. > <https://reviews.apache.org/r/2427/diff/3/?file=52892#file52892line416>
bq. >
bq. > Will this line of code be executed? I was not sure about the
semantics of fail.
I think u mean javadoc. added it.
bq. On 2011-10-24 20:47:37, Santhosh Srinivasan wrote:
bq. >
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java,
line 398
bq. > <https://reviews.apache.org/r/2427/diff/3/?file=52892#file52892line398>
bq. >
bq. > Can you please document the purpose of this method?
added it.
bq. On 2011-10-24 20:47:37, Santhosh Srinivasan wrote:
bq. >
/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java,
line 177
bq. > <https://reviews.apache.org/r/2427/diff/3/?file=52892#file52892line177>
bq. >
bq. > 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?
added it.
bq. On 2011-10-24 20:47:37, Santhosh Srinivasan wrote:
bq. > /trunk/core/src/main/java/org/apache/oozie/DagELFunctions.java, line 66
bq. > <https://reviews.apache.org/r/2427/diff/3/?file=52890#file52890line66>
bq. >
bq. > 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.
fixed it.
- Angelo K.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2427/#review2803
-----------------------------------------------------------
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
