Merge pull request #29220 from kamipo/consolidate_database_specific_json_types

Consolidate database specific JSON types to `Type::Json`

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -543,7 +543,7 @@ module ActiveRecord
           m.register_type %r(longblob)i,   Type::Binary.new(limit: 2**32 - 1)
           m.register_type %r(^float)i,     Type::Float.new(limit: 24)
           m.register_type %r(^double)i,    Type::Float.new(limit: 53)
-          m.register_type %r(^json)i,      MysqlJson.new
+          m.register_type %r(^json)i,      Type::Json.new
 
           register_integer_type m, %r(^bigint)i,    limit: 8
           register_integer_type m, %r(^int)i,       limit: 4
@@ -837,7 +837,7 @@ module ActiveRecord
           end
         end
 
-        class MysqlJson < Type::Internal::AbstractJson # :nodoc:
+        class MysqlJson < Type::Json # :nodoc:
         end
 
         class MysqlString < Type::String # :nodoc:
@@ -860,7 +860,6 @@ module ActiveRecord
             end
         end
 
-        ActiveRecord::Type.register(:json, MysqlJson, adapter: :mysql2)
         ActiveRecord::Type.register(:string, MysqlString, adapter: :mysql2)
         ActiveRecord::Type.register(:unsigned_integer, Type::UnsignedInteger, adapter: :mysql2)
     end

activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb

@@ -2,7 +2,7 @@ module ActiveRecord
   module ConnectionAdapters
     module PostgreSQL
       module OID # :nodoc:
-        class Json < Type::Internal::AbstractJson
+        class Json < Type::Json # :nodoc:
         end
       end
     end

activerecord/lib/active_record/connection_adapters/postgresql/oid/jsonb.rb

@@ -2,7 +2,7 @@ module ActiveRecord
   module ConnectionAdapters
     module PostgreSQL
       module OID # :nodoc:
-        class Jsonb < Json # :nodoc:
+        class Jsonb < Type::Json # :nodoc:
           def type
             :jsonb
           end

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

@@ -458,7 +458,7 @@ module ActiveRecord
           m.register_type "bytea", OID::Bytea.new
           m.register_type "point", OID::Point.new
           m.register_type "hstore", OID::Hstore.new
-          m.register_type "json", OID::Json.new
+          m.register_type "json", Type::Json.new
           m.register_type "jsonb", OID::Jsonb.new
           m.register_type "cidr", OID::Cidr.new
           m.register_type "inet", OID::Inet.new
@@ -843,7 +843,6 @@ module ActiveRecord
         ActiveRecord::Type.register(:enum, OID::Enum, adapter: :postgresql)
         ActiveRecord::Type.register(:hstore, OID::Hstore, adapter: :postgresql)
         ActiveRecord::Type.register(:inet, OID::Inet, adapter: :postgresql)
-        ActiveRecord::Type.register(:json, OID::Json, adapter: :postgresql)
         ActiveRecord::Type.register(:jsonb, OID::Jsonb, adapter: :postgresql)
         ActiveRecord::Type.register(:money, OID::Money, adapter: :postgresql)
         ActiveRecord::Type.register(:point, OID::Point, adapter: :postgresql)

activerecord/lib/active_record/type.rb

@@ -1,11 +1,11 @@
 require "active_model/type"
 
-require "active_record/type/internal/abstract_json"
 require "active_record/type/internal/timezone"
 
 require "active_record/type/date"
 require "active_record/type/date_time"
 require "active_record/type/decimal_without_scale"
+require "active_record/type/json"
 require "active_record/type/time"
 require "active_record/type/text"
 require "active_record/type/unsigned_integer"
@@ -69,6 +69,7 @@ module ActiveRecord
     register(:decimal, Type::Decimal, override: false)
     register(:float, Type::Float, override: false)
     register(:integer, Type::Integer, override: false)
+    register(:json, Type::Json, override: false)
     register(:string, Type::String, override: false)
     register(:text, Type::Text, override: false)
     register(:time, Type::Time, override: false)

