Support Rails 6.1

* Setup github actions for CI (cherry picked from commit e894827) * Rails 6.1 (#1352) * Rails v6.1 compatibility (#1346) * Fixes for rails 6.1.1 * Fixes for ruby 3.0.0 * Remove rails 5.0 support Co-authored-by: Igor Gonchar <[email protected]> (cherry picked from commit 1322b59)
cerebris · Feb 4, 2021 · 7e9fd44 · 7e9fd44
1 parent ba1c1b6
commit 7e9fd44
Show file tree

Hide file tree

Showing 10 changed files with 107 additions and 55 deletions.
diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml
@@ -0,0 +1,62 @@
+name: CI
+
+on:
+  push:
+    branches: [ 'master', 'release-0-8', 'release-0-9', 'release-0-10' ]
+  pull_request:
+    branches: ['**']
+
+jobs:
+  tests:
+    runs-on: ubuntu-latest
+    services:
+      postgres:
+        image: postgres
+        env:
+          POSTGRES_PASSWORD: password
+          POSTGRES_DB: test
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+        ports:
+          - 5432:5432
+    strategy:
+      fail-fast: false
+      matrix:
+        ruby:
+          - 2.6.6
+          - 2.7.2
+          - 3.0.0
+        rails:
+          - 6.1.1
+          - 6.0.3.4
+          - 5.2.4.4
+          - 5.1.7
+        database_url:
+          - postgresql://postgres:password@localhost:5432/test
+          - sqlite3:test_db
+        exclude:
+          - ruby: 3.0.0
+            rails: 6.0.3.4
+          - ruby: 3.0.0
+            rails: 5.2.4.4
+          - ruby: 3.0.0
+            rails: 5.1.7
+          - database_url: postgresql://postgres:password@localhost:5432/test
+            rails: 5.1.7
+    env:
+      RAILS_VERSION: ${{ matrix.rails }}
+      DATABASE_URL: ${{ matrix.database_url }}
+    name: Ruby ${{ matrix.ruby }} Rails ${{ matrix.rails }} DB ${{ matrix.database_url }}
+    steps:
+      - uses: actions/checkout@v2
+      - name: Set up Ruby
+        uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: ${{ matrix.ruby }}
+      - name: Install dependencies
+        run: bundle install --jobs 4 --retry 3
+      - name: Run tests
+        run: bundle exec rake test
diff --git a/.travis.yml b/.travis.yml
diff --git a/lib/jsonapi/active_relation_resource.rb b/lib/jsonapi/active_relation_resource.rb
@@ -756,7 +756,7 @@ def apply_single_sort(records, field, direction, options)
 
       # Assumes ActiveRecord's counting. Override if you need a different counting method
       def count_records(records)
-        if Rails::VERSION::MAJOR >= 5 && ActiveRecord::VERSION::MINOR >= 1
+        if (Rails::VERSION::MAJOR == 5 && ActiveRecord::VERSION::MINOR >= 1) || Rails::VERSION::MAJOR >= 6
           records.count(:all)
         else
           records.count

diff --git a/lib/jsonapi/request_parser.rb b/lib/jsonapi/request_parser.rb
@@ -418,7 +418,7 @@ def parse_sort_criteria(resource_klass, sort_criteria)
         sorts = sort_criteria
       elsif sort_criteria.is_a?(String)
         begin
-          raw = URI.unescape(sort_criteria)
+          raw = URI.decode_www_form_component(sort_criteria)
           sorts = CSV.parse_line(raw)
         rescue CSV::MalformedCSVError
           fail JSONAPI::Exceptions::InvalidSortCriteria.new(format_key(resource_klass._type), raw)

diff --git a/lib/jsonapi/resource_controller_metal.rb b/lib/jsonapi/resource_controller_metal.rb
@@ -5,10 +5,10 @@ class ResourceControllerMetal < ActionController::Metal
       ActionController::Rendering,
       ActionController::Renderers::All,
       ActionController::StrongParameters,
-      ActionController::ForceSSL,
+      Gem::Requirement.new('< 6.1').satisfied_by?(ActionPack.gem_version) ? ActionController::ForceSSL : nil,
       ActionController::Instrumentation,
       JSONAPI::ActsAsResourceController
-    ].freeze
+    ].compact.freeze
 
     # Note, the url_helpers are not loaded. This will prevent links from being generated for resources, and warnings
     # will be emitted. Link support can be added by including `Rails.application.routes.url_helpers`, and links

