Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a test setup similar to actiondispatch's Cookie Store tests #221

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 6 additions & 13 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,14 @@ jobs:
strategy:
fail-fast: false
matrix:
ruby: ['2.7', '3.0', '3.1', '3.2', '3.3']
rails: ['7.0', '7.1', '7.2', '8.0', 'edge']
exclude:
- ruby: '2.7'
rails: '7.2'
ruby: ['3.1', '3.2', '3.3']
rails: ['7.1', '7.2', '8.0', 'edge']
include:
- ruby: '2.7'
rails: '8.0'
- ruby: '2.7'
rails: 'edge'
rails: '7.1'
- ruby: '3.0'
rails: '7.2'
- ruby: '3.0'
rails: '8.0'
- ruby: '3.0'
rails: 'edge'
rails: '7.1'
exclude:
- ruby: '3.1'
rails: '8.0'
- ruby: '3.1'
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## Unreleased

* Drop Rails 7.0 support.

## 2.2.0

* Drop dependency on `multi_json`.
8 changes: 4 additions & 4 deletions activerecord-session_store.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Gem::Specification.new do |s|
s.version = ActiveRecord::SessionStore::VERSION
s.summary = 'An Action Dispatch session store backed by an Active Record class.'

s.required_ruby_version = '>= 2.5.0'
s.required_ruby_version = '>= 2.7.0'
s.license = 'MIT'

s.author = 'David Heinemeier Hansson'
Expand All @@ -23,9 +23,9 @@ Gem::Specification.new do |s|
s.extra_rdoc_files = %w( README.md )
s.rdoc_options.concat ['--main', 'README.md']

s.add_dependency('activerecord', '>= 7.0')
s.add_dependency('actionpack', '>= 7.0')
s.add_dependency('railties', '>= 7.0')
s.add_dependency('activerecord', '>= 7.1')
s.add_dependency('actionpack', '>= 7.1')
s.add_dependency('railties', '>= 7.1')
s.add_dependency('rack', '>= 2.0.8', '< 4')
s.add_dependency('cgi', '>= 0.3.6')
end
9 changes: 0 additions & 9 deletions gemfiles/rails_6.1.gemfile

This file was deleted.

9 changes: 0 additions & 9 deletions gemfiles/rails_7.0.gemfile

This file was deleted.

12 changes: 9 additions & 3 deletions test/action_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,9 @@ def test_prevents_session_fixation
end

def test_allows_session_fixation
with_test_route_set(:cookie_only => false) do
session_options(cookie_only: false)

with_test_route_set do
get '/set_session_value'
assert_response :success
assert cookies['_session_id']
Expand Down Expand Up @@ -238,7 +240,9 @@ def test_incoming_invalid_session_id_via_cookie_should_be_ignored
end

def test_incoming_invalid_session_id_via_parameter_should_be_ignored
with_test_route_set(:cookie_only => false) do
session_options(cookie_only: false)

with_test_route_set do
open_session do |sess|
sess.get '/set_session_value', :params => { :_session_id => 'INVALID' }
new_session_id = sess.cookies['_session_id']
Expand All @@ -252,7 +256,9 @@ def test_incoming_invalid_session_id_via_parameter_should_be_ignored
end

def test_session_store_with_all_domains
with_test_route_set(:domain => :all) do
session_options(domain: :all)

with_test_route_set do
get '/set_session_value'
assert_response :success
end
Expand Down
33 changes: 21 additions & 12 deletions test/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def config
class ActionDispatch::IntegrationTest < ActiveSupport::TestCase
include ActionDispatch::SharedRoutes

def self.build_app(routes, options)
def self.build_app(routes = nil)
RoutedRackApp.new(routes || ActionDispatch::Routing::RouteSet.new) do |middleware|
middleware.use ActionDispatch::DebugExceptions
middleware.use ActionDispatch::ActionableExceptions
Expand All @@ -53,16 +53,25 @@ def self.build_app(routes, options)
middleware.use ActionDispatch::Flash
middleware.use Rack::MethodOverride
middleware.use Rack::Head
middleware.use ActionDispatch::Session::ActiveRecordStore, options.reverse_merge(key: "_session_id")
yield(middleware) if block_given?
end
end

self.app = build_app(nil, {})
self.app = build_app

private

def with_test_route_set(options = {})
def session_options(options = {})
(@session_options ||= {key: "_session_id"}).merge!(options)
end

def app
@app ||= self.class.build_app do |middleware|
middleware.use ActionDispatch::Session::ActiveRecordStore, session_options
end
end

def with_test_route_set
controller_namespace = self.class.to_s.underscore
actions = %w[set_session_value get_session_value call_reset_session renew get_session_id]

Expand All @@ -71,14 +80,7 @@ def with_test_route_set(options = {})
actions.each { |action| get action, controller: "#{controller_namespace}/test" }
end

old_app = self.class.app
begin
self.class.app = self.class.build_app(set, options)

yield
ensure
self.class.app = old_app
end
yield
end
end

Expand All @@ -89,6 +91,13 @@ def with_store(class_name)
ensure
ActionDispatch::Session::ActiveRecordStore.session_class = session_class
end

# Patch in support for with_routing for integration tests, which was introduced in Rails 7.2
if !defined?(ActionDispatch::Assertions::RoutingAssertions::WithIntegrationRouting)
require_relative 'with_integration_routing_patch'

include WithIntegrationRoutingPatch
end
end

ActiveSupport::TestCase.test_order = :random
62 changes: 62 additions & 0 deletions test/with_integration_routing_patch.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# Bring with_routing method support for integration tests back to Rails 7.1.
# We can remove this when we drop support for Rails 7.1.
# See: https://github.com/rails/rails/pull/49819
module WithIntegrationRoutingPatch # :nodoc:
extend ActiveSupport::Concern

module ClassMethods
def with_routing(&block)
old_routes = nil
old_integration_session = nil

setup do
old_routes = app.routes
old_integration_session = integration_session
create_routes(&block)
end

teardown do
reset_routes(old_routes, old_integration_session)
end
end
end

def with_routing(&block)
old_routes = app.routes
old_integration_session = integration_session
create_routes(&block)
ensure
reset_routes(old_routes, old_integration_session)
end

private

def create_routes
app = self.app
routes = ActionDispatch::Routing::RouteSet.new
rack_app = app.config.middleware.build(routes)
https = integration_session.https?
host = integration_session.host

app.instance_variable_set(:@routes, routes)
app.instance_variable_set(:@app, rack_app)
@integration_session = Class.new(ActionDispatch::Integration::Session) do
include app.routes.url_helpers
include app.routes.mounted_helpers
end.new(app)
@integration_session.https! https
@integration_session.host! host
@routes = routes

yield routes
end

def reset_routes(old_routes, old_integration_session)
old_rack_app = app.config.middleware.build(old_routes)

app.instance_variable_set(:@routes, old_routes)
app.instance_variable_set(:@app, old_rack_app)
@integration_session = old_integration_session
@routes = old_routes
end
end