-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add support for UUID keys #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -214,6 +214,7 @@ GEM | |
| zeitwerk (2.7.4) | ||
|
|
||
| PLATFORMS | ||
| arm64-darwin-23 | ||
| arm64-darwin-24 | ||
| x86_64-linux | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,37 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require "zlib" | ||
| require "digest/sha1" | ||
|
|
||
| module FixtureBot | ||
| module Key | ||
| # RFC 4122 URL namespace UUID, used as the base namespace for UUID v5 generation. | ||
| URL_NAMESPACE = "6ba7b811-9dad-11d1-80b4-00c04fd430c8" | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the pre-defined namespace ID for URL was just arbitrary, our use case doesn't fit the classes given. We append "fixturebot:" to ensure we don't have collisions should another gem use the same We could create our own "starting point" using UUID v4 to generate a random one to serve as our "seed", but either way we should be collision proof. |
||
|
|
||
| def self.generate(table_name, record_name) | ||
| Zlib.crc32("#{table_name}:#{record_name}") & 0x7FFFFFFF | ||
| end | ||
|
|
||
| def self.generate_uuid(table_name, record_name) | ||
| uuid_v5(URL_NAMESPACE, "fixturebot:#{table_name}:#{record_name}") | ||
| end | ||
|
|
||
| def self.uuid_v5(namespace_uuid, name) | ||
| # Parse namespace UUID string to 16 bytes | ||
| namespace_bytes = [namespace_uuid.tr("-", "")].pack("H32") | ||
|
|
||
| # SHA-1 hash of namespace bytes + name (RFC 4122 Section 4.3) | ||
| hash = Digest::SHA1.digest(namespace_bytes + name.to_s) | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think not having extra dependencies here is a bonus. |
||
|
|
||
| # Take first 16 bytes and set version/variant bits | ||
| bytes = hash.bytes[0, 16] | ||
| bytes[6] = (bytes[6] & 0x0F) | 0x50 # Version 5 | ||
| bytes[8] = (bytes[8] & 0x3F) | 0x80 # RFC 4122 variant | ||
|
|
||
| # Format as standard UUID string (8-4-4-4-12) | ||
| hex = bytes.map { |b| "%02x" % b }.join | ||
| "#{hex[0, 8]}-#{hex[8, 4]}-#{hex[12, 4]}-#{hex[16, 4]}-#{hex[20, 12]}" | ||
| end | ||
| private_class_method :uuid_v5 | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,8 @@ def build_table(name) | |
| .reject { |c| framework_column?(c.name) } | ||
| .map { |c| c.name.to_sym } | ||
|
|
||
| primary_key_type = detect_primary_key_type(name) | ||
|
|
||
| associations = @connection.foreign_keys(name).map do |fk| | ||
| Schema::BelongsTo.new( | ||
| name: association_name(fk.column), | ||
|
|
@@ -54,10 +56,21 @@ def build_table(name) | |
| name: name.to_sym, | ||
| singular_name: singularize(name), | ||
| columns: columns, | ||
| belongs_to_associations: associations | ||
| belongs_to_associations: associations, | ||
| primary_key_type: primary_key_type | ||
| ) | ||
| end | ||
|
|
||
| def detect_primary_key_type(table_name) | ||
| pk = @connection.primary_key(table_name) | ||
| return :integer unless pk | ||
|
|
||
| column = @connection.columns(table_name).find { |c| c.name == pk } | ||
| return :integer unless column | ||
|
|
||
| column.type == :uuid ? :uuid : :integer | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I started by looking at the |
||
| end | ||
|
|
||
| def build_join_table(name) | ||
| fk_columns = foreign_key_columns(name) | ||
|
|
||
|
|
||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wanted to ensure we covered the Rails-supported databases (at least in theory since we're just using sqlite3 and stubbing out the rest), so I added a new directory and put adapter specific specs in it. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require "fixturebot/rails" | ||
|
|
||
| RSpec.describe FixtureBot::Rails::SchemaLoader, "MySQL UUID detection" do | ||
| before do | ||
| ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:") | ||
|
|
||
| ActiveRecord::Schema.define(version: 2024_01_01_000000) do | ||
| create_table "users", force: :cascade do |t| | ||
| t.string "name" | ||
| t.timestamps | ||
| end | ||
| end | ||
| end | ||
|
|
||
| after do | ||
| ActiveRecord::Base.connection_pool.disconnect! | ||
| end | ||
|
|
||
| let(:connection) { ActiveRecord::Base.connection } | ||
|
|
||
| subject(:schema) { described_class.load } | ||
|
|
||
| # MySQL has no native uuid column type. UUIDs are typically stored in | ||
| # char(36) columns, which the adapter reports as `column.type` `:string`. | ||
| # Auto-detection cannot distinguish these from regular string columns, so | ||
| # the loader falls back to the :integer strategy. | ||
| before do | ||
| string_column = instance_double( | ||
| ActiveRecord::ConnectionAdapters::Column, | ||
| name: "id", | ||
| type: :string | ||
| ) | ||
| allow(connection).to receive(:primary_key).and_call_original | ||
| allow(connection).to receive(:primary_key).with("users").and_return("id") | ||
| allow(connection).to receive(:columns).and_call_original | ||
| allow(connection).to receive(:columns).with("users").and_return([string_column]) | ||
| end | ||
|
|
||
| it "returns :integer because column type is :string, not :uuid" do | ||
| expect(schema.tables[:users].primary_key_type).to eq(:integer) | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require "fixturebot/rails" | ||
|
|
||
| RSpec.describe FixtureBot::Rails::SchemaLoader, "PostgreSQL UUID detection" do | ||
| before do | ||
| ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:") | ||
|
|
||
| ActiveRecord::Schema.define(version: 2024_01_01_000000) do | ||
| create_table "users", force: :cascade do |t| | ||
| t.string "name" | ||
| t.timestamps | ||
| end | ||
|
|
||
| create_table "posts", force: :cascade do |t| | ||
| t.string "title" | ||
| t.timestamps | ||
| end | ||
| end | ||
| end | ||
|
|
||
| after do | ||
| ActiveRecord::Base.connection_pool.disconnect! | ||
| end | ||
|
|
||
| let(:connection) { ActiveRecord::Base.connection } | ||
|
|
||
| subject(:schema) { described_class.load } | ||
|
|
||
| # PostgreSQL has a native uuid column type. When a table is created with | ||
| # `id: :uuid`, the adapter reports `column.type` as `:uuid`. | ||
| before do | ||
| uuid_column = instance_double( | ||
| ActiveRecord::ConnectionAdapters::Column, | ||
| name: "id", | ||
| type: :uuid | ||
| ) | ||
| allow(connection).to receive(:primary_key).and_call_original | ||
| allow(connection).to receive(:primary_key).with("users").and_return("id") | ||
| allow(connection).to receive(:columns).and_call_original | ||
| allow(connection).to receive(:columns).with("users").and_return([uuid_column]) | ||
| end | ||
|
|
||
| it "detects :uuid when column type is :uuid" do | ||
| expect(schema.tables[:users].primary_key_type).to eq(:uuid) | ||
| end | ||
|
|
||
| it "still detects :integer for other tables" do | ||
| expect(schema.tables[:posts].primary_key_type).to eq(:integer) | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require "fixturebot/rails" | ||
|
|
||
| RSpec.describe FixtureBot::Rails::SchemaLoader, "SQLite UUID detection" do | ||
| before do | ||
| ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:") | ||
|
|
||
| ActiveRecord::Schema.define(version: 2024_01_01_000000) do | ||
| create_table "users", force: :cascade do |t| | ||
| t.string "name" | ||
| t.timestamps | ||
| end | ||
| end | ||
| end | ||
|
|
||
| after do | ||
| ActiveRecord::Base.connection_pool.disconnect! | ||
| end | ||
|
|
||
| let(:connection) { ActiveRecord::Base.connection } | ||
|
|
||
| subject(:schema) { described_class.load } | ||
|
|
||
| # SQLite has no native uuid column type. UUIDs are typically stored in | ||
| # varchar(36) columns, which the adapter reports as `column.type` `:string`. | ||
| # Auto-detection cannot distinguish these from regular string columns, so | ||
| # the loader falls back to the :integer strategy. | ||
| before do | ||
| string_column = instance_double( | ||
| ActiveRecord::ConnectionAdapters::Column, | ||
| name: "id", | ||
| type: :string | ||
| ) | ||
| allow(connection).to receive(:primary_key).and_call_original | ||
| allow(connection).to receive(:primary_key).with("users").and_return("id") | ||
| allow(connection).to receive(:columns).and_call_original | ||
| allow(connection).to receive(:columns).with("users").and_return([string_column]) | ||
| end | ||
|
|
||
| it "returns :integer because column type is :string, not :uuid" do | ||
| expect(schema.tables[:users].primary_key_type).to eq(:integer) | ||
| end | ||
|
|
||
| context "when table has no primary key" do | ||
| before do | ||
| allow(connection).to receive(:primary_key).with("users").and_return(nil) | ||
| end | ||
|
|
||
| it "returns :integer" do | ||
| expect(schema.tables[:users].primary_key_type).to eq(:integer) | ||
| end | ||
| end | ||
| end |
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also moved this spec in to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We now pass the full hash of tables along in a few places, mostly for the sake of join tables.
For example:
When building a
postsrow, the builder needs to generatetenant_id. That foreign key must match the tenants table's key type (UUID), not the posts table's type (integer). So it needs to look up@tables[:tenants].primary_key_typeto decide.I don't love it, but I couldn't come up with a cleaner way.