KEYCLOAK-4218

model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java

@@ -1845,6 +1845,7 @@ public class RealmAdapter implements RealmModel, JpaModel<RealmEntity> {
 
         ComponentEntity c = em.find(ComponentEntity.class, component.getId());
         if (c == null) return;
+        ComponentModel old = entityToModel(c);
         c.setName(component.getName());
         c.setProviderId(component.getProviderId());
         c.setProviderType(component.getProviderType());
@@ -1853,7 +1854,7 @@ public class RealmAdapter implements RealmModel, JpaModel<RealmEntity> {
         em.createNamedQuery("deleteComponentConfigByComponent").setParameter("component", c).executeUpdate();
         em.flush();
         setConfig(component, c);
-        ComponentUtil.notifyUpdated(session, this, component);
+        ComponentUtil.notifyUpdated(session, this, old, component);
 
 
     }

model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java

@@ -1748,14 +1748,17 @@ public class RealmAdapter extends AbstractMongoAdapter<MongoRealmEntity> impleme
     public void updateComponent(ComponentModel model) {
         ComponentUtil.getComponentFactory(session, model).validateConfiguration(session, this, model);
 
+        ComponentModel old = null;
         for (ComponentEntity entity : realm.getComponentEntities()) {
             if (entity.getId().equals(model.getId())) {
+                old = entityToModel(entity);
                 updateComponentEntity(entity, model);
-
+                break;
             }
         }
+        if (old == null) return; // wasn't updated
         updateRealm();
-        ComponentUtil.notifyUpdated(session, this, model);
+        ComponentUtil.notifyUpdated(session, this, old, model);
 
     }
 

server-spi-private/src/main/java/org/keycloak/models/utils/ComponentUtil.java

@@ -26,6 +26,7 @@ import org.keycloak.provider.ProviderConfigProperty;
 import org.keycloak.provider.ProviderFactory;
 import org.keycloak.representations.idm.ComponentRepresentation;
 import org.keycloak.storage.OnCreateComponent;
+import org.keycloak.storage.OnUpdateComponent;
 import org.keycloak.storage.UserStorageProviderFactory;
 
 import java.util.HashMap;
@@ -94,9 +95,12 @@ public class ComponentUtil {
             ((OnCreateComponent)session.userStorageManager()).onCreate(session, realm, model);
         }
     }
-    public static void notifyUpdated(KeycloakSession session, RealmModel realm, ComponentModel model) {
+    public static void notifyUpdated(KeycloakSession session, RealmModel realm, ComponentModel oldModel, ComponentModel newModel) {
-        ComponentFactory factory = getComponentFactory(session, model);
+        ComponentFactory factory = getComponentFactory(session, newModel);
-        factory.onUpdate(session, realm, model);
+        factory.onUpdate(session, realm, newModel);
+        if (factory instanceof UserStorageProviderFactory) {
+            ((OnUpdateComponent)session.userStorageManager()).onUpdate(session, realm, oldModel, newModel);
+        }
     }
 
 }

server-spi-private/src/main/java/org/keycloak/storage/OnUpdateComponent.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2016 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.storage;
+
+import org.keycloak.component.ComponentModel;
+import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.RealmModel;
+
+/**
+ * Callback for component update.  Only hardcoded classes like UserStorageManager implement it.  In future we
+ * may allow anybody to implement this interface.
+ *
+ * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
+ * @version $Revision: 1 $
+ */
+public interface OnUpdateComponent {
+    void onUpdate(KeycloakSession session, RealmModel realm, ComponentModel oldModel, ComponentModel newModel);
+}

services/src/main/java/org/keycloak/storage/UserStorageManager.java

@@ -60,7 +60,7 @@ import static org.keycloak.models.utils.KeycloakModelUtils.runJobInTransaction;
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
  * @version $Revision: 1 $
  */
