From 538e55596cc632e9e9fc6d2274285e5287e0c9b9 Mon Sep 17 00:00:00 2001 From: Tabitha Cromarty Date: Mon, 14 Oct 2024 12:37:28 +0100 Subject: [PATCH] Include reference to anonymised record in `Anony::Result` (#112) * Include reference to anonymised record in `Anony::Result` When using selectors to anonymise records, an array of `Result` objects is returned and these results now contain a reference to the record that was anonymised. This means that it is significantly easier to match the changes from the `fields` on each result with the record that those changes came from in case the changes don't include any unique identifiers like the primary key field. --- CHANGELOG.md | 1 + README.md | 4 ++- anony.gemspec | 1 + lib/anony/anonymisable.rb | 2 +- lib/anony/model_config.rb | 2 +- lib/anony/result.rb | 21 ++++++++------- lib/anony/strategies/destroy.rb | 2 +- lib/anony/strategies/overwrite.rb | 2 +- spec/anony/anonymisable_spec.rb | 14 +++++----- spec/anony/result_spec.rb | 44 ++++++++++++++++++++++++++++--- spec/spec_helper.rb | 12 +++++++++ 11 files changed, 79 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed14c3d..e2511f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Add support for Rails 7.2 - Fix Dependabot updates - Fix `NoMethodError` when calling `selector_for?` or `anonymise_for!` on a model class without an `anonymise` config block +- Include reference to anonymised record in `Anony::Result` to allow easier matching of results to records when using selectors. # v1.4.0 diff --git a/README.md b/README.md index 3beb9ef..603dc42 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,7 @@ The result object has 3 attributes: * `status` - If the model was `destroyed`, `overwritten`, `skipped` or the operation `failed` * `fields` - In the event the model was `overwritten`, the fields that were updated (excludes timestamps) * `error` - In the event the anonymisation `failed`, then the associated error. Note only rescues the following errors: `ActiveRecord::RecordNotSaved`, `ActiveRecord::RecordNotDestroyed`. Anything else is thrown. + * `record` - The model instance that was anonymised to produce this result. For convenience, the result object can also be queried with `destroyed?`, `overwritten?`, `skipped?` and `failed?`, so that it can be directly interrogated or used in a `switch case` with the `status` property. @@ -278,6 +279,7 @@ ModelName.anonymise_for!(:user_id, "user_1234") If you attempt to anonymise records with a selector that has not been defined it will throw an error. +When anonymising models using selectors, an array of `Anony::Result` objects will be returned, one result per anonymised record in the model. These results contain a reference to the record that was anonymised to produce that result, so that changes made or failures can easily be linked back to the specific record. ### Identifying anonymised records @@ -492,7 +494,7 @@ Lint/DefineDeletionStrategy: If your models use multiple superclasses, you can specify a list of superclasses in your `.rubocop.yml`. Note that you will have to specify `ApplicationRecord` explicitly in this list should you want to lint all models which inherit from `ApplicationRecord`. ```yml Lint/DefineDeletionStrategy: - ModelSuperclass: + ModelSuperclass: - Acme::Record - UmbrellaCorp::Record diff --git a/anony.gemspec b/anony.gemspec index b707919..0e7ef98 100644 --- a/anony.gemspec +++ b/anony.gemspec @@ -25,6 +25,7 @@ Gem::Specification.new do |spec| spec.required_ruby_version = ">= 3.1" spec.add_development_dependency "bundler", "~> 2" + spec.add_development_dependency "database_cleaner-active_record", "~> 2.2" spec.add_development_dependency "gc_ruboconfig", "~> 5.0.0" spec.add_development_dependency "rspec", "~> 3.9" spec.add_development_dependency "rspec-github", "~> 2.4.0" diff --git a/lib/anony/anonymisable.rb b/lib/anony/anonymisable.rb index 2ad704b..a38a11b 100644 --- a/lib/anony/anonymisable.rb +++ b/lib/anony/anonymisable.rb @@ -102,7 +102,7 @@ def anonymise! self.class.anonymise_config.validate! self.class.anonymise_config.apply(self) rescue ActiveRecord::RecordNotSaved, ActiveRecord::RecordNotDestroyed => e - Result.failed(e) + Result.failed(e, self) end def anonymised? diff --git a/lib/anony/model_config.rb b/lib/anony/model_config.rb index 9cf4eca..ea9b38f 100644 --- a/lib/anony/model_config.rb +++ b/lib/anony/model_config.rb @@ -41,7 +41,7 @@ def initialize(model_class, &block) # @example # Anony::ModelConfig.new(Manager).apply(Manager.new) def apply(instance) - return Result.skipped if @skip_filter && instance.instance_exec(&@skip_filter) + return Result.skipped(instance) if @skip_filter && instance.instance_exec(&@skip_filter) @strategy.apply(instance) end diff --git a/lib/anony/result.rb b/lib/anony/result.rb index 0349ddf..35f8b54 100644 --- a/lib/anony/result.rb +++ b/lib/anony/result.rb @@ -9,32 +9,33 @@ class Result OVERWRITTEN = "overwritten" SKIPPED = "skipped" - attr_reader :status, :fields, :error + attr_reader :status, :fields, :error, :record delegate :failed?, :overwritten?, :skipped?, :destroyed?, to: :status - def self.failed(error) - new(FAILED, error: error) + def self.failed(error, record) + new(FAILED, record: record, error: error) end - def self.overwritten(fields) - new(OVERWRITTEN, fields: fields) + def self.overwritten(fields, record) + new(OVERWRITTEN, record: record, fields: fields) end - def self.skipped - new(SKIPPED) + def self.skipped(record) + new(SKIPPED, record: record) end - def self.destroyed - new(DESTROYED) + def self.destroyed(record) + new(DESTROYED, record: record) end - private def initialize(status, fields: [], error: nil) + private def initialize(status, record:, fields: [], error: nil) raise ArgumentError, "No error provided" if status == FAILED && error.nil? @status = ActiveSupport::StringInquirer.new(status) @fields = fields @error = error + @record = record end end end diff --git a/lib/anony/strategies/destroy.rb b/lib/anony/strategies/destroy.rb index c891e94..ed73b37 100644 --- a/lib/anony/strategies/destroy.rb +++ b/lib/anony/strategies/destroy.rb @@ -32,7 +32,7 @@ def validate! # @param [ActiveRecord::Base] instance An instance of the model def apply(instance) instance.destroy! - Result.destroyed + Result.destroyed(instance) end end end diff --git a/lib/anony/strategies/overwrite.rb b/lib/anony/strategies/overwrite.rb index d7ed40b..f0c7428 100644 --- a/lib/anony/strategies/overwrite.rb +++ b/lib/anony/strategies/overwrite.rb @@ -58,7 +58,7 @@ def apply(instance) instance.save! - Result.overwritten(result_fields) + Result.overwritten(result_fields, instance) end # Configure a custom strategy for one or more fields. If a block is given that is used diff --git a/spec/anony/anonymisable_spec.rb b/spec/anony/anonymisable_spec.rb index 71a6034..19aaaf5 100644 --- a/spec/anony/anonymisable_spec.rb +++ b/spec/anony/anonymisable_spec.rb @@ -69,18 +69,18 @@ def some_instance_method? end end - context "single record" do - it "changes the matching record" do - klass.anonymise_for!(:first_name, model.first_name) - expect(model.reload.anonymised?).to eq(true) - expect(model_b.reload.anonymised?).to eq(false) - end + it "anonymises only the matching models: first_name" do + results = klass.anonymise_for!(:first_name, model.first_name) + expect(model.reload.anonymised?).to eq(true) + expect(model_b.reload.anonymised?).to eq(false) + expect(results.map(&:record)).to contain_exactly(model) end it "anonymises only the matching models: company_name" do - klass.anonymise_for!(:company_name, model.company_name) + results = klass.anonymise_for!(:company_name, model.company_name) expect(model.reload.anonymised?).to be(true) expect(model_b.reload.anonymised?).to be(true) + expect(results.map(&:record)).to contain_exactly(model, model_b) end end diff --git a/spec/anony/result_spec.rb b/spec/anony/result_spec.rb index a01746d..aa8dece 100644 --- a/spec/anony/result_spec.rb +++ b/spec/anony/result_spec.rb @@ -10,8 +10,25 @@ } end + shared_context "with model instance" do + let(:klass) do + Class.new(ActiveRecord::Base) do + include Anony::Anonymisable + + def self.name + "Employee" + end + + self.table_name = :employees + end + end + + let(:model) { klass.new } + end + context "anonymised" do - let(:result) { described_class.overwritten(field_values) } + include_context "with model instance" + let(:result) { described_class.overwritten(field_values, model) } it "has enumbeable state" do expect(result.status).to eq("overwritten") @@ -20,10 +37,15 @@ it "responds to .overwritten?" do expect(result).to be_overwritten end + + it "contains the model" do + expect(result.record).to be model + end end context "deleted" do - let(:result) { described_class.destroyed } + include_context "with model instance" + let(:result) { described_class.destroyed(model) } it "has enumbeable state" do expect(result.status).to eq("destroyed") @@ -36,10 +58,15 @@ it "has no fields" do expect(result.fields).to be_empty end + + it "contains the model" do + expect(result.record).to be model + end end context "skipped" do - let(:result) { described_class.skipped } + include_context "with model instance" + let(:result) { described_class.skipped(model) } it "has enumbeable state" do expect(result.status).to eq("skipped") @@ -52,11 +79,16 @@ it "has no fields" do expect(result.fields).to be_empty end + + it "contains the model" do + expect(result.record).to be model + end end context "failed" do + include_context "with model instance" let(:error) { anything } - let(:result) { described_class.failed(error) } + let(:result) { described_class.failed(error, model) } it "has an error" do expect(result.error).to eq(error) @@ -75,5 +107,9 @@ expect { described_class.failed(nil) }.to raise_error(ArgumentError) end end + + it "contains the model" do + expect(result.record).to be model + end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9a7e6bc..af35739 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,6 +2,7 @@ require "bundler/setup" require "anony" +require "database_cleaner/active_record" RSpec.configure do |config| # Disable RSpec exposing methods globally on `Module` and `main` @@ -10,4 +11,15 @@ config.expect_with :rspec do |c| c.syntax = :expect end + + config.before(:suite) do + DatabaseCleaner.strategy = :transaction + DatabaseCleaner.clean_with(:truncation) + end + + config.around do |example| + DatabaseCleaner.cleaning do + example.run + end + end end