UrlMappings with group and collection produce error URL

[todoubaba]

Steps to Reproduce

1. grails create-app test
2. modify UrlMappings.groovy, add

      "/foos"(resources: 'foo') {
          collection {
              '/baz'(controller: 'foo', action: 'baz')
          }
      }

      group "/g", {
          "/bars"(resources: 'bar') {
              collection {
                  '/baz'(controller: 'bar', action: 'baz')
              }
          }
      }

3. grails url-mapping-report

Expected Behaviour

Controller: bar
 |   GET    | /g/bars/create                                    | Action: create           |
 |   GET    | /g/bars/${id}/edit                                | Action: edit             |
 |    *     | /g/bars/${barId}/baz                              | Action: baz              | <---
 |   POST   | /g/bars                                           | Action: save             |
 |   GET    | /g/bars                                           | Action: index            |
 |  DELETE  | /g/bars/${id}                                     | Action: delete           |
 |  PATCH   | /g/bars/${id}                                     | Action: patch            |
 |   PUT    | /g/bars/${id}                                     | Action: update           |
 |   GET    | /g/bars/${id}                                     | Action: show             |

Controller: foo
 |   GET    | /foos/create                                      | Action: create           |
 |    *     | /foos/baz                                         | Action: baz              |
 |   GET    | /foos/${id}/edit                                  | Action: edit             |
 |   POST   | /foos                                             | Action: save             |
 |   GET    | /foos                                             | Action: index            |
 |  DELETE  | /foos/${id}                                       | Action: delete           |
 |  PATCH   | /foos/${id}                                       | Action: patch            |
 |   PUT    | /foos/${id}                                       | Action: update           |
 |   GET    | /foos/${id}                                       | Action: show             |

Actual Behaviour

Controller: bar
 |   GET    | /g/bars/create                                    | Action: create           |
 |   GET    | /g/bars/${id}/edit                                | Action: edit             |
 |    *     | /g/bars/baz                                       | Action: baz              | <---
 |   POST   | /g/bars                                           | Action: save             |
 |   GET    | /g/bars                                           | Action: index            |
 |  DELETE  | /g/bars/${id}                                     | Action: delete           |
 |  PATCH   | /g/bars/${id}                                     | Action: patch            |
 |   PUT    | /g/bars/${id}                                     | Action: update           |
 |   GET    | /g/bars/${id}                                     | Action: show             |

Controller: foo
 |   GET    | /foos/create                                      | Action: create           |
 |    *     | /foos/baz                                         | Action: baz              |
 |   GET    | /foos/${id}/edit                                  | Action: edit             |
 |   POST   | /foos                                             | Action: save             |
 |   GET    | /foos                                             | Action: index            |
 |  DELETE  | /foos/${id}                                       | Action: delete           |
 |  PATCH   | /foos/${id}                                       | Action: patch            |
 |   PUT    | /foos/${id}                                       | Action: update           |
 |   GET    | /foos/${id}                                       | Action: show             |

Environment Information

• Operating System: Windows 10
• Grails Version: 3.2.2
• JDK Version: 1.8

[graemerocher]

Seems to be already fixed in 3.2.3, I get the following as expected:

 |    *     | /g/bars/${barId}/baz                              | Action: baz              |

[alexkramer]

Am I reading this correctly? Using the collection syntax will result in different behavior depending on whether its contained within a group closure? Why does that make sense? If an id is expected in the url path to the collection wouldn't you nest without using the collection syntax? Isn't the whole point of having the collection syntax that it doesn't require an id in the url?

[danielritter]

I find this is a good question. My opinion is there should also be a ${barId} when you don't use "group" over a collection. At the moment this is not the case.

See:
| * | /foos/baz | Action: baz |

My opinion it should be:
| * | /foos/${barId}/baz | Action: baz |

@graemerocher Is this a bug or what is the reason for this actual behaviour? Why is there ${barId} within a "group" and without "group" not?

[danielritter]

How I see in the documentation it should in both cases not be a ${barId} when using a collection. Neither within a "group" or without "group" section.
See: http://docs.grails.org/latest/guide/theWebLayer.html#_nested_resources

[rainboyan]

I agree with @alexkramer , this is a bug. with or without group block, using nested collection should be same behavior.

[rainboyan]

I agree with @alexkramer , this is a bug. Whether within group block or not, using nested collection should be same behavior.

[matrei]

In the process of upgrading Grails to Groovy 4, the test added by a74bf4c fails.

This begs again to ask the question: What is the correct behavior when using collection (in, or not in, a group)?

The consensus among the people who commented on this issue seems to be that:

group "/g", {
    "/bars"(resources: 'bar') {
        collection {
            '/baz'(controller: 'bar', action: 'baz')
        }
    }
}

should generate:

/g/bars/baz

and not:

/g/bars/${barId}/baz

For some reason, having to do with what this resolves to in DefaultUrlMappingsEvaluator.UrlMappingBuilder.collection(Closure callable), /g/bars/baz gets generated with Groovy 4, and /g/bars/${barId}/baz gets generated with Groovy 3.

If we follow the consensus here, the test is wrong, and the behavior is now correct when running with Groovy 4.
Anyone have any comments on that?