(core) Treating API urls as external in cells

Summary:
Links for the API endpoints in a cell didn't work as they were interpreted as
internal routes. Now they are properly detected as external.

Test Plan: Added new test

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D4078
This commit is contained in:
Jarosław Sadziński 2023-10-17 18:31:33 +02:00
parent 77726849ad
commit 69d5ee53a8
5 changed files with 50 additions and 8 deletions

View File

@ -188,6 +188,9 @@ export class UrlStateImpl {
* a matter of DocWorker requiring a different version (e.g. /v/OTHER/doc/...).
*/
public needPageLoad(prevState: IGristUrlState, newState: IGristUrlState): boolean {
// If we have an API URL we can't use it to switch the state, so we need a page load.
if (newState.api || prevState.api) { return true; }
const gristConfig = this._window.gristConfig || {};
const orgReload = prevState.org !== newState.org;
// Reload when moving to/from a document or between doc and non-doc.

View File

@ -135,6 +135,9 @@ export interface IGristUrlState {
themeName?: ThemeName;
};
hash?: HashLink; // if present, this specifies an individual row within a section of a page.
api?: boolean; // indicates that the URL should be encoded as an API URL, not as a landing page.
// But this barely works, and is suitable only for documents. For decoding it
// indicates that the URL probably points to an API endpoint.
}
// Subset of GristLoadConfig used by getOrgUrlInfo(), which affects the interpretation of the
@ -217,9 +220,6 @@ export function getOrgUrlInfo(newOrg: string, currentHost: string, options: OrgU
export function encodeUrl(gristConfig: Partial<GristLoadConfig>,
state: IGristUrlState, baseLocation: Location | URL,
options: {
// make an api url - warning: just barely works, and
// only for documents
api?: boolean,
tweaks?: UrlTweaks,
} = {}): string {
const url = new URL(baseLocation.href);
@ -236,12 +236,12 @@ export function encodeUrl(gristConfig: Partial<GristLoadConfig>,
}
}
if (options.api) {
if (state.api) {
parts.push(`api/`);
}
if (state.ws) { parts.push(`ws/${state.ws}/`); }
if (state.doc) {
if (options.api) {
if (state.api) {
parts.push(`docs/${encodeURIComponent(state.doc)}`);
} else if (state.slug) {
parts.push(`${encodeURIComponent(state.doc)}/${encodeURIComponent(state.slug)}`);
@ -329,10 +329,27 @@ export function decodeUrl(gristConfig: Partial<GristLoadConfig>, location: Locat
location = new URL(location.href); // Make sure location is a URL.
options?.tweaks?.preDecode?.({ url: location });
const parts = location.pathname.slice(1).split('/');
const state: IGristUrlState = {};
// Bare minimum we can do to detect API URLs.
if (parts[0] === 'api') { // When it starts with /api/...
parts.shift();
state.api = true;
} else if (parts[0] === 'o' && parts[2] === 'api') { // or with /o/{org}/api/...
parts.splice(2, 1);
state.api = true;
}
const map = new Map<string, string>();
for (let i = 0; i < parts.length; i += 2) {
map.set(parts[i], decodeURIComponent(parts[i + 1]));
}
// For the API case, we need to map "docs" to "doc" (as this is what we did in encodeUrl and what API expects).
if (state.api && map.has('docs')) {
map.set('doc', map.get('docs')!);
}
// When the urlId is a prefix of the docId, documents are identified
// as "<urlId>/slug" instead of "doc/<urlId>". We can detect that because
// the minimum length of a urlId prefix is longer than the maximum length
@ -346,7 +363,6 @@ export function decodeUrl(gristConfig: Partial<GristLoadConfig>, location: Locat
}
}
const state: IGristUrlState = {};
const subdomain = parseSubdomain(location.host);
if (gristConfig.org || gristConfig.singleOrg) {
state.org = gristConfig.org || gristConfig.singleOrg;

View File

@ -1547,9 +1547,9 @@ export class FlexServer implements GristServer {
state.slug = getSlugIfNeeded(resource);
}
state.org = this._dbManager.normalizeOrgDomain(org.id, org.domain, org.ownerId);
state.api = purpose === 'api';
if (!gristConfig.homeUrl) { throw new Error('Computing a resource URL requires a home URL'); }
return encodeUrl(gristConfig, state, new URL(gristConfig.homeUrl),
{ api: purpose === 'api' });
return encodeUrl(gristConfig, state, new URL(gristConfig.homeUrl));
}
public addUsage() {

View File

@ -90,6 +90,12 @@ describe('gristUrlState', function() {
// Billing routes
assert.deepEqual(prod.decodeUrl(new URL('https://bar.example.com/o/baz/billing')),
{org: 'baz', billing: 'billing'});
// API routes
assert.deepEqual(prod.decodeUrl(new URL('https://bar.example.com/api/docs/bar')),
{org: 'bar', doc: 'bar', api: true});
assert.deepEqual(prod.decodeUrl(new URL('http://localhost:8080/o/baz/api/docs/bar')),
{org: 'baz', doc: 'bar', api: true});
});
it('should decode query strings in URLs correctly', function() {
@ -139,6 +145,11 @@ describe('gristUrlState', function() {
// Billing routes
assert.equal(prod.encodeUrl({org: 'baz', billing: 'billing'}, hostBase),
'https://baz.example.com/billing');
// API routes
assert.equal(prod.encodeUrl({org: 'baz', doc: 'bar', api: true}, hostBase), 'https://baz.example.com/api/docs/bar');
assert.equal(prod.encodeUrl({org: 'baz', doc: 'bar', api: true}, localBase),
'http://localhost:8080/o/baz/api/docs/bar');
});
it('should encode state in billing URLs correctly', function() {

View File

@ -45,6 +45,18 @@ describe('gristUrls', function() {
{params: {themeName: 'GristDark'}},
);
});
it('should detect API URLs', function() {
assertUrlDecode(
'http://localhost/o/docs/api/docs',
{api: true},
);
assertUrlDecode(
'http://public.getgrist.com/api/docs',
{api: true},
);
});
});
describe('parseFirstUrlPart', function() {