DEV: Avoid $ globals (#15453)

Also:
* Remove an unused method (#fill_email)
* Replace a method that was used just once (#generate_username) with `SecureRandom.alphanumeric`
* Remove an obsolete dev puma `tmp/restart` file logic

app/controllers/forums_controller.rb

@@ -17,11 +17,6 @@ def status
       end
     end
 
-    if $shutdown # rubocop:disable Style/GlobalVars
-      render plain: "shutting down", status: (params[:shutdown_ok] ? 200 : 500)
-    else
-      render plain: "ok"
-    end
+    render plain: "ok"
   end
-
 end

app/services/destroy_task.rb

@@ -1,8 +1,7 @@
 # frozen_string_literal: true
 
 class DestroyTask
-
-  def initialize(io = $stdout)
+  def initialize(io = STDOUT)
     @io = io
   end
 

bin/unicorn

@@ -80,14 +80,14 @@ if ARGV.include?("--help")
   exit
 end
 
-# this dev_mode hackery enables, the following to be used to restart unicorn:
+# this dev_mode hackery enables the following to be used to restart unicorn:
 #
 # pkill -USR2 -f 'ruby bin/unicorn'
 #
 # This is handy if you want to bind a key to restarting unicorn in dev
 
 if dev_mode
-  $unicorn_dev_supervisor_pid = Process.pid # rubocop:disable Style/GlobalVars
+  UNICORN_DEV_SUPERVISOR_PID = Process.pid
 
   restart = true
   while restart

config/application.rb

@@ -291,9 +291,12 @@ def watchable_args
     require 'logster/redis_store'
     # Use redis for our cache
     config.cache_store = DiscourseRedis.new_redis_store
-    $redis = DiscourseRedis.new # rubocop:disable Style/GlobalVars
+    Discourse.redis = DiscourseRedis.new
     Logster.store = Logster::RedisStore.new(DiscourseRedis.new)
 
+    # Deprecated
+    $redis = Discourse.redis # rubocop:disable Style/GlobalVars
+
     # we configure rack cache on demand in an initializer
     # our setup does not use rack cache and instead defers to nginx
     config.action_dispatch.rack_cache = nil

config/initializers/000-development_reload_warnings.rb

@@ -12,7 +12,7 @@
   ]
 
   Listen.to(*paths, only: /\.rb$/) do |modified, added, removed|
-    supervisor_pid = $unicorn_dev_supervisor_pid # rubocop:disable Style/GlobalVars
+    supervisor_pid = UNICORN_DEV_SUPERVISOR_PID
     auto_restart = supervisor_pid && ENV["AUTO_RESTART"] != "0"
 
     files = modified + added + removed

config/unicorn.conf.rb

@@ -26,7 +26,7 @@
 pid (ENV["UNICORN_PID_PATH"] || "#{discourse_path}/tmp/pids/unicorn.pid")
 
 if ENV["RAILS_ENV"] != "production"
-  logger Logger.new($stdout)
+  logger Logger.new(STDOUT)
   # we want a longer timeout in dev cause first request can be really slow
   timeout (ENV["UNICORN_TIMEOUT"] && ENV["UNICORN_TIMEOUT"].to_i || 60)
 else

lib/discourse.rb

@@ -1,5 +1,4 @@
 # frozen_string_literal: true
-# rubocop:disable Style/GlobalVars
 
 require 'cache'
 require 'open3'
@@ -653,36 +652,32 @@ def self.request_refresh!(user_ids: nil)
   end
 
   def self.git_version
-    $git_version ||=
-      begin
-        git_cmd = 'git rev-parse HEAD'
-        self.try_git(git_cmd, Discourse::VERSION::STRING)
-      end # rubocop:disable Style/GlobalVars
+    @git_version ||= begin
+      git_cmd = 'git rev-parse HEAD'
+      self.try_git(git_cmd, Discourse::VERSION::STRING)
+    end
   end
 
   def self.git_branch
-    $git_branch ||=
-      begin
-        git_cmd = 'git rev-parse --abbrev-ref HEAD'
-        self.try_git(git_cmd, 'unknown')
-      end
+    @git_branch ||= begin
+      git_cmd = 'git rev-parse --abbrev-ref HEAD'
+      self.try_git(git_cmd, 'unknown')
+    end
   end
 
   def self.full_version
-    $full_version ||=
-      begin
-        git_cmd = 'git describe --dirty --match "v[0-9]*"'
-        self.try_git(git_cmd, 'unknown')
-      end
+    @full_version ||= begin
+      git_cmd = 'git describe --dirty --match "v[0-9]*"'
+      self.try_git(git_cmd, 'unknown')
+    end
   end
 
   def self.last_commit_date
-    $last_commit_date ||=
-      begin
-        git_cmd = 'git log -1 --format="%ct"'
-        seconds = self.try_git(git_cmd, nil)
-        seconds.nil? ? nil : DateTime.strptime(seconds, '%s')
-      end
+    @last_commit_date ||= begin
+      git_cmd = 'git log -1 --format="%ct"'
+      seconds = self.try_git(git_cmd, nil)
+      seconds.nil? ? nil : DateTime.strptime(seconds, '%s')
+    end
   end
 
   def self.try_git(git_cmd, default_value)
@@ -1001,9 +996,7 @@ def self.preload_rails!
     @preloaded_rails = true
   end
 
-  def self.redis
-    $redis
-  end
+  mattr_accessor :redis
 
   def self.is_parallel_test?
     ENV['RAILS_ENV'] == "test" && ENV['TEST_ENV_NUMBER']
@@ -1031,5 +1024,3 @@ def self.allow_dev_populate?
     Rails.env.development? || ENV["ALLOW_DEV_POPULATE"] == "1"
   end
 end
-
-# rubocop:enable Style/GlobalVars

lib/tasks/themes.rake

@@ -145,7 +145,7 @@ task "themes:isolated_test" => :environment do |t, args|
 
   redis = TemporaryRedis.new
   redis.start
-  $redis = redis.instance # rubocop:disable Style/GlobalVars
+  Discourse.redis = redis.instance
   db = TemporaryDb.new
   db.start
   db.migrate

spec/models/user_option_spec.rb

@@ -125,7 +125,7 @@
             end
 
             after do
-              $redis.flushdb
+              Discourse.redis.flushdb
             end
 
             it "should have a reason for the first visit" do

spec/rails_helper.rb

@@ -14,21 +14,15 @@
 
 require 'rubygems'
 require 'rbtrace'
-
 require 'pry'
 require 'pry-byebug'
 require 'pry-rails'
-
-# Loading more in this block will cause your tests to run faster. However,
-# if you change any configuration or code from libraries loaded here, you'll
-# need to restart spork for it take effect.
 require 'fabrication'
 require 'mocha/api'
 require 'certified'
 require 'webmock/rspec'
 
 class RspecErrorTracker
-
   def self.last_exception=(ex)
     @ex = ex
   end

spec/requests/forums_controller_spec.rb

@@ -19,24 +19,6 @@
     end
   end
 
-  describe "during shutdown" do
-    before(:each) do
-      $shutdown = true
-    end
-    after(:each) do
-      $shutdown = nil
-    end
-
-    it "returns a 500 response" do
-      get "/srv/status"
-      expect(response.status).to eq(500)
-    end
-    it "returns a 200 response when given shutdown_ok" do
-      get "/srv/status?shutdown_ok=1"
-      expect(response.status).to eq(200)
-    end
-  end
-
   describe "cluster parameter" do
     it "returns a 500 response if the cluster is not configured" do
       get "/srv/status?cluster=abc"

spec/requests/users_controller_spec.rb

@@ -1610,13 +1610,13 @@ def post_user(extra_params = {})
 
     context 'is too long' do
       before do
-        get "/u/check_username.json", params: { username: generate_username(User.username_length.last + 1) }
+        get "/u/check_username.json", params: { username: SecureRandom.alphanumeric(SiteSetting.max_username_length.to_i + 1) }
       end
       include_examples 'checking an invalid username'
 
       it 'should return the "too long" message' do
         expect(response.status).to eq(200)
-        expect(response.parsed_body['errors']).to include(I18n.t(:'user.username.long', max: User.username_length.end))
+        expect(response.parsed_body['errors']).to include(I18n.t(:'user.username.long', max: SiteSetting.max_username_length))
       end
     end
 

spec/support/helpers.rb

@@ -58,11 +58,6 @@ def create_post(args = {})
     post
   end
 
-  def generate_username(length = 10)
-    range = [*'a'..'z']
-    Array.new(length) { range.sample }.join
-  end
-
   def stub_guardian(user)
     guardian = Guardian.new(user)
     yield(guardian) if block_given?
@@ -82,14 +77,6 @@ def wait_for(on_fail: nil, &blk)
     expect(result).to eq(true)
   end
 
-  def fill_email(mail, from, to, body = nil, subject = nil, cc = nil)
-    result = mail.gsub("FROM", from).gsub("TO", to)
-    result.gsub!(/Hey.*/m, body)  if body
-    result.sub!(/We .*/, subject) if subject
-    result.sub!("CC", cc.presence || "")
-    result
-  end
-
   def email(email_name)
     fixture_file("emails/#{email_name}.eml")
   end