From edae533cd47384c155485af74ef968453de69b9e Mon Sep 17 00:00:00 2001
From: Alfred Schmid <A.Schmid@ff-muenchen.de>
Date: Fri, 31 Jan 2014 14:19:58 +0100
Subject: [PATCH] Fixed error with negative periods. Using at least ldapCaching
 period to sync with ldap

---
 src/main/distrib/data/gitblit.properties      |  2 +-
 .../com/gitblit/auth/LdapAuthProvider.java    | 22 +++++++++----------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/main/distrib/data/gitblit.properties b/src/main/distrib/data/gitblit.properties
index d1b0e4bd..bd0efd9e 100644
--- 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.
 #
 # Must be of the form '<long> <TimeUnit>' where <TimeUnit> is one of 'MILLISECONDS', 'SECONDS', 'MINUTES', 'HOURS', 'DAYS' 
-# <long> is at least 5 Minutes if lower values are given the default is used.
+# <long> is at least the value from realm.ldap.ldapCachePeriod if lower the value from realm.ldap.ldapCachePeriod is used.
 # default: 5 MINUTES
 #
 # RESTART REQUIRED
diff --git a/src/main/java/com/gitblit/auth/LdapAuthProvider.java b/src/main/java/com/gitblit/auth/LdapAuthProvider.java
index 58fb8219..1ec273a9 100644
--- 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.");
-- 
2.39.5