[CLOSED] datatables-spring3 criteria resolver does not work with spring 3.1+ if using RequestMappingHandlerMethodAdapter

[tduchateau]

Issue by gorder from Sunday Jul 21, 2013 at 03:20 GMT

---

This would work...
see:
https://jira.springsource.org/browse/SPR-8234

/**

• A replacement for {@code DatatablesCriteriasResolver} which is not compatible with Spring 3.1+

• {@code RequestMappingHandlerMethodAdapter}
  */
  public class DatatablesCriteriasMethodArgumentResolver implements HandlerMethodArgumentResolver
  {

  @OverRide
  public boolean supportsParameter(MethodParameter parameter)
  {
  DatatablesParams parameterAnnotation = parameter
  .getParameterAnnotation(DatatablesParams.class);
  return null != parameterAnnotation;
  }

  @OverRide
  public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer,
  NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception
  {
  HttpServletRequest request = (HttpServletRequest) webRequest.getNativeRequest();
  return DatatablesCriterias.getFromRequest(request);
  }

}

[tduchateau]

Comment by tduchateau from Sunday Jul 21, 2013 at 20:23 GMT

Hi,

I think I missed something.

I'm using Spring 3.2.0.RELEASE in the latest dandelion-datatables' sample and the current DatatablesCriteriasResolver works well.

What did I miss? :-)

[tduchateau]

Comment by gorder from Sunday Jul 21, 2013 at 21:07 GMT

Hello tduchateau,

Great work on this so far by the way! I have been climbing the learning curve the last couple days but I like it so far :)

Basically what happened is in 3.1.x they moved away from the AnnotationMethodHandlerAdapter (which takes the WebArgumentResolver you have) and now use the RequestMappingHandler adapter which requires what I posted above. As a matter of fact in Spring 3.2 the former is deprecated. See the API here:
http://static.springsource.org/spring/docs/3.2.x/javadoc-api/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.html
http://static.springsource.org/spring/docs/3.2.0.BUILD-SNAPSHOT/api/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.html

As for why it works in your config it might be because you specifically use the 3.1 xsd (vs 3.2 xsd)
http://www.springframework.org/schema/mvc/spring-mvc-3.1.xsd

I can't be sure without looking but since you are setting an incompatible method argument resolver the Spring XML parser may be smart enough to revert back to using the older (deprecated) AnnotationMethodHandler adapter when enabling annotation driven MVC. The problem with configuring Spring via XML vs the new Java Config is it does not call out deprecated code use and it is not as clear exactly what is being done :)

Read this part of the docs for more info
http://static.springsource.org/spring/docs/3.2.x/spring-framework-reference/html/mvc.html#mvc-ann-requestmapping-31-vs-30

If you wanted to support versions of Spring pre 3.1 it does not hurt to include both.

Hope this helps explain. Please let me know if you need more info.

[tduchateau]

Comment by tduchateau from Sunday Jul 21, 2013 at 21:22 GMT

Thanks for the heads-up! :-)

No version is declared in XSD, so Spring should pick up the highest available from the classpath, i.e. 3.2.0.RELEASE. :-/

Anyway, I clearly need to make some tests but since there exists a workaround, I'll do that in the next minor version (v0.10.0).
The workaround is to use the plain old HttpServletRequest object in the @controller and use the DatatablesCriterias.getFromRequest(request) utility method.

For example:

@RequestMapping(value = "/persons", method = RequestMethod.GET)
public @ResponseBody DatatablesResponse<Person> findAllForDataTables(HttpServletRequest request) {
    DatatablesCriterias criterias = DatatablesCriterias.getFromRequest(request);
    DataSet<Person> persons = personService.findPersonsWithDatatablesCriterias(criterias);
    return DatatablesResponse.build(persons, criterias);
}

Thanks again @gorder!

[tduchateau]

Comment by tduchateau from Sunday Jul 21, 2013 at 21:32 GMT

By the way, if you have any suggestion/feedback regarding the docs, feel free to share before I start updating it for the upcoming release ;-)

[tduchateau]

