From 7e9fd4465f063394efe851f4d078cf38cd693557 Mon Sep 17 00:00:00 2001 From: Larry Gebhardt Date: Thu, 4 Feb 2021 15:00:13 -0500 Subject: [PATCH] Support Rails 6.1 * Setup github actions for CI (cherry picked from commit e89482775c278c7af6b40bb2d3cc8d1f478e7170) * 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 (cherry picked from commit 1322b5939767687cbc8a0942584aa36afb2bca83) --- .github/workflows/ruby.yml | 62 +++++++++++++++++++ .travis.yml | 31 ---------- lib/jsonapi/active_relation_resource.rb | 2 +- lib/jsonapi/request_parser.rb | 2 +- lib/jsonapi/resource_controller_metal.rb | 4 +- test/fixtures/active_record.rb | 6 +- test/integration/requests/request_test.rb | 8 +-- test/test_helper.rb | 10 +-- .../join_manager_test.rb | 33 ++++++++-- test/unit/resource/resource_test.rb | 4 +- 10 files changed, 107 insertions(+), 55 deletions(-) create mode 100644 .github/workflows/ruby.yml delete mode 100644 .travis.yml diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml new file mode 100644 index 000000000..1c2087bc8 --- /dev/null +++ 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 \ No newline at end of file diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 679787e7d..000000000 --- a/.travis.yml +++ /dev/null @@ -1,31 +0,0 @@ -language: ruby -sudo: false -services: - - postgresql -env: - - RAILS_VERSION=6.0.0 DATABASE_URL=postgres://postgres@localhost/jr_test - - RAILS_VERSION=6.0.0 - - RAILS_VERSION=5.2.3 DATABASE_URL=postgres://postgres@localhost/jr_test - - RAILS_VERSION=5.2.3 - - RAILS_VERSION=5.1.7 - - RAILS_VERSION=5.0.7.2 - - RAILS_VERSION=4.2.11 -rvm: - - 2.4.9 - - 2.5.7 - - 2.6.5 -matrix: - exclude: - - rvm: 2.6.5 - env: "RAILS_VERSION=4.2.11" - - rvm: 2.4.9 - env: "RAILS_VERSION=6.0.0" - - rvm: 2.4.9 - env: "RAILS_VERSION=6.0.0 DATABASE_URL=postgres://postgres@localhost/jr_test" - - rvm: 2.4.9 - env: "RAILS_VERSION=5.2.3 DATABASE_URL=postgres://postgres@localhost/jr_test" -before_install: - - gem install bundler --version 1.17.3 -before_script: - - sh -c "if [ '$DATABASE_URL' = 'postgres://postgres@localhost/jr_test' ]; then psql -c 'DROP DATABASE IF EXISTS jr_test;' -U postgres; fi" - - sh -c "if [ '$DATABASE_URL' = 'postgres://postgres@localhost/jr_test' ]; then psql -c 'CREATE DATABASE jr_test;' -U postgres; fi" \ No newline at end of file diff --git a/lib/jsonapi/active_relation_resource.rb b/lib/jsonapi/active_relation_resource.rb index e2611613f..cb60faabd 100644 --- 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 index b8a8a8de9..4d72e911e 100644 --- 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 index c950e4659..f6f82e246 100644 --- 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 index bdb718bbf..c1f3cc674 100644 --- 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 index e59cdd580..1863b5c7d 100644 --- 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 index 97e51fe7d..34506e99a 100644 --- 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 index 9fb13a82b..a1198bf28 100644 --- 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 index 0bb8d1aa7..84a5ed565 100644 --- 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