You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by "Robert Kanter (JIRA)" <ji...@apache.org> on 2013/07/22 20:22:50 UTC

[jira] [Commented] (OOZIE-1425) param checker should validate cron syntax

    [ https://issues.apache.org/jira/browse/OOZIE-1425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13715487#comment-13715487 ] 

Robert Kanter commented on OOZIE-1425:
--------------------------------------

*pom*
- The quartz version is defined in the {{<dependencyManagement>}} section in the root pom, so you can remove the version from the {{<dependency>}} section of the core pom

*ParamChecker*
- Importing {{UnsupportedOperationException}} isn't needed
- The line containing the message thrown in the exception in {{checkFrequency}} is longer than 132 characters
- There's a number of unused variables in {{checkFrequency}}.  You can get rid of {{ret}} and simply do the {{Integer.toString(Integer.parseInt(val))}} without the return value.  You also don't need to keep {{expr1}} and {{expr2}} and can simply do the {{new CronExpression(...)}} calls.
- It would be good to perform some additional verification.  For example, you could keep {{ret}} and check that {{ret}} and {{val}} are equal and throw an exception if they are not.  I'm assuming that creating a {{CronExpression}} should verify that the individual values in the cron syntax are valid, but before getting there, you need to check the arrays; for example, when you do a {{split(" ")}}, you shouldn't assume that {{cronArray[4]}} exists; you could get an {{ArrayOutOfBounds}} exception, which you're not catching.  
- When you catch the {{ParseException}}, you're throwing a new exception, but giving it the {{NumberFormatException}}, {{ex}} from above; shouldn't it be the {{ParseException}}, {{pex}}?

It would also be good to add some unit tests to make sure that the {{checkFrequency}} method properly handles anything the user could throw at it (both good and bad).  
                
> param checker should validate cron syntax
> -----------------------------------------
>
>                 Key: OOZIE-1425
>                 URL: https://issues.apache.org/jira/browse/OOZIE-1425
>             Project: Oozie
>          Issue Type: Sub-task
>            Reporter: Bowen Zhang
>            Assignee: Bowen Zhang
>         Attachments: oozie-1425.patch
>
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira