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

Add Rails 7.1 testing #35

Merged
Merged
Show file tree
Hide file tree
Changes from 10 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
3 changes: 3 additions & 0 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,16 @@ jobs:
- gemfiles/rails_6_0.gemfile
- gemfiles/rails_6_1.gemfile
- gemfiles/rails_7_0.gemfile
- gemfiles/rails_7_1.gemfile
exclude:
- ruby: '3.1'
gemfile: gemfiles/rails_5_2.gemfile
- ruby: '3.0'
gemfile: gemfiles/rails_5_2.gemfile
- ruby: '2.6'
gemfile: gemfiles/rails_7_0.gemfile
- ruby: '2.6'
gemfile: gemfiles/rails_7_1.gemfile

steps:
- uses: actions/checkout@v2
Expand Down
2 changes: 1 addition & 1 deletion .ruby-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2.7.5
3.2.2
6 changes: 6 additions & 0 deletions Appraisals
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

appraise 'rails-5-2' do
gem 'railties', '~> 5.2.0'
end
Expand All @@ -13,3 +15,7 @@ end
appraise 'rails-7-0' do
gem 'railties', '~> 7.0.0'
end

appraise 'rails-7-1' do
gem 'railties', '~> 7.1.0'
end
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

source 'https://rubygems.org'

gemspec
2 changes: 2 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

begin
require 'bundler/setup'
rescue LoadError
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/concerns/journaled/actor.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Journaled::Actor
extend ActiveSupport::Concern

Expand Down
2 changes: 2 additions & 0 deletions app/jobs/journaled/application_job.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Journaled
class ApplicationJob < Journaled.job_base_class_name.constantize
end
Expand Down
4 changes: 3 additions & 1 deletion app/jobs/journaled/delivery_job.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# frozen_string_literal: true

module Journaled
class DeliveryJob < ApplicationJob
DEFAULT_REGION = 'us-east-1'.freeze
DEFAULT_REGION = 'us-east-1'

rescue_from(Aws::Kinesis::Errors::InternalFailure, Aws::Kinesis::Errors::ServiceUnavailable, Aws::Kinesis::Errors::Http503Error) do |e|
Rails.logger.error "Kinesis Error - Server Error occurred - #{e.class}"
Expand Down
2 changes: 2 additions & 0 deletions app/models/concerns/journaled/changes.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Journaled::Changes
extend ActiveSupport::Concern

Expand Down
2 changes: 2 additions & 0 deletions app/models/journaled/actor_uri_provider.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

class Journaled::ActorUriProvider
include Singleton

Expand Down
4 changes: 3 additions & 1 deletion app/models/journaled/audit_log/event.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# FIXME: This cannot be included in lib/ because Journaled::Event is autoloaded via app/models
# Autoloading Journaled::Event isn't strictly necessary, and for compatibility it would
# make sense to move it to lib/.
Expand Down Expand Up @@ -83,7 +85,7 @@ def filter_params

def filtered_attributes
attrs = record.attributes.dup.symbolize_keys
attrs.each do |key, _value|
attrs.each_key do |key|
attrs[key] = '[FILTERED]' if filter_key?(key)
end
end
Expand Down
2 changes: 2 additions & 0 deletions app/models/journaled/change.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

class Journaled::Change
include Journaled::Event

Expand Down
2 changes: 2 additions & 0 deletions app/models/journaled/change_definition.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

class Journaled::ChangeDefinition
attr_reader :attribute_names, :logical_operation

Expand Down
2 changes: 2 additions & 0 deletions app/models/journaled/change_writer.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

class Journaled::ChangeWriter
attr_reader :model, :change_definition

Expand Down
2 changes: 2 additions & 0 deletions app/models/journaled/event.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Journaled::Event
extend ActiveSupport::Concern

Expand Down
2 changes: 2 additions & 0 deletions app/models/journaled/json_schema_model/validator.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

class Journaled::JsonSchemaModel::Validator
def initialize(schema_name)
@schema_name = schema_name
Expand Down
2 changes: 2 additions & 0 deletions app/models/journaled/not_truly_exceptional_error.rb
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
# frozen_string_literal: true

class Journaled::NotTrulyExceptionalError < RuntimeError
end
2 changes: 2 additions & 0 deletions app/models/journaled/writer.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

