Hello. Trying to dry-up our app, I have found this recurring pattern on list-type RESTful endpoints (we are exposing JSONAPI format):
The case
An endpoint receives a filter
input of type Hash[String => String]
, and all the key-value pairs must be parsed and dealt with.
scope = load_initial_scope(current_user)
params[:format].reduce(Right(scope)) do |tmp, (key, value)|
normalized_key = key.to_s.underscore
if respond_to?("filter_by_#{normalized_key}")
value.blank? ? tmp : tmp.bind(method("filter_by_#{normalized_key}"), value)
else
Left("Filter #{key} not supported")
end
end
It relies on having a specific implementation of the filter, like
# GET path?filter[created-at]=2017-01-01|2017-01-02
def filter_by_created_at(scope, value)
dates = value.split('|', 2).map(&:to_date)
Right(scope.where(created_at: Range.new(dates[0], dates[1]))
end
The problems
- Every endpoint’s specific filter needs to deal with parsing the inputs.
- Normalizing the keys beforehand cannot be done because it would be surprising to send
filter[foo-bar]=baz
orfilter[fooBar]=baz
and get back an error saying"Filter foo_bar is not supported"
. - Relying on method names is quite error_prone and not really elegant
- The emerging pattern of three phases (match key, parse value, return a modified scope) is mixed up: parsing the value into “understandable” form is not part of the business logic, but of configuration;
Possible solution using Dry::Matcher
I have come out with the following using Dry::Matcher, but I wanted to check if that’s a reasonable usage and possible drawbacks:
# These are reused across everywhere
# They deal with the value parsing, resolving the value to a specific form.
# They also deal with normalization of the filter key
class CaseMatchingUnderscore < Dry::Matcher::Case
DEFAULT_MATCH = ->((key, _), name) { key.to_s.underscore == name }
def initialize(match: DEFAULT_MATCH, **)
super
end
end
DateRangeCase = CaseMatchingUnderscore.new(
resolve: ->((_, value)) { Range.new(*value.split('|').map(&:to_date)) }
)
SimpleCase = CaseMatchingUnderscore.new(
resolve: ->((_, value)) { value }
)
UnderscoreCase = CaseMatchingUnderscore.new(
resolve: ->((_, value)) { value.to_s.underscore }
)
BlankCase = Dry::Matcher::Case.new(match: ->((_, value)) { value.blank? })
FallbackCase = Dry::Matcher::Case.new(match: ->(*) { true })
So these are completely generic cases, which deal with matching the key (passed as pattern) and resolving the value with some preprocessing.
Follows again a single matcher that just exposes the cases.
GenericMatcher = Dry::Matcher.new(
date_range: DateRangeCase,
blank: BlankCase,
otherwise: FallbackCase,
simple: SimpleCase,
underscore: UnderscoreCase
)
Finally this would be in the endpoint, just mapping filter keys to specific implementations. All the key-value pairs are anyways passed through the matcher.
def match_filter(scope, (key, value))
GenericMatcher.((key, value)) do |m|
m.blank { Right(scope) }
m.date_range 'created_at' { |range| Right(scope.where(created_at: range)) }
m.date_range 'updated_at' { |range| Right(scope.where(updated_at: range)) }
m.simple 'name' { |name| Right(scope.where(["name LIKE ?", %("%#{name}%")]) }
m.underscored 'status' { |status| Right(scope.send("with_#{status}_status") }
m.otherwise { |(key, _)| Left("Filter #{key} not supported") }
end
end
def call
scope = load_initial_scope(current_user)
params[:format].reduce(Right(scope)) do |s, kv| s.bind(kv, &method(:match_filter)) end
end
Now, this might be better because key parsing, normalization and value preparation is out of the specific endpoint and in reusable objects. But on the other side all the implementations are stuck inside the single matcher.call block.
Moreover the Dry::Matcher::Evaluator checks that all “cases” are covered, but in this case they are just possible pattern types, they do not need to be exhausted in all cases (some endpoints might not have any date-range filter)
As an alternative, which doesn’t solve any of the mentioned issue but uses another iteration strategy, would be
# just take a keyvalue pair and return a proc
def match_filter((key, value))
GenericMatcher.((key, value)) do |m|
m.blank { ->(scope) { Right(scope) } }
m.date_range 'created_at' { |range| ->(scope) { Right(scope.where(created_at: range)) } }
# ...
end
end
def call
scope = load_initial_scope(current_user)
params[:format].map(&method(:match_filter)).reduce(Right(scope), &:bind)
end
In the end, I’m not really sure if it is an advantage to use pattern matching in this context.
Can you, oh dear community, shed some light on these doubts? Does it really pay off to have this refactor?
PS: As always, a big kudos to the dry team to make it possible to reason about it.