From 2ff6dcec6d74457231bd8ea92396f1ba4f68fdf1 Mon Sep 17 00:00:00 2001 From: lgebhardt Date: Thu, 11 Jan 2024 10:17:29 -0500 Subject: [PATCH 1/6] Make SortedSet for identity arrays optional --- jsonapi-resources.gemspec | 1 + lib/jsonapi/active_relation_retrieval.rb | 13 +++++++++---- lib/jsonapi/configuration.rb | 9 ++++++++- lib/jsonapi/resource_fragment.rb | 2 +- lib/jsonapi/resource_set.rb | 2 +- test/test_helper.rb | 2 ++ 6 files changed, 22 insertions(+), 7 deletions(-) diff --git a/jsonapi-resources.gemspec b/jsonapi-resources.gemspec index 65372c9f..2f2044aa 100644 --- a/jsonapi-resources.gemspec +++ b/jsonapi-resources.gemspec @@ -31,4 +31,5 @@ Gem::Specification.new do |spec| spec.add_dependency 'activerecord', '>= 5.1' spec.add_dependency 'railties', '>= 5.1' spec.add_dependency 'concurrent-ruby' + spec.add_dependency 'sorted_set' end diff --git a/lib/jsonapi/active_relation_retrieval.rb b/lib/jsonapi/active_relation_retrieval.rb index 1e1a72fb..89939021 100644 --- a/lib/jsonapi/active_relation_retrieval.rb +++ b/lib/jsonapi/active_relation_retrieval.rb @@ -271,7 +271,7 @@ def find_related_fragments(source_fragment, relationship, options = {}) source_resource_klasses.each do |resource_klass| inverse_direct_relationship = _relationship(resource_klass._type.to_s.singularize) - fragments.merge!(resource_klass.find_related_fragments_from_inverse([source_fragment], inverse_direct_relationship, options, true)) + fragments.merge!(resource_klass.find_related_fragments_from_inverse([source_fragment], inverse_direct_relationship, options, false)) end fragments else @@ -317,9 +317,14 @@ def find_related_fragments_from_inverse(source, source_relationship, options, co linkage_relationships = to_one_relationships_for_linkage(include_directives[:include_related]) sort_criteria = [] - options[:sort_criteria].try(:each) do |sort| - field = sort[:field].to_s == 'id' ? _primary_key : sort[:field] - sort_criteria << { field: field, direction: sort[:direction] } + + # Do not sort the includes. Key off connect_source_identity to indicate wether this is a related resource + # primary step vs an include step + unless connect_source_identity + options[:sort_criteria].try(:each) do |sort| + field = sort[:field].to_s == 'id' ? _primary_key : sort[:field] + sort_criteria << { field: field, direction: sort[:direction] } + end end join_manager = ActiveRelation::JoinManager.new(resource_klass: self, diff --git a/lib/jsonapi/configuration.rb b/lib/jsonapi/configuration.rb index 1d2c235e..343107cf 100644 --- a/lib/jsonapi/configuration.rb +++ b/lib/jsonapi/configuration.rb @@ -43,7 +43,8 @@ class Configuration :resource_cache_usage_report_function, :default_exclude_links, :default_resource_retrieval_strategy, - :use_related_resource_records_for_joins + :use_related_resource_records_for_joins, + :sort_related_identities_by_primary_key def initialize #:underscored_key, :camelized_key, :dasherized_key, or custom @@ -182,6 +183,10 @@ def initialize # This setting allows included resources to account for permission scopes. It can be overridden explicitly per # relationship. Furthermore, specifying a `relation_name` on a relationship will cause this setting to be ignored. self.use_related_resource_records_for_joins = true + + # Collect the include keys into a SortedSet. This carries a small performance cost in the rails app but produces + # consistent and more human navigable result sets. + self.sort_related_identities_by_primary_key = false end def cache_formatters=(bool) @@ -327,6 +332,8 @@ def allow_include=(allow_include) attr_writer :default_resource_retrieval_strategy attr_writer :use_related_resource_records_for_joins + + attr_writer :sort_related_identities_by_primary_key end class << self diff --git a/lib/jsonapi/resource_fragment.rb b/lib/jsonapi/resource_fragment.rb index c354f9aa..28c3ff7b 100644 --- a/lib/jsonapi/resource_fragment.rb +++ b/lib/jsonapi/resource_fragment.rb @@ -25,7 +25,7 @@ def initialize(identity, resource: nil, cache: nil, primary: false) @primary = primary @related = {} - @related_from = Set.new + @related_from = JSONAPI.configuration.sort_related_identities_by_primary_key ? SortedSet.new : Set.new end def initialize_related(relationship_name) diff --git a/lib/jsonapi/resource_set.rb b/lib/jsonapi/resource_set.rb index f4d6186f..09374b55 100644 --- a/lib/jsonapi/resource_set.rb +++ b/lib/jsonapi/resource_set.rb @@ -180,7 +180,7 @@ def flatten_resource_tree(resource_tree, flattened_tree = {}) flattened_tree[resource_klass][id][:resource] ||= fragment.resource if fragment.resource fragment.related.try(:each_pair) do |relationship_name, related_rids| - flattened_tree[resource_klass][id][:relationships][relationship_name] ||= Set.new + flattened_tree[resource_klass][id][:relationships][relationship_name] ||= JSONAPI.configuration.sort_related_identities_by_primary_key ? SortedSet.new : Set.new flattened_tree[resource_klass][id][:relationships][relationship_name].merge(related_rids) end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 834c8577..9c67680f 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -66,6 +66,8 @@ class TestApp < ::Rails::Application end config.hosts << "www.example.com" + + config.sort_related_identities_by_primary_key = true end require 'rails/test_help' From f240f2cfbe4a5969beeceeebdeda18f3a8794223 Mon Sep 17 00:00:00 2001 From: lgebhardt Date: Thu, 11 Jan 2024 10:39:53 -0500 Subject: [PATCH 2/6] Fix tests to use sort_related_identities_by_primary_key option override --- test/test_helper.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_helper.rb b/test/test_helper.rb index 9c67680f..e8be76f6 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -39,6 +39,7 @@ JSONAPI.configure do |config| config.json_key_format = :camelized_key + config.sort_related_identities_by_primary_key = true end ActiveSupport::Deprecation.silenced = true From 3931e0ce18459403f1aa51733ad64db95d511370 Mon Sep 17 00:00:00 2001 From: lgebhardt Date: Thu, 11 Jan 2024 11:28:09 -0500 Subject: [PATCH 3/6] Keep SortedSet as a development dependency, unless required --- jsonapi-resources.gemspec | 1 + lib/jsonapi/configuration.rb | 12 +++++++----- lib/jsonapi/resource_fragment.rb | 2 +- lib/jsonapi/resource_set.rb | 2 +- test/test_helper.rb | 6 +++--- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/jsonapi-resources.gemspec b/jsonapi-resources.gemspec index 2f2044aa..2417ed27 100644 --- a/jsonapi-resources.gemspec +++ b/jsonapi-resources.gemspec @@ -28,6 +28,7 @@ Gem::Specification.new do |spec| spec.add_development_dependency 'concurrent-ruby-ext' spec.add_development_dependency 'database_cleaner' spec.add_development_dependency 'hashie' + spec.add_development_dependency 'sorted_set' spec.add_dependency 'activerecord', '>= 5.1' spec.add_dependency 'railties', '>= 5.1' spec.add_dependency 'concurrent-ruby' diff --git a/lib/jsonapi/configuration.rb b/lib/jsonapi/configuration.rb index 343107cf..a93adf2d 100644 --- a/lib/jsonapi/configuration.rb +++ b/lib/jsonapi/configuration.rb @@ -44,7 +44,7 @@ class Configuration :default_exclude_links, :default_resource_retrieval_strategy, :use_related_resource_records_for_joins, - :sort_related_identities_by_primary_key + :related_identities_set def initialize #:underscored_key, :camelized_key, :dasherized_key, or custom @@ -184,9 +184,11 @@ def initialize # relationship. Furthermore, specifying a `relation_name` on a relationship will cause this setting to be ignored. self.use_related_resource_records_for_joins = true - # Collect the include keys into a SortedSet. This carries a small performance cost in the rails app but produces - # consistent and more human navigable result sets. - self.sort_related_identities_by_primary_key = false + # Collect the include keys into a Set or a SortedSet. SortedSet carries a small performance cost in the rails app + # but produces consistent and more human navigable result sets. + # Note: If using SortedSet be sure to add `sorted_set` to your Gemfile and require 'sorted_set'` + # require 'sorted_set' + self.related_identities_set = Set end def cache_formatters=(bool) @@ -333,7 +335,7 @@ def allow_include=(allow_include) attr_writer :use_related_resource_records_for_joins - attr_writer :sort_related_identities_by_primary_key + attr_writer :related_identities_set end class << self diff --git a/lib/jsonapi/resource_fragment.rb b/lib/jsonapi/resource_fragment.rb index 28c3ff7b..0149f1ee 100644 --- a/lib/jsonapi/resource_fragment.rb +++ b/lib/jsonapi/resource_fragment.rb @@ -25,7 +25,7 @@ def initialize(identity, resource: nil, cache: nil, primary: false) @primary = primary @related = {} - @related_from = JSONAPI.configuration.sort_related_identities_by_primary_key ? SortedSet.new : Set.new + @related_from = JSONAPI.configuration.related_identities_set.new end def initialize_related(relationship_name) diff --git a/lib/jsonapi/resource_set.rb b/lib/jsonapi/resource_set.rb index 09374b55..e5846994 100644 --- a/lib/jsonapi/resource_set.rb +++ b/lib/jsonapi/resource_set.rb @@ -180,7 +180,7 @@ def flatten_resource_tree(resource_tree, flattened_tree = {}) flattened_tree[resource_klass][id][:resource] ||= fragment.resource if fragment.resource fragment.related.try(:each_pair) do |relationship_name, related_rids| - flattened_tree[resource_klass][id][:relationships][relationship_name] ||= JSONAPI.configuration.sort_related_identities_by_primary_key ? SortedSet.new : Set.new + flattened_tree[resource_klass][id][:relationships][relationship_name] ||= JSONAPI.configuration.related_identities_set.new flattened_tree[resource_klass][id][:relationships][relationship_name].merge(related_rids) end end diff --git a/test/test_helper.rb b/test/test_helper.rb index e8be76f6..b92d8428 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -39,7 +39,9 @@ JSONAPI.configure do |config| config.json_key_format = :camelized_key - config.sort_related_identities_by_primary_key = true + + require 'sorted_set' + config.related_identities_set = SortedSet end ActiveSupport::Deprecation.silenced = true @@ -67,8 +69,6 @@ class TestApp < ::Rails::Application end config.hosts << "www.example.com" - - config.sort_related_identities_by_primary_key = true end require 'rails/test_help' From b300d0091f3b58561836ba3fa9426898b6549036 Mon Sep 17 00:00:00 2001 From: lgebhardt Date: Thu, 11 Jan 2024 14:30:38 -0500 Subject: [PATCH 4/6] Remove sorted_set dependency --- jsonapi-resources.gemspec | 1 - 1 file changed, 1 deletion(-) diff --git a/jsonapi-resources.gemspec b/jsonapi-resources.gemspec index 2417ed27..f6b2f8af 100644 --- a/jsonapi-resources.gemspec +++ b/jsonapi-resources.gemspec @@ -32,5 +32,4 @@ Gem::Specification.new do |spec| spec.add_dependency 'activerecord', '>= 5.1' spec.add_dependency 'railties', '>= 5.1' spec.add_dependency 'concurrent-ruby' - spec.add_dependency 'sorted_set' end From 92f992af4f9294b9ba3000b51352cfeb23b33992 Mon Sep 17 00:00:00 2001 From: lgebhardt Date: Thu, 11 Jan 2024 14:51:52 -0500 Subject: [PATCH 5/6] Add better messaging about using SortedSet --- lib/jsonapi/configuration.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/jsonapi/configuration.rb b/lib/jsonapi/configuration.rb index a93adf2d..b9f4b772 100644 --- a/lib/jsonapi/configuration.rb +++ b/lib/jsonapi/configuration.rb @@ -186,8 +186,9 @@ def initialize # Collect the include keys into a Set or a SortedSet. SortedSet carries a small performance cost in the rails app # but produces consistent and more human navigable result sets. - # Note: If using SortedSet be sure to add `sorted_set` to your Gemfile and require 'sorted_set'` + # To use SortedSet be sure to add `sorted_set` to your Gemfile and the following two lines to your JR initializer: # require 'sorted_set' + # config.related_identities_set = SortedSet self.related_identities_set = Set end From a637e73e07add7a61a615385c6e4c14fb30a7c73 Mon Sep 17 00:00:00 2001 From: lgebhardt Date: Thu, 11 Jan 2024 14:54:11 -0500 Subject: [PATCH 6/6] Clarify setting sort_criteria for includes vs. related resources --- lib/jsonapi/active_relation_retrieval.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/jsonapi/active_relation_retrieval.rb b/lib/jsonapi/active_relation_retrieval.rb index 89939021..f331bd59 100644 --- a/lib/jsonapi/active_relation_retrieval.rb +++ b/lib/jsonapi/active_relation_retrieval.rb @@ -318,9 +318,11 @@ def find_related_fragments_from_inverse(source, source_relationship, options, co sort_criteria = [] - # Do not sort the includes. Key off connect_source_identity to indicate wether this is a related resource - # primary step vs an include step - unless connect_source_identity + # Do not sort the related_fragments. This can be keyed off `connect_source_identity` to indicate whether this + # is a related resource primary step vs. an include step. + sort_related_fragments = !connect_source_identity + + if sort_related_fragments options[:sort_criteria].try(:each) do |sort| field = sort[:field].to_s == 'id' ? _primary_key : sort[:field] sort_criteria << { field: field, direction: sort[:direction] }