From e8ebfd837dd272d4c4a32290fa0fa283a698e6f9 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Thu, 4 Jan 2024 17:39:34 +0530 Subject: [PATCH 1/4] fix: check if relation retrieval in included via included_modules --- lib/jsonapi/active_relation_retrieval.rb | 2 ++ lib/jsonapi/active_relation_retrieval_v09.rb | 2 ++ lib/jsonapi/active_relation_retrieval_v10.rb | 2 ++ lib/jsonapi/relation_retrieval.rb | 7 +++++++ lib/jsonapi/resource_common.rb | 17 +++++++++-------- 5 files changed, 22 insertions(+), 8 deletions(-) create mode 100644 lib/jsonapi/relation_retrieval.rb diff --git a/lib/jsonapi/active_relation_retrieval.rb b/lib/jsonapi/active_relation_retrieval.rb index f331bd59..af4e62d1 100644 --- a/lib/jsonapi/active_relation_retrieval.rb +++ b/lib/jsonapi/active_relation_retrieval.rb @@ -2,6 +2,8 @@ module JSONAPI module ActiveRelationRetrieval + include ::JSONAPI::RelationRetrieval + def find_related_ids(relationship, options = {}) self.class.find_related_fragments(self, relationship, options).keys.collect { |rid| rid.id } end diff --git a/lib/jsonapi/active_relation_retrieval_v09.rb b/lib/jsonapi/active_relation_retrieval_v09.rb index 0b9b38cd..0ca023ea 100644 --- a/lib/jsonapi/active_relation_retrieval_v09.rb +++ b/lib/jsonapi/active_relation_retrieval_v09.rb @@ -2,6 +2,8 @@ module JSONAPI module ActiveRelationRetrievalV09 + include ::JSONAPI::RelationRetrieval + def find_related_ids(relationship, options = {}) self.class.find_related_fragments(self.fragment, relationship, options).keys.collect { |rid| rid.id } end diff --git a/lib/jsonapi/active_relation_retrieval_v10.rb b/lib/jsonapi/active_relation_retrieval_v10.rb index bf8466f8..f48c54c6 100644 --- a/lib/jsonapi/active_relation_retrieval_v10.rb +++ b/lib/jsonapi/active_relation_retrieval_v10.rb @@ -2,6 +2,8 @@ module JSONAPI module ActiveRelationRetrievalV10 + include ::JSONAPI::RelationRetrieval + def find_related_ids(relationship, options = {}) self.class.find_related_fragments(self, relationship, options).keys.collect { |rid| rid.id } end diff --git a/lib/jsonapi/relation_retrieval.rb b/lib/jsonapi/relation_retrieval.rb new file mode 100644 index 00000000..7c510955 --- /dev/null +++ b/lib/jsonapi/relation_retrieval.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module JSONAPI + module RelationRetrieval + end +end + diff --git a/lib/jsonapi/resource_common.rb b/lib/jsonapi/resource_common.rb index 0cd29051..a6fdf2da 100644 --- a/lib/jsonapi/resource_common.rb +++ b/lib/jsonapi/resource_common.rb @@ -431,19 +431,20 @@ def find_related_ids(relationship, options = {}) module ClassMethods def resource_retrieval_strategy(module_name = JSONAPI.configuration.default_resource_retrieval_strategy) - if @_resource_retrieval_strategy_loaded - warn "Resource retrieval strategy #{@_resource_retrieval_strategy_loaded} already loaded for #{self.name}" - return - end - module_name = module_name.to_s return if module_name.blank? || module_name == 'self' || module_name == 'none' - class_eval do - resource_retrieval_module = module_name.safe_constantize - raise "Unable to find resource_retrieval_strategy #{module_name}" unless resource_retrieval_module + resource_retrieval_module = module_name.safe_constantize + + raise "Unable to find resource_retrieval_strategy #{module_name}" unless resource_retrieval_module + if included_modules.include?(::JSONAPI::RelationRetrieval) + warn "Resource retrieval strategy #{module_name} already loaded for #{self.name}" + return + end + + class_eval do include resource_retrieval_module extend "#{module_name}::ClassMethods".safe_constantize @_resource_retrieval_strategy_loaded = module_name From ff0df46baa8cfd449f01d77e2d4c70cc55d0c1f7 Mon Sep 17 00:00:00 2001 From: lgebhardt Date: Thu, 4 Jan 2024 08:55:30 -0500 Subject: [PATCH 2/4] require 'jsonapi/relation_retrieval' --- lib/jsonapi-resources.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/jsonapi-resources.rb b/lib/jsonapi-resources.rb index 5d46099a..04d7908f 100644 --- a/lib/jsonapi-resources.rb +++ b/lib/jsonapi-resources.rb @@ -3,6 +3,7 @@ require 'jsonapi/resources/railtie' require 'jsonapi/naive_cache' require 'jsonapi/compiled_json' +require 'jsonapi/relation_retrieval' require 'jsonapi/active_relation_retrieval' require 'jsonapi/active_relation_retrieval_v09' require 'jsonapi/active_relation_retrieval_v10' From 04a2b4abb0a024194b52ddfee3f504b87a80894a Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Wed, 17 Jan 2024 01:12:52 -0600 Subject: [PATCH 3/4] feat: raise when cannot include different retrieval strategy --- lib/jsonapi/resource_common.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/jsonapi/resource_common.rb b/lib/jsonapi/resource_common.rb index a6fdf2da..c4731df1 100644 --- a/lib/jsonapi/resource_common.rb +++ b/lib/jsonapi/resource_common.rb @@ -440,8 +440,12 @@ def resource_retrieval_strategy(module_name = JSONAPI.configuration.default_reso raise "Unable to find resource_retrieval_strategy #{module_name}" unless resource_retrieval_module if included_modules.include?(::JSONAPI::RelationRetrieval) - warn "Resource retrieval strategy #{module_name} already loaded for #{self.name}" - return + if _resource_retrieval_strategy_loaded.nil? || module_name == _resource_retrieval_strategy_loaded + warn "Resource retrieval strategy #{module_name} already loaded for #{self.name}" + return + else + fail ArgumentError.new("Resource retrieval strategy #{_resource_retrieval_strategy_loaded} already loaded for #{self.name}. Cannot load #{module_name}") + end end class_eval do From b4905b4bb8486e1e8bdc31a7a67c6053f1574976 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Wed, 17 Jan 2024 01:14:58 -0600 Subject: [PATCH 4/4] test: multiple retrieval strategies --- ...active_relation_resource_retrieval_test.rb | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 test/unit/resource/multilple_active_relation_resource_retrieval_test.rb diff --git a/test/unit/resource/multilple_active_relation_resource_retrieval_test.rb b/test/unit/resource/multilple_active_relation_resource_retrieval_test.rb new file mode 100644 index 00000000..012924ed --- /dev/null +++ b/test/unit/resource/multilple_active_relation_resource_retrieval_test.rb @@ -0,0 +1,78 @@ +require File.expand_path('../../../test_helper', __FILE__) + +module VX +end + +class MultipleActiveRelationResourceTest < ActiveSupport::TestCase + def setup + end + + def teardown + teardown_test_constant(::VX, :BaseResource) + teardown_test_constant(::VX, :DuplicateSubBaseResource) + teardown_test_constant(::VX, :InvalidSubBaseResource) + teardown_test_constant(::VX, :ValidCustomBaseResource) + end + + def teardown_test_constant(namespace, constant_name) + return unless namespace.const_defined?(constant_name) + namespace.send(:remove_const, constant_name) + rescue NameError + end + + def test_correct_resource_retrieval_strategy + expected = 'JSONAPI::ActiveRelationRetrieval' + default = JSONAPI.configuration.default_resource_retrieval_strategy + assert_equal expected, default + assert_nil JSONAPI::Resource._resource_retrieval_strategy_loaded + + expected = 'JSONAPI::ActiveRelationRetrieval' + assert_silent do + ::VX.module_eval <<~MODULE + class BaseResource < JSONAPI::Resource + abstract + end + MODULE + end + assert_equal expected, VX::BaseResource._resource_retrieval_strategy_loaded + + strategy = 'JSONAPI::ActiveRelationRetrieval' + expected = 'JSONAPI::ActiveRelationRetrieval' + assert_output nil, "Resource retrieval strategy #{expected} already loaded for VX::DuplicateSubBaseResource\n" do + ::VX.module_eval <<~MODULE + class DuplicateSubBaseResource < JSONAPI::Resource + resource_retrieval_strategy '#{strategy}' + abstract + end + MODULE + end + assert_equal expected, VX::DuplicateSubBaseResource._resource_retrieval_strategy_loaded + + strategy = 'JSONAPI::ActiveRelationRetrievalV10' + expected = "Resource retrieval strategy #{default} already loaded for VX::InvalidSubBaseResource. Cannot load #{strategy}" + ex = assert_raises ArgumentError do + ::VX.module_eval <<~MODULE + class InvalidSubBaseResource < JSONAPI::Resource + resource_retrieval_strategy '#{strategy}' + abstract + end + MODULE + end + assert_equal expected, ex.message + + strategy = 'JSONAPI::ActiveRelationRetrievalV10' + expected = 'JSONAPI::ActiveRelationRetrievalV10' + assert_silent do + ::VX.module_eval <<~MODULE + class ValidCustomBaseResource + include JSONAPI::ResourceCommon + root_resource + abstract + immutable + resource_retrieval_strategy '#{strategy}' + end + MODULE + end + assert_equal expected, VX::ValidCustomBaseResource._resource_retrieval_strategy_loaded + end +end