UriTemplate expansion is inconsistent when using beans to pass the values

[yawkat]

Discussed in https://github.com/micronaut-projects/micronaut-core/discussions/11191

^{Originally posted by **nedelva** September 18, 2024} I was playing with UriTemplate and I wrote a JUnit 5 test to check if it implements all of the features shown in [URI.js](https://medialize.github.io/URI.js/uri-template.html). ```java package io.micronaut.http.uri; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.converter.ArgumentConversionException; import org.junit.jupiter.params.converter.ConvertWith; import org.junit.jupiter.params.converter.SimpleArgumentConverter; import org.junit.jupiter.params.provider.CsvSource; import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; import static org.junit.jupiter.api.Assertions.assertEquals; public class UriTemplateTest { private UriTemplate uriTemplate; @ParameterizedTest @CsvSource(delimiter = '|', value = { "/users{?id*} | 3,4,5 | /users?id=3&id=4&id=5", //style==form,spaceDelimited & explode==true "/users{?id} | 3,4,5 | /users?id=3,4,5", //style==form & explode==false "/users{?id*} | 3,4,5 | /users?id=3&id=4&id=5" //style==pipeDelimited & explode==true }) public void testQueryParams(String uriTemplate, @ConvertWith(StringToListConverter.class) List inputList, String expectedExpansion) { this.uriTemplate = new UriTemplate(uriTemplate); String expansion = this.uriTemplate.expand(Map.of("id", inputList)); assertEquals(expectedExpansion, expansion); } @ParameterizedTest @CsvSource(delimiter = '|', value = { "{+id*}|one=alpha,two=bravo", "{#id*}|#one=alpha,two=bravo", "{#id*}|#one=alpha,two=bravo", "{;id*}|;one=alpha;two=bravo", "{&id*}|&one=alpha&two=bravo", }) void testDeepObjectMapParam(String template, String expected) { uriTemplate = new UriTemplate(template); Map map = new HashMap<>(); map.put("one", "alpha"); map.put("two", "bravo"); assertEquals(expected, uriTemplate.expand(Map.of("id", map))); } @ParameterizedTest @CsvSource(delimiter = '|', value = { "{+id*}|one=alpha,two=bravo", "{#id*}|#one=alpha,two=bravo", "{#id*}|#one=alpha,two=bravo", "{;id*}|;one=alpha;two=bravo", "{&id*}|&one=alpha&two=bravo", }) void testDeepObjectParam(String template, String expected) { uriTemplate = new UriTemplate(template); DeepObject deepObject = new DeepObject("alpha", "bravo"); String expansion = this.uriTemplate.expand(Map.of("id", deepObject)); assertEquals(expected, expansion); } @ParameterizedTest @CsvSource(delimiter = '|', value = { "/users/{+id*} | 3,4,5 | /users/3,4,5", //style==simple & explode==true "/users/{+id} | 3,4,5 | /users/3,4,5", //style==simple & explode==false "/users/{.id*} | 3,4,5 | /users/.3.4.5", //style==label & explode==true "/users/{.id} | 3,4,5 | /users/.3,4,5", //style==label & explode==false "/users/{;id*} | 3,4,5 | /users/;id=3;id=4;id=5", //style==label & explode==true "/users/{;id} | 3,4,5 | /users/;id=3,4,5", //style==label & explode==false }) public void testPathParams(String uriTemplate, @ConvertWith(StringToListConverter.class) List inputList, String expectedExpansion) { this.uriTemplate = new UriTemplate(uriTemplate); String expansion = this.uriTemplate.expand(Map.of("id", inputList)); assertEquals(expectedExpansion, expansion); } static class StringToListConverter extends SimpleArgumentConverter { @Override protected Object convert(Object source, Class targetType) throws ArgumentConversionException { if (source instanceof String && List.class.isAssignableFrom(targetType)) { String[] values = ((String) source).split(","); return Arrays.asList(values); } else { throw new ArgumentConversionException("Cannot convert source type " + source.getClass() + " to target type List"); } } } } ``` In the test `testDeepObjectParam` I am using a record defined as: ```java package io.micronaut.http.uri; import io.micronaut.core.annotation.Introspected; @Introspected public record DeepObject(String one, String two) { } ``` The test using `DeepObject` is only mimicking the `testDeepObjectMapParam` but using a `record` instead of a `Map`. The funny thing is it fails only a _few cases_ but not always the same lines (in my runs, lines 1, 3, 5 or just 3 and 4 from the `CsvSource`). The failure always is about the order of evaluation of the values in the expansion: ``` [3] template={#id*}, expected=#one=alpha,two=bravo : Expected :#one=alpha,two=bravo Actual :#two=bravo,one=alpha [4] template={;id*}, expected=;one=alpha;two=bravo Expected :;one=alpha;two=bravo Actual :;two=bravo;one=alpha ``` I also noticed that if the test method `testDeepObjectParam` is copied as the sole test in another test class, _different cases fail_ (!) (Perhaps some race condition is occurring?) Shouldn't this expansion produce predictable results? (Not that it matters for the expanded URI) Final note: the HashMap is evaluated in the same way, but this is a happy case. If I use `Map.of("one","alpha","two","bravo")` the test `testDeepObjectMapParam` will start failing as well a few cases.

[dstepanov]

That class is replaced by UriTemplateMatcher

[nedelva]

That class is replaced by UriTemplateMatcher

I did not get this memo 😀 . Now that you mention it @dstepanov, I believe this intention should be properly documented (a mention in the Guide and the UriTemplate class marked as @Deprecated)

[nedelva]

Well, the class UriTemplateMatchersuffers from the same ailment as UriTemplatedoes. Just run this test to see for yourself:

package io.micronaut.http.uri;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;

import java.util.Map;

import static org.junit.jupiter.api.Assertions.assertEquals;

class UriTemplateMatcherTest {

    @ParameterizedTest
    @CsvSource(delimiter = '|', value = {
            "{+id*}|one=alpha,two=bravo",
            "{#id*}|#one=alpha,two=bravo",
            "{#id*}|#one=alpha,two=bravo",
            "{;id*}|;one=alpha;two=bravo",
            "{&id*}|&one=alpha&two=bravo",
    })
    void testDeepObjectParam(String template, String expected) {
        UriTemplateMatcher templateMatcher = new UriTemplateMatcher(template);
        DeepObject deepObject = new DeepObject("alpha", "bravo");
        String expansion = templateMatcher.expand(Map.of("id", deepObject));
        assertEquals(expected, expansion);

    }
}

[dstepanov]

I didn't deprecate it because it's used in the routes API, and there is no way to use the new one—the idea is to replace one with another in v5. I think that in the future version (maybe even before v5), I would like to have an option for controllers to select what kind of URI style you want: current one based on uri template or simple JAX-RS style.

[nedelva]

@dstepanov, @yawkat Are there any plans to fix this bug?