Ensure the value passed to find_by(id:) is a Valkyrie::ID

Fixed by #271

lib/valkyrie/persistence/active_fedora/query_service.rb

@@ -11,6 +11,7 @@ def find_all_of_model(model:)
       end
 
       def find_by(id:)
+        validate_id(id)
         Valkyrie::Persistence::ActiveFedora::Queries::FindByIdQuery.new(id).run
       end
 
@@ -33,6 +34,12 @@ def find_inverse_references_by(resource:, property:)
       def custom_queries
         @custom_queries ||= ::Valkyrie::Persistence::CustomQueryContainer.new(query_service: self)
       end
+
+      private
+
+        def validate_id(id)
+          raise ArgumentError, 'id must be a Valkyrie::ID' unless id.is_a? Valkyrie::ID
+        end
     end
   end
 end

valkyrie/lib/valkyrie/persistence/fedora/query_service.rb

@@ -8,6 +8,7 @@ def initialize(adapter:)
     end
 
     def find_by(id:)
+      validate_id(id)
       uri = adapter.id_to_uri(id)
       begin
         resource = Ldp::Resource.for(connection, uri, connection.get(uri))
@@ -82,5 +83,11 @@ def find_inverse_references_by(resource:, property:)
     def custom_queries
       @custom_queries ||= ::Valkyrie::Persistence::CustomQueryContainer.new(query_service: self)
     end
+
+    private
+
+      def validate_id(id)
+        raise ArgumentError, 'id must be a Valkyrie::ID' unless id.is_a? Valkyrie::ID
+      end
   end
 end

valkyrie/lib/valkyrie/persistence/memory/query_service.rb

@@ -15,6 +15,7 @@ def initialize(adapter:)
     #   isn't in the persistence backend.
     # @return [Valkyrie::Resource] The object being searched for.
     def find_by(id:)
+      validate_id(id)
       cache[id] || raise(::Valkyrie::Persistence::ObjectNotFoundError)
     end
 
@@ -90,5 +91,11 @@ def member_ids(resource:)
     def custom_queries
       @custom_queries ||= ::Valkyrie::Persistence::CustomQueryContainer.new(query_service: self)
     end
+
+    private
+
+      def validate_id(id)
+        raise ArgumentError, 'id must be a Valkyrie::ID' unless id.is_a? Valkyrie::ID
+      end
   end
 end

valkyrie/lib/valkyrie/persistence/postgres/query_service.rb

@@ -24,6 +24,7 @@ def find_all_of_model(model:)
 
     # (see Valkyrie::Persistence::Memory::QueryService#find_by)
     def find_by(id:)
+      validate_id(id)
       resource_factory.to_resource(object: orm_class.find(id))
     rescue ActiveRecord::RecordNotFound
       raise Valkyrie::Persistence::ObjectNotFoundError
@@ -99,5 +100,11 @@ def find_references_query
     def custom_queries
       @custom_queries ||= ::Valkyrie::Persistence::CustomQueryContainer.new(query_service: self)
     end
+
+    private
+
+      def validate_id(id)
+        raise ArgumentError, 'id must be a Valkyrie::ID' unless id.is_a? Valkyrie::ID
+      end
   end
 end

valkyrie/lib/valkyrie/persistence/solr/query_service.rb

@@ -12,6 +12,7 @@ def initialize(connection:, resource_factory:)
 
     # (see Valkyrie::Persistence::Memory::QueryService#find_by)
     def find_by(id:)
+      validate_id(id)
       Valkyrie::Persistence::Solr::Queries::FindByIdQuery.new(id, connection: connection, resource_factory: resource_factory).run
     end
 
@@ -48,5 +49,11 @@ def find_inverse_references_by(resource:, property:)
     def custom_queries
       @custom_queries ||= ::Valkyrie::Persistence::CustomQueryContainer.new(query_service: self)
     end
+
+    private
+
+      def validate_id(id)
+        raise ArgumentError, 'id must be a Valkyrie::ID' unless id.is_a? Valkyrie::ID
+      end
   end
 end

valkyrie/lib/valkyrie/specs/shared_specs/queries.rb

@@ -57,8 +57,13 @@ class SecondResource < Valkyrie::Resource
 
       expect(query_service.find_by(id: resource.id).id).to eq resource.id
     end
+
     it "returns a Valkyrie::Persistence::ObjectNotFoundError for a non-found ID" do
-      expect { query_service.find_by(id: "123123123") }.to raise_error ::Valkyrie::Persistence::ObjectNotFoundError
+      expect { query_service.find_by(id: Valkyrie::ID.new("123123123")) }.to raise_error ::Valkyrie::Persistence::ObjectNotFoundError
+    end
+
+    it 'raises an error if the id is not a Valkyrie::ID' do
+      expect { query_service.find_by(id: "123123123") }.to raise_error ArgumentError
     end
   end