Skip to content

Commit 52f80b3

Browse files
committed
Move schema loading monitor back up to model
We want to ensure attributes aren't defined concurrently, which per-context monitors wouldn't prevent
1 parent a870d4e commit 52f80b3

2 files changed

Lines changed: 51 additions & 38 deletions

File tree

activerecord/lib/active_record/model_schema.rb

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -544,16 +544,38 @@ def reset_column_information
544544

545545
# Load the model's schema information either from the schema cache
546546
# or directly from the database.
547+
#
548+
# This separates two concerns:
549+
# 1. Adapter-specific: populating the current Schema context's columns_hash
550+
# and default_attributes (runs per context, since different adapters may
551+
# return different column types).
552+
# 2. Model-specific: the load_schema! super chain (counter_cache, encryption,
553+
# etc.) that mutates model-class-level state (runs once, since it's the
554+
# same regardless of adapter).
547555
def load_schema
548-
model_schema.load_schema
556+
return if schema_loaded?
557+
@load_schema_monitor.synchronize do
558+
return if schema_loaded?
559+
560+
model_schema.load_schema!
561+
562+
unless @model_schema_loaded
563+
load_schema!
564+
@model_schema_loaded = true
565+
end
566+
rescue
567+
reload_schema_from_cache # If the schema loading failed half way through, we must reset the state.
568+
raise
569+
end
549570
end
550571

551572
protected
552573
def initialize_load_schema_monitor
553-
# No longer needed - each Schema has its own monitor
574+
@load_schema_monitor = Monitor.new
554575
end
555576

556577
def reload_schema_from_cache(recursive = true)
578+
@model_schema_loaded = false
557579
@attribute_names = nil
558580

559581
# Reset all Schema instances
@@ -578,11 +600,17 @@ def inherited(child_class)
578600
end
579601

580602
def schema_loaded?
581-
model_schema.send(:schema_loaded?)
603+
model_schema.instance_variable_get(:@schema_loaded)
582604
end
583605

584606
def load_schema!
585-
model_schema.send(:load_schema!)
607+
# Base implementation is a no-op. The adapter-specific schema loading
608+
# (columns_hash, default_attributes) is handled by model_schema.load_schema!
609+
# which is called directly from load_schema before this super chain.
610+
#
611+
# This method exists as the hook point for the super chain — overrides
612+
# in CounterCache, EncryptableRecord, etc. call super and then do
613+
# model-class-level setup that should only happen once.
586614
end
587615

588616
# Guesses the table name, but does not decorate it with prefix and suffix information.

activerecord/lib/active_record/model_schema/schema.rb

Lines changed: 19 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
# frozen_string_literal: true
22

3-
require "monitor"
4-
53
module ActiveRecord
64
module ModelSchema
75
# Encapsulates all schema-context-dependent state for a model.
@@ -17,7 +15,6 @@ def initialize(model_class, context_key)
1715
@model_class = model_class
1816
@context_key = context_key
1917
@schema_loaded = false
20-
@load_schema_monitor = Monitor.new
2118

2219
# Schema-context-dependent state
2320
@columns_hash = nil
@@ -41,7 +38,7 @@ def initialize(model_class, context_key)
4138

4239
# Returns the columns hash for this schema context
4340
def columns_hash
44-
load_schema unless @columns_hash
41+
model_class.send(:load_schema) unless @columns_hash
4542
@columns_hash
4643
end
4744

@@ -115,7 +112,7 @@ def attributes_builder
115112

116113
# Returns column defaults hash
117114
def column_defaults
118-
load_schema
115+
model_class.send(:load_schema)
119116
@column_defaults ||= _default_attributes.deep_dup.to_hash.freeze
120117
end
121118

@@ -196,42 +193,30 @@ def initialize_find_by_cache
196193
@find_by_statement_cache = { true => Concurrent::Map.new, false => Concurrent::Map.new }
197194
end
198195

199-
# Load schema information from the schema cache
200-
def load_schema
201-
return if schema_loaded?
202-
@load_schema_monitor.synchronize do
203-
return if schema_loaded?
196+
# Populate this schema context's columns and default attributes
197+
# from the schema cache. Called by the model class's load_schema!.
198+
def load_schema!
199+
return if @schema_loaded
204200

205-
model_class.send(:load_schema!)
206-
@schema_loaded = true
207-
rescue
208-
reload_schema_from_cache
209-
raise
201+
unless table_name
202+
raise ActiveRecord::TableNotSpecified, "#{model_class} has no table configured. Set one with #{model_class}.table_name="
210203
end
211-
end
212204

213-
private
214-
def schema_loaded?
215-
@schema_loaded
205+
columns_hash = schema_cache.columns_hash(table_name)
206+
if model_class.only_columns.present?
207+
columns_hash = columns_hash.slice(*model_class.only_columns)
208+
elsif model_class.ignored_columns.present?
209+
columns_hash = columns_hash.except(*model_class.ignored_columns)
216210
end
211+
@columns_hash = columns_hash.freeze
217212

218-
def load_schema!
219-
unless table_name
220-
raise ActiveRecord::TableNotSpecified, "#{model_class} has no table configured. Set one with #{model_class}.table_name="
221-
end
213+
# Precompute default attributes to cache DB-dependent attribute types
214+
_default_attributes
222215

223-
columns_hash = schema_cache.columns_hash(table_name)
224-
if model_class.only_columns.present?
225-
columns_hash = columns_hash.slice(*model_class.only_columns)
226-
elsif model_class.ignored_columns.present?
227-
columns_hash = columns_hash.except(*model_class.ignored_columns)
228-
end
229-
@columns_hash = columns_hash.freeze
230-
231-
# Precompute default attributes to cache DB-dependent attribute types
232-
_default_attributes
233-
end
216+
@schema_loaded = true
217+
end
234218

219+
private
235220
def schema_cache
236221
connection_pool.schema_cache
237222
end

0 commit comments

Comments
 (0)