Merge pull request #16072 from sgrif/sg-xml-quoting

Don't rely on the sql type to quote XML columns in PG

5 files changed, 56 insertions, 5 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb
index 33a98b4fcb..d05ce61330 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb
@@ -21,6 +21,7 @@ require 'active_record/connection_adapters/postgresql/oid/specialized_string'
 require 'active_record/connection_adapters/postgresql/oid/time'
 require 'active_record/connection_adapters/postgresql/oid/uuid'
 require 'active_record/connection_adapters/postgresql/oid/vector'
+require 'active_record/connection_adapters/postgresql/oid/xml'
 
 require 'active_record/connection_adapters/postgresql/oid/type_map_initializer'
 
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/xml.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/xml.rb
new file mode 100644
index 0000000000..7323f12763
--- /dev/null
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/xml.rb
@@ -0,0 +1,32 @@
+module ActiveRecord
+  module ConnectionAdapters
+    module PostgreSQL
+      module OID # :nodoc:
+        class Xml < Type::String # :nodoc:
+          def type
+            :xml
+          end
+
+          def text?
+            false
+          end
+
+          def type_cast_for_database(value)
+            return unless value
+            Data.new(super)
+          end
+
+          class Data # :nodoc:
+            def initialize(value)
+              @value = value
+            end
+
+            def to_s
+              @value
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb
index be598f997f..d2e12417a6 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb
@@ -29,7 +29,6 @@ module ActiveRecord
             end
           when String
             case sql_type
-            when 'xml'   then "xml '#{quote_string(value)}'"
             when /^bit/
               case value
               when /\A[01]*\Z/      then "B'#{value}'" # Bit-string notation
@@ -111,19 +110,25 @@ module ActiveRecord
         private
 
         def _quote(value)
-          if value.is_a?(Type::Binary::Data)
+          case value
+          when Type::Binary::Data
             "'#{escape_bytea(value.to_s)}'"
+          when OID::Xml::Data
+            "xml '#{quote_string(value.to_s)}'"
           else
             super
           end
         end
 
         def _type_cast(value)
-          if value.is_a?(Type::Binary::Data)
+          case value
+          when Type::Binary::Data
             # Return a bind param hash with format as binary.
             # See http://deveiate.org/code/pg/PGconn.html#method-i-exec_prepared-doc
             # for more information
             { value: value.to_s, format: 1 }
+          when OID::Xml::Data
+            value.to_s
           else
             super
           end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 8e31e165b1..f660fc41cf 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -454,7 +454,7 @@ module ActiveRecord
           m.register_type 'cidr', OID::Cidr.new
           m.register_type 'inet', OID::Inet.new
           m.register_type 'uuid', OID::Uuid.new
-          m.register_type 'xml', OID::SpecializedString.new(:xml)
+          m.register_type 'xml', OID::Xml.new
           m.register_type 'tsvector', OID::SpecializedString.new(:tsvector)
           m.register_type 'macaddr', OID::SpecializedString.new(:macaddr)
           m.register_type 'citext', OID::SpecializedString.new(:citext)
diff --git a/activerecord/test/cases/adapters/postgresql/xml_test.rb b/activerecord/test/cases/adapters/postgresql/xml_test.rb
index 48c6eeb62c..4165dd5ac9 100644
--- a/activerecord/test/cases/adapters/postgresql/xml_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/xml_test.rb
@@ -11,7 +11,7 @@ class PostgresqlXMLTest < ActiveRecord::TestCase
     begin
       @connection.transaction do
         @connection.create_table('xml_data_type') do |t|
-          t.xml 'payload', default: {}
+          t.xml 'payload'
         end
       end
     rescue ActiveRecord::StatementInvalid
@@ -32,4 +32,17 @@ class PostgresqlXMLTest < ActiveRecord::TestCase
     @connection.execute %q|insert into xml_data_type (payload) VALUES(null)|
     assert_nil XmlDataType.first.payload
   end
+
+  def test_round_trip
+    data = XmlDataType.new(payload: "<foo>bar</foo>")
+    assert_equal "<foo>bar</foo>", data.payload
+    data.save!
+    assert_equal "<foo>bar</foo>", data.reload.payload
+  end
+
+  def test_update_all
+    data = XmlDataType.create!
+    XmlDataType.update_all(payload: "<bar>baz</bar>")
+    assert_equal "<bar>baz</bar>", data.reload.payload
+  end
 end