Merge pull request #25395 from yawboakye/use_gen_random_uuid_from_pgcrypto_extension

For PostgreSQL >= 9.4 use `gen_random_uuid()`

6 files changed, 86 insertions, 22 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index d0cc808133..0b9a2f9943 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   For PostgreSQL >= 9.4 use `pgcrypto`'s `gen_random_uuid()` instead of
+    `uuid-ossp`'s UUID generation function.
+
+    *Yuji Yaginuma*, *Yaw Boakye*
+
 *   Introduce `Model#reload_<association>` to bring back the behavior
     of `Article.category(true)` where `category` is a singular
     association.
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb
index a11dbe7dce..5d689c2dc3 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb
@@ -11,11 +11,12 @@ module ActiveRecord
         #     t.timestamps
         #   end
         #
-        # By default, this will use the +uuid_generate_v4()+ function from the
-        # +uuid-ossp+ extension, which MUST be enabled on your database. To enable
-        # the +uuid-ossp+ extension, you can use the +enable_extension+ method in your
-        # migrations. To use a UUID primary key without +uuid-ossp+ enabled, you can
-        # set the +:default+ option to +nil+:
+        # By default, this will use the +gen_random_uuid()+ function from the
+        # +pgcrypto+ extension (only PostgreSQL >= 9.4), or +uuid_generate_v4()+
+        # function from the +uuid-ossp+ extension. To enable the appropriate
+        # extension, which is a requirement, you can use the +enable_extension+
+        # method in your migrations. To use a UUID primary key without any of
+        # of extensions, you can set the +:default+ option to +nil+:
         #
         #   create_table :stuffs, id: false do |t|
         #     t.primary_key :id, :uuid, default: nil
@@ -23,15 +24,15 @@ module ActiveRecord
         #     t.timestamps
         #   end
         #
-        # You may also pass a different UUID generation function from +uuid-ossp+
-        # or another library.
+        # You may also pass a custom stored procedure that returns a UUID or use a
+        # different UUID generation function from another library.
         #
         # Note that setting the UUID primary key default value to +nil+ will
         # require you to assure that you always provide a UUID value before saving
         # a record (as primary keys cannot be +nil+). This might be done via the
         # +SecureRandom.uuid+ method and a +before_save+ callback, for instance.
         def primary_key(name, type = :primary_key, **options)
-          options[:default] = options.fetch(:default, "uuid_generate_v4()") if type == :uuid
+          options[:default] = options.fetch(:default, "gen_random_uuid()") if type == :uuid
           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 710b5cd887..140ad4827a 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -315,6 +315,10 @@ module ActiveRecord
         postgresql_version >= 90300
       end
 
+      def supports_pgcrypto_uuid?
+        postgresql_version >= 90400
+      end
+
       def get_advisory_lock(lock_id) # :nodoc:
         unless lock_id.is_a?(Integer) && lock_id.bit_length <= 63
           raise(ArgumentError, "Postgres requires advisory lock ids to be a signed 64 bit integer")
diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb
index 04e538baa5..ae45ac7157 100644
--- a/activerecord/lib/active_record/migration/compatibility.rb
+++ b/activerecord/lib/active_record/migration/compatibility.rb
@@ -103,6 +103,14 @@ module ActiveRecord
       end
 
       class V5_0 < V5_1
+        def create_table(table_name, options = {})
+          if ActiveRecord::Base.connection.adapter_name == "PostgreSQL"
+            if options[:id] == :uuid && !options[:default]
+              options[:default] = "uuid_generate_v4()"
+            end
+          end
+          super
+        end
       end
 
       class V4_2 < V5_0
diff --git a/activerecord/test/cases/adapters/postgresql/uuid_test.rb b/activerecord/test/cases/adapters/postgresql/uuid_test.rb
index 9a59691737..ab0815631f 100644
--- a/activerecord/test/cases/adapters/postgresql/uuid_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/uuid_test.rb
@@ -9,6 +9,10 @@ module PostgresqlUUIDHelper
   def drop_table(name)
     connection.drop_table name, if_exists: true
   end
+
+  def uuid_function
+    connection.supports_pgcrypto_uuid? ? "gen_random_uuid()" : "uuid_generate_v4()"
+  end
 end
 
 class PostgresqlUUIDTest < ActiveRecord::PostgreSQLTestCase
@@ -21,6 +25,7 @@ class PostgresqlUUIDTest < ActiveRecord::PostgreSQLTestCase
 
   setup do
     enable_extension!("uuid-ossp", connection)
+    enable_extension!("pgcrypto",  connection) if connection.supports_pgcrypto_uuid?
 
     connection.create_table "uuid_data_type" do |t|
       t.uuid "guid"
@@ -31,19 +36,27 @@ class PostgresqlUUIDTest < ActiveRecord::PostgreSQLTestCase
     drop_table "uuid_data_type"
   end
 
