Skip to content

Commit

Permalink
DEV: s/\$redis/Discourse\.redis (discourse#8431)
Browse files Browse the repository at this point in the history
This commit also adds a rubocop rule to prevent global variables.
  • Loading branch information
jjaffeux authored Dec 3, 2019
1 parent 9eccfb7 commit 0d3d2c4
Show file tree
Hide file tree
Showing 118 changed files with 378 additions and 362 deletions.
17 changes: 13 additions & 4 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,17 @@ Style/Semicolon:
Style/RedundantReturn:
Enabled: true

DiscourseCops/NoChdir:
Enabled: true
Exclude:
- 'spec/**/*' # Specs are run sequentially, so chdir can be used
DiscourseCops/NoChdir:
Enabled: true
Exclude:
- 'spec/**/*' # Specs are run sequentially, so chdir can be used
- 'plugins/*/spec/**/*'

Style/GlobalVars:
Enabled: true
Severity: warning
Exclude:
- 'lib/tasks/**/*'
- 'script/**/*'
- 'spec/**/*.rb'
- 'plugins/*/spec/**/*'
4 changes: 2 additions & 2 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -768,14 +768,14 @@ def build_not_found_page(opts = {})

if !SiteSetting.login_required? || (current_user rescue false)
key = "page_not_found_topics"
if @topics_partial = $redis.get(key)
if @topics_partial = Discourse.redis.get(key)
@topics_partial = @topics_partial.html_safe
else
category_topic_ids = Category.pluck(:topic_id).compact
@top_viewed = TopicQuery.new(nil, except_topic_ids: category_topic_ids).list_top_for("monthly").topics.first(10)
@recent = Topic.includes(:category).where.not(id: category_topic_ids).recent(10)
@topics_partial = render_to_string partial: '/exceptions/not_found_topics', formats: [:html]
$redis.setex(key, 10.minutes, @topics_partial)
Discourse.redis.setex(key, 10.minutes, @topics_partial)
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/forums_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class ForumsController < ActionController::Base
after_action :add_readonly_header

def status
if $shutdown
if $shutdown # rubocop:disable Style/GlobalVars
render plain: "shutting down", status: 500
else
render plain: "ok"
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/session_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -409,15 +409,15 @@ def email_login
end

def one_time_password
@otp_username = otp_username = $redis.get "otp_#{params[:token]}"
@otp_username = otp_username = Discourse.redis.get "otp_#{params[:token]}"

if otp_username && user = User.find_by_username(otp_username)
if current_user&.username == otp_username
$redis.del "otp_#{params[:token]}"
Discourse.redis.del "otp_#{params[:token]}"
return redirect_to path("/")
elsif request.post?
log_on_user(user)
$redis.del "otp_#{params[:token]}"
Discourse.redis.del "otp_#{params[:token]}"
return redirect_to path("/")
else
# Display the form
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/user_api_keys_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ def one_time_password(public_key, username)
raise Discourse::InvalidAccess unless UserApiKey.allowed_scopes.superset?(Set.new(["one_time_password"]))

otp = SecureRandom.hex
$redis.setex "otp_#{otp}", 10.minutes, username
Discourse.redis.setex "otp_#{otp}", 10.minutes, username

Base64.encode64(public_key.public_encrypt(otp))
end
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/users/associate_accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def connect_info
# Presents a confirmation screen to the user. Accessed via GET, with no CSRF checks
def connect
auth = get_auth_hash
$redis.del "#{REDIS_PREFIX}_#{current_user&.id}_#{params[:token]}"
Discourse.redis.del "#{REDIS_PREFIX}_#{current_user&.id}_#{params[:token]}"

provider_name = auth.provider
authenticator = Discourse.enabled_authenticators.find { |a| a.name == provider_name }
Expand All @@ -37,7 +37,7 @@ def connect

def get_auth_hash
token = params[:token]
json = $redis.get "#{REDIS_PREFIX}_#{current_user&.id}_#{token}"
json = Discourse.redis.get "#{REDIS_PREFIX}_#{current_user&.id}_#{token}"
raise Discourse::NotFound if json.nil?

OmniAuth::AuthHash.new(JSON.parse(json))
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def complete
if session.delete(:auth_reconnect) && authenticator.can_connect_existing_user? && current_user
# Save to redis, with a secret token, then redirect to confirmation screen
token = SecureRandom.hex
$redis.setex "#{Users::AssociateAccountsController::REDIS_PREFIX}_#{current_user.id}_#{token}", 10.minutes, auth.to_json
Discourse.redis.setex "#{Users::AssociateAccountsController::REDIS_PREFIX}_#{current_user.id}_#{token}", 10.minutes, auth.to_json
return redirect_to Discourse.base_uri("/associate/#{token}")
else
@auth_result = authenticator.after_authenticate(auth)
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/webhooks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ def valid_mailgun_signature?(token, timestamp, signature)

# prevent replay attacks
key = "mailgun_token_#{token}"
return false unless $redis.setnx(key, 1)
$redis.expire(key, 10.minutes)
return false unless Discourse.redis.setnx(key, 1)
Discourse.redis.expire(key, 10.minutes)

# ensure timestamp isn't too far from current time
return false if (Time.at(timestamp.to_i) - Time.now).abs > 12.hours.to_i
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def shared_session_key
return request.env[sk] if request.env[sk]

request.env[sk] = key = (session[sk] ||= SecureRandom.hex)
$redis.setex "#{sk}_#{key}", 7.days, current_user.id.to_s
Discourse.redis.setex "#{sk}_#{key}", 7.days, current_user.id.to_s
key
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/jobs/onceoff/clean_up_sidekiq_statistic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module Jobs
class CleanUpSidekiqStatistic < ::Jobs::Onceoff
def execute_onceoff(args)
$redis.without_namespace.del('sidekiq:sidekiq:statistic')
Discourse.redis.without_namespace.del('sidekiq:sidekiq:statistic')
end
end
end
4 changes: 2 additions & 2 deletions app/jobs/onceoff/onceoff.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def running_key_name
# Pass `force: true` to force it happen again
def execute(args)
job_name = self.class.name_for(self.class)
has_lock = $redis.setnx(running_key_name, Time.now.to_i)
has_lock = Discourse.redis.setnx(running_key_name, Time.now.to_i)

# If we can't get a lock, just noop
if args[:force] || has_lock
Expand All @@ -25,7 +25,7 @@ def execute(args)
execute_onceoff(args)
OnceoffLog.create!(job_name: job_name)
ensure
$redis.del(running_key_name) if has_lock
Discourse.redis.del(running_key_name) if has_lock
end
end

Expand Down
4 changes: 2 additions & 2 deletions app/jobs/regular/run_heartbeat.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ def self.heartbeat_key
end

def execute(args)
$redis.set(self.class.heartbeat_key, Time.new.to_i.to_s)
Discourse.redis.set(self.class.heartbeat_key, Time.new.to_i.to_s)
end

def self.last_heartbeat
$redis.get(heartbeat_key).to_i
Discourse.redis.get(heartbeat_key).to_i
end
end
end
6 changes: 3 additions & 3 deletions app/jobs/scheduled/clean_up_uploads.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,16 @@ def execute(args)
end

def last_cleanup=(v)
$redis.setex(last_cleanup_key, 7.days.to_i, v.to_s)
Discourse.redis.setex(last_cleanup_key, 7.days.to_i, v.to_s)
end

def last_cleanup
v = $redis.get(last_cleanup_key)
v = Discourse.redis.get(last_cleanup_key)
v ? v.to_i : v
end

def reset_last_cleanup!
$redis.del(last_cleanup_key)
Discourse.redis.del(last_cleanup_key)
end

protected
Expand Down
4 changes: 2 additions & 2 deletions app/jobs/scheduled/pending_queued_posts_reminder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ def should_notify_ids
end

def last_notified_id
(i = $redis.get(self.class.last_notified_key)) && i.to_i
(i = Discourse.redis.get(self.class.last_notified_key)) && i.to_i
end

def last_notified_id=(arg)
$redis.set(self.class.last_notified_key, arg)
Discourse.redis.set(self.class.last_notified_key, arg)
end

def self.last_notified_key
Expand Down
6 changes: 3 additions & 3 deletions app/jobs/scheduled/pending_reviewables_reminder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,19 @@ def execute(args)
end

def self.last_notified_id
$redis.get(last_notified_key).to_i
Discourse.redis.get(last_notified_key).to_i
end

def self.last_notified_id=(arg)
$redis.set(last_notified_key, arg)
Discourse.redis.set(last_notified_key, arg)
end

def self.last_notified_key
"last_notified_reviewable_id".freeze
end

def self.clear_key
$redis.del(last_notified_key)
Discourse.redis.del(last_notified_key)
end

def active_moderator_usernames
Expand Down
4 changes: 2 additions & 2 deletions app/jobs/scheduled/pending_users_reminder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ def execute(args)
end

def previous_newest_username
$redis.get previous_newest_username_cache_key
Discourse.redis.get previous_newest_username_cache_key
end

def previous_newest_username=(username)
$redis.setex previous_newest_username_cache_key, 7.days, username
Discourse.redis.setex previous_newest_username_cache_key, 7.days, username
end

def previous_newest_username_cache_key
Expand Down
12 changes: 6 additions & 6 deletions app/jobs/scheduled/poll_mailbox.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ def poll_pop3
end
end
rescue Net::OpenTimeout => e
count = $redis.incr(POLL_MAILBOX_TIMEOUT_ERROR_KEY).to_i
count = Discourse.redis.incr(POLL_MAILBOX_TIMEOUT_ERROR_KEY).to_i

$redis.expire(
Discourse.redis.expire(
POLL_MAILBOX_TIMEOUT_ERROR_KEY,
SiteSetting.pop3_polling_period_mins.minutes * 3
) if count == 1

if count > 3
$redis.del(POLL_MAILBOX_TIMEOUT_ERROR_KEY)
Discourse.redis.del(POLL_MAILBOX_TIMEOUT_ERROR_KEY)
mark_as_errored!
add_admin_dashboard_problem_message('dashboard.poll_pop3_timeout')
Discourse.handle_job_exception(e, error_context(@args, "Connecting to '#{SiteSetting.pop3_polling_host}' for polling emails."))
Expand All @@ -65,13 +65,13 @@ def poll_pop3
POLL_MAILBOX_ERRORS_KEY ||= "poll_mailbox_errors".freeze

def self.errors_in_past_24_hours
$redis.zremrangebyscore(POLL_MAILBOX_ERRORS_KEY, 0, 24.hours.ago.to_i)
$redis.zcard(POLL_MAILBOX_ERRORS_KEY).to_i
Discourse.redis.zremrangebyscore(POLL_MAILBOX_ERRORS_KEY, 0, 24.hours.ago.to_i)
Discourse.redis.zcard(POLL_MAILBOX_ERRORS_KEY).to_i
end

def mark_as_errored!
now = Time.now.to_i
$redis.zadd(POLL_MAILBOX_ERRORS_KEY, now, now.to_s)
Discourse.redis.zadd(POLL_MAILBOX_ERRORS_KEY, now, now.to_s)
end

def add_admin_dashboard_problem_message(i18n_key)
Expand Down
4 changes: 2 additions & 2 deletions app/mailers/user_notifications.rb
Original file line number Diff line number Diff line change
Expand Up @@ -697,9 +697,9 @@ def self.summary_new_users_count_key(min_date_str)
def summary_new_users_count(min_date)
min_date_str = min_date.is_a?(String) ? min_date : min_date.strftime('%Y-%m-%d')
key = self.class.summary_new_users_count_key(min_date_str)
((count = $redis.get(key)) && count.to_i) || begin
((count = Discourse.redis.get(key)) && count.to_i) || begin
count = User.real.where(active: true, staged: false).not_suspended.where("created_at > ?", min_date_str).count
$redis.setex(key, 1.day, count)
Discourse.redis.setex(key, 1.day, count)
count
end
end
Expand Down
16 changes: 8 additions & 8 deletions app/models/admin_dashboard_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,16 @@ def self.problems_started_key
end

def self.set_problems_started
existing_time = $redis.get(problems_started_key)
$redis.setex(problems_started_key, 14.days.to_i, existing_time || Time.zone.now.to_s)
existing_time = Discourse.redis.get(problems_started_key)
Discourse.redis.setex(problems_started_key, 14.days.to_i, existing_time || Time.zone.now.to_s)
end

def self.clear_problems_started
$redis.del problems_started_key
Discourse.redis.del problems_started_key
end

def self.problems_started_at
s = $redis.get(problems_started_key)
s = Discourse.redis.get(problems_started_key)
s ? Time.zone.parse(s) : nil
end

Expand Down Expand Up @@ -109,19 +109,19 @@ def self.fetch_problems(opts = {})
end

def self.problem_message_check(i18n_key)
$redis.get(problem_message_key(i18n_key)) ? I18n.t(i18n_key, base_path: Discourse.base_path) : nil
Discourse.redis.get(problem_message_key(i18n_key)) ? I18n.t(i18n_key, base_path: Discourse.base_path) : nil
end

def self.add_problem_message(i18n_key, expire_seconds = nil)
if expire_seconds.to_i > 0
$redis.setex problem_message_key(i18n_key), expire_seconds.to_i, 1
Discourse.redis.setex problem_message_key(i18n_key), expire_seconds.to_i, 1
else
$redis.set problem_message_key(i18n_key), 1
Discourse.redis.set problem_message_key(i18n_key), 1
end
end

def self.clear_problem_message(i18n_key)
$redis.del problem_message_key(i18n_key)
Discourse.redis.del problem_message_key(i18n_key)
end

def self.problem_message_key(i18n_key)
Expand Down
2 changes: 1 addition & 1 deletion app/models/application_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def self.clear_cache!(date = nil)

req_types.each do |req_type, _|
key = redis_key(req_type, date)
$redis.del key
Discourse.redis.del key
end
end

Expand Down
6 changes: 3 additions & 3 deletions app/models/category_featured_topic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def self.feature_topics(batched: false, batch_size: nil)

batch_size ||= DEFAULT_BATCH_SIZE

next_category_id = batched ? $redis.get(NEXT_CATEGORY_ID_KEY).to_i : 0
next_category_id = batched ? Discourse.redis.get(NEXT_CATEGORY_ID_KEY).to_i : 0

categories = Category.select(:id, :topic_id, :num_featured_topics)
.where('id >= ?', next_category_id)
Expand All @@ -27,7 +27,7 @@ def self.feature_topics(batched: false, batch_size: nil)
if batched
if categories.length == batch_size
next_id = Category.where('id > ?', categories.last.id).order('id asc').limit(1).pluck(:id)[0]
next_id ? $redis.setex(NEXT_CATEGORY_ID_KEY, 1.day, next_id) : clear_batch!
next_id ? Discourse.redis.setex(NEXT_CATEGORY_ID_KEY, 1.day, next_id) : clear_batch!
else
clear_batch!
end
Expand All @@ -39,7 +39,7 @@ def self.feature_topics(batched: false, batch_size: nil)
end

def self.clear_batch!
$redis.del(NEXT_CATEGORY_ID_KEY)
Discourse.redis.del(NEXT_CATEGORY_ID_KEY)
end

def self.feature_topics_for(c, existing = nil)
Expand Down
10 changes: 5 additions & 5 deletions app/models/concerns/cached_counting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ class << self

class_methods do
def perform_increment!(key, opts = nil)
val = $redis.incr(key).to_i
val = Discourse.redis.incr(key).to_i

# readonly mode it is going to be 0, skip
return if val == 0

# 3.days, see: https://github.com/rails/rails/issues/21296
$redis.expire(key, 259200)
Discourse.redis.expire(key, 259200)

autoflush = (opts && opts[:autoflush]) || self.autoflush
if autoflush > 0 && val >= autoflush
Expand All @@ -51,9 +51,9 @@ def write_cache!(date = nil)
# this may seem a bit fancy but in so it allows
# for concurrent calls without double counting
def get_and_reset(key)
namespaced_key = $redis.namespace_key(key)
val = $redis.without_namespace.eval(GET_AND_RESET, keys: [namespaced_key]).to_i
$redis.expire(key, 259200) # SET removes expiry, so set it again
namespaced_key = Discourse.redis.namespace_key(key)
val = Discourse.redis.without_namespace.eval(GET_AND_RESET, keys: [namespaced_key]).to_i
Discourse.redis.expire(key, 259200) # SET removes expiry, so set it again
val
end

Expand Down
Loading

0 comments on commit 0d3d2c4

Please sign in to comment.