Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Support Routes with named wildcards #122

Closed
dbwiddis opened this issue Sep 1, 2022 · 3 comments · Fixed by #123
Closed

[FEATURE] Support Routes with named wildcards #122

dbwiddis opened this issue Sep 1, 2022 · 3 comments · Fixed by #123
Assignees
Labels
enhancement New feature or request

Comments

@dbwiddis
Copy link
Member

dbwiddis commented Sep 1, 2022

Is your feature request related to a problem?

In addition to forwarding the params, should we also be able to support routes with parameters?

i.e. new Route(POST, "/crud/update/{doc_id}")

Our current process for locating ExtensionRestHandlers looks for an exact match to the URI to find the respective handler (See https://github.com/opensearch-project/opensearch-sdk-java/blob/main/src/main/java/org/opensearch/sdk/ExtensionsRunner.java#L228).

Are route parameters functionality we want to support?

Originally posted by @cwperks in #111 (comment)

What solution would you like?

We need to change the matching in the ExtensionsRunner hashmap from exact, to parsing a request URI and allowing "wildcard" parameters.

What alternatives have you considered?

Java's path glob matching is probably a great fit here. I'll dig through Rest Handler code to see if that's how it's done there.

@dbwiddis dbwiddis added the enhancement New feature or request label Sep 1, 2022
@dbwiddis dbwiddis self-assigned this Sep 1, 2022
@dbwiddis
Copy link
Member Author

dbwiddis commented Sep 2, 2022

Looking at the RestController, it implements using a PathTrie with a wildcard defined (*) and internally checks (and throws exception) for conflicting parameterized names with the same wildcard. So this shouldn't be too hard to implement.

We can replace {foo} with * in the "exact match" map, and do that same translation of the URI when checking against the map. When inserting in the map we need to do a check whether it's already registered with a different action and return an exception.

@dbwiddis dbwiddis changed the title [FEATURE] Support parameterized routes [FEATURE] Support Routes with named wildcards Sep 2, 2022
@dbwiddis
Copy link
Member Author

dbwiddis commented Sep 2, 2022

While insertion is easy with the *, checking an incoming REST action vs. the map is slightly more complicated as it will have text rather than an asterisk. Might be useful to have two maps: one for exact matches (easy lookup) and one with the wildcards.

When an incoming request comes in, first check the exact map. If no match, then iterate over the map's entryset and attempt a key using glob matching with PathMatcher.

@dbwiddis
Copy link
Member Author

dbwiddis commented Sep 6, 2022

@cwperks see #123 and OS #4415

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant