Spring AntPathMatcher enhancement

Current implementation of the AntPathMatcher.extractUriTemplateVariables is not able to handle defined variable if the pattern contains multiple groups (additional inner ( and ) ) and I don't think it's necessary. The current code relies on the group indexes, but it's already possible to use group names (since Java 1.7). This test explains the problem:

import org.junit.Test;
import org.springframework.util.AntPathMatcher;

import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

public class AntPathMatcherBug {

    static final String PATTERN = "**/appname{version:(-[0-9]+(\\.[0-9a-zA-Z_+]+)+?)?}-xx.jar";
    static final String PATH = "somepath/appname-1.0.0-xx.jar";

    @Test
    public void testGetVersionVariableFail() {
        final AntPathMatcher antPathMatcher = new AntPathMatcher();

        assertTrue(antPathMatcher.match(PATTERN, PATH));

        // antPathMatcher generates \Qappname\E((-[0-9]+(\.[0-9a-zA-Z_+]+)+?)?)\Q-xx.jar\E
        final Map<String, String> variablesMap = antPathMatcher.extractUriTemplateVariables(PATTERN, PATH);
        // throws Exception
        // java.lang.IllegalArgumentException: The number of capturing groups in the pattern segment \Qappname\E((-[0-9]+(\.[0-9a-zA-Z_+]+)+?)?)\Q-xx.jar\E
        // does not match the number of URI template variables it defines, which can occur if capturing groups are used in a java.net.URI template regex.
        // Use non-capturing groups instead.

    }

    @Test
    public void testGetVersionVariableSuccess() {
        final Pattern regex = Pattern.compile("\\Qappname\\E(?<version>(-[0-9]+(\\.[0-9a-zA-Z_+]+)+?)?)\\Q-xx.jar\\E");
        final Matcher matcher = regex.matcher(PATH);
        assertTrue(matcher.find());

        assertEquals(matcher.group("version"), "-1.0.0");

    }
}

My suggestion is to add group naming (?<version> in this case) and not to use group indexes to identify and to get variable value. Using non-capturing groups makes regex more difficult to read and this solution will avoid throwing exception at runtime.

What do you think?

Comment From: snicoll

My suggestion is to add group naming (? in this case) and not to use group indexes to identify and to get variable value.

That's unfortunately would be breaking the semantic of what extractUriTemplateVariables supports. Given how widely this is used, I don't see how we could support this in a backward compatible way.