-public class UserStorageManager implements UserProvider, OnUserCache, OnCreateComponent {
+public class UserStorageManager implements UserProvider, OnUserCache, OnCreateComponent, OnUpdateComponent {
 
     private static final Logger logger = Logger.getLogger(UserStorageManager.class);
 
@@ -640,6 +640,16 @@ public class UserStorageManager implements UserProvider, OnUserCache, OnCreateCo
 
     }
 
+    @Override
+    public void onUpdate(KeycloakSession session, RealmModel realm, ComponentModel oldModel, ComponentModel newModel) {
+        ComponentFactory factory = ComponentUtil.getComponentFactory(session, newModel);
+        if (!(factory instanceof UserStorageProviderFactory)) return;
+        UserStorageProviderModel old = new UserStorageProviderModel(oldModel);
+        UserStorageProviderModel newP= new UserStorageProviderModel(newModel);
+        if (old.getChangedSyncPeriod() != newP.getChangedSyncPeriod() || old.getFullSyncPeriod() != newP.getFullSyncPeriod()
+                || old.isImportEnabled() != newP.isImportEnabled()) {
+            new UserStorageSyncManager().notifyToRefreshPeriodicSync(session, realm, new UserStorageProviderModel(newModel), false);
+        }
 
-
+    }
 }

testsuite/integration/src/test/java/org/keycloak/testsuite/federation/sync/SyncFederationTest.java

@@ -61,9 +61,19 @@ public class SyncFederationTest {
     });
 
 
+    /**
+     * Test that period sync is triggered when creating a synchronized User Storage Provider
+     *
+     */
     @Test