Comment by gorder from Sunday Jul 21, 2013 at 22:33 GMT

Ok I looked into this some more and the secret sauce is in the AnnotationDrivenBeanDefinitionParser. This class is responsible for parsing the mvc: namespace stuff in the Spring configuration XML. What happens is they have an adapter class which converts what you have to what I posted above. What this means is that if you are using XML config what you have works. Here is what that code looks like:

private ManagedList<BeanDefinitionHolder> wrapWebArgumentResolverBeanDefs(List<BeanDefinitionHolder> beanDefs) {
    ManagedList<BeanDefinitionHolder> result = new ManagedList<BeanDefinitionHolder>();

    for (BeanDefinitionHolder beanDef : beanDefs) {
        String className = beanDef.getBeanDefinition().getBeanClassName();
        Class<?> clazz = ClassUtils.resolveClassName(className, ClassUtils.getDefaultClassLoader());

        if (WebArgumentResolver.class.isAssignableFrom(clazz)) {
            RootBeanDefinition adapter = new RootBeanDefinition(ServletWebArgumentResolverAdapter.class);
            adapter.getConstructorArgumentValues().addIndexedArgumentValue(0, beanDef);
            result.add(new BeanDefinitionHolder(adapter, beanDef.getBeanName() + "Adapter"));
        } else {
            result.add(beanDef);
        }
    }

    return result;
}

The problem is I use Java Config and it of course is not as forgiving, and unless you know about the existence of the ServletWebArgumentResolverAdapter you hit a dead end which is why I wrote the class in my first post. Here is what the equivalent java config looks like

@configuration
@EnableWebMvc
public class MyWebConfig extends WebMvcConfigurerAdapter
{
@OverRide
public void addArgumentResolvers(List argumentResolvers)
{
argumentResolvers.add(new DatatablesCriteriasMethodArgumentResolver());
}
}

Here you can clearly see that the argument resolver must implement the HandlerMethodArgumentResolver. In my code I use this last approach with the HandlerMethodArgumentResolver defined as I have it in my first post. Of course I could have used the workaround you mentioned as well, but passing in HttpServletRequest is more work to mock for test cases :)

I hope this will help for your documentation purposes. I would personally think it a good idea to use the more modern HandlerMethodArgumentResolver. Typically once Spring deprecates something they remove it entirely after a release or 2. Like I said before you could leave the current resolver in there for support of older Spring versions (pre 3.1) however I would clearly document that on the class and online doc.

[tduchateau]

Comment by gorder from Sunday Jul 21, 2013 at 22:38 GMT

Oops looks like I lost some formatting here it is again.

@Configuration
@EnableWebMvc
public class MyWebConfig extends WebMvcConfigurerAdapter
{
    @Override
    public void addArgumentResolvers(List<HandlerMethodArgumentResolver> argumentResolvers)
    {
       argumentResolvers.add(new DatatablesCriteriasMethodArgumentResolver());
    }
}

[tduchateau]

Comment by gorder from Sunday Jul 21, 2013 at 23:43 GMT

Also note depending on this adapter is not without other risks from the java doc for http://static.springsource.org/spring/docs/3.1.x/javadoc-api/org/springframework/web/method/annotation/AbstractWebArgumentResolverAdapter.html

Note: This class is provided for backwards compatibility. However it is recommended to re-write a WebArgumentResolver as HandlerMethodArgumentResolver. Since supportsParameter(org.springframework.core.MethodParameter) can only be implemented by actually resolving the value and then checking the result is not WebArgumentResolver#UNRESOLVED any exceptions raised must be absorbed and ignored since it's not clear whether the adapter doesn't support the parameter or whether it failed for an internal reason. The HandlerMethodArgumentResolver contract also provides access to model attributes and to WebDataBinderFactory (for type conversion).

[tduchateau]

Comment by gorder from Monday Jul 22, 2013 at 00:14 GMT

I guess one last suggestion, I would probably harden what I did in the first post just a little, to make sure the parameter type is also what you expect. Something like below: I will probably put this change in the code I am using (above) until you release your next version.

public class DatatablesCriteriasMethodArgumentResolver implements HandlerMethodArgumentResolver
{

