Move Caching module out of GithubImporter namespace to be reused

Caching module is general enough to be used in other importers or
other cases, so moving it out of GithubImporter module simply
to break linkage with github importer.

lib/gitlab/cache/import/caching.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Cache
+    module Import
+      module Caching
+        # The default timeout of the cache keys.
+        TIMEOUT = 24.hours.to_i
+
+        WRITE_IF_GREATER_SCRIPT = <<-EOF.strip_heredoc.freeze
+        local key, value, ttl = KEYS[1], tonumber(ARGV[1]), ARGV[2]
+        local existing = tonumber(redis.call("get", key))
+
+        if existing == nil or value > existing then
+          redis.call("set", key, value)
+          redis.call("expire", key, ttl)
+          return true
+        else
+          return false
+        end
+        EOF
+
+        # Reads a cache key.
+        #
+        # If the key exists and has a non-empty value its TTL is refreshed
+        # automatically.
+        #
+        # raw_key - The cache key to read.
+        # timeout - The new timeout of the key if the key is to be refreshed.
+        def self.read(raw_key, timeout: TIMEOUT)
+          key = cache_key_for(raw_key)
+          value = Redis::Cache.with { |redis| redis.get(key) }
+
+          if value.present?
+            # We refresh the expiration time so frequently used keys stick
+            # around, removing the need for querying the database as much as
+            # possible.
+            #
+            # A key may be empty when we looked up a GitHub user (for example) but
+            # did not find a matching GitLab user. In that case we _don't_ want to
+            # refresh the TTL so we automatically pick up the right data when said
+            # user were to register themselves on the GitLab instance.
+            Redis::Cache.with { |redis| redis.expire(key, timeout) }
+          end
+
+          value
+        end
+
+        # Reads an integer from the cache, or returns nil if no value was found.
+        #
+        # See Caching.read for more information.
+        def self.read_integer(raw_key, timeout: TIMEOUT)
+          value = read(raw_key, timeout: timeout)
+
+          value.to_i if value.present?
+        end
+
+        # Sets a cache key to the given value.
+        #
+        # key - The cache key to write.
+        # value - The value to set.
+        # timeout - The time after which the cache key should expire.
+        def self.write(raw_key, value, timeout: TIMEOUT)
+          key = cache_key_for(raw_key)
+
+          Redis::Cache.with do |redis|
+            redis.set(key, value, ex: timeout)
+          end
+
+          value
+        end
+
+        # Adds a value to a set.
+        #
+        # raw_key - The key of the set to add the value to.
+        # value - The value to add to the set.
+        # timeout - The new timeout of the key.
+        def self.set_add(raw_key, value, timeout: TIMEOUT)
+          key = cache_key_for(raw_key)
+
+          Redis::Cache.with do |redis|
+            redis.multi do |m|
+              m.sadd(key, value)
+              m.expire(key, timeout)
+            end
+          end
+        end
+
+        # Returns true if the given value is present in the set.
+        #
+        # raw_key - The key of the set to check.
+        # value - The value to check for.
+        def self.set_includes?(raw_key, value)
+          key = cache_key_for(raw_key)
+
+          Redis::Cache.with do |redis|
+            redis.sismember(key, value)
+          end
+        end
+
+        # Sets multiple keys to a given value.
+        #
+        # mapping - A Hash mapping the cache keys to their values.
+        # timeout - The time after which the cache key should expire.
+        def self.write_multiple(mapping, timeout: TIMEOUT)
+          Redis::Cache.with do |redis|
+            redis.multi do |multi|
+              mapping.each do |raw_key, value|
+                multi.set(cache_key_for(raw_key), value, ex: timeout)
+              end
+            end
+          end
+        end
+
+        # Sets the expiration time of a key.
+        #
+        # raw_key - The key for which to change the timeout.
+        # timeout - The new timeout.
+        def self.expire(raw_key, timeout)
+          key = cache_key_for(raw_key)
+
+          Redis::Cache.with do |redis|
+            redis.expire(key, timeout)
+          end
+        end
+
+        # Sets a key to the given integer but only if the existing value is
+        # smaller than the given value.
+        #
+        # This method uses a Lua script to ensure the read and write are atomic.
+        #
+        # raw_key - The key to set.
+        # value - The new value for the key.
+        # timeout - The key timeout in seconds.
+        #
+        # Returns true when the key was overwritten, false otherwise.
+        def self.write_if_greater(raw_key, value, timeout: TIMEOUT)
+          key = cache_key_for(raw_key)
+          val = Redis::Cache.with do |redis|
+            redis
+              .eval(WRITE_IF_GREATER_SCRIPT, keys: [key], argv: [value, timeout])
+          end
+
+          val ? true : false
+        end
+
+        def self.cache_key_for(raw_key)
+          "#{Redis::Cache::CACHE_NAMESPACE}:#{raw_key}"
+        end
+      end
+    end
+  end
+end

