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

feat: prefer after_create instead of after-commit due to transactional guarentees #97

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## 0.13.0 - 2023-11-27
### Added
- Disallow `after_commit` as it has no transactional guarantee, switch to `before_commit`.
AlexRiedler marked this conversation as resolved.
Show resolved Hide resolved

## 0.12.1 - 2023-08-01
### Fixed
- Fixes for `ws-sdk` path injection rubocops
Expand Down
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
rubocop-vendor (0.12.1)
rubocop-vendor (0.13.0)
rubocop

GEM
Expand Down
23 changes: 23 additions & 0 deletions lib/rubocop/cop/vendor/disallow_after_commit.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Vendor
class DisallowAfterCommit < Base
extend AutoCorrector

NO_METH_MSG = "Do not not use `%<old>s` %<desc>s"
NO_METHS = {
after_commit: "use `before_commit` because after_commit is non-transactional an may not run",
AlexRiedler marked this conversation as resolved.
Show resolved Hide resolved
}.freeze

def on_send(node)
name = node.method_name
if NO_METHS.key?(name)
add_offense(node, message: format(NO_METH_MSG, old: name, desc: NO_METHS[name]))
end
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/vendor_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module RuboCop
end

require_relative 'vendor/active_record_connection_execute'
require_relative 'vendor/disallow_after_commit'
require_relative 'vendor/recursive_open_struct_gem'
require_relative 'vendor/sidekiq_throttled_gem'
require_relative 'vendor/recursive_open_struct_use'
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/vendor/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@

module RuboCop
module Vendor
VERSION = '0.12.1'
VERSION = '0.13.0'
end
end
1 change: 1 addition & 0 deletions manual/cops.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#### Department [Vendor](cops_vendor.md)

* [Vendor/ActiveRecordConnectionExecute](cops_vendor.md#vendoractiverecordconnectionexecute)
* [Vendor/DisallowAfterCommit](cops_vendor.md#vendordisallowaftercommit)
* [Vendor/RecursiveOpenStructGem](cops_vendor.md#vendorrecursiveopenstructgem)
* [Vendor/RecursiveOpenStructUse](cops_vendor.md#vendorrecursiveopenstructuse)
* [Vendor/RollbarInsideRescue](cops_vendor.md#vendorrollbarinsiderescue)
Expand Down
22 changes: 15 additions & 7 deletions manual/cops_vendor.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,20 @@ ApplicationRecord.connection.select_all('SELECT * FROM users')
User.connection.select_all('SELECT * FROM users')
```

## Vendor/RecursiveOpenStructGem
## Vendor/DisallowAfterCommit

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | No | - | -

No documentation

## Vendor/RecursiveOpenStructGem

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | No | 0.1.0 | -

This cop flags uses of the recursive-open-struct gem.

RecursiveOpenStruct inherits from OpenStruct, which is now officially discouraged to be used
Expand All @@ -46,7 +54,7 @@ https://ruby-doc.org/stdlib-3.0.1/libdoc/ostruct/rdoc/OpenStruct.html#class-Open

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | No | - | -
Enabled | Yes | No | 0.1.0 | -

This cop flags uses of RecursiveOpenStruct. RecursiveOpenStruct is a library used in the
Wealthsimple ecosystem that is being phased out due to security issues.
Expand Down Expand Up @@ -79,7 +87,7 @@ allow(test_double).to receive(:a).and_return('b')

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | No | - | -
Enabled | Yes | No | 0.1.0 | -

This cop checks for Rollbar calls outside `rescue` blocks.

Expand Down Expand Up @@ -205,7 +213,7 @@ Rollbar.error(exception, "Unable to sync account")

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | No | - | -
Enabled | Yes | No | 0.1.0 | -

This cop flags uses of the sidekiq-throttled gem.

Expand All @@ -220,7 +228,7 @@ https://wealthsimple.slack.com/archives/C19UB3HNZ/p1683721247371709 for more det

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | No | - | -
Enabled | Yes | No | 0.1.0 | -

This cop flags uses of DryStruct without strict mode

Expand All @@ -231,7 +239,7 @@ We want to enfore strict mode which will throw an error in that case.

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | No | - | -
Enabled | Yes | No | 0.12.0 | -

This cop checks for `Ws::Service#get,patch,post,put,delete,...` usage
where the array format is used, but it contains (probably not) intended slashes.
Expand All @@ -251,7 +259,7 @@ Ws::AccountService.post(["test", "foo"])

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | No | - | -
Enabled | Yes | No | 0.12.0 | -

This cop checks for `Ws::Service#get,patch,post,put,delete,...` usage and suggests to use component based paths
instead of using interpolated values that could be user input.
Expand Down
17 changes: 17 additions & 0 deletions spec/rubocop/cop/vendor/disallow_after_commit_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Vendor::DisallowAfterCommit, :config do
it 'registers an offense when using `after_commit` with no args' do
expect_offense(<<~RUBY)
after_commit
^^^^^^^^^^^^ Do not not use `after_commit` use `before_commit` because after_commit is non-transactional an may not run
AlexRiedler marked this conversation as resolved.
Show resolved Hide resolved
RUBY
end

it 'registers an offense when using `after_commit` with args' do
expect_offense(<<~RUBY)
after_commit :foo_bar
^^^^^^^^^^^^^^^^^^^^^ Do not not use `after_commit` use `before_commit` because after_commit is non-transactional an may not run
AlexRiedler marked this conversation as resolved.
Show resolved Hide resolved
RUBY
end
end