Merge branch 'master' into fix/rails7-polymorphic-inverse-relationship-1473

Author: takaokouji

lib/jsonapi-resources.rb

@@ -9,13 +9,10 @@
 require 'jsonapi/cached_response_fragment'
 require 'jsonapi/response_document'
 require 'jsonapi/acts_as_resource_controller'
-if Rails::VERSION::MAJOR >= 6
-  ActiveSupport.on_load(:action_controller_base) do
-    require 'jsonapi/resource_controller'
-  end
-else
-  require 'jsonapi/resource_controller'
-end
+# Load resource_controller directly to avoid Zeitwerk autoloading issues in Rails 7.1+
+# Previously used ActiveSupport.on_load which caused race conditions with Zeitwerk
+# See: https://github.com/cerebris/jsonapi-resources/issues/1464
+require 'jsonapi/resource_controller'
 require 'jsonapi/resource_controller_metal'
 require 'jsonapi/resources/version'
 require 'jsonapi/configuration'

lib/jsonapi/configuration.rb

@@ -328,13 +328,18 @@ def self.configure
   # Rails 7.2+ made ActiveSupport::Deprecation.warn a private method
   # This helper provides backward-compatible deprecation warnings
   def self.warn_deprecated(message)
-    if defined?(ActiveSupport::Deprecation) && ActiveSupport::Deprecation.respond_to?(:warn)
-      # Rails < 7.2
-      ActiveSupport::Deprecation.warn(message)
+    if defined?(ActiveSupport::Deprecation)
+      begin
+        # Try to call warn as a class method (Rails < 7.2)
+        ActiveSupport::Deprecation.warn(message)
+      rescue NoMethodError
+        # Rails 7.2+: warn is now private, use instance method instead
+        version = defined?(JSONAPI::Resources::VERSION) ? JSONAPI::Resources::VERSION : '0.11.0'
+        deprecation = ActiveSupport::Deprecation.new(version, 'jsonapi-resources')
+        deprecation.warn(message)
+      end
     else
-      # Rails 7.2+ or fallback - use standard warning with deprecation formatting
-      # Rails 7.2 doesn't provide a public API for custom deprecation warnings
-      # So we use Kernel#warn with a deprecation prefix
+      # Fallback for environments without ActiveSupport
       warn "[DEPRECATION] #{message}"
     end
   end

test/fixtures/active_record.rb

@@ -2730,6 +2730,92 @@ class ArticleCommentResource < JSONAPI::Resource
   has_one :commentable, polymorphic: true
 end
 
+# Models and Resources for testing Issue #1467: ActiveModel-based resources
+# ActiveModel class without ActiveRecord
+class SimpleModel
+  include ActiveModel::Model
+
+  attr_accessor :id, :name, :description
+
+  def initialize(attributes = {})
+    @id = attributes[:id]
+    @name = attributes[:name]
+    @description = attributes[:description]
+  end
+
+  # Simple in-memory store for testing
+  @@store = {}
+
+  def self.reset_store!
+    @@store = {}
+  end
+
+  def self.create(attributes)
+    model = new(attributes)
+    @@store[model.id] = model
+    model
+  end
+
+  def self.find(id)
+    @@store[id]
+  end
+
+  def self.all
+    @@store.values
+  end
+
+  def self.find_by(attributes)
+    all.find { |model| attributes.all? { |key, value| model.send(key) == value } }
+  end
+end
+
+class SimpleModelResource < JSONAPI::BasicResource
+  model_name 'SimpleModel'
+  model_hint model: SimpleModel, resource: :simple_model
+
+  attribute :name
+  attribute :description
+
+  # Override find_by_key for non-ActiveRecord models
+  def self.find_by_key(key, options = {})
+    model = _model_class.find(key.to_i)
+    return nil unless model
+    new(model, options[:context])
+  end
+
+  # Override find_fragments for non-ActiveRecord models
+  def self.find_fragments(filters, options = {})
+    models = if filters.empty?
+      _model_class.all
+    else
+      # Simple filtering implementation
+      _model_class.all.select do |model|
+        filters.all? { |key, value| model.send(key).to_s == value.to_s }
+      end
+    end
+
+    models.each_with_object({}) do |model, hash|
+      resource = new(model, options[:context])
+      hash[resource.identity] = {
+        identity: resource.identity,
+        cache: nil,
+        attributes: nil,
+        related: nil
+      }
+    end
+  end
+end
+
+# This resource uses the default JSONAPI::Resource (ActiveRelationResource)
+# which will fail with ActiveModel models - reproduces Issue #1467
+class BrokenActiveModelResource < JSONAPI::Resource
+  model_name 'SimpleModel'
+  model_hint model: SimpleModel, resource: :broken_active_model
+
+  attribute :name
+  attribute :description
+end
+
 ### PORO Data - don't do this in a production app
 $breed_data = BreedData.new
 $breed_data.add(Breed.new(0, 'persian'))

test/unit/configuration/deprecation_test.rb

