Enhancement: Brand new DSL about attr_masker

[synth]

Sounds good. So, to provide some more information about our specific cases that we discovered implementing this gem, here you go:

1. Simple attribute masking (this is what is very well covered now)
2. Class level query conditions to filter rows from being masked across all attributes.
3. Ability to nullify a set of attributes
4. Ability to order class processing. And ability to order mask commands inside of a class.
5. Ability for class level sql updates. There are times (like when nullifying) that instead of doing row level processing, you can get away with an update_all on the model which is obviously way faster than doing row level processing.

So, here's a contrived example that illustrates all of the above:

attr_masker Company do # This class runs first
  global_condition ->{ where.not(id: 1) } # sets global query scope
  mask :domain, -> { Faker::Internet.domain_name }
end

attr_masker User do # This class runs second
  global_condition ->{ where.not(company_id: 1) } # sets global query scope

  nullify :crypted_password, :last_login_ip # runs first clearing out all the crud we don't need
  mask :first_name, -> { Faker::Name.first_name }  # runs second
  mask :last_name, -> { Faker::Name.last_name }  # runs third
  mask :email, ->(model: ,**) { "#{model.first_name}.#{model.last_name}@#{model.company.domain}") # User runs after Company so we can guarantee the domain has been masked. Same goes for first/last name.
  update_all "global_unsubscribe = true" # finally, do a final update_all on any other mass assignments like making sure everyone is globally unsubscribed to make sure they dont accidentally get emails (even if we've masked them).
end

I know I'm being cheeky by getting rid of the keyword argument masker in the block syntax...It just feels nicer to me :)

Anyway, let me know what you think when you get to it! This is just how we're using it and I think could definitely add value to others. Cheers!

[skalee]

Feature request for global configuration file (without API changes) has been extracted to #118. I prefer to keep this discussion open, so I've renamed the issue.

[skalee]

So, I definitely see an argument for putting the attr_masker calls inside the models as its currently implemented. But there is also an argument for a centralized global configuration so all the masker calls are together and managed independently of the ORM (ActiveRecord) models. Further, this enables common conditions and custom maskers to be reused across models.

I remember that some time ago I was thinking about some kind of global masker configuration too. Actually, I liked the idea: everything is in one place, models are not polluted with attr_masker calls… But then I realized that if someone wants it this way, he will be better off implementing his own script. It's really simple like that:

User.unscoped.each do |usr|
  # do something
  usr.save!
end

But if you want to do it with attr_masker (maybe because you like that declarative syntax or maybe because you get a progress bar), then how re-opening your models and calling attr_masker in a Rake task like that:

namespace :db do
  task :setup_maskers => :environment do
    Rails.application.eager_load!

    class User
      attr_masker :name
    end

    # or

    User.class_exec do
      attr_masker :name
    end
  end
end

# see: https://stackoverflow.com/a/47784838/304175
Rake::Task["db:mask"].enhance(["db:setup_maskers"])

At the moment I don't have a good Rails application to test that, so consider it a hint rather than a proven solution, sorry! Oh, and I'm not sure if it works well with tools like Spring or Zeus.

I guess we should mention that in documentation.

Also, if you just want re-using, then remember that anything what responds to #call can be a masker. So you can define your custom masker, for example in Rails initializer:

MY_MASKER = lambda do |**args|
  # do sth
end

# or

class MyMasker
  def call(**args)
    # do sth
  end
end

and use it:

class User
  attr_masker :name, masker: MY_MASKER
  # or
  attr_masker :name, masker: MyMasker.new
end

[ribose-jeffreylau]

Thanks @synth for the good points, especially 5) since this is something we'd like to have as well. We hope to be able support most if not all of these use cases in the next API update.

[ronaldtse]

Thank you @synth for the great insights — the proposed DSL is thoughtful and demonstrates good use. Agree with @ribose-jeffreylau .

[skalee]

Cool. So, I forked this repo and ran rspec spec and got this error.

For reference, this error is now fixed.

[ronaldtse]

Hi @synth , thank you for the suggestion! Agree this is a good addition and any PR would be very welcome. 👍

cc @ribose-jeffreylau @skalee

[ribose-jeffreylau]

@synth Looks good! PR is very welcome.

[synth]

Cool. So, I forked this repo and ran rspec spec and got this error:

An error occurred in a `before(:suite)` hook.
Failure/Error: DatabaseCleaner[:mongoid].strategy = :truncation, { only: "users" }

DatabaseCleaner::UnknownStrategySpecified:
  The 'truncation' strategy does not exist for the mongoid ORM!  Available strategies: deletion
# ./spec/support/db_cleaner.rb:15:in `block (2 levels) in <top (required)>'
# ------------------
# --- Caused by: ---
# NameError:
#   uninitialized constant DatabaseCleaner::Mongoid::Truncation
#   ./spec/support/db_cleaner.rb:15:in `block (2 levels) in <top (required)>'

Any ideas?

[synth]

Indeed, doing a User.class_eval initializer vs opening up the class in the rake task is probably a matter of preference. The former seems nicer syntactically but also permanently pollutes the class hierarchy which isn't cool - but that's also the main suggestion of this gem - eg attr_masker calls inside the main model. The rake task isn't as nice syntax, but makes more sense as the configuration really is only needed at Rake execution time.

Our initializer/Class.eval approach is working really well for us and in fact, we'll wrap the whole initializer in a Rails.env.production? check so its not executed in prod environments. Its just not very DRY for our particular use case.

Imagine something like this:

global_condition = ->(model: ,**) { model.company_id.in?(valid_company_ids) }
email_masker = ->(**) { Faker::Name.email }

attr_masker User do
  condition global_condition
  mask :email, masker: email_masker
  nullify :ip_address, :crypted_password
end

Some notes about the above potential syntax:

• By having a global config and an attr_masker block syntax, you can create a really nice expressive DSL (condition, mask, nullify expressive high level specifications)
• For instance, condition is a condition or scope to applied to all the masks
• nullify: I've found that in a lot of cases, I don't really want to mask the data but I simply want to nullify it, so why not have a nice high level dsl for it.

Indeed, I concede that all of the above is possible without the DSL as you have described @skalee. In fact, we've met our somewhat advanced case as is by using the initializer/Class.eval approach and so we're super happy as is. But as rubyists, we strive for expressiveness, no? In any case, I'm blocked from proceeding to develop a PR due to the aforementioned issue just trying to run the tests. Happy to move forward with a potential global config DSL, if I can get unblocked. Otherwise, happy to leave as is and drop this idea as well. Cheers! :)

[skalee]

Indeed, doing a User.class_eval initializer vs opening up the class in the rake task is probably a matter of preference. The former seems nicer syntactically but also permanently pollutes the class hierarchy which isn't cool - but that's also the main suggestion of this gem - eg attr_masker calls inside the main model.

This is more a legacy thing rather than a deliberate decision – this gem is forked from attr_encrypted.

I really like the idea of moving configuration out of models to one place. Also, I am already convinced that the Rakefile trick is not obvious enough. IMO config/attr_masker.rb would be the best place for configuration. Adding support for that file with current API is as easy as adding one conditional require call. Let's do that.

Furthermore, I agree that API should be updated too, as dedicated configuration file opens new possibilities. However, I need to think the details through, sorry. One reason is that we want to support more ORMs eventually, Sequel for instance. It was difficult to do that with current API and I want the new API to be elastic enough. Regarding DSL and block syntax, I hope that a good set of keyword arguments will be a better option, but I'll think about that too. For sure the new API should not involve class re-opening.

So, I'll think on that and post updates here.