Convert all BRANCH_LIST and TAG_LIST routes to one unified regex pattern
Our goal is that all routes become regex patterns. At the moment some of
them are strings, which means that they will get special handling by
page.js, most importantly it will construct its own regex pattern for
tokens such as `:filter`. Apart from not wanting to deal with as few as
possible page.js specialities and trying to be consistent with all the
route patterns, these special page.js tokens also come along with a
fundamental problem: They use `[^/]+` and thus don't match `/`, which at
least for `repo` matching is a problem and creates extra complexity.
Release-Notes: skip
Change-Id: If75ea4ad46e406b8d54a04092c73b91314db5929
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 36bbfa4..94467b5 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -169,17 +169,6 @@
// Matches /admin/repos/<repos>,access.
REPO_DASHBOARDS: /^\/admin\/repos\/(.+),dashboards$/,
- // Matches /admin/repos/<repo>,branches[,<offset>].
- BRANCH_LIST_OFFSET: /^\/admin\/repos\/(.+),branches(,(.+))?$/,
- BRANCH_LIST_FILTER: '/admin/repos/:repo,branches/q/filter::filter',
- BRANCH_LIST_FILTER_OFFSET:
- '/admin/repos/:repo,branches/q/filter::filter,:offset',
-
- // Matches /admin/repos/<repo>,tags[,<offset>].
- TAG_LIST_OFFSET: /^\/admin\/repos\/(.+),tags(,(.+))?$/,
- TAG_LIST_FILTER: '/admin/repos/:repo,tags/q/filter::filter',
- TAG_LIST_FILTER_OFFSET: '/admin/repos/:repo,tags/q/filter::filter,:offset',
-
PLUGINS: /^\/plugins\/(.+)$/,
// Matches /admin/plugins with optional filter and offset.
@@ -188,6 +177,11 @@
GROUP_LIST: /^\/admin\/groups\/?(?:\/q\/filter:(.*?))?(?:,(\d+))?$/,
// Matches /admin/repos with optional filter and offset.
REPO_LIST: /^\/admin\/repos\/?(?:\/q\/filter:(.*?))?(?:,(\d+))?$/,
+ // Matches /admin/repos/$REPO,branches with optional filter and offset.
+ BRANCH_LIST:
+ /^\/admin\/repos\/(.+),branches\/?(?:\/q\/filter:(.*?))?(?:,(\d+))?$/,
+ // Matches /admin/repos/$REPO,tags with optional filter and offset.
+ TAG_LIST: /^\/admin\/repos\/(.+),tags\/?(?:\/q\/filter:(.*?))?(?:,(\d+))?$/,
QUERY: /^\/q\/([^,]+)(,(\d+))?$/,
@@ -738,40 +732,12 @@
ctx => this.handleRepoDashboardsRoute(ctx)
);
- this.mapRoute(
- RoutePattern.BRANCH_LIST_OFFSET,
- 'handleBranchListOffsetRoute',
- ctx => this.handleBranchListOffsetRoute(ctx)
+ this.mapRoute(RoutePattern.BRANCH_LIST, 'handleBranchListRoute', ctx =>
+ this.handleBranchListRoute(ctx)
);
- this.mapRoute(
- RoutePattern.BRANCH_LIST_FILTER_OFFSET,
- 'handleBranchListFilterOffsetRoute',
- ctx => this.handleBranchListFilterOffsetRoute(ctx)
- );
-
- this.mapRoute(
- RoutePattern.BRANCH_LIST_FILTER,
- 'handleBranchListFilterRoute',
- ctx => this.handleBranchListFilterRoute(ctx)
- );
-
- this.mapRoute(
- RoutePattern.TAG_LIST_OFFSET,
- 'handleTagListOffsetRoute',
- ctx => this.handleTagListOffsetRoute(ctx)
- );
-
- this.mapRoute(
- RoutePattern.TAG_LIST_FILTER_OFFSET,
- 'handleTagListFilterOffsetRoute',
- ctx => this.handleTagListFilterOffsetRoute(ctx)
- );
-
- this.mapRoute(
- RoutePattern.TAG_LIST_FILTER,
- 'handleTagListFilterRoute',
- ctx => this.handleTagListFilterRoute(ctx)
+ this.mapRoute(RoutePattern.TAG_LIST, 'handleTagListRoute', ctx =>
+ this.handleTagListRoute(ctx)
);
this.mapRoute(
@@ -1207,78 +1173,26 @@
this.reporting.setRepoName(repo);
}
- handleBranchListOffsetRoute(ctx: PageContext) {
+ handleBranchListRoute(ctx: PageContext) {
const state: RepoViewState = {
view: GerritView.REPO,
detail: RepoDetailView.BRANCHES,
repo: ctx.params[0] as RepoName,
offset: ctx.params[2] ?? '0',
- filter: null,
+ filter: ctx.params[1] ?? null,
};
// Note that router model view must be updated before view models.
this.setState(state);
this.repoViewModel.setState(state);
}
- handleBranchListFilterOffsetRoute(ctx: PageContext) {
- const state: RepoViewState = {
- view: GerritView.REPO,
- detail: RepoDetailView.BRANCHES,
- repo: ctx.params['repo'] as RepoName,
- offset: ctx.params['offset'] ?? '0',
- filter: ctx.params['filter'] ?? null,
- };
- // Note that router model view must be updated before view models.
- this.setState(state);
- this.repoViewModel.setState(state);
- }
-
- handleBranchListFilterRoute(ctx: PageContext) {
- const state: RepoViewState = {
- view: GerritView.REPO,
- detail: RepoDetailView.BRANCHES,
- repo: ctx.params['repo'] as RepoName,
- filter: ctx.params['filter'] ?? null,
- offset: '0',
- };
- // Note that router model view must be updated before view models.
- this.setState(state);
- this.repoViewModel.setState(state);
- }
-
- handleTagListOffsetRoute(ctx: PageContext) {
+ handleTagListRoute(ctx: PageContext) {
const state: RepoViewState = {
view: GerritView.REPO,
detail: RepoDetailView.TAGS,
repo: ctx.params[0] as RepoName,
offset: ctx.params[2] ?? '0',
- filter: null,
- };
- // Note that router model view must be updated before view models.
- this.setState(state);
- this.repoViewModel.setState(state);
- }
-
- handleTagListFilterOffsetRoute(ctx: PageContext) {
- const state: RepoViewState = {
- view: GerritView.REPO,
- detail: RepoDetailView.TAGS,
- repo: ctx.params['repo'] as RepoName,
- offset: ctx.params['offset'] ?? '0',
- filter: ctx.params['filter'],
- };
- // Note that router model view must be updated before view models.
- this.setState(state);
- this.repoViewModel.setState(state);
- }
-
- handleTagListFilterRoute(ctx: PageContext) {
- const state: RepoViewState = {
- view: GerritView.REPO,
- detail: RepoDetailView.TAGS,
- repo: ctx.params['repo'] as RepoName,
- offset: '0',
- filter: ctx.params['filter'] ?? null,
+ filter: ctx.params[1] ?? null,
};
// Note that router model view must be updated before view models.
this.setState(state);
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
index c9eb52e..6cad45c 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
@@ -165,9 +165,7 @@
assert.deepEqual(actualRequiresAuth, shouldRequireAutoAuth);
const unauthenticatedHandlers = [
- 'handleBranchListFilterOffsetRoute',
- 'handleBranchListFilterRoute',
- 'handleBranchListOffsetRoute',
+ 'handleBranchListRoute',
'handleChangeIdQueryRoute',
'handleChangeNumberLegacyRoute',
'handleChangeRoute',
@@ -193,9 +191,7 @@
'handleQueryLegacySuffixRoute',
'handleQueryRoute',
'handleRegisterRoute',
- 'handleTagListFilterOffsetRoute',
- 'handleTagListFilterRoute',
- 'handleTagListOffsetRoute',
+ 'handleTagListRoute',
'handlePluginScreen',
];
@@ -682,72 +678,66 @@
});
});
- suite('BRANCH_LIST_*', () => {
- test('BRANCH_LIST_OFFSET', async () => {
- // BRANCH_LIST_OFFSET: /^\/admin\/repos\/(.+),branches(,(.+))?$/,
- await checkUrlToState('/admin/repos/4321,branches', {
- ...createRepoBranchesViewState(),
- repo: '4321' as RepoName,
- });
- await checkUrlToState('/admin/repos/4321,branches,42', {
- ...createRepoBranchesViewState(),
- repo: '4321' as RepoName,
- offset: '42',
- });
+ test('BRANCH_LIST', async () => {
+ await checkUrlToState('/admin/repos/4321,branches', {
+ ...createRepoBranchesViewState(),
+ repo: '4321' as RepoName,
});
-
- test('BRANCH_LIST_FILTER_OFFSET', async () => {
- // BRANCH_LIST_FILTER_OFFSET: '/admin/repos/:repo,branches/q/filter::filter,:offset',
- await checkUrlToState('/admin/repos/4321,branches/q/filter:foo,42', {
- ...createRepoBranchesViewState(),
- repo: '4321' as RepoName,
- offset: '42',
- filter: 'foo',
- });
+ await checkUrlToState('/admin/repos/4321,branches,42', {
+ ...createRepoBranchesViewState(),
+ repo: '4321' as RepoName,
+ offset: '42',
});
-
- test('BRANCH_LIST_FILTER', async () => {
- // BRANCH_LIST_FILTER: '/admin/repos/:repo,branches/q/filter::filter',
- await checkUrlToState('/admin/repos/4321,branches/q/filter:foo', {
- ...createRepoBranchesViewState(),
- repo: '4321' as RepoName,
- filter: 'foo',
- });
+ await checkUrlToState('/admin/repos/4321,branches/q/filter:foo,42', {
+ ...createRepoBranchesViewState(),
+ repo: '4321' as RepoName,
+ offset: '42',
+ filter: 'foo',
});
+ await checkUrlToState('/admin/repos/4321,branches/q/filter:foo', {
+ ...createRepoBranchesViewState(),
+ repo: '4321' as RepoName,
+ filter: 'foo',
+ });
+ await checkUrlToState(
+ '/admin/repos/asdf/%2F%20%2525%252Fqwer,branches/q/filter:foo/%2F%20%2525%252F',
+ {
+ ...createRepoBranchesViewState(),
+ repo: 'asdf// %/qwer' as RepoName,
+ filter: 'foo// %/',
+ }
+ );
});
- suite('TAG_LIST_*', () => {
- test('TAG_LIST_OFFSET', async () => {
- // TAG_LIST_OFFSET: /^\/admin\/repos\/(.+),tags(,(.+))?$/,
- await checkUrlToState('/admin/repos/4321,tags', {
- ...createRepoTagsViewState(),
- repo: '4321' as RepoName,
- });
- await checkUrlToState('/admin/repos/4321,tags,42', {
- ...createRepoTagsViewState(),
- repo: '4321' as RepoName,
- offset: '42',
- });
+ test('TAG_LIST', async () => {
+ await checkUrlToState('/admin/repos/4321,tags', {
+ ...createRepoTagsViewState(),
+ repo: '4321' as RepoName,
});
-
- test('TAG_LIST_FILTER_OFFSET', async () => {
- // TAG_LIST_FILTER_OFFSET: '/admin/repos/:repo,tags/q/filter::filter,:offset',
- await checkUrlToState('/admin/repos/4321,tags/q/filter:foo,42', {
- ...createRepoTagsViewState(),
- repo: '4321' as RepoName,
- offset: '42',
- filter: 'foo',
- });
+ await checkUrlToState('/admin/repos/4321,tags,42', {
+ ...createRepoTagsViewState(),
+ repo: '4321' as RepoName,
+ offset: '42',
});
-
- test('TAG_LIST_FILTER', async () => {
- // TAG_LIST_FILTER: '/admin/repos/:repo,tags/q/filter::filter',
- await checkUrlToState('/admin/repos/4321,tags/q/filter:foo', {
- ...createRepoTagsViewState(),
- repo: '4321' as RepoName,
- filter: 'foo',
- });
+ await checkUrlToState('/admin/repos/4321,tags/q/filter:foo,42', {
+ ...createRepoTagsViewState(),
+ repo: '4321' as RepoName,
+ offset: '42',
+ filter: 'foo',
});
+ await checkUrlToState('/admin/repos/4321,tags/q/filter:foo', {
+ ...createRepoTagsViewState(),
+ repo: '4321' as RepoName,
+ filter: 'foo',
+ });
+ await checkUrlToState(
+ '/admin/repos/asdf/%2F%20%2525%252Fqwer,tags/q/filter:foo/%2F%20%2525%252F',
+ {
+ ...createRepoTagsViewState(),
+ repo: 'asdf// %/qwer' as RepoName,
+ filter: 'foo// %/',
+ }
+ );
});
test('REPO_LIST', async () => {
