fix: prevent duplicate configuration lines in production.rb

[uurcank]

The generator was adding duplicate lines to production.rb each time it is run

  config.active_job.queue_adapter = :solid_queue
  config.solid_queue.connects_to = { database: { writing: :queue } }
  
  config.solid_queue.connects_to = { database: { writing: :queue } }

This PR refactors the generator to:

1. Check if the line already exists before adding it.
2. With this, generator logs to the terminal whether a new line was inserted along with gsub

[dhh]

Why are you running the generator more than once? I mean, I think it would be nice for it to be idempotent, but we gotta find a simpler way to do that than what's proposed.

[uurcank]

This happens when upgrading from one version of another to generate new files added by newer versions. I guess once a stable version is out, it would not happen or if you never run the generator again.

[uurcank]

perhaps we can include both in the replacement string like this

gsub_file Pathname(destination_root).join("config/environments/production.rb"),
      /(# )?config\.active_job\.queue_adapter\s+=.*/,
      "config.active_job.queue_adapter = :solid_queue\n  config.solid_queue.connects_to = { database: { writing: :queue } }"

[dhh]

I thought that was what we were already doing? Another option is just not to do a replacement if it's already set to solid_queue.

[uurcank]

Just to clarify the generator adds these two lines

config.active_job.queue_adapter = :solid_queue
config.solid_queue.connects_to = { database: { writing: :queue } }

but if generator runs again it ends adding another line because replacement string does not contain the whole thing

config.active_job.queue_adapter = :solid_queue
config.solid_queue.connects_to = { database: { writing: :queue } }

config.solid_queue.connects_to = { database: { writing: :queue } }

so this PR proposes to check separately whether config.solid_queue.connects_to = { database: { writing: :queue } } is present. Because config.active_job.queue_adapter may still be set to solid_queue but the second line could be missing.

I thought we could do all in a single replacement string (without +) but that still does not check if config.solid_queue.connects_to is there or not. So without using File.read we can check like this and it seems to be working

 # Replace or set `config.active_job.queue_adapter`
    gsub_file Pathname(destination_root).join("config/environments/production.rb"),
    /config\.active_job\.queue_adapter\s+=.*/,
    "config.active_job.queue_adapter = :solid_queue"

    # Inject `config.solid_queue.connects_to` if not already present
    gsub_file Pathname(destination_root).join("config/environments/production.rb"),
    /^\s*config\.solid_queue\.connects_to\s+=\s+\{.*\}\n/,
    '',
    verbose: false # If found, do nothing

    inject_into_file Pathname(destination_root).join("config/environments/production.rb"),
    "\n  config.solid_queue.connects_to = { database: { writing: :queue } }",
    after: /config\.active_job\.queue_adapter\s+=.*/

will push this shortly.

[Shaglock]

Hi DHH and @uurcank ,

I've encountered this issue as well while experimenting with Enlitenment and running their generators multiple times with different configurations. Initially, I didn't realize it was coming from Rails.

I believe the solution using File.read is robust and a bit more readable. Here's my suggestion that may be a little bit more readable. What do you think?

In any case, both solutions work fine for me as long as they are added. :)

  # Inject `config.solid_queue.connects_to` if not already present
  unless File.read(production_rb). include?("config.solid_queue.connects_to")
    inject_into_file production_rb, 
      "\n  config.solid_queue.connects_to = { database: { writing: :queue } }", 
      after: /config\.active_job\.queue_adapter\s+=.*/
  end

Thanks!

[rosa]

I ended up with a different but similar version in https://github.com/rails/solid_queue/pull/439, where we always delete solid_queue.connects_to first, and then proceed as before.

[rosa]

Thanks a lot for the work and ideas here, @uurcank and @Shaglock!