Skip to content

Commit

Permalink
Ensure prefixes are unique (#86)
Browse files Browse the repository at this point in the history
When two or more models are defined with the same prefix, `PrefixedIds.find` will load the model for whichever one was defined last, and could lead to loading an unexpected model for a given prefixed id.

For apps with lots of models it’s quite easy to add the same prefix on two or more models by mistake. It’s also not obvious how to access or list the prefixes already in use, so I think it makes sense for the gem to protect against this.

This PR adds a check when calling `has_prefix_id` and raises `PrefixedIds::Error` if the same prefix was already defined for a different model.
  • Loading branch information
samoli authored Nov 25, 2024
1 parent 5c51967 commit 9e391c1
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
10 changes: 9 additions & 1 deletion lib/prefixed_ids.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ def self.split_id(prefix_id, delimiter = PrefixedIds.delimiter)
[prefix, id]
end

def self.register_prefix(prefix, model:)
if (existing_model = PrefixedIds.models[prefix]) && existing_model != model
raise Error, "Prefix #{prefix} already defined for model #{model}"
end

PrefixedIds.models[prefix] = model
end

# Adds `has_prefix_id` method
module Rails
extend ActiveSupport::Concern
Expand All @@ -46,7 +54,7 @@ def has_prefix_id(prefix, override_find: true, override_param: true, fallback: t
self._prefix_id_fallback = fallback

# Register with PrefixedIds to support PrefixedIds#find
PrefixedIds.models[prefix.to_s] = self
PrefixedIds.register_prefix(prefix.to_s, model: self)
end
end
end
Expand Down
24 changes: 24 additions & 0 deletions test/prefixed_ids_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -227,4 +227,28 @@ class PrefixedIdsTest < ActiveSupport::TestCase
assert_equal prefix_decoded, [1, 1]
end
end

test "register_prefix adds the expected prefix and model" do
model = Class.new(ApplicationRecord) do
def self.name
"TestModel"
end
end

PrefixedIds.register_prefix("test_model", model: model)
assert_equal model, PrefixedIds.models["test_model"]
end

test "has_prefix_id raises when prefix was already used" do
assert PrefixedIds.models.key?("user")
assert_raises PrefixedIds::Error do
Class.new(ApplicationRecord) do
def self.name
"TestModel"
end

has_prefix_id :user
end
end
end
end

0 comments on commit 9e391c1

Please sign in to comment.