[ISSUE-287] GetQueriesTemplates and GetQueriesRegexp extraction #304
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Developers can now extract the query templates and regexps from a router as lists of combined query pairs.
paulbes
changed the title
paulbes
changed the title
elithrar
approved these changes
Oct 11, 2017
|
Thanks for the detailed write-up: it helps a lot. The patch looks good to
me; @kisielk as a second pair of eyes before merging.
…On Wed, Oct 11, 2017 at 12:22 PM Paul B. Beskow ***@***.***> wrote:
Developers can now extract the query templates and regexps from a router
as lists of combined query pairs.
Description
The implementation returns lists of combined pairs of registered Queries;
as templates or regexps. It was deemed more appropriate to return lists, at
least for regexps, rather than a joined string of all combined pairs, since
each query pair is considered a separate matcher, e.g., if we have the
following: Queries("foo", "{v1}", "baz", "{v2}"), when joined using
regexps we would end up with: ^foo=(?P<v0>.*)$,^baz=(?P<v0>.*)$. The
multiple v0 entries might be confusing. For the query template, we could
consider joining, but even then it will be a bit strange, e.g.,
foo={v1},baz={v2}.
Motivation and Context
Being able to extract all parts of a registered route, including queries,
can be very useful. In particular when generating REST API documentation.
In addition, it can be useful to determine that tests have been written for
all registered routes; by matching recorded and registered routes with
their various parts.
How Has This Been Tested (if appropriate)?
Added testQueriesRegexp and testQueriesTemplates methods and added
queriesTemplate and queriesRegexp fields to routeTest. All TestQueries
cases are using this functionality.
Types of changes
- Bug fix (non-breaking change which fixes an issue)
- New feature (non-breaking change which adds functionality)
- Breaking change (fix or feature that would cause existing
functionality to change)
- Refactoring (non-breaking changes, no bug fixing, no new features)
- Testing (New unit/integration/performance tests)
Checklist:
- I have updated the documentation accordingly.
- I have added tests to cover my changes.
Related tickets or issues:
This resolves issue #287 <#287>
------------------------------
You can view, comment on, or merge this pull request online at:
#304
Commit Summary
- GetQueryTemplates and GetQueryRegexp extraction
File Changes
- *M* README.md <https://github.com/gorilla/mux/pull/304/files#diff-0>
(28)
- *M* mux_test.go
<https://github.com/gorilla/mux/pull/304/files#diff-1> (588)
- *M* route.go <https://github.com/gorilla/mux/pull/304/files#diff-2>
(38)
Patch Links:
- https://github.com/gorilla/mux/pull/304.patch
- https://github.com/gorilla/mux/pull/304.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#304>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABIcH0xTE2I6GvnwjUr1pNouebnNj5eks5srIi9gaJpZM4P1KaM>
.
|
|
Is there a reason you'd want just the Regexp or just the templates? Is there value in it being two separate functions? |
|
I can't speak for all use-cases, but we will end up using Are you considering merging them into one or dropping either? If merging, what would this look like? |
|
Do we need to make any changes to this PR or should it be merged? |
|
If @kisielk is OK with the API here then it’s OK to merge. |
|
The symmetry with the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Developers can now extract the query templates and regexps from a router as lists of combined query pairs.
Description
The implementation returns lists of combined pairs of registered
Queries; as templates or regexps. It was deemed more appropriate to return lists, at least for regexps, rather than a joined string of all combined pairs, since each query pair is considered a separate matcher, e.g., if we have the following:Queries("foo", "{v1}", "baz", "{v2}"), when joined using regexps we would end up with:^foo=(?P<v0>.*)$,^baz=(?P<v0>.*)$. The multiplev0entries might be confusing. For the query template, we could consider joining, but even then it will be a bit strange, e.g.,foo={v1},baz={v2}.Motivation and Context
Being able to extract all parts of a registered route, including queries, can be very useful. In particular when generating REST API documentation. In addition, it can be useful to determine that tests have been written for all registered routes; by matching recorded and registered routes with their various parts.
How Has This Been Tested (if appropriate)?
Added
testQueriesRegexpandtestQueriesTemplatesmethods and addedqueriesTemplateandqueriesRegexpfields torouteTest. AllTestQueriescases are using this functionality.Types of changes
Checklist:
Related tickets or issues:
This resolves issue #287