From 5cbbd140ba6f94f40bb77e652449351f0814e09c Mon Sep 17 00:00:00 2001 From: Richard Kramer <34233066+richard-kramer@users.noreply.github.com> Date: Fri, 12 May 2023 03:40:04 +0200 Subject: [PATCH 1/3] Fix Zeitwerk conventions for `Refinery::Pages::Finder` (#3524) Resolves #3523 The issue stems from the definition of `Refinery::Pages::Finder` and its sibling classes (`Refinery::Pages::FinderByTitle` and so on). These are all defined beside each other in [`pages/lib/refinery/pages/finder.rb`](https://github.com/refinery/refinerycms/blob/aee49a603860bf7d5fdb1532b9add5f4e7f839f4/pages/lib/refinery/pages/finder.rb) which gets required in [`pages/app/models/refinery/page.rb`](https://github.com/refinery/refinerycms/blob/aee49a603860bf7d5fdb1532b9add5f4e7f839f4/pages/app/models/refinery/page.rb#L6). When reloading, Zeitwerk reloads the model, which in turn loads the finder again. But because the file is named `finder.rb`, only `Refinery::Pages::Finder` gets unloaded and reloaded again. All other classes stay loaded and would be patched by the reload. But because `Refinery::Pages::Finder` has been reloaded and initialized again, it got a new `object_id`, thus triggering the **`superclass mismatch`** error. The solution, that fixed the error, was to move all sibling-classes of `Refinery::Pages::Finder` into it, making them children of `Refinery::Pages::Finder`. Because they are only called inside `Refinery::Pages::Finder` itself, this *should* not make problems. --- pages/lib/refinery/pages/finder.rb | 175 ++++++++++++++--------------- 1 file changed, 87 insertions(+), 88 deletions(-) diff --git a/pages/lib/refinery/pages/finder.rb b/pages/lib/refinery/pages/finder.rb index fa030d7d96..f32af40a97 100644 --- a/pages/lib/refinery/pages/finder.rb +++ b/pages/lib/refinery/pages/finder.rb @@ -57,123 +57,122 @@ def translations_conditions(original_conditions) translations_conditions end - end + class FinderByTitle < Finder + def initialize(title) + @title = title + @conditions = default_conditions + end - class FinderByTitle < Finder - def initialize(title) - @title = title - @conditions = default_conditions - end + def default_conditions + { :title => title } + end - def default_conditions - { :title => title } + private + attr_accessor :title end - private - attr_accessor :title - end + class FinderBySlug < Finder + def initialize(slug, conditions) + @slug = slug + @conditions = default_conditions.merge(conditions) + end - class FinderBySlug < Finder - def initialize(slug, conditions) - @slug = slug - @conditions = default_conditions.merge(conditions) - end + def default_conditions + { + :locale => Refinery::I18n.frontend_locales.map(&:to_s), + :slug => slug + } + end - def default_conditions - { - :locale => Refinery::I18n.frontend_locales.map(&:to_s), - :slug => slug - } + private + attr_accessor :slug end - private - attr_accessor :slug - end - - class FinderByPath - def initialize(path) - @path = path - end + class FinderByPath + def initialize(path) + @path = path + end - def find - if slugs_scoped_by_parent? - FinderByScopedPath.new(path).find - else - FinderByUnscopedPath.new(path).find + def find + if slugs_scoped_by_parent? + FinderByScopedPath.new(path).find + else + FinderByUnscopedPath.new(path).find + end end - end - private - attr_accessor :path + private + attr_accessor :path - def slugs_scoped_by_parent? - ::Refinery::Pages.scope_slug_by_parent - end + def slugs_scoped_by_parent? + ::Refinery::Pages.scope_slug_by_parent + end - def by_slug(slug_path, conditions = {}) - Finder.by_slug(slug_path, conditions) + def by_slug(slug_path, conditions = {}) + Finder.by_slug(slug_path, conditions) + end end - end - class FinderByScopedPath < FinderByPath - def find - # With slugs scoped to the parent page we need to find a page by its full path. - # For example with about/example we would need to find 'about' and then its child - # called 'example' otherwise it may clash with another page called /example. - page = parent_page - while page && path_segments.any? do - page = next_page(page) + class FinderByScopedPath < FinderByPath + def find + # With slugs scoped to the parent page we need to find a page by its full path. + # For example with about/example we would need to find 'about' and then its child + # called 'example' otherwise it may clash with another page called /example. + page = parent_page + while page && path_segments.any? do + page = next_page(page) + end + page end - page - end - private + private - def path_segments - @path_segments ||= path.split('/').select(&:present?) - end + def path_segments + @path_segments ||= path.split('/').select(&:present?) + end - def parent_page - parent_page_segment = path_segments.shift - if parent_page_segment.friendly_id? - by_slug(parent_page_segment, :parent_id => nil).first - else - Page.find(parent_page_segment) + def parent_page + parent_page_segment = path_segments.shift + if parent_page_segment.friendly_id? + by_slug(parent_page_segment, :parent_id => nil).first + else + Page.find(parent_page_segment) + end end - end - def next_page(page) - slug_or_id = path_segments.shift - page.children.by_slug(slug_or_id).first || page.children.find(slug_or_id) + def next_page(page) + slug_or_id = path_segments.shift + page.children.by_slug(slug_or_id).first || page.children.find(slug_or_id) + end end - end - class FinderByUnscopedPath < FinderByPath - def find - by_slug(path).first + class FinderByUnscopedPath < FinderByPath + def find + by_slug(path).first + end end - end - class FinderByPathOrId - def initialize(path, id) - @path = path - @id = id - end + class FinderByPathOrId + def initialize(path, id) + @path = path + @id = id + end - def find - if path.present? - if path.friendly_id? - FinderByPath.new(path).find - else - Page.friendly.find(path) + def find + if path.present? + if path.friendly_id? + FinderByPath.new(path).find + else + Page.friendly.find(path) + end + elsif id.present? + Page.friendly.find(id) end - elsif id.present? - Page.friendly.find(id) end - end - private - attr_accessor :id, :path + private + attr_accessor :id, :path + end end end end From 1d3222b2b8427a61ebea1d97a7078b21cb213963 Mon Sep 17 00:00:00 2001 From: Philip Arndt Date: Tue, 23 May 2023 10:12:10 +1200 Subject: [PATCH 2/3] Update `File.exists?` to `File.exist?` (#3529) The former was removed in more recent versions of Ruby. --- Rakefile | 2 +- .../refinery/dummy/dummy_generator.rb | 2 +- .../refinery/engine/templates/Rakefile | 2 +- .../extension_plural_name_generator.rb.erb | 2 +- core/spec/lib/refinery/cli_spec.rb | 2 +- .../4 - Refinery Extensions/3 - Testing.md | 2 +- .../4 - Docker-deploy-to-DigitalOcean.md | 30 +++++++++---------- .../refinery/pages/pages_generator.rb | 2 +- .../support/refinery/pages/caching_helpers.rb | 2 +- tasks/release.rake | 2 +- .../refinery/testing/templates/Guardfile | 2 +- 11 files changed, 25 insertions(+), 25 deletions(-) diff --git a/Rakefile b/Rakefile index 75e56a31f1..e294ee1fc1 100644 --- a/Rakefile +++ b/Rakefile @@ -7,7 +7,7 @@ end APP_RAKEFILE = File.expand_path("../spec/dummy/Rakefile", __FILE__) -if File.exists?(APP_RAKEFILE) +if File.exist?(APP_RAKEFILE) load 'rails/tasks/engine.rake' end diff --git a/core/lib/generators/refinery/dummy/dummy_generator.rb b/core/lib/generators/refinery/dummy/dummy_generator.rb index c22174ca9d..80a9556f69 100644 --- a/core/lib/generators/refinery/dummy/dummy_generator.rb +++ b/core/lib/generators/refinery/dummy/dummy_generator.rb @@ -83,7 +83,7 @@ def module_name def application_definition @application_definition ||= begin dummy_application_path = File.expand_path("#{dummy_path}/config/application.rb", destination_root) - unless options[:pretend] || !File.exists?(dummy_application_path) + unless options[:pretend] || !File.exist?(dummy_application_path) contents = File.read(dummy_application_path) contents[(contents.index("module #{module_name}"))..-1] end diff --git a/core/lib/generators/refinery/engine/templates/Rakefile b/core/lib/generators/refinery/engine/templates/Rakefile index 834f8539f5..9090376655 100644 --- a/core/lib/generators/refinery/engine/templates/Rakefile +++ b/core/lib/generators/refinery/engine/templates/Rakefile @@ -8,7 +8,7 @@ end ENGINE_PATH = File.dirname(__FILE__) APP_RAKEFILE = File.expand_path "../spec/dummy/Rakefile", __FILE__ -if File.exists? APP_RAKEFILE +if File.exist? APP_RAKEFILE load 'rails/tasks/engine.rake' end diff --git a/core/lib/generators/refinery/engine/templates/lib/generators/refinery/extension_plural_name_generator.rb.erb b/core/lib/generators/refinery/engine/templates/lib/generators/refinery/extension_plural_name_generator.rb.erb index 27f09b1fea..8c20155a51 100644 --- a/core/lib/generators/refinery/engine/templates/lib/generators/refinery/extension_plural_name_generator.rb.erb +++ b/core/lib/generators/refinery/engine/templates/lib/generators/refinery/extension_plural_name_generator.rb.erb @@ -6,7 +6,7 @@ module Refinery end def append_load_seed_data - create_file 'db/seeds.rb' unless File.exists?(File.join(destination_root, 'db', 'seeds.rb')) + create_file 'db/seeds.rb' unless File.exist?(File.join(destination_root, 'db', 'seeds.rb')) append_file 'db/seeds.rb', :verbose => true do <<-EOH diff --git a/core/spec/lib/refinery/cli_spec.rb b/core/spec/lib/refinery/cli_spec.rb index f68a5b0711..6c783881ab 100644 --- a/core/spec/lib/refinery/cli_spec.rb +++ b/core/spec/lib/refinery/cli_spec.rb @@ -98,7 +98,7 @@ Array(spec_success_message).each do |message_fragment| expect(msg).to include(message_fragment) end - expect(File.exists?(Rails.root.join(copied_file_location))).to be_truthy + expect(File.exist?(Rails.root.join(copied_file_location))).to be_truthy end end end diff --git a/doc/guides/4 - Refinery Extensions/3 - Testing.md b/doc/guides/4 - Refinery Extensions/3 - Testing.md index 0f417d3fd3..e4b75083f9 100644 --- a/doc/guides/4 - Refinery Extensions/3 - Testing.md +++ b/doc/guides/4 - Refinery Extensions/3 - Testing.md @@ -32,7 +32,7 @@ Add the following lines to your extension's Rakefile ENGINE_PATH = File.dirname(*FILE*) APP_RAKEFILE = File.expand_path("../spec/dummy/Rakefile", *FILE*) -load 'rails/tasks/extension.rake' if File.exists?(APP_RAKEFILE) +load 'rails/tasks/extension.rake' if File.exist?(APP_RAKEFILE) require "refinerycms-testing" Refinery::Testing::Railtie.load_dummy_tasks ENGINE_PATH diff --git a/doc/guides/7 - Hosting and Deployment/4 - Docker-deploy-to-DigitalOcean.md b/doc/guides/7 - Hosting and Deployment/4 - Docker-deploy-to-DigitalOcean.md index 7b7460c820..3a14e0efe8 100644 --- a/doc/guides/7 - Hosting and Deployment/4 - Docker-deploy-to-DigitalOcean.md +++ b/doc/guides/7 - Hosting and Deployment/4 - Docker-deploy-to-DigitalOcean.md @@ -50,7 +50,7 @@ bundle exec puma -C config/puma.rb ``` Apply executable permission: -``` +``` chmod +x bin/run.docker.sh ``` @@ -68,7 +68,7 @@ done mkdir -p /app/shared/nginx/cache/dragonfly ``` Apply executable permission: -``` +``` chmod +x run.symlinks.docker.sh ``` @@ -186,7 +186,7 @@ threads threads_min, threads_max environment ENV.fetch("RAILS_ENV") { "development" } # Executed in Docker container -if File.exists?('/.dockerenv') +if File.exist?('/.dockerenv') app_dir = File.expand_path("../..", __FILE__) stdout_redirect "#{app_dir}/log/puma.stdout.log", "#{app_dir}/log/puma.stderr.log", true @@ -462,7 +462,7 @@ if you are going only to play with the droplet for few hours. Install the `ufw` firewall via `apt-get install ufw` and do some initial settings: ``` ufw default deny incoming -ufw allow 22 +ufw allow 22 ufw allow 5000 ufw allow 80 ufw allow 443 @@ -686,16 +686,16 @@ We can now delete the public allow 5433 rule in ufw: ``` To Action From -- ------ ---- -[ 1] 22 ALLOW IN Anywhere -[ 2] 5000 ALLOW IN Anywhere -[ 3] 80 ALLOW IN Anywhere -[ 4] 443 ALLOW IN Anywhere -[ 5] 5433 ALLOW IN Anywhere -[ 6] 5433 ALLOW IN 172.18.0.0/16 -[ 7] 22 (v6) ALLOW IN Anywhere (v6) -[ 8] 5000 (v6) ALLOW IN Anywhere (v6) -[ 9] 80 (v6) ALLOW IN Anywhere (v6) -[10] 443 (v6) ALLOW IN Anywhere (v6) +[ 1] 22 ALLOW IN Anywhere +[ 2] 5000 ALLOW IN Anywhere +[ 3] 80 ALLOW IN Anywhere +[ 4] 443 ALLOW IN Anywhere +[ 5] 5433 ALLOW IN Anywhere +[ 6] 5433 ALLOW IN 172.18.0.0/16 +[ 7] 22 (v6) ALLOW IN Anywhere (v6) +[ 8] 5000 (v6) ALLOW IN Anywhere (v6) +[ 9] 80 (v6) ALLOW IN Anywhere (v6) +[10] 443 (v6) ALLOW IN Anywhere (v6) [11] 5433 (v6) ALLOW IN Anywhere (v6) ``` - to delete 5th and 11th rules, run `ufw delete 5` and `ufw delete 11` @@ -705,7 +705,7 @@ use the Postgres with non-docker on `5432` and Postgres from Docker via `5433` o Restart the docker-compose again, it should start correctly. -Now we will check, if the `5433` port is available from the public. From your local machine, run: (replace IP) +Now we will check, if the `5433` port is available from the public. From your local machine, run: (replace IP) `nmap 46.101.99.215` You should not see `5433` port available to public. diff --git a/pages/lib/generators/refinery/pages/pages_generator.rb b/pages/lib/generators/refinery/pages/pages_generator.rb index c1f530bde6..cbc700184e 100644 --- a/pages/lib/generators/refinery/pages/pages_generator.rb +++ b/pages/lib/generators/refinery/pages/pages_generator.rb @@ -15,7 +15,7 @@ def generate_pages_initializer end def append_load_seed_data - create_file "db/seeds.rb" unless File.exists?(File.join(destination_root, 'db', 'seeds.rb')) + create_file "db/seeds.rb" unless File.exist?(File.join(destination_root, 'db', 'seeds.rb')) append_file 'db/seeds.rb', :verbose => true do <<-EOH diff --git a/pages/spec/support/refinery/pages/caching_helpers.rb b/pages/spec/support/refinery/pages/caching_helpers.rb index e25a6f1d33..1c694a80fa 100644 --- a/pages/spec/support/refinery/pages/caching_helpers.rb +++ b/pages/spec/support/refinery/pages/caching_helpers.rb @@ -15,7 +15,7 @@ def cache_page(page) RSpec::Matchers.define :be_cached do match do |page| - File.exists?(cached_file_path(page)) + File.exist?(cached_file_path(page)) end end diff --git a/tasks/release.rake b/tasks/release.rake index 5eb7b32a35..561349b241 100644 --- a/tasks/release.rake +++ b/tasks/release.rake @@ -14,7 +14,7 @@ root = File.expand_path('../../', __FILE__) task :clean do package_dir = "#{root}/pkg" - mkdir_p package_dir unless File.exists?(package_dir) + mkdir_p package_dir unless File.exist?(package_dir) rm_f gem_filename end diff --git a/testing/lib/generators/refinery/testing/templates/Guardfile b/testing/lib/generators/refinery/testing/templates/Guardfile index 550b817a1f..63bdcdccdd 100644 --- a/testing/lib/generators/refinery/testing/templates/Guardfile +++ b/testing/lib/generators/refinery/testing/templates/Guardfile @@ -14,7 +14,7 @@ guard 'spork', :wait => 60, :cucumber => false, :rspec_env => { 'RAILS_ENV' => ' end guard 'rspec', :version => 2, :spec_paths => extensions.map{ |e| "#{e}/spec"}, - :cli => (File.read('.rspec').split("\n").join(' ') if File.exists?('.rspec')) do + :cli => (File.read('.rspec').split("\n").join(' ') if File.exist?('.rspec')) do extensions.each do |extension| watch(%r{^#{extension}/spec/.+_spec\.rb$}) watch(%r{^#{extension}/app/(.+)\.rb$}) { |m| "#{extension}/spec/#{m[1]}_spec.rb" } From 04f07c1e661dae7a0501d075e104a52fce4b71b1 Mon Sep 17 00:00:00 2001 From: Philip Arndt Date: Tue, 23 May 2023 10:53:33 +1200 Subject: [PATCH 3/3] Fix `sqlite3` gsub in `Refinery::CmsGenerator` (#3530) I noticed when running `rails new -m https://www.refinerycms.com/t/edge` that it was mangling the `Gemfile` when the `sqlite3` line looked like: ```ruby gem 'sqlite3', '~> 1.4' ``` Instead of the expected: ```ruby gem 'sqlite3' ``` Our `gsub_file` instruction now captures the entire line and inserts it inside the `group :development, :test do` block. --- core/lib/generators/refinery/cms/cms_generator.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/lib/generators/refinery/cms/cms_generator.rb b/core/lib/generators/refinery/cms/cms_generator.rb index db1dcc11eb..b4a86e645d 100644 --- a/core/lib/generators/refinery/cms/cms_generator.rb +++ b/core/lib/generators/refinery/cms/cms_generator.rb @@ -62,8 +62,8 @@ def append_asset_pipeline! def append_gemfile! if destination_path.join('Gemfile').file? && destination_path.join('Gemfile').read !~ %r{group :development, :test do\n.+?gem 'sqlite3'\nend}m - gsub_file 'Gemfile', %q{gem 'sqlite3'}, %q{group :development, :test do - gem 'sqlite3' + gsub_file 'Gemfile', /(gem\ ['|"]sqlite3['|"].*)$/, %q{group :development, :test do + \1 end} end end