    @Override
    public boolean supportsParameter(MethodParameter parameter)
    {
        DatatablesParams parameterAnnotation = parameter
            .getParameterAnnotation(DatatablesParams.class);
        if (parameterAnnotation != null) {
            if (DatatablesCriterias.class.isAssignableFrom(parameter.getParameterType())) {
                return true;
            }
        }
        return false;
    }

    @Override
    public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer,
        NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception
    {
        HttpServletRequest request = (HttpServletRequest) webRequest.getNativeRequest();
        return DatatablesCriterias.getFromRequest(request);

    }

}

[tduchateau]

Comment by gorder from Monday Jul 22, 2013 at 11:49 GMT

Here is a stab at the java doc for it (Your website is pretty good already but I expect it would say much the same). It might not hurt to put example usage on the java doc for the DatatablesParams annotation either. I would have this show that the object type being annotated should be a DatatablesParams object. It would also reference the java doc below for 'how to enable', and probably show the alternative way (getting it off the HttpServletRequest)

The only doc issue I have run into this far is around the dom element for the datatable. I am still determining if its just tymeleaf/datatables noobness on my part or maybe the doc can be improved slightly for that. If I have time maybe I will open a separate ticket for that. Hope that helps :)

 * Resolves a method argument of type {@code DatatablesCriterias} annotated with
 * {@code DatatablesParams}.
 * 
 * <p>
 * Datatable attributes are obtained from the {@code HttpServletRequest}, and mapped to the
 * annotated {@code DatatablesCriterias} object.
 * 
 * This HandlerMethodArgumentResolver can be enabled in Spring 3.1 and greater using either Java or
 * XML configuration as shown below:
 * 
 * <pre>
 * &lt;mvc:annotation-driven&gt;
 *    &lt;mvc:argument-resolvers&gt;
 *       &lt;bean class="com.github.dandelion.datatables.extras.spring3.ajax.DatatablesCriteriasResolver" /&gt;
 *    &lt;/mvc:argument-resolvers&gt;
 * &lt;/mvc:annotation-driven&gt;
 * </pre>
 * 
 * <pre>
 * &#064;Configuration
 * &#064;EnableWebMvc
 * public class MyWebConfig extends WebMvcConfigurerAdapter
 * {
 *     &#064;Override
 *     public void addArgumentResolvers(List&lt;HandlerMethodArgumentResolver&gt; argumentResolvers)
 *     {
 *         argumentResolvers.add(new DatatablesCriteriasMethodArgumentResolver());
 *     }
 * }
 * </pre>
 * 
 * @author xxx
 * @since xx
 * @see DatatablesParams
 * @see DatatablesCriterias

[tduchateau]

Comment by gorder from Monday Jul 22, 2013 at 13:39 GMT

Also in DatatablesCriterias getFromRequest()

There are lines like this

Integer iEcho = Integer.parseInt(sEcho);
Integer iDisplayStart = Integer.parseInt(sDisplayStart);
Integer iDisplayLength = Integer.parseInt(sDisplayLength);

This throws null pointers if you are testing your endpoint from a browser rest client rather than the actual UI. In my case I want to check the JSON, but the data table params are absent in this case. I can see logging an error or warning in this case but a null pointer probably is not good. I would do something similar to what you did with the sorting column number

Integer sortingColNumber = request.getParameter(DTConstants.DT_I_SORTING_COLS) != null
? Integer.parseInt(request.getParameter(DTConstants.DT_I_SORTING_COLS))
: 0;

or since your are using the Object Integer just set it to null in that case.

[tduchateau]

Comment by tduchateau from Friday Jul 26, 2013 at 17:05 GMT

Huge THANKS for your comments and suggestions Bill!

Finally, since I'm refactoring the export features and since it's now related to DatatablesCriterias and DatatablesParams objects, let's do that in the 0.9.0. :-)

Please don't mind if I split some of your comments into multiple issues.

[tduchateau]

Comment by gorder from Sunday Jul 28, 2013 at 00:40 GMT

Looks good! Thanks.