fix: allow multiple resource relation retrieval methods (#1425)

* fix: check if relation retrieval in included via included_modules

* require 'jsonapi/relation_retrieval'

* feat: raise when cannot include different retrieval strategy

* test: multiple retrieval strategies

---------

Co-authored-by: lgebhardt <larry@cerebris.com>

Author: bf4

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'

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

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

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

lib/jsonapi/relation_retrieval.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+module JSONAPI
+  module RelationRetrieval
+  end
+end
+

lib/jsonapi/resource_common.rb

@@ -431,19 +431,24 @@ 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)
+          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
           include resource_retrieval_module
           extend "#{module_name}::ClassMethods".safe_constantize
           @_resource_retrieval_strategy_loaded = module_name

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