activerecord/lib/active_record/type/internal/abstract_json.rb

@@ -1,37 +0,0 @@
-module ActiveRecord
-  module Type
-    module Internal # :nodoc:
-      class AbstractJson < ActiveModel::Type::Value # :nodoc:
-        include ActiveModel::Type::Helpers::Mutable
-
-        def type
-          :json
-        end
-
-        def deserialize(value)
-          if value.is_a?(::String)
-            ::ActiveSupport::JSON.decode(value) rescue nil
-          else
-            value
-          end
-        end
-
-        def serialize(value)
-          if value.nil?
-            nil
-          else
-            ::ActiveSupport::JSON.encode(value)
-          end
-        end
-
-        def changed_in_place?(raw_old_value, new_value)
-          deserialize(raw_old_value) != new_value
-        end
-
-        def accessor
-          ActiveRecord::Store::StringKeyedHashAccessor
-        end
-      end
-    end
-  end
-end

activerecord/lib/active_record/type/json.rb

@@ -0,0 +1,28 @@
+module ActiveRecord
+  module Type
+    class Json < ActiveModel::Type::Value
+      include ActiveModel::Type::Helpers::Mutable
+
+      def type
+        :json
+      end
+
+      def deserialize(value)
+        return value unless value.is_a?(::String)
+        ActiveSupport::JSON.decode(value) rescue nil
+      end
+
+      def serialize(value)
+        ActiveSupport::JSON.encode(value) unless value.nil?
+      end
+
+      def changed_in_place?(raw_old_value, new_value)
+        deserialize(raw_old_value) != new_value
+      end
+
+      def accessor
+        ActiveRecord::Store::StringKeyedHashAccessor
+      end
+    end
+  end
+end

activerecord/test/cases/adapters/mysql2/json_test.rb

@@ -7,20 +7,13 @@ if ActiveRecord::Base.connection.supports_json?
     self.use_transactional_tests = false
 
     def setup
-      @connection = ActiveRecord::Base.connection
-      begin
-        @connection.create_table("json_data_type") do |t|
-          t.json "payload"
-          t.json "settings"
-        end
+      super
+      @connection.create_table("json_data_type") do |t|
+        t.json "payload"
+        t.json "settings"
       end
     end
 
-    def teardown
-      @connection.drop_table :json_data_type, if_exists: true
-      JsonDataType.reset_column_information
-    end
-
     private
       def column_type
         :json

activerecord/test/cases/adapters/postgresql/json_test.rb

@@ -5,35 +5,26 @@ module PostgresqlJSONSharedTestCases
   include JSONSharedTestCases
 
   def setup
-    @connection = ActiveRecord::Base.connection
-    begin
-      @connection.create_table("json_data_type") do |t|
-        t.public_send column_type, "payload", default: {} # t.json 'payload', default: {}
-        t.public_send column_type, "settings"             # t.json 'settings'
-        t.public_send column_type, "objects", array: true # t.json 'objects', array: true
-      end
-    rescue ActiveRecord::StatementInvalid
-      skip "do not test on PostgreSQL without #{column_type} type."
+    super
+    @connection.create_table("json_data_type") do |t|
+      t.public_send column_type, "payload", default: {} # t.json 'payload', default: {}
+      t.public_send column_type, "settings"             # t.json 'settings'
+      t.public_send column_type, "objects", array: true # t.json 'objects', array: true
     end
-  end
-
-  def teardown
-    @connection.drop_table :json_data_type, if_exists: true
-    JsonDataType.reset_column_information
+  rescue ActiveRecord::StatementInvalid
+    skip "do not test on PostgreSQL without #{column_type} type."
   end
 
   def test_default
     @connection.add_column "json_data_type", "permissions", column_type, default: { "users": "read", "posts": ["read", "write"] }
