fix: use a safer gsub_file and update/remove file gsubs that were n…

…o longer doing anything (#533) So it turns out that `gsub_file` does not actually check if it matched anything and so we have a few misc. changes silently not being applied due to changes in Rails 7.1. This addresses that by switching us to use `gsub_file!` which reads the file into memory before it's gsub'd and then compares the results to make sure it actually changed. I've opened rails/thor#874 to add this to Thor itself
ackama · Mar 8, 2024 · dd5b3e9 · dd5b3e9
1 parent a46e96d
commit dd5b3e9
Show file tree

Hide file tree

Showing 17 changed files with 129 additions and 141 deletions.
diff --git a/template.rb b/template.rb
@@ -381,4 +381,12 @@ def create_initial_migration
   run_with_clean_bundler_env "bin/rake db:migrate"
 end
 
+def gsub_file!(path, flag, *args, &block)
+  content = File.binread(path)
+
+  gsub_file(path, flag, *args, &block)
+
+  raise StandardError, "the contents of #{path} did not change!" if content == File.binread(path)
+end
+
 apply_template!
diff --git a/variants/backend-base/app/template.rb b/variants/backend-base/app/template.rb
@@ -17,6 +17,6 @@
 empty_directory_with_keep_file "app/services"
 
 # Configure the default mailer to use the our default from address
-gsub_file "app/mailers/application_mailer.rb",
-          "default from: 'from@example.com'",
-          "default from: Rails.application.config.app.mail_from"
+gsub_file! "app/mailers/application_mailer.rb",
+           /default from: ['"]from@example\.com['"]/,
+           "default from: Rails.application.config.app.mail_from"
diff --git a/variants/backend-base/config/application.rb b/variants/backend-base/config/application.rb
@@ -1,6 +1,6 @@
-gsub_file "config/application.rb",
-          "# config.time_zone = 'Central Time (US & Canada)'",
-          "config.time_zone = 'Wellington'"
+gsub_file! "config/application.rb",
+           /# config.time_zone = ['"]Central Time \(US & Canada\)['"]/,
+           "config.time_zone = 'Wellington'"
 
 insert_into_file "config/application.rb", after: /^require_relative ['"]boot['"]/ do
   # the empty line at the beginning of this string is required

diff --git a/variants/backend-base/config/environments/development.rb b/variants/backend-base/config/environments/development.rb
@@ -15,10 +15,6 @@
   RUBY
 end
 
-gsub_file "config/environments/development.rb",
-          "join('tmp', 'caching-dev.txt')",
-          'join("tmp/caching-dev.txt")'
-
-gsub_file "config/environments/development.rb",
-          "config.action_controller.raise_on_missing_callback_actions = true",
-          "# config.action_controller.raise_on_missing_callback_actions = true"
+gsub_file! "config/environments/development.rb",
+           "config.action_controller.raise_on_missing_callback_actions = true",
+           "# config.action_controller.raise_on_missing_callback_actions = true"
diff --git a/variants/backend-base/config/environments/production.rb b/variants/backend-base/config/environments/production.rb
@@ -8,14 +8,14 @@
   RUBY
 end
 
-gsub_file "config/environments/production.rb",
-          "# config.force_ssl = true",
-          <<~RUBY
-            ##
-            # `force_ssl` defaults to on. Set `force_ssl` to false if (and only if) RAILS_FORCE_SSL=false, otherwise set it to true.
-            #
-            config.force_ssl = ENV.fetch("RAILS_FORCE_SSL", "true").downcase != "false"
-          RUBY
+gsub_file! "config/environments/production.rb",
+           "config.force_ssl = true",
+           <<~RUBY
+             ##
+             # `force_ssl` defaults to on. Set `force_ssl` to false if (and only if) RAILS_FORCE_SSL=false, otherwise set it to true.
+             #
+             config.force_ssl = ENV.fetch("RAILS_FORCE_SSL", "true").downcase != "false"
+           RUBY
 
 insert_into_file "config/environments/production.rb",
                  after: /# config\.action_mailer\.raise_deliv.*\n/ do
@@ -42,13 +42,13 @@
   RUBY
 end
 
-gsub_file "config/environments/production.rb",
-          "config.log_level = :info",
-          'config.log_level = ENV.fetch("LOG_LEVEL", "info").to_sym'
+gsub_file! "config/environments/production.rb",
+           'ENV.fetch("RAILS_LOG_LEVEL", "info")',
+           'ENV.fetch("RAILS_LOG_LEVEL", ENV.fetch("LOG_LEVEL", "info"))'
 
-gsub_file "config/environments/production.rb",
-          "ActiveSupport::Logger.new(STDOUT)",
-          "ActiveSupport::Logger.new($stdout)"
+gsub_file! "config/environments/production.rb",
+           "ActiveSupport::Logger.new(STDOUT)",
+           "ActiveSupport::Logger.new($stdout)"
 
 insert_into_file "config/environments/production.rb",
                  after: /.*config\.public_file_server\.enabled.*\n/ do

diff --git a/variants/backend-base/config/environments/test.rb b/variants/backend-base/config/environments/test.rb
@@ -1,7 +1,3 @@
-gsub_file "config/environments/test.rb",
-          "config.eager_load = false",
-          "config.eager_load = defined?(SimpleCov).present?"
-
 insert_into_file \
   "config/environments/test.rb",
   after: /config\.action_mailer\.delivery_method = :test\n/ do
@@ -17,6 +13,6 @@
   RUBY
 end
 
-gsub_file "config/environments/test.rb",
-          "config.action_controller.raise_on_missing_callback_actions = true",
-          "# config.action_controller.raise_on_missing_callback_actions = true"
+gsub_file! "config/environments/test.rb",
+           "config.action_controller.raise_on_missing_callback_actions = true",
+           "# config.action_controller.raise_on_missing_callback_actions = true"
diff --git a/variants/backend-base/config/template.rb b/variants/backend-base/config/template.rb
@@ -17,9 +17,9 @@
 copy_file "variants/backend-base/config/initializers/check_env.rb", "config/initializers/check_env.rb"
 copy_file "variants/backend-base/config/initializers/sentry.rb", "config/initializers/sentry.rb"
 
-gsub_file "config/initializers/filter_parameter_logging.rb", /\[:password\]/ do
-  "%w[password secret session cookie csrf]"
-end
+gsub_file! "config/initializers/filter_parameter_logging.rb",
+           / {2}:passw, :secret, /,
+           "  :passw, :secret, :session, :cookie, :csrf, "
 
 apply "variants/backend-base/config/environments/development.rb"
 apply "variants/backend-base/config/environments/production.rb"
@@ -59,7 +59,7 @@
 EO_ROUTES
 
 if File.exist? "config/storage.yml"
-  gsub_file "config/storage.yml", /#   service: S3/ do
+  gsub_file! "config/storage.yml", /#   service: S3/ do
     <<~YAML
       #   service: S3
       #   upload:

diff --git a/variants/deploy_with_ackama_ec2_capistrano/template.rb b/variants/deploy_with_ackama_ec2_capistrano/template.rb
@@ -177,7 +177,7 @@
 
 EO_RUBY
 
-gsub_file("config/deploy.rb", old_generated_cap_config_snippet, new_ackama_cap_config_snippet)
+gsub_file!("config/deploy.rb", old_generated_cap_config_snippet, new_ackama_cap_config_snippet)
 
 insert_into_file "Capfile", after: /install_plugin Capistrano::SCM::Git/ do
   <<~EO_RUBY

diff --git a/variants/deploy_with_capistrano/template.rb b/variants/deploy_with_capistrano/template.rb
@@ -165,7 +165,7 @@
 
 EO_RUBY
 
-gsub_file("config/deploy.rb", old_generated_cap_config_snippet, new_ackama_cap_config_snippet)
+gsub_file!("config/deploy.rb", old_generated_cap_config_snippet, new_ackama_cap_config_snippet)
 
 insert_into_file "Capfile", after: /install_plugin Capistrano::SCM::Git/ do
   <<~EO_RUBY

diff --git a/variants/devise/template.rb b/variants/devise/template.rb
@@ -10,29 +10,26 @@
 TERMINAL.puts_header "Generating User model with devise"
 run "bundle exec rails generate devise User"
 
-gsub_file "app/models/user.rb",
-          ":validatable",
-          ":validatable, :lockable"
+gsub_file! "app/models/user.rb",
+           ":validatable",
+           ":validatable, :lockable"
 
 devise_migration_filename = Dir.children("db/migrate").find { |filename| filename.end_with?("_devise_create_users.rb") }
 devise_migration_path = "db/migrate/#{devise_migration_filename}"
 
 TERMINAL.puts_header "Tweaking auto-generated devise migration '#{devise_migration_path}'"
-gsub_file devise_migration_path,
-          "      # t.integer  :failed_attempts",
-          "      t.integer  :failed_attempts"
-gsub_file devise_migration_path,
-          "      # t.string   :unlock_token",
-          "      t.string   :unlock_token"
-gsub_file devise_migration_path,
-          "      # t.datetime :locked_at",
-          "      t.datetime :locked_at"
-gsub_file devise_migration_path,
-          "      # add_index :users, :unlock_token",
-          "      add_index :users, :unlock_token"
-gsub_file devise_migration_path,
-          /  # add_index :users, :unlock_token.+/,
-          "  add_index :users, :unlock_token, unique: true"
+gsub_file! devise_migration_path,
+           "      # t.integer  :failed_attempts",
+           "      t.integer  :failed_attempts"
+gsub_file! devise_migration_path,
+           "      # t.string   :unlock_token",
+           "      t.string   :unlock_token"
+gsub_file! devise_migration_path,
+           "      # t.datetime :locked_at",
+           "      t.datetime :locked_at"
+gsub_file! devise_migration_path,
+           "    # add_index :users, :unlock_token",
+           "    add_index :users, :unlock_token"
 
 TERMINAL.puts_header "Running db migration"
 run "bundle exec rails db:migrate"
@@ -45,51 +42,51 @@
 #
 TERMINAL.puts_header "Tweaking config/initializers/devise.rb"
 
-gsub_file "config/initializers/devise.rb",
-          "  config.mailer_sender = 'please-change-me-at-config-initializers-devise@example.com'",
-          "  config.mailer_sender = Rails.application.config.app.mail_from"
+gsub_file! "config/initializers/devise.rb",
+           "  config.mailer_sender = 'please-change-me-at-config-initializers-devise@example.com'",
+           "  config.mailer_sender = Rails.application.config.app.mail_from"
 
-gsub_file "config/initializers/devise.rb",
-          "  # config.scoped_views = false",
-          "  config.scoped_views = true"
+gsub_file! "config/initializers/devise.rb",
+           "  # config.scoped_views = false",
+           "  config.scoped_views = true"
 
-gsub_file "config/initializers/devise.rb",
-          "  config.password_length = 6..128",
-          "  config.password_length = 16..128"
+gsub_file! "config/initializers/devise.rb",
+           "  config.password_length = 6..128",
+           "  config.password_length = 16..128"
 
-gsub_file "config/initializers/devise.rb",
-          "  # config.paranoid = true",
-          "  config.paranoid = true"
+gsub_file! "config/initializers/devise.rb",
+           "  # config.paranoid = true",
+           "  config.paranoid = true"
 
-gsub_file "config/initializers/devise.rb",
-          /  # config.secret_key = '.+'/,
-          "  # config.secret_key = 'do_not_put_secrets_in_source_control_please'"
+gsub_file! "config/initializers/devise.rb",
+           /  # config.secret_key = '.+'/,
+           "  # config.secret_key = 'do_not_put_secrets_in_source_control_please'"
 
-gsub_file "config/initializers/devise.rb",
-          /  # config.lock_strategy = .+/,
-          "  config.lock_strategy = :failed_attempts"
+gsub_file! "config/initializers/devise.rb",
+           /  # config.lock_strategy = .+/,
+           "  config.lock_strategy = :failed_attempts"
 
-gsub_file "config/initializers/devise.rb",
-          /  # config.unlock_strategy = .+/,
-          "  config.unlock_strategy = :email"
+gsub_file! "config/initializers/devise.rb",
+           /  # config.unlock_strategy = .+/,
+           "  config.unlock_strategy = :email"
 
-gsub_file "config/initializers/devise.rb",
-          "  # config.parent_mailer = 'ActionMailer::Base'",
-          "  config.parent_mailer = 'ApplicationMailer'"
+gsub_file! "config/initializers/devise.rb",
+           "  # config.parent_mailer = 'ActionMailer::Base'",
+           "  config.parent_mailer = 'ApplicationMailer'"
 
-gsub_file "config/initializers/devise.rb",
-          /  # config.maximum_attempts = .+/,
-          <<-EO_CHUNK
+gsub_file! "config/initializers/devise.rb",
+           /  # config.maximum_attempts = .+/,
+           <<-EO_CHUNK
   #
   # https://www.nzism.gcsb.govt.nz/ism-document/#1887 recommends 3 as a default. FYI to
   # be fully compliant with https://www.nzism.gcsb.govt.nz/ism-document/#1887 then only
   # Administrators should be able to unlock.
   config.maximum_attempts = 3
-          EO_CHUNK
+           EO_CHUNK
 
-gsub_file "config/initializers/devise.rb",
-          /  # config.last_attempt_warning = .+/,
-          "  config.last_attempt_warning = true"
+gsub_file! "config/initializers/devise.rb",
+           /  # config.last_attempt_warning = .+/,
+           "  config.last_attempt_warning = true"
 
 ##
 # Add a block to config/routes.rb demonstrating how to create authenticated
@@ -140,13 +137,13 @@
 
 copy_file "app/controllers/users/sessions_controller.rb"
 
-gsub_file "config/routes.rb",
-          "devise_for :users",
-          <<~EO_DEVISE
-            devise_for :users, controllers: {
-              sessions: "users/sessions"
-            }
-          EO_DEVISE
+gsub_file! "config/routes.rb",
+           "devise_for :users",
+           <<~EO_DEVISE
+             devise_for :users, controllers: {
+               sessions: "users/sessions"
+             }
+           EO_DEVISE
 
 insert_into_file "app/models/user.rb", before: /^end/ do
   <<~'RUBY'
@@ -202,14 +199,14 @@ def authenticatable_salt
 copy_file "spec/requests/session_cookie_expiry_spec.rb"
 
 # tell pundit not to check that authorization was called on devise controllers
-gsub_file("app/controllers/application_controller.rb",
-          "after_action :verify_authorized, except: :index",
-          "after_action :verify_authorized, except: :index, unless: :devise_controller?"
-         )
-gsub_file("app/controllers/application_controller.rb",
-          "after_action :verify_policy_scoped, only: :index",
-          "after_action :verify_policy_scoped, only: :index, unless: :devise_controller?"
-         )
+gsub_file!("app/controllers/application_controller.rb",
+           "after_action :verify_authorized, except: :index",
+           "after_action :verify_authorized, except: :index, unless: :devise_controller?"
+          )
+gsub_file!("app/controllers/application_controller.rb",
+           "after_action :verify_policy_scoped, only: :index",
+           "after_action :verify_policy_scoped, only: :index, unless: :devise_controller?"
+          )
 
 TERMINAL.puts_header "Running rubocop -A to fix formatting in files generated by devise"
 run "bundle exec rubocop -A -c ./.rubocop.yml"

diff --git a/variants/frontend-base-typescript/template.rb b/variants/frontend-base-typescript/template.rb
@@ -35,7 +35,7 @@ def rename_js_file_to_ts(file)
 copy_file ".eslintrc.js", force: true
 copy_file "types.d.ts", force: true
 
-gsub_file(
+gsub_file!(
   "app/frontend/packs/application.ts",
   "process.env.SENTRY_ENV || process.env.RAILS_ENV",
   "process.env.SENTRY_ENV ?? process.env.RAILS_ENV"
@@ -53,7 +53,7 @@ def rename_js_file_to_ts(file)
     return String(images(name));
   };
 EO_TS_ENABLE_IMAGES
-gsub_file("app/frontend/packs/application.ts", js_load_images_chunk, ts_load_images_chunk, force: true)
+gsub_file!("app/frontend/packs/application.ts", js_load_images_chunk, ts_load_images_chunk, force: true)
 
 update_package_json do |package_json|
   package_json["scripts"]["typecheck"] = "tsc -p . --noEmit"

diff --git a/variants/frontend-base/sentry/template.rb b/variants/frontend-base/sentry/template.rb
@@ -46,7 +46,3 @@
     //
   EO_JS
 end
-
-gsub_file "config/initializers/content_security_policy.rb",
-          /# policy.report_uri ".+"/,
-          'policy.report_uri(ENV["SENTRY_CSP_HEADER_REPORT_ENDPOINT"]) if ENV["SENTRY_CSP_HEADER_REPORT_ENDPOINT"]'