Interceptor's method option not working.

[sergiomichels]

Steps to Reproduce

1. Create an application
2. Generate all from some domain class
3. Create an interceptor that matches only POST actions

class PostInterceptor {

    PostInterceptor() {
        //match(controller: 'person', method: 'POST', action: '*') -> not working
        //match(controller: 'person', action: '*') -> working
        match(method: 'POST') //not working
    }

    boolean before() {
        println "controller action intercepted!"
        true
    }

    boolean after() { true }

    void afterView() {
        // no-op
    }
}

Expected Behaviour

When calling save the interceptor should be used

Actual Behaviour

The method option of the interceptor is not working as expected.

Environment Information

• Operating System: Win 10
• Grails Version: 3.1.6
• JDK Version: 1.8.0_77

DefaultUrlMappingInfo instance have * in the httpMethod property, so it's not matching with 'POST' or any other method.

[sergiomichels]

@graemerocher I think this is a simple yet important feature. I noticed that folks have to make workarounds using matches with method and namespaces too. If you hint me where to look I can try to help.

[graemerocher]

@sergiomichels https://github.com/grails/grails-core/blob/master/grails-plugin-interceptors/src/main/groovy/org/grails/plugins/web/interceptors/UrlMappingMatcher.groovy#L59 needs to be changed to take into the method

[sergiomichels]

@graemerocher OK, I was not clear in my first comment. The UrlMappingInfo in this method is wrong (* instead of the http method). Which class is responsible for creating this info?

[graemerocher]

@sergiomichels https://github.com/grails/grails-core/blob/master/grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/RegexUrlMapping.java#L292

[sergiomichels]

@graemerocher If i got it, RegexUrlMapping always have * in the method because of the UrlMappings of the application:

"/$controller/$action?/$id?(.$format)?"{
  constraints {
    // apply constraints here
  }
}

Regardless of the http method, this mapping will be used. Probably is the same problem with namespaces.

Currently the only way to get the correct method is to create a custom mapping, like:

"/test-method"(controller: "person", action: "testMethod", method: "GET")

[sergiomichels]

@graemerocher OK, looking from outside now - Interceptor.groovy calls the doesMatch(). Shouldn't it create a new UrlMappingInfo with the request's http method to use it to match?

https://github.com/grails/grails-core/blob/master/grails-plugin-interceptors/src/main/groovy/grails/artefact/Interceptor.groovy#L101

[sergiomichels]

Does it look ok? Maybe it should just replace the old doesMatch signature?

sergiomichels@c1b23a0

I will test, but probably this will be needed for action too, maybe something like RequestMappingInfo class with the information to perform the match.

[graemerocher]

@sergiomichels yes, but is this import necessary? sergiomichels@c1b23a0#diff-a693465d54d8d350c7369999a56ae3e2R33

[sergiomichels]

@graemerocher Nope :-) I will do some tests with action and namespace before making a pull req, and will remove unused imports.

[graemerocher]

Ok just be aware i need he pull request by Monday if it is to make it into the next release

[ctoestreich]

I think this is great, @sergiomichels I would love to see more positive and negative test cases using all the http methods matching the current w3 spec to ensure this works across all types than just the one test for POST.

https://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html

[sergiomichels]

@ctoestreich Tests for all http methods is redundant because it's a String, and if you declare it correctly in your interceptor according to the HttpRequest you will be fine.

[ctoestreich]

Perhaps, but it wont hurt to have the tests. I will add them. Then the behavior is clearly outlined and tested.