lib/gitlab/github_import.rb

@@ -16,7 +16,7 @@ module Gitlab
     def self.ghost_user_id
       key = 'github-import/ghost-user-id'
 
-      Caching.read_integer(key) || Caching.write(key, User.select(:id).ghost.id)
+      Gitlab::Cache::Import::Caching.read_integer(key) || Gitlab::Cache::Import::Caching.write(key, User.select(:id).ghost.id)
     end
   end
 end

lib/gitlab/github_import/caching.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module GithubImport
-    module Caching
-      # The default timeout of the cache keys.
-      TIMEOUT = 24.hours.to_i
-
-      WRITE_IF_GREATER_SCRIPT = <<-EOF.strip_heredoc.freeze
-      local key, value, ttl = KEYS[1], tonumber(ARGV[1]), ARGV[2]
-      local existing = tonumber(redis.call("get", key))
-
-      if existing == nil or value > existing then
-        redis.call("set", key, value)
-        redis.call("expire", key, ttl)
-        return true
-      else
-        return false
-      end
-      EOF
-
-      # Reads a cache key.
-      #
-      # If the key exists and has a non-empty value its TTL is refreshed
-      # automatically.
-      #
-      # raw_key - The cache key to read.
-      # timeout - The new timeout of the key if the key is to be refreshed.
-      def self.read(raw_key, timeout: TIMEOUT)
-        key = cache_key_for(raw_key)
-        value = Redis::Cache.with { |redis| redis.get(key) }
-
-        if value.present?
-          # We refresh the expiration time so frequently used keys stick
-          # around, removing the need for querying the database as much as
-          # possible.
-          #
-          # A key may be empty when we looked up a GitHub user (for example) but
-          # did not find a matching GitLab user. In that case we _don't_ want to
-          # refresh the TTL so we automatically pick up the right data when said
-          # user were to register themselves on the GitLab instance.
-          Redis::Cache.with { |redis| redis.expire(key, timeout) }
-        end
-
-        value
-      end
-
-      # Reads an integer from the cache, or returns nil if no value was found.
-      #
-      # See Caching.read for more information.
-      def self.read_integer(raw_key, timeout: TIMEOUT)
-        value = read(raw_key, timeout: timeout)
-
-        value.to_i if value.present?
-      end
-
-      # Sets a cache key to the given value.
-      #
-      # key - The cache key to write.
-      # value - The value to set.
-      # timeout - The time after which the cache key should expire.
-      def self.write(raw_key, value, timeout: TIMEOUT)
-        key = cache_key_for(raw_key)
-
-        Redis::Cache.with do |redis|
-          redis.set(key, value, ex: timeout)
-        end
-
-        value
-      end
-
-      # Adds a value to a set.
-      #
-      # raw_key - The key of the set to add the value to.
-      # value - The value to add to the set.
-      # timeout - The new timeout of the key.
-      def self.set_add(raw_key, value, timeout: TIMEOUT)
-        key = cache_key_for(raw_key)
-
-        Redis::Cache.with do |redis|
-          redis.multi do |m|
-            m.sadd(key, value)
-            m.expire(key, timeout)
-          end
-        end
-      end
-
-      # Returns true if the given value is present in the set.
-      #
-      # raw_key - The key of the set to check.
-      # value - The value to check for.
-      def self.set_includes?(raw_key, value)
-        key = cache_key_for(raw_key)
-
-        Redis::Cache.with do |redis|
-          redis.sismember(key, value)
-        end
-      end
-
-      # Sets multiple keys to a given value.
-      #
-      # mapping - A Hash mapping the cache keys to their values.
-      # timeout - The time after which the cache key should expire.
-      def self.write_multiple(mapping, timeout: TIMEOUT)
-        Redis::Cache.with do |redis|
-          redis.multi do |multi|
-            mapping.each do |raw_key, value|
-              multi.set(cache_key_for(raw_key), value, ex: timeout)
-            end
-          end
-        end
-      end
-
-      # Sets the expiration time of a key.
-      #
-      # raw_key - The key for which to change the timeout.
-      # timeout - The new timeout.
-      def self.expire(raw_key, timeout)
-        key = cache_key_for(raw_key)
-
-        Redis::Cache.with do |redis|
-          redis.expire(key, timeout)
-        end
-      end
-
-      # Sets a key to the given integer but only if the existing value is
-      # smaller than the given value.
-      #
-      # This method uses a Lua script to ensure the read and write are atomic.
-      #
-      # raw_key - The key to set.
-      # value - The new value for the key.
-      # timeout - The key timeout in seconds.
-      #
-      # Returns true when the key was overwritten, false otherwise.
-      def self.write_if_greater(raw_key, value, timeout: TIMEOUT)
-        key = cache_key_for(raw_key)
-        val = Redis::Cache.with do |redis|
-          redis
-            .eval(WRITE_IF_GREATER_SCRIPT, keys: [key], argv: [value, timeout])
-        end
-
-        val ? true : false
-      end
-
-      def self.cache_key_for(raw_key)
-        "#{Redis::Cache::CACHE_NAMESPACE}:#{raw_key}"
-      end
-    end
-  end
-end

