Skip to content

Commit

Permalink
V0 11 dev performance (#1431)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
lgebhardt committed Apr 18, 2024
1 parent 52da65e commit c052d46
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 52 deletions.
14 changes: 5 additions & 9 deletions lib/jsonapi/link_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
95 changes: 57 additions & 38 deletions lib/jsonapi/resource_common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/jsonapi/resource_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions lib/jsonapi/resource_tree.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions test/controllers/controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand All @@ -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']
Expand Down

0 comments on commit c052d46

Please sign in to comment.