From 7e0206f1e3b001c108950c8771ffa2bad00f1097 Mon Sep 17 00:00:00 2001 From: James Bracy Date: Thu, 17 Jul 2025 12:37:26 -0400 Subject: [PATCH 1/9] Support for Polymorphic Include --- .github/workflows/ci.yml | 1 - lib/standard_api/access_control_list.rb | 23 ++++++++----- lib/standard_api/controller.rb | 25 +++++++++++--- standardapi.gemspec | 6 ++-- .../nested_attributes/belongs_to_test.rb | 34 +++++++++++++++---- test/standard_api/standard_api_test.rb | 16 ++++----- .../app/controllers/acl/reference_acl.rb | 4 +++ 7 files changed, 78 insertions(+), 31 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c426ab2..31ee86e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,7 +20,6 @@ jobs: - 3.2 - 3.3 rails-version: - - 7.1.5.1 - 7.2.2.1 - 8.0.1 postgres-version: diff --git a/lib/standard_api/access_control_list.rb b/lib/standard_api/access_control_list.rb index 54bc3cb..4821f6d 100644 --- a/lib/standard_api/access_control_list.rb +++ b/lib/standard_api/access_control_list.rb @@ -52,7 +52,7 @@ def model_params @model_params = if self.respond_to?("filter_#{model_name(model)}_params", true) self.send("filter_#{model_name(model)}_params", resource, params[model_name(model)], id: params[:id]) else - filter_model_params(resource, params[model_name(model)]) + filter_model_params(resource, params[model_name(model)] || ActionController::Parameters.new) end end @@ -76,21 +76,26 @@ def filter_model_params(resource, model_params, id: nil, allow_id: nil) if self.respond_to?("nested_#{model_name(resource.class)}_attributes", true) self.send("nested_#{model_name(resource.class)}_attributes").each do |relation| association = resource.association(relation) + association_klass = if association.reflection.polymorphic? + model_params&.[]("#{ relation }_type")&.constantize + else + association.klass.base_class + end attributes_key = association.reflection.name.to_s if model_params.has_key?(attributes_key) - filter_method = "filter_#{association.klass.base_class.model_name.singular}_params" + filter_method = "filter_#{ association_klass.model_name.singular }_params" if model_params[attributes_key].nil? permitted_params[attributes_key] = nil elsif model_params[attributes_key].is_a?(Array) && model_params[attributes_key].all? { |a| a.keys.map(&:to_sym) == [:id] } permitted_params["#{association.reflection.name.to_s.singularize}_ids"] = model_params[attributes_key].map{|a| a['id']} elsif self.respond_to?(filter_method, true) permitted_params[attributes_key] = if model_params[attributes_key].is_a?(Array) - models = association.klass.find(model_params[attributes_key].map { |i| i['id'] }.compact) + models = association_klass.find(model_params[attributes_key].map { |i| i['id'] }.compact) model_params[attributes_key].map { |i| r = if i['id'] models.find { |r| r.id == i['id'] } else - association.klass.new + association_klass.new end i_params = self.send(filter_method, r, i, allow_id: true) r.assign_attributes(i_params) @@ -98,9 +103,9 @@ def filter_model_params(resource, model_params, id: nil, allow_id: nil) } else r = if id = model_params[attributes_key]['id'] - association.klass.find(id) + association_klass.find(id) else - association.klass.new() + association_klass.new() end i_params = self.send(filter_method, r, model_params[attributes_key], allow_id: true) r.assign_attributes(i_params) @@ -108,15 +113,15 @@ def filter_model_params(resource, model_params, id: nil, allow_id: nil) end else permitted_params[attributes_key] = if model_params[attributes_key].is_a?(Array) - models = association.klass.find(model_params[attributes_key].map { |i| i['id'] }.compact) + models = association_klass.find(model_params[attributes_key].map { |i| i['id'] }.compact) model_params[attributes_key].map { |i| - nested_record = i['id'] ? models.find { |r| r.id == i['id'] } : association.klass.new + nested_record = i['id'] ? models.find { |r| r.id == i['id'] } : association_klass.new i_params = filter_model_params(nested_record, i, allow_id: true) nested_record.assign_attributes(i_params) nested_record } else - nested_record = model_params[attributes_key]['id'] ? association.klass.find(model_params[attributes_key]['id']) : association.klass.new + nested_record = model_params[attributes_key]['id'] ? association_klass.find(model_params[attributes_key]['id']) : association_klass.new i_params = filter_model_params(nested_record, model_params[attributes_key], allow_id: true) nested_record.assign_attributes(i_params) nested_record diff --git a/lib/standard_api/controller.rb b/lib/standard_api/controller.rb index a8e0995..5f00822 100644 --- a/lib/standard_api/controller.rb +++ b/lib/standard_api/controller.rb @@ -7,7 +7,18 @@ def self.included(klass) klass.helper_method :includes, :orders, :model, :models, :resource_limit, :default_limit klass.before_action :set_standardapi_headers - klass.before_action :includes, except: [:destroy, :add_resource, :remove_resource, :json_schema] + + # We don't need includes on certain actions that don't return includes + # or on html requests since they are redirects + klass.before_action :includes, if: -> (c) do + ![ + :destroy, + :add_resource, + :remove_resource, + :json_schema + ].include?(c.action_name.to_sym) && c.request.format != :html + end + klass.rescue_from StandardAPI::ParameterMissing, with: :bad_request klass.rescue_from StandardAPI::UnpermittedParameters, with: :bad_request klass.append_view_path(File.join(File.dirname(__FILE__), 'views')) @@ -29,7 +40,7 @@ def schema Rails.application.eager_load! if !Rails.application.config.eager_load end end - + def json_schema Rails.application.eager_load! if !Rails.application.config.eager_load @includes = StandardAPI::Includes.normalize(params[:include] || {}) @@ -290,7 +301,7 @@ def resources query end - + def resource return @resource if instance_variable_get('@resource') @resource = if action_name == "create" @@ -304,7 +315,13 @@ def nested_includes(model, attributes) includes = {} attributes&.each do |key, value| if association = model.reflect_on_association(key) - includes[key] = value.is_a?(Array) ? {} : nested_includes(association.klass, value) + association_klass = if association.polymorphic? + attributes[key + "_type"]&.constantize || association.klass + else + association.klass + end + + includes[key] = value.is_a?(Array) ? {} : nested_includes(association_klass, value) end end diff --git a/standardapi.gemspec b/standardapi.gemspec index 5df19f3..c5a6cda 100644 --- a/standardapi.gemspec +++ b/standardapi.gemspec @@ -17,9 +17,9 @@ Gem::Specification.new do |spec| spec.test_files = `git ls-files -- {test}/*`.split("\n") spec.require_paths = ["lib", "test"] - spec.add_runtime_dependency 'rails', '>= 7.1.3' - spec.add_runtime_dependency 'activesupport', '>= 7.1.3' - spec.add_runtime_dependency 'actionpack', '>= 7.1.3' + spec.add_runtime_dependency 'rails', '>= 7.2.2' + spec.add_runtime_dependency 'activesupport', '>= 7.2.2' + spec.add_runtime_dependency 'actionpack', '>= 7.2.2' spec.add_runtime_dependency 'activerecord-sort', '>= 6.1.0' spec.add_runtime_dependency 'activerecord-filter', '>= 8.0.0' diff --git a/test/standard_api/nested_attributes/belongs_to_test.rb b/test/standard_api/nested_attributes/belongs_to_test.rb index 9885dd2..d6138c4 100644 --- a/test/standard_api/nested_attributes/belongs_to_test.rb +++ b/test/standard_api/nested_attributes/belongs_to_test.rb @@ -16,10 +16,10 @@ class BelongsToTest < ActionDispatch::IntegrationTest photo = Photo.last assert_equal 'Big Ben', photo.account.name end - + test 'create record and update nested record' do account = create(:account, name: 'Big Ben') - + @controller = PhotosController.new post photos_path, params: { photo: { account: {id: account.id, name: 'Little Jimmie'}} }, as: :json @@ -31,13 +31,13 @@ class BelongsToTest < ActionDispatch::IntegrationTest end # = Update Test - + test 'update record and create nested record' do photo = create(:photo) @controller = PhotosController.new put photo_path(photo), params: { photo: { account: {name: 'Big Ben'}} }, as: :json - + assert_response :ok photo.reload assert_equal 'Big Ben', photo.account.name @@ -49,7 +49,7 @@ class BelongsToTest < ActionDispatch::IntegrationTest @controller = PhotosController.new put photo_path(photo), params: { photo: { account: {name: 'Little Jimmie'}} }, as: :json - + assert_response :ok photo.reload assert_equal 'Little Jimmie', photo.account.name @@ -61,11 +61,33 @@ class BelongsToTest < ActionDispatch::IntegrationTest @controller = PhotosController.new put photo_path(photo), params: { photo: { account: nil} }, as: :json - + assert_response :ok photo.reload assert_nil photo.account end + test 'polymorphic include' do + account = create(:account) + + @controller = ReferencesController.new + post references_path, params: { + reference: { + subject: { + id: account.id, + name: 'Little Jimmie' + }, + subject_type: 'Account' + } + }, as: :json + + assert_response :created + + reference = JSON(response.body) + assert_equal account.id, reference['subject_id'] + assert_equal "Account", reference['subject_type'] + assert_equal 'Little Jimmie', reference["subject"]["name"] + end + end end diff --git a/test/standard_api/standard_api_test.rb b/test/standard_api/standard_api_test.rb index f04a1f6..13d8cf0 100644 --- a/test/standard_api/standard_api_test.rb +++ b/test/standard_api/standard_api_test.rb @@ -96,16 +96,16 @@ def normalizers @controller = DocumentsController.new @controller.params = ActionController::Parameters.new @controller.action_name = 'create' - assert_equal @controller.send(:model_params), ActionController::Parameters.new + assert_equal @controller.send(:model_params), ActionController::Parameters.new.permit! end test 'Controller#model_params defaults to ActionController::Parameters when no resource_attributes' do @controller = ReferencesController.new @controller.params = ActionController::Parameters.new @controller.action_name = 'create' - assert_equal @controller.send(:model_params), ActionController::Parameters.new + assert_equal @controller.send(:model_params), ActionController::Parameters.new.permit! end - + test 'Controller#model_params is conditional based on existing resource' do document = create(:document, type: 'movie') @controller = DocumentsController.new @@ -265,12 +265,12 @@ def normalizers patch document_path(pdf), params: { document: pdf.attributes } assert_redirected_to document_path(pdf) end - + test 'Controller#create has Affected-Rows header' do attrs = attributes_for(:property) post properties_path, params: { property: attrs }, as: :json assert_equal response.headers['Affected-Rows'], 1 - + attrs = attributes_for(:property, :invalid) post properties_path, params: { property: attrs }, as: :json assert_equal response.headers['Affected-Rows'], 0 @@ -280,7 +280,7 @@ def normalizers property = create(:property) patch property_path(property), params: { property: property.attributes }, as: :json assert_equal response.headers['Affected-Rows'], 1 - + attrs = attributes_for(:property, :invalid) patch property_path(property), params: { property: attrs }, as: :json assert_equal response.headers['Affected-Rows'], 0 @@ -290,13 +290,13 @@ def normalizers property = create(:property) delete property_path(property), as: :json assert_equal response.headers['Affected-Rows'], 1 - + assert_raises ActiveRecord::RecordNotFound do delete property_path(property), as: :json assert_equal response.headers['Affected-Rows'], 0 end end - + # = View Tests test 'rendering tables' do diff --git a/test/standard_api/test_app/app/controllers/acl/reference_acl.rb b/test/standard_api/test_app/app/controllers/acl/reference_acl.rb index aac2862..e90170f 100644 --- a/test/standard_api/test_app/app/controllers/acl/reference_acl.rb +++ b/test/standard_api/test_app/app/controllers/acl/reference_acl.rb @@ -1,5 +1,9 @@ module ReferenceACL + def nested + [ :subject ] + end + def includes { subject: [ :landlord, :photos ] } end From 9e0bed01bec242e0ca0fa086e1e36430143988b1 Mon Sep 17 00:00:00 2001 From: James Bracy Date: Tue, 22 Jul 2025 12:17:39 -0400 Subject: [PATCH 2/9] Filter nested includes --- lib/standard_api/controller.rb | 8 ++++---- lib/standard_api/includes.rb | 12 +++++++----- lib/standard_api/test_case.rb | 7 +++++-- test/standard_api/controller/include_test.rb | 14 +++++++++++++- .../nested_attributes/has_many_test.rb | 13 ------------- .../standard_api/nested_attributes/has_one_test.rb | 6 +++--- .../test_app/app/controllers/acl/property_acl.rb | 2 +- test/standard_api/test_app/models.rb | 5 ++++- 8 files changed, 37 insertions(+), 30 deletions(-) diff --git a/lib/standard_api/controller.rb b/lib/standard_api/controller.rb index 5f00822..5fae496 100644 --- a/lib/standard_api/controller.rb +++ b/lib/standard_api/controller.rb @@ -245,9 +245,9 @@ def models @models.map!(&:model).compact! end - def model_includes - if self.respond_to?("#{model.model_name.singular}_includes", true) - self.send("#{model.model_name.singular}_includes") + def model_includes(model_class=model) + if self.respond_to?("#{model_class.model_name.singular}_includes", true) + self.send("#{model_class.model_name.singular}_includes") else [] end @@ -325,7 +325,7 @@ def nested_includes(model, attributes) end end - includes + StandardAPI::Includes.sanitize(includes, model_includes(model), raise_on_unpermitted: false) end def includes diff --git a/lib/standard_api/includes.rb b/lib/standard_api/includes.rb index 1ed67da..5315bce 100644 --- a/lib/standard_api/includes.rb +++ b/lib/standard_api/includes.rb @@ -69,7 +69,7 @@ def self.normalize(includes) # sanitize({:key => {:value => {}}}, {:key => [:value]}) # => {:key => {:value => {}}} # sanitize({:key => {:value => {}}}, {:key => {:value => true}}) # => {:key => {:value => {}}} # sanitize({:key => {:value => {}}}, [:key]) => # Raises ParseError - def self.sanitize(includes, permit, normalized=false) + def self.sanitize(includes, permit, normalized=false, raise_on_unpermitted: true) includes = normalize(includes) if !normalized permitted = ActiveSupport::HashWithIndifferentAccess.new @@ -79,12 +79,14 @@ def self.sanitize(includes, permit, normalized=false) permit = normalize(permit.with_indifferent_access) includes.each do |k, v| - permitted[k] = if permit.has_key?(k) - sanitize(v, permit[k] || {}, true) + if permit.has_key?(k) + permitted[k] = sanitize(v, permit[k] || {}, true) elsif ['limit', 'when', 'where', 'order', 'distinct', 'distinct_on'].include?(k.to_s) - v + permitted[k] = v else - raise StandardAPI::UnpermittedParameters.new([k]) + if raise_on_unpermitted + raise StandardAPI::UnpermittedParameters.new([k]) + end end end diff --git a/lib/standard_api/test_case.rb b/lib/standard_api/test_case.rb index 6e0e9fe..aaf8c0a 100644 --- a/lib/standard_api/test_case.rb +++ b/lib/standard_api/test_case.rb @@ -20,7 +20,7 @@ def assert_equal_or_nil(expected, *args) end def self.included(klass) - [:filters, :orders, :includes].each do |attribute| + [:filters, :includes].each do |attribute| klass.send(:class_attribute, attribute) end @@ -28,7 +28,6 @@ def self.included(klass) model_class = klass.name.gsub(/Test$/, '').constantize.model klass.send(:filters=, model_class.attribute_names) - klass.send(:orders=, model_class.attribute_names) klass.send(:includes=, model_class.reflect_on_all_associations.map(&:name)) rescue NameError => e raise e if e.message != "uninitialized constant #{model_class_name}" @@ -60,6 +59,10 @@ def supports_format(format, action=nil) count > 0 end + def orders + controller_class.new.send(model.model_name.singular + "_orders") + end + def default_orders controller_class.new.send(:default_orders) end diff --git a/test/standard_api/controller/include_test.rb b/test/standard_api/controller/include_test.rb index 8fbb568..cbbf80c 100644 --- a/test/standard_api/controller/include_test.rb +++ b/test/standard_api/controller/include_test.rb @@ -4,6 +4,18 @@ class ControllerIncludesTest < ActionDispatch::IntegrationTest # = Including an invalid include + # = Including an invalid nested include + + test "Controller#create with a invalid nested include" do + property_attributes = attributes_for(:property, non_include_photo: attributes_for(:photo)) + + assert_difference 'Photo.count', 1 do + post '/properties', params: { property: property_attributes }, as: :json + end + json = JSON.parse(response.body) + assert_nil json['non_include_photo'] + end + test "Controller#create with a valid include" do property = build(:property) @@ -92,7 +104,7 @@ class ControllerIncludesTest < ActionDispatch::IntegrationTest assert_equal 1, property.reload.photos.count assert_response :created end - + # This test passes because includes are not used, response is a HEAD response # and no includes are used. test "Controller#remove_resource with an invalid include" do diff --git a/test/standard_api/nested_attributes/has_many_test.rb b/test/standard_api/nested_attributes/has_many_test.rb index 5e4c981..55dc1fd 100644 --- a/test/standard_api/nested_attributes/has_many_test.rb +++ b/test/standard_api/nested_attributes/has_many_test.rb @@ -68,18 +68,5 @@ class HasManyTest < ActionDispatch::IntegrationTest assert_equal [], property.accounts end - test 'update record and include nested record in response' do - account = create(:account, name: 'A Co.') - property = create(:property, name: 'Empire State Building', accounts: [account]) - - @controller = PropertiesController.new - put property_path(property), params: { property: { name: 'John Hancock Center', accounts: [{id: account.id, name: "B Co."}]} }, as: :json - - attributes = JSON.parse(response.body) - assert_response :ok - assert_equal account.id, attributes["accounts"][0]["id"] - assert_equal "B Co.", attributes["accounts"][0]["name"] - end - end end diff --git a/test/standard_api/nested_attributes/has_one_test.rb b/test/standard_api/nested_attributes/has_one_test.rb index 527cf38..b6357dd 100644 --- a/test/standard_api/nested_attributes/has_one_test.rb +++ b/test/standard_api/nested_attributes/has_one_test.rb @@ -17,7 +17,7 @@ class HasOneTest < ActionDispatch::IntegrationTest assert_equal photo.id, photo.camera.photo_id assert_equal 'Sony', photo.camera.make end - + test 'create record and update nested record' do camera = create(:camera, make: 'Sony') @@ -31,7 +31,7 @@ class HasOneTest < ActionDispatch::IntegrationTest end # = Update Test - + test 'update record and create nested record' do photo = create(:photo) @@ -68,4 +68,4 @@ class HasOneTest < ActionDispatch::IntegrationTest end end -end \ No newline at end of file +end diff --git a/test/standard_api/test_app/app/controllers/acl/property_acl.rb b/test/standard_api/test_app/app/controllers/acl/property_acl.rb index 8dd9ef2..14ec8b1 100644 --- a/test/standard_api/test_app/app/controllers/acl/property_acl.rb +++ b/test/standard_api/test_app/app/controllers/acl/property_acl.rb @@ -41,7 +41,7 @@ def includes # allowed to .... # only add to and from the relation, can also create or update the subresource def nested - [ :photos, :accounts ] + [ :photos, :accounts, :non_include_photo ] end end diff --git a/test/standard_api/test_app/models.rb b/test/standard_api/test_app/models.rb index c771137..8e67724 100644 --- a/test/standard_api/test_app/models.rb +++ b/test/standard_api/test_app/models.rb @@ -30,6 +30,8 @@ class Property < ActiveRecord::Base has_one :document_attachments, class_name: "Attachment", as: :record, inverse_of: :record has_one :document, through: "document_attachments" + belongs_to :non_include_photo, class_name: 'Photo' + accepts_nested_attributes_for :photos def english_name @@ -151,7 +153,8 @@ def self.up t.integer "numericality", default: 2 t.string "build_type" t.boolean "agree_to_terms", default: true - t.string "phone_number", default: '999-999-9999' + t.string "phone_number", default: '999-999-9999' + t.integer "non_include_photo_id" end create_table "references", force: :cascade do |t| From e194fb42d7c21d03103cc5074163086c26009008 Mon Sep 17 00:00:00 2001 From: James Bracy Date: Tue, 22 Jul 2025 12:22:05 -0400 Subject: [PATCH 3/9] Remove polymorphic stuff --- Rakefile | 3 ++- lib/standard_api/access_control_list.rb | 23 ++++++++----------- lib/standard_api/controller.rb | 8 +------ test/standard_api/json_schema_test.rb | 2 +- .../nested_attributes/belongs_to_test.rb | 22 ------------------ test/standard_api/standard_api_test.rb | 4 ++-- .../app/controllers/acl/reference_acl.rb | 4 ---- 7 files changed, 15 insertions(+), 51 deletions(-) diff --git a/Rakefile b/Rakefile index 938787b..3c217fb 100644 --- a/Rakefile +++ b/Rakefile @@ -11,6 +11,7 @@ namespace :test do Rake::TestTask.new(encoder => ["#{encoder}:env"]) do |t| t.libs << 'lib' << 'test' t.test_files = FileList['test/**/*_test.rb'] + # t.test_files = FileList['test/standard_api/nested_attributes/has_one_test.rb'] t.warning = true t.verbose = false end @@ -19,7 +20,7 @@ namespace :test do task(:env) { ENV["TSENCODER"] = encoder } end end - + desc "Run test with all encoders" task all: ENCODERS.shuffle.map{ |e| "test:#{e}" } end diff --git a/lib/standard_api/access_control_list.rb b/lib/standard_api/access_control_list.rb index 4821f6d..54bc3cb 100644 --- a/lib/standard_api/access_control_list.rb +++ b/lib/standard_api/access_control_list.rb @@ -52,7 +52,7 @@ def model_params @model_params = if self.respond_to?("filter_#{model_name(model)}_params", true) self.send("filter_#{model_name(model)}_params", resource, params[model_name(model)], id: params[:id]) else - filter_model_params(resource, params[model_name(model)] || ActionController::Parameters.new) + filter_model_params(resource, params[model_name(model)]) end end @@ -76,26 +76,21 @@ def filter_model_params(resource, model_params, id: nil, allow_id: nil) if self.respond_to?("nested_#{model_name(resource.class)}_attributes", true) self.send("nested_#{model_name(resource.class)}_attributes").each do |relation| association = resource.association(relation) - association_klass = if association.reflection.polymorphic? - model_params&.[]("#{ relation }_type")&.constantize - else - association.klass.base_class - end attributes_key = association.reflection.name.to_s if model_params.has_key?(attributes_key) - filter_method = "filter_#{ association_klass.model_name.singular }_params" + filter_method = "filter_#{association.klass.base_class.model_name.singular}_params" if model_params[attributes_key].nil? permitted_params[attributes_key] = nil elsif model_params[attributes_key].is_a?(Array) && model_params[attributes_key].all? { |a| a.keys.map(&:to_sym) == [:id] } permitted_params["#{association.reflection.name.to_s.singularize}_ids"] = model_params[attributes_key].map{|a| a['id']} elsif self.respond_to?(filter_method, true) permitted_params[attributes_key] = if model_params[attributes_key].is_a?(Array) - models = association_klass.find(model_params[attributes_key].map { |i| i['id'] }.compact) + models = association.klass.find(model_params[attributes_key].map { |i| i['id'] }.compact) model_params[attributes_key].map { |i| r = if i['id'] models.find { |r| r.id == i['id'] } else - association_klass.new + association.klass.new end i_params = self.send(filter_method, r, i, allow_id: true) r.assign_attributes(i_params) @@ -103,9 +98,9 @@ def filter_model_params(resource, model_params, id: nil, allow_id: nil) } else r = if id = model_params[attributes_key]['id'] - association_klass.find(id) + association.klass.find(id) else - association_klass.new() + association.klass.new() end i_params = self.send(filter_method, r, model_params[attributes_key], allow_id: true) r.assign_attributes(i_params) @@ -113,15 +108,15 @@ def filter_model_params(resource, model_params, id: nil, allow_id: nil) end else permitted_params[attributes_key] = if model_params[attributes_key].is_a?(Array) - models = association_klass.find(model_params[attributes_key].map { |i| i['id'] }.compact) + models = association.klass.find(model_params[attributes_key].map { |i| i['id'] }.compact) model_params[attributes_key].map { |i| - nested_record = i['id'] ? models.find { |r| r.id == i['id'] } : association_klass.new + nested_record = i['id'] ? models.find { |r| r.id == i['id'] } : association.klass.new i_params = filter_model_params(nested_record, i, allow_id: true) nested_record.assign_attributes(i_params) nested_record } else - nested_record = model_params[attributes_key]['id'] ? association_klass.find(model_params[attributes_key]['id']) : association_klass.new + nested_record = model_params[attributes_key]['id'] ? association.klass.find(model_params[attributes_key]['id']) : association.klass.new i_params = filter_model_params(nested_record, model_params[attributes_key], allow_id: true) nested_record.assign_attributes(i_params) nested_record diff --git a/lib/standard_api/controller.rb b/lib/standard_api/controller.rb index 5fae496..b1be3de 100644 --- a/lib/standard_api/controller.rb +++ b/lib/standard_api/controller.rb @@ -315,13 +315,7 @@ def nested_includes(model, attributes) includes = {} attributes&.each do |key, value| if association = model.reflect_on_association(key) - association_klass = if association.polymorphic? - attributes[key + "_type"]&.constantize || association.klass - else - association.klass - end - - includes[key] = value.is_a?(Array) ? {} : nested_includes(association_klass, value) + includes[key] = value.is_a?(Array) ? {} : nested_includes(association.klass, value) end end diff --git a/test/standard_api/json_schema_test.rb b/test/standard_api/json_schema_test.rb index 81ca65d..f577214 100644 --- a/test/standard_api/json_schema_test.rb +++ b/test/standard_api/json_schema_test.rb @@ -37,7 +37,7 @@ class JSONSchemaTest < ActionDispatch::IntegrationTest schema = JSON(response.body) assert_equal true, schema.dig('properties', 'created_at', 'readOnly') - assert_equal nil, schema.dig('properties', 'name', 'readOnly') + assert_nil schema.dig('properties', 'name', 'readOnly') end test 'Controller#json_schema.json type' do diff --git a/test/standard_api/nested_attributes/belongs_to_test.rb b/test/standard_api/nested_attributes/belongs_to_test.rb index d6138c4..94d03c9 100644 --- a/test/standard_api/nested_attributes/belongs_to_test.rb +++ b/test/standard_api/nested_attributes/belongs_to_test.rb @@ -67,27 +67,5 @@ class BelongsToTest < ActionDispatch::IntegrationTest assert_nil photo.account end - test 'polymorphic include' do - account = create(:account) - - @controller = ReferencesController.new - post references_path, params: { - reference: { - subject: { - id: account.id, - name: 'Little Jimmie' - }, - subject_type: 'Account' - } - }, as: :json - - assert_response :created - - reference = JSON(response.body) - assert_equal account.id, reference['subject_id'] - assert_equal "Account", reference['subject_type'] - assert_equal 'Little Jimmie', reference["subject"]["name"] - end - end end diff --git a/test/standard_api/standard_api_test.rb b/test/standard_api/standard_api_test.rb index 13d8cf0..526e260 100644 --- a/test/standard_api/standard_api_test.rb +++ b/test/standard_api/standard_api_test.rb @@ -96,14 +96,14 @@ def normalizers @controller = DocumentsController.new @controller.params = ActionController::Parameters.new @controller.action_name = 'create' - assert_equal @controller.send(:model_params), ActionController::Parameters.new.permit! + assert_equal @controller.send(:model_params), ActionController::Parameters.new end test 'Controller#model_params defaults to ActionController::Parameters when no resource_attributes' do @controller = ReferencesController.new @controller.params = ActionController::Parameters.new @controller.action_name = 'create' - assert_equal @controller.send(:model_params), ActionController::Parameters.new.permit! + assert_equal @controller.send(:model_params), ActionController::Parameters.new end test 'Controller#model_params is conditional based on existing resource' do diff --git a/test/standard_api/test_app/app/controllers/acl/reference_acl.rb b/test/standard_api/test_app/app/controllers/acl/reference_acl.rb index e90170f..aac2862 100644 --- a/test/standard_api/test_app/app/controllers/acl/reference_acl.rb +++ b/test/standard_api/test_app/app/controllers/acl/reference_acl.rb @@ -1,9 +1,5 @@ module ReferenceACL - def nested - [ :subject ] - end - def includes { subject: [ :landlord, :photos ] } end From 2f12e46753d5539abb4aa291257640f92fb37e51 Mon Sep 17 00:00:00 2001 From: Jon Bracy Date: Tue, 22 Jul 2025 13:18:38 -0500 Subject: [PATCH 4/9] Update CI and Rakefile --- .github/workflows/ci.yml | 2 ++ Rakefile | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 31ee86e..be92cca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,6 +19,8 @@ jobs: ruby-version: - 3.2 - 3.3 + - 3.4 + - 3.5.0-preview1 rails-version: - 7.2.2.1 - 8.0.1 diff --git a/Rakefile b/Rakefile index 3c217fb..b771bae 100644 --- a/Rakefile +++ b/Rakefile @@ -10,8 +10,7 @@ namespace :test do ENCODERS.each do |encoder| Rake::TestTask.new(encoder => ["#{encoder}:env"]) do |t| t.libs << 'lib' << 'test' - t.test_files = FileList['test/**/*_test.rb'] - # t.test_files = FileList['test/standard_api/nested_attributes/has_one_test.rb'] + t.test_files = FileList[ARGV[1] ? ARGV[1] : 'test/**/*_test.rb'] t.warning = true t.verbose = false end From 2240f125f601cc27a1ec4da8ccb473d381302280 Mon Sep 17 00:00:00 2001 From: James Bracy Date: Tue, 22 Jul 2025 15:13:52 -0400 Subject: [PATCH 5/9] version bump --- lib/standard_api/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/standard_api/version.rb b/lib/standard_api/version.rb index ed51e2b..cbd0a54 100644 --- a/lib/standard_api/version.rb +++ b/lib/standard_api/version.rb @@ -1,3 +1,3 @@ module StandardAPI - VERSION = '8.0.1' + VERSION = '8.0.2' end From 33866e80260caf8ea1b9d7fea72c59feb4bcd82b Mon Sep 17 00:00:00 2001 From: Jon Bracy Date: Wed, 23 Jul 2025 12:27:46 -0500 Subject: [PATCH 6/9] Revert some changes and get test working --- lib/standard_api/controller.rb | 31 +++++++++++++------------------ lib/standard_api/includes.rb | 12 +++++------- standardapi.gemspec | 1 + test/standard_api/test_helper.rb | 4 ++++ 4 files changed, 23 insertions(+), 25 deletions(-) diff --git a/lib/standard_api/controller.rb b/lib/standard_api/controller.rb index b1be3de..fd70298 100644 --- a/lib/standard_api/controller.rb +++ b/lib/standard_api/controller.rb @@ -7,17 +7,7 @@ def self.included(klass) klass.helper_method :includes, :orders, :model, :models, :resource_limit, :default_limit klass.before_action :set_standardapi_headers - - # We don't need includes on certain actions that don't return includes - # or on html requests since they are redirects - klass.before_action :includes, if: -> (c) do - ![ - :destroy, - :add_resource, - :remove_resource, - :json_schema - ].include?(c.action_name.to_sym) && c.request.format != :html - end + klass.before_action :includes, except: [:destroy, :add_resource, :remove_resource, :json_schema] klass.rescue_from StandardAPI::ParameterMissing, with: :bad_request klass.rescue_from StandardAPI::UnpermittedParameters, with: :bad_request @@ -245,9 +235,9 @@ def models @models.map!(&:model).compact! end - def model_includes(model_class=model) - if self.respond_to?("#{model_class.model_name.singular}_includes", true) - self.send("#{model_class.model_name.singular}_includes") + def model_includes + if self.respond_to?("#{model.model_name.singular}_includes", true) + self.send("#{model.model_name.singular}_includes") else [] end @@ -303,7 +293,8 @@ def resources end def resource - return @resource if instance_variable_get('@resource') + return @resource if instance_variable_defined?(:@resource) + @resource = if action_name == "create" model.new else @@ -314,15 +305,19 @@ def resource def nested_includes(model, attributes) includes = {} attributes&.each do |key, value| - if association = model.reflect_on_association(key) + if association = model.reflect_on_association(key) && + self.respond_to?("nested_#{model_name(model)}_attributes", true) && + self.send("nested_#{model_name(model)}_attributes").include?(model.model_name.singular) includes[key] = value.is_a?(Array) ? {} : nested_includes(association.klass, value) end end - StandardAPI::Includes.sanitize(includes, model_includes(model), raise_on_unpermitted: false) + includes end def includes + return @includes if instance_variable_defined?(:@includes) + @includes ||= if params[:include] StandardAPI::Includes.sanitize(params[:include], model_includes) else @@ -330,7 +325,7 @@ def includes end if (action_name == 'create' || action_name == 'update') && model && params.has_key?(model.model_name.singular) - @includes.reverse_merge!(nested_includes(model, params[model.model_name.singular].to_unsafe_h)) + @includes.reverse_merge!(nested_includes(model, params[model.model_name.singular].to_unsafe_h)) end @includes diff --git a/lib/standard_api/includes.rb b/lib/standard_api/includes.rb index 5315bce..1ed67da 100644 --- a/lib/standard_api/includes.rb +++ b/lib/standard_api/includes.rb @@ -69,7 +69,7 @@ def self.normalize(includes) # sanitize({:key => {:value => {}}}, {:key => [:value]}) # => {:key => {:value => {}}} # sanitize({:key => {:value => {}}}, {:key => {:value => true}}) # => {:key => {:value => {}}} # sanitize({:key => {:value => {}}}, [:key]) => # Raises ParseError - def self.sanitize(includes, permit, normalized=false, raise_on_unpermitted: true) + def self.sanitize(includes, permit, normalized=false) includes = normalize(includes) if !normalized permitted = ActiveSupport::HashWithIndifferentAccess.new @@ -79,14 +79,12 @@ def self.sanitize(includes, permit, normalized=false, raise_on_unpermitted: true permit = normalize(permit.with_indifferent_access) includes.each do |k, v| - if permit.has_key?(k) - permitted[k] = sanitize(v, permit[k] || {}, true) + permitted[k] = if permit.has_key?(k) + sanitize(v, permit[k] || {}, true) elsif ['limit', 'when', 'where', 'order', 'distinct', 'distinct_on'].include?(k.to_s) - permitted[k] = v + v else - if raise_on_unpermitted - raise StandardAPI::UnpermittedParameters.new([k]) - end + raise StandardAPI::UnpermittedParameters.new([k]) end end diff --git a/standardapi.gemspec b/standardapi.gemspec index c5a6cda..cc53b75 100644 --- a/standardapi.gemspec +++ b/standardapi.gemspec @@ -30,6 +30,7 @@ Gem::Specification.new do |spec| spec.add_development_dependency 'jbuilder' spec.add_development_dependency "rake" spec.add_development_dependency 'minitest' + spec.add_development_dependency 'minitest-reporters' spec.add_development_dependency "simplecov" spec.add_development_dependency "factory_bot_rails" spec.add_development_dependency "faker" diff --git a/test/standard_api/test_helper.rb b/test/standard_api/test_helper.rb index 62353fd..64a38d7 100644 --- a/test/standard_api/test_helper.rb +++ b/test/standard_api/test_helper.rb @@ -7,6 +7,10 @@ require 'standard_api/test_case' require 'byebug' require 'mocha/minitest' +require 'minitest/reporters' + + +Minitest::Reporters.use! Minitest::Reporters::SpecReporter.new # Setup the test db ActiveSupport.test_order = :random From 5243af330a0d8625fc21de78f7d961679313bee5 Mon Sep 17 00:00:00 2001 From: Jon Bracy Date: Wed, 23 Jul 2025 13:27:17 -0500 Subject: [PATCH 7/9] Add testcase we are trying to fix --- lib/standard_api/controller.rb | 2 -- .../belongs_to_polymorphic_test.rb | 21 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 test/standard_api/nested_attributes/belongs_to_polymorphic_test.rb diff --git a/lib/standard_api/controller.rb b/lib/standard_api/controller.rb index fd70298..bb469b9 100644 --- a/lib/standard_api/controller.rb +++ b/lib/standard_api/controller.rb @@ -316,8 +316,6 @@ def nested_includes(model, attributes) end def includes - return @includes if instance_variable_defined?(:@includes) - @includes ||= if params[:include] StandardAPI::Includes.sanitize(params[:include], model_includes) else diff --git a/test/standard_api/nested_attributes/belongs_to_polymorphic_test.rb b/test/standard_api/nested_attributes/belongs_to_polymorphic_test.rb new file mode 100644 index 0000000..31711b0 --- /dev/null +++ b/test/standard_api/nested_attributes/belongs_to_polymorphic_test.rb @@ -0,0 +1,21 @@ +require 'standard_api/test_helper' + +module NestedAttributes + class BelongsToPolymorphicTest < ActionDispatch::IntegrationTest + # include StandardAPI::TestCase + include StandardAPI::Helpers + + # = Create Test + + # TODO: we maybe should return an error response when something is filtered + # out of a create or update + test 'create record and nested polymorphic record not created because rejected by the ACL' do + @controller = AccountsController.new + + assert_nothing_raised do + post account_path, params: { account: { name: 'Smee', subject: {make: 'Nokia'}, subject_type: 'Camera' } }, as: :json + end + end + + end +end From 0760db6fd9f4f43437d9d47c559ddc5766d998e4 Mon Sep 17 00:00:00 2001 From: Jon Bracy Date: Wed, 23 Jul 2025 13:38:54 -0500 Subject: [PATCH 8/9] Add back removed test --- lib/standard_api/controller.rb | 2 +- test/standard_api/controller/include_test.rb | 12 ------------ .../nested_attributes/has_many_test.rb | 14 ++++++++++++++ .../test_app/app/controllers/acl/property_acl.rb | 2 +- test/standard_api/test_app/models.rb | 3 --- 5 files changed, 16 insertions(+), 17 deletions(-) diff --git a/lib/standard_api/controller.rb b/lib/standard_api/controller.rb index bb469b9..f377d36 100644 --- a/lib/standard_api/controller.rb +++ b/lib/standard_api/controller.rb @@ -307,7 +307,7 @@ def nested_includes(model, attributes) attributes&.each do |key, value| if association = model.reflect_on_association(key) && self.respond_to?("nested_#{model_name(model)}_attributes", true) && - self.send("nested_#{model_name(model)}_attributes").include?(model.model_name.singular) + self.send("nested_#{model_name(model)}_attributes").include?(association.name) includes[key] = value.is_a?(Array) ? {} : nested_includes(association.klass, value) end end diff --git a/test/standard_api/controller/include_test.rb b/test/standard_api/controller/include_test.rb index cbbf80c..daa65b2 100644 --- a/test/standard_api/controller/include_test.rb +++ b/test/standard_api/controller/include_test.rb @@ -4,18 +4,6 @@ class ControllerIncludesTest < ActionDispatch::IntegrationTest # = Including an invalid include - # = Including an invalid nested include - - test "Controller#create with a invalid nested include" do - property_attributes = attributes_for(:property, non_include_photo: attributes_for(:photo)) - - assert_difference 'Photo.count', 1 do - post '/properties', params: { property: property_attributes }, as: :json - end - json = JSON.parse(response.body) - assert_nil json['non_include_photo'] - end - test "Controller#create with a valid include" do property = build(:property) diff --git a/test/standard_api/nested_attributes/has_many_test.rb b/test/standard_api/nested_attributes/has_many_test.rb index 55dc1fd..84e2569 100644 --- a/test/standard_api/nested_attributes/has_many_test.rb +++ b/test/standard_api/nested_attributes/has_many_test.rb @@ -68,5 +68,19 @@ class HasManyTest < ActionDispatch::IntegrationTest assert_equal [], property.accounts end + test 'update record and include nested record in response' do + account = create(:account, name: 'A Co.') + property = create(:property, name: 'Empire State Building', accounts: [account]) + + @controller = PropertiesController.new + put property_path(property), params: { property: { name: 'John Hancock Center', accounts: [{id: account.id, name: "B Co."}]} }, as: :json + + attributes = JSON.parse(response.body) + assert_response :ok + puts response.body + assert_equal account.id, attributes["accounts"][0]["id"] + assert_equal "B Co.", attributes["accounts"][0]["name"] + end + end end diff --git a/test/standard_api/test_app/app/controllers/acl/property_acl.rb b/test/standard_api/test_app/app/controllers/acl/property_acl.rb index 14ec8b1..8dd9ef2 100644 --- a/test/standard_api/test_app/app/controllers/acl/property_acl.rb +++ b/test/standard_api/test_app/app/controllers/acl/property_acl.rb @@ -41,7 +41,7 @@ def includes # allowed to .... # only add to and from the relation, can also create or update the subresource def nested - [ :photos, :accounts, :non_include_photo ] + [ :photos, :accounts ] end end diff --git a/test/standard_api/test_app/models.rb b/test/standard_api/test_app/models.rb index 8e67724..493c595 100644 --- a/test/standard_api/test_app/models.rb +++ b/test/standard_api/test_app/models.rb @@ -29,8 +29,6 @@ class Property < ActiveRecord::Base has_one :landlord, class_name: 'Account' has_one :document_attachments, class_name: "Attachment", as: :record, inverse_of: :record has_one :document, through: "document_attachments" - - belongs_to :non_include_photo, class_name: 'Photo' accepts_nested_attributes_for :photos @@ -154,7 +152,6 @@ def self.up t.string "build_type" t.boolean "agree_to_terms", default: true t.string "phone_number", default: '999-999-9999' - t.integer "non_include_photo_id" end create_table "references", force: :cascade do |t| From 68fe830fac94ab198bc151caa2fdc13e34fc6eaf Mon Sep 17 00:00:00 2001 From: Jon Bracy Date: Thu, 24 Jul 2025 10:13:20 -0500 Subject: [PATCH 9/9] Fix test --- lib/standard_api/controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/standard_api/controller.rb b/lib/standard_api/controller.rb index f377d36..b371a41 100644 --- a/lib/standard_api/controller.rb +++ b/lib/standard_api/controller.rb @@ -305,7 +305,8 @@ def resource def nested_includes(model, attributes) includes = {} attributes&.each do |key, value| - if association = model.reflect_on_association(key) && + association = model.reflect_on_association(key) + if association && self.respond_to?("nested_#{model_name(model)}_attributes", true) && self.send("nested_#{model_name(model)}_attributes").include?(association.name) includes[key] = value.is_a?(Array) ? {} : nested_includes(association.klass, value)