diff --git a/test/fixtures/active_record.rb b/test/fixtures/active_record.rb
@@ -503,7 +503,7 @@ class Post < ActiveRecord::Base
   belongs_to :writer, class_name: 'Person', foreign_key: 'author_id'
   has_many :comments
   has_and_belongs_to_many :tags, join_table: :posts_tags
-  has_many :special_post_tags, source: :tag
+  has_many :special_post_tags
   has_many :special_tags, through: :special_post_tags, source: :tag
   belongs_to :section
   belongs_to :parent_post, class_name: 'Post', foreign_key: 'parent_post_id'
@@ -745,8 +745,8 @@ class Picture < ActiveRecord::Base
   belongs_to :author, class_name: 'Person', foreign_key: 'author_id'
 
   belongs_to :imageable, polymorphic: true
-  belongs_to :document, -> { where( pictures: { imageable_type: 'Document' } ).eager_load( :pictures ) }, foreign_key: 'imageable_id'
-  belongs_to :product, -> { where( pictures: { imageable_type: 'Product' } ).eager_load( :pictures ) }, foreign_key: 'imageable_id'
+  belongs_to :document, -> { where( pictures: { imageable_type: 'Document' } ) }, foreign_key: 'imageable_id'
+  belongs_to :product, -> { where( pictures: { imageable_type: 'Product' } ) }, foreign_key: 'imageable_id'
 
   has_one :file_properties, as: 'fileable'
 end

diff --git a/test/integration/requests/request_test.rb b/test/integration/requests/request_test.rb
@@ -1608,7 +1608,7 @@ def test_caching_included_singleton
                        }
 
     $test_user = Person.find(1001)
-    assert_equal 2, JSONAPI.configuration.resource_cache.instance_variable_get(:@key_access).length
+    assert_equal 2, JSONAPI.configuration.resource_cache.instance_variable_get(:@data).length
 
     get "/api/v9/people/#{$test_user.id}?include=preferences"
     assert_jsonapi_response 200
@@ -1655,7 +1655,7 @@ def test_caching_included_singleton
                          ]
                        }
 
-    assert_equal 4, JSONAPI.configuration.resource_cache.instance_variable_get(:@key_access).length
+    assert_equal 4, JSONAPI.configuration.resource_cache.instance_variable_get(:@data).length
 
   ensure
     JSONAPI.configuration = original_config
@@ -1700,7 +1700,7 @@ def test_caching_singleton_primary
                          }
                        }
 
-    assert_equal 1, JSONAPI.configuration.resource_cache.instance_variable_get(:@key_access).length
+    assert_equal 1, JSONAPI.configuration.resource_cache.instance_variable_get(:@data).length
 
     $test_user = Person.find(1001)
 
@@ -1728,7 +1728,7 @@ def test_caching_singleton_primary
                          }
                        }
 
-    assert_equal 2, JSONAPI.configuration.resource_cache.instance_variable_get(:@key_access).length
+    assert_equal 2, JSONAPI.configuration.resource_cache.instance_variable_get(:@data).length
 
   ensure
     JSONAPI.configuration = original_config

diff --git a/test/test_helper.rb b/test/test_helper.rb
@@ -88,9 +88,9 @@ class Engine < ::Rails::Engine
 # Monkeypatch ActionController::TestCase to delete the RAW_POST_DATA on subsequent calls in the same test.
 if Rails::VERSION::MAJOR >= 5
   module ClearRawPostHeader
-    def process(action, *args)
+    def process(action, **args)
       @request.delete_header 'RAW_POST_DATA'
-      super
+      super action, **args
     end
   end
 
@@ -560,13 +560,13 @@ def assert_cacheable_jsonapi_get(url, cached_classes = :all)
 end
 
 class ActionController::TestCase
