Adding feature ignoreDuplicateSlashes with tests

[code-ape]

Overview

This is a PR for the feature discussed in #240 which relates to expected behavior for duplicate slashes in path. It was stated this would be a welcome PR and @mcollina directed me to PR #50 for reference of implementing a similar feature.

Related Issues & PRs

1. Unexpected behavior on double slash in path #240
2. Add support for ignoring trailing slashes #50

Changes

1. Add support for config option ignoreDuplicateSlashes which will condense any continuous occurrence of / in the path into a single /.
2. Add tests for ignoreDuplicateSlashes following pattens of existing tests for ignoreTrailingSlash.
3. Basic documentation of this feature added to README.md.

[ivan-tymoshenko]

Hi, thanks for your contribution. I guess the main idea was to not match empty params, but not to remove duplicate slashes. I mean if someone decided to add a route with two slashes in a row we should match it.

Try to check if a parameter length is not equal to 0, how we check the maximum parameter length, and see if it works.

find-my-way/index.js

Line 446 in 2608f2b

if (decoded.length > maxParamLength) {

[code-ape]

Hi @ivan-tymoshenko, thanks for the quick reply 😄

I think you may have misunderstood #240. I filed it a couple months ago and the desired functionality was to remove duplicate sequential /. This would be of great benefit to me and my team for projects using fastify as an optional config parameter. This is common behavior for web servers, for example this works: https://github.com/delvedor//////find-my-way

You can check the code example for my issue which has paths like /a///d as test cases. I was opening this PR based on the following comment which indicted it would be of interest: #240 (comment)

And thanks for the callout on regex! I will replace that with a proper loop shortly for better performance 🚀

[code-ape]

I did a hand coding of the method for removing duplicate slashes and a benchmark to compare it to Regex. All work for that has been pushed to this branch. Both seem to perform well and the regex appears to do better in more "real world" examples. Below is a run from my M1 MacBook Pro.

Slowest performing scenario seems to be ~150ns / function call or ~500 CPU clock cycles for a 40 character string, which comes to ~10 CPU cycles per character. Also note that "no-op" scenarios where there are no duplicate slashes (which is presumably the majority of paths that would be processed in the real world) appears to only ~3 CPU cycles per character! 😄

$ node ./bench-duplicate.js
[/a/b]
removeDuplicateSlashes(0)      x 96,554,737 ops/sec ±0.21% (296 runs sampled)
removeDuplicateSlashesRegex(0) x 39,600,222 ops/sec ±0.10% (295 runs sampled)

[/api/v1/myNamespace/myObject/action1] 
removeDuplicateSlashes(1)      x 12,750,900 ops/sec ±0.41% (292 runs sampled)
removeDuplicateSlashesRegex(1) x 25,132,656 ops/sec ±0.29% (295 runs sampled)

[/a//b]
removeDuplicateSlashes(2)      x 35,200,687 ops/sec ±0.21% (297 runs sampled)
removeDuplicateSlashesRegex(2) x 18,131,065 ops/sec ±0.49% (289 runs sampled)

[/api/v1/myNamespace/myObject//action1]
removeDuplicateSlashes(3)      x 9,241,756 ops/sec ±0.31% (293 runs sampled)
removeDuplicateSlashesRegex(3) x 15,398,355 ops/sec ±0.36% (290 runs sampled)

[/a///////////////////////b]
removeDuplicateSlashes(4)      x 12,526,865 ops/sec ±0.32% (292 runs sampled)
removeDuplicateSlashesRegex(4) x 15,384,914 ops/sec ±0.20% (295 runs sampled)

[/api/v1/myNamespace/myObject///////////////////////action1]
removeDuplicateSlashes(5)      x 5,976,074 ops/sec ±0.32% (293 runs sampled)
removeDuplicateSlashesRegex(5) x 13,493,742 ops/sec ±0.21% (291 runs sampled)

[/api//v1//myNamespace//myObject//action1]
removeDuplicateSlashes(6)      x 6,328,285 ops/sec ±0.23% (294 runs sampled)
removeDuplicateSlashesRegex(6) x 6,712,087 ops/sec ±0.18% (295 runs sampled)

[ivan-tymoshenko]

Hmm, it looks like regex in that case works really faster. Can you provide npm run bench:cmp:ci command results?

[ivan-tymoshenko]

Another issue I'm worried about is that we shouldn't modify a wildcard part of the path. You can see that we return a wildcard param without trimming the last slash.

[ivan-tymoshenko]

@code-ape Hi, do you need any help? Can you remove non-regexp implementation and provide benchmark results?

[ivan-tymoshenko]

[ivan-tymoshenko]

LGTM

[mcollina]

lgtm