From e5584ad80048b6563dbeceeddf331eec28aaa29f Mon Sep 17 00:00:00 2001 From: mrloop Date: Wed, 14 Sep 2016 18:49:05 +0100 Subject: [PATCH 1/7] ::always_include additional includes for querying See #485 for background. --- lib/jsonapi/resource.rb | 72 +++++++++++++++++++++++++++-- test/unit/resource/resource_test.rb | 71 ++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 5 deletions(-) diff --git a/lib/jsonapi/resource.rb b/lib/jsonapi/resource.rb index 25697c536..f83beb2ef 100644 --- a/lib/jsonapi/resource.rb +++ b/lib/jsonapi/resource.rb @@ -461,7 +461,7 @@ def resource_type_for(model) end end - attr_accessor :_attributes, :_relationships, :_type, :_model_hints + attr_accessor :_attributes, :_relationships, :_type, :_model_hints, :always_includes attr_writer :_allowed_filters, :_paginator def create(context) @@ -572,6 +572,10 @@ def cache_field(field) @_cache_field = field.to_sym end + def always_include(*relationships) + _add_always_includes(*relationships) + end + # Override in your resource to filter the updatable keys def updatable_fields(_context = nil) _updatable_relationships | _attributes.keys - [:id] @@ -611,14 +615,64 @@ def resolve_relationship_names_to_relations(resource_klass, model_includes, opti end end - def apply_includes(records, options = {}) + def _append_always_includes(resource_klass, model_include, options) + if resource_klass.present? + resource_klass.always_includes + model_include + else + model_include + end + end + + def relationship_klass_from(resource_klass, key) + reflection = resource_klass._model_class._reflections[key.to_s] + if reflection.present? + relationship_klass = resource_klass.resource_for(reflection.class_name) + else + nil + end + end + + def resolve_always_includes(resource_klass, model_includes, options = {}) + case model_includes + when Array + model_includes.uniq.map do |value| + resolve_always_includes(resource_klass, value, options) + end + when Hash + Hash[model_includes.map do |key, value| + relationship_klass = relationship_klass_from(resource_klass, key) + if relationship_klass.present? + [key, resolve_always_includes(relationship_klass, _append_always_includes(relationship_klass, value, options), options)] + end + end.compact] + when Symbol + relationship_klass = relationship_klass_from(resource_klass, model_includes.to_s) + if relationship_klass.present? && relationship_klass.always_includes.present? + { model_includes => resolve_always_includes(relationship_klass, relationship_klass.always_includes, options)} + else + model_includes + end + end + end + + def model_includes(options) include_directives = options[:include_directives] if include_directives - model_includes = resolve_relationship_names_to_relations(self, include_directives.model_includes, options) - records = records.includes(model_includes) + return resolve_relationship_names_to_relations(self, include_directives.model_includes, options) end + [] + end - records + def get_includes(options = {}) + model_includes = model_includes(options) + first_level_includes = model_includes.map{ |inc| inc.try(:keys) || inc }.flatten + includes = model_includes + ( always_includes - first_level_includes ) + + resolve_always_includes(self, includes, options) + end + + def apply_includes(records, options = {}) + records.includes(get_includes(options)) end def apply_pagination(records, paginator, order_options) @@ -1015,6 +1069,14 @@ def _add_relationship(klass, *attrs) end end + def always_includes + @always_includes ||= [] + end + + def _add_always_includes(*relationships) + always_includes.concat(Array(relationships)) + end + # Allows JSONAPI::RelationshipBuilder to access metaprogramming hooks def inject_method_definition(name, body) define_method(name, body) diff --git a/test/unit/resource/resource_test.rb b/test/unit/resource/resource_test.rb index 2fd8b68f8..f631af4f2 100644 --- a/test/unit/resource/resource_test.rb +++ b/test/unit/resource/resource_test.rb @@ -98,6 +98,77 @@ class RelatedResource < MyModule::RelatedResource end end +module Ai + class PersonResource < ::PersonResource + always_include :hair_cut, :comments + end + + class CommentResource < ::CommentResource + end + + class TagResource < ::TagResource + always_include :posts + end + + class PostResource < ::PostResource + end + + class VehicleResource < ::VehicleResource + end + + class HairCutResource < ::HairCutResource + end + + class SectionResource < ::SectionResource + end +end + +class AlwaysIncludeTest < ActiveSupport::TestCase + + def test_always_includes + assert_equal([:hair_cut, :comments], Ai::PersonResource.always_includes) + end + + def test_resolve_always_includes + result = JSONAPI::Resource.resolve_always_includes(Ai::PersonResource, [:vehicles, :comments]) + assert_equal([:vehicles, :comments], result) + end + + def test_resolve_always_includes_nested + expected = {comments: [{tags: [:posts]}]} + + result = JSONAPI::Resource.resolve_always_includes(Ai::PersonResource, comments: [:tags]) + assert_equal(expected, result) + + result = JSONAPI::Resource.resolve_always_includes(Ai::PersonResource, comments: [{tags: [:posts]}]) + assert_equal(expected, result) + end + + def test_get_includes + include_directive = JSONAPI::IncludeDirectives.new(Ai::PersonResource, ['comments']) + expected = [:comments, :hair_cut] + + result = Ai::PersonResource.get_includes(include_directives: include_directive) + assert_equal(expected, result) + end + + def test_get_includes_nested + include_directive = JSONAPI::IncludeDirectives.new(Ai::PersonResource, ['comments.tags']) + expected = [{comments: [{tags: [:posts]}]}, :hair_cut] + + result = Ai::PersonResource.get_includes(include_directives: include_directive) + assert_equal(expected, result) + end + + def test_get_includes_nested_more_complex + include_directive = JSONAPI::IncludeDirectives.new(Ai::PersonResource, ['comments.tags', 'posts.section']) + expected = [{comments: [{tags: [:posts]}]}, {posts: [:section]}, :hair_cut] + + result = Ai::PersonResource.get_includes(include_directives: include_directive) + assert_equal(expected, result) + end +end + class ResourceTest < ActiveSupport::TestCase def setup @post = Post.first From c57bfbbd0fc70e0ce1d0944d42fde5499af1bf4d Mon Sep 17 00:00:00 2001 From: mrloop Date: Thu, 15 Sep 2016 14:51:03 +0100 Subject: [PATCH 2/7] ::always_include circular reference protection --- lib/jsonapi/resource.rb | 12 +++--- test/unit/resource/resource_test.rb | 63 ++++++++++++++++++++--------- 2 files changed, 50 insertions(+), 25 deletions(-) diff --git a/lib/jsonapi/resource.rb b/lib/jsonapi/resource.rb index f83beb2ef..c94ef5c75 100644 --- a/lib/jsonapi/resource.rb +++ b/lib/jsonapi/resource.rb @@ -632,23 +632,24 @@ def relationship_klass_from(resource_klass, key) end end - def resolve_always_includes(resource_klass, model_includes, options = {}) + def resolve_always_includes(resource_klass, model_includes, options = {}, seen = []) case model_includes when Array model_includes.uniq.map do |value| - resolve_always_includes(resource_klass, value, options) + resolve_always_includes(resource_klass, value, options, seen) end when Hash Hash[model_includes.map do |key, value| relationship_klass = relationship_klass_from(resource_klass, key) if relationship_klass.present? - [key, resolve_always_includes(relationship_klass, _append_always_includes(relationship_klass, value, options), options)] + [key, resolve_always_includes(relationship_klass, _append_always_includes(relationship_klass, value, options), options, seen)] end end.compact] when Symbol relationship_klass = relationship_klass_from(resource_klass, model_includes.to_s) - if relationship_klass.present? && relationship_klass.always_includes.present? - { model_includes => resolve_always_includes(relationship_klass, relationship_klass.always_includes, options)} + if relationship_klass.present? && relationship_klass.always_includes.present? && !seen.include?(model_includes) + seen.push(model_includes) + { model_includes => resolve_always_includes(relationship_klass, relationship_klass.always_includes, options, seen)} else model_includes end @@ -667,7 +668,6 @@ def get_includes(options = {}) model_includes = model_includes(options) first_level_includes = model_includes.map{ |inc| inc.try(:keys) || inc }.flatten includes = model_includes + ( always_includes - first_level_includes ) - resolve_always_includes(self, includes, options) end diff --git a/test/unit/resource/resource_test.rb b/test/unit/resource/resource_test.rb index f631af4f2..9e9126c2b 100644 --- a/test/unit/resource/resource_test.rb +++ b/test/unit/resource/resource_test.rb @@ -98,32 +98,31 @@ class RelatedResource < MyModule::RelatedResource end end -module Ai - class PersonResource < ::PersonResource - always_include :hair_cut, :comments - end +class AlwaysIncludeTest < ActiveSupport::TestCase + module Ai + class PersonResource < ::PersonResource + always_include :hair_cut, :comments + end - class CommentResource < ::CommentResource - end + class CommentResource < ::CommentResource + end - class TagResource < ::TagResource - always_include :posts - end + class TagResource < ::TagResource + always_include :posts + end - class PostResource < ::PostResource - end + class PostResource < ::PostResource + end - class VehicleResource < ::VehicleResource - end + class VehicleResource < ::VehicleResource + end - class HairCutResource < ::HairCutResource - end + class HairCutResource < ::HairCutResource + end - class SectionResource < ::SectionResource + class SectionResource < ::SectionResource + end end -end - -class AlwaysIncludeTest < ActiveSupport::TestCase def test_always_includes assert_equal([:hair_cut, :comments], Ai::PersonResource.always_includes) @@ -169,6 +168,32 @@ def test_get_includes_nested_more_complex end end +class AlwayIncludeCircularTest < ActiveSupport::TestCase + module Circular + class PersonResource < ::PersonResource + always_include :comments + end + + class CommentResource < ::CommentResource + always_include :tags + end + + class TagResource < ::TagResource + always_include :posts + end + + class PostResource < ::PostResource + always_include :comments + end + end + + def test_always_includes_circular + expected = [{comments: [{tags: [{posts: [:comments]}]}]}] + result = JSONAPI::Resource.resolve_always_includes(Circular::PersonResource, [:comments]) + assert_equal(expected, result) + end +end + class ResourceTest < ActiveSupport::TestCase def setup @post = Post.first From b3cfd8790f32448815cb03aa41b6ad8f49f42b14 Mon Sep 17 00:00:00 2001 From: mrloop Date: Thu, 15 Sep 2016 18:00:49 +0100 Subject: [PATCH 3/7] ::always_include use symbol if no resource if no resource for specified always_include presume it is an existing model with no associated resource and use the symbol. --- lib/jsonapi/resource.rb | 2 ++ test/unit/resource/resource_test.rb | 18 ++++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/jsonapi/resource.rb b/lib/jsonapi/resource.rb index c94ef5c75..8f7c4249e 100644 --- a/lib/jsonapi/resource.rb +++ b/lib/jsonapi/resource.rb @@ -630,6 +630,8 @@ def relationship_klass_from(resource_klass, key) else nil end + rescue NameError => ex + nil end def resolve_always_includes(resource_klass, model_includes, options = {}, seen = []) diff --git a/test/unit/resource/resource_test.rb b/test/unit/resource/resource_test.rb index 9e9126c2b..830023846 100644 --- a/test/unit/resource/resource_test.rb +++ b/test/unit/resource/resource_test.rb @@ -98,7 +98,7 @@ class RelatedResource < MyModule::RelatedResource end end -class AlwaysIncludeTest < ActiveSupport::TestCase +class AlwaysIncludeTests < ActiveSupport::TestCase module Ai class PersonResource < ::PersonResource always_include :hair_cut, :comments @@ -121,6 +121,7 @@ class HairCutResource < ::HairCutResource end class SectionResource < ::SectionResource + end end @@ -168,7 +169,7 @@ def test_get_includes_nested_more_complex end end -class AlwayIncludeCircularTest < ActiveSupport::TestCase +class AlwayIncludeCircularTests < ActiveSupport::TestCase module Circular class PersonResource < ::PersonResource always_include :comments @@ -194,6 +195,19 @@ def test_always_includes_circular end end +class AlwaysIncludeNoResourceTest < ActiveSupport::TestCase + module NoResource + class MoonResource < ::MoonResource + always_include :craters + end + end + + def test_always_include_no_resource_no_error_raised + result = JSONAPI::Resource.resolve_always_includes(NoResource::MoonResource, [:craters]) + assert_equal([:craters], result) + end +end + class ResourceTest < ActiveSupport::TestCase def setup @post = Post.first From a2c132e28f99676879735b46d9149e1e3b554998 Mon Sep 17 00:00:00 2001 From: mrloop Date: Tue, 20 Sep 2016 16:25:49 +0100 Subject: [PATCH 4/7] ::always_include reduce related resource includes only use related type includes when loading related resource to reduce the number of unused includes. --- lib/jsonapi/processor.rb | 4 ++-- lib/jsonapi/resource.rb | 11 ++++++++++- test/unit/resource/resource_test.rb | 8 ++++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/lib/jsonapi/processor.rb b/lib/jsonapi/processor.rb index 82566568a..e97f29da5 100644 --- a/lib/jsonapi/processor.rb +++ b/lib/jsonapi/processor.rb @@ -148,7 +148,7 @@ def show_related_resource fields = params[:fields] # TODO Should fetch related_resource from cache if caching enabled - source_resource = source_klass.find_by_key(source_id, context: context, fields: fields) + source_resource = source_klass.find_by_key(source_id, context: context, fields: fields, relationship_type: relationship_type) related_resource = source_resource.public_send(relationship_type) @@ -165,7 +165,7 @@ def show_related_resources fields = params[:fields] include_directives = params[:include_directives] - source_resource ||= source_klass.find_by_key(source_id, context: context, fields: fields) + source_resource ||= source_klass.find_by_key(source_id, context: context, fields: fields, relationship_type: relationship_type) rel_opts = { filters: filters, diff --git a/lib/jsonapi/resource.rb b/lib/jsonapi/resource.rb index 8f7c4249e..eb34bce80 100644 --- a/lib/jsonapi/resource.rb +++ b/lib/jsonapi/resource.rb @@ -670,7 +670,16 @@ def get_includes(options = {}) model_includes = model_includes(options) first_level_includes = model_includes.map{ |inc| inc.try(:keys) || inc }.flatten includes = model_includes + ( always_includes - first_level_includes ) - resolve_always_includes(self, includes, options) + includes = resolve_always_includes(self, includes, options) + filter_for_relationship_type(includes, options) + end + + def filter_for_relationship_type(includes, options) + if options[:relationship_type].present? + includes.select{|v| (v.try(:keys).try(:first) || v) == options[:relationship_type] } + else + includes + end end def apply_includes(records, options = {}) diff --git a/test/unit/resource/resource_test.rb b/test/unit/resource/resource_test.rb index 830023846..6cbd5c948 100644 --- a/test/unit/resource/resource_test.rb +++ b/test/unit/resource/resource_test.rb @@ -167,6 +167,14 @@ def test_get_includes_nested_more_complex result = Ai::PersonResource.get_includes(include_directives: include_directive) assert_equal(expected, result) end + + def test_get_includes_for_relationship + include_directive = JSONAPI::IncludeDirectives.new(Ai::PersonResource, ['comments.tags', 'posts.section']) + expected = [{comments: [{tags: [:posts]}]}] + + result = Ai::PersonResource.get_includes(include_directives: include_directive, relationship_type: :comments) + assert_equal(expected, result) + end end class AlwayIncludeCircularTests < ActiveSupport::TestCase From e1288409c36c6ce29cf089cabfdfa2b1f996d8aa Mon Sep 17 00:00:00 2001 From: mrloop Date: Wed, 21 Sep 2016 19:27:51 +0100 Subject: [PATCH 5/7] ::always_include use merge not recursive strategy The previous recursive strategy for constructing the Active Record includes argument could result in unused eager loads being performed. The new strategy normalizes and merges the model includes and always includes, then iterates over the model includes and normalizes and merges their always includes. The model and always includes can both be nested. For example the recursive strategy gave the following includes for the app I'm working on: [{:raw_show=>[{:events=>[:raw_show, {:run=>[{:show=>[:events, :photos]}]}, :master_allocations, {:venue_layout=>[:venue]}]}, :photos]}, :bookings, :run, :master_allocations, :venue_layout] While the merge strategy results in: [:run, :master_allocations, :venue_layout, :bookings, {:raw_show=>[:events, :photos]}] --- lib/jsonapi/resource.rb | 108 ++++++++++++++++++++++++---- test/unit/resource/resource_test.rb | 64 ++++++++++++----- 2 files changed, 141 insertions(+), 31 deletions(-) diff --git a/lib/jsonapi/resource.rb b/lib/jsonapi/resource.rb index eb34bce80..a22a51b09 100644 --- a/lib/jsonapi/resource.rb +++ b/lib/jsonapi/resource.rb @@ -615,9 +615,9 @@ def resolve_relationship_names_to_relations(resource_klass, model_includes, opti end end - def _append_always_includes(resource_klass, model_include, options) + def merge_always_includes(resource_klass, model_include, options) if resource_klass.present? - resource_klass.always_includes + model_include + merge_includes(resource_klass.always_includes, model_include) else model_include end @@ -634,24 +634,23 @@ def relationship_klass_from(resource_klass, key) nil end - def resolve_always_includes(resource_klass, model_includes, options = {}, seen = []) + def resolve_always_includes(resource_klass, model_includes, options = {}) case model_includes when Array model_includes.uniq.map do |value| - resolve_always_includes(resource_klass, value, options, seen) + resolve_always_includes(resource_klass, value, options) end when Hash Hash[model_includes.map do |key, value| relationship_klass = relationship_klass_from(resource_klass, key) if relationship_klass.present? - [key, resolve_always_includes(relationship_klass, _append_always_includes(relationship_klass, value, options), options, seen)] + [key, resolve_always_includes(relationship_klass, merge_always_includes(relationship_klass, value, options), options)] end end.compact] when Symbol relationship_klass = relationship_klass_from(resource_klass, model_includes.to_s) - if relationship_klass.present? && relationship_klass.always_includes.present? && !seen.include?(model_includes) - seen.push(model_includes) - { model_includes => resolve_always_includes(relationship_klass, relationship_klass.always_includes, options, seen)} + if relationship_klass.present? && relationship_klass.always_includes.present? + { model_includes => relationship_klass.always_includes } else model_includes end @@ -667,19 +666,98 @@ def model_includes(options) end def get_includes(options = {}) - model_includes = model_includes(options) - first_level_includes = model_includes.map{ |inc| inc.try(:keys) || inc }.flatten - includes = model_includes + ( always_includes - first_level_includes ) - includes = resolve_always_includes(self, includes, options) + model_includes = resolve_always_includes(self, model_includes(options), options) + includes = merge_includes(always_includes, model_includes) filter_for_relationship_type(includes, options) end + def merge_includes(includes_a, includes_b) + includes_a = normalize_includes(includes_a) + includes_b = normalize_includes(includes_b) + + hash_a = includes_a.detect{|inc| inc.is_a? Hash} + hash_b = includes_b.detect{|inc| inc.is_a? Hash} + + merged_hash = merge_includes_hashes(hash_a, hash_b) + if merged_hash.empty? + (includes_a + includes_b).uniq + else + without_hash_keys = includes_a.reject{|inc| inc.is_a? Hash} + includes_b.reject{|inc| inc.is_a? Hash} + without_hash_keys = without_hash_keys.uniq - merged_hash.keys + without_hash_keys << merged_hash + end + end + + def merge_includes_hashes(hash_a, hash_b) + if hash_a && hash_b + same_keys(hash_a, hash_b).inject(uniq_hash(hash_a, hash_b)) do |hsh, key| + hsh[key] = merge_includes(hash_a[key], hash_b[key]) + hsh + end + else + hash_a || hash_b || {} + end + end + + def uniq_keys(hash_a, hash_b) + (hash_a.keys - hash_b.keys) + (hash_b.keys - hash_a.keys) + end + + def same_keys(hash_a, hash_b) + hash_a.keys - uniq_keys(hash_a, hash_b) + end + + def uniq_hash(hash_a, hash_b) + uniq_keys(hash_a, hash_b).inject({}) do |hsh, key| + hsh[key] = hash_a[key] || hash_b[key] + hsh + end + end + + def normalize_includes(includes) + case includes + when Array + hsh = {} + arr = includes.inject([]) do |arr, inc| + case inc + when Hash + hsh.merge!(inc) + when Symbol + arr << inc + end + arr + end + arr << hsh unless hsh.empty? + arr + when Symbol + Array(includes) + end + end + def filter_for_relationship_type(includes, options) if options[:relationship_type].present? - includes.select{|v| (v.try(:keys).try(:first) || v) == options[:relationship_type] } - else - includes + relationship = self._relationships[options[:relationship_type].to_sym] + relation_name = relationship.relation_name(options) + + includes = includes.reduce([]) do |arr, val| + if val == relation_name + arr << val + elsif val.try(:fetch, relation_name, nil).present? + arr << { relation_name => val[relation_name] } + end + arr + end + + if includes.empty? + if relationship.resource_klass.always_include.present? + includes << { relation_name => relationship.resource_klass.always_include } + else + includes << relation_name + end + end end + + includes end def apply_includes(records, options = {}) diff --git a/test/unit/resource/resource_test.rb b/test/unit/resource/resource_test.rb index 6cbd5c948..3ba9c3731 100644 --- a/test/unit/resource/resource_test.rb +++ b/test/unit/resource/resource_test.rb @@ -146,7 +146,15 @@ def test_resolve_always_includes_nested def test_get_includes include_directive = JSONAPI::IncludeDirectives.new(Ai::PersonResource, ['comments']) - expected = [:comments, :hair_cut] + expected = [:hair_cut, :comments] + + result = Ai::PersonResource.get_includes(include_directives: include_directive) + assert_equal(expected, result) + end + + def test_get_includes_again + include_directive = JSONAPI::IncludeDirectives.new(Ai::PersonResource, ['comments.tags', 'posts.section']) + expected = [:hair_cut, {comments: [{tags: [:posts]}], posts: [:section]}] result = Ai::PersonResource.get_includes(include_directives: include_directive) assert_equal(expected, result) @@ -154,7 +162,7 @@ def test_get_includes def test_get_includes_nested include_directive = JSONAPI::IncludeDirectives.new(Ai::PersonResource, ['comments.tags']) - expected = [{comments: [{tags: [:posts]}]}, :hair_cut] + expected = [:hair_cut, {comments: [{tags: [:posts]}]}] result = Ai::PersonResource.get_includes(include_directives: include_directive) assert_equal(expected, result) @@ -162,43 +170,67 @@ def test_get_includes_nested def test_get_includes_nested_more_complex include_directive = JSONAPI::IncludeDirectives.new(Ai::PersonResource, ['comments.tags', 'posts.section']) - expected = [{comments: [{tags: [:posts]}]}, {posts: [:section]}, :hair_cut] + expected = [:hair_cut, {comments: [{tags: [:posts]}], posts: [:section]}] result = Ai::PersonResource.get_includes(include_directives: include_directive) assert_equal(expected, result) end - def test_get_includes_for_relationship - include_directive = JSONAPI::IncludeDirectives.new(Ai::PersonResource, ['comments.tags', 'posts.section']) - expected = [{comments: [{tags: [:posts]}]}] + def test_normalize_includes + result = JSONAPI::Resource.normalize_includes([{comments: [{tags: [:posts]}]}, {posts: [:section]}, :hair_cut]) + assert_equal([:hair_cut, {comments: [{tags: [:posts]}], posts: [:section]}], result) + end - result = Ai::PersonResource.get_includes(include_directives: include_directive, relationship_type: :comments) + def test_merge_includes + includes_a = [:hair_cut, {comments: [{tags: [:posts]}]}, :posts] + includes_b = [{comments: :tags, posts: [:section]}, :hair_cut] + expected = [:hair_cut, {comments: [{tags: [:posts]}], posts: [:section]}] + result = JSONAPI::Resource.merge_includes(includes_a, includes_b) assert_equal(expected, result) end end -class AlwayIncludeCircularTests < ActiveSupport::TestCase - module Circular +class AlwaysIncludeRelationshipTest < ActiveSupport::TestCase + module AiRel class PersonResource < ::PersonResource - always_include :comments + always_include :hair_cut, :comments end class CommentResource < ::CommentResource - always_include :tags end class TagResource < ::TagResource - always_include :posts + always_include posts: [:section] end class PostResource < ::PostResource - always_include :comments end + + class SectionResource < ::SectionResource + end + end + + def test_get_includes_for_relationship + include_directive = JSONAPI::IncludeDirectives.new(AiRel::PersonResource, ['comments.tags', 'posts.section']) + expected = [{comments: [{tags: [{posts: [:section]}]}]}] + + result = AiRel::PersonResource.get_includes(include_directives: include_directive, relationship_type: :comments) + assert_equal(expected, result) end - def test_always_includes_circular - expected = [{comments: [{tags: [{posts: [:comments]}]}]}] - result = JSONAPI::Resource.resolve_always_includes(Circular::PersonResource, [:comments]) + def test_get_includes_for_relationship_2 + include_directive = JSONAPI::IncludeDirectives.new(AiRel::PersonResource, ['posts.section']) + expected = [:comments] + + result = AiRel::PersonResource.get_includes(include_directives: include_directive, relationship_type: :comments) + assert_equal(expected, result) + end + + def test_get_include_for_relationship_no_model_includes + include_directive = JSONAPI::IncludeDirectives.new(AiRel::CommentResource, ['post']) + expected = [{tags: [{posts: [:section]}]}] + + result = AiRel::CommentResource.get_includes(include_directives: include_directive, relationship_type: :tags) assert_equal(expected, result) end end From c380eb91c52bb4cbce0a6c967e32c024a180a6be Mon Sep 17 00:00:00 2001 From: mrloop Date: Fri, 30 Sep 2016 20:05:23 +0100 Subject: [PATCH 6/7] ::always_include renamed to ::always_load Rename to better express the intent from `always_include` to `always_load` as in `preload` or `eager_load` --- lib/jsonapi/resource.rb | 86 ++++++++++++++--------------- test/unit/resource/resource_test.rb | 68 +++++++++++------------ 2 files changed, 77 insertions(+), 77 deletions(-) diff --git a/lib/jsonapi/resource.rb b/lib/jsonapi/resource.rb index a22a51b09..39c1bada8 100644 --- a/lib/jsonapi/resource.rb +++ b/lib/jsonapi/resource.rb @@ -461,7 +461,7 @@ def resource_type_for(model) end end - attr_accessor :_attributes, :_relationships, :_type, :_model_hints, :always_includes + attr_accessor :_attributes, :_relationships, :_type, :_model_hints, :always_loads attr_writer :_allowed_filters, :_paginator def create(context) @@ -572,8 +572,8 @@ def cache_field(field) @_cache_field = field.to_sym end - def always_include(*relationships) - _add_always_includes(*relationships) + def always_load(*relationships) + _add_always_loads(*relationships) end # Override in your resource to filter the updatable keys @@ -615,9 +615,9 @@ def resolve_relationship_names_to_relations(resource_klass, model_includes, opti end end - def merge_always_includes(resource_klass, model_include, options) + def merge_always_loads(resource_klass, model_include, options) if resource_klass.present? - merge_includes(resource_klass.always_includes, model_include) + merge_loads(resource_klass.always_loads, model_include) else model_include end @@ -634,23 +634,23 @@ def relationship_klass_from(resource_klass, key) nil end - def resolve_always_includes(resource_klass, model_includes, options = {}) + def resolve_always_loads(resource_klass, model_includes, options = {}) case model_includes when Array model_includes.uniq.map do |value| - resolve_always_includes(resource_klass, value, options) + resolve_always_loads(resource_klass, value, options) end when Hash Hash[model_includes.map do |key, value| relationship_klass = relationship_klass_from(resource_klass, key) if relationship_klass.present? - [key, resolve_always_includes(relationship_klass, merge_always_includes(relationship_klass, value, options), options)] + [key, resolve_always_loads(relationship_klass, merge_always_loads(relationship_klass, value, options), options)] end end.compact] when Symbol relationship_klass = relationship_klass_from(resource_klass, model_includes.to_s) - if relationship_klass.present? && relationship_klass.always_includes.present? - { model_includes => relationship_klass.always_includes } + if relationship_klass.present? && relationship_klass.always_loads.present? + { model_includes => relationship_klass.always_loads } else model_includes end @@ -665,33 +665,33 @@ def model_includes(options) [] end - def get_includes(options = {}) - model_includes = resolve_always_includes(self, model_includes(options), options) - includes = merge_includes(always_includes, model_includes) + def get_loads(options = {}) + model_includes = resolve_always_loads(self, model_includes(options), options) + includes = merge_loads(always_loads, model_includes) filter_for_relationship_type(includes, options) end - def merge_includes(includes_a, includes_b) - includes_a = normalize_includes(includes_a) - includes_b = normalize_includes(includes_b) + def merge_loads(loads_a, loads_b) + loads_a = normalize_loads(loads_a) + loads_b = normalize_loads(loads_b) - hash_a = includes_a.detect{|inc| inc.is_a? Hash} - hash_b = includes_b.detect{|inc| inc.is_a? Hash} + hash_a = loads_a.detect{|inc| inc.is_a? Hash} + hash_b = loads_b.detect{|inc| inc.is_a? Hash} - merged_hash = merge_includes_hashes(hash_a, hash_b) + merged_hash = merge_loads_hashes(hash_a, hash_b) if merged_hash.empty? - (includes_a + includes_b).uniq + (loads_a + loads_b).uniq else - without_hash_keys = includes_a.reject{|inc| inc.is_a? Hash} + includes_b.reject{|inc| inc.is_a? Hash} + without_hash_keys = loads_a.reject{|inc| inc.is_a? Hash} + loads_b.reject{|inc| inc.is_a? Hash} without_hash_keys = without_hash_keys.uniq - merged_hash.keys without_hash_keys << merged_hash end end - def merge_includes_hashes(hash_a, hash_b) + def merge_loads_hashes(hash_a, hash_b) if hash_a && hash_b same_keys(hash_a, hash_b).inject(uniq_hash(hash_a, hash_b)) do |hsh, key| - hsh[key] = merge_includes(hash_a[key], hash_b[key]) + hsh[key] = merge_loads(hash_a[key], hash_b[key]) hsh end else @@ -714,11 +714,11 @@ def uniq_hash(hash_a, hash_b) end end - def normalize_includes(includes) - case includes + def normalize_loads(loads) + case loads when Array hsh = {} - arr = includes.inject([]) do |arr, inc| + arr = loads.inject([]) do |arr, inc| case inc when Hash hsh.merge!(inc) @@ -730,16 +730,16 @@ def normalize_includes(includes) arr << hsh unless hsh.empty? arr when Symbol - Array(includes) + Array(loads) end end - def filter_for_relationship_type(includes, options) + def filter_for_relationship_type(loads, options) if options[:relationship_type].present? relationship = self._relationships[options[:relationship_type].to_sym] relation_name = relationship.relation_name(options) - includes = includes.reduce([]) do |arr, val| + loads = loads.reduce([]) do |arr, val| if val == relation_name arr << val elsif val.try(:fetch, relation_name, nil).present? @@ -748,20 +748,20 @@ def filter_for_relationship_type(includes, options) arr end - if includes.empty? - if relationship.resource_klass.always_include.present? - includes << { relation_name => relationship.resource_klass.always_include } + if loads.empty? + if relationship.resource_klass.always_load.present? + loads << { relation_name => relationship.resource_klass.always_load } else - includes << relation_name + loads << relation_name end end end - includes + loads end - def apply_includes(records, options = {}) - records.includes(get_includes(options)) + def apply_loads(records, options = {}) + records.includes(get_loads(options)) end def apply_pagination(records, paginator, order_options) @@ -846,7 +846,7 @@ def apply_filters(records, filters, options = {}) end if required_includes.any? - records = apply_includes(records, options.merge(include_directives: IncludeDirectives.new(self, required_includes, force_eager_load: true))) + records = apply_loads(records, options.merge(include_directives: IncludeDirectives.new(self, required_includes, force_eager_load: true))) end records @@ -854,7 +854,7 @@ def apply_filters(records, filters, options = {}) def filter_records(filters, options, records = records(options)) records = apply_filters(records, filters, options) - apply_includes(records, options) + apply_loads(records, options) end def sort_records(records, order_options, context = {}) @@ -884,7 +884,7 @@ def resources_for(records, context) def find_by_keys(keys, options = {}) context = options[:context] records = records(options) - records = apply_includes(records, options) + records = apply_loads(records, options) models = records.where({_primary_key => keys}) models.collect do |model| self.resource_for_model(model).new(model, context) @@ -1158,12 +1158,12 @@ def _add_relationship(klass, *attrs) end end - def always_includes - @always_includes ||= [] + def always_loads + @always_loads ||= [] end - def _add_always_includes(*relationships) - always_includes.concat(Array(relationships)) + def _add_always_loads(*relationships) + always_loads.concat(Array(relationships)) end # Allows JSONAPI::RelationshipBuilder to access metaprogramming hooks diff --git a/test/unit/resource/resource_test.rb b/test/unit/resource/resource_test.rb index 3ba9c3731..2a1eb9bef 100644 --- a/test/unit/resource/resource_test.rb +++ b/test/unit/resource/resource_test.rb @@ -101,14 +101,14 @@ class RelatedResource < MyModule::RelatedResource class AlwaysIncludeTests < ActiveSupport::TestCase module Ai class PersonResource < ::PersonResource - always_include :hair_cut, :comments + always_load :hair_cut, :comments end class CommentResource < ::CommentResource end class TagResource < ::TagResource - always_include :posts + always_load :posts end class PostResource < ::PostResource @@ -125,67 +125,67 @@ class SectionResource < ::SectionResource end end - def test_always_includes - assert_equal([:hair_cut, :comments], Ai::PersonResource.always_includes) + def test_always_loads + assert_equal([:hair_cut, :comments], Ai::PersonResource.always_loads) end - def test_resolve_always_includes - result = JSONAPI::Resource.resolve_always_includes(Ai::PersonResource, [:vehicles, :comments]) + def test_resolve_always_loads + result = JSONAPI::Resource.resolve_always_loads(Ai::PersonResource, [:vehicles, :comments]) assert_equal([:vehicles, :comments], result) end - def test_resolve_always_includes_nested + def test_resolve_always_loads_nested expected = {comments: [{tags: [:posts]}]} - result = JSONAPI::Resource.resolve_always_includes(Ai::PersonResource, comments: [:tags]) + result = JSONAPI::Resource.resolve_always_loads(Ai::PersonResource, comments: [:tags]) assert_equal(expected, result) - result = JSONAPI::Resource.resolve_always_includes(Ai::PersonResource, comments: [{tags: [:posts]}]) + result = JSONAPI::Resource.resolve_always_loads(Ai::PersonResource, comments: [{tags: [:posts]}]) assert_equal(expected, result) end - def test_get_includes + def test_get_loads include_directive = JSONAPI::IncludeDirectives.new(Ai::PersonResource, ['comments']) expected = [:hair_cut, :comments] - result = Ai::PersonResource.get_includes(include_directives: include_directive) + result = Ai::PersonResource.get_loads(include_directives: include_directive) assert_equal(expected, result) end - def test_get_includes_again - include_directive = JSONAPI::IncludeDirectives.new(Ai::PersonResource, ['comments.tags', 'posts.section']) + def test_get_loads_again + include_directive = JSONAPI::IncludeDirectives.new(Ai::PersonResource, ['comments.tags', 'posts.section']) expected = [:hair_cut, {comments: [{tags: [:posts]}], posts: [:section]}] - result = Ai::PersonResource.get_includes(include_directives: include_directive) + result = Ai::PersonResource.get_loads(include_directives: include_directive) assert_equal(expected, result) end - def test_get_includes_nested + def test_get_loads_nested include_directive = JSONAPI::IncludeDirectives.new(Ai::PersonResource, ['comments.tags']) expected = [:hair_cut, {comments: [{tags: [:posts]}]}] - result = Ai::PersonResource.get_includes(include_directives: include_directive) + result = Ai::PersonResource.get_loads(include_directives: include_directive) assert_equal(expected, result) end - def test_get_includes_nested_more_complex + def test_get_loads_nested_more_complex include_directive = JSONAPI::IncludeDirectives.new(Ai::PersonResource, ['comments.tags', 'posts.section']) expected = [:hair_cut, {comments: [{tags: [:posts]}], posts: [:section]}] - result = Ai::PersonResource.get_includes(include_directives: include_directive) + result = Ai::PersonResource.get_loads(include_directives: include_directive) assert_equal(expected, result) end - def test_normalize_includes - result = JSONAPI::Resource.normalize_includes([{comments: [{tags: [:posts]}]}, {posts: [:section]}, :hair_cut]) + def test_normalize_loads + result = JSONAPI::Resource.normalize_loads([{comments: [{tags: [:posts]}]}, {posts: [:section]}, :hair_cut]) assert_equal([:hair_cut, {comments: [{tags: [:posts]}], posts: [:section]}], result) end - def test_merge_includes - includes_a = [:hair_cut, {comments: [{tags: [:posts]}]}, :posts] - includes_b = [{comments: :tags, posts: [:section]}, :hair_cut] + def test_merge_loads + loads_a = [:hair_cut, {comments: [{tags: [:posts]}]}, :posts] + loads_b = [{comments: :tags, posts: [:section]}, :hair_cut] expected = [:hair_cut, {comments: [{tags: [:posts]}], posts: [:section]}] - result = JSONAPI::Resource.merge_includes(includes_a, includes_b) + result = JSONAPI::Resource.merge_loads(loads_a, loads_b) assert_equal(expected, result) end end @@ -193,14 +193,14 @@ def test_merge_includes class AlwaysIncludeRelationshipTest < ActiveSupport::TestCase module AiRel class PersonResource < ::PersonResource - always_include :hair_cut, :comments + always_load :hair_cut, :comments end class CommentResource < ::CommentResource end class TagResource < ::TagResource - always_include posts: [:section] + always_load posts: [:section] end class PostResource < ::PostResource @@ -210,27 +210,27 @@ class SectionResource < ::SectionResource end end - def test_get_includes_for_relationship + def test_get_loads_for_relationship include_directive = JSONAPI::IncludeDirectives.new(AiRel::PersonResource, ['comments.tags', 'posts.section']) expected = [{comments: [{tags: [{posts: [:section]}]}]}] - result = AiRel::PersonResource.get_includes(include_directives: include_directive, relationship_type: :comments) + result = AiRel::PersonResource.get_loads(include_directives: include_directive, relationship_type: :comments) assert_equal(expected, result) end - def test_get_includes_for_relationship_2 + def test_get_loads_for_relationship_2 include_directive = JSONAPI::IncludeDirectives.new(AiRel::PersonResource, ['posts.section']) expected = [:comments] - result = AiRel::PersonResource.get_includes(include_directives: include_directive, relationship_type: :comments) + result = AiRel::PersonResource.get_loads(include_directives: include_directive, relationship_type: :comments) assert_equal(expected, result) end - def test_get_include_for_relationship_no_model_includes + def test_get_loads_for_relationship_no_model_includes include_directive = JSONAPI::IncludeDirectives.new(AiRel::CommentResource, ['post']) expected = [{tags: [{posts: [:section]}]}] - result = AiRel::CommentResource.get_includes(include_directives: include_directive, relationship_type: :tags) + result = AiRel::CommentResource.get_loads(include_directives: include_directive, relationship_type: :tags) assert_equal(expected, result) end end @@ -238,12 +238,12 @@ def test_get_include_for_relationship_no_model_includes class AlwaysIncludeNoResourceTest < ActiveSupport::TestCase module NoResource class MoonResource < ::MoonResource - always_include :craters + always_load :craters end end def test_always_include_no_resource_no_error_raised - result = JSONAPI::Resource.resolve_always_includes(NoResource::MoonResource, [:craters]) + result = JSONAPI::Resource.resolve_always_loads(NoResource::MoonResource, [:craters]) assert_equal([:craters], result) end end From a68616e26c6863f5875b2f158a94b0f34b2524a8 Mon Sep 17 00:00:00 2001 From: mrloop Date: Tue, 4 Oct 2016 06:14:28 +0100 Subject: [PATCH 7/7] ::always_load documentation --- README.md | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/README.md b/README.md index 63b389a51..03fbcdb3b 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,7 @@ backed by ActiveRecord models or by custom objects. * [Filters] (#filters) * [Pagination] (#pagination) * [Included relationships (side-loading resources)] (#included-relationships-side-loading-resources) + * [Loaded relationships (eager-loading resources)] (#loaded-relationships-eager-loading-resources) * [Resource meta] (#resource-meta) * [Custom Links] (#custom-links) * [Callbacks] (#callbacks) @@ -1036,6 +1037,40 @@ Will get you the following payload by default: } ``` +#### Loaded relationships (eager-loading resources) + +To avoid N+1 queries when a resource attribute relies on attributes of related model records to calculate it `always_load` can be used. [Included relationships](#included-relationships-side-loading-resources) have their `always_load` relationships loaded. + +```ruby +class CompanyResource + attribute :plan_start_at + + always_load :subscription + + def plan_start_at + @model.subscription.created_at + end +end +``` + +Nested records to eager load can be declared. + +```ruby +class CompanyResource + attribute :plan_start_at + + always_load subscription: [:settings] + + def plan_start_at + @model.subscription.created_at + end + + def needs_tokens + @model.subscription.settings.needs_tokens + end +end +``` + #### Resource Meta Meta information can be included for each resource using the meta method in the resource declaration. For example: