From 862bebb8362e8f618c43e78730edca593ff81999 Mon Sep 17 00:00:00 2001
From: Foivos Zakkak <fzakkak@redhat.com>
Date: Tue, 23 Jul 2024 13:04:39 +0300
Subject: [PATCH] Improve the way we register providers for reflection

Instead of registering all constructors we only need to register the
nullary constructor.

Furthermore, we need to register the `provider()` method for reflective
lookup to avoid `MissingReflectionRegistrationError`s at run-time when
using `-H:+ThrowMissingRegistrationErrors` or
`--exact-reachability-metadata`.

The said constructor and method are accessed in
ServiceLoader#loadProvider and ServiceLoader#findStaticProviderMethod.

Relates to https://github.com/quarkusio/quarkus/issues/41995
---
 .../ReflectiveMethodBuildItem.java            | 27 +++++++++
 .../steps/NativeImageReflectConfigStep.java   | 59 +++++++++++--------
 2 files changed, 63 insertions(+), 23 deletions(-)

diff --git a/core/deployment/src/main/java/io/quarkus/deployment/builditem/nativeimage/ReflectiveMethodBuildItem.java b/core/deployment/src/main/java/io/quarkus/deployment/builditem/nativeimage/ReflectiveMethodBuildItem.java
index 2a1f7515548db..395488ba8df89 100644
--- a/core/deployment/src/main/java/io/quarkus/deployment/builditem/nativeimage/ReflectiveMethodBuildItem.java
+++ b/core/deployment/src/main/java/io/quarkus/deployment/builditem/nativeimage/ReflectiveMethodBuildItem.java
@@ -13,8 +13,13 @@ public final class ReflectiveMethodBuildItem extends MultiBuildItem {
     final String declaringClass;
     final String name;
     final String[] params;
+    final boolean queryOnly;
 
     public ReflectiveMethodBuildItem(MethodInfo methodInfo) {
+        this(false, methodInfo);
+    }
+
+    public ReflectiveMethodBuildItem(boolean queryOnly, MethodInfo methodInfo) {
         String[] params = new String[methodInfo.parametersCount()];
         for (int i = 0; i < params.length; ++i) {
             params[i] = methodInfo.parameterType(i).name().toString();
@@ -22,9 +27,14 @@ public ReflectiveMethodBuildItem(MethodInfo methodInfo) {
         this.name = methodInfo.name();
         this.params = params;
         this.declaringClass = methodInfo.declaringClass().name().toString();
+        this.queryOnly = queryOnly;
     }
 
     public ReflectiveMethodBuildItem(Method method) {
+        this(false, method);
+    }
+
+    public ReflectiveMethodBuildItem(boolean queryOnly, Method method) {
         this.params = new String[method.getParameterCount()];
         if (method.getParameterCount() > 0) {
             Class<?>[] parameterTypes = method.getParameterTypes();
@@ -34,23 +44,36 @@ public ReflectiveMethodBuildItem(Method method) {
         }
         this.name = method.getName();
         this.declaringClass = method.getDeclaringClass().getName();
+        this.queryOnly = queryOnly;
     }
 
     public ReflectiveMethodBuildItem(String declaringClass, String name,
             String... params) {
+        this(false, declaringClass, name, params);
+    }
+
+    public ReflectiveMethodBuildItem(boolean queryOnly, String declaringClass, String name,
+            String... params) {
         this.declaringClass = declaringClass;
         this.name = name;
         this.params = params;
+        this.queryOnly = queryOnly;
     }
 
     public ReflectiveMethodBuildItem(String declaringClass, String name,
             Class<?>... params) {
+        this(false, declaringClass, name, params);
+    }
+
+    public ReflectiveMethodBuildItem(boolean queryOnly, String declaringClass, String name,
+            Class<?>... params) {
         this.declaringClass = declaringClass;
         this.name = name;
         this.params = new String[params.length];
         for (int i = 0; i < params.length; ++i) {
             this.params[i] = params[i].getName();
         }
+        this.queryOnly = queryOnly;
     }
 
     public String getName() {
@@ -65,6 +88,10 @@ public String getDeclaringClass() {
         return declaringClass;
     }
 
+    public boolean isQueryOnly() {
+        return queryOnly;
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o)
diff --git a/core/deployment/src/main/java/io/quarkus/deployment/steps/NativeImageReflectConfigStep.java b/core/deployment/src/main/java/io/quarkus/deployment/steps/NativeImageReflectConfigStep.java
index 5800346551893..27b489aea3394 100644
--- a/core/deployment/src/main/java/io/quarkus/deployment/steps/NativeImageReflectConfigStep.java
+++ b/core/deployment/src/main/java/io/quarkus/deployment/steps/NativeImageReflectConfigStep.java
@@ -52,8 +52,14 @@ void generateReflectConfig(BuildProducer<GeneratedResourceBuildItem> reflectConf
         }
 
         for (ServiceProviderBuildItem i : serviceProviderBuildItems) {
-            addReflectiveClass(reflectiveClasses, forcedNonWeakClasses, true, false, false, false, false, false, false, false,
-                    i.providers().toArray(new String[] {}));
+            for (String provider : i.providers()) {
+                // Register the nullary constructor
+                addReflectiveMethod(reflectiveClasses, new ReflectiveMethodBuildItem(provider, "<init>", new String[0]));
+                // Register public provider() method for lookkup to avoid throwing a MissingReflectionRegistrationError at run time.
+                // See ServiceLoader#loadProvider and ServiceLoader#findStaticProviderMethod.
+                addReflectiveMethod(reflectiveClasses,
+                        new ReflectiveMethodBuildItem(true, provider, "provider", new String[0]));
+            }
         }
 
         // Perform this as last step, since it augments the already added reflective classes
@@ -72,6 +78,7 @@ void generateReflectConfig(BuildProducer<GeneratedResourceBuildItem> reflectConf
 
             ReflectionInfo info = entry.getValue();
             JsonArrayBuilder methodsArray = Json.array();
+            JsonArrayBuilder queriedMethodsArray = Json.array();
             if (info.typeReachable != null) {
                 json.put("condition", Json.object().put("typeReachable", info.typeReachable));
             }
@@ -82,16 +89,7 @@ void generateReflectConfig(BuildProducer<GeneratedResourceBuildItem> reflectConf
                     json.put("queryAllDeclaredConstructors", true);
                 }
                 if (!info.ctorSet.isEmpty()) {
-                    for (ReflectiveMethodBuildItem ctor : info.ctorSet) {
-                        JsonObjectBuilder methodObject = Json.object();
-                        methodObject.put("name", ctor.getName());
-                        JsonArrayBuilder paramsArray = Json.array();
-                        for (int i = 0; i < ctor.getParams().length; ++i) {
-                            paramsArray.add(ctor.getParams()[i]);
-                        }
-                        methodObject.put("parameterTypes", paramsArray);
-                        methodsArray.add(methodObject);
-                    }
+                    extractToJsonArray(info.ctorSet, methodsArray);
                 }
             }
             if (info.methods) {
@@ -101,21 +99,18 @@ void generateReflectConfig(BuildProducer<GeneratedResourceBuildItem> reflectConf
                     json.put("queryAllDeclaredMethods", true);
                 }
                 if (!info.methodSet.isEmpty()) {
-                    for (ReflectiveMethodBuildItem method : info.methodSet) {
-                        JsonObjectBuilder methodObject = Json.object();
-                        methodObject.put("name", method.getName());
-                        JsonArrayBuilder paramsArray = Json.array();
-                        for (int i = 0; i < method.getParams().length; ++i) {
-                            paramsArray.add(method.getParams()[i]);
-                        }
-                        methodObject.put("parameterTypes", paramsArray);
-                        methodsArray.add(methodObject);
-                    }
+                    extractToJsonArray(info.methodSet, methodsArray);
+                }
+                if (!info.queriedMethodSet.isEmpty()) {
+                    extractToJsonArray(info.queriedMethodSet, queriedMethodsArray);
                 }
             }
             if (!methodsArray.isEmpty()) {
                 json.put("methods", methodsArray);
             }
+            if (!queriedMethodsArray.isEmpty()) {
+                json.put("queriedMethods", queriedMethodsArray);
+            }
 
             if (info.fields) {
                 json.put("allDeclaredFields", true);
@@ -142,6 +137,19 @@ void generateReflectConfig(BuildProducer<GeneratedResourceBuildItem> reflectConf
         }
     }
 
+    private static void extractToJsonArray(Set<ReflectiveMethodBuildItem> methodSet, JsonArrayBuilder methodsArray) {
+        for (ReflectiveMethodBuildItem method : methodSet) {
+            JsonObjectBuilder methodObject = Json.object();
+            methodObject.put("name", method.getName());
+            JsonArrayBuilder paramsArray = Json.array();
+            for (int i = 0; i < method.getParams().length; ++i) {
+                paramsArray.add(method.getParams()[i]);
+            }
+            methodObject.put("parameterTypes", paramsArray);
+            methodsArray.add(methodObject);
+        }
+    }
+
     public void addReflectiveMethod(Map<String, ReflectionInfo> reflectiveClasses, ReflectiveMethodBuildItem methodInfo) {
         String cl = methodInfo.getDeclaringClass();
         ReflectionInfo existing = reflectiveClasses.get(cl);
@@ -151,7 +159,11 @@ public void addReflectiveMethod(Map<String, ReflectionInfo> reflectiveClasses, R
         if (methodInfo.getName().equals("<init>")) {
             existing.ctorSet.add(methodInfo);
         } else {
-            existing.methodSet.add(methodInfo);
+            if (methodInfo.isQueryOnly()) {
+                existing.queriedMethodSet.add(methodInfo);
+            } else {
+                existing.methodSet.add(methodInfo);
+            }
         }
     }
 
@@ -211,6 +223,7 @@ static final class ReflectionInfo {
         String typeReachable;
         Set<String> fieldSet = new HashSet<>();
         Set<ReflectiveMethodBuildItem> methodSet = new HashSet<>();
+        Set<ReflectiveMethodBuildItem> queriedMethodSet = new HashSet<>();
         Set<ReflectiveMethodBuildItem> ctorSet = new HashSet<>();
 
         private ReflectionInfo() {