Skip to content

Commit 6892241

Browse files
committed
Deprecate sessid column fallback
I'm working on removing queries in my application that should be going through the SchemaCache in production but currently are not. I found that the `sessions` table is being queried during runtime due to the `reset_column_information` when the class is loaded (which clears the SchemaCache for the `sessions` table). This commit deprecates the cause of the schema cache clearing: the `sessid` fallback. This code checks the model's columns and conditionally redefines methods to allow using `sessid` instead of `session_id`. The `session_id` -> `sessid` fallback has been in place for [twenty years][1], I'd be surprised if there's even a single app still using `sessid`. Also of note: many `app.deprecators` implementations check that the `app` responds to `deprecators` (which was added in Rails 7.1), but that's our minimum version so we can skip it. [1]: rails/rails@452442d
1 parent 284156d commit 6892241

5 files changed

Lines changed: 20 additions & 1 deletion

File tree

lib/active_record/session_store.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
require 'active_record'
22
require 'active_record/session_store/version'
33
require 'action_dispatch/session/active_record_store'
4+
require 'active_support'
45
require 'active_support/core_ext/hash/keys'
56
require 'json'
67

78
module ActiveRecord
89
module SessionStore
910
autoload :Session, 'active_record/session_store/session'
1011

12+
class << self
13+
def deprecator
14+
@deprecator ||= ActiveSupport::Deprecation.new("3.0", "ActiveRecord::SessionStore")
15+
end
16+
end
17+
1118
module ClassMethods # :nodoc:
1219
mattr_accessor :serializer
1320

lib/active_record/session_store/railtie.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ module ActiveRecord
44
module SessionStore
55
class Railtie < Rails::Railtie
66
rake_tasks { load File.expand_path("../../../tasks/database.rake", __FILE__) }
7+
8+
initializer "activerecord-session_store.deprecator" do |app|
9+
app.deprecators[:"activerecord-session_store"] = SessionStore.deprecator
10+
end
711
end
812
end
913
end

lib/active_record/session_store/session.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ def setup_sessid_compatibility!
3838
# Reset column info since it may be stale.
3939
reset_column_information
4040
if columns_hash['sessid']
41+
SessionStore.deprecator.warn <<~MSG
42+
Using a session ID column other than `session_id` is deprecated without replacement. You should migrate your session table to use `session_id`.
43+
MSG
44+
4145
def self.find_by_session_id(session_id)
4246
find_by_sessid(session_id)
4347
end

test/helper.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
require 'active_record/session_store'
99

10+
ActiveRecord::SessionStore.deprecator.behavior = :raise
11+
1012
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
1113

1214
SharedTestRoutes = ActionDispatch::Routing::RouteSet.new

test/session_test.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,9 @@ def self.session_id_column
100100
session.sessid = "100"
101101
session.save!
102102

103-
found = klass.find_by_session_id("100")
103+
found = assert_deprecated(ActiveRecord::SessionStore.deprecator) do
104+
klass.find_by_session_id("100")
105+
end
104106
assert_equal session, found
105107
assert_equal session.sessid, found.session_id
106108
ensure

0 commit comments

Comments
 (0)