implemented same path wildcard & static conflict fix

[maybephilipp]

Short:

• implemented support of having exact route definitions for the same path as wildcard definitions;
• adjusted test to cover the case;
• added .idea to .gitignore;
• improved tests to check equality of found callback to wildcard route

Notes:

/cmd/foo/bar
/cmd/test
/cmd/test/3

• In my implementation, exact defs must be added before wildcards, otherwise it will conflict with wildcard.
• Allowed having more than 1 child in a wildcard (besides wildcard itself)
• Added backup stack to the tree search function. That for the following case:

  Path: /cmd/test/3
  Tree:
  - /cmd
  - -----/foo/bar
  - -----/:p1
  - --------:p2
  -> Goes inside /foo/bar - didn't satisfy
  -> Pops backup from stack, so search starts with /cmd and child offset == 1
  -> Goes inside /:p1
  -> Goes inside /:p2 - FOUND
  

Fixes #28
Probably fixes: #19

[maybephilipp]

Hm, will look at it in the morning. Tests were passing on my local using node v20. Will check & fix

[steambap]

Your fix allows some crazy router definitions like:

/cmd/a_:tool
/cmd/:tool/:sub
/cmd/:tool/

[maybephilipp]

Your fix allows some crazy router definitions like

@steambap Could you please explain why the routes are crazy? a_:tool is more exact than :tool and has a different prefix, so it's completely ok. The order of defining two last routes doesn't matter since ends up to be the same tree.

I've rolled out few fixes:

1. Fixed code so that failing test is passing.
2. Rollbacked wildcard duplicates check (I messed it up last time)
3. Improved wildcard tests to check equality of matched handles (to be sure that found route is the one that was supposed to be found)

[steambap]

What about routers like this:

/cmd/a_:tool
/cmd/ab_:tool

It is true that a_:tool is more exact than :tool. However, I prefer a solution that only allows either exact path or wildcard. There shouldn't be 2 wildcards.

[maybephilipp]

However, I prefer a solution that only allows either exact path or wildcard. There shouldn't be 2 wildcards.

Okay, got it. What is the reasoning behind this? Why won't we allow multiple wildcards if they have a different prefix? It's the same performance for search since the prefix narrows down the route path. Could you please explain what are bad consequences of having it?

[steambap]

I think it's confusing. There are probably multiple ways to make route paths harder to read like:

/info/user/:public
/info/:user/public

Also, you mentioned performance. Backtrack search in this PR reduce performance by 1/10 ~ 1/3 in the router benchmark linked in readme, although the benchmark does not test anything that requires backtrack.

[maybephilipp]

There are probably multiple ways to make route paths harder to read like

I agree that these routes are confusing, but I think it's 100% in the field of a tool user's responsibility, same as following correct REST route practices etc. It's all about code quality which must not be fixed by tools. Tools must provide functionality and I don't see a reason restricting it since exact/wildcard routes matching in similar paths is supported in most of well-known routers.

Backtrack search in this PR reduce performance by 1/10 ~ 1/3

Oh, got it. Thanks for noticing. I will review performance and make changes then.

[maybephilipp]

@steambap I have done some optimizations, significantly improving performance. Now it runs with the same or +- same performance.

macOS, M1 Pro (arm), Node v20

Before fix:

First run:

short static: 39,148,776 ops/sec
static with same radix: 16,651,172 ops/sec
dynamic route: 8,349,365 ops/sec
mixed static dynamic: 10,523,080 ops/sec
long static: 17,918,632 ops/sec
wildcard: 10,751,185 ops/sec
all together: 2,139,514 ops/sec

Second run:

short static: 39,988,004 ops/sec
static with same radix: 16,642,120 ops/sec
dynamic route: 8,342,339 ops/sec
mixed static dynamic: 10,567,501 ops/sec
long static: 17,936,764 ops/sec
wildcard: 10,537,075 ops/sec
all together: 2,267,038 ops/sec

After fix

short static: 39,804,230 ops/sec
static with same radix: 16,203,987 ops/sec
dynamic route: 7,787,487 ops/sec
mixed static dynamic: 10,000,538 ops/sec
long static: 17,007,767 ops/sec
wildcard: 10,268,140 ops/sec
all together: 2,168,849 ops/sec

short static: 40,279,540 ops/sec
static with same radix: 16,566,337 ops/sec
dynamic route: 8,094,932 ops/sec
mixed static dynamic: 10,459,490 ops/sec
long static: 18,006,230 ops/sec
wildcard: 10,525,970 ops/sec
all together: 2,245,313 ops/sec

[steambap]

If this is a regex based router, I agree with you. We should provide user the tool to write /:totally{-:insane}?{-:path}?, and it's the user's responsibility to make sure they are correct.
However, for this module, I want to stay the same as the original Golang implementation. No multiple wildcards on the same route please.