@@ -0,0 +1,35 @@
+require File.expand_path('../../../test_helper', __FILE__)
+
+# Test for Issue #1465: ActiveSupport::Deprecation private method error in Rails 7.2
+# https://github.com/cerebris/jsonapi-resources/issues/1465
+#
+# Rails 7.2 made ActiveSupport::Deprecation.warn a private method
+# This test ensures our warn_deprecated helper handles both old and new Rails versions
+
+class DeprecationTest < ActiveSupport::TestCase
+  def test_warn_deprecated_does_not_raise_error
+    # This should not raise NoMethodError: private method `warn' called
+    assert_nothing_raised do
+      JSONAPI.warn_deprecated('Test deprecation warning')
+    end
+  end
+
+  def test_warn_deprecated_with_activerecord_present
+    # Ensure the warning works when ActiveSupport::Deprecation is available
+    skip 'ActiveSupport::Deprecation not available' unless defined?(ActiveSupport::Deprecation)
+
+    assert_nothing_raised do
+      JSONAPI.warn_deprecated('Test deprecation with ActiveSupport')
+    end
+  end
+
+  def test_warn_deprecated_with_multiple_calls
+    # Test that multiple calls don't cause issues
+    # This is especially important for Rails 7.2 compatibility
+    assert_nothing_raised do
+      3.times do |i|
+        JSONAPI.warn_deprecated("Test deprecation warning #{i}")
+      end
+    end
+  end
+end

test/unit/resource/active_model_broken_test.rb

@@ -0,0 +1,37 @@
+require File.expand_path('../../../test_helper', __FILE__)
+
+# Test for Issue #1467: Demonstrate the problem with ActiveRelationResource and ActiveModel
+# https://github.com/cerebris/jsonapi-resources/issues/1467
+#
+# This test shows that using JSONAPI::Resource (which inherits from ActiveRelationResource)
+# with an ActiveModel-based model will fail because ActiveRelationResource expects
+# ActiveRecord methods like 'where'
+
+class ActiveModelBrokenTest < ActiveSupport::TestCase
+  def setup
+    # Seed some test data in the mock store
+    SimpleModel.reset_store!
+    @model1 = SimpleModel.create(id: 1, name: 'Test 1', description: 'First test')
+    @model2 = SimpleModel.create(id: 2, name: 'Test 2', description: 'Second test')
+  end
+
+  def teardown
+    SimpleModel.reset_store!
+  end
+
+  # This test demonstrates the problem: JSONAPI::Resource (ActiveRelationResource)
+  # attempts to use ActiveRecord methods on ActiveModel models
+  def test_activerelation_resource_fails_with_activemodel
+    resource_klass = BrokenActiveModelResource
+    context = {}
+
+    # This WILL raise NoMethodError because ActiveRelationResource expects ActiveRecord methods
+    # The error could be 'order', 'where', or other ActiveRecord-specific methods
+    error = assert_raises(NoMethodError) do
+      fragments = resource_klass.find_fragments({}, { context: context })
+    end
+
+    # Verify it's trying to use ActiveRecord-like query methods
+    assert_match(/undefined method.*(order|where|limit|offset)/, error.message)
+  end
+end

test/unit/resource/active_model_resource_test.rb

@@ -0,0 +1,63 @@
+require File.expand_path('../../../test_helper', __FILE__)
+
+# Test for Issue #1467: ActiveModel-based resources fail with 'where' method error
+# https://github.com/cerebris/jsonapi-resources/issues/1467
+#
+# This test reproduces the scenario where:
+# - A model uses ActiveModel::Model instead of ActiveRecord::Base
+# - The resource should work with BasicResource or proper configuration
+# - In v0.10.7+, these resources fail with "undefined method 'where'"
+
+class ActiveModelResourceTest < ActiveSupport::TestCase
+  def setup
+    # Seed some test data in the mock store
+    SimpleModel.reset_store!
+    @model1 = SimpleModel.create(id: 1, name: 'Test 1', description: 'First test')
+    @model2 = SimpleModel.create(id: 2, name: 'Test 2', description: 'Second test')
+  end
+
+  def teardown
+    SimpleModel.reset_store!
+  end
+
+  # Test that ActiveModel-based resources can be found without using ActiveRecord methods
+  def test_find_activemodel_resources
+    resource_klass = SimpleModelResource
+    context = {}
+
+    # This should not raise "undefined method 'where'"
+    assert_nothing_raised do
+      fragments = resource_klass.find_fragments({}, { context: context })
+      assert fragments.any?, "Should find SimpleModel fragments"
+      assert_equal 2, fragments.size, "Should find 2 fragments"
+    end
+  end
+
+  # Test that ActiveModel-based resources can be filtered
+  def test_filter_activemodel_resources
+    resource_klass = SimpleModelResource
+    context = {}
+
+    # Test with a filter
+    filters = { name: 'Test 1' }
+
+    # This should not attempt to call _model_class.where(name: 'Test 1')
+    assert_nothing_raised do
+      fragments = resource_klass.find_fragments(filters, { context: context })
+      assert_equal 1, fragments.size, "Should find 1 filtered fragment"
+    end
+  end
+
+  # Test that ActiveModel-based resources work with find_by_key
+  def test_find_by_key_activemodel_resources
+    resource_klass = SimpleModelResource
+    context = {}
+
+    # find_by_key should work for ActiveModel resources
+    assert_nothing_raised do
+      resource = resource_klass.find_by_key(1, context: context)
+      assert resource, "Should find resource by key"
+      assert_equal 'Test 1', resource.name
+    end
+  end
+end