lib/gitlab/github_import/issuable_finder.rb

@@ -23,7 +23,7 @@ module Gitlab
       #
       # This method will return `nil` if no ID could be found.
       def database_id
-        val = Caching.read(cache_key)
+        val = Gitlab::Cache::Import::Caching.read(cache_key)
 
         val.to_i if val.present?
       end
@@ -32,7 +32,7 @@ module Gitlab
       #
       # database_id - The ID of the corresponding database row.
       def cache_database_id(database_id)
-        Caching.write(cache_key, database_id)
+        Gitlab::Cache::Import::Caching.write(cache_key, database_id)
       end
 
       private

lib/gitlab/github_import/label_finder.rb

@@ -15,7 +15,7 @@ module Gitlab
 
       # Returns the label ID for the given name.
       def id_for(name)
-        Caching.read_integer(cache_key_for(name))
+        Gitlab::Cache::Import::Caching.read_integer(cache_key_for(name))
       end
 
       # rubocop: disable CodeReuse/ActiveRecord
@@ -27,7 +27,7 @@ module Gitlab
             hash[cache_key_for(name)] = id
           end
 
-        Caching.write_multiple(mapping)
+        Gitlab::Cache::Import::Caching.write_multiple(mapping)
       end
       # rubocop: enable CodeReuse/ActiveRecord
 

lib/gitlab/github_import/milestone_finder.rb

@@ -18,7 +18,7 @@ module Gitlab
       def id_for(issuable)
         return unless issuable.milestone_number
 
-        Caching.read_integer(cache_key_for(issuable.milestone_number))
+        Gitlab::Cache::Import::Caching.read_integer(cache_key_for(issuable.milestone_number))
       end
 
       # rubocop: disable CodeReuse/ActiveRecord
@@ -30,7 +30,7 @@ module Gitlab
             hash[cache_key_for(iid)] = id
           end
 
