wings: use AttributesTransformer to populate ordered reflections

we had previously manually populated valkyrie Resource `#member_ids` from
ActiveFedora's `#ordered_member_ids` when it was present. the implementation had
four problems:

  - it accessed `#member_ids` on objects for which it was defined as a
  `HasManyReflection`, causing the tranformation to load all the members(!!);
  - it failed to retain unordered `#member_ids` if any `#ordered_member_ids`
  were present. in Hydra::Works, it's possible to have both.
  - it ignored `OrdersReflections` other than `#ordered_member_ids`;
  - it repeatedly loaded the `#member_ids`, since `AttributeTransformer` already
  handled the unordered mapping via `IndirectlyContainsReflection`.

this should solve all three problems by acting on `OrdersReflection` as well the
various `*ContainsReflection` instances. if an `OrdersReflection` is present the
resulting ids will contain the relevant values in order, either before
or after the unordered values (which may, most often, be empty).

---

after this change:

1        0.032793   0.006267   0.039060 (  0.047836)
10       0.057116   0.003111   0.060227 (  0.067870)
100      0.044683   0.003256   0.047939 (  0.057653)
1_000    0.102635   0.000026   0.102661 (  0.120755)
10_000

lib/wings/attribute_transformer.rb

@@ -15,29 +15,36 @@ def self.for(obj)
       end
     end
 
-    def self.run(obj) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength
+    def self.run(obj) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity, Metrics/BlockLength
       attrs = obj.reflections.each_with_object({}) do |(key, reflection), mem|
         case reflection
-        when ActiveFedora::Reflection::HasManyReflection,
-             ActiveFedora::Reflection::HasAndBelongsToManyReflection,
-             ActiveFedora::Reflection::IndirectlyContainsReflection
-          mem[:"#{key.to_s.singularize}_ids"] =
-            obj.association(key).ids_reader
-        when ActiveFedora::Reflection::DirectlyContainsReflection
-          mem[:"#{key.to_s.singularize}_ids"] =
-            Array(obj.public_send(reflection.name)).map(&:id)
-        when ActiveFedora::Reflection::FilterReflection,
-             ActiveFedora::Reflection::OrdersReflection,
-             ActiveFedora::Reflection::HasSubresourceReflection,
-             ActiveFedora::Reflection::BelongsToReflection,
-             ActiveFedora::Reflection::BasicContainsReflection
+        when ActiveFedora::Reflection::BelongsToReflection, # uses foreign_key SingularRDFPropertyReflection
+             ActiveFedora::Reflection::BasicContainsReflection, # ???
+             ActiveFedora::Reflection::FilterReflection, # rely on :extending_from
+             ActiveFedora::Reflection::HasAndBelongsToManyReflection, # uses foreign_key RDFPropertyReflection
+             ActiveFedora::Reflection::HasManyReflection, # captured by inverse relation
+             ActiveFedora::Reflection::HasSubresourceReflection, # ???
           :noop
+        when ActiveFedora::Reflection::OrdersReflection
+          mem[:"#{reflection.options[:unordered_reflection].name.to_s.singularize}_ids"] ||= []
+          mem[:"#{reflection.options[:unordered_reflection].name.to_s.singularize}_ids"] +=
+            obj.association(reflection.name).target_ids_reader
         when ActiveFedora::Reflection::DirectlyContainsOneReflection
           mem[:"#{key.to_s.singularize}_id"] =
             obj.public_send(reflection.name)&.id
+        when ActiveFedora::Reflection::IndirectlyContainsReflection
+          mem[:"#{key.to_s.singularize}_ids"] ||= []
+          mem[:"#{key.to_s.singularize}_ids"] +=
+            obj.association(key).ids_reader
+        when ActiveFedora::Reflection::DirectlyContainsReflection
+          mem[:"#{key.to_s.singularize}_ids"] ||= []
+          mem[:"#{key.to_s.singularize}_ids"] +=
+            Array(obj.public_send(reflection.name)).map(&:id)
+        when ActiveFedora::Reflection::RDFPropertyReflection
+          mem[reflection.name.to_sym] =
+            obj.public_send(reflection.name.to_sym)
         else
-          mem[reflection.foreign_key.to_sym] =
-            obj.public_send(reflection.foreign_key.to_sym)
+          raise NotImplementedError, "Expected a known ActiveFedora::Reflection, but got #{reflection}"
         end
       end
 

lib/wings/model_transformer.rb

@@ -150,17 +150,9 @@ def additional_attributes
       { :id => pcdm_object.id,
         :created_at => pcdm_object.try(:create_date),
         :updated_at => pcdm_object.try(:modified_date),
-        :member_ids => member_ids,
         ::Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK => lock_token }
     end
 
-    # Prefer ordered members, but if ordered members don't exist, use non-ordered members.
-    def member_ids
-      ordered_member_ids = pcdm_object.try(:ordered_member_ids)
-      return ordered_member_ids if ordered_member_ids.present?
-      pcdm_object.try(:member_ids)
-    end
-
     def lock_token
       result = []
       result << ::Valkyrie::Persistence::OptimisticLockToken.new(adapter_id: 'wings-fedora-etag', token: pcdm_object.etag) unless pcdm_object.new_record?

lib/wings/orm_converter.rb

@@ -81,12 +81,12 @@ def to_model
         (klass.try(:reflections) || []).each do |reflection_key, reflection|
           case reflection
           when ActiveFedora::Reflection::BelongsToReflection, # uses foreign_key SingularRDFPropertyReflection
-               ActiveFedora::Reflection::BasicContainsReflection, #???
+               ActiveFedora::Reflection::BasicContainsReflection, # ???
                ActiveFedora::Reflection::FilterReflection, # rely on :extending_from
                ActiveFedora::Reflection::HasAndBelongsToManyReflection, # uses foreign_key RDFPropertyReflection
                ActiveFedora::Reflection::HasManyReflection, # captured by inverse relation
                ActiveFedora::Reflection::HasSubresourceReflection, # ???
-               ActiveFedora::Reflection::OrdersReflection # depend on unordered association (refactor to do opposite?)
+               ActiveFedora::Reflection::OrdersReflection # map to :unordered_association in Wings::AttributeTransformer (but retain order)
             next
           when ActiveFedora::Reflection::DirectlyContainsOneReflection
             attribute_name = (reflection_key.to_s.singularize + '_id').to_sym
@@ -108,7 +108,6 @@ def to_model
           attribute(attribute_name, type)
         end
 
-
         def internal_resource
           self.class.internal_resource
         end

spec/wings/model_transformer_spec.rb

@@ -219,6 +219,13 @@ class Book < Hyrax::Resource
           work.members << child_work2
         end
 
+        it 'includes ordered_members too' do
+          ordered_members = [FactoryBot.create(:work), FactoryBot.create(:work)]
+          ordered_members.each { |w| work.ordered_members << w }
+
+          expect(factory.build.member_ids).to include(*['cw1', 'cw2'] + ordered_members.map(&:id))
+        end
+
         it 'sets member_ids to the ids of the unordered members' do
           expect(subject.build.member_ids).to match_valkyrie_ids_with_active_fedora_ids(['cw1', 'cw2'])
         end
@@ -380,7 +387,9 @@ class Book < Hyrax::Resource
     let(:pcdm_object) { AdminSet.create(title: ['my admin set']) }
 
     before do
-      5.times { |i| GenericWork.create(title: ["#{i}"], admin_set_id: pcdm_object.id) }
+      5.times do |i|
+        GenericWork.create(title: ["#{i}"], admin_set_id: pcdm_object.id)
+      end
     end
 
     it 'does not have member_ids' do