Enhance Rails railtie to respect existing rails configuration (#478)

* Infer defaults from Rails' config

* Use ActiveSupport.on_load hooks when setting Sitemap options

* Allow setting config_file without using ENV vars

* Allow lazily setting adapter class without forcing an eagerload

Author: jasonkarns

integration/spec/sitemap_generator/railtie_spec.rb

@@ -0,0 +1,145 @@
+require 'spec_helper'
+
+RSpec.describe "SitemapGenerator::Railtie" do
+  let(:app) { Rails.application }
+  let(:config) { app.config }
+  let(:initializers) { app.initializers.index_by(&:name) }
+
+  it "adds a top-level configuration namespace" do
+    expect(config.sitemap).to be_a ActiveSupport::OrderedOptions
+  end
+
+  after { config.sitemap.clear }
+
+  describe "set_configs initializer" do
+    subject(:initializer) { initializers["sitemap_generator.set_configs"] }
+
+    describe ".default_host" do
+      after { app.routes.default_url_options = config.action_controller.default_url_options = {} }
+
+      it "ignores Rails if set directly" do
+        app.routes.default_url_options = { host: "from_routes.test" }
+        config.sitemap.default_host = "http://custom.test"
+
+        initializer.run(app)
+
+        expect(config.sitemap.default_host).to eq "http://custom.test"
+      end
+
+      it "is inferred from Rails' routes default_url_options" do
+        app.routes.default_url_options = { host: "from_routes.test" }
+
+        initializer.run(app)
+
+        expect(config.sitemap.default_host).to eq "http://from_routes.test"
+      end
+
+      it "falls back to action_mailer, action_controller, and active_job" do
+        config.action_controller.default_url_options = { host: "from_action_controller.test" }
+
+        initializer.run(app)
+
+        expect(config.sitemap.default_host).to eq "http://from_action_controller.test"
+      end
+
+      it "doesn't construct a default_host if missing :host" do
+        config.action_controller.default_url_options = { trailing_slash: true }
+
+        initializer.run(app)
+
+        expect(config.sitemap.default_host).to be_nil
+      end
+    end
+
+    describe ".sitemaps_host" do
+      after { config.asset_host = config.action_controller.asset_host = nil }
+
+      it "can be set directly" do
+        config.action_controller.asset_host = "http://from_action_controller.test"
+        config.sitemap.sitemaps_host = "http://custom.test"
+
+        initializer.run(app)
+
+        expect(config.sitemap.sitemaps_host).to eq "http://custom.test"
+      end
+
+      it "is inferred from action_controller/assets_host" do
+        config.action_controller.asset_host = "http://from_action_controller.test"
+
+        initializer.run(app)
+
+        expect(config.sitemap.sitemaps_host).to eq "http://from_action_controller.test"
+      end
+
+      it "doesn't accept procs" do
+        config.action_controller.asset_host = -> { "dynamically construct hsot" }
+
+        initializer.run(app)
+
+        expect(config.sitemap.sitemaps_host).to be_nil
+      end
+    end
+
+    describe ".compress" do
+      # config.assets provided by Propshaft or Sprockets
+      before { config.assets = ActiveSupport::OrderedOptions[{gzip: true}] }
+      after { config.assets = nil }
+
+      it "is inferred from config.assets.gzip" do
+        initializer.run(app)
+
+        expect(config.sitemap.compress).to be true
+      end
+
+      it "can be set directly (nil != false)" do
+        config.sitemap.compress = false
+
+        initializer.run(app)
+
+        expect(config.sitemap.compress).to be false
+      end
+    end
+
+    describe ".public_path" do
+      after { app.paths["public"] = "public" }
+
+      it "can be set directly" do
+        config.sitemap.public_path = "custom"
+
+        initializer.run(app)
+
+        expect(config.sitemap.public_path).to eq "custom"
+      end
+
+      it "is inferred from Rails paths" do
+        app.paths["public"].unshift "inferred"
+
+        initializer.run(app)
+
+        expect(config.sitemap.public_path).to match "/inferred"
+      end
+    end
+  end
+
+  describe "config_file initializer" do
+    subject(:initializer) { initializers["sitemap_generator.config_file"] }
+
+    after { ENV.delete "CONFIG_FILE" }
+
+    it "sets CONFIG_FILE" do
+      config.sitemap.config_file = "custom.rb"
+
+      expect { initializer.run(app) }
+        .to change { ENV["CONFIG_FILE"] }.to("custom.rb")
+        .and change(config, :sitemap).from have_key(:config_file)
+    end
+
+    it "does not override CONFIG_FILE" do
+      ENV["CONFIG_FILE"] = "existing.rb"
+      config.sitemap.config_file = "override.rb"
+
+      expect { initializer.run(app) }
+        .to_not change { ENV["CONFIG_FILE"] }.from("existing.rb")
+    end
+  end
+end

integration/spec/sitemap_generator/tasks_spec.rb

@@ -69,6 +69,8 @@ class << self
     end
   end
 
+  # This group of tests needs reworked.
+  # They fail if rails app is reloaded or prepare hooks are re-run.
   describe "generate sitemap with normal config" do
     before :all do
       SitemapGenerator::Sitemap.reset!

lib/sitemap_generator/railtie.rb

@@ -2,8 +2,60 @@
 
 module SitemapGenerator
   class Railtie < Rails::Railtie
+    # Top level options object to namespace all settings
+    config.sitemap = ActiveSupport::OrderedOptions.new
+
     rake_tasks do
       load 'tasks/sitemap_generator_tasks.rake'
     end
+
+    # Recognize existing Rails options as defaults for config.sitemap.*
+    initializer 'sitemap_generator.set_configs' do |app|
+      # routes.default_url_options takes precedence, falling back to configs
+      url_opts = (app.default_url_options || {})
+                 .with_defaults(config.try(:action_controller).try(:default_url_options) || {})
+                 .with_defaults(config.try(:action_mailer).try(:default_url_options) || {})
+                 .with_defaults(config.try(:active_job).try(:default_url_options) || {})
+
+      config.sitemap.default_host ||= ActionDispatch::Http::URL.full_url_for(url_opts) if url_opts.key?(:host)
+
+      # Rails defaults action_controller.asset_host and action_mailer.asset_host
+      # to the top-level config.asset_host so we get that for free here.
+      config.sitemap.sitemaps_host ||= [
+        config.try(:action_controller).try(:asset_host),
+        config.try(:action_mailer).try(:asset_host)
+      ].grep(String).first
+
+      config.sitemap.compress = config.try(:assets).try(:gzip) if config.sitemap.compress.nil?
+
+      config.sitemap.public_path ||= app.paths['public'].first
+
+      # "Compile" config.sitemap options onto the Sitemap class.
+      config.after_initialize do
+        ActiveSupport.on_load(:sitemap_generator, yield: true) do |sitemap|
+          config.sitemap.except(:adapter).each { |k, v| sitemap.public_send("#{k}=", v) }
+        end
+      end
+    end
+
+    # Allow setting the CONFIG_FILE without relying on env var;
+    # (e.g in config/application or environments/*.rb)
+    initializer 'sitemap_generator.config_file' do
+      if (config_file = config.sitemap.delete(:config_file).presence) && ENV['CONFIG_FILE'].blank?
+        ENV['CONFIG_FILE'] = config_file
+      end
+    end
+
+    # Allow lazily setting the adapter class without forcing an autoload.
+    # (ie. string or symbol name; or Callable (proc/lambda/etc))
+    initializer 'sitemap_generator.adapter' do |app|
+      config.to_prepare do
+        ActiveSupport.on_load(:sitemap_generator) do
+          self.adapter = Utilities.find_adapter app.config.sitemap.adapter
+        end
+      end
+    end
   end
+
+  ActiveSupport.run_load_hooks(:sitemap_generator, Sitemap)
 end

lib/sitemap_generator/utilities.rb

@@ -180,5 +180,29 @@ def ellipsis(string, max)
     def bytesize(string)
       string.respond_to?(:bytesize) ? string.bytesize : string.length
     end
+
+    # Looks up an adapter from various sources:
+    # 1. symbol/string names are camelized and constantized
+    #   - :fog becomes SitemapGenerator::FogAdapter
+    #   - 'external/adapter_class' becomes External::AdapterClass
+    # 2. classes are instantiated
+    # 3. callables (procs, lambdas, #call) are "called"
+    # All steps recurse, so a proc could return a string that names a class that is instantiated.
+    #
+    # This method is used by the Rails railtie and as such,
+    # safely depends on ActiveSupport::Inflector.
+    def find_adapter(candidate)
+      if candidate.is_a?(Symbol) || candidate.is_a?(String)
+        candidate.to_s.camelize.then { |name|
+          "SitemapGenerator::#{name}Adapter".safe_constantize || name.safe_constantize
+        }.then { |c| find_adapter(c) }
+      elsif candidate.respond_to?(:new)
+        find_adapter candidate.new
+      elsif candidate.respond_to?(:call)
+        find_adapter candidate.call
+      else
+        candidate
+      end
+    end
   end
 end

spec/sitemap_generator/utilities_spec.rb

@@ -100,4 +100,47 @@
       expect(SitemapGenerator::Utilities.ellipsis('aaa', 1)).to eq('...')
     end
   end
+
+  describe 'find_adapter' do
+    require 'active_support/core_ext/string/inflections'
+
+    context 'when candidate is an adapter (#write)' do
+      let(:candidate) { SitemapGenerator::FileAdapter.new }
+      it "returns the same candidate" do
+        expect(subject.find_adapter candidate).to be candidate
+      end
+    end
+
+    context 'when candidate is a callable (proc, lambda, #call)' do
+      it "invokes call and returns that" do
+        expect(subject.find_adapter -> { SitemapGenerator::FileAdapter.new }).to be_a SitemapGenerator::FileAdapter
+      end
+
+      it "recurses on the return" do
+        expect(subject.find_adapter -> { SitemapGenerator::FileAdapter }).to be_a SitemapGenerator::FileAdapter
+        expect(subject.find_adapter -> { :file }).to be_a SitemapGenerator::FileAdapter
+      end
+    end
+
+    context 'when candidate is a class (#new)' do
+      it "instantiates the class" do
+        expect(subject.find_adapter SitemapGenerator::FileAdapter).to be_a SitemapGenerator::FileAdapter
+      end
+    end
+
+    context 'when candidate is a class name' do
+      it "constantizes" do
+        expect(subject.find_adapter :file).to be_a SitemapGenerator::FileAdapter
+        expect(subject.find_adapter "file").to be_a SitemapGenerator::FileAdapter
+        expect(subject.find_adapter 'sitemap_generator/file_adapter').to be_a SitemapGenerator::FileAdapter
+      end
+    end
+
+    context 'when candidate is anything else' do
+      let(:candidate) { double }
+      it "returns the same candidate" do
+        expect(subject.find_adapter candidate).to be candidate
+      end
+    end
+  end
 end