Fix declarative request parameter codegen (helidon-io#11745)

* Fix declarative request parameter codegen

* Fix declarative parameter codegen CI validation

* Fix declarative mapper qualifier fallback

* Fix defaulted list query codegen

* Protect mapper qualifier cache keys

* Document declarative parameter helpers

Author: tomas-langer

common/mapper/src/main/java/io/helidon/common/mapper/Mappers.java

@@ -18,6 +18,7 @@
 
 import java.util.Optional;
 import java.util.function.Consumer;
+import java.util.function.Function;
 
 import io.helidon.builder.api.RuntimeType;
 import io.helidon.common.Api;
@@ -125,4 +126,30 @@ <SOURCE, TARGET> TARGET map(SOURCE source, Class<SOURCE> sourceType, Class<TARGE
     <SOURCE, TARGET> Optional<Mapper<SOURCE, TARGET>> mapper(GenericType<SOURCE> sourceType,
                                                              GenericType<TARGET> targetType,
                                                              String... qualifiers);
+
+    /**
+     * Map from source to target, also mapping exception to a custom exception type.
+     *
+     * @param source          object to map
+     * @param sourceType      type of the source object (to locate the mapper)
+     * @param targetType      type of the target object (to locate the mapper)
+     * @param exceptionMapper a function to map from {@link io.helidon.common.mapper.MapperException} to your type
+     * @param qualifiers      qualifiers of the usage (such as {@code http-headers, http}, most specific one first)
+     * @param <SOURCE>        type of the source
+     * @param <TARGET>        type of the target
+     * @param <T>             type of the exception produced by {@code exceptionMapper}
+     * @return result of the mapping
+     * @throws T if mapping fails and {@code exceptionMapper} produces an exception
+     */
+    default <SOURCE, TARGET, T extends Throwable> TARGET map(SOURCE source,
+                                                             GenericType<SOURCE> sourceType,
+                                                             GenericType<TARGET> targetType,
+                                                             Function<MapperException, T> exceptionMapper,
+                                                             String... qualifiers) throws T {
+        try {
+            return map(source, sourceType, targetType, qualifiers);
+        } catch (MapperException e) {
+            throw exceptionMapper.apply(e);
+        }
+    }
 }

common/mapper/src/main/java/io/helidon/common/mapper/MappersImpl.java

@@ -20,6 +20,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.BiFunction;
@@ -123,7 +124,9 @@ private <SOURCE, TARGET> Mapper<SOURCE, TARGET> findMapper(Class<SOURCE> sourceT
                                                                Class<TARGET> targetType,
                                                                boolean fromTypes,
                                                                String... qualifiers) {
-        Mapper<?, ?> mapper = classCache.computeIfAbsent(new ClassCacheKey(sourceType, targetType, List.of(qualifiers)), key -> {
+        Mapper<?, ?> mapper = classCache.computeIfAbsent(new ClassCacheKey(sourceType,
+                                                                           targetType,
+                                                                           new QualifiersWrapper(qualifiers)), key -> {
             // first attempt to find by classes
             return fromProviders(sourceType, targetType, qualifiers)
                     .orElseGet(() -> {
@@ -143,7 +146,9 @@ private <SOURCE, TARGET> Mapper<SOURCE, TARGET> findMapper(GenericType<SOURCE> s
                                                                GenericType<TARGET> targetType,
                                                                boolean fromClasses,
                                                                String... qualifiers) {
-        Mapper<?, ?> mapper = typeCache.computeIfAbsent(new GenericCacheKey(sourceType, targetType, List.of(qualifiers)), key -> {
+        Mapper<?, ?> mapper = typeCache.computeIfAbsent(new GenericCacheKey(sourceType,
+                                                                            targetType,
+                                                                            new QualifiersWrapper(qualifiers)), key -> {
             // first attempt to find by types
             return fromProviders(sourceType, targetType, qualifiers)
                     .orElseGet(() -> {
@@ -223,10 +228,33 @@ private <SOURCE, TARGET> Mapper<SOURCE, TARGET> findMapper(GenericType<SOURCE> s
                              qualifiers);
     }
 
-    private record GenericCacheKey(GenericType<?> sourceType, GenericType<?> targetType, List<String> qualifiers) {
+    // simple array wrapper that has equals and hashcode - faster than creating a List
+    private static class QualifiersWrapper {
+        private final String[] qualifiers;
+
+        private QualifiersWrapper(String[] qualifiers) {
+            // defensive copy, so if somebody modifies the original array, we do not get broken
+            this.qualifiers = Arrays.copyOf(qualifiers, qualifiers.length);
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (!(o instanceof QualifiersWrapper that)) {
+                return false;
+            }
+            return Objects.deepEquals(qualifiers, that.qualifiers);
+        }
+
+        @Override
+        public int hashCode() {
+            return Arrays.hashCode(qualifiers);
+        }
+    }
+
+    private record GenericCacheKey(GenericType<?> sourceType, GenericType<?> targetType, QualifiersWrapper qualifiers) {
     }
 
-    private record ClassCacheKey(Class<?> sourceType, Class<?> targetType, List<String> qualifiers) {
+    private record ClassCacheKey(Class<?> sourceType, Class<?> targetType, QualifiersWrapper qualifiers) {
     }
 
     @SuppressWarnings("rawtypes")

common/mapper/src/test/java/io/helidon/common/mapper/MappersTest.java

@@ -328,6 +328,38 @@ void testCacheWorks() {
         assertThat(mm.typeCacheSize(), is(1));
     }
 
+    @Test
+    void testQualifierArrayMutationDoesNotCorruptCache() {
+        CountingQualifiedProvider provider = new CountingQualifiedProvider();
+        MappersImpl mm = new MappersImpl(Mappers.builder()
+                                                 .mapperProvidersDiscoverServices(false)
+                                                 .useBuiltInMappers(false)
+                                                 .addMapperProvider(provider)
+                                                 .buildPrototype());
+
+        String[] classQualifiers = {"http", "query"};
+        assertThat(mm.map("value", String.class, String.class, classQualifiers), is("class_value"));
+        assertThat(mm.classCacheSize(), is(1));
+        int classCalls = provider.classCalls();
+
+        classQualifiers[1] = "header";
+
+        assertThat(mm.map("value", String.class, String.class, "http", "query"), is("class_value"));
+        assertThat(mm.classCacheSize(), is(1));
+        assertThat(provider.classCalls(), is(classCalls));
+
+        String[] genericQualifiers = {"http", "query"};
+        assertThat(mm.map("value", GenericType.STRING, GenericType.STRING, genericQualifiers), is("type_value"));
+        assertThat(mm.typeCacheSize(), is(1));
+        int typeCalls = provider.typeCalls();
+
+        genericQualifiers[1] = "header";
+
+        assertThat(mm.map("value", GenericType.STRING, GenericType.STRING, "http", "query"), is("type_value"));
+        assertThat(mm.typeCacheSize(), is(1));
+        assertThat(provider.typeCalls(), is(typeCalls));
+    }
+
     private static class TestProvider implements MapperProvider {
         @Override
         public ProviderResponse mapper(Class<?> sourceClass, Class<?> targetClass, String qualifier) {
@@ -340,4 +372,35 @@ public ProviderResponse mapper(GenericType<?> sourceType, GenericType<?> targetT
         }
     }
 
+    private static class CountingQualifiedProvider implements MapperProvider {
+        private int classCalls;
+        private int typeCalls;
+
+        @Override
+        public ProviderResponse mapper(Class<?> sourceClass, Class<?> targetClass, String qualifier) {
+            classCalls++;
+            if (String.class.equals(sourceClass) && String.class.equals(targetClass) && qualifier.equals("http/query")) {
+                return new ProviderResponse(Support.SUPPORTED, req -> "class_" + req);
+            }
+            return ProviderResponse.unsupported();
+        }
+
+        @Override
+        public ProviderResponse mapper(GenericType<?> sourceType, GenericType<?> targetType, String qualifier) {
+            typeCalls++;
+            if (GenericType.STRING.equals(sourceType) && GenericType.STRING.equals(targetType) && qualifier.equals("http/query")) {
+                return new ProviderResponse(Support.SUPPORTED, req -> "type_" + req);
+            }
+            return ProviderResponse.unsupported();
+        }
+
+        int classCalls() {
+            return classCalls;
+        }
+
+        int typeCalls() {
+            return typeCalls;
+        }
+    }
+
 }