-        Caching.write_multiple(mapping)
+        Gitlab::Cache::Import::Caching.write_multiple(mapping)
       end
       # rubocop: enable CodeReuse/ActiveRecord
 

lib/gitlab/github_import/page_counter.rb

@@ -19,12 +19,12 @@ module Gitlab
       #
       # Returns true if the page number was overwritten, false otherwise.
       def set(page)
-        Caching.write_if_greater(cache_key, page)
+        Gitlab::Cache::Import::Caching.write_if_greater(cache_key, page)
       end
 
       # Returns the current value from the cache.
       def current
-        Caching.read_integer(cache_key) || 1
+        Gitlab::Cache::Import::Caching.read_integer(cache_key) || 1
       end
     end
   end

lib/gitlab/github_import/parallel_scheduling.rb

@@ -42,7 +42,7 @@ module Gitlab
         # still scheduling duplicates while. Since all work has already been
         # completed those jobs will just cycle through any remaining pages while
         # not scheduling anything.
-        Caching.expire(already_imported_cache_key, 15.minutes.to_i)
+        Gitlab::Cache::Import::Caching.expire(already_imported_cache_key, 15.minutes.to_i)
 
         retval
       end
@@ -112,14 +112,14 @@ module Gitlab
       def already_imported?(object)
         id = id_for_already_imported_cache(object)
 
-        Caching.set_includes?(already_imported_cache_key, id)
+        Gitlab::Cache::Import::Caching.set_includes?(already_imported_cache_key, id)
       end
 
       # Marks the given object as "already imported".
       def mark_as_imported(object)
         id = id_for_already_imported_cache(object)
 
-        Caching.set_add(already_imported_cache_key, id)
+        Gitlab::Cache::Import::Caching.set_add(already_imported_cache_key, id)
       end
 
       # Returns the ID to use for the cache used for checking if an object has

lib/gitlab/github_import/user_finder.rb

@@ -102,11 +102,11 @@ module Gitlab
 
       def email_for_github_username(username)
         cache_key = EMAIL_FOR_USERNAME_CACHE_KEY % username
-        email = Caching.read(cache_key)
+        email = Gitlab::Cache::Import::Caching.read(cache_key)
 
         unless email
           user = client.user(username)
-          email = Caching.write(cache_key, user.email) if user
+          email = Gitlab::Cache::Import::Caching.write(cache_key, user.email) if user
         end
 
         email
@@ -125,7 +125,7 @@ module Gitlab
       def id_for_github_id(id)
         gitlab_id = query_id_for_github_id(id) || nil
 
-        Caching.write(ID_CACHE_KEY % id, gitlab_id)
+        Gitlab::Cache::Import::Caching.write(ID_CACHE_KEY % id, gitlab_id)
       end
 
       # Queries and caches the GitLab user ID for a GitHub email, if one was
@@ -133,7 +133,7 @@ module Gitlab
       def id_for_github_email(email)
         gitlab_id = query_id_for_github_email(email) || nil
 
-        Caching.write(ID_FOR_EMAIL_CACHE_KEY % email, gitlab_id)
+        Gitlab::Cache::Import::Caching.write(ID_FOR_EMAIL_CACHE_KEY % email, gitlab_id)
       end
 
       # rubocop: disable CodeReuse/ActiveRecord
@@ -155,7 +155,7 @@ module Gitlab
       # 1. A boolean indicating if the key was present or not.
       # 2. The ID as an Integer, or nil in case no ID could be found.
       def read_id_from_cache(key)
-        value = Caching.read(key)
+        value = Gitlab::Cache::Import::Caching.read(key)
         exists = !value.nil?
         number = value.to_i
 

spec/lib/gitlab/github_import/caching_spec.rb → spec/lib/gitlab/cache/import/caching_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Gitlab::GithubImport::Caching, :clean_gitlab_redis_cache do
+describe Gitlab::Cache::Import::Caching, :clean_gitlab_redis_cache do
   describe '.read' do
     it 'reads a value from the cache' do
       described_class.write('foo', 'bar')

