I have some local changes for the standalone checker... a new FormBeanAnnotationProcessorFactory that hooks up the FormBeanCoreAnnotationProcessor and added an entry in the META-INF/services/com.sun.mirror.apt.AnnotationProcessorFactory for the factory. Initial tests seem to be OK. I also have my change that will check an external form bean from within FlowControllerChecker, only when there is *no* @FormBean.
I have not added a warning message to users that they should always have a @FormBean annotation when using any of the @ValidatableProperty annotations in a form bean. I was concerned that there is already a precedent for just using the @FormBean to define a message bundle. See the NetUI tutorial and the note about using @FormBean... http://beehive.apache.org/docs/1.0.1/netui/tutorial.html#validation Does anyone feel strongly that we should change the behavior and always expect @FormBean annotation when using any of the @ValidatableProperty annotations? If so, I will add the warning and change the tutorial. Kind regards, Carlin On 8/15/06, Rich Feit <[EMAIL PROTECTED]> wrote:
That makes sense to me -- basically we'd be saying that we require a top-level annotation if you want to use method- or field-level annotations. You can also just go with your original suggestion, but I think that in that case you could end up with multiple errors for a single class. Rich Carlin Rogers wrote: > Rich, > > I'll take a look at this more closely and make sure I follow as I'm > not yet > sure how that impacts the use of the @ValidatableBean annotation > (which is > used with the validatableBeans attribute of the Controller annot). I > guess > either @ValidatableBean or @FormBean would be required for > @ValidatableProperty? > > Thanks, > Carlin > > On 8/15/06, Rich Feit <[EMAIL PROTECTED]> wrote: >> >> Carlin, >> >> Makes sense -- I'd forgotten that we don't require the annotation. >> Seems like we're being too lax in this case -- I actually think that it >> would be best to require the @FormBean annotation when using any of the >> @ValidatableProperty annotations. We don't want to require @FormBean >> just to use an external bean, but I don't see the harm in requiring it >> when any of the other annotations are used inside the bean. That way, >> we can preserve the idea of a standalone checker. >> >> For back-compat, you could run a checker on any external form bean from >> within PageFlowChecker (as you suggested), but only when there is *no* >> @FormBean. If you encounter any @ValidatableProperty annotations in >> that case, you could deprecation-warn that @FormBean should be used. >> >> What do you think? >> >> Rich >> >> Carlin Rogers wrote: >> > Rich, >> > >> > I have a question about the suggestion to create and use the FBAPF. It >> > seems >> > that this would work only in the case that the standalone includes >> the @ >> > Jpf.FormBean annotation. However, users can define validation rules >> > without >> > @Jpf.FormBean and just use @Jpf.ValidatableProperty. The FormBean >> > allows the >> > class to define its own message bundle for validation errors, but it's >> > just >> > an option. >> > >> > We need to make sure that we check the ValidatableProperty before >> > going to >> > generate() to provide AP error information. The ValidatableProperty >> > can be >> > on a method (of a bean) and an annotation type (listed in the @ >> > Jpf.Controller or @Jpf.Action). Since it's not just associated to a >> type, >> > would we still be able to do something like you suggested and create >> > an AP >> > factory for ValidatableProperty? Maybe I'm missing something though >> with >> > what you were suggesting. >> > >> > Thanks, >> > Carlin >> > >> > On 8/14/06, Rich Feit <[EMAIL PROTECTED]> wrote: >> >> >> >> Hey Carlin, >> >> >> >> I think what might be better would be to make a >> >> FormBeanAnnotationProcessorFactory in >> >> compiler-apt/org/apache/beehive/netui/compiler/apt, and make it >> >> responsible for the form bean annotation (you'd need to remove that >> >> annotation from the list of supported annotations in >> >> PageFlowAnnotationProcessorFactory). FBAPF could then simply >> return a >> >> FormBeanCoreAnnotationProcessorFactory -- which already exists -- in >> its >> >> getCoreProcessorFor(). I honestly cannot remember why I didn't do >> that >> >> -- sorry. >> >> >> >> If it turns out that this doesn't work for some reason, you could >> also >> >> roll the functionality from FormBeanCoreAnnotationProcessor into >> >> PageFlowCoreAnnotationProcessor (and to delete the former). >> >> >> >> In either case, you end up with standalone checking form bean >> classes, >> >> which is nicer because you don't have to worry about multiple >> >> PageFlowCheckers triggering checking on the same external form bean >> >> class, and it's just cleaner to have it be standalone. >> >> >> >> All that said, I didn't wire it up in the first place, so if you run >> >> into any difficulties let me know -- it's possible there's something >> I'm >> >> missing here. >> >> >> >> Rich >> >> >> >> Carlin Rogers wrote: >> >> > I'm taking a look at BEEHIVE-1127 and wanted to share some thoughts >> >> about >> >> > ensuring the annotation processing of validatable bean properties >> >> > declared >> >> > in an external form bean class during the check() phase. >> >> > >> >> > The current implementation for processing of the Controller >> >> > annotations does >> >> > not process external form bean during the check() phase of the >> >> > TwoPhaseCoreAnnotationProcessor. During the Controller annotation >> >> > processing, FlowControllerChecker.onCheck() creates the >> >> > FormBeanChecker and >> >> > then during the onCheckInternal() uses it to check >> ValidatableProperty >> >> > annotations on inner classes only. >> >> > >> >> > I'd like to modify FlowControllerChecker.checkMethod() (for action >> >> > grammar >> >> > checking) to also look at the parameter of an action and check for >> >> > validation rules. The change would only use the FormBeanChecker on >> the >> >> > parameter class if it is a ClassDeclaration without a declaring >> type >> >> > (so we >> >> > don't re-check an inner class). Does this sound right? Anyone have >> >> some >> >> > additional thoughts? >> >> > >> >> > Rich, do you have some input on this AP change? >> >> > >> >> > Thanks, >> >> > Carlin >> >> > >> >> >> > >> >
