Merge pull request #15 from speee/fix/quote-method-mysql-compatibility

Fix quote method to use database adapter for MySQL compatibility

Author: takaokouji

Dockerfile.ruby3.1

@@ -4,7 +4,7 @@ FROM ruby:3.1.5
 
 # Install dependencies
 RUN apt-get update -qq && \
-    apt-get install -y build-essential libpq-dev nodejs && \
+    apt-get install -y build-essential libpq-dev default-libmysqlclient-dev nodejs && \
     apt-get clean && \
     rm -rf /var/lib/apt/lists/*
 

Gemfile

@@ -10,6 +10,7 @@ version = ENV['RAILS_VERSION'] || 'default'
 
 platforms :ruby do
   gem 'pg'
+  gem 'mysql2'
 
   if version.start_with?('4.2', '5.0')
     gem 'sqlite3', '~> 1.3.13'

docker-compose.yml

@@ -115,7 +115,91 @@ services:
       - RAILS_VERSION=${RAILS_VERSION:-8.1.2}
     command: /bin/bash
 
+  # Database services
+  mysql:
+    image: mysql:8.0
+    container_name: jsonapi-mysql
+    environment:
+      MYSQL_ROOT_PASSWORD: root
+      MYSQL_DATABASE: jsonapi_test
+    ports:
+      - "3306:3306"
+    volumes:
+      - mysql-data:/var/lib/mysql
+    healthcheck:
+      test: ["CMD", "mysqladmin", "ping", "-h", "localhost"]
+      interval: 10s
+      timeout: 5s
+      retries: 5
+
+  postgres:
+    image: postgres:14
+    container_name: jsonapi-postgres
+    environment:
+      POSTGRES_USER: postgres
+      POSTGRES_PASSWORD: postgres
+      POSTGRES_DB: jsonapi_test
+    ports:
+      - "5432:5432"
+    volumes:
+      - postgres-data:/var/lib/postgresql/data
+    healthcheck:
+      test: ["CMD-SHELL", "pg_isready -U postgres"]
+      interval: 10s
+      timeout: 5s
+      retries: 5
+
+  # Rails 6.1 with MySQL
+  rails-6.1-mysql:
+    <<: *test-base
+    container_name: jsonapi-rails-6.1-mysql
+    depends_on:
+      mysql:
+        condition: service_healthy
+    environment:
+      - RAILS_VERSION=6.1.7.10
+      - DATABASE_URL=mysql2://root:root@mysql:3306/jsonapi_test
+    command: bash -c "rm -f Gemfile.lock && /usr/local/bin/bundler _2.4.14_ install && /usr/local/bin/bundler _2.4.14_ exec rake test"
+
+  # Rails 6.1 with PostgreSQL
+  rails-6.1-postgres:
+    <<: *test-base
+    container_name: jsonapi-rails-6.1-postgres
+    depends_on:
+      postgres:
+        condition: service_healthy
+    environment:
+      - RAILS_VERSION=6.1.7.10
+      - DATABASE_URL=postgresql://postgres:postgres@postgres:5432/jsonapi_test
+    command: bash -c "rm -f Gemfile.lock && /usr/local/bin/bundler _2.4.14_ install && /usr/local/bin/bundler _2.4.14_ exec rake test"
+
+  # Rails 7.0 with MySQL
+  rails-7.0-mysql:
+    <<: *test-base
+    container_name: jsonapi-rails-7.0-mysql
+    depends_on:
+      mysql:
+        condition: service_healthy
+    environment:
+      - RAILS_VERSION=7.0.10
+      - DATABASE_URL=mysql2://root:root@mysql:3306/jsonapi_test
+    command: bash -c "rm -f Gemfile.lock && /usr/local/bin/bundler _2.4.14_ install && /usr/local/bin/bundler _2.4.14_ exec rake test"
+
+  # Rails 7.0 with PostgreSQL
+  rails-7.0-postgres:
+    <<: *test-base
+    container_name: jsonapi-rails-7.0-postgres
+    depends_on:
+      postgres:
+        condition: service_healthy
+    environment:
+      - RAILS_VERSION=7.0.10
+      - DATABASE_URL=postgresql://postgres:postgres@postgres:5432/jsonapi_test
+    command: bash -c "rm -f Gemfile.lock && /usr/local/bin/bundler _2.4.14_ install && /usr/local/bin/bundler _2.4.14_ exec rake test"
+
 volumes:
   bundle-cache-ruby31:
   bundle-cache-ruby27:
   bundle-cache-ruby34:
+  mysql-data:
+  postgres-data:

lib/jsonapi/active_relation_resource.rb

@@ -96,6 +96,12 @@ def find_to_populate_by_keys(keys, options = {})
       #    the ResourceInstances matching the filters, sorting, and pagination rules along with any request
       #    additional_field values
       def find_fragments(filters, options = {})
+        # PORO compatibility: if _model_class doesn't respond to :all, it's not ActiveRecord
+        # Fall back to find_by_key-based implementation for 0.9.x compatibility
+        unless _model_class.respond_to?(:all)
+          return find_fragments_for_non_active_record(filters, options)
+        end
+
         include_directives = options.fetch(:include_directives, {})
         resource_klass = self
 
@@ -368,6 +374,22 @@ def join_relationship(records:, relationship:, resource_type: nil, join_type: :i
         records.merge(relationship_records)
       end
 
+      # Fallback implementation of find_fragments for non-ActiveRecord models (PORO)
+      # This provides 0.9.x compatibility by using find_by_key instead of records.pluck
+      def find_fragments_for_non_active_record(filters, options)
+        context = options[:context]
+        primary_keys = options.fetch(:primary_keys, nil) || filters[_primary_key]
+        return {} if primary_keys.blank?
+
+        fragments = {}
+        Array(primary_keys).each do |key|
+          resource = find_by_key(key, context: context)
+          identity = JSONAPI::ResourceIdentity.new(self, key)
+          fragments[identity] = JSONAPI::ResourceFragment.new(identity, resource: resource)
+        end
+        fragments
+      end
+
       protected
 
       def to_one_relationships_for_linkage(include_related)
@@ -847,7 +869,7 @@ def alias_table_field(table, field, quoted = false)
       end
 
       def quote(field)
-        "\"#{field.to_s}\""
+        _model_class.connection.quote_column_name(field.to_s)
       end
 
       def apply_filters(records, filters, options = {})
@@ -859,6 +881,7 @@ def apply_filters(records, filters, options = {})
 
         records
       end
+      public :apply_filters  # Public for 0.9.x backward compatibility
 
       def get_aliased_field(path_with_field, join_manager)
         path = JSONAPI::Path.new(resource_klass: self, path_string: path_with_field)

lib/jsonapi/operation_result.rb

@@ -21,6 +21,35 @@ def to_hash(serializer = nil)
     end
   end
 
+  # Backward compatibility module for 0.9.x result.resource access
+  # In 0.9.x, OperationResult had a `resource` accessor that returned the single resource.
+  # This module provides the same interface for code that relied on `result.resource`.
+  module ResourceSetResultMethods
+    # Returns the first resource from the resource_set
+    def resource
+      each_resource { |r| return r }
+      nil
+    end
+
+    # Returns all resources from the resource_set
+    def resources
+      result = []
+      each_resource { |r| result << r }
+      result
+    end
+
+    private
+
+    def each_resource
+      return unless resource_set&.resource_klasses
+      resource_set.resource_klasses.each_value do |identities|
+        identities.each_value do |data|
+          yield data[:resource] if data[:resource]
+        end
+      end
+    end
+  end
+
   class ErrorsOperationResult < OperationResult
     attr_accessor :errors
 
@@ -41,6 +70,8 @@ def to_hash(serializer = nil)
   end
 
   class ResourceSetOperationResult < OperationResult
+    include ResourceSetResultMethods
+
     attr_accessor :resource_set, :pagination_params
 
     def initialize(code, resource_set, options = {})
@@ -61,6 +92,8 @@ def to_hash(serializer)
   end
 
   class ResourcesSetOperationResult < OperationResult
+    include ResourceSetResultMethods
+
     attr_accessor :resource_set, :pagination_params, :record_count, :page_count
 
     def initialize(code, resource_set, options = {})

test/unit/resource/backward_compatibility_test.rb

@@ -0,0 +1,73 @@
+require File.expand_path('../../../test_helper', __FILE__)
+
+# Tests for apply_filters public accessibility (0.9.x compatibility)
+class ApplyFiltersAccessibilityTest < ActiveSupport::TestCase
+  def test_apply_filters_is_publicly_accessible
+    # In 0.9.x, apply_filters was public and could be called from outside
+    # Verify it's accessible as a public method
+    assert PostResource.respond_to?(:apply_filters, false),
+      "apply_filters should be a public class method for 0.9.x compatibility"
+  end
+
+  def test_apply_filters_can_be_called_externally
+    # Verify apply_filters can be called from outside the class
+    records = Post.all
+    filters = {}
+
+    # This should not raise NoMethodError
+    result = PostResource.apply_filters(records, filters)
+
+    assert_kind_of ActiveRecord::Relation, result
+  end
+end
+
+# Tests for ResourceSetOperationResult backward compatibility
+class ResourceSetOperationResultBackwardCompatibilityTest < ActiveSupport::TestCase
+  def setup
+    # Create a helper to build ResourceSet with resources
+  end
+
+  def test_resource_method_returns_first_resource
+    # Create a ResourceSet with a single resource using the proper API
+    post = Post.find(1)
+    resource = PostResource.new(post, nil)
+
+    # Use the ResourceSet with a resource (not nil)
+    resource_set = JSONAPI::ResourceSet.new(resource)
+    resource_set.mark_populated!
+
+    result = JSONAPI::ResourceSetOperationResult.new(:ok, resource_set)
+
+    # 0.9.x style: result.resource
+    assert result.respond_to?(:resource), "ResourceSetOperationResult should respond to :resource for 0.9.x compatibility"
+    assert_equal resource, result.resource
+  end
+
+  def test_resource_method_returns_nil_for_empty_resource_set
+    # Create an empty resource set by passing an empty array
+    resource_set = JSONAPI::ResourceSet.new([])
+    result = JSONAPI::ResourceSetOperationResult.new(:ok, resource_set)
+
+    assert_nil result.resource
+  end
+
+  def test_resources_method_returns_all_resources
+    # Create a ResourceSet with multiple resources
+    post1 = Post.find(1)
+    post2 = Post.find(2)
+    resource1 = PostResource.new(post1, nil)
+    resource2 = PostResource.new(post2, nil)
+
+    # Use the ResourceSet with an array of resources
+    resource_set = JSONAPI::ResourceSet.new([resource1, resource2])
+    resource_set.mark_populated!
+
+    result = JSONAPI::ResourceSetOperationResult.new(:ok, resource_set)
+
+    # 0.9.x style: result.resources (for collections)
+    assert result.respond_to?(:resources), "ResourceSetOperationResult should respond to :resources for 0.9.x compatibility"
+    assert_equal 2, result.resources.length
+    assert_includes result.resources, resource1
+    assert_includes result.resources, resource2
+  end
+end

test/unit/resource/poro_resource_test.rb

@@ -0,0 +1,101 @@
+require File.expand_path('../../../test_helper', __FILE__)
+
+# PORO (Plain Old Ruby Object) for testing
+class PoroModel
+  attr_accessor :id, :name, :created_at
+
+  def initialize(id, name)
+    @id = id
+    @name = name
+    @created_at = Time.now
+  end
+
+  # Note: No .all method - this is a PORO, not ActiveRecord
+end
+
+# Resource for PORO model
+class PoroModelResource < JSONAPI::Resource
+  model_name 'PoroModel'
+  attributes :name
+
+  class << self
+    def find_by_key(key, options = {})
+      context = options[:context]
+      model = PoroModel.new(key, "PORO Item #{key}")
+      new(model, context)
+    end
+
+    def create(context)
+      model = PoroModel.new(SecureRandom.uuid, "New PORO")
+      new(model, context)
+    end
+  end
+end
+
+class PoroResourceTest < ActiveSupport::TestCase
+  def test_poro_model_does_not_respond_to_all
+    # Verify our test PORO doesn't have .all method
+    refute PoroModel.respond_to?(:all),
+      "PORO model should not respond to :all"
+  end
+
+  def test_find_fragments_works_with_poro
+    # find_fragments should work with PORO by falling back to find_by_key
+    filters = {}
+    options = {
+      context: {},
+      primary_keys: ['key1', 'key2']
+    }
+
+    fragments = PoroModelResource.find_fragments(filters, options)
+
+    assert_equal 2, fragments.length, "Should return 2 fragments"
+
+    # Verify fragments have correct structure
+    fragments.each do |identity, fragment|
+      assert_kind_of JSONAPI::ResourceIdentity, identity
+      assert_kind_of JSONAPI::ResourceFragment, fragment
+      assert_not_nil fragment.resource, "Fragment should have resource for PORO"
+    end
+  end
+
+  def test_find_fragments_returns_empty_for_poro_without_keys
+    filters = {}
+    options = {
+      context: {},
+      primary_keys: nil
+    }
+
+    fragments = PoroModelResource.find_fragments(filters, options)
+
+    assert_equal({}, fragments, "Should return empty hash when no keys")
+  end
+
+  def test_find_fragments_with_single_key_for_poro
+    filters = {}
+    options = {
+      context: {},
+      primary_keys: 'single_key'
+    }
+
+    fragments = PoroModelResource.find_fragments(filters, options)
+
+    assert_equal 1, fragments.length, "Should return 1 fragment"
+  end
+
+  def test_active_record_resource_still_uses_original_find_fragments
+    # Ensure ActiveRecord resources still use the optimized pluck-based implementation
+    # This test verifies we didn't break existing behavior
+    filters = { PostResource._primary_key => [1, 2] }
+    options = { context: {} }
+
+    # PostResource uses ActiveRecord, should work normally
+    fragments = PostResource.find_fragments(filters, options)
+
+    assert_equal 2, fragments.length
+    fragments.each do |identity, fragment|
+      assert_kind_of JSONAPI::ResourceIdentity, identity
+      assert_kind_of JSONAPI::ResourceFragment, fragment
+    end
+  end
+end