From c052d465514f254827ca618ef2a86ed989330480 Mon Sep 17 00:00:00 2001 From: Larry Gebhardt Date: Tue, 16 Jan 2024 14:45:09 -0500 Subject: [PATCH] V0 11 dev performance (#1431) * Reduce number of string allocations in LinkBuilder * Consistently access `include_related` * Remove unused class variable * Cache `id` after retrieving it from the model * Cache `module_path` * Cache resource_klass_for and resource_type_for * Remove no longer used method _setup_relationship * Delete nil values without creating a new object * Rework resource naming for method caches --- lib/jsonapi/link_builder.rb | 14 ++--- lib/jsonapi/resource_common.rb | 95 +++++++++++++++++------------ lib/jsonapi/resource_serializer.rb | 4 +- lib/jsonapi/resource_tree.rb | 5 +- test/controllers/controller_test.rb | 4 ++ 5 files changed, 70 insertions(+), 52 deletions(-) diff --git a/lib/jsonapi/link_builder.rb b/lib/jsonapi/link_builder.rb index d78f414e..23b94a4d 100644 --- a/lib/jsonapi/link_builder.rb +++ b/lib/jsonapi/link_builder.rb @@ -49,8 +49,8 @@ def query_link(query_params) def relationships_related_link(source, relationship, query_params = {}) if relationship._routed - url = "#{ self_link(source) }/#{ route_for_relationship(relationship) }" - url = "#{ url }?#{ query_params.to_query }" if query_params.present? + url = +"#{ self_link(source) }/#{ route_for_relationship(relationship) }" + url << "?#{ query_params.to_query }" if query_params.present? url else if JSONAPI.configuration.warn_on_missing_routes && !relationship._warned_missing_route @@ -129,18 +129,14 @@ def resources_path(source_klass) @_resources_path[source_klass] ||= formatted_module_path_from_class(source_klass) + format_route(source_klass._type.to_s) end - def resource_path(source) + def resource_url(source) if source.class.singleton? - resources_path(source.class) + "#{ base_url }#{ engine_mount_point }#{ resources_path(source.class) }" else - "#{resources_path(source.class)}/#{source.id}" + "#{ base_url }#{ engine_mount_point }#{resources_path(source.class)}/#{source.id}" end end - def resource_url(source) - "#{ base_url }#{ engine_mount_point }#{ resource_path(source) }" - end - def route_for_relationship(relationship) format_route(relationship.name) end diff --git a/lib/jsonapi/resource_common.rb b/lib/jsonapi/resource_common.rb index 5c10f07b..0cd29051 100644 --- a/lib/jsonapi/resource_common.rb +++ b/lib/jsonapi/resource_common.rb @@ -39,7 +39,7 @@ def _model end def id - _model.public_send(self.class._primary_key) + @id ||= _model.public_send(self.class._primary_key) end def identity @@ -510,7 +510,10 @@ def inherited(subclass) subclass._routed = false subclass._warned_missing_route = false - subclass._clear_cached_attribute_options + subclass._attribute_options_cache = {} + subclass._model_class_to_resource_type_cache = {} + subclass._resource_type_to_class_cache = {} + subclass._clear_fields_cache subclass._resource_retrieval_strategy_loaded = @_resource_retrieval_strategy_loaded @@ -533,15 +536,19 @@ def rebuild_relationships(relationships) end def resource_klass_for(type) + @_resource_type_to_class_cache ||= {} type = type.underscore - type_with_module = type.start_with?(module_path) ? type : module_path + type - resource_name = _resource_name_from_type(type_with_module) - resource = resource_name.safe_constantize if resource_name - if resource.nil? - fail NameError, "JSONAPI: Could not find resource '#{type}'. (Class #{resource_name} not found)" + @_resource_type_to_class_cache.fetch(type) do + type_with_module = type.start_with?(module_path) ? type : module_path + type + + resource_name = _resource_name_from_type(type_with_module) + resource_klass = resource_name.safe_constantize if resource_name + if resource_klass.nil? + fail NameError, "JSONAPI: Could not find resource '#{type}'. (Class #{resource_name} not found)" + end + @_resource_type_to_class_cache[type] = resource_klass end - resource end def resource_klass_for_model(model) @@ -553,17 +560,33 @@ def _resource_name_from_type(type) end def resource_type_for(model) - model_name = model.class.to_s.underscore - if _model_hints[model_name] - _model_hints[model_name] - else - model_name.rpartition('/').last + @_model_class_to_resource_type_cache.fetch(model.class) do + model_name = model.class.name.underscore + + resource_type = if _model_hints[model_name] + _model_hints[model_name] + else + model_name.rpartition('/').last + end + + @_model_class_to_resource_type_cache[model.class] = resource_type end end - attr_accessor :_attributes, :_relationships, :_type, :_model_hints, :_routed, :_warned_missing_route, + attr_accessor :_attributes, + :_relationships, + :_type, + :_model_hints, + :_routed, + :_warned_missing_route, :_resource_retrieval_strategy_loaded - attr_writer :_allowed_filters, :_paginator, :_allowed_sort + + attr_writer :_allowed_filters, + :_paginator, + :_allowed_sort, + :_model_class_to_resource_type_cache, + :_resource_type_to_class_cache, + :_attribute_options_cache def create(context) new(create_model, context) @@ -590,7 +613,7 @@ def attributes(*attrs) end def attribute(attribute_name, options = {}) - _clear_cached_attribute_options + _clear_attribute_options_cache _clear_fields_cache attr = attribute_name.to_sym @@ -903,7 +926,7 @@ def verify_relationship_filter(filter, raw, _context = nil) # quasi private class methods def _attribute_options(attr) - @_cached_attribute_options[attr] ||= default_attribute_options.merge(@_attributes[attr]) + @_attribute_options_cache[attr] ||= default_attribute_options.merge(@_attributes[attr]) end def _attribute_delegated_name(attr) @@ -915,11 +938,11 @@ def _has_attribute?(attr) end def _updatable_attributes - _attributes.map { |key, options| key unless options[:readonly] }.compact + _attributes.map { |key, options| key unless options[:readonly] }.delete_if {|v| v.nil? } end def _updatable_relationships - @_relationships.map { |key, relationship| key unless relationship.readonly? }.compact + @_relationships.map { |key, relationship| key unless relationship.readonly? }.delete_if {|v| v.nil? } end def _relationship(type) @@ -1132,11 +1155,11 @@ def _has_sort?(sorting) end def module_path - if name == 'JSONAPI::Resource' - '' - else - name =~ /::[^:]+\Z/ ? ($`.freeze.gsub('::', '/') + '/').underscore : '' - end + @module_path ||= if name == 'JSONAPI::Resource' + '' + else + name =~ /::[^:]+\Z/ ? ($`.freeze.gsub('::', '/') + '/').underscore : '' + end end def default_sort @@ -1169,18 +1192,6 @@ def _add_relationship(klass, *attrs) end end - def _setup_relationship(klass, *attrs) - _clear_fields_cache - - options = attrs.extract_options! - options[:parent_resource] = self - - relationship_name = attrs[0].to_sym - check_duplicate_relationship_name(relationship_name) - - define_relationship_methods(relationship_name.to_sym, klass, options) - end - # ResourceBuilder methods def define_relationship_methods(relationship_name, relationship_klass, options) relationship = register_relationship( @@ -1214,8 +1225,16 @@ def register_relationship(name, relationship_object) @_relationships[name] = relationship_object end - def _clear_cached_attribute_options - @_cached_attribute_options = {} + def _clear_attribute_options_cache + @_attribute_options_cache&.clear + end + + def _clear_model_to_resource_type_cache + @_model_class_to_resource_type_cache&.clear + end + + def _clear_resource_type_to_klass_cache + @_resource_type_to_class_cache&.clear end def _clear_fields_cache diff --git a/lib/jsonapi/resource_serializer.rb b/lib/jsonapi/resource_serializer.rb index 1e7d5102..6f876ffb 100644 --- a/lib/jsonapi/resource_serializer.rb +++ b/lib/jsonapi/resource_serializer.rb @@ -262,7 +262,7 @@ def links_hash(source) if !links.key?('self') && !source.class.exclude_link?(:self) links['self'] = link_builder.self_link(source) end - links.compact + links.delete_if {|k,v| v.nil? } end def custom_links_hash(source) @@ -340,7 +340,7 @@ def default_relationship_links(source, relationship) links = {} links['self'] = self_link(source, relationship) unless relationship.exclude_link?(:self) links['related'] = related_link(source, relationship) unless relationship.exclude_link?(:related) - links.compact + links.delete_if {|k,v| v.nil? } end def to_many_linkage(rids) diff --git a/lib/jsonapi/resource_tree.rb b/lib/jsonapi/resource_tree.rb index 5cbd830e..1e93d3fb 100644 --- a/lib/jsonapi/resource_tree.rb +++ b/lib/jsonapi/resource_tree.rb @@ -68,13 +68,13 @@ def add_resource(resource, include_related) private def init_included_relationships(fragment, include_related) - include_related && include_related.each_key do |relationship_name| + include_related&.each_key do |relationship_name| fragment.initialize_related(relationship_name) end end def load_included(resource_klass, source_resource_tree, include_related, options) - include_related.try(:each_key) do |key| + include_related&.each_key do |key| relationship = resource_klass._relationship(key) relationship_name = relationship.name.to_sym @@ -159,7 +159,6 @@ def initialize(parent_relationship, source_resource_tree) @related_resource_trees ||= {} @parent_relationship = parent_relationship - @parent_relationship_name = parent_relationship.name.to_sym @source_resource_tree = source_resource_tree end diff --git a/test/controllers/controller_test.rb b/test/controllers/controller_test.rb index 12e3cbcc..2fe181d4 100644 --- a/test/controllers/controller_test.rb +++ b/test/controllers/controller_test.rb @@ -4028,6 +4028,8 @@ def test_immutable_update_not_supported class Api::V7::ClientsControllerTest < ActionController::TestCase def test_get_namespaced_model_not_matching_resource_using_model_hint + Api::V7::ClientResource._clear_model_to_resource_type_cache + Api::V7::ClientResource._clear_resource_type_to_klass_cache assert_cacheable_get :index assert_response :success assert_equal 'clients', json_response['data'][0]['type'] @@ -4037,6 +4039,8 @@ def test_get_namespaced_model_not_matching_resource_using_model_hint def test_get_namespaced_model_not_matching_resource_not_using_model_hint Api::V7::ClientResource._model_hints.delete('api/v7/customer') + Api::V7::ClientResource._clear_model_to_resource_type_cache + Api::V7::ClientResource._clear_resource_type_to_klass_cache assert_cacheable_get :index assert_response :success assert_equal 'customers', json_response['data'][0]['type']