Skip to content

Commit ae5291a

Browse files
committed
Add config to disable 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 introduces a new configuration to disable the `sessid` column fallback (which is the source of the `reset_column_information`). Since the `session_id` -> `sessid` fallback has been in place for [twenty years][1], there may be an argument to just remove it, but this gem is very very stable so breaking changes should probably be held to an even higher threshold. [1]: rails/rails@452442d
1 parent 284156d commit ae5291a

File tree

3 files changed

+49
-10
lines changed

3 files changed

+49
-10
lines changed

lib/active_record/session_store.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ module ActiveRecord
88
module SessionStore
99
autoload :Session, 'active_record/session_store/session'
1010

11+
@disable_sessid_fallback = false
12+
singleton_class.attr_accessor :disable_sessid_fallback
13+
1114
module ClassMethods # :nodoc:
1215
mattr_accessor :serializer
1316

lib/active_record/session_store/session.rb

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,19 @@ def data_column_size_limit
2222
@data_column_size_limit ||= columns_hash[data_column_name].limit
2323
end
2424

25-
# Hook to set up sessid compatibility.
26-
def find_by_session_id(session_id)
27-
SEMAPHORE.synchronize { setup_sessid_compatibility! }
28-
find_by_session_id(session_id)
29-
end
30-
31-
private
32-
def session_id_column
33-
'session_id'
25+
if SessionStore.disable_sessid_fallback
26+
def find_by_session_id(session_id)
27+
where(session_id: session_id).first
28+
end
29+
else
30+
# Hook to set up sessid compatibility.
31+
def find_by_session_id(session_id)
32+
SEMAPHORE.synchronize { setup_sessid_compatibility! }
33+
find_by_session_id(session_id)
3434
end
3535

3636
# Compatibility with tables using sessid instead of session_id.
37-
def setup_sessid_compatibility!
37+
private def setup_sessid_compatibility!
3838
# Reset column info since it may be stale.
3939
reset_column_information
4040
if columns_hash['sessid']
@@ -52,6 +52,12 @@ def self.find_by_session_id(session_id)
5252
end
5353
end
5454
end
55+
end
56+
57+
private
58+
def session_id_column
59+
'session_id'
60+
end
5561
end
5662

5763
def initialize(*)

test/session_test.rb

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,36 @@ def self.session_id_column
108108
Session.reset_column_information
109109
end
110110

111+
def test_find_by_sess_id_compat_disabled
112+
old_fallback = ActiveRecord::SessionStore.disable_sessid_fallback
113+
ActiveRecord::SessionStore.disable_sessid_fallback = true
114+
115+
# Force class reload, as we need to redo the meta-programming
116+
ActiveRecord::SessionStore.send(:remove_const, :Session)
117+
load 'active_record/session_store/session.rb'
118+
119+
Session.reset_column_information
120+
klass = Class.new(Session) do
121+
def self.session_id_column
122+
'sessid'
123+
end
124+
end
125+
klass.create_table!
126+
127+
assert klass.columns_hash['sessid'], 'sessid column exists'
128+
session = klass.new(:data => 'hello')
129+
session.sessid = "100"
130+
session.save!
131+
132+
assert_raises(ActiveRecord::StatementInvalid) do
133+
klass.find_by_session_id("100")
134+
end
135+
ensure
136+
klass.drop_table!
137+
Session.reset_column_information
138+
ActiveRecord::SessionStore.disable_sessid_fallback = old_fallback
139+
end
140+
111141
def test_find_by_session_id
112142
Session.create_table!
113143
session_id = "10"

0 commit comments

Comments
 (0)