-    JsonDataType.reset_column_information
+    klass.reset_column_information
 
     assert_equal({ "users" => "read", "posts" => ["read", "write"] }, JsonDataType.column_defaults["permissions"])
     assert_equal({ "users" => "read", "posts" => ["read", "write"] }, JsonDataType.new.permissions)
-  ensure
-    JsonDataType.reset_column_information
   end
 
   def test_deserialize_with_array
-    x = JsonDataType.new(objects: ["foo" => "bar"])
+    x = klass.new(objects: ["foo" => "bar"])
     assert_equal ["foo" => "bar"], x.objects
     x.save!
     assert_equal ["foo" => "bar"], x.objects

activerecord/test/cases/json_attribute_test.rb

@@ -0,0 +1,33 @@
+require "cases/helper"
+require "cases/json_shared_test_cases"
+
+class JsonAttributeTest < ActiveRecord::TestCase
+  include JSONSharedTestCases
+  self.use_transactional_tests = false
+
+  class JsonDataTypeOnText < ActiveRecord::Base
+    self.table_name = "json_data_type"
+
+    attribute :payload,  :json
+    attribute :settings, :json
+
+    store_accessor :settings, :resolution
+  end
+
+  def setup
+    super
+    @connection.create_table("json_data_type") do |t|
+      t.text "payload"
+      t.text "settings"
+    end
+  end
+
+  private
+    def column_type
+      :text
+    end
+
+    def klass
+      JsonDataTypeOnText
+    end
+end

activerecord/test/cases/json_shared_test_cases.rb

@@ -9,12 +9,21 @@ module JSONSharedTestCases
     store_accessor :settings, :resolution
   end
 
+  def setup
+    @connection = ActiveRecord::Base.connection
+  end
+
+  def teardown
+    @connection.drop_table :json_data_type, if_exists: true
+    klass.reset_column_information
+  end
+
   def test_column
-    column = JsonDataType.columns_hash["payload"]
+    column = klass.columns_hash["payload"]
     assert_equal column_type, column.type
     assert_equal column_type.to_s, column.sql_type
 
-    type = JsonDataType.type_for_attribute("payload")
+    type = klass.type_for_attribute("payload")
     assert_not type.binary?
   end
 
@@ -22,8 +31,8 @@ module JSONSharedTestCases
     @connection.change_table("json_data_type") do |t|
       t.public_send column_type, "users"
     end
-    JsonDataType.reset_column_information
-    column = JsonDataType.columns_hash["users"]
+    klass.reset_column_information
+    column = klass.columns_hash["users"]
     assert_equal column_type, column.type
     assert_equal column_type.to_s, column.sql_type
   end
@@ -34,7 +43,7 @@ module JSONSharedTestCases
   end
 
   def test_cast_value_on_write
-    x = JsonDataType.new(payload: { "string" => "foo", :symbol => :bar })
+    x = klass.new(payload: { "string" => "foo", :symbol => :bar })
     assert_equal({ "string" => "foo", :symbol => :bar }, x.payload_before_type_cast)
     assert_equal({ "string" => "foo", "symbol" => "bar" }, x.payload)
     x.save!
@@ -42,7 +51,7 @@ module JSONSharedTestCases
   end
 
   def test_type_cast_json
-    type = JsonDataType.type_for_attribute("payload")
+    type = klass.type_for_attribute("payload")
 
     data = '{"a_key":"a_value"}'
     hash = type.deserialize(data)
@@ -56,75 +65,75 @@ module JSONSharedTestCases
 
   def test_rewrite
     @connection.execute(%q|insert into json_data_type (payload) VALUES ('{"k":"v"}')|)
-    x = JsonDataType.first
+    x = klass.first
     x.payload = { '"a\'' => "b" }
     assert x.save!
   end
 
   def test_select
     @connection.execute(%q|insert into json_data_type (payload) VALUES ('{"k":"v"}')|)