class Journaled::Writer
EVENT_METHOD_NAMES = %i(
journaled_schema_name
Expand Down
2 changes: 2 additions & 0 deletions bin/rails
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#!/usr/bin/env ruby
# frozen_string_literal: true

# This command will automatically be run when you run "rails" with Rails 4 gems installed from the root of your application.

ENGINE_ROOT = File.expand_path('..', __dir__)
Expand Down
2 changes: 2 additions & 0 deletions config/initializers/change_protection.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

if Rails::VERSION::MAJOR > 5 || (Rails::VERSION::MAJOR == 5 && Rails::VERSION::MINOR >= 2)
require 'journaled/relation_change_protection'
ActiveRecord::Relation.class_eval { prepend Journaled::RelationChangeProtection }
Expand Down
2 changes: 2 additions & 0 deletions config/spring.rb
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
# frozen_string_literal: true

Spring.application_root = './spec/dummy'
2 changes: 2 additions & 0 deletions gemfiles/rails_5_2.gemfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# This file was generated by Appraisal

source "https://rubygems.org"
Expand Down
2 changes: 2 additions & 0 deletions gemfiles/rails_6_0.gemfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# This file was generated by Appraisal

source "https://rubygems.org"
Expand Down
2 changes: 2 additions & 0 deletions gemfiles/rails_6_1.gemfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# This file was generated by Appraisal

source "https://rubygems.org"
Expand Down
2 changes: 2 additions & 0 deletions gemfiles/rails_7_0.gemfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# This file was generated by Appraisal

source "https://rubygems.org"
Expand Down
9 changes: 9 additions & 0 deletions gemfiles/rails_7_1.gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# frozen_string_literal: true

# This file was generated by Appraisal

source "https://rubygems.org"

gem "railties", "~> 7.1.0"

gemspec path: "../"
4 changes: 3 additions & 1 deletion journaled.gemspec
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

$LOAD_PATH.push File.expand_path('lib', __dir__)

# Maintain your gem's version:
Expand Down Expand Up @@ -34,7 +36,7 @@ Gem::Specification.new do |s|
s.add_development_dependency "rspec-rails"
s.add_development_dependency "spring"
s.add_development_dependency "spring-commands-rspec"
s.add_development_dependency "sqlite3"
s.add_development_dependency "sqlite3", '~> 1.4'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was facing some CI troubles when this was unpinned, e.g. https://github.com/Betterment/journaled/actions/runs/8727969396/job/23946685922. I'm guessing this hadn't surfaced before because the newer versions didn't exist(?)

s.add_development_dependency "timecop"
s.add_development_dependency "uncruft"
s.add_development_dependency "webmock"
Expand Down
2 changes: 2 additions & 0 deletions lib/journaled.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require "aws-sdk-kinesis"
require "active_job"
require "json-schema"
Expand Down
12 changes: 11 additions & 1 deletion lib/journaled/audit_log.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'active_support/core_ext/module/attribute_accessors_per_thread'

module Journaled
Expand Down Expand Up @@ -52,7 +54,7 @@ def without_audit_logging
private

def zeitwerk_exclude!(name)
name.constantize.skip_audit_log if Object.const_defined?(name)
name.constantize.skip_audit_log if Object.const_defined?(name) && !independent_class_in_7_1?(name)
Rails.autoloaders.main.on_load(name) { |klass, _path| klass.skip_audit_log }
end

Expand All @@ -61,6 +63,14 @@ def classic_exclude!(name)
rescue NameError
nil
end

def independent_class_in_7_1?(name)
return false if Gem::Version.new(Rails.version) < Gem::Version.new('7.1')

# These classes do not inherit from ActiveRecord::Base in 7.1
# so calling `skip_audit_log` on them will raise.
%w(ActiveRecord::InternalMetadata ActiveRecord::SchemaMigration).include?(name)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am assuming the original reason for skipping these was that we didn't want them to have the callbacks defined in this module, but that they would have them because we include the module in the on_load hook for AR (i.e. in 7.1, since these classes don't inherit from Base, then they don't get the methods after all)

Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to exclude these from DEFAULT_EXCLUDED_CLASSES at class load time if we're on 7.1 or greater?

Copy link
Member

Choose a reason for hiding this comment

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

(And, yeah, that was the goal. If they don't inherit, then they don't need to be included.)

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, we could check .respond_to?(:skip_audit_log), but I would start with the more explicit list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes total sense; I updated the const's assignment and removed this method! I felt like inlining the array for both branches of the if-expr wasn't that bad/verbose (and decided to drop it by a line to avoid having a huge indent).

end
end

Config = Struct.new(:enabled, :ignored_columns, :enqueue_opts) do
Expand Down
2 changes: 2 additions & 0 deletions lib/journaled/connection.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Journaled
module Connection
class << self
Expand Down
2 changes: 2 additions & 0 deletions lib/journaled/current.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Journaled
class Current < ActiveSupport::CurrentAttributes
attribute :tags
Expand Down
2 changes: 2 additions & 0 deletions lib/journaled/engine.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Journaled
class Engine < ::Rails::Engine
config.after_initialize do
Expand Down
2 changes: 2 additions & 0 deletions lib/journaled/errors.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Journaled
class TransactionSafetyError < StandardError; end
end
2 changes: 2 additions & 0 deletions lib/journaled/relation_change_protection.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Journaled::RelationChangeProtection
def update_all(updates, opts = { force: false }) # rubocop:disable Metrics/AbcSize
unless opts[:force] || !@klass.respond_to?(:journaled_attribute_names) || @klass.journaled_attribute_names.empty?
Expand Down
2 changes: 2 additions & 0 deletions lib/journaled/rspec.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'rspec/expectations'

RSpec::Matchers.define :journal_changes_to do |*attribute_names, as:|
Expand Down
2 changes: 2 additions & 0 deletions lib/journaled/transaction_ext.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'active_record/connection_adapters/abstract/transaction'

module Journaled
Expand Down
4 changes: 3 additions & 1 deletion lib/journaled/version.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Journaled
VERSION = "5.3.1".freeze
VERSION = "5.3.2"
Copy link
Contributor Author

@argvniyx-enroute argvniyx-enroute Apr 17, 2024

Choose a reason for hiding this comment

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

I'm only bumping patch because our actual dependencies were already sitting on 7.1 versions and I'm not dropping support for anything. We simply didn't actually test them in CI.

end
2 changes: 2 additions & 0 deletions spec/dummy/Rakefile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# Add your own tasks in files placed in lib/tasks ending in .rake,
# for example lib/tasks/capistrano.rake, and they will automatically be available to Rake.

Expand Down
2 changes: 2 additions & 0 deletions spec/dummy/bin/bundle
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#!/usr/bin/env ruby
# frozen_string_literal: true

ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../Gemfile', __dir__)
load Gem.bin_path('bundler', 'bundle')
2 changes: 2 additions & 0 deletions spec/dummy/bin/rails
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#!/usr/bin/env ruby
# frozen_string_literal: true

APP_PATH = File.expand_path('../config/application', __dir__)
require_relative '../config/boot'
require 'rails/commands'
2 changes: 2 additions & 0 deletions spec/dummy/bin/rake
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#!/usr/bin/env ruby
# frozen_string_literal: true

require_relative '../config/boot'
require 'rake'
Rake.application.run
2 changes: 2 additions & 0 deletions spec/dummy/config.ru
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# This file is used by Rack-based servers to start the application.

require ::File.expand_path('config/environment', __dir__)
Expand Down
4 changes: 3 additions & 1 deletion spec/dummy/config/application.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require File.expand_path('boot', __dir__)

require "active_record/railtie"
Expand All @@ -12,6 +14,6 @@ module Dummy
class Application < Rails::Application
config.autoloader = Rails::VERSION::MAJOR >= 7 ? :zeitwerk : :classic
config.active_record.sqlite3.represent_boolean_as_integer = true if Rails::VERSION::MAJOR < 6
config.active_record.legacy_connection_handling = false if Rails::VERSION::MAJOR >= 7
config.active_record.legacy_connection_handling = false if Rails::VERSION::MAJOR == 7 && Rails::VERSION::MINOR == 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This setting was removed in 7.1.

end
end
2 changes: 2 additions & 0 deletions spec/dummy/config/boot.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# Set up gems listed in the Gemfile.
ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../../../Gemfile', __dir__)

Expand Down
2 changes: 2 additions & 0 deletions spec/dummy/config/environment.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# Load the Rails application.
require File.expand_path('application', __dir__)

Expand Down
2 changes: 2 additions & 0 deletions spec/dummy/config/environments/development.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

Rails.application.configure do
# Settings specified here will take precedence over those in config/application.rb.

Expand Down
2 changes: 2 additions & 0 deletions spec/dummy/config/environments/test.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

Rails.application.configure do
# Settings specified here will take precedence over those in config/application.rb.

Expand Down
2 changes: 2 additions & 0 deletions spec/dummy/config/initializers/backtrace_silencers.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# Be sure to restart your server when you modify this file.

# You can add backtrace silencers for libraries that you're using but don't wish to see in your backtraces.
Expand Down
2 changes: 2 additions & 0 deletions spec/dummy/config/initializers/cookies_serializer.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# Be sure to restart your server when you modify this file.

Rails.application.config.action_dispatch.cookies_serializer = :json
2 changes: 2 additions & 0 deletions spec/dummy/config/initializers/filter_parameter_logging.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# Be sure to restart your server when you modify this file.

# Configure sensitive parameters which will be filtered from the log file.
Expand Down
2 changes: 2 additions & 0 deletions spec/dummy/config/initializers/inflections.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# Be sure to restart your server when you modify this file.

# Add new inflection rules using the following format. Inflections
Expand Down
2 changes: 2 additions & 0 deletions spec/dummy/config/initializers/mime_types.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# Be sure to restart your server when you modify this file.

# Add new mime types for use in respond_to blocks:
Expand Down
2 changes: 2 additions & 0 deletions spec/dummy/config/initializers/session_store.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# Be sure to restart your server when you modify this file.

Rails.application.config.session_store :cookie_store, key: '_dummy_session'
2 changes: 2 additions & 0 deletions spec/dummy/config/initializers/wrap_parameters.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# Be sure to restart your server when you modify this file.

# This file contains settings for ActionController::ParamsWrapper which
Expand Down
Loading
Loading