From 87fb324de4472770cb96ec363172629e40b8c412 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Mon, 12 Mar 2018 14:29:52 +0200 Subject: [PATCH] Allow limiting number of audits stored (#405) Adds a setting to allow limiting the maximum number of audits per object. Fixes #274 --- CHANGELOG.md | 2 + README.md | 27 ++++++++++ lib/audited.rb | 2 +- lib/audited/auditor.rb | 32 ++++++++++- spec/audited/auditor_spec.rb | 80 +++++++++++++++++++++++++++- spec/audited_spec_helpers.rb | 4 +- spec/support/active_record/models.rb | 5 ++ 7 files changed, 146 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 706fd81f4..fce503848 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ Added [#413](https://github.com/collectiveidea/audited/pull/413) - Add functionality to conditionally audit models [#414](https://github.com/collectiveidea/audited/pull/414) +- Limit number of audits stored + [#405](https://github.com/collectiveidea/audited/pull/405) Changed diff --git a/README.md b/README.md index 541bd8947..72f32000d 100644 --- a/README.md +++ b/README.md @@ -142,6 +142,33 @@ class User < ActiveRecord::Base end ``` +### Limiting stored audits + +You can limit the number of audits stored for your model. To configure limiting for all audited models, put the following in an initializer: + +```ruby +Audited.max_audits = 10 # keep only 10 latest audits +``` + +or customize per model: + +```ruby +class User < ActiveRecord::Base + audited max_audits: 2 +end +``` + +Whenever an object is updated or destroyed, extra audits are combined with newer ones and the old ones are destroyed. + +```ruby +user = User.create!(name: "Steve") +user.audits.count # => 1 +user.update_attributes!(name: "Ryan") +user.audits.count # => 2 +user.destroy +user.audits.count # => 2 +``` + ### Current User Tracking If you're using Audited in a Rails application, all audited changes made within a request will automatically be attributed to the current user. By default, Audited uses the `current_user` method in your controller. diff --git a/lib/audited.rb b/lib/audited.rb index eaf34cb99..904bfd28d 100644 --- a/lib/audited.rb +++ b/lib/audited.rb @@ -2,7 +2,7 @@ module Audited class << self - attr_accessor :ignored_attributes, :current_user_method + attr_accessor :ignored_attributes, :current_user_method, :max_audits attr_writer :audit_class def audit_class diff --git a/lib/audited/auditor.rb b/lib/audited/auditor.rb index aaf122161..74a59effa 100644 --- a/lib/audited/auditor.rb +++ b/lib/audited/auditor.rb @@ -33,6 +33,7 @@ module ClassMethods # # * +require_comment+ - Ensures that audit_comment is supplied before # any create, update or destroy operation. + # * +max_audits+ - Limits the number of stored audits. # # * +if+ - Only audit the model when the given function returns true # * +unless+ - Only audit the model when the given function returns false @@ -144,6 +145,18 @@ def audited_attributes attributes.except(*non_audited_columns) end + # Combine multiple audits into one. + def combine_audits(audits_to_combine) + combine_target = audits_to_combine.last + combine_target.audited_changes = audits_to_combine.pluck(:audited_changes).reduce(&:merge) + combine_target.comment = "#{combine_target.comment}\nThis audit is the result of multiple audits being combined." + + transaction do + combine_target.save! + audits_to_combine.unscope(:limit).where("version < ?", combine_target.version).delete_all + end + end + protected def non_audited_columns @@ -227,7 +240,14 @@ def audit_destroy def write_audit(attrs) attrs[:associated] = send(audit_associated_with) unless audit_associated_with.nil? self.audit_comment = nil - run_callbacks(:audit) { audits.create(attrs) } if auditing_enabled + + if auditing_enabled + run_callbacks(:audit) { + audit = audits.create(attrs) + combine_audits_if_needed if attrs[:action] != 'create' + audit + } + end end def presence_of_audit_comment @@ -247,6 +267,14 @@ def presence_of_audit_comment end end + def combine_audits_if_needed + max_audits = audited_options[:max_audits] + if max_audits && (extra_count = audits.count - max_audits) > 0 + audits_to_combine = audits.limit(extra_count + 1) + combine_audits(audits_to_combine) + end + end + def require_comment if auditing_enabled && audit_comment.blank? errors.add(:audit_comment, "Comment required before destruction") @@ -351,6 +379,8 @@ def normalize_audited_options audited_options[:on] = [:create, :update, :destroy] if audited_options[:on].empty? audited_options[:only] = Array.wrap(audited_options[:only]).map(&:to_s) audited_options[:except] = Array.wrap(audited_options[:except]).map(&:to_s) + max_audits = audited_options[:max_audits] || Audited.max_audits + audited_options[:max_audits] = Integer(max_audits).abs if max_audits end end end diff --git a/spec/audited/auditor_spec.rb b/spec/audited/auditor_spec.rb index 8a54cd35e..16eaaace9 100644 --- a/spec/audited/auditor_spec.rb +++ b/spec/audited/auditor_spec.rb @@ -144,9 +144,14 @@ class Secret2 < ::ActiveRecord::Base end it "should not save non-audited columns" do - Models::ActiveRecord::User.non_audited_columns = (Models::ActiveRecord::User.non_audited_columns << :favourite_device) + previous = Models::ActiveRecord::User.non_audited_columns + begin + Models::ActiveRecord::User.non_audited_columns += [:favourite_device] - expect(create_user.audits.first.audited_changes.keys.any? { |col| ['favourite_device', 'created_at', 'updated_at', 'password'].include?( col ) }).to eq(false) + expect(create_user.audits.first.audited_changes.keys.any? { |col| ['favourite_device', 'created_at', 'updated_at', 'password'].include?( col ) }).to eq(false) + ensure + Models::ActiveRecord::User.non_audited_columns = previous + end end it "should not save other columns than specified in 'only' option" do @@ -437,6 +442,77 @@ def non_column_attr=(val) end end + describe "max_audits" do + it "should respect global setting" do + stub_global_max_audits(10) do + expect(Models::ActiveRecord::User.audited_options[:max_audits]).to eq(10) + end + end + + it "should respect per model setting" do + stub_global_max_audits(10) do + expect(Models::ActiveRecord::MaxAuditsUser.audited_options[:max_audits]).to eq(5) + end + end + + it "should delete old audits when keeped amount exceeded" do + stub_global_max_audits(2) do + user = create_versions(2) + user.update(name: 'John') + expect(user.audits.pluck(:version)).to eq([2, 3]) + end + end + + it "should not delete old audits when keeped amount not exceeded" do + stub_global_max_audits(3) do + user = create_versions(2) + user.update(name: 'John') + expect(user.audits.pluck(:version)).to eq([1, 2, 3]) + end + end + + it "should delete old extra audits after introducing limit" do + stub_global_max_audits(nil) do + user = Models::ActiveRecord::User.create!(name: 'Brandon', username: 'brandon') + user.update_attributes(name: 'Foobar') + user.update_attributes(name: 'Awesome', username: 'keepers') + user.update_attributes(activated: true) + + Audited.max_audits = 3 + Models::ActiveRecord::User.send(:normalize_audited_options) + user.update_attributes(favourite_device: 'Android Phone') + audits = user.audits + + expect(audits.count).to eq(3) + expect(audits[0].audited_changes).to include({'name' => ['Foobar', 'Awesome'], 'username' => ['brandon', 'keepers']}) + expect(audits[1].audited_changes).to eq({'activated' => [nil, true]}) + expect(audits[2].audited_changes).to eq({'favourite_device' => [nil, 'Android Phone']}) + end + end + + it "should add comment line for combined audit" do + stub_global_max_audits(2) do + user = Models::ActiveRecord::User.create!(name: 'Foobar 1') + user.update(name: 'Foobar 2', audit_comment: 'First audit comment') + user.update(name: 'Foobar 3', audit_comment: 'Second audit comment') + expect(user.audits.first.comment).to match(/First audit comment.+is the result of multiple/m) + end + end + + def stub_global_max_audits(max_audits) + previous_max_audits = Audited.max_audits + previous_user_audited_options = Models::ActiveRecord::User.audited_options.dup + begin + Audited.max_audits = max_audits + Models::ActiveRecord::User.send(:normalize_audited_options) # reloads audited_options + yield + ensure + Audited.max_audits = previous_max_audits + Models::ActiveRecord::User.audited_options = previous_user_audited_options + end + end + end + describe "revisions" do let( :user ) { create_versions } diff --git a/spec/audited_spec_helpers.rb b/spec/audited_spec_helpers.rb index 75cbbe5d6..105c5fbf1 100644 --- a/spec/audited_spec_helpers.rb +++ b/spec/audited_spec_helpers.rb @@ -8,8 +8,8 @@ def build_user(attrs = {}) Models::ActiveRecord::User.new({name: 'darth', username: 'darth', password: 'noooooooo'}.merge(attrs)) end - def create_versions(n = 2) - Models::ActiveRecord::User.create(name: 'Foobar 1').tap do |u| + def create_versions(n = 2, attrs = {}) + Models::ActiveRecord::User.create(name: 'Foobar 1', **attrs).tap do |u| (n - 1).times do |i| u.update_attribute :name, "Foobar #{i + 2}" end diff --git a/spec/support/active_record/models.rb b/spec/support/active_record/models.rb index ccc2050c8..11a8b49f9 100644 --- a/spec/support/active_record/models.rb +++ b/spec/support/active_record/models.rb @@ -72,6 +72,11 @@ def around_audit end end + class MaxAuditsUser < ::ActiveRecord::Base + self.table_name = :users + audited max_audits: 5 + end + class Company < ::ActiveRecord::Base audited end