-  def assert_cacheable_get(action, *args)
+  def assert_cacheable_get(action, **args)
     assert_nil JSONAPI.configuration.resource_cache
 
     normal_queries = []
     normal_query_callback = lambda {|_, _, _, _, payload| normal_queries.push payload[:sql] }
     ActiveSupport::Notifications.subscribed(normal_query_callback, 'sql.active_record') do
-      get action, *args
+      get action, **args
     end
     non_caching_response = json_response_sans_all_backtraces
     non_caching_status = response.status
@@ -602,7 +602,7 @@ def assert_cacheable_get(action, *args)
               @controller = nil
               setup_controller_request_and_response
               @request.headers.merge!(orig_request_headers.dup)
-              get action, *args
+              get action, **args
             end
           end
         rescue Exception

diff --git a/test/unit/active_relation_resource_finder/join_manager_test.rb b/test/unit/active_relation_resource_finder/join_manager_test.rb
@@ -110,7 +110,11 @@ def test_add_nested_scoped_joins
     records = Api::V10::PostResource.records({})
     records = join_manager.join(records, {})
 
-    sql = 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" ON "people"."id" = "posts"."author_id" LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = ' + db_true +  ' AND "author"."special" = ' + db_true
+    if (Rails::VERSION::MAJOR == 6 && Rails::VERSION::MINOR >= 1) || Rails::VERSION::MAJOR > 6
+      sql = 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" author ON author."id" = "posts"."author_id" LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = ' + db_true + ' AND "author"."special" = ' + db_true
+    else
+      sql = 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" ON "people"."id" = "posts"."author_id" LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = ' + db_true + ' AND "author"."special" = ' + db_true
+    end
 
     assert_equal sql, records.to_sql
 
@@ -132,7 +136,11 @@ def test_add_nested_scoped_joins
     records = join_manager.join(records, {})
 
     # Note sql is in different order, but aliases should still be right
-    sql = 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "people" ON "people"."id" = "posts"."author_id" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = ' + db_true +  ' AND "author"."special" = ' + db_true
+    if (Rails::VERSION::MAJOR == 6 && Rails::VERSION::MINOR >= 1) || Rails::VERSION::MAJOR > 6
+      sql = 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "people" author ON author."id" = "posts"."author_id" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = ' + db_true + ' AND "author"."special" = ' + db_true
+    else
+      sql = 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "people" ON "people"."id" = "posts"."author_id" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = ' + db_true + ' AND "author"."special" = ' + db_true
+    end
 
     assert_equal sql, records.to_sql
 
@@ -171,7 +179,11 @@ def test_add_nested_joins_with_fields
     records = Api::V10::PostResource.records({})
     records = join_manager.join(records, {})
 
-    sql = 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" ON "people"."id" = "posts"."author_id" LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = ' + db_true +  ' AND "author"."special" = ' + db_true
+    if (Rails::VERSION::MAJOR == 6 && Rails::VERSION::MINOR >= 1) || Rails::VERSION::MAJOR > 6
+      sql = 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" author ON author."id" = "posts"."author_id" LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = ' + db_true + ' AND "author"."special" = ' + db_true
+    else
+      sql = 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" ON "people"."id" = "posts"."author_id" LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = ' + db_true + ' AND "author"."special" = ' + db_true
+    end
 
     assert_equal sql, records.to_sql
 
@@ -190,13 +202,18 @@ def test_add_joins_with_sub_relationship
     records = Api::V10::PostResource.records({})
     records = join_manager.join(records, {})
 
-    sql = 'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" ON "people"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" LEFT OUTER JOIN "comments" "comments_people" ON "comments_people"."author_id" = "people"."id" WHERE "comments"."approved" = ' + db_true +  ' AND "author"."special" = ' + db_true
+    if (Rails::VERSION::MAJOR == 6 && Rails::VERSION::MINOR >= 1) || Rails::VERSION::MAJOR > 6
+      sql = 'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" author ON author."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" LEFT OUTER JOIN "comments" "comments_people" ON "comments_people"."author_id" = "people"."id" WHERE "comments"."approved" = ' + db_true + ' AND "author"."special" = ' + db_true
+      assert_hash_equals({alias: 'author', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::CommentResource._relationship(:author)))
+    else
+      sql = 'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" ON "people"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" LEFT OUTER JOIN "comments" "comments_people" ON "comments_people"."author_id" = "people"."id" WHERE "comments"."approved" = ' + db_true + ' AND "author"."special" = ' + db_true
+      assert_hash_equals({alias: 'people', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::CommentResource._relationship(:author)))
+    end
 
     assert_equal sql, records.to_sql
 
     assert_hash_equals({alias: 'comments', join_type: :inner}, join_manager.source_join_details)
     assert_hash_equals({alias: 'comments', join_type: :inner}, join_manager.join_details_by_relationship(Api::V10::PostResource._relationship(:comments)))
