From 99a7c44f1af2d2bfd890dc1827634c80ed26c495 Mon Sep 17 00:00:00 2001
From: Tyler Willingham <171991+tylerwillingham@users.noreply.github.com>
Date: Thu, 22 Aug 2024 20:10:53 -0700
Subject: [PATCH 1/4] Allow creating a GID from a model with an alternative to
 the .primary_key

---
 lib/global_id/uri/gid.rb              |  6 +++++-
 test/cases/uri_gid_test.rb            | 25 +++++++++++++++++++++++++
 test/helper.rb                        |  1 +
 test/models/configurable_key_model.rb | 12 ++++++++++++
 4 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 test/models/configurable_key_model.rb

diff --git a/lib/global_id/uri/gid.rb b/lib/global_id/uri/gid.rb
index 20a22a7..37c2679 100644
--- a/lib/global_id/uri/gid.rb
+++ b/lib/global_id/uri/gid.rb
@@ -70,7 +70,11 @@ def parse(uri)
       #
       #   URI::GID.create('bcx', Person.find(5), database: 'superhumans')
       def create(app, model, params = nil)
-        build app: app, model_name: model.class.name, model_id: model.id, params: params
+        model_id_method = if params.is_a?(Hash) && (model_id_key = params.delete(:model_id_key))
+                            model_id_key
+                          end || :id
+
+        build app: app, model_name: model.class.name, model_id: model.send(model_id_method), params: params
       end
 
       # Create a new URI::GID from components with argument check.
diff --git a/test/cases/uri_gid_test.rb b/test/cases/uri_gid_test.rb
index 3c3b3fa..7c2edc7 100644
--- a/test/cases/uri_gid_test.rb
+++ b/test/cases/uri_gid_test.rb
@@ -6,6 +6,8 @@ class URI::GIDTest <  ActiveSupport::TestCase
     @gid = URI::GID.parse(@gid_string)
     @cpk_gid_string = 'gid://bcx/CompositePrimaryKeyModel/tenant-key-value/id-value'
     @cpk_gid = URI::GID.parse(@cpk_gid_string)
+    @ckm_gid_string = 'gid://bcx/ConfigurableKeyModel/external-id-123'
+    @ckm_gid = URI::GID.parse(@ckm_gid_string)
   end
 
   test 'parsed' do
@@ -13,6 +15,9 @@ class URI::GIDTest <  ActiveSupport::TestCase
     assert_equal @gid.model_name, 'Person'
     assert_equal @gid.model_id, '5'
     assert_equal ["tenant-key-value", "id-value"], @cpk_gid.model_id
+    assert_equal @ckm_gid.app, 'bcx'
+    assert_equal @ckm_gid.model_name, 'ConfigurableKeyModel'
+    assert_equal @ckm_gid.model_id, 'external-id-123'
   end
 
   test 'parsed for non existing model class' do
@@ -39,6 +44,11 @@ class URI::GIDTest <  ActiveSupport::TestCase
     assert_equal @cpk_gid_string, URI::GID.create('bcx', model).to_s
   end
 
+  test 'create from a configurable key model' do
+    model = ConfigurableKeyModel.new(id: 'id-value', external_id: 'external-id-123')
+    assert_equal @ckm_gid_string, URI::GID.create('bcx', model, model_id_key: :external_id).to_s
+  end
+
   test 'build' do
     array = URI::GID.build(['bcx', 'Person', '5', nil])
     assert array
@@ -65,6 +75,21 @@ class URI::GIDTest <  ActiveSupport::TestCase
     assert_equal("gid://bcx/CompositePrimaryKeyModel/tenant-key-value/id-value", array.to_s)
   end
 
+  # NOTE: I'm not sure if this test is valuable, it's pretty duplicative with the standard
+  #   test path, but with a different value passed in for `model_id:`
+  test 'build with a configurable key model' do
+    array = URI::GID.build(['bcx', 'ConfigurableKeyModel', 'external-id-123', nil])
+    gid = URI::GID.build(
+      app: 'bcx',
+      model_name: 'ConfigurableKeyModel',
+      model_id: 'external-id-123',
+      params: nil
+    )
+
+    assert_equal array, gid
+    assert_equal 'gid://bcx/ConfigurableKeyModel/external-id-123', array.to_s
+  end
+
   test 'build with wrong ordered array creates a wrong ordered gid' do
     assert_not_equal @gid_string, URI::GID.build(['Person', '5', 'bcx', nil]).to_s
   end
