Skip to content

Commit 7fc49ba

Browse files
stephanwleestephanwlee
committed
Use two arg constructor for URL for relative path
1 parent e6ed295 commit 7fc49ba

File tree

2 files changed

+45
-64
lines changed

2 files changed

+45
-64
lines changed

‎tensorboard/components/tf_backend/router.ts‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ export interface Router {
2828
* Create a router for communicating with the TensorBoard backend. You
2929
* can pass this to `setRouter` to make it the global router.
3030
*
31-
* @param pathPrefix {string} The base prefix for data endpoints.
31+
* @param pathPrefix {string=} The base prefix for data endpoints.
3232
* @param demoMode {boolean=} Whether to modify urls for filesystem demo usage.
3333
*/
34-
export function createRouter(pathPrefix, demoMode = false): Router {
34+
export function createRouter(pathPrefix = 'data', demoMode = false): Router {
3535
if (pathPrefix[pathPrefix.length - 1] === '/') {
3636
pathPrefix = pathPrefix.slice(0, pathPrefix.length - 1);
3737
}
@@ -62,13 +62,7 @@ export function createRouter(pathPrefix, demoMode = false): Router {
6262
};
6363
};
6464

65-
export function getDefaultRouter(): Router {
66-
const sep = window.location.pathname.endsWith('/') ? '' : '/';
67-
const pathPrefix = window.location.pathname + sep + 'data';
68-
return createRouter(pathPrefix);
69-
}
70-
71-
let _router: Router = getDefaultRouter();
65+
let _router: Router = createRouter();
7266

7367
/**
7468
* @return {Router} the global router
@@ -94,9 +88,9 @@ export function setRouter(router: Router): void {
9488

9589
function createProdPath(pathPrefix: string, path: string,
9690
ext: string, params?: URLSearchParams): string {
97-
const url = new URL(window.location.origin);
9891
// Use URL to normalize pathPrefix with leading slash and without.
99-
url.pathname = pathPrefix + path;
92+
const maybeRelativePath = pathPrefix + path;
93+
const url = new URL(maybeRelativePath, window.location.href);
10094
if (params) url.search = params.toString();
10195
return url.pathname + url.search;
10296
}

‎tensorboard/components/tf_backend/test/backendTests.ts‎

Lines changed: 40 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -76,54 +76,39 @@ describe('backend tests', () => {
7676
});
7777

7878
describe('router', () => {
79-
it('removes trailing slash from base route', () => {
80-
const r = createRouter('foo/');
81-
assert.equal(r.runs(), '/foo/runs');
82-
});
83-
84-
describe('default router', () => {
85-
beforeEach(function() {
86-
this.router = getDefaultRouter();
87-
});
88-
89-
it('makes sure test is not degenerate', function() {
90-
// Test becomes degenerate if location.pathname is empty.
91-
assert.notEqual('/', window.location.pathname);
92-
});
93-
94-
it('creates a URL including the pathname', function() {
95-
assert.equal(
96-
this.router.runs(),
97-
`${window.location.pathname}/data/runs`);
98-
});
99-
});
100-
10179
describe('prod mode', () => {
10280
beforeEach(function() {
10381
this.router = createRouter(base, /*demoMode=*/false);
10482
});
10583

