Skip to content

Use Rails session store options in table tasks and migrations #157

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

Merged
merged 5 commits into from
Nov 11, 2024
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ Unreleased Changes

* Issue - `ActionDispatch::Session::DynamoDbStore` now inherits `ActionDispatch::Session::AbstractStore` by wrapping `Aws::SessionStore::DynamoDB::RackMiddleware`.

* Issue - `DynamoDbStore` is now configured with the `:dynamo_db_store` configuration instead of `:dynamodb_store`.

* Feature - `DYNAMO_DB_SESSION_CONFIG_FILE` is now searched and with precedence over the default Rails configuration YAML file locations.

* Feature - Session Store configuration passed into `:dynamo_db_store` will now be considered when using the ActiveRecord migrations or rake tasks that create, delete, or clean session tables.

* Issue - Do not skip autoload modules for `Aws::Rails.instrument_sdk_operations`.

4.1.0 (2024-09-27)
Expand Down
3 changes: 2 additions & 1 deletion lib/action_dispatch/session/dynamo_db_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ def delete_session(req, sid, options)
end

def config_file
file = Rails.root.join("config/dynamo_db_session_store/#{Rails.env}.yml")
file = ENV.fetch('DYNAMO_DB_SESSION_CONFIG_FILE', nil)
file ||= Rails.root.join("config/dynamo_db_session_store/#{Rails.env}.yml")
file = Rails.root.join('config/dynamo_db_session_store.yml') unless File.exist?(file)
file if File.exist?(file)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@

class <%= name.camelize %> < ActiveRecord::Migration[<%= migration_version %>]
def up
Aws::SessionStore::DynamoDB::Table.create_table
options = Rails.application.config.session_options
Aws::SessionStore::DynamoDB::Table.create_table(options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably dumb question - does this need to be idempotent? (Ie check if table already exists/update?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create_table in this gem is already idempotent, sorta. It uses options to construct create table options and will rescue ResourceInUseException (DDB validates). https://github.com/aws/aws-sessionstore-dynamodb-ruby/blob/main/lib/aws/session_store/dynamo_db/table.rb

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate that this is idempotent so I can easily include in code which gets executed at every bin/dev and/or deploy.

end

def down
Aws::SessionStore::DynamoDB::Table.delete_table
options = Rails.application.config.session_options
Aws::SessionStore::DynamoDB::Table.delete_table(options)
end
end
9 changes: 6 additions & 3 deletions lib/tasks/dynamo_db/session_store.rake
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,20 @@ namespace 'dynamo_db' do
namespace 'session_store' do
desc 'Create the Amazon DynamoDB session store table'
task create_table: :environment do
Aws::SessionStore::DynamoDB::Table.create_table
options = Rails.application.config.session_options
Aws::SessionStore::DynamoDB::Table.create_table(options)
end

desc 'Delete the Amazon DynamoDB session store table'
task delete_table: :environment do
Aws::SessionStore::DynamoDB::Table.delete_table
options = Rails.application.config.session_options
Aws::SessionStore::DynamoDB::Table.delete_table(options)
end

desc 'Clean up old sessions in the Amazon DynamoDB session store table'
task clean: :environment do
Aws::SessionStore::DynamoDB::GarbageCollection.collect_garbage
options = Rails.application.config.session_options
Aws::SessionStore::DynamoDB::GarbageCollection.collect_garbage(options)
end
end
end

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# frozen_string_literal: true

class CreateDynamoDbSessionsTable < ActiveRecord::Migration[7.2]
def up
options = Rails.application.config.session_options
Aws::SessionStore::DynamoDB::Table.create_table(options)
end

def down
options = Rails.application.config.session_options
Aws::SessionStore::DynamoDB::Table.delete_table(options)
end
end
2 changes: 1 addition & 1 deletion sample-app/db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.2].define(version: 2024_10_28_193427) do
ActiveRecord::Schema[7.2].define(version: 2024_11_06_152613) do
create_table "users", force: :cascade do |t|
t.string "email"
t.string "password_digest"
Expand Down
Binary file modified sample-app/storage/development.sqlite3
Binary file not shown.
26 changes: 22 additions & 4 deletions test/action_dispatch/session/dynamo_db_store_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,21 @@ class DynamoDbStoreTest < ActiveSupport::TestCase
{ dynamo_db_client: Aws::DynamoDB::Client.new(stub_responses: true) }
end

def setup_env
ENV['DYNAMO_DB_SESSION_CONFIG_FILE'] = 'test/dummy/config/session_store.yml'
end

def teardown_env
ENV.delete('DYNAMO_DB_SESSION_CONFIG_FILE')
end

it 'loads config file' do
store = ActionDispatch::Session::DynamoDbStore.new(nil, options)
config_file_path = store.config.config_file.to_s
assert_match(/dynamo_db_session_store.yml/, config_file_path)
end

it 'loads environment config file and with precedence' do
it 'loads environment specific config files with precedence' do
# Set Rails.env to something else so the environment.yml file is loaded
old_env = Rails.env
Rails.env = 'development'
Expand All @@ -30,19 +38,29 @@ class DynamoDbStoreTest < ActiveSupport::TestCase
Rails.env = old_env
end

it 'allows config file override' do
it 'loads a config file from the environment variable with precedence' do
setup_env
store = ActionDispatch::Session::DynamoDbStore.new(nil, options)
config_file_path = store.config.config_file.to_s
assert_match(/session_store.yml/, config_file_path)
teardown_env
end

it 'allows overriding the config file at runtime with highest precedence' do
setup_env
options[:config_file] = 'test/dummy/config/session_store.yml'
store = ActionDispatch::Session::DynamoDbStore.new(nil, options)
config_file_path = store.config.config_file.to_s
assert_match(/session_store.yml/, config_file_path)
teardown_env
end

it 'uses rails secret key base' do
it 'uses the rails secret key base' do
store = ActionDispatch::Session::DynamoDbStore.new(nil, options)
assert_equal store.config.secret_key, Rails.application.secret_key_base
end

it 'allows secret key override' do
it 'allows for secret key to be overridden' do
secret_key = 'SECRET_KEY'
options[:secret_key] = secret_key
store = ActionDispatch::Session::DynamoDbStore.new(nil, options)
Expand Down
11 changes: 10 additions & 1 deletion test/tasks/dynamo_db/session_store_rake_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ class SessionStoreRakeTest < ActiveSupport::TestCase
before do
Rake.application.rake_require 'tasks/dynamo_db/session_store'
Rake::Task.define_task(:environment)
# MiniTest has an issue with kwargs in 2.7
# https://github.com/minitest/minitest/blob/master/lib/minitest/mock.rb#L293C8-L293C30
ENV["MT_KWARGS_HAC\K"] = '1' if RUBY_VERSION < '3'
end

after do
ENV.delete("MT_KWARGS_HAC\K")
end

# Functionality for these methods are tested in aws-sessionstore-dynamodb.
Expand All @@ -23,7 +30,9 @@ def expect_mock(method, task)
end

mock = MiniTest::Mock.new
mock.expect(:call, nil)
# After removing ENV["MT_KWARGS_HAC\K"], this can be stronger by asserting
# Rails.application.config.session_options is passed to the method.
mock.expect(:call, nil, [Hash])
klass.stub(method, mock) { Rake.application.invoke_task task }
assert_mock mock
end
Expand Down