ruby-grape / grape / 33768866081

97%

push

github

Constrain the version capture with a Regexp instead of an Array (#2889)

* Constrain the version capture with a Regexp instead of an Array

`Pattern#extract_capture` hands the declared versions to Mustermann as the
Array of Strings they arrive in:

    requirements.merge(version: map_str(version))

Mustermann uses `capture:` for two things: shaping the regexp, and deriving
*param converters*. The second is the problem. `AST::ParamScanner::Capture`
only knows how to build a converter from a Class or a Symbol, so for an Array
of plain Strings every entry contributes nothing — and it still hands one back:

    entries = capture.map { |item| converter(item) }.compact   # => []
    converter = ->(string) do
      _, c = entries.find { |r, _| r.match?(string) }          # => nil
      c&.call(string) || string                                # => string
    end

An identity function, registered for `version` on every path-versioned route.

Its cost is not the call. A non-empty converter table makes
`AST::Pattern#identity_params?` false forever, and that is the predicate
`RegexpBased#build_params` consults to decide whether it may hand back the
capture Hash as it came:

    if @simple_captures
      params = match.named_captures
      return params if params.empty? || identity_params?(params)
      params.each_with_object({}) { |(k, v), h| h[k] = map_param(k, v) }

So every request on the route rebuilt the Hash through `map_param` and called
the do-nothing lambda on each value — `Route#params_for` runs this on every
matched request. Passing a Regexp registers no converter at all, and produces
a simpler regexp into the bargain:

    array:   (?<version>(?-mix:(?-mix:(?:v|%76)(?:1|%31))|(?-mix:(?:v|%76)(?:2|%32))))
    regexp:  (?<version>(?-mix:v1|v2))

`Regexp.union` escapes its arguments, so versions carrying regexp
metacharacters (`v1.0`, `v2+beta`) still match literally.

The stringify that `map_str` did stays, inlined at the only call site it ever
had, an... (continued)

1244 of 1338 branches covered (92.97%)

Branch coverage included in aggregate %.

9 of 9 new or added lines in 4 files covered. (100.0%)

3943 of 4012 relevant lines covered (98.28%)

27020.83 hits per line