-    public void test01PeriodicSync() {
+    public void test01PeriodicSyncOnCreate() {
 
+        KeycloakSession session = keycloakRule.startSession();
+        KeycloakSessionFactory sessionFactory = session.getKeycloakSessionFactory();
+        DummyUserFederationProviderFactory dummyFedFactory = (DummyUserFederationProviderFactory) sessionFactory.getProviderFactory(UserStorageProvider.class, DummyUserFederationProviderFactory.PROVIDER_NAME);
+        int full = dummyFedFactory.getFullSyncCounter();
+        int changed = dummyFedFactory.getChangedSyncCounter();
+        keycloakRule.stopSession(session, false);
         // Enable timer for SyncDummyUserFederationProvider
         keycloakRule.update(new KeycloakRule.KeycloakSetup() {
 
@@ -81,16 +91,11 @@ public class SyncFederationTest {
 
         });
 
-        KeycloakSession session = keycloakRule.startSession();
+        session = keycloakRule.startSession();
         try {
-            KeycloakSessionFactory sessionFactory = session.getKeycloakSessionFactory();
-            DummyUserFederationProviderFactory dummyFedFactory = (DummyUserFederationProviderFactory)sessionFactory.getProviderFactory(UserStorageProvider.class, DummyUserFederationProviderFactory.PROVIDER_NAME);
-            int full = dummyFedFactory.getFullSyncCounter();
-            int changed = dummyFedFactory.getChangedSyncCounter();
 
             // Assert that after some period was DummyUserFederationProvider triggered
             UserStorageSyncManager usersSyncManager = new UserStorageSyncManager();
-            usersSyncManager.bootstrapPeriodic(sessionFactory, session.getProvider(TimerProvider.class));
             sleep(1800);
 
             // Cancel timer
@@ -134,8 +139,114 @@ public class SyncFederationTest {
         });
     }
 
+    /**
+     * Test that period sync is triggered when updating a synchronized User Storage Provider to have a non-negative sync period
+     *
+     */
     @Test
-    public void test02ConcurrentSync() throws Exception {
+    public void test02PeriodicSyncOnUpdate() {
+
+        KeycloakSession session = keycloakRule.startSession();
+        KeycloakSessionFactory sessionFactory = session.getKeycloakSessionFactory();
+        DummyUserFederationProviderFactory dummyFedFactory = (DummyUserFederationProviderFactory) sessionFactory.getProviderFactory(UserStorageProvider.class, DummyUserFederationProviderFactory.PROVIDER_NAME);
+        int full = dummyFedFactory.getFullSyncCounter();
+        int changed = dummyFedFactory.getChangedSyncCounter();
+        keycloakRule.stopSession(session, false);
+        // Enable timer for SyncDummyUserFederationProvider
+        keycloakRule.update(new KeycloakRule.KeycloakSetup() {
+
+            @Override
+            public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                UserStorageProviderModel model = new UserStorageProviderModel();
+                model.setProviderId(DummyUserFederationProviderFactory.PROVIDER_NAME);
+                model.setPriority(1);
+                model.setName("test-sync-dummy");
+                model.setFullSyncPeriod(-1);
+                model.setChangedSyncPeriod(-1);
+                model.setLastSync(0);
+                dummyModel = new UserStorageProviderModel(appRealm.addComponentModel(model));
+            }
+
+        });
+
+        session = keycloakRule.startSession();
+        try {
+
+            // Assert that after some period was DummyUserFederationProvider triggered
+            UserStorageSyncManager usersSyncManager = new UserStorageSyncManager();
+            // Assert that dummy provider won't be invoked anymore
+            sleep(1800);
+            Assert.assertEquals(full, dummyFedFactory.getFullSyncCounter());
+            int newestChanged = dummyFedFactory.getChangedSyncCounter();
+            Assert.assertEquals("Assertion failed. newChanged=" + changed + ", newestChanged=" + newestChanged, changed, newestChanged);
+        } finally {
+            keycloakRule.stopSession(session, true);
+        }
+
+        keycloakRule.update(new KeycloakRule.KeycloakSetup() {
+
+            @Override
+            public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                dummyModel.setChangedSyncPeriod(1);
+                appRealm.updateComponent(dummyModel);
+            }
+
+        });
+
+
+        session = keycloakRule.startSession();
+        try {
+
+            // Assert that after some period was DummyUserFederationProvider triggered
+            UserStorageSyncManager usersSyncManager = new UserStorageSyncManager();
+            sleep(1800);
+
+            // Cancel timer
+            RealmModel appRealm = session.realms().getRealmByName("test");
+            usersSyncManager.notifyToRefreshPeriodicSync(session, appRealm, dummyModel, true);
+            log.infof("Notified sync manager about cancel periodic sync");
+
+            // This sync is here just to ensure that we have lock (doublecheck that periodic sync, which was possibly triggered before canceling timer is finished too)
+            while (true) {
+                SynchronizationResult result = usersSyncManager.syncChangedUsers(session.getKeycloakSessionFactory(), appRealm.getId(), dummyModel);
+                if (result.isIgnored()) {
+                    log.infof("Still waiting for lock before periodic sync is finished", result.toString());
+                    sleep(1000);
+                } else {
+                    break;
+                }
+            }
+
+            // Assert that DummyUserFederationProviderFactory.syncChangedUsers was invoked at least 2 times (once periodically and once for us)
+            int newChanged = dummyFedFactory.getChangedSyncCounter();
+            Assert.assertEquals(full, dummyFedFactory.getFullSyncCounter());
+            log.info("Asserting. newChanged=" + newChanged + " > changed=" + changed);
+            Assert.assertTrue("Assertion failed. newChanged=" + newChanged + ", changed=" + changed, newChanged > (changed + 1));
+
+            // Assert that dummy provider won't be invoked anymore
+            sleep(1800);
+            Assert.assertEquals(full, dummyFedFactory.getFullSyncCounter());
+            int newestChanged = dummyFedFactory.getChangedSyncCounter();
+            Assert.assertEquals("Assertion failed. newChanged=" + newChanged + ", newestChanged=" + newestChanged, newChanged, newestChanged);
+        } finally {
+            keycloakRule.stopSession(session, true);
+        }
+
+
+        // remove dummyProvider
+        keycloakRule.update(new KeycloakRule.KeycloakSetup() {
+
+            @Override
+            public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                appRealm.removeComponent(dummyModel);
+            }
+
+        });
+    }
+
+
+    @Test
+    public void test03ConcurrentSync() throws Exception {
         SyncDummyUserFederationProviderFactory.restartLatches();
 
         // Enable timer for SyncDummyUserFederationProvider