spec/lib/gitlab/github_import/issuable_finder_spec.rb

@@ -30,7 +30,7 @@ describe Gitlab::GithubImport::IssuableFinder, :clean_gitlab_redis_cache do
 
   describe '#cache_database_id' do
     it 'caches the ID of a database row' do
-      expect(Gitlab::GithubImport::Caching)
+      expect(Gitlab::Cache::Import::Caching)
         .to receive(:write)
         .with('github-import/issuable-finder/4/MergeRequest/1', 10)
 

spec/lib/gitlab/github_import/label_finder_spec.rb

@@ -21,7 +21,7 @@ describe Gitlab::GithubImport::LabelFinder, :clean_gitlab_redis_cache do
       it 'returns nil for an empty cache key' do
         key = finder.cache_key_for(bug.name)
 
-        Gitlab::GithubImport::Caching.write(key, '')
+        Gitlab::Cache::Import::Caching.write(key, '')
 
         expect(finder.id_for(bug.name)).to be_nil
       end
@@ -40,7 +40,7 @@ describe Gitlab::GithubImport::LabelFinder, :clean_gitlab_redis_cache do
 
   describe '#build_cache' do
     it 'builds the cache of all project labels' do
-      expect(Gitlab::GithubImport::Caching)
+      expect(Gitlab::Cache::Import::Caching)
         .to receive(:write_multiple)
         .with(
           {

spec/lib/gitlab/github_import/milestone_finder_spec.rb

@@ -22,7 +22,7 @@ describe Gitlab::GithubImport::MilestoneFinder, :clean_gitlab_redis_cache do
       it 'returns nil for an empty cache key' do
         key = finder.cache_key_for(milestone.iid)
 
-        Gitlab::GithubImport::Caching.write(key, '')
+        Gitlab::Cache::Import::Caching.write(key, '')
 
         expect(finder.id_for(issuable)).to be_nil
       end
@@ -41,7 +41,7 @@ describe Gitlab::GithubImport::MilestoneFinder, :clean_gitlab_redis_cache do
 
   describe '#build_cache' do
     it 'builds the cache of all project milestones' do
-      expect(Gitlab::GithubImport::Caching)
+      expect(Gitlab::Cache::Import::Caching)
         .to receive(:write_multiple)
         .with("github-import/milestone-finder/#{project.id}/1" => milestone.id)
         .and_call_original

spec/lib/gitlab/github_import/page_counter_spec.rb

@@ -12,7 +12,7 @@ describe Gitlab::GithubImport::PageCounter, :clean_gitlab_redis_cache do
     end
 
     it 'sets the initial page number to the cached value when one is present' do
-      Gitlab::GithubImport::Caching.write(counter.cache_key, 2)
+      Gitlab::Cache::Import::Caching.write(counter.cache_key, 2)
 
       expect(described_class.new(project, :issues).current).to eq(2)
     end

spec/lib/gitlab/github_import/parallel_scheduling_spec.rb

@@ -57,7 +57,7 @@ describe Gitlab::GithubImport::ParallelScheduling do
 
       expect(importer).to receive(:parallel_import)
 
-      expect(Gitlab::GithubImport::Caching)
+      expect(Gitlab::Cache::Import::Caching)
         .to receive(:expire)
         .with(importer.already_imported_cache_key, a_kind_of(Numeric))
 
@@ -287,7 +287,7 @@ describe Gitlab::GithubImport::ParallelScheduling do
         .with(object)
         .and_return(object.id)
 
-      expect(Gitlab::GithubImport::Caching)
+      expect(Gitlab::Cache::Import::Caching)
         .to receive(:set_add)
         .with(importer.already_imported_cache_key, object.id)
         .and_call_original

spec/lib/gitlab/github_import/user_finder_spec.rb

@@ -162,7 +162,7 @@ describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do
 
     context 'when an Email address is cached' do
       it 'reads the Email address from the cache' do
-        expect(Gitlab::GithubImport::Caching)
+        expect(Gitlab::Cache::Import::Caching)
           .to receive(:read)
           .and_return(email)
 
@@ -182,7 +182,7 @@ describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do
       it 'caches the Email address when an Email address is available' do
         expect(client).to receive(:user).with('kittens').and_return(user)
 
-        expect(Gitlab::GithubImport::Caching)
+        expect(Gitlab::Cache::Import::Caching)
           .to receive(:write)
           .with(an_instance_of(String), email)
 
@@ -195,7 +195,7 @@ describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do
           .with('kittens')
           .and_return(nil)
 
-        expect(Gitlab::GithubImport::Caching)
+        expect(Gitlab::Cache::Import::Caching)
           .not_to receive(:write)
 
         expect(finder.email_for_github_username('kittens')).to be_nil
@@ -207,7 +207,7 @@ describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do
     let(:id) { 4 }
 
     it 'reads a user ID from the cache' do
-      Gitlab::GithubImport::Caching
+      Gitlab::Cache::Import::Caching
         .write(described_class::ID_CACHE_KEY % id, 4)
 
       expect(finder.cached_id_for_github_id(id)).to eq([true, 4])
@@ -222,7 +222,7 @@ describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do
     let(:email) { 'kittens@example.com' }
 
     it 'reads a user ID from the cache' do
-      Gitlab::GithubImport::Caching
+      Gitlab::Cache::Import::Caching
         .write(described_class::ID_FOR_EMAIL_CACHE_KEY % email, 4)
 
       expect(finder.cached_id_for_github_email(email)).to eq([true, 4])
@@ -241,7 +241,7 @@ describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do
         .with(id)
         .and_return(42)
 
-      expect(Gitlab::GithubImport::Caching)
+      expect(Gitlab::Cache::Import::Caching)
         .to receive(:write)
         .with(described_class::ID_CACHE_KEY % id, 42)
 
@@ -253,7 +253,7 @@ describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do
         .with(id)
         .and_return(nil)
 
-      expect(Gitlab::GithubImport::Caching)
+      expect(Gitlab::Cache::Import::Caching)
         .to receive(:write)
         .with(described_class::ID_CACHE_KEY % id, nil)
 
@@ -269,7 +269,7 @@ describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do
         .with(email)
         .and_return(42)
 
-      expect(Gitlab::GithubImport::Caching)
+      expect(Gitlab::Cache::Import::Caching)
         .to receive(:write)
         .with(described_class::ID_FOR_EMAIL_CACHE_KEY % email, 42)
 
@@ -281,7 +281,7 @@ describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do
         .with(email)
         .and_return(nil)
 
-      expect(Gitlab::GithubImport::Caching)
+      expect(Gitlab::Cache::Import::Caching)
         .to receive(:write)
         .with(described_class::ID_FOR_EMAIL_CACHE_KEY % email, nil)
 
@@ -317,13 +317,13 @@ describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do
 
   describe '#read_id_from_cache' do
     it 'reads an ID from the cache' do
-      Gitlab::GithubImport::Caching.write('foo', 10)
+      Gitlab::Cache::Import::Caching.write('foo', 10)
 
       expect(finder.read_id_from_cache('foo')).to eq([true, 10])
     end
 
     it 'reads a cache key with an empty value' do
-      Gitlab::GithubImport::Caching.write('foo', nil)
+      Gitlab::Cache::Import::Caching.write('foo', nil)
 
       expect(finder.read_id_from_cache('foo')).to eq([true, nil])
     end

spec/lib/gitlab/github_import_spec.rb

@@ -35,7 +35,7 @@ describe Gitlab::GithubImport do
     end
 
     it 'caches the ghost user ID' do
-      expect(Gitlab::GithubImport::Caching)
+      expect(Gitlab::Cache::Import::Caching)
         .to receive(:write)
         .once
         .and_call_original