Add find_all_with to batch Group members lookup and eliminate N+1 in PATCH

app/models/scimitar/resources/mixin.rb

@@ -145,7 +145,8 @@ module Resources
     #             display: :full_name  # <-- i.e. Team.users[n].full_name
     #           },
     #           class: Team, # Optional; see below
-    #           find_with: -> (scim_list_entry) {...} # See below
+    #           find_with:      -> (scim_list_entry) {...}, # See below
+    #           find_all_with:  -> (scim_list_entries) {...} # Optional, See below
     #         }
     #       ],
     #       #...
@@ -165,6 +166,11 @@ module Resources
     # Scimitar::EngineConfiguration::schema_list_from_attribute_mappings is
     # defined; see documentation of that option for more information.
     #
+    # To avoid N+1 queries when resolving many entries (e.g. Group members
+    # during PATCH), you can instead provide ":find_all_with" which is passed
+    # the entire Array of SCIM entries and should return an Array of resolved
+    # model instances. If both are provided, ":find_all_with" is preferred.
+    #
     # Note that you can only use either:
     #
     # * One or more static maps where each matches some other piece of source
@@ -315,7 +321,7 @@ def scim_mutable_attributes
                     enum.each do | static_or_dynamic_mapping |
                       if static_or_dynamic_mapping.key?(:match) # Static
                         extractor.call(static_or_dynamic_mapping[:using])
-                      elsif static_or_dynamic_mapping.key?(:find_with) # Dynamic
+                      elsif static_or_dynamic_mapping.key?(:find_with) || static_or_dynamic_mapping.key?(:find_all_with) # Dynamic
                         @scim_mutable_attributes << static_or_dynamic_mapping[:list]
                       end
                     end
@@ -839,9 +845,17 @@ def from_scim_backend!(
                     method    = "#{mapped_array_entry[:list]}="
 
                     if (attribute&.mutability == 'readWrite' || attribute&.mutability == 'writeOnly') && self.respond_to?(method)
-                      find_with_proc = mapped_array_entry[:find_with]
+                      find_all_with_proc = mapped_array_entry[:find_all_with]
+                      find_with_proc     = mapped_array_entry[:find_with]
+
+                      if find_all_with_proc.respond_to?(:call)
+                        scim_entries = (scim_hash_or_leaf_value || [])
+                        mapped_list  = find_all_with_proc.call(scim_entries) || []
 
-                      unless find_with_proc.nil?
+                        mapped_list.compact!
+
+                        self.public_send(method, mapped_list)
+                      elsif find_with_proc.respond_to?(:call)
                         mapped_list = (scim_hash_or_leaf_value || []).map do | source_list_entry |
                           find_with_proc.call(source_list_entry)
                         end

spec/apps/dummy/app/controllers/mock_batch_groups_controller.rb

@@ -0,0 +1,13 @@
+class MockBatchGroupsController < Scimitar::ActiveRecordBackedResourcesController
+
+  protected
+
+    def storage_class
+      MockGroupBatch
+    end
+
+    def storage_scope
+      MockGroupBatch.all
+    end
+
+end

spec/apps/dummy/app/models/mock_group_batch.rb

@@ -0,0 +1,20 @@
+class MockGroupBatch < MockGroup
+  def self.scim_attributes_map
+    {
+      id:          :id,
+      externalId:  :scim_uid,
+      displayName: :display_name,
+      members:     [
+        {
+          list:  :scim_users_and_groups,
+          using: { value: :id },
+          # Minimal mock: assume user-only entries (type omitted => User)
+          find_all_with: -> (entries) do
+            ids = entries.map { |e| e['value'] }
+            users = MockUser.where(primary_key: ids).to_a
+          end
+        }
+      ]
+    }
+  end
+end

spec/apps/dummy/config/routes.rb

@@ -17,6 +17,11 @@
   get    'Groups/:id', to: 'mock_groups#show'
   patch  'Groups/:id', to: 'mock_groups#update'
 
+  # Batch lookup variant for testing find_all_with
+  get    'BatchGroups',     to: 'mock_batch_groups#index'
+  get    'BatchGroups/:id', to: 'mock_batch_groups#show'
+  patch  'BatchGroups/:id', to: 'mock_batch_groups#update'
+
   # For testing blocks passed to ActiveRecordBackedResourcesController#create,
   # #update, #replace and #destroy.
   #

spec/requests/active_record_backed_resources_controller_spec.rb

@@ -840,6 +840,36 @@
 
   context '#update' do
     shared_examples 'an updater' do | force_upper_case: |
+    context "when updating group members using :find_all_with" do
+      it "uses :find_all_with to batch-resolve users and updates associations" do
+        payload = {
+          schemas: [ 'urn:ietf:params:scim:api:messages:2.0:PatchOp' ],
+          Operations: [
+            {
+              op:   'add',
+              path: 'members',
+              value: [
+                { 'value' => @u1.primary_key },
+                { 'value' => @u2.primary_key }
+              ]
+            }
+          ]
+        }
+
+        patch "/BatchGroups/#{@g1.id}", params: payload.merge({ format: :scim })
+
+        expect(response.status).to eql(200)
+
+        # Verify membership updated
+        get "/BatchGroups/#{@g1.id}", params: { format: :scim }
+        expect(response.status).to eql(200)
+        result = JSON.parse(response.body)
+
+        values = result.fetch('members', []).map { |m| m['value'] }
+        expect(values).to include(@u1.primary_key.to_s)
+        expect(values).to include(@u2.primary_key.to_s)
+      end
+    end
       it 'which patches regular attributes' do
         payload = {
           Operations: [