Skip to content
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

Disable fallback lookups for find_by_hashid methods #41

Merged
merged 2 commits into from
Nov 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions lib/hashid/rails.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,14 @@ def encode_id(ids)
end
end

def decode_id(ids)
# @param ids [String, Integer, Array<Integer, String>] id(s) to decode.
# @param fallback [Boolean] indicates whether to return the passed in
# id(s) if unable to decode or if already decoded.
def decode_id(ids, fallback: false)
if ids.is_a?(Array)
ids.map { |id| hashid_decode(id) }
ids.map { |id| hashid_decode(id, fallback: fallback) }
else
hashid_decode(ids)
hashid_decode(ids, fallback: fallback)
end
end

Expand All @@ -67,18 +70,18 @@ def find(*ids)
uniq_ids = uniq_ids.first unless expects_array || uniq_ids.size > 1

if Hashid::Rails.configuration.override_find
super(decode_id(uniq_ids))
super(decode_id(uniq_ids, fallback: true))
else
super
end
end

def find_by_hashid(hashid)
find_by(id: decode_id(hashid))
find_by(id: decode_id(hashid, fallback: false))
end

def find_by_hashid!(hashid)
find_by!(id: decode_id(hashid))
find_by!(id: decode_id(hashid, fallback: false))
end

private
Expand All @@ -95,10 +98,12 @@ def hashid_encode(id)
end
end

def hashid_decode(id)
def hashid_decode(id, fallback:)
decoded_hashid = hashids.decode(id.to_s)
fallback_value = fallback ? id : nil

if Hashid::Rails.configuration.sign_hashids
valid_hashid?(decoded_hashid) ? decoded_hashid.last : id
valid_hashid?(decoded_hashid) ? decoded_hashid.last : fallback_value
else
decoded_hashid.first
end
Expand Down
51 changes: 43 additions & 8 deletions spec/hashid/rails_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,13 @@
expect(decoded_id).to eq(100_117)
end

it "returns already decoded id" do
decoded_id = FakeModel.decode_id(100_117)
it "returns nil when already decoded id" do
decoded_id = FakeModel.decode_id(100_117, fallback: false)
expect(decoded_id).to eq(nil)
end

it "returns already decoded id when fallback" do
decoded_id = FakeModel.decode_id(100_117, fallback: true)
expect(decoded_id).to eq(100_117)
end
end
Expand All @@ -118,8 +123,13 @@
expect(decoded_ids).to eq([1])
end

it "returns array with already decoded id" do
decoded_ids = FakeModel.decode_id([1])
it "returns array with nil when already decoded id" do
decoded_ids = FakeModel.decode_id([1], fallback: false)
expect(decoded_ids).to eq([nil])
end

it "returns array with already decoded id when fallback" do
decoded_ids = FakeModel.decode_id([1], fallback: true)
expect(decoded_ids).to eq([1])
end
end
Expand All @@ -130,8 +140,13 @@
expect(decoded_ids).to eq([1, 2, 3])
end

it "returns array of already decoded ids" do
decoded_ids = FakeModel.decode_id([1, 2, 3])
it "returns array of nil when already decoded ids" do
decoded_ids = FakeModel.decode_id([1, 2, 3], fallback: false)
expect(decoded_ids).to eq([nil, nil, nil])
end

it "returns array of already decoded ids when fallback" do
decoded_ids = FakeModel.decode_id([1, 2, 3], fallback: true)
expect(decoded_ids).to eq([1, 2, 3])
end
end
Expand Down Expand Up @@ -244,7 +259,12 @@
end

it "does not try and decode regular ids" do
decoded_id = FakeModel.decode_id(100_117)
decoded_id = FakeModel.decode_id(100_117, fallback: false)
expect(decoded_id).to eq(nil)
end

it "does not try and decode regular ids when fallback" do
decoded_id = FakeModel.decode_id(100_117, fallback: true)
expect(decoded_id).to eq(100_117)
end

Expand Down Expand Up @@ -294,6 +314,14 @@

expect(result).to eq(nil)
end

it "returns nil for non-hashid" do
model = FakeModel.create!

result = FakeModel.find_by_hashid(model.id)

expect(result).to eq(nil)
end
end

describe ".find_by_hashid!" do
Expand All @@ -305,10 +333,17 @@
expect(result).to eq(model)
end

it "returns nil when unable to find model" do
it "raises record not found when unable to find model" do
expect { FakeModel.find_by_hashid!("ABC") }
.to raise_error(ActiveRecord::RecordNotFound)
end

it "raises record not found for non-hashid" do
model = FakeModel.create!

expect { FakeModel.find_by_hashid!(model.id) }
.to raise_error(ActiveRecord::RecordNotFound)
end
end

describe ".configure" do
Expand Down