Merge pull request #572 from samvera-labs/issue/551-ordered-references

Queries on ordered properties should return results in order

lib/valkyrie/persistence/postgres/query_service.rb

@@ -75,7 +75,12 @@ def find_parents(resource:)
     # (see Valkyrie::Persistence::Memory::QueryService#find_references_by)
     def find_references_by(resource:, property:)
       return [] if resource.id.blank? || resource[property].blank?
-      run_query(find_references_query, property, resource.id.to_s)
+      # only return ordered if needed to avoid performance penalties
+      if ordered_property?(resource: resource, property: property)
+        run_query(find_ordered_references_query, property, resource.id.to_s)
+      else
+        run_query(find_references_query, property, resource.id.to_s)
+      end
     end
 
     # (see Valkyrie::Persistence::Memory::QueryService#find_inverse_references_by)
@@ -125,6 +130,15 @@ def find_references_query
       SQL
     end
 
+    def find_ordered_references_query
+      <<-SQL
+        SELECT member.* FROM orm_resources a,
+        jsonb_array_elements(a.metadata->?) WITH ORDINALITY AS b(member, member_pos)
+        JOIN orm_resources member ON (b.member->>'id')::#{id_type} = member.id WHERE a.id = ?
+        ORDER BY b.member_pos
+      SQL
+    end
+
     def custom_queries
       @custom_queries ||= ::Valkyrie::Persistence::CustomQueryContainer.new(query_service: self)
     end
@@ -142,5 +156,9 @@ def ensure_persisted(resource)
       def id_type
         @id_type ||= orm_class.columns_hash["id"].type
       end
+
+      def ordered_property?(resource:, property:)
+        resource.class.schema[property].meta.try(:[], :ordered)
+      end
   end
 end

lib/valkyrie/persistence/solr/queries.rb

@@ -11,5 +11,6 @@ module Queries
     require 'valkyrie/persistence/solr/queries/find_inverse_references_query'
     require 'valkyrie/persistence/solr/queries/find_members_query'
     require 'valkyrie/persistence/solr/queries/find_references_query'
+    require 'valkyrie/persistence/solr/queries/find_ordered_references_query'
   end
 end

lib/valkyrie/persistence/solr/queries/find_ordered_references_query.rb

@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+module Valkyrie::Persistence::Solr::Queries
+  # Responsible for returning all {Valkyrie::Resource}s which are referenced in
+  # a given {Valkyrie::Resource}'s property, in the order given in that property.
+  class FindOrderedReferencesQuery
+    attr_reader :resource, :property, :connection, :resource_factory
+    def initialize(resource:, property:, connection:, resource_factory:)
+      @resource = resource
+      @property = property
+      @connection = connection
+      @resource_factory = resource_factory
+    end
+
+    def run
+      enum_for(:each)
+    end
+
+    def each
+      # map them off of the property to fix solr's deduplication
+      property_values.map { |id| unordered_members.find { |member| member.id == id } } .each do |value|
+        yield value
+      end
+    end
+
+    def unordered_members
+      docs.map do |doc|
+        resource_factory.to_resource(object: doc)
+      end
+    end
+
+    def docs
+      options = { q: query, rows: 1_000_000_000 }
+      options[:defType] = 'lucene'
+      result = connection.get("select", params: options)
+      result.fetch('response').fetch('docs')
+    end
+
+    def property_values
+      Array.wrap(resource[property])
+    end
+
+    def query
+      "{!join from=#{property}_ssim to=join_id_ssi}id:#{id}"
+    end
+
+    def id
+      resource.id.to_s
+    end
+  end
+end

lib/valkyrie/persistence/solr/query_service.rb

@@ -57,7 +57,11 @@ def find_members(resource:, model: nil)
 
     # (see Valkyrie::Persistence::Memory::QueryService#find_references_by)
     def find_references_by(resource:, property:)
-      Valkyrie::Persistence::Solr::Queries::FindReferencesQuery.new(resource: resource, property: property, connection: connection, resource_factory: resource_factory).run
+      if ordered_property?(resource: resource, property: property)
+        Valkyrie::Persistence::Solr::Queries::FindOrderedReferencesQuery.new(resource: resource, property: property, connection: connection, resource_factory: resource_factory).run
+      else
+        Valkyrie::Persistence::Solr::Queries::FindReferencesQuery.new(resource: resource, property: property, connection: connection, resource_factory: resource_factory).run
+      end
     end
 
     # (see Valkyrie::Persistence::Memory::QueryService#find_inverse_references_by)
@@ -79,5 +83,9 @@ def validate_id(id)
       def ensure_persisted(resource)
         raise ArgumentError, 'resource is not saved' unless resource.persisted?
       end
+
+      def ordered_property?(resource:, property:)
+        resource.class.schema[property].meta.try(:[], :ordered)
+      end
   end
 end

lib/valkyrie/specs/shared_specs/queries.rb

@@ -8,6 +8,7 @@ class CustomResource < Valkyrie::Resource
       attribute :title
       attribute :member_ids, Valkyrie::Types::Array
       attribute :a_member_of
+      attribute :an_ordered_member_of, Valkyrie::Types::Array.meta(ordered: true)
     end
     class SecondResource < Valkyrie::Resource
     end
@@ -217,6 +218,17 @@ class SecondResource < Valkyrie::Resource
       child = persister.save(resource: resource_class.new)
       expect(query_service.find_references_by(resource: child, property: :a_member_of).to_a).to eq []
     end
+
+    context "when the property is ordered" do
+      it "returns all references in order" do
+        parent = persister.save(resource: resource_class.new)
+        parent2 = persister.save(resource: resource_class.new)
+        child = persister.save(resource: resource_class.new(an_ordered_member_of: [parent.id, parent2.id, parent.id]))
+        persister.save(resource: resource_class.new)
+
+        expect(query_service.find_references_by(resource: child, property: :an_ordered_member_of).map(&:id).to_a).to eq [parent.id, parent2.id, parent.id]
+      end
+    end
   end
 
   describe ".find_inverse_references_by" do