-  def test_change_column_default
-    @connection.add_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v1()"
-    UUIDType.reset_column_information
-    column = UUIDType.columns_hash["thingy"]
-    assert_equal "uuid_generate_v1()", column.default_function
-
-    @connection.change_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v4()"
-
-    UUIDType.reset_column_information
-    column = UUIDType.columns_hash["thingy"]
-    assert_equal "uuid_generate_v4()", column.default_function
-  ensure
-    UUIDType.reset_column_information
+  if ActiveRecord::Base.connection.supports_pgcrypto_uuid?
+    def test_uuid_column_default
+      connection.add_column :uuid_data_type, :thingy, :uuid, null: false, default: "gen_random_uuid()"
+      UUIDType.reset_column_information
+      column = UUIDType.columns_hash["thingy"]
+      assert_equal "gen_random_uuid()", column.default_function
+    end
+  else
+    def test_change_column_default
+      connection.add_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v1()"
+      UUIDType.reset_column_information
+      column = UUIDType.columns_hash["thingy"]
+      assert_equal "uuid_generate_v1()", column.default_function
+
+      connection.change_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v4()"
+      UUIDType.reset_column_information
+      column = UUIDType.columns_hash["thingy"]
+      assert_equal "uuid_generate_v4()", column.default_function
+    ensure
+      UUIDType.reset_column_information
+    end
   end
 
   def test_data_type_of_uuid_types
@@ -155,7 +168,7 @@ class PostgresqlUUIDGenerationTest < ActiveRecord::PostgreSQLTestCase
     # to test dumping tables which columns have defaults with custom functions
     connection.execute <<-SQL
     CREATE OR REPLACE FUNCTION my_uuid_generator() RETURNS uuid
-    AS $$ SELECT * FROM uuid_generate_v4() $$
+    AS $$ SELECT * FROM #{uuid_function} $$
     LANGUAGE SQL VOLATILE;
     SQL
 
@@ -164,11 +177,16 @@ class PostgresqlUUIDGenerationTest < ActiveRecord::PostgreSQLTestCase
       t.string "name"
       t.uuid "other_uuid_2", default: "my_uuid_generator()"
     end
+
+    connection.create_table("pg_uuids_3", id: :uuid) do |t|
+      t.string "name"
+    end
   end
 
   teardown do
     drop_table "pg_uuids"
     drop_table "pg_uuids_2"
+    drop_table "pg_uuids_3"
     connection.execute "DROP FUNCTION IF EXISTS my_uuid_generator();"
   end
 
@@ -206,6 +224,33 @@ class PostgresqlUUIDGenerationTest < ActiveRecord::PostgreSQLTestCase
       assert_match(/\bcreate_table "pg_uuids_2", id: :uuid, default: -> { "my_uuid_generator\(\)" }/, schema)
       assert_match(/t\.uuid "other_uuid_2", default: -> { "my_uuid_generator\(\)" }/, schema)
     end
+
+    def test_schema_dumper_for_uuid_primary_key_default
+      schema = dump_table_schema "pg_uuids_3"
+      if connection.supports_pgcrypto_uuid?
+        assert_match(/\bcreate_table "pg_uuids_3", id: :uuid, default: -> { "gen_random_uuid\(\)" }/, schema)
+      else
+        assert_match(/\bcreate_table "pg_uuids_3", id: :uuid, default: -> { "uuid_generate_v4\(\)" }/, schema)
+      end
+    end
+
+    if ActiveRecord::Base.connection.supports_pgcrypto_uuid?
+      def test_schema_dumper_for_uuid_primary_key_default_in_legacy_migration
+        migration = Class.new(ActiveRecord::Migration[4.2]) do
+          def version; 101 end
+          def migrate(x)
+            create_table("pg_uuids_4", id: :uuid)
+          end
+        end.new
+        ActiveRecord::Migrator.new(:up, [migration]).migrate
+
+        schema = dump_table_schema "pg_uuids_4"
+        assert_match(/\bcreate_table "pg_uuids_4", id: :uuid, default: -> { "uuid_generate_v4\(\)" }/, schema)
+      ensure
+        drop_table "pg_uuids_4"
+      end
+    else
+    end
   end
 end
 
diff --git a/activerecord/test/schema/postgresql_specific_schema.rb b/activerecord/test/schema/postgresql_specific_schema.rb
index f00b858ea6..15ba2d67ab 100644
--- a/activerecord/test/schema/postgresql_specific_schema.rb
+++ b/activerecord/test/schema/postgresql_specific_schema.rb
@@ -1,6 +1,7 @@
 ActiveRecord::Schema.define do
 
   enable_extension!("uuid-ossp", ActiveRecord::Base.connection)
+  enable_extension!("pgcrypto",  ActiveRecord::Base.connection) if ActiveRecord::Base.connection.supports_pgcrypto_uuid?
 
   create_table :uuid_parents, id: :uuid, force: true do |t|
     t.string :name