You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Nacho Gonzalez <na...@arnet-es.com> on 2004/03/19 22:10:36 UTC

validator & inheritance

Here is the aproach I was thinking of. I beleive there should only be 
changes in no more than 2 classes and 1 file:

validator.dtd:    <!ATTLIST form    name         CDATA #REQUIRED
                                                      extends      CDATA 
#IMPLIED >

org.apache.commons.validator.Form (or Bean if it gets changed) should 
reflect the change in the dtd and a pointer to the Form it extends (this 
could probably be set int the formSet.process(Map) while iterating the 
forms of the formSet)

The methods getFields() and getFieldMap() should include 
super.getFields() and super.getFieldMap() making them Unmodifiable after 
merging.

The only restriction would be that the extended form should belong to 
the formSet but this definetely makes sense!

If this makes sense to you, then I am willing to do it (as well as any 
other necessary changes). If it doesn't make any sense, then I am 
willing to contribute as I beleive this could be a great improvement.

Nacho G. Mac Dowell

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: validator & inheritance

Posted by David Graham <gr...@yahoo.com>.
Please attach cvs diff -u formatted patches to a bugzilla ticket so they
don't get lost in the mailing list.

Thanks,
David

--- Nacho Gonzalez <na...@arnet-es.com> wrote:
> Here are the diff files for validator_1_0.dtd, 
> org.apache.commons.validator.FormSet and 
> org.apache.commons.validator.Form. I have also done the tests. Should I 
> include them as plain text or attach them? The test would be a 
> validation.xml file and the TestCase.
> 
> I didn't follow exactly the Tiles pattern as it would imply quite a few 
> more changes. I believe solving inheritance is naturally part of the 
> process method anyway.
> I don't know if it would be interesting to reference the parent form. 
> For this purpose I didn't find it useful, but it might be useful in the 
> future.
> It would probably be wise not to use process(Map, Map) at all in case it
> 
> may get deprecated (just guessing).
> 
> A couple of issues:
> 
> 1. Should it throw an exception:
>        
>         throw new ValidatorException("Error while resolving form 
> definition inheritance: child '"
>                     + getName()
>                     + "' can't find its ancestor '"
>                     + getExtends()
>                     + "'. Please check your validation definition
> file.");
>                    
>     if its parent form is not found? The thing is that it should go all 
> the way up to ValidatorResources.initialize...
>    
> 2. It would probably be interesting to have a notation similar to the 
> one in the attribte depends (comma delimited), making it possible to 
> extend from multiple forms. What do you think?
> 
> ______________________
> DIFF validator_1_0.dtd
> 82,84d81
> <      The "extends" attribute makes it possible to extend the set of 
> rules of this
> <      form with those of the "extends" one. It is also possible to 
> override any
> <      of the parent's rules.
> 87,88c84
> < <!ATTLIST form    name         CDATA #REQUIRED
> <                                 extends       CDATA #IMPLIED >
> ---
>  > <!ATTLIST form    name         CDATA #REQUIRED>
> 
> 
> 
> _________________________________________
> DIFF org.apache.commons.validator.FormSet
> 221c221,225
> <                 f.process(globalConstants, hConstants);
> ---
>  >                 if (f.isExtending()) {
>  >                       f.process(globalConstants, hConstants, hForms);
>  >                 } else {
>  >                       f.process(globalConstants, hConstants);
>  >                 }
> 
> 
> 
> ______________________________________
> DIFF org.apache.commons.validator.Form
> 89a90,100
>  >       /**
>  >        * The name/key of the form which this form extends from.
>  >       */
>  >       protected String inherit = null;
>  >
>  >       /**
>  >        * Whether or not the this <code>Form</code> was processed
>  >        * for replacing variables in strings with their values.
>  >       */
>  >       private boolean bProcessed = false;
>  >
> 153a165
>  >          bProcessed = true;
> 155a168,203
>  >       /**
>  >        * Processes all of the <code>Form</code>'s
> <code>Field</code>s.
>  >       */
>  >       public void process(Map globalConstants, Map constants, 
> FastHashMap forms) {
>  >
>  >               if (isProcessed()) {
>  >                       return;
>  >               }
>  >               int n = 0; //we want the fields from its parent first
>  >               if (isExtending()) {
>  >                       Form parent = (Form) forms.get(inherit);
>  >                       if (parent != null) {
>  >                               if (!parent.isProcessed()) {
>  >                                       //we want to go all the way up 
> the tree
>  >                                       parent.process(constants, 
> globalConstants, forms);
>  >                               }
>  >                               for (Iterator i = 
> parent.getFields().iterator(); i.hasNext();) {
>  >                                       Field f = (Field) i.next();
>  >                                       //we want to be able to 
> override any fields we like
>  >                                       if (hFields.get(f.getKey()) == 
> null) {
>  >                                               lFields.add(n, f);
>  >                                               hFields.put(f.getKey(),
> f);
>  >                                               n++;
>  >                                       }
>  >                               }
>  >                       }
>  >               }
>  >               hFields.setFast(true);
>  >               //no need to reprocess parent's fields, we iterate from
> 'n'
>  >               for (Iterator i = lFields.listIterator(n);
> i.hasNext();) {
>  >                       Field f = (Field) i.next();
>  >                       f.process(globalConstants, constants);
>  >               }
>  >               bProcessed = true;
>  >       }
>  >
> 173a222,230
>  >
>  >
>  >       /**
>  >        * Whether or not the this <code>Form</code> was processed
>  >        * for replacing variables in strings with their values.
>  >       */
>  >       public boolean isProcessed() {
>  >          return bProcessed;
>  >       }
> 174a232,251
>  >       /**
>  >        * Gets the name/key of the parent set of validation rules.
>  >        */
>  >       public String getExtends() {
>  >               return inherit;
>  >       }
>  >
>  >       /**
>  >        * Sets the name/key of the parent set of validation rules.
>  >        */
>  >       public void setExtends(String string) {
>  >               inherit = string;
>  >       }
>  >
>  >       /**
>  >        * Get extends flag.
>  >        */
>  >       public boolean isExtending() {
>  >         return inherit!=null;
>  >       }
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
> 


__________________________________
Do you Yahoo!?
Yahoo! Finance Tax Center - File online. File on time.
http://taxes.yahoo.com/filing.html

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


validator & inheritance

Posted by Nacho Gonzalez <na...@arnet-es.com>.
Here are the diff files for validator_1_0.dtd, 
org.apache.commons.validator.FormSet and 
org.apache.commons.validator.Form. I have also done the tests. Should I 
include them as plain text or attach them? The test would be a 
validation.xml file and the TestCase.

I didn't follow exactly the Tiles pattern as it would imply quite a few 
more changes. I believe solving inheritance is naturally part of the 
process method anyway.
I don't know if it would be interesting to reference the parent form. 
For this purpose I didn't find it useful, but it might be useful in the 
future.
It would probably be wise not to use process(Map, Map) at all in case it 
may get deprecated (just guessing).

A couple of issues:

1. Should it throw an exception:
       
        throw new ValidatorException("Error while resolving form 
definition inheritance: child '"
                    + getName()
                    + "' can't find its ancestor '"
                    + getExtends()
                    + "'. Please check your validation definition file.");
                   
    if its parent form is not found? The thing is that it should go all 
the way up to ValidatorResources.initialize...
   
2. It would probably be interesting to have a notation similar to the 
one in the attribte depends (comma delimited), making it possible to 
extend from multiple forms. What do you think?

______________________
DIFF validator_1_0.dtd
82,84d81
<      The "extends" attribute makes it possible to extend the set of 
rules of this
<      form with those of the "extends" one. It is also possible to 
override any
<      of the parent's rules.
87,88c84
< <!ATTLIST form    name         CDATA #REQUIRED
<                                 extends       CDATA #IMPLIED >
---
 > <!ATTLIST form    name         CDATA #REQUIRED>



_________________________________________
DIFF org.apache.commons.validator.FormSet
221c221,225
<                 f.process(globalConstants, hConstants);
---
 >                 if (f.isExtending()) {
 >                       f.process(globalConstants, hConstants, hForms);
 >                 } else {
 >                       f.process(globalConstants, hConstants);
 >                 }



______________________________________
DIFF org.apache.commons.validator.Form
89a90,100
 >       /**
 >        * The name/key of the form which this form extends from.
 >       */
 >       protected String inherit = null;
 >
 >       /**
 >        * Whether or not the this <code>Form</code> was processed
 >        * for replacing variables in strings with their values.
 >       */
 >       private boolean bProcessed = false;
 >
153a165
 >          bProcessed = true;
155a168,203
 >       /**
 >        * Processes all of the <code>Form</code>'s <code>Field</code>s.
 >       */
 >       public void process(Map globalConstants, Map constants, 
FastHashMap forms) {
 >
 >               if (isProcessed()) {
 >                       return;
 >               }
 >               int n = 0; //we want the fields from its parent first
 >               if (isExtending()) {
 >                       Form parent = (Form) forms.get(inherit);
 >                       if (parent != null) {
 >                               if (!parent.isProcessed()) {
 >                                       //we want to go all the way up 
the tree
 >                                       parent.process(constants, 
globalConstants, forms);
 >                               }
 >                               for (Iterator i = 
parent.getFields().iterator(); i.hasNext();) {
 >                                       Field f = (Field) i.next();
 >                                       //we want to be able to 
override any fields we like
 >                                       if (hFields.get(f.getKey()) == 
null) {
 >                                               lFields.add(n, f);
 >                                               hFields.put(f.getKey(), f);
 >                                               n++;
 >                                       }
 >                               }
 >                       }
 >               }
 >               hFields.setFast(true);
 >               //no need to reprocess parent's fields, we iterate from 'n'
 >               for (Iterator i = lFields.listIterator(n); i.hasNext();) {
 >                       Field f = (Field) i.next();
 >                       f.process(globalConstants, constants);
 >               }
 >               bProcessed = true;
 >       }
 >
173a222,230
 >
 >
 >       /**
 >        * Whether or not the this <code>Form</code> was processed
 >        * for replacing variables in strings with their values.
 >       */
 >       public boolean isProcessed() {
 >          return bProcessed;
 >       }
174a232,251
 >       /**
 >        * Gets the name/key of the parent set of validation rules.
 >        */
 >       public String getExtends() {
 >               return inherit;
 >       }
 >
 >       /**
 >        * Sets the name/key of the parent set of validation rules.
 >        */
 >       public void setExtends(String string) {
 >               inherit = string;
 >       }
 >
 >       /**
 >        * Get extends flag.
 >        */
 >       public boolean isExtending() {
 >         return inherit!=null;
 >       }


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org