You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@isis.apache.org by "ASF subversion and git services (JIRA)" <ji...@apache.org> on 2014/02/21 11:48:19 UTC

[jira] [Commented] (ISIS-712) Inconsistency in domain logic for validation of optional strings causes Wicket viewer to trip up.

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

ASF subversion and git services commented on ISIS-712:
------------------------------------------------------

Commit a248c61a617dc9cca6c4cf944f823cab5d49c1fe in isis's branch refs/heads/master from [~danhaywood]
[ https://git-wip-us.apache.org/repos/asf?p=isis.git;h=a248c61 ]

ISIS-712: validation handling in wicket viewer and metamodel

NB, the PropertyValidateFacet now respects the presence of @Optional
(ie MandatoryFacet.isInvertedSemantics()) and skips validation if the proposed arg is null


> Inconsistency in domain logic for validation of optional strings causes Wicket viewer to trip up.
> -------------------------------------------------------------------------------------------------
>
>                 Key: ISIS-712
>                 URL: https://issues.apache.org/jira/browse/ISIS-712
>             Project: Isis
>          Issue Type: Bug
>          Components: Core, Viewer: Wicket
>    Affects Versions: viewer-wicket-1.3.1, core-1.3.0
>            Reporter: Dan Haywood
>            Assignee: Dan Haywood
>            Priority: Critical
>             Fix For: viewer-wicket-1.4.0, core-1.4.0
>
>
> First, to explain the issue.  We have the following code:
>     private String turnoverRentRule;
>     @javax.jdo.annotations.Column(allowsNull = "true", length = 254)
>     @Optional
>     public String getTurnoverRentRule() { ... }
>     public void setTurnoverRentRule(final String turnoverRentRule) { ... }
>     public String validateTurnoverRentRule(final String rule) {
>         if (rule == null || rule.trim().length() == 0) {
>             return "Rule cannot be empty";
>         }
>         return rule.equals("INVALID")? "invalid rule!": null;
>     }
> This code is logically inconsistent: the validateXxx() method suggests that the turnoverRentRule is required, yet the @Column(allowsNull="true") (and also the @Optional, though that is redundant) suggest the opposite.
> In the Wicket viewer, what I'm seeing when the field is left empty, is that Wicket's form#validate phase skips over and does not call the validate.  This is because the validator that is installed by StringPanel's TextField implements IValidator but not INullAcceptingValidator, and Wicket does not check nulls on simple IValidators.
> I think the above is correct and shouldn't change.
> However, in the Wicket viewer, I then also go on in the form#submit phase to run the validation again, this time using Isis' infrastructure (see the stack trace below).  This (redundantly) validates each property again, but (not redundantly) also performs any object-level checking, as per the any validate() method on the object. 
> The above is also ok.
> Here's where the issues are:
> 1. with the above faulty domain object code, what we see is that the validateXxx() method does get called (by Isis) in the submit phase, even though it wasn't called (by Wicket) in the validate phase.  This results in the "Rule cannot be empty" message being returned.  However, the handling in EntityPropertiesForm#okButton#onSubmit that then detects this change accidentally switches the form to view mode.  It should stay in edit mode.
> 2. the Isis PropertyValidateFacet ought to honour the @Optional semantic; if the proposed value is null, and the facet holder is not mandatory, then the validation should be skipped.
> This ticket is to fix both of these issues.
> Thread [1934620956@qtp-1127736517-7] (Suspended (breakpoint at line 194 in ToDoItem))	
> 	ToDoItem.validateTurnoverRentRule() line: 194	
> 	GeneratedMethodAccessor127.invoke(Object, Object[]) line: not available	
> 	DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 43	
> 	Method.invoke(Object, Object...) line: 606	
> 	uV.invoke(Method, Object, Object[]) line: 1078	
> 	Method.invoke(Object, Object...) line: 592	
> 	MethodExtensions.invoke(Method, Object, Object[]) line: 50	
> 	AdapterInvokeUtils.invoke(Method, ObjectAdapter, Object) line: 48	
> 	AdapterInvokeUtils.invoke(Method, ObjectAdapter, ObjectAdapter) line: 52	
> 	PropertyValidateFacetViaMethod.invalidReason(ObjectAdapter, ObjectAdapter) line: 61	
> 	PropertyValidateFacetViaMethod(PropertyValidateFacetAbstract).invalidates(ValidityContext<ValidityEvent>) line: 55	
> 	InteractionUtils.isValidResult(FacetHolder, ValidityContext<?>) line: 69	
> 	OneToOneAssociationImpl.isAssociationValidResult(ObjectAdapter, ObjectAdapter) line: 110	
> 	OneToOneAssociationImpl.isAssociationValid(ObjectAdapter, ObjectAdapter) line: 105	
> 	ObjectValidPropertiesFacetImpl.invalidReason(ObjectValidityContext) line: 58	
> 	ObjectValidPropertiesFacetImpl(ObjectValidPropertiesFacetAbstract).invalidates(ValidityContext<ValidityEvent>) line: 45	
> 	InteractionUtils.isValidResult(FacetHolder, ValidityContext<?>) line: 69	
> 	ObjectSpecificationDefault(ObjectSpecificationAbstract).isValidResult(ObjectAdapter) line: 1050	
> 	ObjectSpecificationDefault(ObjectSpecificationAbstract).isValid(ObjectAdapter) line: 1040	
> 	EntityModel.getReasonInvalidIfAny() line: 572	
> 	EntityPropertiesForm$2.onSubmit() line: 377	
> 	EntityPropertiesForm(Form).delegateSubmit(IFormSubmitter) line: 1253	
> 	EntityPropertiesForm(Form).process(IFormSubmitter) line: 925	
> 	EntityPropertiesForm(FormAbstract).process(IFormSubmitter) line: 118	
> 	EntityPropertiesForm(Form).onFormSubmitted(IFormSubmitter) line: 771	
> 	EntityPropertiesForm(Form).onFormSubmitted() line: 704	
> 	GeneratedMethodAccessor163.invoke(Object, Object[]) line: not available	
> 	DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 43	
> 	Method.invoke(Object, Object...) line: 606	
> 	RequestListenerInterface.internalInvoke(Component, Object) line: 258	
> 	RequestListenerInterface.invoke(IRequestableComponent) line: 216	
> 	ListenerInterfaceRequestHandler.invokeListener() line: 240	
> 	ListenerInterfaceRequestHandler.respond(IRequestCycle) line: 226	
> 	RequestCycle$HandlerExecutor.respond(IRequestHandler) line: 861	
> 	RequestCycle$HandlerExecutor(RequestHandlerStack).execute(IRequestHandler) line: 64	
> 	RequestCycle.execute(IRequestHandler) line: 261	
> 	RequestCycle.processRequest() line: 218	
> 	RequestCycle.processRequestAndDetach() line: 289	
> 	WicketFilter.processRequestCycle(RequestCycle, WebResponse, HttpServletRequest, HttpServletResponse, FilterChain) line: 259	
> 	WicketFilter.processRequest(ServletRequest, ServletResponse, FilterChain) line: 201	
> 	WicketFilter.doFilter(ServletRequest, ServletResponse, FilterChain) line: 282	
> 	ServletHandler$CachedChain.doFilter(ServletRequest, ServletResponse) line: 1212	
> 	ShiroFilter(AbstractShiroFilter).executeChain(ServletRequest, ServletResponse, FilterChain) line: 449	
> 	AbstractShiroFilter$1.call() line: 365	
> 	SubjectCallable.doCall(Callable<V>) line: 90	
> 	SubjectCallable.call() line: 83	
> 	WebDelegatingSubject(DelegatingSubject).execute(Callable<V>) line: 383	
> 	ShiroFilter(AbstractShiroFilter).doFilterInternal(ServletRequest, ServletResponse, FilterChain) line: 362	
> 	ShiroFilter(OncePerRequestFilter).doFilter(ServletRequest, ServletResponse, FilterChain) line: 125	
> 	ServletHandler$CachedChain.doFilter(ServletRequest, ServletResponse) line: 1212	
> 	ServletHandler.handle(String, HttpServletRequest, HttpServletResponse, int) line: 399	
> 	SecurityHandler.handle(String, HttpServletRequest, HttpServletResponse, int) line: 216	
> 	SessionHandler.handle(String, HttpServletRequest, HttpServletResponse, int) line: 182	
> 	WebAppContext(ContextHandler).__handle(String, HttpServletRequest, HttpServletResponse, int) line: 766	
> 	WebAppContext(ContextHandler).handle(String, HttpServletRequest, HttpServletResponse, int) line: not available	
> 	WebAppContext.handle(String, HttpServletRequest, HttpServletResponse, int) line: 450	
> 	Server(HandlerWrapper).handle(String, HttpServletRequest, HttpServletResponse, int) line: 152	
> 	Server.handle(HttpConnection) line: 326	
> 	HttpConnection.handleRequest() line: 542	
> 	HttpConnection$RequestHandler.content(Buffer) line: 945	
> 	HttpParser.parseNext() line: 756	
> 	HttpParser.parseAvailable() line: 212	
> 	HttpConnection.handle() line: 404	
> 	SocketConnector$Connection.run() line: 228	
> 	QueuedThreadPool$PoolThread.run() line: 582	



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)