diff --git a/test/helper.rb b/test/helper.rb
index 3cdeb65..efb19dd 100644
--- a/test/helper.rb
+++ b/test/helper.rb
@@ -7,6 +7,7 @@
 require 'models/person'
 require 'models/person_model'
 require 'models/composite_primary_key_model'
+require 'models/configurable_key_model'
 
 require 'json'
 
diff --git a/test/models/configurable_key_model.rb b/test/models/configurable_key_model.rb
new file mode 100644
index 0000000..eaa25fa
--- /dev/null
+++ b/test/models/configurable_key_model.rb
@@ -0,0 +1,12 @@
+require 'active_model'
+
+class ConfigurableKeyModel
+  include ActiveModel::Model
+  include GlobalID::Identification
+
+  attr_accessor :id, :external_id
+
+  def self.primary_key
+    :id
+  end
+end

From 46208a1c94979600f860a5993795ebe26bb46641 Mon Sep 17 00:00:00 2001
From: Tyler Willingham <171991+tylerwillingham@users.noreply.github.com>
Date: Thu, 22 Aug 2024 20:30:20 -0700
Subject: [PATCH 2/4] Enable setting Model.global_id_column

This allows overriding the column used to define GIDs
---
 lib/global_id/global_id.rb            |  1 +
 lib/global_id/identification.rb       | 13 +++++++++++++
 lib/global_id/uri/gid.rb              |  7 +++++--
 test/cases/global_id_test.rb          | 13 +++++++++++++
 test/cases/uri_gid_test.rb            |  2 +-
 test/models/configurable_key_model.rb | 16 ++++++++++++++--
 test/models/person.rb                 |  4 ++++
 7 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/lib/global_id/global_id.rb b/lib/global_id/global_id.rb
index 1082326..be86d0c 100644
--- a/lib/global_id/global_id.rb
+++ b/lib/global_id/global_id.rb
@@ -11,6 +11,7 @@ class << self
     def create(model, options = {})
       if app = options.fetch(:app) { GlobalID.app }
         params = options.except(:app, :verifier, :for)
+        params[:global_id_column] ||= model.global_id_column
         new URI::GID.create(app, model, params), options
       else
         raise ArgumentError, 'An app is required to create a GlobalID. ' \
diff --git a/lib/global_id/identification.rb b/lib/global_id/identification.rb
index 1cd4941..eead1a1 100644
--- a/lib/global_id/identification.rb
+++ b/lib/global_id/identification.rb
@@ -26,6 +26,19 @@ class GlobalID
   #   GlobalID::Locator.locate person_gid
   #   # => #<Person:0x007fae94bf6298 @id="1">
   module Identification
+    def self.included(base)
+      base.extend(ClassMethods)
+    end
+
+    module ClassMethods
+      def global_id_column(column_name = nil)
+        @global_id_column ||= column_name
+      end
+    end
+
+    def global_id_column
+      self.class.global_id_column || self.class.try(:primary_key) || :id
+    end
 
     # Returns the Global ID of the model.
     #
diff --git a/lib/global_id/uri/gid.rb b/lib/global_id/uri/gid.rb
index 37c2679..9fe32c2 100644
--- a/lib/global_id/uri/gid.rb
+++ b/lib/global_id/uri/gid.rb
@@ -70,8 +70,11 @@ def parse(uri)
       #
       #   URI::GID.create('bcx', Person.find(5), database: 'superhumans')
       def create(app, model, params = nil)
-        model_id_method = if params.is_a?(Hash) && (model_id_key = params.delete(:model_id_key))
-                            model_id_key
+        global_id_column = params&.delete(:global_id_column)
+        model_id_method = if model.id.is_a?(Array)
+                            :id
+                          else
+                            global_id_column
                           end || :id
 
         build app: app, model_name: model.class.name, model_id: model.send(model_id_method), params: params
