Make @EnableFeature to handle the case with added provider of current…

…ly non-used SPI closes #36425 Signed-off-by: mposolda <[email protected]>
keycloak · Jan 15, 2025 · 0332319 · 0332319
1 parent fd9db3a
commit 0332319
Show file tree

Hide file tree

Showing 6 changed files with 102 additions and 5 deletions.
diff --git a/services/src/main/java/org/keycloak/provider/ProviderManager.java b/services/src/main/java/org/keycloak/provider/ProviderManager.java
@@ -20,6 +20,7 @@
 import org.keycloak.common.util.MultivaluedHashMap;
 import org.keycloak.services.DefaultKeycloakSessionFactory;
 
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedList;
@@ -48,8 +49,7 @@ public ProviderManager(KeycloakDeploymentInfo info, ClassLoader baseClassLoader,
 
         logger.debugv("Provider loaders {0}", factories);
 
-        loaders.add(new DefaultProviderLoader(info, baseClassLoader));
-        loaders.add(new DeploymentProviderLoader(info));
+        addDefaultLoaders(baseClassLoader);
 
         if (resources != null) {
             for (String r : resources) {
@@ -71,6 +71,20 @@ public ProviderManager(KeycloakDeploymentInfo info, ClassLoader baseClassLoader,
             }
         }
     }
+
+    public ProviderManager(KeycloakDeploymentInfo info, ClassLoader baseClassLoader, Collection<ProviderLoader> additionalProviderLoaders) {
+        this.info = info;
+        addDefaultLoaders(baseClassLoader);
+        if (additionalProviderLoaders != null) {
+            loaders.addAll(additionalProviderLoaders);
+        }
+    }
+
+    private void addDefaultLoaders(ClassLoader baseClassLoader) {
+        loaders.add(new DefaultProviderLoader(info, baseClassLoader));
+        loaders.add(new DeploymentProviderLoader(info));
+    }
+
     public synchronized List<Spi> loadSpis() {
         // Use a map to prevent duplicates, since the loaders may have overlapping classpaths.
         Map<String, Spi> spiMap = new HashMap<>();

diff --git a/services/src/main/java/org/keycloak/services/DefaultKeycloakSessionFactory.java b/services/src/main/java/org/keycloak/services/DefaultKeycloakSessionFactory.java
@@ -31,6 +31,7 @@
 import java.util.Stack;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.Function;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import org.jboss.logging.Logger;
@@ -181,6 +182,8 @@ protected Map<Class<? extends Provider>, Map<String, ProviderFactory>> getFactor
 
     @Override
     public void deploy(ProviderManager pm) {
+        registerNewSpis(pm);
+
         Map<Class<? extends Provider>, Map<String, ProviderFactory>> copy = getFactoriesCopy();
         Map<Class<? extends Provider>, Map<String, ProviderFactory>> newFactories = loadFactories(pm);
         Map<Class<? extends Provider>, Map<String, ProviderFactory>> deployed = new HashMap<>();
@@ -223,6 +226,20 @@ public void deploy(ProviderManager pm) {
         }
     }
 
+    // Register SPIs of this providerManager, which are possibly not yet registered in this factory
+    private void registerNewSpis(ProviderManager pm) {
+        Set<String> existingSpiNames = this.spis.stream()
+                .map(spi -> spi.getName())
+                .collect(Collectors.toSet());
+
+        this.spis = new HashSet<>(this.spis);
+        for (Spi newSpi : pm.loadSpis()) {
+            if (!existingSpiNames.contains(newSpi.getName())) {
+                this.spis.add(newSpi);
+            }
+        }
+    }
+
     @Override
     public void undeploy(ProviderManager pm) {
         logger.debug("undeploy");

diff --git a/...es/testsuite-providers/src/main/java/org/keycloak/testsuite/util/FeatureDeployerUtil.java b/...es/testsuite-providers/src/main/java/org/keycloak/testsuite/util/FeatureDeployerUtil.java
@@ -30,6 +30,7 @@
 import org.keycloak.provider.Spi;
 import org.keycloak.services.DefaultKeycloakSession;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -72,7 +73,7 @@ public static void deployFactoriesAfterFeatureEnabled(Profile.Feature feature) {
 
             KeycloakDeploymentInfo di = createDeploymentInfo(factories);
 
-            manager = new ProviderManager(di, FeatureDeployerUtil.class.getClassLoader());
+            manager = new ProviderManager(di, FeatureDeployerUtil.class.getClassLoader(), Collections.singleton(new TestsuiteProviderLoader(di)));
             deployersCache.put(feature, manager);
         }
         ProviderManagerRegistry.SINGLETON.deploy(manager);

diff --git a/...estsuite-providers/src/main/java/org/keycloak/testsuite/util/TestsuiteProviderLoader.java b/...estsuite-providers/src/main/java/org/keycloak/testsuite/util/TestsuiteProviderLoader.java
@@ -0,0 +1,65 @@
+/*
+ * Copyright 2025 Red Hat, Inc. and/or its affiliates
+ *  and other contributors as indicated by the @author tags.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+
+package org.keycloak.testsuite.util;
+
+import java.lang.reflect.InvocationTargetException;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.keycloak.provider.KeycloakDeploymentInfo;
+import org.keycloak.provider.ProviderFactory;
+import org.keycloak.provider.ProviderLoader;
+import org.keycloak.provider.Spi;
+
+/**
+ * Loads additional SPIs from provided KeycloakDeploymentInfo
+ *
+ * @author <a href="mailto:[email protected]">Marek Posolda</a>
+ */
+class TestsuiteProviderLoader implements ProviderLoader {
+
+    private final KeycloakDeploymentInfo info;
+
+    TestsuiteProviderLoader(KeycloakDeploymentInfo info) {
+        this.info = info;
+    }
+
+    @Override
+    public List<Spi> loadSpis() {
+        return info.getProviders().keySet()
+                .stream()
+                .map(this::instantiateSpi)
+                .collect(Collectors.toList());
+    }
+
+    private Spi instantiateSpi(Class<? extends Spi> clazz) {
+        try {
+            return clazz.getDeclaredConstructor().newInstance();
+        } catch (NoSuchMethodException | IllegalAccessException | InstantiationException | InvocationTargetException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+
+    @Override
+    public List<ProviderFactory> load(Spi spi) {
+        return List.of();
+    }
+}
diff --git a/...on-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientTypesTest.java b/...on-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientTypesTest.java
@@ -58,7 +58,7 @@
 /**
  * @author <a href="mailto:[email protected]">Marek Posolda</a>
  */
-@EnableFeature(value = CLIENT_TYPES)
+@EnableFeature(value = CLIENT_TYPES, skipRestart = true)
 public class ClientTypesTest extends AbstractTestRealmKeycloakTest {
 
     @Override

diff --git a/...n/tests/base/src/test/java/org/keycloak/testsuite/oid4vc/issuance/signing/OID4VCTest.java b/...n/tests/base/src/test/java/org/keycloak/testsuite/oid4vc/issuance/signing/OID4VCTest.java
@@ -62,7 +62,7 @@
 /**
  * Super class for all OID4VC tests. Provides convenience methods to ease the testing.
  */
-@EnableFeature(value = Profile.Feature.OID4VC_VCI, skipRestart = false)
+@EnableFeature(value = Profile.Feature.OID4VC_VCI, skipRestart = true)
 public abstract class OID4VCTest extends AbstractTestRealmKeycloakTest {
 
     private static final Logger LOGGER = Logger.getLogger(OID4VCTest.class);