You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@isis.apache.org by "Dan Haywood (JIRA)" <ji...@apache.org> on 2014/02/21 11:10:29 UTC

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

Dan Haywood created ISIS-712:
--------------------------------

             Summary: 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: core-1.3.0, viewer-wicket-1.3.1
            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)