Skip to content

Commit

Permalink
Merge pull request #47032 from Shopify/shapes-friendliness-2
Browse files Browse the repository at this point in the history
Improve Rails' Shape friendliness (second pass)
  • Loading branch information
byroot committed Jan 17, 2023
2 parents 03a1da9 + 0b42dab commit 9bfb8f5
Show file tree
Hide file tree
Showing 13 changed files with 87 additions and 36 deletions.
18 changes: 13 additions & 5 deletions actionpack/lib/action_controller/metal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,17 @@ def self.action_encoding_template(action) # :nodoc:
false
end

class << self
private
def inherited(subclass)
super
subclass.middleware_stack = middleware_stack.dup
subclass.class_eval do
@controller_name = nil
end
end
end

# Delegates to the class's ::controller_name.
def controller_name
self.class.controller_name
Expand Down Expand Up @@ -166,7 +177,9 @@ def controller_name
def initialize
@_request = nil
@_response = nil
@_response_body = nil
@_routes = nil
@_params = nil
super
end

Expand Down Expand Up @@ -240,11 +253,6 @@ def reset_session

class_attribute :middleware_stack, default: ActionController::MiddlewareStack.new

def self.inherited(base) # :nodoc:
base.middleware_stack = middleware_stack.dup
super
end

class << self
# Pushes the given Rack middleware and its arguments to the bottom of the
# middleware stack.
Expand Down
9 changes: 7 additions & 2 deletions actionview/lib/action_view/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,8 @@ def changed?(other) # :nodoc:
delegate :formats, :formats=, :locale, :locale=, :view_paths, :view_paths=, to: :lookup_context

def assign(new_assigns) # :nodoc:
@_assigns = new_assigns.each { |key, value| instance_variable_set("@#{key}", value) }
@_assigns = new_assigns
new_assigns.each { |key, value| instance_variable_set("@#{key}", value) }
end

# :stopdoc:
Expand All @@ -232,9 +233,13 @@ def initialize(lookup_context, assigns, controller) # :nodoc:
@view_renderer = ActionView::Renderer.new @lookup_context
@current_template = nil

assign(assigns)
assign_controller(controller)
_prepare_context

super()

# Assigns must be called last to minimize the number of shapes
assign(assigns)
end

def _run(method, template, locals, buffer, add_to_stack: true, has_strict_locals: false, &block)
Expand Down
4 changes: 4 additions & 0 deletions actionview/lib/action_view/helpers/controller_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ def assign_controller(controller)
@_request = controller.request if controller.respond_to?(:request)
@_config = controller.config.inheritable_copy if controller.respond_to?(:config)
@_default_form_builder = controller.default_form_builder if controller.respond_to?(:default_form_builder)
else
@_request ||= nil
@_config ||= nil
@_default_form_builder ||= nil
end
end

Expand Down
6 changes: 5 additions & 1 deletion actionview/lib/action_view/test_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,10 @@ def view
:@_result,
:@_routes,
:@controller,
:@_controller,
:@_request,
:@_config,
:@_default_form_builder,
:@_layouts,
:@_files,
:@_rendered_views,
Expand All @@ -243,7 +247,7 @@ def view
:@view_context_class,
:@view_flow,
:@_subscribers,
:@html_document
:@html_document,
]

def _user_defined_ivars
Expand Down
1 change: 1 addition & 0 deletions activemodel/lib/active_model/validations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,7 @@ def validate!(context = nil)
private
def init_internals
super
@errors = nil
@validation_context = nil
end

Expand Down
2 changes: 2 additions & 0 deletions activerecord/lib/active_record/attribute_methods/dirty.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@ def init_internals
super
@mutations_before_last_save = nil
@mutations_from_database = nil
@_touch_attr_names = nil
@_skip_dirty_tracking = nil
end

def _touch_row(attribute_names, time)
Expand Down
12 changes: 5 additions & 7 deletions activerecord/lib/active_record/autosave_association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -154,13 +154,6 @@ def self.valid_options

module ClassMethods # :nodoc:
private
def inherited(base)
super
base.class_eval do
@_already_called = nil
end
end

def define_non_cyclic_method(name, &block)
return if method_defined?(name, false)

Expand Down Expand Up @@ -280,6 +273,11 @@ def changed_for_autosave?
end

private
def init_internals
super
@_already_called = nil
end

# Returns the record for an association collection that should be validated
# or saved. If +autosave+ is +false+ only new records will be returned,
# unless the parent is/was a new record itself.
Expand Down
1 change: 1 addition & 0 deletions activerecord/lib/active_record/persistence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1123,6 +1123,7 @@ def touch(*names, time: nil)
def init_internals
super
@_trigger_destroy_callback = @_trigger_update_callback = nil
@previously_new_record = false
end

def strict_loaded_associations
Expand Down
58 changes: 38 additions & 20 deletions activerecord/lib/active_record/reflection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,14 @@ def inherited(subclass)
# PolymorphicReflection
# RuntimeReflection
class AbstractReflection # :nodoc:
def initialize
@class_name = nil
@counter_cache_column = nil
@inverse_of = nil
@inverse_which_updates_counter_cache_defined = false
@inverse_which_updates_counter_cache = nil
end