-    x = JsonDataType.first
+    x = klass.first
     assert_equal({ "k" => "v" }, x.payload)
   end
 
   def test_select_multikey
     @connection.execute(%q|insert into json_data_type (payload) VALUES ('{"k1":"v1", "k2":"v2", "k3":[1,2,3]}')|)
-    x = JsonDataType.first
+    x = klass.first
     assert_equal({ "k1" => "v1", "k2" => "v2", "k3" => [1, 2, 3] }, x.payload)
   end
 
   def test_null_json
     @connection.execute("insert into json_data_type (payload) VALUES(null)")
-    x = JsonDataType.first
+    x = klass.first
     assert_nil(x.payload)
   end
 
   def test_select_nil_json_after_create
-    json = JsonDataType.create!(payload: nil)
-    x = JsonDataType.where(payload: nil).first
+    json = klass.create!(payload: nil)
+    x = klass.where(payload: nil).first
     assert_equal(json, x)
   end
 
   def test_select_nil_json_after_update
-    json = JsonDataType.create!(payload: "foo")
-    x = JsonDataType.where(payload: nil).first
+    json = klass.create!(payload: "foo")
+    x = klass.where(payload: nil).first
     assert_nil(x)
 
     json.update_attributes(payload: nil)
-    x = JsonDataType.where(payload: nil).first
+    x = klass.where(payload: nil).first
     assert_equal(json.reload, x)
   end
 
   def test_select_array_json_value
     @connection.execute(%q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')|)
-    x = JsonDataType.first
+    x = klass.first
     assert_equal(["v0", { "k1" => "v1" }], x.payload)
   end
 
   def test_rewrite_array_json_value
     @connection.execute(%q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')|)
-    x = JsonDataType.first
+    x = klass.first
     x.payload = ["v1", { "k2" => "v2" }, "v3"]
     assert x.save!
   end
 
   def test_with_store_accessors
-    x = JsonDataType.new(resolution: "320×480")
+    x = klass.new(resolution: "320×480")
     assert_equal "320×480", x.resolution
 
     x.save!
-    x = JsonDataType.first
+    x = klass.first
     assert_equal "320×480", x.resolution
 
     x.resolution = "640×1136"
     x.save!
 
-    x = JsonDataType.first
+    x = klass.first
     assert_equal "640×1136", x.resolution
   end
 
   def test_duplication_with_store_accessors
-    x = JsonDataType.new(resolution: "320×480")
+    x = klass.new(resolution: "320×480")
     assert_equal "320×480", x.resolution
 
     y = x.dup
@@ -132,7 +141,7 @@ module JSONSharedTestCases
   end
 
   def test_yaml_round_trip_with_store_accessors
-    x = JsonDataType.new(resolution: "320×480")
+    x = klass.new(resolution: "320×480")
     assert_equal "320×480", x.resolution
 
     y = YAML.load(YAML.dump(x))
@@ -140,7 +149,7 @@ module JSONSharedTestCases
   end
 
   def test_changes_in_place
-    json = JsonDataType.new
+    json = klass.new
     assert_not json.changed?
 
     json.payload = { "one" => "two" }
@@ -162,7 +171,7 @@ module JSONSharedTestCases
 
   def test_changes_in_place_with_ruby_object
     time = Time.now.utc
-    json = JsonDataType.create!(payload: time)
+    json = klass.create!(payload: time)
 
     json.reload
     assert_not json.changed?
@@ -172,17 +181,22 @@ module JSONSharedTestCases
   end
 
   def test_assigning_string_literal
-    json = JsonDataType.create!(payload: "foo")
+    json = klass.create!(payload: "foo")
     assert_equal "foo", json.payload
   end
 
   def test_assigning_number
-    json = JsonDataType.create!(payload: 1.234)
+    json = klass.create!(payload: 1.234)
     assert_equal 1.234, json.payload
   end
 
   def test_assigning_boolean
-    json = JsonDataType.create!(payload: true)
+    json = klass.create!(payload: true)
     assert_equal true, json.payload
   end
+
+  private
+    def klass
+      JsonDataType
+    end
 end