Throw a ServiceConfigurationError if the service is not JCA-compliant.

oracle · Dec 17, 2024 · 2b504d0 · 2b504d0
1 parent 1cdb5aa
commit 2b504d0
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 56 deletions.
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ServiceLoaderFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ServiceLoaderFeature.java
@@ -41,8 +41,8 @@
 import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
 import com.oracle.svm.core.feature.InternalFeature;
 import com.oracle.svm.core.jdk.ServiceCatalogSupport;
-import com.oracle.svm.core.option.HostedOptionKey;
 import com.oracle.svm.core.option.AccumulatingLocatableMultiOptionValue;
+import com.oracle.svm.core.option.HostedOptionKey;
 import com.oracle.svm.hosted.analysis.Inflation;
 
 import jdk.graal.compiler.options.Option;
@@ -219,6 +219,8 @@ void handleServiceClassIsReachable(DuringAnalysisAccess access, Class<?> service
                 RuntimeReflection.register(providerClass);
                 if (nullaryConstructor != null) {
                     RuntimeReflection.register(nullaryConstructor);
+                } else {
+                    RuntimeReflection.registerConstructorLookup(providerClass);
                 }
                 if (nullaryProviderMethod != null) {
                     RuntimeReflection.register(nullaryProviderMethod);
@@ -229,8 +231,14 @@ void handleServiceClassIsReachable(DuringAnalysisAccess access, Class<?> service
                      */
                     RuntimeReflection.registerMethodLookup(providerClass, "provider");
                 }
-                registeredProviders.add(provider);
             }
+            /*
+             * Register the provider in both cases: when it is JCA-compliant (has a nullary
+             * constructor or a provider method) or when it lacks both. If neither is present, a
+             * ServiceConfigurationError will be thrown at runtime, consistent with HotSpot
+             * behavior.
+             */
+            registeredProviders.add(provider);
         }
         if (!registeredProviders.isEmpty()) {
             String serviceResourceLocation = "META-INF/services/" + serviceProvider.getName();

diff --git a/...c/com.oracle.svm.test/src/com/oracle/svm/test/NoProviderConstructorServiceLoaderTest.java b/...c/com.oracle.svm.test/src/com/oracle/svm/test/NoProviderConstructorServiceLoaderTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -40,12 +40,8 @@
 // Checkstyle: allow Class.getSimpleName
 
 /**
- * Tests a workaround for {@linkplain ServiceLoader services} without a {@linkplain ServiceLoader
- * provider constructor} (nullary constructor) [GR-19958]. The workaround completely ignores
- * services without a provider constructor, instead of throwing an {@link ServiceConfigurationError}
- * when iterating the services. See the Github issue
- * <a href="https://github.com/oracle/graal/issues/2652">"Spring Service Registry native-image
- * failure due to service loader handling in jersey #2652"</a> for more details.
+ * Test both JCA-compliant services and non-JCA-compliant services (without a nullary constructor),
+ * and compare the behavior between Native Image and Hotspot.
  */
 public class NoProviderConstructorServiceLoaderTest {
 
@@ -73,81 +69,77 @@ public abstract static class NoProviderConstructorService implements ServiceInte
 
     private static final Set<String> EXPECTED = Set.of(ProperService.class.getSimpleName());
 
-    /**
-     * This should actually throw an {@link ServiceConfigurationError}.
-     *
-     * @see #testLazyStreamHotspot()
-     */
-    @Test
+    @Test(expected = ServiceConfigurationError.class)
     public void testLazyStreamNativeImage() {
-        Assume.assumeTrue("native image specific behavior", ImageInfo.inImageRuntimeCode());
-        Set<String> simpleNames = ServiceLoader.load(ServiceInterface.class).stream()
-                        .map(provider -> provider.type().getSimpleName())
-                        .collect(Collectors.toSet());
+        assumeEnvironment(true);
+        Set<String> simpleNames = loadLazyStreamNames();
         Assert.assertEquals(EXPECTED, simpleNames);
     }
 
-    /**
-     * This should actually throw an {@link ServiceConfigurationError}.
-     *
-     * @see #testEagerStreamHotspot()
-     */
-    @Test
+    @Test(expected = ServiceConfigurationError.class)
     public void testEagerStreamNativeImage() {
-        Assume.assumeTrue("native image specific behavior", ImageInfo.inImageRuntimeCode());
-        Set<String> simpleNames = ServiceLoader.load(ServiceInterface.class).stream()
-                        .map(provider -> provider.get().getClass().getSimpleName())
-                        .collect(Collectors.toSet());
+        assumeEnvironment(true);
+        Set<String> simpleNames = loadEagerStreamNames();
         Assert.assertEquals(EXPECTED, simpleNames);
     }
 
-    /**
-     * This should actually throw an {@link ServiceConfigurationError}.
-     *
-     * @see #testEagerIteratorHotspot()
-     */
-    @Test
+    @Test(expected = ServiceConfigurationError.class)
     public void testEagerIteratorNativeImage() {
-        Assume.assumeTrue("native image specific behavior", ImageInfo.inImageRuntimeCode());
-        Set<String> simpleNames = new HashSet<>();
-        ServiceLoader.load(ServiceInterface.class).iterator()
-                        .forEachRemaining(s -> simpleNames.add(s.getClass().getSimpleName()));
+        assumeEnvironment(true);
+        Set<String> simpleNames = loadEagerIteratorNames();
         Assert.assertEquals(EXPECTED, simpleNames);
     }
 
-    /**
-     * @see #testLazyStreamNativeImage()
-     */
     @Test(expected = ServiceConfigurationError.class)
     public void testLazyStreamHotspot() {
-        Assume.assumeFalse("hotspot specific behavior", ImageInfo.inImageRuntimeCode());
-        Set<String> simpleNames = ServiceLoader.load(ServiceInterface.class).stream()
+        assumeEnvironment(false);
+        Set<String> simpleNames = loadLazyStreamNames();
+        Assert.assertNull("should not reach", simpleNames);
+    }
+
+    @Test(expected = ServiceConfigurationError.class)
+    public void testEagerIteratorHotspot() {
+        assumeEnvironment(false);
+        Set<String> simpleNames = loadEagerIteratorNames();
+        Assert.assertNull("should not reach", simpleNames);
+    }
+
+    /**
+     * Helper method to assume the environment (hotspot/native image).
+     */
+    private void assumeEnvironment(boolean isNativeImage) {
+        if (isNativeImage) {
+            Assume.assumeTrue("native image specific behavior", ImageInfo.inImageRuntimeCode());
+        } else {
+            Assume.assumeFalse("hotspot specific behavior", ImageInfo.inImageRuntimeCode());
+        }
+    }
+
+    /**
+     * Helper method for lazy stream tests.
+     */
+    private Set<String> loadLazyStreamNames() {
+        return ServiceLoader.load(ServiceInterface.class).stream()
                         .map(provider -> provider.type().getSimpleName())
                         .collect(Collectors.toSet());
-        Assert.assertNull("should not reach", simpleNames);
     }
 
     /**
-     * @see #testEagerStreamNativeImage()
+     * Helper method for eager stream tests.
      */
-    @Test(expected = ServiceConfigurationError.class)
-    public void testEagerStreamHotspot() {
-        Assume.assumeFalse("hotspot specific behavior", ImageInfo.inImageRuntimeCode());
-        Set<String> simpleNames = ServiceLoader.load(ServiceInterface.class).stream()
+    private Set<String> loadEagerStreamNames() {
+        return ServiceLoader.load(ServiceInterface.class).stream()
                         .map(provider -> provider.get().getClass().getSimpleName())
                         .collect(Collectors.toSet());
-        Assert.assertNull("should not reach", simpleNames);
     }
 
     /**
-     * @see #testEagerIteratorNativeImage()
+     * Helper method for eager iterator tests.
      */
-    @Test(expected = ServiceConfigurationError.class)
-    public void testEagerIteratorHotspot() {
-        Assume.assumeFalse("hotspot specific behavior", ImageInfo.inImageRuntimeCode());
+    private Set<String> loadEagerIteratorNames() {
         Set<String> simpleNames = new HashSet<>();
         ServiceLoader.load(ServiceInterface.class).iterator()
                         .forEachRemaining(s -> simpleNames.add(s.getClass().getSimpleName()));
-        Assert.assertNull("should not reach", simpleNames);
+        return simpleNames;
     }
 }