Ensure has_one artificially limits (#91)

We don't want to limit a has_one. If a Post has_one Detail, and we were
listing all posts and sideloading their detail, only one *total* detail
would be in the response.

So we can't paginate, and instead we assign only the relevant children.
The work done here is to remove the excessive results from the array.
This is important because subsequent sideloads will scope to that array.

To make this work, we ensure the `assign` block is fired before
processing further sideloads, which makes better mental sense in any
case.

See also: rails/rails#10621

Author: richmolj

lib/jsonapi_compliable/adapters/active_record_sideloading.rb

@@ -53,7 +53,15 @@ def has_one(association_name, scope: nil, resource:, foreign_key:, primary_key:
             _scope.call.where(foreign_key => parent_ids.uniq.compact)
           end
 
+          # The 'assigned' code here is to remove all children that do not
+          # get assigned. This is because there is no 'limit(1)' in the query.
+          # If we did 'limit(1)' for the query, it wouldn't work for index
+          # actions (only 1 would come back, when we want one *per result*).
+          #
+          # Instead, avoid pagination in the query, assign only one result, and
+          # remove anything else. This is more or less what AR does.
           assign do |parents, children|
+            assigned = []
             parents.each do |parent|
               parent.association(association_name).loaded!
               relevant_child = children.find { |c| c.send(foreign_key) == parent.send(primary_key) }
@@ -65,6 +73,10 @@ def has_one(association_name, scope: nil, resource:, foreign_key:, primary_key:
               association.send(:set_owner_attributes, relevant_child)
               association.send(:set_inverse_instance, relevant_child)
               association.send(:target=, relevant_child)
+              assigned << relevant_child
+            end
+            (children - assigned).each do |unassigned|
+              children.delete(unassigned)
             end
           end
 

lib/jsonapi_compliable/scope.rb

@@ -64,6 +64,7 @@ def resolve
         []
       else
         resolved = @resource.resolve(@object)
+        yield resolved if block_given?
         sideload(resolved, query_hash[:include]) if query_hash[:include]
         resolved
       end

lib/jsonapi_compliable/sideload.rb

@@ -399,8 +399,9 @@ def resolve_polymorphic(parents, query)
     def resolve_basic(parents, query, namespace)
       sideload_scope   = scope_proc.call(parents)
       sideload_scope   = Scope.new(sideload_scope, resource_class.new, query, default_paginate: false, namespace: namespace)
-      sideload_results = sideload_scope.resolve
-      assign_proc.call(parents, sideload_results)
+      sideload_scope.resolve do |sideload_results|
+        assign_proc.call(parents, sideload_results)
+      end
     end
   end
 end

spec/fixtures/legacy.rb

@@ -40,6 +40,10 @@
     t.string :picture
   end
 
+  create_table :bio_labels do |t|
+    t.integer :bio_id
+  end
+
   create_table :genres do |t|
     t.string :name
     t.timestamps
@@ -111,6 +115,11 @@ class Hobby < LegacyApplicationRecord
 
 class Bio < LegacyApplicationRecord
   belongs_to :author
+  has_many :bio_labels
+end
+
+class BioLabel < LegacyApplicationRecord
+  belongs_to :bio
 end
 
 class Genre < LegacyApplicationRecord
@@ -213,6 +222,12 @@ class SerializableBio < LegacySerializableAbstract
   extra_attribute :created_at do
     Time.now
   end
+
+  has_many :bio_labels
+end
+
+class SerializableBioLabel < LegacySerializableAbstract
+  type 'bio_labels'
 end
 
 class SerializableState < LegacySerializableAbstract

spec/integration/rails/finders_spec.rb

@@ -34,9 +34,27 @@ class DwellingResource < JsonapiCompliable::Resource
           resource: StateResource
       end
 
+      class BioLabelResource < JsonapiCompliable::Resource
+        type :bio_labels
+        use_adapter JsonapiCompliable::Adapters::ActiveRecord
+      end
+
       class BioResource < JsonapiCompliable::Resource
         type :bios
         use_adapter JsonapiCompliable::Adapters::ActiveRecord
+
+        has_many :bio_labels,
+          foreign_key: :bio_id,
+          scope: -> { BioLabel.all },
+          resource: BioLabelResource do
+            # Ensure if we get too many bios/labels, they
+            # will still come back in the response.
+            assign do |bios, labels|
+              bios.each do |b|
+                b.bio_labels = labels
+              end
+            end
+          end
       end
 
       class HobbyResource < JsonapiCompliable::Resource
@@ -267,6 +285,23 @@ def json
         expect(bio).to have_key('created_at')
         expect(bio).to_not have_key('picture')
       end
+
+      # Model/Resource has has_one, but it's just a subset of a has_many
+      context 'when multiple records (faux-has_one)' do
+        let!(:bio2) { Bio.create!(author: author1, picture: 'imgur', description: 'author bio') }
+
+        context 'and there is another level of association' do
+          before do
+            bio.bio_labels << BioLabel.create!
+            bio2.bio_labels << BioLabel.create!
+          end
+
+          it 'still works' do
+            get :index, params: { include: 'bio.bio_labels' }
+            expect(json_includes('bio_labels').length).to eq(1)
+          end
+        end
+      end
     end
 
     context 'sideloading has_and_belongs_to_many' do

spec/sideload_spec.rb

@@ -77,7 +77,11 @@ def foo
       let(:results)    { [{ parent_id: 1 }] }
       let(:base_scope) { double }
       let(:scope_proc) { ->(parents) { base_scope } }
-      let(:scope)      { double(resolve: results) }
+      let(:scope) do
+        scope = double
+        allow(scope).to receive(:resolve).and_yield(results)
+        scope
+      end
 
       before do
         instance.scope  { |parents| base_scope }