You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "codeconsole (via GitHub)" <gi...@apache.org> on 2026/01/28 00:50:35 UTC

[PR] 7.1.x Fix for UrlMapping greedy not working when no id is present [grails-core]

codeconsole opened a new pull request, #15358:
URL: https://github.com/apache/grails-core/pull/15358

   Follow up to https://github.com/apache/grails-core/pull/15213
   
   ```groovy
   "/$controller/$action?/$id?(.$format)?"
   ```
   
   resulted in `/user.json` not matching. only `/user` would match. This fixes that.
   
   Is seems like the best pathway forward to support formats and string $id's with .'s is the following mapping:
   
   ```groovy
     // First: greedy with format constraint                                                                                              
     "/$controller/$action/$id+(.$format)?" {                                                                                             
         constraints {                                                                                                                    
             format(inList: ['json', 'xml', 'html', 'csv'])                                                                               
         }                                                                                                                                
     }                                                                                                                                    
     // Second: fallback for IDs with dots but no valid format                                                                            
     "/$controller/$action/$id"()  
   ```
   
     | URL | id | format | Which mapping? |                                                                                               
     |-----|-----|--------|----------------|                                                                                              
     | `/user/show/bob.smith` | `bob.smith` | `null` | Fallback (no valid format) |                                                       
     | `/user/show/bob.smith.json` | `bob.smith` | `json` | Greedy (valid format) |                                                       
     | `/user/show/bob.smith.jones` | `bob.smith.jones` | `null` | Fallback (no valid format) |                                           
     | `/user/show/bob.smith.jones.xml` | `bob.smith.jones` | `xml` | Greedy (valid format) |  


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] 7.1.x Fix for UrlMapping greedy not working when no id is present [grails-core]

Posted by "codeconsole (via GitHub)" <gi...@apache.org>.
codeconsole merged PR #15358:
URL: https://github.com/apache/grails-core/pull/15358


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] 7.1.x Fix for UrlMapping greedy not working when no id is present [grails-core]

Posted by "sbglasius (via GitHub)" <gi...@apache.org>.
sbglasius commented on code in PR #15358:
URL: https://github.com/apache/grails-core/pull/15358#discussion_r2735104760


##########
grails-web-url-mappings/src/test/groovy/grails/web/mapping/UrlMappingsWithGreedyExtensionSpec.groovy:
##########
@@ -227,4 +227,223 @@ class UrlMappingsWithGreedyExtensionSpec extends AbstractUrlMappingsSpec {
             info.urlData.hasGreedyExtensionParam()
             info.urlData.hasOptionalExtension()
     }
