grails / grails-core

The Grails Web Application Framework
http://grails.org
Apache License 2.0
2.78k stars 950 forks source link

GRAILS-11256: Reverse URL lookup does not return correct URL for index action when using HTTP method mapping #2385

Closed graemerocher closed 9 years ago

graemerocher commented 10 years ago

Original Reporter: andrewjstrominger Environment: Not Specified Version: 2.3.4 Migrated From: http://jira.grails.org/browse/GRAILS-11256

When using HTTP method mapping in the URL mappings for some actions the index action reverse url lookup returns the HTTP method url.

This was working correctly in 2.1.1 but is broken in 2.3.4. Attached is an example project with a failing test.

{code:title=grails-app/conf/UrlMappings.groovy|borderStyle=solid} class UrlMappings {

static mappings = {
    "/external/book/$name"(controller:"book"){
        action = [GET:'getByName']
    }

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

    "/"(view:"/index")
    "500"(view:'/error')
}

} {code}

{code:title=grails-app/controllers/tester/BookController.groovy|borderStyle=solid} package tester

class BookController {

def index() { }

def save() {}
def create() {}
def delete() {}
def update() {}

def getByName() {}

} {code}

{code:title=test/unit/UrlMappingsSpec.groovy|borderStyle=solid} import tester.BookController import grails.test.mixin.TestFor import spock.lang.Specification

@TestFor(UrlMappings) @Mock(BookController) class UrlMappingsSpec extends Specification {

void "test BookController default mappings"() {
    expect:
        assertUrlMapping(path, controller:'book', action:action)

    where:
        path                   | action  
        "/book/save"           | 'save' 
        "/book/create"         | 'create' 
        "/book/delete"         | 'delete' 
        "/book/update"         | 'update' 
        "/book/index"          | 'index' 
}

void "test BookController REST mappings"() {
    expect:
        webRequest.currentRequest.setMethod(method) 
        assertForwardUrlMapping(path, controller:'book', action:action)
    where:
        path                        | action           | method
        "/external/book/bookName"   | 'getByName'      | "GET"
}

} {code}

In the above test the index action reverse lookup fails and it returns /external/book when it should return /book/index. It passes for all of the other actions in the BookController. {code:title=TestFailure|borderStyle=solid} | Failure: test BookController default mappings(UrlMappingsSpec) | junit.framework.ComparisonFailure: reverse mapping assertion for {controller = book, action = index, params = [:]} expected:</[book/index]> but was:</[external/book]> at junit.framework.Assert.assertEquals(Assert.java:100) at grails.test.mixin.web.UrlMappingsUnitTestMixin.assertReverseUrlMapping(UrlMappingsUnitTestMixin.groovy:288) at grails.test.mixin.web.UrlMappingsUnitTestMixin.assertUrlMapping(UrlMappingsUnitTestMixin.groovy:176) at grails.test.mixin.web.UrlMappingsUnitTestMixin.assertUrlMapping(UrlMappingsUnitTestMixin.groovy:158) at UrlMappingsSpec.test BookController default mappings(UrlMappingsTest.groovy:11) {code}

A workaround is to explicitly add the mapping for the index action: {code:title=grails-app/conf/UrlMappings.groovy|borderStyle=solid} class UrlMappings {

static mappings = {
    "/book/index"(controller:"book", action:"index")

    "/external/book/$name"(controller:"book"){
        action = [GET:'getByName']
    }

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

    "/"(view:"/index")
    "500"(view:'/error')
}

} {code}

graemerocher commented 10 years ago

brownj said: Having controller actions with names that look like property getters (like getByName) is problematic. I don't know if that is relevant to this particular problem though. Do you have the same problem if your controller action is not a property accessor?