Handle breaking change in zoekt repo URL templates

:shakefist: Ref: sourcegraph/zoekt@5687809
isker · Nov 1, 2024 · 67a04c1 · 67a04c1
1 parent 9a1c731
commit 67a04c1
Show file tree

Hide file tree

Showing 10 changed files with 196 additions and 22 deletions.
diff --git a/.changeset/sweet-tables-attend.md b/.changeset/sweet-tables-attend.md
@@ -0,0 +1,5 @@
+---
+"neogrok": patch
+---
+
+Handle breaking change in zoekt repo URL templates
diff --git a/src/lib/server/search-api.ts b/src/lib/server/search-api.ts
@@ -6,6 +6,7 @@ import {
   parseFileNameMatch,
 } from "./content-parser";
 import { makeZoektRequest } from "./zoekt-client";
+import { evaluateFileUrlTemplate } from "$lib/url-templates";
 
 export const searchQuerySchema = v.object({
   query: v.string(),
@@ -232,6 +233,7 @@ const searchResultSchema = v.object({
         },
         files: files.map(
           ({ repository, version, fileName, chunks, ...rest }) => {
+            const fileUrlTemplate = repoUrls[repository];
             return {
               ...rest,
               repository,
@@ -243,9 +245,12 @@ const searchResultSchema = v.object({
               chunks,
               fileUrl:
                 version &&
-                repoUrls[repository]
-                  ?.replaceAll("{{.Version}}", version)
-                  .replaceAll("{{.Path}}", fileName.text),
+                fileUrlTemplate &&
+                evaluateFileUrlTemplate(
+                  fileUrlTemplate,
+                  version,
+                  fileName.text,
+                ),
               // The 'template' is such that the line number can be `join`ed
               // into it. JSON serializable!
               lineNumberTemplate:

diff --git a/src/lib/url-templates.test.ts b/src/lib/url-templates.test.ts
@@ -0,0 +1,83 @@
+import { describe, it, expect } from "vitest";
+import {
+  evaluateFileUrlTemplate,
+  evaluateCommitUrlTemplate,
+} from "./url-templates";
+
+describe("evaluateFileUrlTemplate", () => {
+  it("evaluates old templates", () => {
+    expect(
+      evaluateFileUrlTemplate(
+        "https://github.com/hanwen/go-fuse/blob/{{.Version}}/{{.Path}}",
+        "notify",
+        "genversion.sh",
+      ),
+    ).toEqual("https://github.com/hanwen/go-fuse/blob/notify/genversion.sh");
+    expect(
+      evaluateFileUrlTemplate(
+        "https://svn.future/r/{{.Version}}/nopath",
+        "12345",
+        "ignored",
+      ),
+    ).toEqual("https://svn.future/r/12345/nopath");
+  });
+
+  it("evaluates new templates", () => {
+    expect(
+      evaluateFileUrlTemplate(
+        '{{URLJoinPath "https://github.com/hanwen/go-fuse/blob" .Version .Path}}',
+        "notify",
+        "genversion.sh",
+      ),
+    ).toEqual("https://github.com/hanwen/go-fuse/blob/notify/genversion.sh");
+    expect(
+      evaluateFileUrlTemplate(
+        '{{ URLJoinPath    "https://github.com/hanwen/go-fuse/blob" .Version .Path  }}',
+        "notify",
+        "genversion.sh",
+      ),
+    ).toEqual("https://github.com/hanwen/go-fuse/blob/notify/genversion.sh");
+    expect(
+      evaluateFileUrlTemplate(
+        '{{URLJoinPath "https://svn.future/r" .Version "nopath"}}',
+        "12345",
+        "ignored",
+      ),
+    ).toEqual("https://svn.future/r/12345/nopath");
+  });
+});
+
+describe("evaluateCommitUrlTemplate", () => {
+  it("evaluates old templates", () => {
+    expect(
+      evaluateCommitUrlTemplate(
+        "https://github.com/hanwen/go-fuse/commit/{{.Version}}",
+        "deadbeef",
+      ),
+    ).toEqual("https://github.com/hanwen/go-fuse/commit/deadbeef");
+    expect(
+      evaluateCommitUrlTemplate("https://svn.future/r/{{.Version}}", "12345"),
+    ).toEqual("https://svn.future/r/12345");
+  });
+
+  it("evaluates new templates", () => {
+    expect(
+      evaluateCommitUrlTemplate(
+        '{{URLJoinPath "https://github.com/hanwen/go-fuse/commit" .Version}}',
+        "deadbeef",
+      ),
+    ).toEqual("https://github.com/hanwen/go-fuse/commit/deadbeef");
+    expect(
+      evaluateCommitUrlTemplate(
+        '{{ URLJoinPath    "https://github.com/hanwen/go-fuse/commit" .Version   }}',
+        "deadbeef",
+      ),
+    ).toEqual("https://github.com/hanwen/go-fuse/commit/deadbeef");
+    expect(
+      evaluateCommitUrlTemplate(
+        '{{URLJoinPath "https://svn.future/r" .Version}}',
+        "12345",
+      ),
+    ).toEqual("https://svn.future/r/12345");
+  });
+});
diff --git a/src/lib/url-templates.ts b/src/lib/url-templates.ts
@@ -0,0 +1,57 @@
+// CommitURLTemplate and FileURLTemplate used to be simple. No longer:
+// https://github.com/sourcegraph/zoekt/commit/5687809315075882a8e7413bdb17b042f3394c02
+//
+// These functions handle both the old and new versions of the templates.
+
+const urlJoinPathTemplate = /^{{\s*URLJoinPath\s+(?<args>.*?)\s*}}$/;
+
+export const evaluateFileUrlTemplate = (
+  template: string,
+  version: string,
+  path: string,
+): string => {
+  const match = template.match(urlJoinPathTemplate);
+  if (match?.groups) {
+    const { args } = match.groups;
+    return args
+      .split(/\s+/)
+      .map((s) => {
+        if (s === ".Version") {
+          return version;
+        } else if (s === ".Path") {
+          return path;
+        } else {
+          // It's a quoted string: https://pkg.go.dev/strconv#Quote.
+          return JSON.parse(s);
+        }
+      })
+      .join("/");
+  } else {
+    return template
+      .replaceAll("{{.Version}}", version)
+      .replaceAll("{{.Path}}", path);
+  }
+};
+
+export const evaluateCommitUrlTemplate = (
+  template: string,
+  version: string,
+): string => {
+  const match = template.match(urlJoinPathTemplate);
+  if (match?.groups) {
+    const { args } = match.groups;
+    return args
+      .split(/\s+/)
+      .map((s) => {
+        if (s === ".Version") {
+          return version;
+        } else {
+          // It's a quoted string: https://pkg.go.dev/strconv#Quote.
+          return JSON.parse(s);
+        }
+      })
+      .join("/");
+  } else {
+    return template.replaceAll("{{.Version}}", version);
+  }
+};
diff --git a/src/routes/(opengrok-compat)/diff/[project]/[...file]/+page.server.ts b/src/routes/(opengrok-compat)/diff/[project]/[...file]/+page.server.ts
@@ -1,6 +1,7 @@
 import { escapeRegExp } from "$lib/regexp";
 import { configuration } from "$lib/server/configuration";
 import { listRepositories } from "$lib/server/zoekt-list-repositories";
+import { evaluateFileUrlTemplate } from "$lib/url-templates";
 import { error, redirect } from "@sveltejs/kit";
 
 export const load: import("./$types").PageServerLoad = async ({
@@ -27,9 +28,13 @@ export const load: import("./$types").PageServerLoad = async ({
   }
 
   const repo = result.results.repositories[0];
-  const fileUrl = repo?.fileUrlTemplate
-    ?.replaceAll("{{.Version}}", revision ?? repo.branches[0].name)
-    .replaceAll("{{.Path}}", file);
+  const fileUrl =
+    repo?.fileUrlTemplate &&
+    evaluateFileUrlTemplate(
+      repo.fileUrlTemplate,
+      revision ?? repo.branches[0]?.name,
+      file,
+    );
 
   setHeaders({
     "cache-control": "no-store,must-revalidate",

diff --git a/src/routes/(opengrok-compat)/download/[project]/[...file]/+page.server.ts b/src/routes/(opengrok-compat)/download/[project]/[...file]/+page.server.ts
@@ -1,6 +1,7 @@
 import { escapeRegExp } from "$lib/regexp";
 import { configuration } from "$lib/server/configuration";
 import { listRepositories } from "$lib/server/zoekt-list-repositories";
+import { evaluateFileUrlTemplate } from "$lib/url-templates";
 import { error, redirect } from "@sveltejs/kit";
 
 export const load: import("./$types").PageServerLoad = async ({
@@ -27,9 +28,13 @@ export const load: import("./$types").PageServerLoad = async ({
   }
 
   const repo = result.results.repositories[0];
-  const fileUrl = repo?.fileUrlTemplate
-    ?.replaceAll("{{.Version}}", revision ?? repo.branches[0].name)
-    .replaceAll("{{.Path}}", file);
+  const fileUrl =
+    repo?.fileUrlTemplate &&
+    evaluateFileUrlTemplate(
+      repo.fileUrlTemplate,
+      revision ?? repo.branches[0]?.name,
+      file,
+    );
 
   setHeaders({
     "cache-control": "no-store,must-revalidate",

diff --git a/src/routes/(opengrok-compat)/history/[project]/[...file]/+page.server.ts b/src/routes/(opengrok-compat)/history/[project]/[...file]/+page.server.ts
@@ -1,6 +1,7 @@
 import { escapeRegExp } from "$lib/regexp";
 import { configuration } from "$lib/server/configuration";
 import { listRepositories } from "$lib/server/zoekt-list-repositories";
+import { evaluateFileUrlTemplate } from "$lib/url-templates";
 import { redirect } from "@sveltejs/kit";
 
 export const load: import("./$types").PageServerLoad = async ({
@@ -22,11 +23,15 @@ export const load: import("./$types").PageServerLoad = async ({
   }
   const repo = result.results.repositories[0];
 
-  let destinationUrl: string | undefined;
+  let destinationUrl: string | null | undefined;
   if (file) {
-    destinationUrl = repo?.fileUrlTemplate
-      ?.replaceAll("{{.Version}}", repo.branches[0].name)
-      .replaceAll("{{.Path}}", file);
+    destinationUrl =
+      repo?.fileUrlTemplate &&
+      evaluateFileUrlTemplate(
+        repo.fileUrlTemplate,
+        repo.branches[0]?.name,
+        file,
+      );
   } else {
     destinationUrl = repo?.url;
   }

diff --git a/src/routes/(opengrok-compat)/raw/[project]/[...file]/+page.server.ts b/src/routes/(opengrok-compat)/raw/[project]/[...file]/+page.server.ts
@@ -1,6 +1,7 @@
 import { escapeRegExp } from "$lib/regexp";
 import { configuration } from "$lib/server/configuration";
 import { listRepositories } from "$lib/server/zoekt-list-repositories";
+import { evaluateFileUrlTemplate } from "$lib/url-templates";
 import { error, redirect } from "@sveltejs/kit";
 
 export const load: import("./$types").PageServerLoad = async ({
@@ -27,9 +28,13 @@ export const load: import("./$types").PageServerLoad = async ({
   }
 
   const repo = result.results.repositories[0];
-  const fileUrl = repo?.fileUrlTemplate
-    ?.replaceAll("{{.Version}}", revision ?? repo.branches[0].name)
-    .replaceAll("{{.Path}}", file);
+  const fileUrl =
+    repo?.fileUrlTemplate &&
+    evaluateFileUrlTemplate(
+      repo.fileUrlTemplate,
+      revision ?? repo.branches[0]?.name,
+      file,
+    );
 
   setHeaders({
     "cache-control": "no-store,must-revalidate",

diff --git a/src/routes/(opengrok-compat)/xref/[project]/[...file]/+page.server.ts b/src/routes/(opengrok-compat)/xref/[project]/[...file]/+page.server.ts
@@ -1,6 +1,7 @@
 import { escapeRegExp } from "$lib/regexp";
 import { configuration } from "$lib/server/configuration";
 import { listRepositories } from "$lib/server/zoekt-list-repositories";
+import { evaluateFileUrlTemplate } from "$lib/url-templates";
 import { redirect } from "@sveltejs/kit";
 
 export const load: import("./$types").PageServerLoad = async ({
@@ -24,11 +25,13 @@ export const load: import("./$types").PageServerLoad = async ({
   }
   const repo = result.results.repositories[0];
 
-  let destinationUrl: string | undefined;
-  if (file) {
-    destinationUrl = repo?.fileUrlTemplate
-      ?.replaceAll("{{.Version}}", revision ?? repo.branches[0].name)
-      .replaceAll("{{.Path}}", file);
+  let destinationUrl: string | null | undefined;
+  if (file && repo?.fileUrlTemplate) {
+    destinationUrl = evaluateFileUrlTemplate(
+      repo.fileUrlTemplate,
+      revision ?? repo.branches[0]?.name,
+      file,
+    );
   } else {
     destinationUrl = repo?.url;
   }

diff --git a/src/routes/repositories/branches.svelte b/src/routes/repositories/branches.svelte
@@ -1,6 +1,7 @@
 <script lang="ts">
   import Link from "$lib/link.svelte";
   import type { Repository } from "$lib/server/zoekt-list-repositories";
+  import { evaluateCommitUrlTemplate } from "$lib/url-templates";
 
   export let branches: Repository["branches"];
   export let commitUrlTemplate: string | undefined;
@@ -13,7 +14,7 @@
 {#each branches as { name: branchName, version }}
   {branchName}@<span class="font-mono">
     {#if commitUrlTemplate}
-      <Link to={commitUrlTemplate.replaceAll("{{.Version}}", version)}
+      <Link to={evaluateCommitUrlTemplate(commitUrlTemplate, version)}
         >{abbreviateVersion(version)}</Link
       >
     {:else}