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)