diff --git a/test/cases/global_id_test.rb b/test/cases/global_id_test.rb
index baf7bc1..8f1239f 100644
--- a/test/cases/global_id_test.rb
+++ b/test/cases/global_id_test.rb
@@ -45,6 +45,8 @@ class GlobalIDCreationTest < ActiveSupport::TestCase
     @person_namespaced_gid = GlobalID.create(Person::Child.new(4))
     @person_model_gid = GlobalID.create(PersonModel.new(id: 1))
     @cpk_model_gid = GlobalID.create(CompositePrimaryKeyModel.new(id: ["tenant-key-value", "id-value"]))
+    @ckm_model = ConfigurableKeyModel.new(id: "id-value", external_id: "external-id-value")
+    @ckm_model_gid = GlobalID.create(@ckm_model)
   end
 
   test 'find' do
@@ -53,6 +55,7 @@ class GlobalIDCreationTest < ActiveSupport::TestCase
     assert_equal Person::Child.find(@person_namespaced_gid.model_id), @person_namespaced_gid.find
     assert_equal PersonModel.find(@person_model_gid.model_id), @person_model_gid.find
     assert_equal CompositePrimaryKeyModel.find(@cpk_model_gid.model_id), @cpk_model_gid.find
+    assert_equal ConfigurableKeyModel.find(@ckm_model_gid.model_id), @ckm_model_gid.find
   end
 
   test 'find with class' do
@@ -60,6 +63,7 @@ class GlobalIDCreationTest < ActiveSupport::TestCase
     assert_equal Person.find(@person_uuid_gid.model_id), @person_uuid_gid.find(only: Person)
     assert_equal PersonModel.find(@person_model_gid.model_id), @person_model_gid.find(only: PersonModel)
     assert_equal CompositePrimaryKeyModel.find(@cpk_model_gid.model_id), @cpk_model_gid.find(only: CompositePrimaryKeyModel)
+    assert_equal ConfigurableKeyModel.find(@ckm_model_gid.model_id), @ckm_model_gid.find(only: ConfigurableKeyModel)
   end
 
   test 'find with class no match' do
@@ -68,6 +72,7 @@ class GlobalIDCreationTest < ActiveSupport::TestCase
     assert_nil @person_namespaced_gid.find(only: String)
     assert_nil @person_model_gid.find(only: Float)
     assert_nil @cpk_model_gid.find(only: Hash)
+    assert_nil @ckm_model_gid.find(only: Hash)
   end
 
   test 'find with subclass' do
@@ -140,6 +145,7 @@ class GlobalIDCreationTest < ActiveSupport::TestCase
     assert_equal 'gid://bcx/Person::Child/4', @person_namespaced_gid.to_s
     assert_equal 'gid://bcx/PersonModel/1', @person_model_gid.to_s
     assert_equal 'gid://bcx/CompositePrimaryKeyModel/tenant-key-value/id-value', @cpk_model_gid.to_s
+    assert_equal 'gid://bcx/ConfigurableKeyModel/external-id-value', @ckm_model_gid.to_s
   end
 
   test 'as param' do
@@ -166,6 +172,7 @@ class GlobalIDCreationTest < ActiveSupport::TestCase
     assert_equal URI('gid://bcx/Person::Child/4'), @person_namespaced_gid.uri
     assert_equal URI('gid://bcx/PersonModel/1'), @person_model_gid.uri
     assert_equal URI('gid://bcx/CompositePrimaryKeyModel/tenant-key-value/id-value'), @cpk_model_gid.uri
+    assert_equal URI('gid://bcx/ConfigurableKeyModel/external-id-value'), @ckm_model_gid.uri
   end
 
   test 'as JSON' do
@@ -183,6 +190,9 @@ class GlobalIDCreationTest < ActiveSupport::TestCase
 
     assert_equal 'gid://bcx/CompositePrimaryKeyModel/tenant-key-value/id-value', @cpk_model_gid.as_json
     assert_equal '"gid://bcx/CompositePrimaryKeyModel/tenant-key-value/id-value"', @cpk_model_gid.to_json
+
+    assert_equal 'gid://bcx/ConfigurableKeyModel/external-id-value', @ckm_model_gid.as_json
+    assert_equal '"gid://bcx/ConfigurableKeyModel/external-id-value"', @ckm_model_gid.to_json
   end
 
   test 'model id' do
