From 4a6dfb100008da2f7b6bf57a4a40d89846bb8f39 Mon Sep 17 00:00:00 2001 From: Michael Wawra Date: Fri, 7 Feb 2020 09:01:47 +0000 Subject: [PATCH 1/2] Docs and Tests for raising FieldException - When there is an incomplete anonymisation config, we want to raise the error and not capture it in a return type. - This makes much more sense for the downstream developer. --- README.md | 10 +++++----- spec/anony/anonymisable_spec.rb | 3 +-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index f48199c..e03e54d 100644 --- a/README.md +++ b/README.md @@ -119,7 +119,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: `Anony::FieldException`, `ActiveRecord::RecordNotSaved`, `ActiveRecord::RecordNotDestroyed`. Anything else is thrown. + * `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. 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. @@ -317,16 +317,16 @@ columns are added/removed or the contents of those columns changes. As such, Anony will validate your model configuration when you try to anonymise the model (unfortunately this cannot be safely done at boot as the database might not be -available). If your configuration is incomplete, calling `#anonymise!` will fail, and a -`FieldsException` will be returned in the `error` attribute of the `Anony:Result` object. -This exception will warn which fields are missing. +available). If your configuration is incomplete, calling `#anonymise!` will raise a +`FieldsException` and will not return an `Anony:Result` object. This is perceived +to a critical error as anony cannot safely anonymise the model. ``` irb(main):001:0> manager = Manager.find(1) => # irb(main):002:0> manager.anonymise! -=> #> +Anony::FieldException (Invalid anonymisation strategy for field(s) [:username]) ``` We recommend adding a test for each model that you anonymise (see [Testing](#testing) diff --git a/spec/anony/anonymisable_spec.rb b/spec/anony/anonymisable_spec.rb index f7fc2c1..ab6b2dc 100644 --- a/spec/anony/anonymisable_spec.rb +++ b/spec/anony/anonymisable_spec.rb @@ -171,8 +171,7 @@ def some_instance_method? let(:model) { klass.new } it "throws an exception" do - result = model.anonymise! - expect(result.error).to be_an_instance_of(Anony::FieldException) + expect { model.anonymise! }.to raise_error(Anony::FieldException) end end end From 6f70255f74fae02d2c5da8ea01aa4bc5e75ad047 Mon Sep 17 00:00:00 2001 From: Michael Wawra Date: Fri, 7 Feb 2020 09:03:37 +0000 Subject: [PATCH 2/2] Do not rescue from FieldException - This is passed to the calling code as it means the anonymisation config is invalid. --- lib/anony/anonymisable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/anony/anonymisable.rb b/lib/anony/anonymisable.rb index 81f512e..ebcd71e 100644 --- a/lib/anony/anonymisable.rb +++ b/lib/anony/anonymisable.rb @@ -68,7 +68,7 @@ def anonymise! self.class.anonymise_config.validate! self.class.anonymise_config.apply(self) - rescue FieldException, ActiveRecord::RecordNotSaved, ActiveRecord::RecordNotDestroyed => e + rescue ActiveRecord::RecordNotSaved, ActiveRecord::RecordNotDestroyed => e Result.failed(e) end