Fixed error with negative periods. Using at least ldapCaching period to

sync with ldap

diff --git a/src/main/distrib/data/gitblit.properties b/src/main/distrib/data/gitblit.properties
index d1b0e4bd4917533cd2e561983fc2bbc1a1510291..bd0efd9e7fe5b3bd3a1234ff3e27ee3072e4378e 100644 (file)
--- a/src/main/distrib/data/gitblit.properties
+++ b/src/main/distrib/data/gitblit.properties
@@ -1512,7 +1512,7 @@ realm.ldap.synchronizeUsers.enable = false
 # only used for LDAP user synchronization.\r
 #\r
 # Must be of the form '<long> <TimeUnit>' where <TimeUnit> is one of 'MILLISECONDS', 'SECONDS', 'MINUTES', 'HOURS', 'DAYS' \r
-# <long> is at least 5 Minutes if lower values are given the default is used.\r
+# <long> is at least the value from realm.ldap.ldapCachePeriod if lower the value from realm.ldap.ldapCachePeriod is used.\r
 # default: 5 MINUTES\r
 #\r
 # RESTART REQUIRED\r

diff --git a/src/main/java/com/gitblit/auth/LdapAuthProvider.java b/src/main/java/com/gitblit/auth/LdapAuthProvider.java
index 58fb8219be01c1ba26ee06d34181e65519dd6643..1ec273a9d2722d53884d84856cc235a30197fed6 100644 (file)
--- a/src/main/java/com/gitblit/auth/LdapAuthProvider.java
+++ b/src/main/java/com/gitblit/auth/LdapAuthProvider.java
@@ -37,7 +37,6 @@ import com.gitblit.models.UserModel;
 import com.gitblit.service.LdapSyncService;
 import com.gitblit.utils.ArrayUtils;
 import com.gitblit.utils.StringUtils;
-import com.gitblit.utils.TimeUtils;
 import com.unboundid.ldap.sdk.Attribute;
 import com.unboundid.ldap.sdk.DereferencePolicy;
 import com.unboundid.ldap.sdk.ExtendedResult;
@@ -67,15 +66,15 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
                super("ldap");
        }
 
-       private long getSynchronizationPeriod() {
-        final String cacheDuration = settings.getString(Keys.realm.ldap.ldapCachePeriod, "2 MINUTES");
+       private long getSynchronizationPeriodInMilliseconds(String name) {
+        final String cacheDuration = settings.getString(name, "2 MINUTES");
         try {
             final String[] s = cacheDuration.split(" ", 2);
-            long duration = Long.parseLong(s[0]);
+            long duration = Math.abs(Long.parseLong(s[0]));
             TimeUnit timeUnit = TimeUnit.valueOf(s[1]);
             return timeUnit.toMillis(duration);
         } catch (RuntimeException ex) {
-            throw new IllegalArgumentException(Keys.realm.ldap.ldapCachePeriod + " must have format '<long> <TimeUnit>' where <TimeUnit> is one of 'MILLISECONDS', 'SECONDS', 'MINUTES', 'HOURS', 'DAYS'");
+            throw new IllegalArgumentException(name + " must have format '<long> <TimeUnit>' where <TimeUnit> is one of 'MILLISECONDS', 'SECONDS', 'MINUTES', 'HOURS', 'DAYS'");
         }
     }
 
@@ -92,7 +91,7 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
        protected synchronized void synchronizeLdapUsers() {
         final boolean enabled = settings.getBoolean(Keys.realm.ldap.synchronizeUsers.enable, false);
         if (enabled) {
-            if (System.currentTimeMillis() > (lastLdapUserSync.get() + getSynchronizationPeriod())) {
+            if (System.currentTimeMillis() > (lastLdapUserSync.get() + getSynchronizationPeriodInMilliseconds(Keys.realm.ldap.ldapCachePeriod))) {
                logger.info("Synchronizing with LDAP @ " + settings.getRequiredString(Keys.realm.ldap.server));
                 final boolean deleteRemovedLdapUsers = settings.getBoolean(Keys.realm.ldap.synchronizeUsers.removeDeleted, true);
                 LDAPConnection ldapConnection = getLdapConnection();
@@ -533,14 +532,15 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
                logger.info("Start configuring ldap sync service");
                LdapSyncService ldapSyncService = new LdapSyncService(settings, this);
                if (ldapSyncService.isReady()) {
-                       int mins = TimeUtils.convertFrequencyToMinutes(settings.getString(Keys.realm.ldap.synchronizeUsers.ldapSyncPeriod, "5 mins"));
-                       if (mins < 5) {
-                               mins = 5;
+                       long ldapSyncPeriod = getSynchronizationPeriodInMilliseconds(Keys.realm.ldap.synchronizeUsers.ldapSyncPeriod);
+                       long ldapCachePeriod = getSynchronizationPeriodInMilliseconds(Keys.realm.ldap.synchronizeUsers.ldapSyncPeriod);
+                       if (ldapSyncPeriod < ldapCachePeriod) {
+                               ldapSyncPeriod = ldapCachePeriod;
                        }
                        int delay = 1;
                        ScheduledExecutorService scheduledExecutorService = Executors.newSingleThreadScheduledExecutor();
-                       scheduledExecutorService.scheduleAtFixedRate(ldapSyncService, delay, mins,  TimeUnit.MINUTES);
-                       logger.info("Ldap sync service will update user and groups every {} minutes.", mins);
+                       scheduledExecutorService.scheduleAtFixedRate(ldapSyncService, delay, ldapSyncPeriod,  TimeUnit.MILLISECONDS);
+                       logger.info("Ldap sync service will update user and groups every {} minutes.", ldapSyncPeriod);
                        logger.info("Next scheduled ldap sync is in {} minutes", delay);
                } else {
                        logger.info("Ldap sync service is disabled.");