Remove code specifically for pre 5.1 rails versions (#1353)

lgebhardt · web-flow · commit 856f139a95ab · 2021-02-04T15:02:38.000-05:00
* Remove code specifically for pre 5.1 rails versions

* Update rails version supported
diff --git a/README.md b/README.md
@@ -8,7 +8,7 @@
 Like JSON:API itself, JR's design is focused on the resources served by an API. JR needs little more than a definition
 of your resources, including their attributes and relationships, to make your server compliant with JSON API.
 
-JR is designed to work with Rails 4.2+, and provides custom routes, controllers, and serializers. JR's resources may be
+JR is designed to work with Rails 5.1+, and provides custom routes, controllers, and serializers. JR's resources may be
 backed by ActiveRecord models or by custom objects.
 
 ## Documentation
diff --git a/jsonapi-resources.gemspec b/jsonapi-resources.gemspec
@@ -27,7 +27,7 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'pry'
   spec.add_development_dependency 'concurrent-ruby-ext'
   spec.add_development_dependency 'database_cleaner'
-  spec.add_dependency 'activerecord', '>= 4.1'
-  spec.add_dependency 'railties', '>= 4.1'
+  spec.add_dependency 'activerecord', '>= 5.1'
+  spec.add_dependency 'railties', '>= 5.1'
   spec.add_dependency 'concurrent-ruby'
 end
diff --git a/lib/jsonapi/active_relation/adapters/join_left_active_record_adapter.rb b/lib/jsonapi/active_relation/adapters/join_left_active_record_adapter.rb
@@ -2,7 +2,6 @@ module JSONAPI
   module ActiveRelation
     module Adapters
       module JoinLeftActiveRecordAdapter
-
         # Extends left_joins functionality to rails 4, and uses the same logic for rails 5.0.x and 5.1.x
         # The default left_joins logic of rails 5.2.x is used. This results in and extra join in some cases. For
         # example Post.joins(:comments).joins_left(comments: :author) will join the comments table twice,
diff --git a/lib/jsonapi/basic_resource.rb b/lib/jsonapi/basic_resource.rb
@@ -575,20 +575,12 @@ def attribute_to_model_field(attribute)
                        attr = @_attributes[attribute]
                        attr && attr[:delegate] ? attr[:delegate].to_sym : attribute
                      end
-        if Rails::VERSION::MAJOR >= 5
-          attribute_type = _model_class.attribute_types[field_name.to_s]
-        else
-          attribute_type = _model_class.column_types[field_name.to_s]
-        end
-        { name: field_name, type: attribute_type}
+
+        { name: field_name, type: _model_class.attribute_types[field_name.to_s]}
       end
 
       def cast_to_attribute_type(value, type)
-        if Rails::VERSION::MAJOR >= 5
-          return type.cast(value)
-        else
-          return type.type_cast_from_database(value)
-        end
+        type.cast(value)
       end
 
       def default_attribute_options
diff --git a/lib/jsonapi/mime_types.rb b/lib/jsonapi/mime_types.rb
@@ -7,16 +7,10 @@ module MimeTypes
     def self.install
       Mime::Type.register JSONAPI::MEDIA_TYPE, :api_json
 
-      # :nocov:
-      if Rails::VERSION::MAJOR >= 5
-        parsers = ActionDispatch::Request.parameter_parsers.merge(
-          Mime::Type.lookup(JSONAPI::MEDIA_TYPE).symbol => parser
-        )
-        ActionDispatch::Request.parameter_parsers = parsers
-      else
-        ActionDispatch::ParamsParser::DEFAULT_PARSERS[Mime::Type.lookup(JSONAPI::MEDIA_TYPE)] = parser
-      end
-      # :nocov:
+      parsers = ActionDispatch::Request.parameter_parsers.merge(
+        Mime::Type.lookup(JSONAPI::MEDIA_TYPE).symbol => parser
+      )
+      ActionDispatch::Request.parameter_parsers = parsers
     end
 
     def self.parser
diff --git a/test/fixtures/active_record.rb b/test/fixtures/active_record.rb
@@ -517,24 +517,10 @@ def destroy_callback
     case title
     when "can't destroy me", "can't destroy me either"
       errors.add(:base, "can't destroy me")
-
-      # :nocov:
-      if Rails::VERSION::MAJOR >= 5
-        throw(:abort)
-      else
-        return false
-      end
-      # :nocov:
+      throw(:abort)
     when "locked title"
       errors.add(:title, "is locked")
-
-      # :nocov:
-      if Rails::VERSION::MAJOR >= 5
-        throw(:abort)
-      else
-        return false
-      end
-      # :nocov:
+      throw(:abort)
     end
   end
 end
@@ -605,13 +591,7 @@ class Planet < ActiveRecord::Base
   def check_not_pluto
     # Pluto can't be a planet, so cancel the save
     if name.downcase == 'pluto'
-      # :nocov:
-      if Rails::VERSION::MAJOR >= 5
-        throw(:abort)
-      else
-        return false
-      end
-      # :nocov:
+      throw(:abort)
     end
   end
 end
diff --git a/test/test_helper.rb b/test/test_helper.rb
@@ -5,16 +5,15 @@
 # COVERAGE=true bundle exec rake test
 
 # To test on a specific rails version use this:
-# export RAILS_VERSION=4.2.6; bundle update rails; bundle exec rake test
-# export RAILS_VERSION=5.0.0; bundle update rails; bundle exec rake test
-# export RAILS_VERSION=5.1.0; bundle update rails; bundle exec rake test
-# export RAILS_VERSION=6.0.0.beta1; bundle update rails; bundle exec rake test
+# export RAILS_VERSION=5.2.4.4; bundle update; bundle exec rake test
+# export RAILS_VERSION=6.0.3.4; bundle update; bundle exec rake test
+# export RAILS_VERSION=6.1.1; bundle update; bundle exec rake test
 
-# We are no longer having Travis test Rails 4.1.x., but you can try it with:
-# export RAILS_VERSION=4.1.0; bundle update rails; bundle exec rake test
+# We are no longer having Travis test Rails 4.2.11., but you can try it with:
+# export RAILS_VERSION=4.2.11; bundle update rails; bundle exec rake test
 
 # To Switch rails versions and run a particular test order:
-# export RAILS_VERSION=4.2.6; bundle update rails; bundle exec rake TESTOPTS="--seed=39333" test
+# export RAILS_VERSION=6.1.1; bundle update; bundle exec rake TESTOPTS="--seed=39333" test
 
 if ENV['COVERAGE']
   SimpleCov.start do
@@ -60,13 +59,11 @@ class TestApp < Rails::Application
   config.active_record.schema_format = :none
   config.active_support.test_order = :random
 
-  if Rails::VERSION::MAJOR >= 5
-    config.active_support.halt_callback_chains_on_return_false = false
-    config.active_record.time_zone_aware_types = [:time, :datetime]
-    config.active_record.belongs_to_required_by_default = false
-    if Rails::VERSION::MINOR >= 2
-      config.active_record.sqlite3.represent_boolean_as_integer = true
-    end
+  config.active_support.halt_callback_chains_on_return_false = false
+  config.active_record.time_zone_aware_types = [:time, :datetime]
+  config.active_record.belongs_to_required_by_default = false
+  unless Rails::VERSION::MAJOR == 5 && Rails::VERSION::MINOR < 2
+    config.active_record.sqlite3.represent_boolean_as_integer = true
   end
 end
 
@@ -86,116 +83,80 @@ class Engine < ::Rails::Engine
 end
 
 # 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)
-      @request.delete_header 'RAW_POST_DATA'
-      super action, **args
-    end
-  end
-
-  class ActionController::TestCase
-    prepend ClearRawPostHeader
+module ClearRawPostHeader
+  def process(action, **args)
+    @request.delete_header 'RAW_POST_DATA'
+    super action, **args
   end
 end
 
-# Tests are now using the rails 5 format for the http methods. So for rails 4 we will simply convert them back
-# in a standard way.
-if Rails::VERSION::MAJOR < 5
-  module Rails4ActionControllerProcess
-    def process(*args)
-      if args[2] && args[2][:params]
-        args[2] = args[2][:params]
-      end
-      super
-    end
-  end
-  class ActionController::TestCase
-    prepend Rails4ActionControllerProcess
-  end
-
-  module ActionDispatch
-    module Integration #:nodoc:
-      module Rails4IntegrationProcess
-        def process(method, path, parameters = nil, headers_or_env = nil)
-          params = parameters.nil? ? nil : parameters[:params]
-          headers = parameters.nil? ? nil : parameters[:headers]
-          super method, path, params, headers
-        end
-      end
-
-      class Session
-        prepend Rails4IntegrationProcess
-      end
-    end
-  end
+class ActionController::TestCase
+  prepend ClearRawPostHeader
 end
 
 # Patch to allow :api_json mime type to be treated as JSON
 # Otherwise it is run through `to_query` and empty arrays are dropped.
-if Rails::VERSION::MAJOR >= 5
-  module ActionController
-    class TestRequest < ActionDispatch::TestRequest
-      def assign_parameters(routes, controller_path, action, parameters, generated_path, query_string_keys)
-        non_path_parameters = {}
-        path_parameters = {}
-
-        parameters.each do |key, value|
-          if query_string_keys.include?(key)
-            non_path_parameters[key] = value
+module ActionController
+  class TestRequest < ActionDispatch::TestRequest
+    def assign_parameters(routes, controller_path, action, parameters, generated_path, query_string_keys)
+      non_path_parameters = {}
+      path_parameters = {}
+
+      parameters.each do |key, value|
+        if query_string_keys.include?(key)
+          non_path_parameters[key] = value
+        else
+          if value.is_a?(Array)
+            value = value.map(&:to_param)
           else
-            if value.is_a?(Array)
-              value = value.map(&:to_param)
-            else
-              value = value.to_param
-            end
-
-            path_parameters[key] = value
+            value = value.to_param
           end
+
+          path_parameters[key] = value
         end
+      end
 
-        if get?
-          if self.query_string.blank?
-            self.query_string = non_path_parameters.to_query
-          end
+      if get?
+        if self.query_string.blank?
+          self.query_string = non_path_parameters.to_query
+        end
+      else
+        if ENCODER.should_multipart?(non_path_parameters)
+          self.content_type = ENCODER.content_type
+          data = ENCODER.build_multipart non_path_parameters
         else
-          if ENCODER.should_multipart?(non_path_parameters)
-            self.content_type = ENCODER.content_type
-            data = ENCODER.build_multipart non_path_parameters
-          else
-            fetch_header('CONTENT_TYPE') do |k|
-              set_header k, 'application/x-www-form-urlencoded'
-            end
-
-            # parser = ActionDispatch::Http::Parameters::DEFAULT_PARSERS[Mime::Type.lookup(fetch_header('CONTENT_TYPE'))]
-
-            case content_mime_type.to_sym
-              when nil
-                raise "Unknown Content-Type: #{content_type}"
-              when :json, :api_json
-                data = ActiveSupport::JSON.encode(non_path_parameters)
-              when :xml
-                data = non_path_parameters.to_xml
-              when :url_encoded_form
-                data = non_path_parameters.to_query
-              else
-                @custom_param_parsers[content_mime_type] = ->(_) { non_path_parameters }
-                data = non_path_parameters.to_query
-            end
+          fetch_header('CONTENT_TYPE') do |k|
+            set_header k, 'application/x-www-form-urlencoded'
           end
 
-          set_header 'CONTENT_LENGTH', data.length.to_s
-          set_header 'rack.input', StringIO.new(data)
+          # parser = ActionDispatch::Http::Parameters::DEFAULT_PARSERS[Mime::Type.lookup(fetch_header('CONTENT_TYPE'))]
+
+          case content_mime_type.to_sym
+            when nil
+              raise "Unknown Content-Type: #{content_type}"
+            when :json, :api_json
+              data = ActiveSupport::JSON.encode(non_path_parameters)
+            when :xml
+              data = non_path_parameters.to_xml
+            when :url_encoded_form
+              data = non_path_parameters.to_query
+            else
+              @custom_param_parsers[content_mime_type] = ->(_) { non_path_parameters }
+              data = non_path_parameters.to_query
+          end
         end
 
-        fetch_header("PATH_INFO") do |k|
-          set_header k, generated_path
-        end
-        path_parameters[:controller] = controller_path
-        path_parameters[:action] = action
+        set_header 'CONTENT_LENGTH', data.length.to_s
+        set_header 'rack.input', StringIO.new(data)
+      end
 
-        self.path_parameters = path_parameters
+      fetch_header("PATH_INFO") do |k|
+        set_header k, generated_path
       end
+      path_parameters[:controller] = controller_path
+      path_parameters[:action] = action
+
+      self.path_parameters = path_parameters
     end
   end
 end
diff --git a/test/unit/active_relation_resource_finder/active_record_adapter_test.rb b/test/unit/active_relation_resource_finder/active_record_adapter_test.rb
@@ -11,19 +11,8 @@ def test_joins_left
 
   def test_joins_left_through_inner
     sql = Post.joins(:comments).joins_left(comments: :author).to_sql
-
-    # Note this joins_left reverts to left_joins on rails 5.2 and later
-    # This behaves slightly differently in that the base join table is joined twice using left the second time (in this test).
-    # This should produce the same result set, but will be slightly less efficient on the database
-    if Rails::VERSION::MAJOR >= 5 && ActiveRecord::VERSION::MINOR >= 2
-      assert_equal 'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" '\
-                   'LEFT OUTER JOIN "comments" "comments_posts" ON "comments_posts"."post_id" = "posts"."id" '\
-                   'LEFT OUTER JOIN "people" ON "people"."id" = "comments_posts"."author_id"',
-                   sql
-    else
-      assert_equal 'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" ' \
-                   'LEFT OUTER JOIN "people" ON "people"."id" = "comments"."author_id"',
-                   sql
-    end
+    assert_equal 'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" ' \
+                 'LEFT OUTER JOIN "people" ON "people"."id" = "comments"."author_id"',
+                 sql
   end
 end
diff --git a/test/unit/resource/resource_test.rb b/test/unit/resource/resource_test.rb
@@ -178,14 +178,9 @@ def test_inherited_calls_superclass
   end
 
   def test_nil_model_class
-    # ToDo:Figure out why this test does not work on Rails 4.0
-    # :nocov:
-    if (Rails::VERSION::MAJOR >= 4 && Rails::VERSION::MINOR >= 1) || (Rails::VERSION::MAJOR >= 5)
-      assert_output nil, "[MODEL NOT FOUND] Model could not be found for NoMatchResource. If this is a base Resource declare it as abstract.\n" do
-        assert_nil NoMatchResource._model_class
-      end
+    assert_output nil, "[MODEL NOT FOUND] Model could not be found for NoMatchResource. If this is a base Resource declare it as abstract.\n" do
+      assert_nil NoMatchResource._model_class
     end
-    # :nocov:
   end
 
   def test_nil_abstract_model_class
