Move method to User

3 files changed, 14 insertions, 14 deletions

diff --git a/lib/gitlab/exclusive_lease.rb b/lib/gitlab/exclusive_lease.rb
index f801e8b60b3..0ed4c357926 100644
--- a/lib/gitlab/exclusive_lease.rb
+++ b/lib/gitlab/exclusive_lease.rb
@@ -8,14 +8,25 @@ module Gitlab
   # servers. It is a 'cheap' alternative to using SQL queries and updates:
   # you do not need to change the SQL schema to start using
   # ExclusiveLease.
+  #
+  # It is important to choose the timeout wisely. If the timeout is very
+  # high (1 hour) then the throughput of your operation gets very low (at
+  # most once an hour). If the timeout is lower than how long your
+  # operation may take then you cannot count on exclusivity. For example,
+  # if the timeout is 10 seconds and you do an operation which may take 20
+  # seconds then two overlapping operations may hold a lease at the
+  # same time.
+  #
   class ExclusiveLease
-    def initialize(key, timeout)
+    def initialize(key, timeout:)
       @key, @timeout = key, timeout
     end
 
     # Try to obtain the lease. Return true on succes,
     # false if the lease is already taken.
     def try_obtain
+      # This is expected to be atomic because we are talking to a
+      # single-threaded Redis server.
       !!redis.set(redis_key, redis_value, nx: true, ex: @timeout)
     end
 
diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb
index 90d5996757f..da4435c7308 100644
--- a/lib/gitlab/ldap/access.rb
+++ b/lib/gitlab/ldap/access.rb
@@ -7,16 +7,6 @@ module Gitlab
     class Access
       attr_reader :provider, :user
 
-      # This timeout acts as a throttle on LDAP user checks. Its value of 600
-      # seconds (10 minutes) means that after calling try_lock_user for user
-      # janedoe, no new LDAP checks can start for that user for the next 10
-      # minutes.
-      LEASE_TIMEOUT = 600
-
-      def self.try_lock_user(user)
-        Gitlab::ExclusiveLease.new("user_ldap_check:#{user.id}", LEASE_TIMEOUT).try_obtain
-      end
-
       def self.open(user, &block)
         Gitlab::LDAP::Adapter.open(user.ldap_identity.provider) do |adapter|
           block.call(self.new(user, adapter))
diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb
index 46ac5825fd1..d1b42c1f9b9 100644
--- a/lib/gitlab/user_access.rb
+++ b/lib/gitlab/user_access.rb
@@ -3,9 +3,8 @@ module Gitlab
     def self.allowed?(user)
       return false if user.blocked?
 
-      if user.requires_ldap_check? && Gitlab::LDAP::Access.try_lock_user(user)
-          return Gitlab::LDAP::Access.allowed?(user)
-        end
+      if user.requires_ldap_check? && user.try_obtain_ldap_lease
+        return false unless Gitlab::LDAP::Access.allowed?(user)
       end
 
       true