+
+    void "Test that greedy id with optional action matches controller-only URL with format"() {
+        given: "A complex URL mapping with optional action and optional greedy id"
+            def urlMappingsHolder = getUrlMappingsHolder {
+                "/$controller/$action?/$id+?(.$format)?"()
+            }
+
+        when: "Matching a URL with just controller and format (no action, no id)"
+            def info = urlMappingsHolder.match('/mobile.json')
+
+        then: "The controller and format are correctly extracted without greedy behavior affecting controller"
+            info != null
+            info.parameters.controller == 'mobile'
+            info.parameters.action == null
+            info.parameters.id == null
+            info.parameters.format == 'json'
+    }
+
+    void "Test that greedy id works correctly when all parameters are present"() {
+        given: "A complex URL mapping with optional action and optional greedy id"
+            def urlMappingsHolder = getUrlMappingsHolder {
+                "/$controller/$action?/$id+?(.$format)?"()
+            }
+
+        when: "Matching a URL with all parameters including id with dots"
+            def info = urlMappingsHolder.match('/user/show/bob.smith.json')
+
+        then: "The greedy id captures everything up to the last dot"
+            info != null
+            info.parameters.controller == 'user'
+            info.parameters.action == 'show'
+            info.parameters.id == 'bob.smith'
+            info.parameters.format == 'json'
+    }
+
+    void "Test that greedy id does not affect controller when only controller and action are present"() {
+        given: "A complex URL mapping with optional action and optional greedy id"
+            def urlMappingsHolder = getUrlMappingsHolder {
+                "/$controller/$action?/$id+?(.$format)?"()
+            }
+
+        when: "Matching a URL with controller and action only"
+            def info = urlMappingsHolder.match('/user/show.json')
+
+        then: "The controller and action are correctly extracted with format"
+            info != null
+            info.parameters.controller == 'user'
+            info.parameters.action == 'show'
+            info.parameters.id == null
+            info.parameters.format == 'json'
+    }
+
+    void "Test that controller with dots is handled correctly without greedy id"() {
+        given: "A complex URL mapping with optional action and optional greedy id"
+            def urlMappingsHolder = getUrlMappingsHolder {
+                "/$controller/$action?/$id+?(.$format)?"()
+            }
+
+        when: "Matching a URL with controller containing dots and format"
+            def info = urlMappingsHolder.match('/test.test.json')
+
+        then: "The URL /test.test.json should NOT apply greedy to controller"
+            info != null
+            println "DEBUG: controller=${info.parameters.controller}, action=${info.parameters.action}, id=${info.parameters.id}, format=${info.parameters.format}"

Review Comment:
   Is leaving a `println` intended? Personally I prefer to annotate the Spec with `@Slf4j` and use `log.debug` instead



##########
grails-web-url-mappings/src/test/groovy/grails/web/mapping/UrlMappingsWithGreedyExtensionSpec.groovy:
##########
@@ -227,4 +227,223 @@ class UrlMappingsWithGreedyExtensionSpec extends AbstractUrlMappingsSpec {
             info.urlData.hasGreedyExtensionParam()
             info.urlData.hasOptionalExtension()
     }
+
+    void "Test that greedy id with optional action matches controller-only URL with format"() {
+        given: "A complex URL mapping with optional action and optional greedy id"
+            def urlMappingsHolder = getUrlMappingsHolder {
+                "/$controller/$action?/$id+?(.$format)?"()
+            }
+
+        when: "Matching a URL with just controller and format (no action, no id)"
+            def info = urlMappingsHolder.match('/mobile.json')
+
+        then: "The controller and format are correctly extracted without greedy behavior affecting controller"
+            info != null
+            info.parameters.controller == 'mobile'
+            info.parameters.action == null
+            info.parameters.id == null
+            info.parameters.format == 'json'
+    }
+
+    void "Test that greedy id works correctly when all parameters are present"() {
+        given: "A complex URL mapping with optional action and optional greedy id"
+            def urlMappingsHolder = getUrlMappingsHolder {
+                "/$controller/$action?/$id+?(.$format)?"()
+            }
+
+        when: "Matching a URL with all parameters including id with dots"
+            def info = urlMappingsHolder.match('/user/show/bob.smith.json')
+
+        then: "The greedy id captures everything up to the last dot"
+            info != null
+            info.parameters.controller == 'user'
+            info.parameters.action == 'show'
+            info.parameters.id == 'bob.smith'
+            info.parameters.format == 'json'
+    }
+
+    void "Test that greedy id does not affect controller when only controller and action are present"() {
+        given: "A complex URL mapping with optional action and optional greedy id"
+            def urlMappingsHolder = getUrlMappingsHolder {
+                "/$controller/$action?/$id+?(.$format)?"()
+            }
+
+        when: "Matching a URL with controller and action only"
+            def info = urlMappingsHolder.match('/user/show.json')
+
+        then: "The controller and action are correctly extracted with format"
+            info != null
+            info.parameters.controller == 'user'
+            info.parameters.action == 'show'
+            info.parameters.id == null
+            info.parameters.format == 'json'
+    }
+
+    void "Test that controller with dots is handled correctly without greedy id"() {
+        given: "A complex URL mapping with optional action and optional greedy id"
+            def urlMappingsHolder = getUrlMappingsHolder {
+                "/$controller/$action?/$id+?(.$format)?"()
+            }
+
+        when: "Matching a URL with controller containing dots and format"
+            def info = urlMappingsHolder.match('/test.test.json')
+
+        then: "The URL /test.test.json should NOT apply greedy to controller"
+            info != null
+            println "DEBUG: controller=${info.parameters.controller}, action=${info.parameters.action}, id=${info.parameters.id}, format=${info.parameters.format}"
+            // The greedy + modifier only applies to $id, not $controller
+            // So /test.test.json should be parsed using non-greedy behavior:
+            // controller=test, format=test.json
+            info.parameters.controller == 'test'
+            info.parameters.action == null
+            info.parameters.id == null
+            info.parameters.format == 'test.json'
+    }
+
+    void "Test greedy id with no explicit format - what happens with bob.smith"() {
+        given: "A complex URL mapping with optional action and optional greedy id"
+            def urlMappingsHolder = getUrlMappingsHolder {
+                "/$controller/$action?/$id+?(.$format)?"()
+            }
+
+        when: "Matching a URL where the id contains a dot but there's no real format"
+            def info = urlMappingsHolder.match('/user/show/bob.smith')
+
+        then: "Check what actually happens - does .smith become the format?"
+            info != null
+            println "DEBUG bob.smith: controller=${info.parameters.controller}, action=${info.parameters.action}, id=${info.parameters.id}, format=${info.parameters.format}"

Review Comment:
   same as above



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] 7.1.x Fix for UrlMapping greedy not working when no id is present [grails-core]

Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on code in PR #15358:
URL: https://github.com/apache/grails-core/pull/15358#discussion_r2736598247


##########
grails-web-url-mappings/src/test/groovy/grails/web/mapping/UrlMappingsWithGreedyExtensionSpec.groovy:
##########
@@ -227,4 +227,223 @@ class UrlMappingsWithGreedyExtensionSpec extends AbstractUrlMappingsSpec {
             info.urlData.hasGreedyExtensionParam()
             info.urlData.hasOptionalExtension()
     }
+
+    void "Test that greedy id with optional action matches controller-only URL with format"() {
+        given: "A complex URL mapping with optional action and optional greedy id"
+            def urlMappingsHolder = getUrlMappingsHolder {
+                "/$controller/$action?/$id+?(.$format)?"()
+            }
+
+        when: "Matching a URL with just controller and format (no action, no id)"
+            def info = urlMappingsHolder.match('/mobile.json')
+
+        then: "The controller and format are correctly extracted without greedy behavior affecting controller"
+            info != null
+            info.parameters.controller == 'mobile'
+            info.parameters.action == null
+            info.parameters.id == null
+            info.parameters.format == 'json'
+    }
+
+    void "Test that greedy id works correctly when all parameters are present"() {
+        given: "A complex URL mapping with optional action and optional greedy id"
+            def urlMappingsHolder = getUrlMappingsHolder {
+                "/$controller/$action?/$id+?(.$format)?"()
+            }
+
+        when: "Matching a URL with all parameters including id with dots"
+            def info = urlMappingsHolder.match('/user/show/bob.smith.json')
+
+        then: "The greedy id captures everything up to the last dot"
+            info != null
+            info.parameters.controller == 'user'
+            info.parameters.action == 'show'
+            info.parameters.id == 'bob.smith'
+            info.parameters.format == 'json'
+    }
+
+    void "Test that greedy id does not affect controller when only controller and action are present"() {
+        given: "A complex URL mapping with optional action and optional greedy id"
+            def urlMappingsHolder = getUrlMappingsHolder {
+                "/$controller/$action?/$id+?(.$format)?"()
+            }
+
+        when: "Matching a URL with controller and action only"
+            def info = urlMappingsHolder.match('/user/show.json')
+
+        then: "The controller and action are correctly extracted with format"
+            info != null
+            info.parameters.controller == 'user'
+            info.parameters.action == 'show'
+            info.parameters.id == null
+            info.parameters.format == 'json'
+    }
+
+    void "Test that controller with dots is handled correctly without greedy id"() {
+        given: "A complex URL mapping with optional action and optional greedy id"
+            def urlMappingsHolder = getUrlMappingsHolder {
+                "/$controller/$action?/$id+?(.$format)?"()
+            }
+
+        when: "Matching a URL with controller containing dots and format"
+            def info = urlMappingsHolder.match('/test.test.json')
+
+        then: "The URL /test.test.json should NOT apply greedy to controller"
+            info != null
+            println "DEBUG: controller=${info.parameters.controller}, action=${info.parameters.action}, id=${info.parameters.id}, format=${info.parameters.format}"
+            // The greedy + modifier only applies to $id, not $controller
+            // So /test.test.json should be parsed using non-greedy behavior:
+            // controller=test, format=test.json
+            info.parameters.controller == 'test'
+            info.parameters.action == null
+            info.parameters.id == null
+            info.parameters.format == 'test.json'
+    }
+
+    void "Test greedy id with no explicit format - what happens with bob.smith"() {
+        given: "A complex URL mapping with optional action and optional greedy id"
+            def urlMappingsHolder = getUrlMappingsHolder {
+                "/$controller/$action?/$id+?(.$format)?"()
+            }
+
+        when: "Matching a URL where the id contains a dot but there's no real format"
+            def info = urlMappingsHolder.match('/user/show/bob.smith')
+
+        then: "Check what actually happens - does .smith become the format?"
+            info != null
+            println "DEBUG bob.smith: controller=${info.parameters.controller}, action=${info.parameters.action}, id=${info.parameters.id}, format=${info.parameters.format}"

Review Comment:
   Remove debug println?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org