KEYCLOAK-8176 fix export issue for required action

keycloak · Sep 5, 2018 · 8761819 · 8761819
1 parent 47066e1
commit 8761819
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 22 deletions.
diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java
@@ -44,6 +44,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.function.Consumer;
 
 /**
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
@@ -410,29 +411,11 @@ public int compare(AuthenticatorConfigModel left, AuthenticatorConfigModel right
 
     public static void exportRequiredActions(RealmModel realm, RealmRepresentation rep) {
 
-        rep.setRequiredActions(new LinkedList<RequiredActionProviderRepresentation>());
+        rep.setRequiredActions(new LinkedList<>());
 
-        List<RequiredActionProviderModel> requiredActionProviders = realm.getRequiredActionProviders();
-        List<RequiredActionProviderModel> copy = new LinkedList<>();
-        copy.addAll(requiredActionProviders);
-        requiredActionProviders = copy;
-        //ensure consistent ordering of requiredActionProviders.
-        Collections.sort(requiredActionProviders, new Comparator<RequiredActionProviderModel>() {
-            @Override
-            public int compare(RequiredActionProviderModel left, RequiredActionProviderModel right) {
-                String l = left.getAlias() != null ? left.getAlias() : "\0";
-                String r = right.getAlias() != null ? right.getAlias() : "\0";
-                return l.compareTo(r);
-            }
-        });
-
-        for (RequiredActionProviderModel model : requiredActionProviders) {
-            RequiredActionProviderRepresentation action = toRepresentation(model);
-            rep.getRequiredActions().add(action);
-        }
+        realm.getRequiredActionProviders().forEach(action -> rep.getRequiredActions().add(toRepresentation(action)));
     }
 
-
     public static RealmEventsConfigRepresentation toEventsConfigReprensetation(RealmModel realm) {
         RealmEventsConfigRepresentation rep = new RealmEventsConfigRepresentation();
         rep.setEventsEnabled(realm.isEventsEnabled());
@@ -694,6 +677,7 @@ public static RequiredActionProviderRepresentation toRepresentation(RequiredActi
         rep.setConfig(model.getConfig());
         rep.setName(model.getName());
         rep.setProviderId(model.getProviderId());
+        rep.setPriority(model.getPriority());
         return rep;
     }
 

diff --git a/server-spi/src/main/java/org/keycloak/models/RequiredActionProviderModel.java b/server-spi/src/main/java/org/keycloak/models/RequiredActionProviderModel.java
@@ -33,7 +33,11 @@ public static class RequiredActionComparator implements Comparator<RequiredActio
 
         @Override
         public int compare(RequiredActionProviderModel o1, RequiredActionProviderModel o2) {
-            return o1.priority - o2.priority;
+
+            return Comparator
+                    .comparingInt(RequiredActionProviderModel::getPriority)
+                    .thenComparing(RequiredActionProviderModel::getName, Comparator.nullsFirst(String.CASE_INSENSITIVE_ORDER))
+                    .compare(o1, o2);
         }
     }
 
@@ -44,7 +48,7 @@ public int compare(RequiredActionProviderModel o1, RequiredActionProviderModel o
     private boolean enabled;
     private boolean defaultAction;
     private int priority;
-    private Map<String, String> config = new HashMap<String, String>();
+    private Map<String, String> config = new HashMap<>();
 
 
     public String getId() {

diff --git a/...illian/tests/base/src/test/java/org/keycloak/testsuite/exportimport/ExportImportTest.java b/...illian/tests/base/src/test/java/org/keycloak/testsuite/exportimport/ExportImportTest.java
@@ -231,6 +231,12 @@ private void testFullExportImport() throws LifecycleException {
         removeRealm("test-realm");
         Assert.assertNames(adminClient.realms().findAll(), "master");
 
+        Map<String, RequiredActionProviderRepresentation> requiredActionsBeforeImport = new HashMap<>();
+        adminClient.realm("master").flows().getRequiredActions().stream()
+                .forEach(action -> {
+                    requiredActionsBeforeImport.put(action.getAlias(), action);
+                });
+
         assertNotAuthenticated("test", "test-user@localhost", "password");
         assertNotAuthenticated("test", "user1", "password");
         assertNotAuthenticated("test", "user2", "password");
@@ -251,6 +257,17 @@ private void testFullExportImport() throws LifecycleException {
 
         // KEYCLOAK-6050 Check SMTP password is exported/imported
         assertEquals("secret", testingClient.server("test").fetch(RunHelpers.internalRealm()).getSmtpServer().get("password"));
+
+        // KEYCLOAK-8176 Check required actions are exported/imported properly
+        List<RequiredActionProviderRepresentation> requiredActionsAfterImport = adminClient.realm("master").flows().getRequiredActions();
+        assertThat(requiredActionsAfterImport.size(), is(equalTo(requiredActionsBeforeImport.size())));
+        requiredActionsAfterImport.stream()
+                .forEach((action) -> {
+                    RequiredActionProviderRepresentation beforeImportAction = requiredActionsBeforeImport.get(action.getAlias());
+                    assertThat(action.getName(), is(equalTo(beforeImportAction.getName())));
+                    assertThat(action.getProviderId(), is(equalTo(beforeImportAction.getProviderId())));
+                    assertThat(action.getPriority(), is(equalTo(beforeImportAction.getPriority())));
+                });
     }
 
     private void testRealmExportImport() throws LifecycleException {