Maskers respond to #call #23
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
skalee
commented
Jul 30, 2017
skalee
commented
- Extract the existing masker into a separate file, rework it, add tests.
- Change the standard masker method from #mask to #call.
- Improve the implementation of the default masker.
- Delete support for the :mask_method options.
houndci-bot
reviewed
Jul 30, 2017
spec/maskers/simple_spec.rb
Outdated
|
|
||
| example{ expect(subject.(value: "Solo")).to eq("(redacted)") } | ||
| example{ expect(subject.(value: Math::PI)).to eq("(redacted)") } | ||
| example{ expect(subject.(value: nil)).to eq("(redacted)") } |
spec/maskers/simple_spec.rb
Outdated
| subject{ described_class } | ||
|
|
||
| example{ expect(subject.(value: "Solo")).to eq("(redacted)") } | ||
| example{ expect(subject.(value: Math::PI)).to eq("(redacted)") } |
spec/maskers/simple_spec.rb
Outdated
| RSpec.describe AttrMasker::Maskers::SIMPLE do | ||
| subject{ described_class } | ||
|
|
||
| example{ expect(subject.(value: "Solo")).to eq("(redacted)") } |
spec/maskers/simple_spec.rb
Outdated
| require "spec_helper" | ||
|
|
||
| RSpec.describe AttrMasker::Maskers::SIMPLE do | ||
| subject{ described_class } |
spec/maskers/simple_spec.rb
Outdated
| # | ||
|
|
||
| # No point in using ApplicationRecord here. | ||
| # rubocop:disable Rails/ApplicationRecord |
There was a problem hiding this comment.
Unnecessary disabling of Rails/ApplicationRecord.
lib/attr_masker/maskers/simple.rb
Outdated
| # +opts+ is a Hash with the key :value that gives you the current attribute | ||
| # value. | ||
| # | ||
| SIMPLE = lambda do |opts| |
There was a problem hiding this comment.
Unused block argument - opts. If it's necessary, use _ or _opts as an argument name to indicate that it won't be used. Also consider using a proc without arguments instead of a lambda if you want it to accept any arguments but don't care about them.
lib/attr_masker/maskers/simple.rb
Outdated
| @@ -0,0 +1,13 @@ | |||
| module AttrMasker | |||
| module Maskers | |||
|
|
|||
There was a problem hiding this comment.
Extra empty line detected at module body beginning.
lib/attr_masker.rb
Outdated
| @@ -127,8 +128,7 @@ def mask(attribute, value, options = {}) | |||
| # if options[:if] && !options[:unless] && !value.nil? && !(value.is_a?(String) && value.empty?) | |||
| if options[:if] && !options[:unless] | |||
| value = options[:marshal] ? options[:marshaler].send(options[:dump_method], value) : value.to_s | |||
| # masker_value = options[:masker].send(options[:mask_method], options.merge!(:value => value)) | |||
| masker_value = options[:masker].send(options[:mask_method], options.merge!(:value => value)) | |||
| masker_value = options[:masker].call(options.merge!(:value => value)) | |||
| @@ -79,8 +81,7 @@ def attr_masker(*attributes) | |||
| :marshaler => Marshal, | |||
| :dump_method => "dump", | |||
| :load_method => "load", | |||
| :masker => AttrMasker::Masker, | |||
| :mask_method => "mask", | |||
| :masker => AttrMasker::Maskers::SIMPLE, | |||
| # :masker => The object to use for masking. Defaults to Masker. | ||
| # | ||
| # :mask_method => The mask method name to call on the <tt>:masker</tt> object. Defaults to 'mask'. | ||
| # :masker => The object to use for masking. It must respond to +#mask+. Defaults to AttrMasker::Maskers::Simple. |
When masking, use the masker's #call method instead of #mask. This also adds support for proc-style maskers.
So far, all the objects passed as options to the ::attr_masker method were eagerly evaluated, if they were responding to #call or were symbols denoting some model methods. Actually, that was the source of problems and could result in a premature masker evaluation. In the worst case, that could cause the unexpected behaviour when checking the :if and :unless options, as the option evaluation order was arbitrary. This is fixed in this commit.
Rename the AttrMasker::Maskers::Simple to AttrMasker::Maskers::SIMPLE and turn it into a lambda object.
The line of code which is being deleted in this commit was an exact copy of the line right below, just commented out.
This option was really pretty useless, that feature can be easily duplicated with help of procs or BoundMethod instances.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.