-    assert_hash_equals({alias: 'people', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::CommentResource._relationship(:author)))
     assert_hash_equals({alias: 'tags', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::CommentResource._relationship(:tags)))
     assert_hash_equals({alias: 'comments_people', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::PersonResource._relationship(:comments)))
   end
@@ -263,7 +280,11 @@ def test_polymorphic_join_belongs_to_filter_on_resource
     records = PictureResource.records({})
     records = join_manager.join(records, {})
 
-    assert_equal 'SELECT "pictures".* FROM "pictures" LEFT OUTER JOIN "documents" ON "documents"."id" = "pictures"."imageable_id" AND "pictures"."imageable_type" = \'Document\' LEFT OUTER JOIN "products" ON "products"."id" = "pictures"."imageable_id" AND "pictures"."imageable_type" = \'Product\' LEFT OUTER JOIN "file_properties" ON "file_properties"."fileable_id" = "pictures"."id" AND "file_properties"."fileable_type" = \'Picture\'', records.to_sql
+    #TODO: Fix this with a better test
+    sql_v1 = 'SELECT "pictures".* FROM "pictures" LEFT OUTER JOIN "documents" ON "documents"."id" = "pictures"."imageable_id" AND "pictures"."imageable_type" = \'Document\' LEFT OUTER JOIN "products" ON "products"."id" = "pictures"."imageable_id" AND "pictures"."imageable_type" = \'Product\' LEFT OUTER JOIN "file_properties" ON "file_properties"."fileable_id" = "pictures"."id" AND "file_properties"."fileable_type" = \'Picture\''
+    sql_v2 = 'SELECT "pictures".* FROM "pictures" LEFT OUTER JOIN "documents" ON "documents"."id" = "pictures"."imageable_id" AND "pictures"."imageable_type" = \'Document\' LEFT OUTER JOIN "products" ON "products"."id" = "pictures"."imageable_id" AND "pictures"."imageable_type" = \'Product\' LEFT OUTER JOIN "file_properties" ON "file_properties"."fileable_type" = \'Picture\' AND "file_properties"."fileable_id" = "pictures"."id"'
+    assert records.to_sql == sql_v1 || records.to_sql == sql_v2, 'did not generate an expected sql statement'
+
     assert_hash_equals({alias: 'pictures', join_type: :root}, join_manager.source_join_details)
     assert_hash_equals({alias: 'products', join_type: :left}, join_manager.join_details_by_polymorphic_relationship(PictureResource._relationship(:imageable), 'products'))
     assert_hash_equals({alias: 'documents', join_type: :left}, join_manager.join_details_by_polymorphic_relationship(PictureResource._relationship(:imageable), 'documents'))

diff --git a/test/unit/resource/resource_test.rb b/test/unit/resource/resource_test.rb
@@ -13,7 +13,7 @@ class PostWithBadAfterSave < ActiveRecord::Base
   after_save :do_some_after_save_stuff
 
   def do_some_after_save_stuff
-    errors[:base] << 'Boom! Error added in after_save callback.'
+    errors.add(:base, 'Boom! Error added in after_save callback.')
     raise ActiveRecord::RecordInvalid.new(self)
   end
 end
@@ -23,7 +23,7 @@ class PostWithCustomValidationContext < ActiveRecord::Base
   validate :api_specific_check, on: :json_api_create
 
   def api_specific_check
-    errors[:base] << 'Record is invalid'
+    errors.add(:base, 'Record is invalid')
   end
 end