Disable fallback lookups for find_by_hashid methods (#41)

* Fix fallback for find_by_hashid methods

This existing implementation was falling back to an ID lookup if unable to decode hashid. It should instead return nil or raise an not found exception.

lib/hashid/rails.rb

@@ -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
 
@@ -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
@@ -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

spec/hashid/rails_spec.rb

@@ -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
@@ -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
@@ -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
@@ -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
 
@@ -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
@@ -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