GRAILS-11512 - improve the handling of URL mappings which contain hyphens

Author: jeffscottbrown

grails-plugin-url-mappings/src/main/groovy/org/codehaus/groovy/grails/web/mapping/RegexUrlMapping.java

@@ -255,11 +255,10 @@ protected Pattern convertToRegex(String url) {
      * @see org.codehaus.groovy.grails.web.mapping.UrlMappingInfo
      */
     public UrlMappingInfo match(String uri) {
-        Integer slashCount = org.springframework.util.StringUtils.countOccurrencesOf(uri, "/");
         for (Pattern pattern : patterns) {
             Matcher m = pattern.matcher(uri);
             if (m.matches()) {
-                UrlMappingInfo urlInfo = createUrlMappingInfo(uri, m, slashCount);
+                UrlMappingInfo urlInfo = createUrlMappingInfo(uri, m);
                 if (urlInfo != null) {
                     return urlInfo;
                 }
@@ -574,15 +573,16 @@ public UrlMappingData getUrlData() {
     }
 
     @SuppressWarnings("unchecked")
-    private UrlMappingInfo createUrlMappingInfo(String uri, Matcher m, int tokenCount) {
+    private UrlMappingInfo createUrlMappingInfo(String uri, Matcher m) {
         boolean hasOptionalExtension = urlData.hasOptionalExtension();
         Map params = new HashMap();
         Errors errors = new MapBindingResult(params, "urlMapping");
+        int groupCount = m.groupCount();
         String lastGroup = null;
-        for (int i = 0, count = m.groupCount(); i < count; i++) {
+        for (int i = 0; i < groupCount; i++) {
             lastGroup = m.group(i + 1);
             // if null optional.. ignore
-            if (i == tokenCount & hasOptionalExtension) {
+            if (i == groupCount - 1 && hasOptionalExtension) {
                 ConstrainedProperty cp = constraints[constraints.length-1];
                 cp.validate(this, lastGroup, errors);
 

grails-test-suite-web/src/test/groovy/org/codehaus/groovy/grails/web/mapping/RegexUrlMappingTests.groovy

@@ -65,8 +65,38 @@ mappings {
       controller = 'reporting'
       action = 'view'
   }
+  "/$first-alpha-$second/$third-beta-$fourth-foo(.$format)?" {
+      controller = 'hyphenTests'
+      action = 'view'
+  }
 }
 '''
+    
+    void testHyphenDelimiters() {
+        def holder = new DefaultUrlMappingsHolder(evaluator.evaluateMappings(new ByteArrayResource(mappingScript.bytes)))
+    
+        def info = holder.match("/one-alpha-two/three-beta-four-foo.json")
+        assertNotNull info
+        assertEquals 'hyphenTests', info.controllerName
+        assertEquals 'view', info.actionName
+        assertEquals 'one', info.params.first
+        assertEquals 'two', info.params.second
+        assertEquals 'three', info.params.third
+        assertEquals 'four', info.params.fourth
+        assertEquals 'json', info.params.format
+
+ 
+        info = holder.match("/one-alpha-two/three-beta-four-foo")
+        assertNotNull info
+        assertEquals 'hyphenTests', info.controllerName
+        assertEquals 'view', info.actionName
+        assertEquals 'one', info.params.first
+        assertEquals 'two', info.params.second
+        assertEquals 'three', info.params.third
+        assertEquals 'four', info.params.fourth
+        assertNull info.params.format
+    }
+
     void testMaptoURI() {
         def res = new ByteArrayResource(mappingScript.bytes)
         def mappings = evaluator.evaluateMappings(res)
@@ -372,7 +402,7 @@ mappings {
         assertEquals 'view', info.actionName
         assertEquals 'my.id', info.params.foo
     }
-
+    
     void testInit() {
         def parser = new DefaultUrlMappingParser()
         def m = new RegexUrlMapping(parser.parse("/(*)/hello"), "test", null, null, null, null, null, UrlMapping.ANY_VERSION,[] as ConstrainedProperty[], servletContext)