diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..4f876ae --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,26 @@ +name: CI + +on: + push: + branches: [master] + pull_request: + +jobs: + test: + name: Ruby ${{ matrix.ruby }} / Rails ${{ matrix.rails }} + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + ruby: ["3.3", "3.4", "4.0"] + rails: ["~> 8.0.0", "~> 8.1.0"] + env: + RAILS_VERSION: ${{ matrix.rails }} + steps: + - uses: actions/checkout@v4 + - uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby }} + bundler-cache: true + - name: Run specs + run: bundle exec rspec diff --git a/.rspec b/.rspec new file mode 100644 index 0000000..c99d2e7 --- /dev/null +++ b/.rspec @@ -0,0 +1 @@ +--require spec_helper diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index a01d453..0000000 --- a/.travis.yml +++ /dev/null @@ -1,3 +0,0 @@ -language: ruby -rvm: - - 2.4.1 diff --git a/CHANGELOG b/CHANGELOG index d7389ad..f966042 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,28 @@ += 5.0.0 + * performance optimizations across routing helpers, request introspection, and instance variable access + * the instance variable read/written by #resource and #resources is now the one named by the resource + specification. This means overriding #resource_name / #resources_name no longer changes which + instance variable those accessors use + * RequestPathIntrospection matches the current route segment literally (Regexp.escape), and no longer + keeps a process-wide cache of compiled segment regexps + * the spec suite runs under any RAILS_ENV, and the specs for the route mapping error messages, which + previously failed, were fixed + * development setup no longer needs Gemfile.lock.development: run bundle install + * CI moved from Travis to GitHub Actions, testing Ruby 3.3, 3.4 and 4.0 against Rails 8.0 and 8.1 + * ResourceService#respond_to? now reports the proxy's own methods - it always returned false for + them, because a BasicObject has no #respond_to? for it to fall back on + * named route helpers are exposed through #respond_to_missing? rather than by overriding #respond_to? + * route_name.pluralize is memoised per controller: inflection was the most expensive operation on + the named route helper path + * lib is now frozen_string_literal: true + * removed the dead Rails 2 find_filter branch, spec/verify_rcov.rb, and the Rakefile's rake/rdoctask + fallback + * removed the spec:make_test_app / spec:rspec_generated_specs tasks: they generated a Rails 2-5 era + app and could no longer run + * gem packaging: added the MIT license and gem metadata, dropped the stale rubygems_version, and + s.files no longer ships the specs, CI config and other development files + * require Rails >= 8.0 and Ruby >= 3.3, dropping support for older versions + = 4.0.0 * replace remaining regex-based routing checks with string helpers for better performance diff --git a/Gemfile b/Gemfile index f6a560f..83f8a77 100644 --- a/Gemfile +++ b/Gemfile @@ -1,3 +1,8 @@ -source :rubygems +source "https://rubygems.org" -gemspec \ No newline at end of file +gemspec + +# CI exercises the oldest and the newest supported Rails releases by setting +# RAILS_VERSION, e.g. RAILS_VERSION='~> 8.0.0' bundle install && bundle exec rspec +rails_version = ENV["RAILS_VERSION"].to_s +gem "rails", rails_version unless rails_version.empty? diff --git a/Gemfile.lock.development b/Gemfile.lock.development deleted file mode 100644 index fa81018..0000000 --- a/Gemfile.lock.development +++ /dev/null @@ -1,152 +0,0 @@ -PATH - remote: . - specs: - rc_rails (3.0.0) - rails (>= 5.1.1) - -GEM - remote: http://rubygems.org/ - specs: - actioncable (5.1.1) - actionpack (= 5.1.1) - nio4r (~> 2.0) - websocket-driver (~> 0.6.1) - actionmailer (5.1.1) - actionpack (= 5.1.1) - actionview (= 5.1.1) - activejob (= 5.1.1) - mail (~> 2.5, >= 2.5.4) - rails-dom-testing (~> 2.0) - actionpack (5.1.1) - actionview (= 5.1.1) - activesupport (= 5.1.1) - rack (~> 2.0) - rack-test (~> 0.6.3) - rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionview (5.1.1) - activesupport (= 5.1.1) - builder (~> 3.1) - erubi (~> 1.4) - rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.0, >= 1.0.3) - activejob (5.1.1) - activesupport (= 5.1.1) - globalid (>= 0.3.6) - activemodel (5.1.1) - activesupport (= 5.1.1) - activerecord (5.1.1) - activemodel (= 5.1.1) - activesupport (= 5.1.1) - arel (~> 8.0) - activesupport (5.1.1) - concurrent-ruby (~> 1.0, >= 1.0.2) - i18n (~> 0.7) - minitest (~> 5.1) - tzinfo (~> 1.1) - arel (8.0.0) - builder (3.2.3) - concurrent-ruby (1.0.5) - diff-lcs (1.3) - erubi (1.6.0) - globalid (0.4.0) - activesupport (>= 4.2.0) - i18n (0.8.4) - loofah (2.0.3) - nokogiri (>= 1.5.9) - mail (2.6.6) - mime-types (>= 1.16, < 4) - method_source (0.8.2) - mime-types (3.1) - mime-types-data (~> 3.2015) - mime-types-data (3.2016.0521) - mini_portile2 (2.2.0) - minitest (5.10.2) - nio4r (2.1.0) - nokogiri (1.8.0) - mini_portile2 (~> 2.2.0) - rack (2.0.3) - rack-test (0.6.3) - rack (>= 1.0) - rails (5.1.1) - actioncable (= 5.1.1) - actionmailer (= 5.1.1) - actionpack (= 5.1.1) - actionview (= 5.1.1) - activejob (= 5.1.1) - activemodel (= 5.1.1) - activerecord (= 5.1.1) - activesupport (= 5.1.1) - bundler (>= 1.3.0, < 2.0) - railties (= 5.1.1) - sprockets-rails (>= 2.0.0) - rails-controller-testing (1.0.2) - actionpack (~> 5.x, >= 5.0.1) - actionview (~> 5.x, >= 5.0.1) - activesupport (~> 5.x) - rails-dom-testing (2.0.3) - activesupport (>= 4.2.0) - nokogiri (>= 1.6) - rails-html-sanitizer (1.0.3) - loofah (~> 2.0) - railties (5.1.1) - actionpack (= 5.1.1) - activesupport (= 5.1.1) - method_source - rake (>= 0.8.7) - thor (>= 0.18.1, < 2.0) - rake (12.0.0) - rspec (3.6.0) - rspec-core (~> 3.6.0) - rspec-expectations (~> 3.6.0) - rspec-mocks (~> 3.6.0) - rspec-activemodel-mocks (1.0.3) - activemodel (>= 3.0) - activesupport (>= 3.0) - rspec-mocks (>= 2.99, < 4.0) - rspec-core (3.6.0) - rspec-support (~> 3.6.0) - rspec-expectations (3.6.0) - diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.6.0) - rspec-mocks (3.6.0) - diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.6.0) - rspec-rails (3.6.0) - actionpack (>= 3.0) - activesupport (>= 3.0) - railties (>= 3.0) - rspec-core (~> 3.6.0) - rspec-expectations (~> 3.6.0) - rspec-mocks (~> 3.6.0) - rspec-support (~> 3.6.0) - rspec-support (3.6.0) - sprockets (3.7.1) - concurrent-ruby (~> 1.0) - rack (> 1, < 3) - sprockets-rails (3.2.0) - actionpack (>= 4.0) - activesupport (>= 4.0) - sprockets (>= 3.0.0) - sqlite3 (1.3.13) - thor (0.19.4) - thread_safe (0.3.6) - tzinfo (1.2.3) - thread_safe (~> 0.1) - websocket-driver (0.6.5) - websocket-extensions (>= 0.1.0) - websocket-extensions (0.1.2) - -PLATFORMS - ruby - -DEPENDENCIES - rails-controller-testing - rc_rails! - rspec (>= 3) - rspec-activemodel-mocks - rspec-rails (>= 3.5) - sqlite3 - -BUNDLED WITH - 1.15.1 diff --git a/README.rdoc b/README.rdoc index 80a739e..1f46b9a 100644 --- a/README.rdoc +++ b/README.rdoc @@ -1,6 +1,6 @@ = resources_controller -{Build Status}[http://travis-ci.org/ianwhite/resources_controller] +{Build Status}[https://github.com/ianwhite/resources_controller/actions/workflows/ci.yml] is a plugin to facilitate inheritance and DRYness in your resources controllers. It introduces some abstraction to help you override the default RESTful functionality in a clean and simple manner. @@ -12,6 +12,10 @@ Add this to your Gemfile gem 'rc_rails' +== Requirements + +Version 5.0.0 and later require Rails >= 8.0 and Ruby >= 3.3. + == Rails 5 version Version 3.x of this gem is compatible with Rails 5.x @@ -38,10 +42,13 @@ To get set up for development, do the following: git clone git://github.com/ianwhite/resources_controller cd resources_controller - cp Gemfile.lock.development Gemfile.lock - bundle - rake spec - rake spec:rspec_generated_specs + bundle install + bundle exec rspec + +The specs are run against Rails 8.0 and 8.1 on GitHub Actions. To run them against a +particular Rails version locally, set RAILS_VERSION: + + RAILS_VERSION='~> 8.0.0' bundle install && bundle exec rspec == Contributors diff --git a/Rakefile b/Rakefile index 394bf4e..34e7697 100644 --- a/Rakefile +++ b/Rakefile @@ -4,13 +4,8 @@ begin rescue LoadError puts 'You must `gem install bundler` and `bundle install` to run rake tasks' end -begin - require 'rdoc/task' -rescue LoadError - require 'rdoc/rdoc' - require 'rake/rdoctask' - RDoc::Task = Rake::RDocTask -end + +require 'rdoc/task' RDoc::Task.new(:rdoc) do |rdoc| rdoc.rdoc_dir = 'rdoc' @@ -23,7 +18,6 @@ end Bundler::GemHelper.install_tasks require 'rspec/core/rake_task' -require File.expand_path('../spec/rspec_generator_task', __FILE__) # for spec:generate task task :default => [:spec] diff --git a/lib/rc_rails.rb b/lib/rc_rails.rb index b848f5c..3268906 100644 --- a/lib/rc_rails.rb +++ b/lib/rc_rails.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'resources_controller' # BC diff --git a/lib/resources_controller.rb b/lib/resources_controller.rb index e9578ac..fb76d1c 100644 --- a/lib/resources_controller.rb +++ b/lib/resources_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'resources_controller/active_record/saved' require 'resources_controller/railtie' if defined?(Rails) @@ -506,11 +508,7 @@ def include_actions(mixin, options = {}) private def load_enclosing_resources_filter_exists? - if respond_to?(:find_filter) # BC 2.0-stable branch - find_filter(:load_enclosing_resources) - else - _process_action_callbacks.detect {|c| c.filter == :load_enclosing_resources} - end + _process_action_callbacks.any? {|c| c.filter == :load_enclosing_resources} end module ClassMethods @@ -557,12 +555,10 @@ def ensure_sane_wildcard module InstanceMethods - def resource_service=(service) - @resource_service = service - end + attr_writer :resource_service def name_prefix - @name_prefix ||= '' + @name_prefix ||= +'' end # name of the singular resource @@ -581,24 +577,29 @@ def resource_class end # returns the controller's current resource. + # + # The instance variable is named after the resource specification (see + # Specification#ivar_name), which is the same name #add_enclosing_resource uses + # for enclosing resources. It is deliberately not derived from #resource_name, + # which may be overridden for params/forms - see ResourceMethods#resource_params. def resource - instance_variable_get("@#{resource_name}") + instance_variable_get(resource_specification.ivar_name) end # sets the controller's current resource, and # decorates the object with a save hook, so we know if it's been saved def resource=(record) - instance_variable_set("@#{resource_name}", record) + instance_variable_set(resource_specification.ivar_name, record) end # returns the controller's current resources collection def resources - instance_variable_get("@#{resources_name}") + instance_variable_get(resource_specification.collection_ivar_name) end # sets the controller's current resource collection def resources=(collection) - instance_variable_set("@#{resources_name}", collection) + instance_variable_set(resource_specification.collection_ivar_name, collection) end # returns the immediately enclosing resource @@ -607,9 +608,7 @@ def enclosing_resource end # returns the name of the immediately enclosing resource - def enclosing_resource_name - @enclosing_resource_name - end + attr_reader :enclosing_resource_name # returns the resource service for the controller - this will be lazilly created # to a ResourceService, or a SingletonResourceService (if :singleton => true) @@ -663,10 +662,12 @@ def save_resource def load_enclosing_resources namespace_segments.each {|segment| update_name_prefix("#{segment}_") } specifications.each_with_index do |spec, idx| - case spec - when '*' then load_wildcards_from(idx) - when /\A\?(.*)/ then load_wildcard($1) - else load_enclosing_resource_from_specification(spec) + if spec == '*' + load_wildcards_from(idx) + elsif spec.is_a?(String) && spec.start_with?('?') + load_wildcard(spec[1..]) + else + load_enclosing_resource_from_specification(spec) end end end @@ -700,7 +701,7 @@ def load_wildcards_from(start) encls = nesting_segments.slice(enclosing_resources.size..-1) if spec = specs.find {|s| s.is_a?(Specification)} - spec_seg = encls.index({:segment => spec.segment, :singleton => spec.singleton?}) or ResourcesController.raise_resource_mismatch(self) + spec_seg = encls.index {|e| e[:segment] == spec.segment && e[:singleton] == spec.singleton?} or ResourcesController.raise_resource_mismatch(self) number_of_wildcards = spec_seg - (specs.index(spec) -1) else number_of_wildcards = encls.length - (specs.length - 1) @@ -721,22 +722,32 @@ def add_enclosing_resource(resource, options = {}) update_name_prefix(options[:name_prefix] || (options[:name_prefix] == false ? '' : "#{name}_")) enclosing_resources << resource enclosing_collection_resources << resource unless options[:is_singleton] - instance_variable_set("@enclosing_resource_name", options[:name]) - instance_variable_set("@#{name}", resource) - instance_variable_set("@#{options[:as]}", resource) if options[:as] + instance_variable_set(:@enclosing_resource_name, options[:name]) + instance_variable_set(:"@#{name}", resource) + instance_variable_set(:"@#{options[:as]}", resource) if options[:as] end # The name prefix is used for forwarding urls and will be different depending on # which route the controller was invoked by. The resource specifications build # up the name prefix as the resources are loaded. - def update_name_prefix(name_prefix) - @name_prefix = "#{@name_prefix}#{name_prefix}" + # + # Empty prefixes are a no-op, but @name_prefix is always left as a String once + # this has been called, so that #name_prefix never has to cope with nil. + def update_name_prefix(prefix) + @name_prefix ||= +'' + return if prefix.nil? || prefix.empty? + @name_prefix = "#{@name_prefix}#{prefix}" end end # Proxy class to provide a consistent API for resource_service. This is mostly # required for Singleton resources. Also allows decoration of the resource service with custom finders class ResourceService < BasicObject + # BasicObject has no #respond_to?, so #respond_to? below has no superclass method + # to fall back on to find the proxy's own methods - they have to be listed here. + PROXY_METHODS = [:controller, :service, :find, :new, :destroy, + :resource_specification, :resource_class, :enclosing_resource].freeze + attr_reader :controller delegate :resource_specification, :resource_class, :enclosing_resource, :to => :controller @@ -775,7 +786,7 @@ def destroy(*args, **kwargs) end def respond_to?(method, include_private = false) - super || service.respond_to?(method) + PROXY_METHODS.include?(method.to_sym) || service.respond_to?(method, include_private) end def service diff --git a/lib/resources_controller/actions.rb b/lib/resources_controller/actions.rb index 0ce86f5..166572e 100644 --- a/lib/resources_controller/actions.rb +++ b/lib/resources_controller/actions.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ResourcesController # standard CRUD actions, with html and json responses, re-written to mnake best use of resources_cotroller. # This helps if you're writing controllers that you want to share via mixin or inheritance. diff --git a/lib/resources_controller/active_record/saved.rb b/lib/resources_controller/active_record/saved.rb index 02e1da8..c9941d9 100644 --- a/lib/resources_controller/active_record/saved.rb +++ b/lib/resources_controller/active_record/saved.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ResourcesController module ActiveRecord module Saved diff --git a/lib/resources_controller/helper.rb b/lib/resources_controller/helper.rb index a13c614..08bed8f 100644 --- a/lib/resources_controller/helper.rb +++ b/lib/resources_controller/helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ResourcesController # Often it won't be appropriate to reuse views, but # sometimes it is. These helper methods enable reuse by referencing whatever resource the @@ -88,8 +90,8 @@ def method_missing(method, *args, &block) end # delegate url help method creation to the controller - def respond_to?(*args) - super(*args) || controller.resource_named_route_helper_method?(args.first) + def respond_to_missing?(method, include_private = false) + controller.resource_named_route_helper_method?(method) || super end private @@ -102,7 +104,7 @@ def form_for_resource_options(resource, resource_name, options) options[:url] ||= if resource.new_record? controller.resource_specification.singleton? ? resource_path(*args) : resources_path(*args) else - controller.resource_specification.singleton? ? resource_path(*args) : resource_path(*([resource] + args)) + controller.resource_specification.singleton? ? resource_path(*args) : resource_path(resource, *args) end end end diff --git a/lib/resources_controller/include_actions.rb b/lib/resources_controller/include_actions.rb index 5a1a2a8..3ef3246 100644 --- a/lib/resources_controller/include_actions.rb +++ b/lib/resources_controller/include_actions.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ResourcesController # extension for any module that is used as an Actions module. # diff --git a/lib/resources_controller/named_route_helper.rb b/lib/resources_controller/named_route_helper.rb index 75f8fdc..399b52e 100644 --- a/lib/resources_controller/named_route_helper.rb +++ b/lib/resources_controller/named_route_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ResourcesController class CantMapRoute < ArgumentError #:nodoc: @@ -45,8 +47,8 @@ def method_missing(method, *args, &block) end end - def respond_to?(*args) - super(*args) || resource_named_route_helper_method?(args.first) + def respond_to_missing?(method, include_private = false) + resource_named_route_helper_method?(method) || super end # return true if the passed method (e.g. 'resources_path') corresponds to a defined @@ -56,9 +58,9 @@ def resource_named_route_helper_method?(resource_method, raise_error = false) return false unless method_str.end_with?('_path', '_url') if method_str.include?('enclosing_resource') - _, route_method = *route_and_method_from_enclosing_resource_method_and_name_prefix(method_str, name_prefix) + _, route_method = route_and_method_from_enclosing_resource_method_and_name_prefix(method_str, name_prefix) elsif method_str.include?('resource') - _, route_method = *route_and_method_from_resource_method_and_name_prefix(method_str, name_prefix) + _, route_method = route_and_method_from_resource_method_and_name_prefix(method_str, name_prefix) else return false end @@ -88,10 +90,11 @@ def route_and_method_from_enclosing_resource_method_and_name_prefix(method, name if enclosing_resource enclosing_route = name_prefix.delete_suffix('_') method_str = method.to_s - substitution = method_str.include?('enclosing_resources') ? enclosing_route.pluralize : enclosing_route - route_method = method_str.sub(method_str.include?('enclosing_resources') ? 'enclosing_resources' : 'enclosing_resource', substitution) - route_key = route_method.delete_suffix('_path').delete_suffix('_url') - return [Rails.application.routes.named_routes.get(route_key.to_sym), route_method] + is_plural = method_str.include?('enclosing_resources') + substitution = is_plural ? enclosing_route.pluralize : enclosing_route + route_method = method_str.sub(is_plural ? 'enclosing_resources' : 'enclosing_resource', substitution) + route_key = route_method.end_with?('_path') ? route_method.delete_suffix('_path') : route_method.delete_suffix('_url') + [Rails.application.routes.named_routes.get(route_key.to_sym), route_method] else raise NoMethodError, "Tried to map :#{method} but there is no enclosing_resource for this controller" end @@ -101,11 +104,18 @@ def route_and_method_from_enclosing_resource_method_and_name_prefix(method, name # return the [route, route_method] for the expanded resource def route_and_method_from_resource_method_and_name_prefix(method, name_prefix) method_str = method.to_s - replacement = method_str.include?('resources_') ? "#{name_prefix}#{route_name.pluralize}" : "#{name_prefix}#{route_name}" - route_method = method_str.sub(method_str.include?('resources_') ? 'resources' : 'resource', replacement) - route_key = route_method.delete_suffix('_path').delete_suffix('_url') + is_resources = method_str.include?('resources_') + replacement = "#{name_prefix}#{is_resources ? resources_route_name : route_name}" + route_method = method_str.sub(is_resources ? 'resources' : 'resource', replacement) + route_key = route_method.end_with?('_path') ? route_method.delete_suffix('_path') : route_method.delete_suffix('_url') [Rails.application.routes.named_routes.get(route_key.to_sym), route_method] end + + # route_name is fixed for the life of the controller, and pluralize is the most + # expensive operation on this path, so it is memoised rather than run per call. + def resources_route_name + @resources_route_name ||= route_name.pluralize + end # defines a method that calls the appropriate named route method, with appropriate args. def define_resource_named_route_helper_method(method) @@ -126,8 +136,8 @@ def resource_named_route_helper_method_for_name_prefix?(method) def define_resource_named_route_helper_method_for_name_prefix(method) method_str = method.to_s resource_method, name_prefix = method_str.split('_for_', 2) - if resource_method.match?(/enclosing_resource/) - route, route_method = *route_and_method_from_enclosing_resource_method_and_name_prefix(resource_method, name_prefix) + if resource_method.include?('enclosing_resource') + route, route_method = route_and_method_from_enclosing_resource_method_and_name_prefix(resource_method, name_prefix) required_args = (route.segment_keys - [:format, :locale]).size self.class.send :module_eval, <<-end_eval, __FILE__, __LINE__ @@ -135,22 +145,22 @@ def #{method}(*args) options = args.extract_options! options.merge!(default_url_options) args = args.size < #{required_args} ? enclosing_collection_resources + args : enclosing_collection_resources - [enclosing_resource] + args - args = args + [options] if options.size > 0 + args << options unless options.empty? send :#{route_method}, *args end end_eval else - route, route_method = *route_and_method_from_resource_method_and_name_prefix(resource_method, name_prefix) + route, route_method = route_and_method_from_resource_method_and_name_prefix(resource_method, name_prefix) required_args = (route.segment_keys - [:format, :locale]).size self.class.send :module_eval, <<-end_eval, __FILE__, __LINE__ def #{method}(*args) options = args.extract_options! options.merge!(default_url_options) - #{"args = [resource] + args if enclosing_collection_resources.size + args.size < #{required_args}" if required_args > 0} - args = args + [options] if options.size > 0 - send :#{route_method}, *(enclosing_collection_resources + args) + #{"args.unshift(resource) if enclosing_collection_resources.size + args.size < #{required_args}" if required_args > 0} + args << options unless options.empty? + send :#{route_method}, *enclosing_collection_resources, *args end end_eval end diff --git a/lib/resources_controller/railtie.rb b/lib/resources_controller/railtie.rb index 4035be6..d8b384a 100644 --- a/lib/resources_controller/railtie.rb +++ b/lib/resources_controller/railtie.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ResourcesController class Railtie < Rails::Railtie initializer 'resources_controller' do diff --git a/lib/resources_controller/request_path_introspection.rb b/lib/resources_controller/request_path_introspection.rb index 51817b1..58c31fc 100644 --- a/lib/resources_controller/request_path_introspection.rb +++ b/lib/resources_controller/request_path_introspection.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ResourcesController # included into ActionController::Base # @@ -34,16 +36,27 @@ def namespace_segments end def param_keys - params.keys.map(&:to_s).select{|k| k.end_with?('_id')} + params.each_key.filter_map do |k| + k_str = k.to_s + k_str if k_str.end_with?('_id') + end end private + # Removes the current resource segment, and everything after it, from the path, + # e.g. '/users/1/forums/2' => '/users/1'. + # + # This is only called from #nesting_request_path, which is memoised per request, + # so the pattern is built at most once per request - there is no need to cache it. + # The segment is escaped so that it is matched literally, as in #remove_namespace. def remove_current_segment(path) - if respond_to?(:resource_specification) && resource_specification.singleton? - path.sub(%r(/#{current_segment}(?!.*/#{current_segment}).*$), '') + segment = Regexp.escape(current_segment.to_s) + pattern = if respond_to?(:resource_specification) && resource_specification.singleton? + %r(/#{segment}(?!.*/#{segment}).*$) else - path.sub(%r(/#{current_segment}(?!.+/#{current_segment}).*$), '') + %r(/#{segment}(?!.+/#{segment}).*$) end + path.sub(pattern, '') end def current_segment @@ -60,7 +73,7 @@ def remove_namespace(path) def segments_for_path_and_keys(path, keys) key_segments = keys.map{|k| segment_for_key(k)} - path_segments = path[1..-1].to_s.split('/') + path_segments = path.delete_prefix('/').split('/') segments = [] while path_segments.any? do segment = path_segments.shift @@ -75,12 +88,13 @@ def segments_for_path_and_keys(path, keys) end def segment_for_key(key) - if respond_to?(:specifications) && spec = specifications.find{|s| s.respond_to?(:key) && s.key == key.to_s} + key_str = key.to_s + if respond_to?(:specifications) && spec = specifications.find{|s| s.respond_to?(:key) && s.key == key_str} spec.segment - elsif spec = resource_specification_map.values.find{|s| s.key == key.to_s} + elsif spec = resource_specification_map.values.find{|s| s.key == key_str} spec.segment else - key.to_s[0..-4].pluralize + key_str.delete_suffix('_id').pluralize end end end diff --git a/lib/resources_controller/resource_methods.rb b/lib/resources_controller/resource_methods.rb index 660b95e..b7713a9 100644 --- a/lib/resources_controller/resource_methods.rb +++ b/lib/resources_controller/resource_methods.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ResourcesController # methods which communicate with the resource_service to find/create resources module ResourceMethods @@ -30,12 +32,16 @@ def destroy_resource(id = nil) end + EXCLUDED_PARAM_COLUMNS = %w[updated_at created_at].freeze + # Never trust parameters from the scary internet, only allow the white list through. def resource_params - if self.respond_to?("#{resource_name}_params", true) - return self.send("#{resource_name}_params") + method_name = "#{resource_name}_params" + if respond_to?(method_name, true) + send(method_name) else - return params.fetch(resource_name, {}).permit( *(resource_service.content_columns.map(&:name) - [ 'updated_at', 'created_at' ]) ) + columns = resource_service.content_columns.map(&:name) - EXCLUDED_PARAM_COLUMNS + params.fetch(resource_name) { {} }.permit(*columns) end end diff --git a/lib/resources_controller/singleton_actions.rb b/lib/resources_controller/singleton_actions.rb index d04c84a..391112c 100644 --- a/lib/resources_controller/singleton_actions.rb +++ b/lib/resources_controller/singleton_actions.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ResourcesController module SingletonActions include Actions diff --git a/lib/resources_controller/specification.rb b/lib/resources_controller/specification.rb index 4c49575..ae24654 100644 --- a/lib/resources_controller/specification.rb +++ b/lib/resources_controller/specification.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ResourcesController # This class holds all the info that is required to find a resource, or determine a name prefix, based on a route segment # or segment pair (e.g. /blog or /users/3). @@ -15,7 +17,7 @@ class Specification class NoClassFoundError < NameError end - attr_reader :name, :source, :klass, :key, :name_prefix, :segment, :find + attr_reader :name, :source, :klass, :key, :name_prefix, :segment, :find, :ivar_name, :collection_ivar_name attr_accessor :as # factory for Specification and SingletonSpecification @@ -49,12 +51,15 @@ def initialize(spec_name, options = {}, &block) options.assert_valid_keys(:class, :source, :key, :find, :name_prefix, :segment, :as) @name = spec_name.to_s @find = block || options.delete(:find) - @segment = (options[:segment] && options[:segment].to_s) || name.pluralize - @source = (options[:source] && options[:source].to_s) || name.pluralize - @name_prefix = options[:name_prefix] || (options[:name_prefix] == false ? '' : "#{name}_") + plural = name.pluralize + @segment = (options[:segment] && options[:segment].to_s) || plural + @source = (options[:source] && options[:source].to_s) || plural + @name_prefix = options[:name_prefix] || (options[:name_prefix] == false ? +'' : "#{name}_") @klass = options[:class] || infer_class @key = (options[:key] && options[:key].to_s) || name.foreign_key @as = options[:as] + @ivar_name = :"@#{name}" + @collection_ivar_name = :"@#{plural}" end # returns false diff --git a/lib/resources_controller/version.rb b/lib/resources_controller/version.rb index 4068078..f88fefd 100644 --- a/lib/resources_controller/version.rb +++ b/lib/resources_controller/version.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ResourcesController - VERSION = "4.0.0" + VERSION = "5.0.0" end diff --git a/resources_controller.gemspec b/resources_controller.gemspec index c06028f..5dd3581 100644 --- a/resources_controller.gemspec +++ b/resources_controller.gemspec @@ -8,21 +8,27 @@ Gem::Specification.new do |s| s.name = "rc_rails" s.version = version s.platform = Gem::Platform::RUBY + s.required_ruby_version = '>= 3.3' s.authors = ["Ian White"] s.email = "ian.w.white@gmail.com" s.homepage = "http://github.com/ianwhite/resources_controller" s.summary = "resources_controller-#{version}" s.description = "rc makes RESTful controllers fun" + s.license = "MIT" - s.rubygems_version = "2.0.3" - - s.files = `git ls-files`.split("\n") - s.test_files = `git ls-files -- {spec}/*`.split("\n") + s.files = Dir["lib/**/*.rb", "CHANGELOG", "MIT-LICENSE", "README.rdoc"] s.extra_rdoc_files = [ "README.rdoc" ] s.rdoc_options = ["--charset=UTF-8"] s.require_path = "lib" - s.add_runtime_dependency "rails", '>= 5.1.1' + s.metadata = { + "source_code_uri" => "https://github.com/ianwhite/resources_controller", + "changelog_uri" => "https://github.com/ianwhite/resources_controller/blob/master/CHANGELOG", + "bug_tracker_uri" => "https://github.com/ianwhite/resources_controller/issues", + "rubygems_mfa_required" => "true" + } + + s.add_runtime_dependency "rails", '>= 8.0' s.add_development_dependency "rspec", '>= 3' s.add_development_dependency "rspec-rails", '>= 3.5' s.add_development_dependency 'rails-controller-testing' diff --git a/spec/app.rb b/spec/app.rb index 80cb592..42e98b1 100644 --- a/spec/app.rb +++ b/spec/app.rb @@ -11,7 +11,7 @@ class Application < Rails::Application end end -ResourcesControllerTest::Application.initialize! +ResourcesControllerTest::Application.initialize! unless ResourcesControllerTest::Application.initialized? ########## # Routing diff --git a/spec/controllers/admin_forums_controller_spec.rb b/spec/controllers/admin_forums_controller_spec.rb index 943af61..e4b90de 100644 --- a/spec/controllers/admin_forums_controller_spec.rb +++ b/spec/controllers/admin_forums_controller_spec.rb @@ -119,8 +119,8 @@ Tried to map :resource_users_path to :admin_forum_users_path, which doesn't exist. You may not have defined the route in config/routes.rb. -Or, if you have unconventianal route names or name prefixes, you may need -to explicictly set the :route option in resources_controller_for, and set +Or, if you have unconventional route names or name prefixes, you may need +to explicitly set the :route option in resources_controller_for, and set the :name_prefix option on your enclosing resources. Currently: diff --git a/spec/controllers/forums_controller_spec.rb b/spec/controllers/forums_controller_spec.rb index 380ef5d..8fc4138 100644 --- a/spec/controllers/forums_controller_spec.rb +++ b/spec/controllers/forums_controller_spec.rb @@ -119,8 +119,8 @@ Tried to map :resource_users_path to :forum_users_path, which doesn't exist. You may not have defined the route in config/routes.rb. -Or, if you have unconventianal route names or name prefixes, you may need -to explicictly set the :route option in resources_controller_for, and set +Or, if you have unconventional route names or name prefixes, you may need +to explicitly set the :route option in resources_controller_for, and set the :name_prefix option on your enclosing resources. Currently: diff --git a/spec/lib/load_enclosing_resources_spec.rb b/spec/lib/load_enclosing_resources_spec.rb index 2ddacb8..c031688 100644 --- a/spec/lib/load_enclosing_resources_spec.rb +++ b/spec/lib/load_enclosing_resources_spec.rb @@ -210,36 +210,13 @@ def setup_common end end -# specing some branching BC code -# find_filter disappeared from edge, but we want to support it for 2.0-stable describe "ResourcesController.load_enclosing_resources_filter_exists?" do before do @klass = Class.new(ActionController::Base) end - - describe "when :find_filter defined" do - before do - class<<@klass - def find_filter; end - end - end - - it "should call :find_filter with :load_enclosing_resources" do - expect(@klass).to receive(:find_filter).with(:load_enclosing_resources) - @klass.send(:load_enclosing_resources_filter_exists?) - end - end - - describe "when :find_filter not defined" do - before do - class<<@klass - undef_method(:find_filter) rescue nil - end - end - - it "should call :_process_action_callbacks" do - expect(@klass).to receive(:_process_action_callbacks).and_return([]) - @klass.send(:load_enclosing_resources_filter_exists?) - end + + it "should ask _process_action_callbacks" do + expect(@klass).to receive(:_process_action_callbacks).and_return([]) + @klass.send(:load_enclosing_resources_filter_exists?) end end diff --git a/spec/lib/named_route_helper_spec.rb b/spec/lib/named_route_helper_spec.rb new file mode 100644 index 0000000..9bce4e4 --- /dev/null +++ b/spec/lib/named_route_helper_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe "ResourcesController::NamedRouteHelper" do + before do + @controller = ForumsController.new + end + + it "should respond to a named route helper for the current resource" do + expect(@controller.respond_to?(:resource_path)).to eq(true) + expect(@controller.respond_to?(:resources_path)).to eq(true) + end + + it "should not respond to something that is not a named route helper" do + expect(@controller.respond_to?(:resource_nonexistent_path)).to eq(false) + expect(@controller.respond_to?(:not_a_route_path)).to eq(false) + end + + # this works because the module defines respond_to_missing?, not respond_to? + it "should expose named route helpers through #method" do + expect(@controller.method(:resource_path)).to be_a(Method) + end +end diff --git a/spec/lib/request_path_introspection_spec.rb b/spec/lib/request_path_introspection_spec.rb index 5496991..e72b4bb 100644 --- a/spec/lib/request_path_introspection_spec.rb +++ b/spec/lib/request_path_introspection_spec.rb @@ -53,6 +53,12 @@ class User < ActiveRecord::Base; end allow(@controller).to receive(:request_path).and_return('/users/1/forum/forum.atom') expect(@controller.send(:nesting_request_path)).to eq('/users/1/forum') end + + it "should match the current segment literally, when it contains regexp characters" do + allow(@controller).to receive(:resource_specification).and_return(ResourcesController::Specification.new(:forum, :class => RequestPathIntrospectionSpec::Forum, :segment => 'v1.0')) + allow(@controller).to receive(:request_path).and_return('/v1X0/2') + expect(@controller.send(:nesting_request_path)).to eq('/v1X0/2') + end it "should remove any controller namespace" do allow(@controller).to receive(:controller_path).and_return('some/name/space/forums') diff --git a/spec/lib/resources_controller_spec.rb b/spec/lib/resources_controller_spec.rb index 0856c56..9c32e11 100644 --- a/spec/lib/resources_controller_spec.rb +++ b/spec/lib/resources_controller_spec.rb @@ -34,12 +34,54 @@ describe "A controller's resource_service" do before do @controller = ForumsController.new + @resource_service = @controller.resource_service end it 'may be explicitly set with #resource_service=' do @controller.resource_service = 'foo' expect(@controller.resource_service).to eq('foo') end + + # ResourceService is a BasicObject, so it has no #respond_to? to fall back on for + # its own methods (see ResourceService::PROXY_METHODS) + it 'should respond to its own methods' do + expect(@resource_service.respond_to?(:controller)).to eq(true) + expect(@resource_service.respond_to?(:service)).to eq(true) + expect(@resource_service.respond_to?(:find)).to eq(true) + expect(@resource_service.respond_to?(:new)).to eq(true) + expect(@resource_service.respond_to?(:destroy)).to eq(true) + end + + it 'should respond to the methods it delegates to the controller' do + expect(@resource_service.respond_to?(:resource_specification)).to eq(true) + expect(@resource_service.respond_to?(:resource_class)).to eq(true) + expect(@resource_service.respond_to?(:enclosing_resource)).to eq(true) + end + + it 'should respond to the methods of the service it proxies' do + expect(Forum.respond_to?(:count)).to eq(true) + expect(@resource_service.respond_to?(:count)).to eq(true) + expect(@resource_service.respond_to?(:no_such_method_at_all)).to eq(false) + end +end + +describe "ResourcesController#update_name_prefix" do + before do + @controller = ForumsController.new + end + + it "should accumulate the prefixes of the enclosing resources" do + @controller.send :update_name_prefix, 'forum_' + @controller.send :update_name_prefix, 'user_' + expect(@controller.name_prefix).to eq('forum_user_') + end + + it "should leave name_prefix as an empty String when the given prefix is empty or nil" do + @controller.send :update_name_prefix, nil + @controller.send :update_name_prefix, '' + expect(@controller.instance_variable_get(:@name_prefix)).to eq('') + expect(@controller.name_prefix).to eq('') + end end describe "deprecated methods" do diff --git a/spec/rspec_generator_task.rb b/spec/rspec_generator_task.rb deleted file mode 100644 index 067e0a6..0000000 --- a/spec/rspec_generator_task.rb +++ /dev/null @@ -1,99 +0,0 @@ -namespace :spec do - desc "Run rspec's generated specs against RC'd controllers" - task :rspec_generated_specs => 'tmp/test_app' do - require 'active_support/all' - - cd 'tmp/test_app' do - begin - generate_resource :author - migrate_up - make_resources_controller :author - puts "** Running generated controller specs" - sh "rake db:test:prepare" - sh "bundle exec rspec" - ensure - migrate_down - cleanup_resource :author - end - end - end - - desc "make a test rails app in tmp/test_app" - task :make_test_app => ['spec:clobber_test_app', 'tmp/test_app' ] - - task :clobber_test_app do - rm_rf 'tmp/test_app' - end - - file 'tmp/test_app' do - puts "Generating test app" - mkdir_p "tmp" - cd 'tmp' do - system "rails new test_app --skip-listen --skip-turbolinks --skip-gemfile --skip-bundle --skip-git --skip-test-unit" - - cd 'test_app' do - File.open('Gemfile', 'w+') do |file| - file << %Q{gem 'rc_rails', :git => "#{File.expand_path('../../../..', __FILE__)}"} - end - system "bundle install --quiet" - system "rails generate rspec:install" - end - end - end - - def migrate_up - puts "** Migrating up" - `rake db:migrate` - end - - def migrate_down - puts "** Migrating down" - `rake db:migrate VERSION=0` - end - - def generate_resource(name) - puts "** Generating rspec_scaffold for resource: #{name}" - `rails generate scaffold #{name.to_s.classify}` - end - - def make_resources_controller(name) - plural = name.to_s.pluralize - - controller = <<-end_eval -class #{plural.classify.pluralize}Controller < ApplicationController - resources_controller_for :#{plural} -end - end_eval - - puts "** Replacing app/controllers/#{plural}_controller.rb with:\n\n#{controller}\n" - File.open("app/controllers/#{plural}_controller.rb", "w+") {|f| f << controller } - end - - def cleanup_resource(name) - puts "** Cleaning up generated files for resource: #{name}" - plural = name.to_s.pluralize - - # remove app files - rm "app/models/#{name}.rb" - rm "app/controllers/#{plural}_controller.rb" - rm "app/helpers/#{plural}_helper.rb" - rm_rf "app/views/#{plural}" - - # remove migration - rm Dir["db/migrate/*_create_#{plural}.rb"] - - # revert routes - routes_rb = File.read("config/routes.rb") - routes_rb.sub!("\n map.resources :#{plural}\n", '') - File.open("config/routes.rb", "w+") {|f| f << routes_rb} - - # remove spec files - rm "spec/models/#{name}_spec.rb" - # rm "spec/fixtures/#{plural}.yml" - rm Dir["spec/controllers/#{plural}*_spec.rb"] - rm "spec/helpers/#{plural}_helper_spec.rb" - rm "spec/requests/#{plural}_spec.rb" - rm Dir["spec/routing/#{plural}*_spec.rb"] - rm_rf "spec/views/#{plural}" - end -end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index dc6bd86..56e93d1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,4 +1,4 @@ -ENV["RAILS_ENV"] ||= "test" +ENV["RAILS_ENV"] = "test" require 'rails/all' require 'rspec' require 'rspec/rails' diff --git a/spec/verify_rcov.rb b/spec/verify_rcov.rb deleted file mode 100644 index 9423b47..0000000 --- a/spec/verify_rcov.rb +++ /dev/null @@ -1,52 +0,0 @@ -module RCov - # A task that can verify that the RCov coverage doesn't - # drop below a certain threshold. It should be run after - # running Spec::Rake::SpecTask. - class VerifyTask < Rake::TaskLib - # Name of the task. Defaults to :verify_rcov - attr_accessor :name - - # Path to the index.html file generated by RCov, which - # is the file containing the total coverage. - # Defaults to 'coverage/index.html' - attr_accessor :index_html - - # Whether or not to output details. Defaults to true. - attr_accessor :verbose - - # The threshold value (in percent) for coverage. If the - # actual coverage is not equal to this value, the task will raise an - # exception. - attr_accessor :threshold - - # Require the threshold value be met exactly. This is the default. - attr_accessor :require_exact_threshold - - def initialize(name=:verify_rcov) - @name = name - @index_html = 'coverage/index.html' - @verbose = true - @require_exact_threshold = true - yield self if block_given? - raise "Threshold must be set" if @threshold.nil? - define - end - - def define - desc "Verify that rcov coverage is at least #{threshold}%" - task @name do - total_coverage = 0 - - File.open(index_html).each_line do |line| - if line =~ /\s*(\d+\.\d+)%\s*<\/tt>/ - total_coverage = $1.to_f - break - end - end - puts "Coverage: #{total_coverage}% (threshold: #{threshold}%)" if verbose - raise "Coverage must be at least #{threshold}% but was #{total_coverage}%" if total_coverage < threshold - raise "Coverage has increased above the threshold of #{threshold}% to #{total_coverage}%. You should update your threshold value." if (total_coverage > threshold) and require_exact_threshold - end - end - end -end \ No newline at end of file