Uploaded image for project: 'Isis'
  1. Isis
  2. ISIS-712

Inconsistency in domain logic for validation of optional strings causes Wicket viewer to trip up.

    Details

      Description

      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

        Attachments

          Activity

            People

            • Assignee:
              danhaywood Dan Haywood
              Reporter:
              danhaywood Dan Haywood
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: