URL alternative resolution criteria (#2497)

GitbookIO · Sep 30, 2024 · 1152445 · 1152445
1 parent 8f73642
commit 1152445
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 23 deletions.
diff --git a/.changeset/wicked-bees-change.md b/.changeset/wicked-bees-change.md
@@ -0,0 +1,5 @@
+---
+'gitbook': minor
+---
+
+Changed the alternative URL resolution criteria in order to support site URLs without /v/ prefix
diff --git a/packages/gitbook/src/lib/api.ts b/packages/gitbook/src/lib/api.ts
@@ -127,7 +127,17 @@ export function withAPI<T>(client: GitBookAPI, fn: () => Promise<T>): Promise<T>
 }
 
 export type PublishedContentWithCache =
-    | ((PublishedContentLookup | PublishedSiteContentLookup) & {
+    | ((
+          | PublishedContentLookup
+          | (PublishedSiteContentLookup & {
+                //TODO: Remove this once the @gitbook/api is bumped
+                /**
+                 * Whether the resolved site URL is complete and at it's terminal point, meaning no more site path segments
+                 * can be further expected before any page path segments.
+                 */
+                complete: boolean;
+            })
+      ) & {
           cacheMaxAge?: number;
           cacheTags?: string[];
       })

diff --git a/packages/gitbook/src/lib/middleware.test.ts b/packages/gitbook/src/lib/middleware.test.ts
@@ -22,6 +22,11 @@ describe('getURLLookupAlternatives', () => {
                 {
                     extraPath: 'c',
                     url: 'https://docs.mycompany.com/a/b',
+                    primary: false,
+                },
+                {
+                    extraPath: '',
+                    url: 'https://docs.mycompany.com/a/b/c',
                     primary: true,
                 },
             ],
@@ -229,6 +234,11 @@ describe('getURLLookupAlternatives', () => {
                 {
                     extraPath: 'c/d',
                     url: 'https://docs.mycompany.com/a/b',
+                    primary: false,
+                },
+                {
+                    extraPath: 'd',
+                    url: 'https://docs.mycompany.com/a/b/c',
                     primary: true,
                 },
             ],
@@ -254,6 +264,11 @@ describe('getURLLookupAlternatives', () => {
                 {
                     extraPath: 'b/c/d',
                     url: 'https://docs.mycompany.com/a/~',
+                    primary: false,
+                },
+                {
+                    extraPath: 'c/d',
+                    url: 'https://docs.mycompany.com/a/~/b',
                     primary: true,
                 },
             ],

diff --git a/packages/gitbook/src/lib/middleware.ts b/packages/gitbook/src/lib/middleware.ts
@@ -91,7 +91,7 @@ export function getURLLookupAlternatives(input: URL) {
         }
 
         // Otherwise match with the first two segments of the path
-        for (let i = 1; i <= 2; i++) {
+        for (let i = 1; i <= 3; i++) {
             if (pathSegments.length >= i) {
                 const shortURL = new URL(url);
                 shortURL.pathname = pathSegments.slice(0, i).join('/');

diff --git a/packages/gitbook/src/middleware.ts b/packages/gitbook/src/middleware.ts
@@ -666,27 +666,38 @@ async function lookupSpaceByAPI(
             return null;
         }
 
-        const changeRequest = data.changeRequest ?? lookup.changeRequest;
-        return {
-            space: data.space,
-            changeRequest,
-            revision: data.revision ?? lookup.revision,
-            basePath: joinPath(data.basePath, lookup.basePath ?? ''),
-            pathname: joinPath(data.pathname, alternative.extraPath),
-            apiToken: data.apiToken,
-            // We don't cache change requests as they often change and we want to have consistent previews
-            // Purging the CDN cache will not be efficient enough.
-            cacheMaxAge: changeRequest ? 0 : data.cacheMaxAge,
-            cacheTags: data.cacheTags,
-            ...('site' in data
-                ? {
-                      site: data.site,
-                      siteSpace: data.siteSpace,
-                      organization: data.organization,
-                      shareKey: data.shareKey,
-                  }
-                : {}),
-        } as PublishedContentWithCache;
+        /**
+         * We use the following criteria to determine if the lookup result is the right one:
+         * - the primary alternative was resolved (because that's the longest or most inclusive path)
+         * - the resolution of the site URL is complete (because we want to resolve the deepest path possible)
+         *
+         * In both cases, the idea is to use the deepest/longest/most inclusive path to resolve the content.
+         */
+        if (alternative.primary || ('site' in data && data.complete)) {
+            const changeRequest = data.changeRequest ?? lookup.changeRequest;
+            return {
+                space: data.space,
+                changeRequest,
+                revision: data.revision ?? lookup.revision,
+                basePath: joinPath(data.basePath, lookup.basePath ?? ''),
+                pathname: joinPath(data.pathname, alternative.extraPath),
+                apiToken: data.apiToken,
+                // We don't cache change requests as they often change and we want to have consistent previews
+                // Purging the CDN cache will not be efficient enough.
+                cacheMaxAge: changeRequest ? 0 : data.cacheMaxAge,
+                cacheTags: data.cacheTags,
+                ...('site' in data
+                    ? {
+                          site: data.site,
+                          siteSpace: data.siteSpace,
+                          organization: data.organization,
+                          shareKey: data.shareKey,
+                      }
+                    : {}),
+            } as PublishedContentWithCache;
+        }
+
+        return null;
     });
 
     return (