106-
it('ignores leading slash in pathPrefix', () => {
84+
function assertRelativePath(actual, expectedRelativePath) {
85+
// Path prefix is /tf-backend/test should match path of the
86+
// test_web_library in the test build target.
87+
assert.equal(actual, `/tf-backend/test/${expectedRelativePath}`);
88+
}
89+
90+
it('leading slash in pathPrefix is an absolute path', () => {
10791
const router = createRouter('/data/', /*demoMode=*/false);
10892
assert.equal(router.runs(), '/data/runs');
10993
});
11094

111-
it('returns leading slash when pathPrefix omits one', () => {
95+
it('returns complete pathname when pathPrefix omits slash', () => {
11296
const router = createRouter('data/', /*demoMode=*/false);
113-
assert.equal(router.runs(), '/data/runs');
97+
assertRelativePath(router.runs(), 'data/runs');
11498
});
11599

116-
it('does not strip leading slash if pathPrefix has more than one', () => {
117-
const router = createRouter('///data/', /*demoMode=*/false);
118-
assert.equal(router.runs(), '///data/runs');
100+
it('treats more than two slashes as absolute url', () => {
101+
const router = createRouter('///data/hello', /*demoMode=*/false);
102+
// This becomes 'http://data/hello/runs'
103+
assert.equal(router.runs(), '/hello/runs');
119104
});
120105

121106
it('returns correct value for #environment', function() {
122-
assert.equal(this.router.environment(), '/data/environment');
107+
assertRelativePath(this.router.environment(), 'data/environment');
123108
});
124109

125110
it('returns correct value for #experiments', function() {
126-
assert.equal(this.router.experiments(), '/data/experiments');
111+
assertRelativePath(this.router.experiments(), 'data/experiments');
127112
});
128113

129114
it('returns correct value for #isDemoMode', function() {
@@ -132,68 +117,69 @@ describe('backend tests', () => {
132117

133118
describe('#pluginRoute', () => {
134119
it('encodes slash correctly', function() {
135-
assert.equal(
120+
assertRelativePath(
136121
this.router.pluginRoute('scalars', '/scalar'),
137-
'/data/plugin/scalars/scalar');
122+
'data/plugin/scalars/scalar');
138123
});
139124

140125
it('encodes query param correctly', function() {
141-
assert.equal(
126+
assertRelativePath(
142127
this.router.pluginRoute(
143128
'scalars',
144129
'/a',
145130
createSearchParam({b: 'c', d: ['1', '2']})),
146-
'/data/plugin/scalars/a?b=c&d=1&d=2');
131+
'data/plugin/scalars/a?b=c&d=1&d=2');
147132
});
148133

149134
it('encodes parenthesis correctly', function() {
150-
assert.equal(
135+
assertRelativePath(
151136
this.router.pluginRoute('scalars', '/a',
152137
createSearchParam({foo: '()'})),
153-
'/data/plugin/scalars/a?foo=%28%29');
138+
'data/plugin/scalars/a?foo=%28%29');
154139
});
155140

156141
it('encodes query param the same as #addParams', function() {
157-
assert.equal(
142+
assertRelativePath(
158143
this.router.pluginRoute(
159144
'scalars',
160145
'/a',
161146
createSearchParam({b: 'c', d: ['1']})),
162-
addParams('/data/plugin/scalars/a', {b: 'c', d: ['1']}));
163-
assert.equal(
147+
addParams('data/plugin/scalars/a', {b: 'c', d: ['1']}));
148+
assertRelativePath(
164149
this.router.pluginRoute(
165150
'scalars',
166151
'/a',
167152
createSearchParam({foo: '()'})),
168-
addParams('/data/plugin/scalars/a', {foo: '()'}));
153+
addParams('data/plugin/scalars/a', {foo: '()'}));
169154
});
170155

171156
it('ignores custom extension', function() {
172-
assert.equal(
157+
assertRelativePath(
173158
this.router.pluginRoute('scalars', '/a', undefined, 'meow'),
174-
'/data/plugin/scalars/a');
159+
'data/plugin/scalars/a');
175160
});
176161
});
177162

178163
it('returns correct value for #pluginsListing', function() {
179-
assert.equal(this.router.pluginsListing(), '/data/plugins_listing');
164+
assertRelativePath(
165+
this.router.pluginsListing(), 'data/plugins_listing');
180166
});
181167

182168
it('returns correct value for #runs', function() {
183-
assert.equal(this.router.runs(), '/data/runs');
169+
assertRelativePath(this.router.runs(), 'data/runs');
184170
});
185171

186172
it('returns correct value for #runsForExperiment', function() {
187173
// No experiment id is passed.
188-
assert.equal(
174+
assertRelativePath(
189175
this.router.runsForExperiment(''),
190-
'/data/experiment_runs');
191-
assert.equal(
176+
'data/experiment_runs');
177+
assertRelativePath(
192178
this.router.runsForExperiment('1'),
193-
'/data/experiment_runs?experiment=1');
194-
assert.equal(
179+
'data/experiment_runs?experiment=1');
180+
assertRelativePath(
195181
this.router.runsForExperiment('1&foo=false'),
196-
'/data/experiment_runs?experiment=1%26foo%3Dfalse');
182+
'data/experiment_runs?experiment=1%26foo%3Dfalse');
197183
});
198184
});
199185

@@ -202,18 +188,19 @@ describe('backend tests', () => {
202188
this.router = createRouter(base, /*demoMode=*/true);
203189
});
204190

205-
it('ignores leading slash in pathPrefix', () => {
191+
it('treats pathPrefix with leading slash as absolute path', () => {
206192
const router = createRouter('/data/', /*demoMode=*/true);
207193
assert.equal(router.runs(), '/data/runs.json');
208194
});
209195

210-
it('returns leading slash when pathPrefix omits one', () => {
196+
it('treats pathPrefix without leading slash as absolute path', () => {
211197
const router = createRouter('data/', /*demoMode=*/true);
212198
assert.equal(router.runs(), '/data/runs.json');
213199
});
214200

215-
it('does not strip leading slash if pathPrefix has more than one', () => {
201+
it('does not form absolute url with multiple leading slashes', () => {
216202
const router = createRouter('///data/', /*demoMode=*/true);
203+
// For prod url, this would be http://data/runs
217204
assert.equal(router.runs(), '///data/runs.json');
218205
});
219206

0 commit comments

Comments
 (0)