@@ -191,6 +201,7 @@ class GlobalIDCreationTest < ActiveSupport::TestCase
     assert_equal '4', @person_namespaced_gid.model_id
     assert_equal '1', @person_model_gid.model_id
     assert_equal ['tenant-key-value', 'id-value'], @cpk_model_gid.model_id
+    assert_equal 'external-id-value', @ckm_model_gid.model_id
   end
 
   test 'model name' do
@@ -199,6 +210,7 @@ class GlobalIDCreationTest < ActiveSupport::TestCase
     assert_equal 'Person::Child', @person_namespaced_gid.model_name
     assert_equal 'PersonModel', @person_model_gid.model_name
     assert_equal 'CompositePrimaryKeyModel', @cpk_model_gid.model_name
+    assert_equal 'ConfigurableKeyModel', @ckm_model_gid.model_name
   end
 
   test 'model class' do
@@ -207,6 +219,7 @@ class GlobalIDCreationTest < ActiveSupport::TestCase
     assert_equal Person::Child, @person_namespaced_gid.model_class
     assert_equal PersonModel, @person_model_gid.model_class
     assert_equal CompositePrimaryKeyModel, @cpk_model_gid.model_class
+    assert_equal ConfigurableKeyModel, @ckm_model_gid.model_class
     assert_raise ArgumentError do
       GlobalID.find 'gid://bcx/SignedGlobalID/5'
     end
diff --git a/test/cases/uri_gid_test.rb b/test/cases/uri_gid_test.rb
index 7c2edc7..e380be0 100644
--- a/test/cases/uri_gid_test.rb
+++ b/test/cases/uri_gid_test.rb
@@ -46,7 +46,7 @@ class URI::GIDTest <  ActiveSupport::TestCase
 
   test 'create from a configurable key model' do
     model = ConfigurableKeyModel.new(id: 'id-value', external_id: 'external-id-123')
-    assert_equal @ckm_gid_string, URI::GID.create('bcx', model, model_id_key: :external_id).to_s
+    assert_equal @ckm_gid_string, URI::GID.create('bcx', model, global_id_column: :external_id).to_s
   end
 
   test 'build' do
diff --git a/test/models/configurable_key_model.rb b/test/models/configurable_key_model.rb
index eaa25fa..c1a08c2 100644
--- a/test/models/configurable_key_model.rb
+++ b/test/models/configurable_key_model.rb
@@ -6,7 +6,19 @@ class ConfigurableKeyModel
 
   attr_accessor :id, :external_id
 
-  def self.primary_key
-    :id
+  global_id_column :external_id
+
+  class << self
+    def primary_key
+      :id
+    end
+
+    def find(external_id)
+      new external_id: external_id, id: "id-value"
+    end
+  end
+
+  def ==(other)
+    external_id == other.try(:external_id)
   end
 end
diff --git a/test/models/person.rb b/test/models/person.rb
index a74ac59..d851ad1 100644
--- a/test/models/person.rb
+++ b/test/models/person.rb
@@ -41,6 +41,10 @@ class PersonUuid < Person
   def self.primary_key
     :uuid
   end
+
+  def uuid
+    id
+  end
 end
 
 class Person::Scoped < Person

From 6cd2b82aa2b281dce686dbd22470fea0e3b277c2 Mon Sep 17 00:00:00 2001
From: Tyler Willingham <171991+tylerwillingham@users.noreply.github.com>
Date: Thu, 22 Aug 2024 21:41:11 -0700
Subject: [PATCH 3/4] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Eliminate=20unnecessar?=
 =?UTF-8?q?y=20hash=20state?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Because of Identification.global_id_column, we can keep this internal
API much cleaner and avoid mucking with hash state to pass around this
column
---
 lib/global_id/global_id.rb | 1 -
 lib/global_id/uri/gid.rb   | 7 +------
 test/cases/uri_gid_test.rb | 2 +-
 3 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/lib/global_id/global_id.rb b/lib/global_id/global_id.rb
index be86d0c..1082326 100644
--- a/lib/global_id/global_id.rb
+++ b/lib/global_id/global_id.rb
@@ -11,7 +11,6 @@ class << self
     def create(model, options = {})
       if app = options.fetch(:app) { GlobalID.app }
         params = options.except(:app, :verifier, :for)
