Merge branch '296706-github-importer-failed-email-lookup-is-cached-for-a-day' into 'master'

Shorten cache time for private emails in GitHub Importer

See merge request gitlab-org/gitlab!70293

lib/gitlab/bitbucket_server_import/importer.rb

@@ -56,7 +56,7 @@ module Gitlab
 
         log_info(stage: "complete")
 
-        Gitlab::Cache::Import::Caching.expire(already_imported_cache_key, 15.minutes.to_i)
+        Gitlab::Cache::Import::Caching.expire(already_imported_cache_key, Gitlab::Cache::Import::Caching::SHORTER_TIMEOUT)
         true
       end
 

lib/gitlab/cache/import/caching.rb

@@ -9,6 +9,8 @@ module Gitlab
 
         LONGER_TIMEOUT = 72.hours.to_i
 
+        SHORTER_TIMEOUT = 15.minutes.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))

lib/gitlab/github_import/parallel_scheduling.rb

@@ -44,7 +44,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.
-        Gitlab::Cache::Import::Caching.expire(already_imported_cache_key, 15.minutes.to_i)
+        Gitlab::Cache::Import::Caching.expire(already_imported_cache_key, Gitlab::Cache::Import::Caching::SHORTER_TIMEOUT)
         info(project.id, message: "importer finished")
 
         retval

lib/gitlab/github_import/user_finder.rb

@@ -106,7 +106,7 @@ module Gitlab
 
         unless email
           user = client.user(username)
-          email = Gitlab::Cache::Import::Caching.write(cache_key, user.email) if user
+          email = Gitlab::Cache::Import::Caching.write(cache_key, user.email, timeout: timeout(user.email)) if user
         end
 
         email
@@ -171,6 +171,16 @@ module Gitlab
         # which we couldn't find an ID.
         [exists, number > 0 ? number : nil]
       end
+
+      private
+
+      def timeout(email)
+        if email
+          Gitlab::Cache::Import::Caching::TIMEOUT
+        else
+          Gitlab::Cache::Import::Caching::SHORTER_TIMEOUT
+        end
+      end
     end
   end
 end

spec/lib/gitlab/github_import/user_finder_spec.rb

@@ -195,7 +195,7 @@ RSpec.describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do
 
         expect(Gitlab::Cache::Import::Caching)
           .to receive(:write)
-          .with(an_instance_of(String), email)
+          .with(an_instance_of(String), email, timeout: Gitlab::Cache::Import::Caching::TIMEOUT)
 
         finder.email_for_github_username('kittens')
       end
@@ -211,6 +211,16 @@ RSpec.describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do
 
         expect(finder.email_for_github_username('kittens')).to be_nil
       end
+
+      it 'shortens the timeout for Email address in cache when an Email address is private/nil from GitHub' do
+        user = double(:user, email: nil)
+        expect(client).to receive(:user).with('kittens').and_return(user)
+
+        expect(Gitlab::Cache::Import::Caching)
+          .to receive(:write).with(an_instance_of(String), nil, timeout: Gitlab::Cache::Import::Caching::SHORTER_TIMEOUT)
+
+        expect(finder.email_for_github_username('kittens')).to be_nil
+      end
     end
   end