def through_reflection?
false
end
Expand Down Expand Up @@ -260,10 +268,13 @@ def check_validity_of_inverse!
#
# Hence this method.
def inverse_which_updates_counter_cache
return @inverse_which_updates_counter_cache if defined?(@inverse_which_updates_counter_cache)
@inverse_which_updates_counter_cache = klass.reflect_on_all_associations(:belongs_to).find do |inverse|
inverse.counter_cache_column == counter_cache_column
unless @inverse_which_updates_counter_cache_defined
@inverse_which_updates_counter_cache = klass.reflect_on_all_associations(:belongs_to).find do |inverse|
inverse.counter_cache_column == counter_cache_column
end
@inverse_which_updates_counter_cache_defined = true
end
@inverse_which_updates_counter_cache
end
alias inverse_updates_counter_cache? inverse_which_updates_counter_cache

Expand Down Expand Up @@ -354,6 +365,7 @@ class MacroReflection < AbstractReflection
attr_reader :plural_name # :nodoc:

def initialize(name, scope, options, active_record)
super()
@name = name
@scope = scope
@options = options
Expand Down Expand Up @@ -457,6 +469,10 @@ def initialize(name, scope, options, active_record)
super
@type = -(options[:foreign_type]&.to_s || "#{options[:as]}_type") if options[:as]
@foreign_type = -(options[:foreign_type]&.to_s || "#{name}_type") if options[:polymorphic]
@join_table = nil
@foreign_key = nil
@association_foreign_key = nil
@association_primary_key = nil

ensure_option_not_given_as_class!(:class_name)
end
Expand Down Expand Up @@ -788,6 +804,7 @@ class ThroughReflection < AbstractReflection # :nodoc:
:active_record_primary_key, :join_foreign_key, to: :source_reflection

def initialize(delegate_reflection)
super()
@delegate_reflection = delegate_reflection
@klass = delegate_reflection.options[:anonymous_class]
@source_reflection_name = delegate_reflection.options[:source]
Expand Down Expand Up @@ -921,24 +938,23 @@ def source_reflection_names
end

def source_reflection_name # :nodoc:
return @source_reflection_name if @source_reflection_name

names = [name.to_s.singularize, name].collect(&:to_sym).uniq
names = names.find_all { |n|
through_reflection.klass._reflect_on_association(n)
}

if names.length > 1
raise AmbiguousSourceReflectionForThroughAssociation.new(
active_record.name,
macro,
name,
options,
source_reflection_names
)
@source_reflection_name ||= begin
names = [name.to_s.singularize, name].collect(&:to_sym).uniq
names = names.find_all { |n|
through_reflection.klass._reflect_on_association(n)
}

if names.length > 1
raise AmbiguousSourceReflectionForThroughAssociation.new(
active_record.name,
macro,
name,
options,
source_reflection_names
)
end
names.first
end

@source_reflection_name = names.first
end

def source_options
Expand Down Expand Up @@ -1042,6 +1058,7 @@ class PolymorphicReflection < AbstractReflection # :nodoc:
:name, :scope_for, to: :@reflection

def initialize(reflection, previous_reflection)
super()
@reflection = reflection
@previous_reflection = previous_reflection
end
Expand All @@ -1067,6 +1084,7 @@ class RuntimeReflection < AbstractReflection # :nodoc:
delegate :scope, :type, :constraints, :join_foreign_key, to: :@reflection

def initialize(reflection, association)
super()
@reflection = reflection
@association = association
end
Expand Down
5 changes: 5 additions & 0 deletions activerecord/lib/active_record/touch_later.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ def touch(*names, time: nil) # :nodoc:
end

private
def init_internals
super
@_defer_touch_attrs = nil
end

def surreptitiously_touch(attr_names)
attr_names.each do |attr_name|
_write_attribute(attr_name, @_touch_time)
Expand Down
2 changes: 2 additions & 0 deletions activerecord/lib/active_record/transactions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,9 @@ def trigger_transactional_callbacks? # :nodoc:

def init_internals
super
@_start_transaction_state = nil
@_committed_already_called = nil
@_new_record_before_last_commit = nil
end

# Save the new record state and id of a record so it can be restored later if a transaction fails.
Expand Down
2 changes: 2 additions & 0 deletions activesupport/lib/active_support/encrypted_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ def message
def initialize(config_path:, key_path:, env_key:, raise_if_missing_key:)
super content_path: config_path, key_path: key_path,
env_key: env_key, raise_if_missing_key: raise_if_missing_key
@config = nil
@options = nil
end

# Reads the file and returns the decrypted content. See EncryptedFile#read.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ def self.convert(number, options)

def initialize(number, options)
@number = number
@opts = options.symbolize_keys
@opts = options.symbolize_keys
@options = nil
end

def execute
Expand Down

0 comments on commit 9bfb8f5

Please sign in to comment.