-        params[:global_id_column] ||= model.global_id_column
         new URI::GID.create(app, model, params), options
       else
         raise ArgumentError, 'An app is required to create a GlobalID. ' \
diff --git a/lib/global_id/uri/gid.rb b/lib/global_id/uri/gid.rb
index 9fe32c2..f0f6012 100644
--- a/lib/global_id/uri/gid.rb
+++ b/lib/global_id/uri/gid.rb
@@ -70,12 +70,7 @@ def parse(uri)
       #
       #   URI::GID.create('bcx', Person.find(5), database: 'superhumans')
       def create(app, model, params = nil)
-        global_id_column = params&.delete(:global_id_column)
-        model_id_method = if model.id.is_a?(Array)
-                            :id
-                          else
-                            global_id_column
-                          end || :id
+        model_id_method = model.id.is_a?(Array) ? :id : model.global_id_column
 
         build app: app, model_name: model.class.name, model_id: model.send(model_id_method), params: params
       end
diff --git a/test/cases/uri_gid_test.rb b/test/cases/uri_gid_test.rb
index e380be0..cc706e4 100644
--- a/test/cases/uri_gid_test.rb
+++ b/test/cases/uri_gid_test.rb
@@ -46,7 +46,7 @@ class URI::GIDTest <  ActiveSupport::TestCase
 
   test 'create from a configurable key model' do
     model = ConfigurableKeyModel.new(id: 'id-value', external_id: 'external-id-123')
-    assert_equal @ckm_gid_string, URI::GID.create('bcx', model, global_id_column: :external_id).to_s
+    assert_equal @ckm_gid_string, URI::GID.create('bcx', model).to_s
   end
 
   test 'build' do

From 314bc0dba5d27bed336a52b66534f5c596a8a1c3 Mon Sep 17 00:00:00 2001
From: Tyler Willingham <171991+tylerwillingham@users.noreply.github.com>
Date: Thu, 22 Aug 2024 22:04:14 -0700
Subject: [PATCH 4/4] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20`Model#global=5Fid=5Fm?=
 =?UTF-8?q?ethod`=20instead=20of=20`Model#global=5Fid=5Fcolumn`?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`Model.primary_key` is actually irrelevant to this feature, the only
thing that matters is the model method.

For composite keys, or primary keys not named "id", we still get the
important value from `Model#id`, the only time we don't is when we need
to use a non-PK column.
---
 lib/global_id/identification.rb | 4 ++--
 lib/global_id/uri/gid.rb        | 4 +---
 test/models/person.rb           | 4 ----
 3 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/lib/global_id/identification.rb b/lib/global_id/identification.rb
index eead1a1..70513a7 100644
--- a/lib/global_id/identification.rb
+++ b/lib/global_id/identification.rb
@@ -36,8 +36,8 @@ def global_id_column(column_name = nil)
       end
     end
 
-    def global_id_column
-      self.class.global_id_column || self.class.try(:primary_key) || :id
+    def global_id_method
+      self.class.global_id_column || :id
     end
 
     # Returns the Global ID of the model.
diff --git a/lib/global_id/uri/gid.rb b/lib/global_id/uri/gid.rb
index f0f6012..0b46591 100644
--- a/lib/global_id/uri/gid.rb
+++ b/lib/global_id/uri/gid.rb
@@ -70,9 +70,7 @@ def parse(uri)
       #
       #   URI::GID.create('bcx', Person.find(5), database: 'superhumans')
       def create(app, model, params = nil)
-        model_id_method = model.id.is_a?(Array) ? :id : model.global_id_column
-
-        build app: app, model_name: model.class.name, model_id: model.send(model_id_method), params: params
+        build app: app, model_name: model.class.name, model_id: model.send(model.global_id_method), params: params
       end
 
       # Create a new URI::GID from components with argument check.
diff --git a/test/models/person.rb b/test/models/person.rb
index d851ad1..a74ac59 100644
--- a/test/models/person.rb
+++ b/test/models/person.rb
@@ -41,10 +41,6 @@ class PersonUuid < Person
   def self.primary_key
     :uuid
   end
-
-  def uuid
-    id
-  end
 end
 
 class Person::Scoped < Person