Merge pull request cerebris#38 from cerebris/pr/35

Fix N+1 issue with show multiple action - slight changes

Author: dgeb

lib/jsonapi/resource.rb

@@ -261,6 +261,18 @@ def find_by_key(key, options = {})
         self.new(model, context)
       end
 
+      def find_by_keys(keys, options = {})
+        context = options[:context]
+        _models = _model_class.where({_primary_key => keys})
+
+        unless _models.length == keys.length
+          key = (keys - _models.pluck(:id).map(&:to_s)).first
+          raise JSONAPI::Exceptions::RecordNotFound.new(key)
+        end
+
+        _models.map { |model| self.new(model, context) }
+      end
+
       def verify_filters(filters, context = nil)
         verified_filters = {}
         filters.each do |filter, raw_value|

lib/jsonapi/resource_controller.rb

@@ -30,14 +30,11 @@ def index
     def show
       keys = parse_key_array(params[resource_klass._primary_key])
 
-      if keys.length > 1
-        resources = []
-        keys.each do |key|
-          resources.push(resource_klass.find_by_key(key, context: context))
-        end
-      else
-        resources = resource_klass.find_by_key(keys[0], context: context)
-      end
+      resources = if keys.length > 1
+                    resource_klass.find_by_keys(keys, context: context)
+                  else
+                    resource_klass.find_by_key(keys[0], context: context)
+                  end
 
       render json: JSONAPI::ResourceSerializer.new.serialize_to_hash(
           resources,

test/controllers/controller_test.rb

@@ -971,6 +971,12 @@ def test_tags_show_multiple_with_include
     assert_equal 4, json_response['tags'].size
     assert_equal 2, json_response['linked']['posts'].size
   end
+
+  def test_tags_show_multiple_with_nonexistent_ids
+    get :show, {id: '6,99,9,100'}
+    assert_response :not_found
+    assert_match /The record identified by 99 could not be found./, json_response['errors'][0]['detail']
+  end
 end
 
 class ExpenseEntriesControllerTest < ActionController::TestCase

test/fixtures/active_record.rb

@@ -432,6 +432,10 @@ def self.find(attrs, options = {})
   def self.find_by_key(id, options = {})
     BreedResource.new($breed_data.breeds[id.to_i], options[:context])
   end
+
+  def self.find_by_keys(keys, options = {})
+    keys.map { |key| self.find_by_key(key, options) }
+  end
 end
 
 class PlanetResource < JSONAPI::Resource