Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-50355

@QueryParameter in form validation is not filled for dropdownDescriptorSelector when using nonpublic class

      I’m trying to use form validation on a dropdownDescriptorSelector, however here the validation method parameters (annotated with @QueryParameter) are not found and set to null (or 0). (If used standalone the validation works).

      Is there anything different with a dropdownDescriptorSelector? Or is this caused since the jelly views are in different plugins that use different class loaders?

      The relevant main jelly part:

      <f:entry description="${description.tool}">
      	<f:dropdownDescriptorSelector field="tool" title="${%title.tool}"
      		descriptors="${descriptor.availableTools}"/>
      </f:entry>
      

      (whole file: ScanForIssuesStep/config.jelly)

      The relevant jelly part of the referenced Describable:

      <f:entry title="${%High priority threshold}"
              description="${%Minimum number of duplicated lines for high priority warnings.}"
              field="highThreshold">
       <f:textbox default="50"/>
      </f:entry>
      

      (whole file: DuplicateCodeScanner/config.jelly)

      My validation method:

      public FormValidation doCheckHighThreshold(@QueryParameter final int highThreshold,
             @QueryParameter final int normalThreshold) {
         return VALIDATION.validateHigh(highThreshold, normalThreshold);
      }
      

      (whole file: DuplicateCodeScanner.java)

      Both parameters are always set to zero when setting a breakpoint here.

          [JENKINS-50355] @QueryParameter in form validation is not filled for dropdownDescriptorSelector when using nonpublic class

          Ulli Hafner added a comment -

          I finally managed it to get it fixed (at least in my plugin). I think there still I a bug in Stapler here:

          If I define the validation method using:

          public FormValidation doCheckHighThreshold(@QueryParameter final int highThreshold,
                  @QueryParameter final int normalThreshold) {
              return VALIDATION.validateHigh(highThreshold, normalThreshold);
          }
          

          then in the generated HTML the attribute checkdependson is empty:

          <input checkdependson="" default="50" checkurl="/view/White%20Mountains/job/New%20-%20Pipeline%20-%20Model/pipeline-syntax/descriptorByName/cpd/checkHighThreshold" name="_.highThreshold" type="text" class="setting-input validated  " value="50">
          

          If I rather use

          public FormValidation doCheckHighThreshold(@QueryParameter(value = "highThreshold") final int highThreshold,
                  @QueryParameter(value = "normalThreshold") final int normalThreshold) {
              return VALIDATION.validateHigh(highThreshold, normalThreshold);
          }
          

          Then in the generated HTML the attribute checkdependson is correct:

          <input checkdependson="highThreshold normalThreshold" default="50" checkurl="/view/White%20Mountains/job/New%20-%20Pipeline%20-%20Model/pipeline-syntax/descriptorByName/cpd/checkHighThreshold" name="_.highThreshold" type="number" class="setting-input validated " value="50">
          

          From the documentation of @QueryParameter: if value is not given, then the name of the parameter should be used.

          Ulli Hafner added a comment - I finally managed it to get it fixed (at least in my plugin). I think there still I a bug in Stapler here: If I define the validation method using: public FormValidation doCheckHighThreshold(@QueryParameter final int highThreshold, @QueryParameter final int normalThreshold) { return VALIDATION.validateHigh(highThreshold, normalThreshold); } then in the generated HTML the attribute checkdependson is empty: <input checkdependson= "" default =" 50 " checkurl=" /view/White%20Mountains/job/New%20-%20Pipeline%20-%20Model/pipeline-syntax/descriptorByName/cpd/checkHighThreshold " name=" _.highThreshold " type=" text " class=" setting-input validated " value=" 50"> If I rather use public FormValidation doCheckHighThreshold(@QueryParameter(value = "highThreshold" ) final int highThreshold, @QueryParameter(value = "normalThreshold" ) final int normalThreshold) { return VALIDATION.validateHigh(highThreshold, normalThreshold); } Then in the generated HTML the attribute checkdependson is correct: <input checkdependson= "highThreshold normalThreshold" default = "50" checkurl= "/view/White%20Mountains/job/New%20-%20Pipeline%20-%20Model/pipeline-syntax/descriptorByName/cpd/checkHighThreshold" name= "_.highThreshold" type= "number" class= "setting-input validated " value= "50" > From the documentation of @QueryParameter: if value is not given, then the name of the parameter should be used.

          Ulli Hafner added a comment -

          The problem seems to be that Stapler can't read the parameter names, since the descriptor base class is not public. Stapler should report an error or warning in this case.

              abstract static class DryDescriptor extends ReportScanningToolDescriptor {
                  private static final ThresholdValidation VALIDATION = new ThresholdValidation();
          
                  /**
                   * Creates the descriptor instance.
                   *
                   * @param id
                   *         ID of the tool
                   */
                  DryDescriptor(final String id) {
                      super(id);
                  }
          
                  /**
                   * Performs on-the-fly validation on threshold for high warnings.
                   *
                   * @param highThreshold
                   *         the threshold for high warnings
                   * @param normalThreshold
                   *         the threshold for normal warnings
                   *
                   * @return the validation result
                   */
                  public FormValidation doCheckHighThreshold(@QueryParameter(value = "highThreshold") final int highThreshold,
                          @QueryParameter(value = "normalThreshold") final int normalThreshold) {
                      return VALIDATION.validateHigh(highThreshold, normalThreshold);
                  }
          
                  /**
                   * Performs on-the-fly validation on threshold for normal warnings.
                   *
                   * @param highThreshold
                   *         the threshold for high warnings
                   * @param normalThreshold
                   *         the threshold for normal warnings
                   *
                   * @return the validation result
                   */
                  public FormValidation doCheckNormalThreshold(@QueryParameter(value = "highThreshold") final int highThreshold,
                          @QueryParameter(value = "normalThreshold") final int normalThreshold) {
                      return VALIDATION.validateNormal(highThreshold, normalThreshold);
                  }
              }
          
          

          Ulli Hafner added a comment - The problem seems to be that Stapler can't read the parameter names, since the descriptor base class is not public. Stapler should report an error or warning in this case. abstract static class DryDescriptor extends ReportScanningToolDescriptor { private static final ThresholdValidation VALIDATION = new ThresholdValidation(); /** * Creates the descriptor instance. * * @param id * ID of the tool */ DryDescriptor( final String id) { super (id); } /** * Performs on-the-fly validation on threshold for high warnings. * * @param highThreshold * the threshold for high warnings * @param normalThreshold * the threshold for normal warnings * * @ return the validation result */ public FormValidation doCheckHighThreshold(@QueryParameter(value = "highThreshold" ) final int highThreshold, @QueryParameter(value = "normalThreshold" ) final int normalThreshold) { return VALIDATION.validateHigh(highThreshold, normalThreshold); } /** * Performs on-the-fly validation on threshold for normal warnings. * * @param highThreshold * the threshold for high warnings * @param normalThreshold * the threshold for normal warnings * * @ return the validation result */ public FormValidation doCheckNormalThreshold(@QueryParameter(value = "highThreshold" ) final int highThreshold, @QueryParameter(value = "normalThreshold" ) final int normalThreshold) { return VALIDATION.validateNormal(highThreshold, normalThreshold); } }

          Jesse Glick added a comment -

          I suspect the issue is that FormValidation.CheckMethod.<init> calls ReflectionUtils.getPublicMethodNamed and this returns null when the class is not public. Normally a Descriptor must be public (and you will get a compile-time error if it is not) because it would be marked with @Extension. Your case is special because you are using an abstract base class; it should also be public.

          Probably the calls to getPublicMethodNamed here should be issuing a warning if the result is null because the Class is not public. I am hesitant to make this a compile-time warning or error in QueryParameterAnnotationProcessor because I am not sure whether Stapler will permit public web methods to be called when defined on a non-public class; this does not yet seem to be covered in DispatcherTest. If it does not work anyway (i.e., adding the value attribute fixes the HTML but does not make validation actually work), then it would be reasonable to just block this during compilation.

          Jesse Glick added a comment - I suspect the issue is that FormValidation.CheckMethod.<init> calls ReflectionUtils.getPublicMethodNamed and this returns null when the class is not public. Normally a Descriptor must be public (and you will get a compile-time error if it is not) because it would be marked with @Extension . Your case is special because you are using an abstract base class; it should also be public . Probably the calls to getPublicMethodNamed here should be issuing a warning if the result is null because the Class is not public . I am hesitant to make this a compile-time warning or error in QueryParameterAnnotationProcessor because I am not sure whether Stapler will permit public web methods to be called when defined on a non- public class; this does not yet seem to be covered in DispatcherTest . If it does not work anyway (i.e., adding the value attribute fixes the HTML but does not make validation actually work), then it would be reasonable to just block this during compilation.

            Unassigned Unassigned
            drulli Ulli Hafner
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated: