Commit c89d61be authored by Doug Stull's avatar Doug Stull

Do not cache user email from github if email is nil/private

- users want to rerun import after exposing their emails, but
  cache is getting in the way and this fixes that.

Changelog: fixed
parent 6de4fb41
...@@ -56,7 +56,7 @@ module Gitlab ...@@ -56,7 +56,7 @@ module Gitlab
log_info(stage: "complete") 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 true
end end
......
...@@ -9,6 +9,8 @@ module Gitlab ...@@ -9,6 +9,8 @@ module Gitlab
LONGER_TIMEOUT = 72.hours.to_i LONGER_TIMEOUT = 72.hours.to_i
SHORTER_TIMEOUT = 15.minutes.to_i
WRITE_IF_GREATER_SCRIPT = <<-EOF.strip_heredoc.freeze WRITE_IF_GREATER_SCRIPT = <<-EOF.strip_heredoc.freeze
local key, value, ttl = KEYS[1], tonumber(ARGV[1]), ARGV[2] local key, value, ttl = KEYS[1], tonumber(ARGV[1]), ARGV[2]
local existing = tonumber(redis.call("get", key)) local existing = tonumber(redis.call("get", key))
......
...@@ -44,7 +44,7 @@ module Gitlab ...@@ -44,7 +44,7 @@ module Gitlab
# still scheduling duplicates while. Since all work has already been # still scheduling duplicates while. Since all work has already been
# completed those jobs will just cycle through any remaining pages while # completed those jobs will just cycle through any remaining pages while
# not scheduling anything. # 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") info(project.id, message: "importer finished")
retval retval
......
...@@ -106,7 +106,7 @@ module Gitlab ...@@ -106,7 +106,7 @@ module Gitlab
unless email unless email
user = client.user(username) 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 end
email email
...@@ -171,6 +171,16 @@ module Gitlab ...@@ -171,6 +171,16 @@ module Gitlab
# which we couldn't find an ID. # which we couldn't find an ID.
[exists, number > 0 ? number : nil] [exists, number > 0 ? number : nil]
end end
private
def timeout(email)
if email
Gitlab::Cache::Import::Caching::TIMEOUT
else
Gitlab::Cache::Import::Caching::SHORTER_TIMEOUT
end
end
end end
end end
end end
...@@ -195,7 +195,7 @@ RSpec.describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do ...@@ -195,7 +195,7 @@ RSpec.describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do
expect(Gitlab::Cache::Import::Caching) expect(Gitlab::Cache::Import::Caching)
.to receive(:write) .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') finder.email_for_github_username('kittens')
end end
...@@ -211,6 +211,16 @@ RSpec.describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do ...@@ -211,6 +211,16 @@ RSpec.describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do
expect(finder.email_for_github_username('kittens')).to be_nil expect(finder.email_for_github_username('kittens')).to be_nil
end 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
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment