Skip to content

Commit

Permalink
Remove rails-observers dependency
Browse files Browse the repository at this point in the history
The `Audited::Sweeper` class made limited use of the cache sweeping
functionality provided by rails-observer, mostly using it as a way to
implement `Audit#before_create` on the same class that was attached
around controller actions.

Since there is no released version of rails-observer that's compatible
with Rails 5.0 (and not in the foreseeable future without considerable
refactoring), the population of IPs, users and request UUIDs is replaced
by thread-local storage and the existing `Audit#before_create` methods.

This is a net code reduction and somewhat simplifies the "sweeper" by
removing the assumptions about how rails-observer works.
  • Loading branch information
domcleal committed Mar 16, 2017
1 parent db49434 commit 87d402a
Show file tree
Hide file tree
Showing 9 changed files with 36 additions and 52 deletions.
3 changes: 0 additions & 3 deletions Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,4 @@ end

appraise 'rails50' do
gem 'rails', '~> 5.0.0'

# The following needs to point to Github until the release of 0.1.3
gem 'rails-observers', github: 'rails/rails-observers', branch: 'master'
end
5 changes: 0 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,6 @@ Add the gem to your Gemfile:
gem "audited", "~> 4.3"
```

If you are using rails 5.0, you would also need the following line in your Gemfile.
```ruby
gem "rails-observers", github: 'rails/rails-observers'
```

Then, from your Rails app directory, create the `audits` table:

```bash
Expand Down
1 change: 0 additions & 1 deletion audited.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ Gem::Specification.new do |gem|
gem.require_paths = ['lib']

gem.add_dependency 'activerecord', '>= 4.0', '< 5.1'
gem.add_dependency 'rails-observers', '~> 0.1.2'

gem.add_development_dependency 'appraisal'
gem.add_development_dependency 'rails', '>= 4.0', '< 5.1'
Expand Down
1 change: 0 additions & 1 deletion gemfiles/rails50.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@
source "https://rubygems.org"

gem "rails", "~> 5.0.0"
gem "rails-observers", :github => "rails/rails-observers", :branch => "master"

gemspec :name => "audited", :path => "../"
1 change: 0 additions & 1 deletion lib/audited.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
require 'rails/observers/active_model/active_model'
require 'active_record'

module Audited
Expand Down
16 changes: 10 additions & 6 deletions lib/audited/audit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,11 @@ module Audited
# * <tt>created_at</tt>: Time that the change was performed
#
class Audit < ::ActiveRecord::Base
include ActiveModel::Observing

belongs_to :auditable, polymorphic: true
belongs_to :user, polymorphic: true
belongs_to :associated, polymorphic: true

before_create :set_version_number, :set_audit_user, :set_request_uuid
before_create :set_version_number, :set_audit_user, :set_request_uuid, :set_remote_address

cattr_accessor :audited_class_names
self.audited_class_names = Set.new
Expand Down Expand Up @@ -95,10 +93,10 @@ def self.audited_classes
# by +user+. This method is hopefully threadsafe, making it ideal
# for background operations that require audit information.
def self.as_user(user, &block)
Thread.current[:audited_user] = user
::Audited.store[:audited_user] = user
yield
ensure
Thread.current[:audited_user] = nil
::Audited.store[:audited_user] = nil
end

# @private
Expand Down Expand Up @@ -138,12 +136,18 @@ def set_version_number
end

def set_audit_user
self.user = Thread.current[:audited_user] if Thread.current[:audited_user]
self.user ||= ::Audited.store[:audited_user] # from .as_user
self.user ||= ::Audited.store[:current_user] # from Sweeper
nil # prevent stopping callback chains
end

def set_request_uuid
self.request_uuid ||= ::Audited.store[:current_request_uuid]
self.request_uuid ||= SecureRandom.uuid
end

def set_remote_address
self.remote_address ||= ::Audited.store[:current_remote_address]
end
end
end
47 changes: 18 additions & 29 deletions lib/audited/sweeper.rb
Original file line number Diff line number Diff line change
@@ -1,60 +1,49 @@
require "rails/observers/activerecord/active_record"
require "rails/observers/action_controller/caching"

module Audited
class Sweeper < ActionController::Caching::Sweeper
observe Audited::Audit
class Sweeper
STORED_DATA = {
current_remote_address: :remote_ip,
current_request_uuid: :request_uuid,
current_user: :current_user
}

delegate :store, to: ::Audited

def around(controller)
self.controller = controller
STORED_DATA.each { |k,m| store[k] = send(m) }
yield
ensure
self.controller = nil
end

def before_create(audit)
audit.user ||= current_user
audit.remote_address = controller.try(:request).try(:remote_ip)
audit.request_uuid = request_uuid if request_uuid
STORED_DATA.keys.each { |k| store.delete(k) }
end

def current_user
controller.send(Audited.current_user_method) if controller.respond_to?(Audited.current_user_method, true)
end

def request_uuid
controller.try(:request).try(:uuid)
end

def add_observer!(klass)
super
define_callback(klass)
def remote_ip
controller.try(:request).try(:remote_ip)
end

def define_callback(klass)
observer = self
callback_meth = :_notify_audited_sweeper
klass.send(:define_method, callback_meth) do
observer.update(:before_create, self)
end
klass.send(:before_create, callback_meth)
def request_uuid
controller.try(:request).try(:uuid)
end

def controller
::Audited.store[:current_controller]
store[:current_controller]
end

def controller=(value)
::Audited.store[:current_controller] = value
store[:current_controller] = value
end
end
end

ActiveSupport.on_load(:action_controller) do
if defined?(ActionController::Base)
ActionController::Base.around_action Audited::Sweeper.instance
ActionController::Base.around_action Audited::Sweeper.new
end
if defined?(ActionController::API)
ActionController::API.around_action Audited::Sweeper.instance
ActionController::API.around_action Audited::Sweeper.new
end
end
2 changes: 1 addition & 1 deletion spec/audited/audit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ class Models::ActiveRecord::CustomUserSubclass < Models::ActiveRecord::CustomUse
raise StandardError.new('expected')
end
}.to raise_exception('expected')
expect(Thread.current[:audited_user]).to be_nil
expect(Audited.store[:audited_user]).to be_nil
end

end
Expand Down
12 changes: 7 additions & 5 deletions spec/audited/sweeper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,21 +86,23 @@ def update
describe Audited::Sweeper do

it "should be thread-safe" do
instance = Audited::Sweeper.new

t1 = Thread.new do
sleep 0.5
Audited::Sweeper.instance.controller = 'thread1 controller instance'
expect(Audited::Sweeper.instance.controller).to eq('thread1 controller instance')
instance.controller = 'thread1 controller instance'
expect(instance.controller).to eq('thread1 controller instance')
end

t2 = Thread.new do
Audited::Sweeper.instance.controller = 'thread2 controller instance'
instance.controller = 'thread2 controller instance'
sleep 1
expect(Audited::Sweeper.instance.controller).to eq('thread2 controller instance')
expect(instance.controller).to eq('thread2 controller instance')
end

t1.join; t2.join

expect(Audited::Sweeper.instance.controller).to be_nil
expect(instance.controller).to be_nil
end

end

0 comments on commit 87d402a

Please sign in to comment.