You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@archiva.apache.org by Brett Porter <br...@apache.org> on 2006/09/13 15:52:47 UTC

Re: svn commit: r442925 - in /maven/archiva/trunk/archiva-webapp/src/main: java/org/apache/maven/archiva/web/action/admin/ConfigureAction.java webapp/WEB-INF/jsp/admin/configure.jsp

You now have the validation in there twice - it should be removed  
from execute() shouldn't it?

I think I'd construct the cron expression using a method as needed  
rather than sticking it in a field, too.

What do you think?

On 13/09/2006, at 8:11 PM, oching@apache.org wrote:

> Author: oching
> Date: Wed Sep 13 03:11:04 2006
> New Revision: 442925
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=442925
> Log:
> Revised the validation for cron expression. Also revised the  
> configure.jsp page (for the cron editor) -- removed the <table> tags.
>
> Modified:
>     maven/archiva/trunk/archiva-webapp/src/main/java/org/apache/ 
> maven/archiva/web/action/admin/ConfigureAction.java
>     maven/archiva/trunk/archiva-webapp/src/main/webapp/WEB-INF/jsp/ 
> admin/configure.jsp
>
> Modified: maven/archiva/trunk/archiva-webapp/src/main/java/org/ 
> apache/maven/archiva/web/action/admin/ConfigureAction.java
> URL: http://svn.apache.org/viewvc/maven/archiva/trunk/archiva- 
> webapp/src/main/java/org/apache/maven/archiva/web/action/admin/ 
> ConfigureAction.java?view=diff&rev=442925&r1=442924&r2=442925
> ====================================================================== 
> ========
> --- maven/archiva/trunk/archiva-webapp/src/main/java/org/apache/ 
> maven/archiva/web/action/admin/ConfigureAction.java (original)
> +++ maven/archiva/trunk/archiva-webapp/src/main/java/org/apache/ 
> maven/archiva/web/action/admin/ConfigureAction.java Wed Sep 13  
> 03:11:04 2006
> @@ -18,6 +18,7 @@
>
>  import com.opensymphony.xwork.ModelDriven;
>  import com.opensymphony.xwork.Preparable;
> +import com.opensymphony.xwork.Validateable;
>  import org.apache.maven.archiva.configuration.Configuration;
>  import  
> org.apache.maven.archiva.configuration.ConfigurationChangeException;
>  import org.apache.maven.archiva.configuration.ConfigurationStore;
> @@ -38,7 +39,7 @@
>   */
>  public class ConfigureAction
>      extends PlexusActionSupport
> -    implements ModelDriven, Preparable
> +    implements ModelDriven, Preparable, Validateable
>  {
>      /**
>       * @plexus.requirement
> @@ -66,6 +67,21 @@
>
>      private String year;
>
> +    private String cronEx = "";
> +
> +    public void validate()
> +    {
> +        cronEx = ( second + " " + minute + " " + hour + " " +  
> dayOfMonth + " " + month +
> +                    " " + dayOfWeek + " " + year ).trim();
> +
> +        //validate cron expression
> +        cronValidator = new CronExpressionValidator();
> +
> +        if( !cronValidator.validate( cronEx ) )
> +        {
> +            addActionError( "Invalid Cron Expression" );
> +        }
> +    }
>
>      public String execute()
>          throws IOException, RepositoryIndexException,  
> RepositoryIndexSearchException, ConfigurationStoreException,
>
> Modified: maven/archiva/trunk/archiva-webapp/src/main/webapp/WEB- 
> INF/jsp/admin/configure.jsp
> URL: http://svn.apache.org/viewvc/maven/archiva/trunk/archiva- 
> webapp/src/main/webapp/WEB-INF/jsp/admin/configure.jsp? 
> view=diff&rev=442925&r1=442924&r2=442925
> ====================================================================== 
> ========
> --- maven/archiva/trunk/archiva-webapp/src/main/webapp/WEB-INF/jsp/ 
> admin/configure.jsp (original)
> +++ maven/archiva/trunk/archiva-webapp/src/main/webapp/WEB-INF/jsp/ 
> admin/configure.jsp Wed Sep 13 03:11:04 2006
> @@ -26,95 +26,47 @@
>
>  <h1>Configuration</h1>
>
> +<div class="errors">
> +  <ww:actionerror />
> +</div>
> +
>  <div id="contentArea">
>    <ww:actionmessage/>
>    <ww:form method="post" action="saveConfiguration" namespace="/ 
> admin" validate="true">
> +    <ww:textfield name="indexPath" label="Index Directory"  
> size="100" required="true"/>
> +    <!--ww:textfield name="indexerCronExpression" label="Indexing  
> Schedule"/-->
> +
> +    <ww:label value="Indexing Schedule" labelposition="top"/>
> +      <ww:div>
> +        <ww:textfield name="second" label="Second" size="2"/>
> +        <ww:textfield name="minute" label="Minute"  
> labelposition="left" size="2"/>
> +        <ww:textfield name="hour" label="Hour" size="2"/>
> +        <ww:textfield name="dayOfMonth" label="Day Of Month"  
> size="2"/>
> +        <ww:textfield name="month" label="Month" size="2"/>
> +        <ww:textfield name="dayOfWeek" label="Day Of Week" size="2"/>
> +        <ww:textfield name="year" label="Year" size="4"/>
> +      </ww:div>
> +
> +    <ww:hidden name="proxy.protocol" value="http"/>
> +    <ww:textfield name="proxy.host" label="HTTP Proxy Host"/>
> +    <ww:textfield name="proxy.port" label="HTTP Proxy Port"/>
> +    <ww:textfield name="proxy.username" label="HTTP Proxy Username"/>
> +    <ww:password name="proxy.password" label="HTTP Proxy Password"/>
>
> -    <div>
> -    <table>
> -        <tbody>
> -          <tr>
> -            <th><font size="2"><ww:label theme="simple"  
> value="Indexing Directory*:"/></font></th>
> -            <td><ww:textfield name="indexPath" theme="simple"  
> size="140" required="true"/></td>
> -          </tr>
> -          <tr>
> -            <th><font size="2"><ww:label theme="simple"  
> value="Indexing Schedule:"/></font></th>
> -            <td>
> -              <table>
> -                <tr>
> -                  <th><ww:label theme="simple" value="Second:"/></th>
> -                  <td><ww:textfield name="second" theme="simple"  
> size="2"/></td>
> -
> -                  <th><ww:label theme="simple" value="Minute:"/></th>
> -                  <td><ww:textfield name="minute" theme="simple"  
> size="2"/></td>
> -
> -                  <th><ww:label theme="simple" value="Hour:"/></th>
> -                  <td><ww:textfield name="hour" theme="simple"  
> size="2"/></td>
> -
> -                  <th><ww:label theme="simple" value="Day of  
> Month:"/></th>
> -                  <td><ww:textfield name="dayOfMonth"  
> theme="simple" size="2"/></td>
> -
> -                  <th><ww:label theme="simple" value="Month:"/></th>
> -                  <td><ww:textfield name="month" theme="simple"  
> size="2"/></td>
> -
> -                  <th><ww:label theme="simple" value="Day of  
> Week:"/></th>
> -                  <td><ww:textfield name="dayOfWeek"  
> theme="simple" size="2"/></td>
> -
> -                  <th><ww:label theme="simple" value="Year  
> [optional]:"/></th>
> -                  <td><ww:textfield name="year" theme="simple"  
> size="4"/></td>
> -                </tr>
> -              </table>
> -            </td>
> -          </tr>
> -        <ww:hidden name="proxy.protocol" value="http"/>
> -        <tr>
> -            <th><font size="2"><ww:label theme="simple"  
> value="HTTP Proxy Host:"/></font></th>
> -            <td><ww:textfield name="proxy.host" theme="simple"/></td>
> -        </tr>
> -        <tr>
> -            <th><font size="2"><ww:label theme="simple"  
> value="HTTP Proxy Port:"/></font></th>
> -            <td><ww:textfield name="proxy.port" theme="simple"/></td>
> -        </tr>
> -        <tr>
> -            <th><font size="2"><ww:label theme="simple"  
> value="HTTP Proxy Username:"/></font></th>
> -            <td><ww:textfield name="proxy.username" theme="simple"/ 
> ></td>
> -        </tr>
> -        <tr>
> -            <th><font size="2"><ww:label theme="simple"  
> value="HTTP Proxy Password:"/></font></th>
> -            <td><ww:textfield name="proxy.password" theme="simple"/ 
> ></td>
> -        </tr>
> -        </tbody>
> -    </table>
> -    </div>
> -
> -    <div>
> -      <p><i>For valid cron expression values for the Indexing  
> Schedule, see <ww:a href="http://www.opensymphony.com/quartz/api/ 
> org/quartz/CronExpression.html">here</ww:a></i></p>
> -    </div>
> -    <div>
> -      <table>
> -        <tr>
> -          <b>Indexing Schedule Keys:</b>
> -        </tr>
> -        <tr>
> -          <th>*</th>
> -          <td>every</td>
> -        </tr>
> -        <tr>
> -          <th>?</th>
> -          <td>any</td>
> -        </tr>
> -        <tr>
> -          <th>-</th>
> -          <td>ranges</td>
> -        </tr>
> -        <tr>
> -          <th>/</th>
> -          <td>increments</td>
> -        </tr>
> -      </table>
> -    </div>
>      <ww:submit value="Save Configuration"/>
> +
> +    <ww:div>
> +      <ww:label value="Indexing Schedule Keys:" labelposition="top"/>
> +      <ww:label value="* = every" labelposition="top"/>
> +      <ww:label value="? = any" labelposition="top"/>
> +      <ww:label value="- = ranges" labelposition="top"/>
> +      <ww:label value="/ = increments" labelposition="top"/>
> +    </ww:div>
>    </ww:form>
> +
> +  <ww:div>
> +    <p><i>For valid cron expression values for the Indexing  
> Schedule, see <ww:a href="http://www.opensymphony.com/quartz/api/ 
> org/quartz/CronExpression.html">here</ww:a></i></p>
> +  </ww:div>
>
>    <script type="text/javascript">
>      document.getElementById("saveConfiguration_indexPath").focus();
>

Re: svn commit: r442925 - in /maven/archiva/trunk/archiva-webapp/src/main: java/org/apache/maven/archiva/web/action/admin/ConfigureAction.java webapp/WEB-INF/jsp/admin/configure.jsp

Posted by Maria Odea Ching <oc...@exist.com>.
Yes, I agree. I think its better to have the cron expression constructed 
using a method.

Sorry about the validation, I forgot to remove it from execute().

Brett Porter wrote:

> You now have the validation in there twice - it should be removed  
> from execute() shouldn't it?
>
> I think I'd construct the cron expression using a method as needed  
> rather than sticking it in a field, too.
>
> What do you think?
>
> On 13/09/2006, at 8:11 PM, oching@apache.org wrote:
>
>> Author: oching
>> Date: Wed Sep 13 03:11:04 2006
>> New Revision: 442925
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=442925
>> Log:
>> Revised the validation for cron expression. Also revised the  
>> configure.jsp page (for the cron editor) -- removed the <table> tags.
>>
>> Modified:
>>     maven/archiva/trunk/archiva-webapp/src/main/java/org/apache/ 
>> maven/archiva/web/action/admin/ConfigureAction.java
>>     maven/archiva/trunk/archiva-webapp/src/main/webapp/WEB-INF/jsp/ 
>> admin/configure.jsp
>>
>> Modified: maven/archiva/trunk/archiva-webapp/src/main/java/org/ 
>> apache/maven/archiva/web/action/admin/ConfigureAction.java
>> URL: http://svn.apache.org/viewvc/maven/archiva/trunk/archiva- 
>> webapp/src/main/java/org/apache/maven/archiva/web/action/admin/ 
>> ConfigureAction.java?view=diff&rev=442925&r1=442924&r2=442925
>> ====================================================================== 
>> ========
>> --- maven/archiva/trunk/archiva-webapp/src/main/java/org/apache/ 
>> maven/archiva/web/action/admin/ConfigureAction.java (original)
>> +++ maven/archiva/trunk/archiva-webapp/src/main/java/org/apache/ 
>> maven/archiva/web/action/admin/ConfigureAction.java Wed Sep 13  
>> 03:11:04 2006
>> @@ -18,6 +18,7 @@
>>
>>  import com.opensymphony.xwork.ModelDriven;
>>  import com.opensymphony.xwork.Preparable;
>> +import com.opensymphony.xwork.Validateable;
>>  import org.apache.maven.archiva.configuration.Configuration;
>>  import  
>> org.apache.maven.archiva.configuration.ConfigurationChangeException;
>>  import org.apache.maven.archiva.configuration.ConfigurationStore;
>> @@ -38,7 +39,7 @@
>>   */
>>  public class ConfigureAction
>>      extends PlexusActionSupport
>> -    implements ModelDriven, Preparable
>> +    implements ModelDriven, Preparable, Validateable
>>  {
>>      /**
>>       * @plexus.requirement
>> @@ -66,6 +67,21 @@
>>
>>      private String year;
>>
>> +    private String cronEx = "";
>> +
>> +    public void validate()
>> +    {
>> +        cronEx = ( second + " " + minute + " " + hour + " " +  
>> dayOfMonth + " " + month +
>> +                    " " + dayOfWeek + " " + year ).trim();
>> +
>> +        //validate cron expression
>> +        cronValidator = new CronExpressionValidator();
>> +
>> +        if( !cronValidator.validate( cronEx ) )
>> +        {
>> +            addActionError( "Invalid Cron Expression" );
>> +        }
>> +    }
>>
>>      public String execute()
>>          throws IOException, RepositoryIndexException,  
>> RepositoryIndexSearchException, ConfigurationStoreException,
>>
>> Modified: maven/archiva/trunk/archiva-webapp/src/main/webapp/WEB- 
>> INF/jsp/admin/configure.jsp
>> URL: http://svn.apache.org/viewvc/maven/archiva/trunk/archiva- 
>> webapp/src/main/webapp/WEB-INF/jsp/admin/configure.jsp? 
>> view=diff&rev=442925&r1=442924&r2=442925
>> ====================================================================== 
>> ========
>> --- maven/archiva/trunk/archiva-webapp/src/main/webapp/WEB-INF/jsp/ 
>> admin/configure.jsp (original)
>> +++ maven/archiva/trunk/archiva-webapp/src/main/webapp/WEB-INF/jsp/ 
>> admin/configure.jsp Wed Sep 13 03:11:04 2006
>> @@ -26,95 +26,47 @@
>>
>>  <h1>Configuration</h1>
>>
>> +<div class="errors">
>> +  <ww:actionerror />
>> +</div>
>> +
>>  <div id="contentArea">
>>    <ww:actionmessage/>
>>    <ww:form method="post" action="saveConfiguration" namespace="/ 
>> admin" validate="true">
>> +    <ww:textfield name="indexPath" label="Index Directory"  
>> size="100" required="true"/>
>> +    <!--ww:textfield name="indexerCronExpression" label="Indexing  
>> Schedule"/-->
>> +
>> +    <ww:label value="Indexing Schedule" labelposition="top"/>
>> +      <ww:div>
>> +        <ww:textfield name="second" label="Second" size="2"/>
>> +        <ww:textfield name="minute" label="Minute"  
>> labelposition="left" size="2"/>
>> +        <ww:textfield name="hour" label="Hour" size="2"/>
>> +        <ww:textfield name="dayOfMonth" label="Day Of Month"  
>> size="2"/>
>> +        <ww:textfield name="month" label="Month" size="2"/>
>> +        <ww:textfield name="dayOfWeek" label="Day Of Week" size="2"/>
>> +        <ww:textfield name="year" label="Year" size="4"/>
>> +      </ww:div>
>> +
>> +    <ww:hidden name="proxy.protocol" value="http"/>
>> +    <ww:textfield name="proxy.host" label="HTTP Proxy Host"/>
>> +    <ww:textfield name="proxy.port" label="HTTP Proxy Port"/>
>> +    <ww:textfield name="proxy.username" label="HTTP Proxy Username"/>
>> +    <ww:password name="proxy.password" label="HTTP Proxy Password"/>
>>
>> -    <div>
>> -    <table>
>> -        <tbody>
>> -          <tr>
>> -            <th><font size="2"><ww:label theme="simple"  
>> value="Indexing Directory*:"/></font></th>
>> -            <td><ww:textfield name="indexPath" theme="simple"  
>> size="140" required="true"/></td>
>> -          </tr>
>> -          <tr>
>> -            <th><font size="2"><ww:label theme="simple"  
>> value="Indexing Schedule:"/></font></th>
>> -            <td>
>> -              <table>
>> -                <tr>
>> -                  <th><ww:label theme="simple" value="Second:"/></th>
>> -                  <td><ww:textfield name="second" theme="simple"  
>> size="2"/></td>
>> -
>> -                  <th><ww:label theme="simple" value="Minute:"/></th>
>> -                  <td><ww:textfield name="minute" theme="simple"  
>> size="2"/></td>
>> -
>> -                  <th><ww:label theme="simple" value="Hour:"/></th>
>> -                  <td><ww:textfield name="hour" theme="simple"  
>> size="2"/></td>
>> -
>> -                  <th><ww:label theme="simple" value="Day of  
>> Month:"/></th>
>> -                  <td><ww:textfield name="dayOfMonth"  
>> theme="simple" size="2"/></td>
>> -
>> -                  <th><ww:label theme="simple" value="Month:"/></th>
>> -                  <td><ww:textfield name="month" theme="simple"  
>> size="2"/></td>
>> -
>> -                  <th><ww:label theme="simple" value="Day of  
>> Week:"/></th>
>> -                  <td><ww:textfield name="dayOfWeek"  theme="simple" 
>> size="2"/></td>
>> -
>> -                  <th><ww:label theme="simple" value="Year  
>> [optional]:"/></th>
>> -                  <td><ww:textfield name="year" theme="simple"  
>> size="4"/></td>
>> -                </tr>
>> -              </table>
>> -            </td>
>> -          </tr>
>> -        <ww:hidden name="proxy.protocol" value="http"/>
>> -        <tr>
>> -            <th><font size="2"><ww:label theme="simple"  value="HTTP 
>> Proxy Host:"/></font></th>
>> -            <td><ww:textfield name="proxy.host" theme="simple"/></td>
>> -        </tr>
>> -        <tr>
>> -            <th><font size="2"><ww:label theme="simple"  value="HTTP 
>> Proxy Port:"/></font></th>
>> -            <td><ww:textfield name="proxy.port" theme="simple"/></td>
>> -        </tr>
>> -        <tr>
>> -            <th><font size="2"><ww:label theme="simple"  value="HTTP 
>> Proxy Username:"/></font></th>
>> -            <td><ww:textfield name="proxy.username" theme="simple"/ 
>> ></td>
>> -        </tr>
>> -        <tr>
>> -            <th><font size="2"><ww:label theme="simple"  value="HTTP 
>> Proxy Password:"/></font></th>
>> -            <td><ww:textfield name="proxy.password" theme="simple"/ 
>> ></td>
>> -        </tr>
>> -        </tbody>
>> -    </table>
>> -    </div>
>> -
>> -    <div>
>> -      <p><i>For valid cron expression values for the Indexing  
>> Schedule, see <ww:a href="http://www.opensymphony.com/quartz/api/ 
>> org/quartz/CronExpression.html">here</ww:a></i></p>
>> -    </div>
>> -    <div>
>> -      <table>
>> -        <tr>
>> -          <b>Indexing Schedule Keys:</b>
>> -        </tr>
>> -        <tr>
>> -          <th>*</th>
>> -          <td>every</td>
>> -        </tr>
>> -        <tr>
>> -          <th>?</th>
>> -          <td>any</td>
>> -        </tr>
>> -        <tr>
>> -          <th>-</th>
>> -          <td>ranges</td>
>> -        </tr>
>> -        <tr>
>> -          <th>/</th>
>> -          <td>increments</td>
>> -        </tr>
>> -      </table>
>> -    </div>
>>      <ww:submit value="Save Configuration"/>
>> +
>> +    <ww:div>
>> +      <ww:label value="Indexing Schedule Keys:" labelposition="top"/>
>> +      <ww:label value="* = every" labelposition="top"/>
>> +      <ww:label value="? = any" labelposition="top"/>
>> +      <ww:label value="- = ranges" labelposition="top"/>
>> +      <ww:label value="/ = increments" labelposition="top"/>
>> +    </ww:div>
>>    </ww:form>
>> +
>> +  <ww:div>
>> +    <p><i>For valid cron expression values for the Indexing  
>> Schedule, see <ww:a href="http://www.opensymphony.com/quartz/api/ 
>> org/quartz/CronExpression.html">here</ww:a></i></p>
>> +  </ww:div>
>>
>>    <script type="text/javascript">
>>      document.getElementById("saveConfiguration_indexPath").focus();
>>
>