Allow a space id to be used wherever a space name is accepted

src/spaceResolver.test.ts

@@ -0,0 +1,71 @@
+import type { Client } from "./client";
+import { Space } from "./features/spaces/space";
+import { resolveSpaceId } from "./spaceResolver";
+
+// resolveSpaceId memoises results in module scope, so each test uses a distinct
+// name/id to keep the cases independent.
+function space(id: string, name: string): Space {
+    return {
+        Id: id,
+        Name: name,
+        Slug: name.toLowerCase().replace(/\s/g, "-"),
+        IsDefault: false,
+        SpaceManagersTeamMembers: [],
+        SpaceManagersTeams: [],
+        TaskQueueStopped: false,
+    };
+}
+
+function clientReturning(handlers: { byName?: Space[]; byId?: Space }): { client: Client; paths: string[] } {
+    const paths: string[] = [];
+    const client = {
+        debug: () => undefined,
+        error: () => undefined,
+        get: (path: string) => {
+            paths.push(path);
+            if (path.includes("partialName=")) {
+                const items = handlers.byName ?? [];
+                return Promise.resolve({ Items: items, TotalResults: items.length });
+            }
+            if (handlers.byId) return Promise.resolve(handlers.byId);
+            return Promise.reject(new Error("404 not found"));
+        },
+    } as unknown as Client;
+    return { client, paths };
+}
+
+describe("resolveSpaceId", () => {
+    test("resolves an exact name match", async () => {
+        const target = space("Spaces-11", "By Name");
+        const { client } = clientReturning({ byName: [target] });
+        await expect(resolveSpaceId(client, "By Name")).resolves.toBe("Spaces-11");
+    });
+
+    test("ignores a partial name match that is not exact", async () => {
+        const { client } = clientReturning({ byName: [space("Spaces-12", "Partial Match Longer")] });
+        await expect(resolveSpaceId(client, "Partial Match")).rejects.toThrow("Unable to resolve space name or id 'Partial Match'");
+    });
+
+    test("resolves a space id when no name matches", async () => {
+        const { client } = clientReturning({ byName: [], byId: space("Spaces-1795", "Swordfish") });
+        await expect(resolveSpaceId(client, "Spaces-1795")).resolves.toBe("Spaces-1795");
+    });
+
+    test("prefers a name match over an id match", async () => {
+        const named = space("Spaces-13", "Spaces-9999");
+        const { client, paths } = clientReturning({ byName: [named], byId: space("Spaces-9999", "Other") });
+        await expect(resolveSpaceId(client, "Spaces-9999")).resolves.toBe("Spaces-13");
+        expect(paths.filter((p) => !p.includes("partialName="))).toHaveLength(0);
+    });
+
+    test("does not attempt an id lookup for a value that is not id-shaped", async () => {
+        const { client, paths } = clientReturning({ byName: [] });
+        await expect(resolveSpaceId(client, "Not An Id")).rejects.toThrow("No spaces exist with name or id 'Not An Id'");
+        expect(paths.filter((p) => !p.includes("partialName="))).toHaveLength(0);
+    });
+
+    test("throws when an id-shaped value matches no space", async () => {
+        const { client } = clientReturning({ byName: [] });
+        await expect(resolveSpaceId(client, "Spaces-4242")).rejects.toThrow("No spaces exist with name or id 'Spaces-4242'");
+    });
+});

src/spaceResolver.ts

@@ -1,40 +1,59 @@
-import type { Client } from "./client";
-import { apiLocation } from "./apiLocation";
-import { Space } from "./features/spaces/space";
-import { ResourceCollection } from "./resourceCollection";
-import { BasicRepository } from "./features";
-
-const knownSpaces: Record<string, string> = {};
-
-export async function resolveSpaceId(client: Client, spaceName: string): Promise<string> {
-    if (knownSpaces[spaceName]) {
-        return knownSpaces[spaceName];
-    }
-
-    client.debug(`Resolving space from name '${spaceName}'`);
-
-    const spaces = await client.get<ResourceCollection<Space>>(
-        `${apiLocation}/spaces?partialName=${encodeURIComponent(spaceName)}&skip=0&take=${BasicRepository.TakeAll}`
-    );
-    let spaceId = "";
-
-    if (spaces.TotalResults === 0) {
-        client.error(`No spaces exist with name '${spaceName}'`);
-        throw new Error(`No spaces exist with name '${spaceName}'`);
-    }
-
-    spaces.Items.forEach((space) => {
-        if (space.Name == spaceName) {
-            spaceId = space.Id;
-            knownSpaces[spaceName] = spaceId;
-            client.debug(`Resolved space name '${spaceName}' to Id ${spaceId}`);
-        }
-    });
-
-    if (spaceId === "") {
-        client.error(`Unable to resolve space name '${spaceName}'`);
-        throw new Error(`Unable to resolve space name '${spaceName}'`);
-    }
-
-    return spaceId;
-}
+import type { Client } from "./client";
+import { apiLocation } from "./apiLocation";
+import { Space } from "./features/spaces/space";
+import { ResourceCollection } from "./resourceCollection";
+import { BasicRepository } from "./features";
+
+const knownSpaces: Record<string, string> = {};
+
+// Server-assigned space ids take the form "Spaces-123".
+const spaceIdFormat = /^Spaces-\d+$/;
+
+export async function resolveSpaceId(client: Client, spaceNameOrId: string): Promise<string> {
+    if (knownSpaces[spaceNameOrId]) {
+        return knownSpaces[spaceNameOrId];
+    }
+
+    client.debug(`Resolving space from name or id '${spaceNameOrId}'`);
+
+    // We prefer to match on name and only then fall back to id, matching the
+    // behaviour of the CLI (https://github.com/OctopusDeploy/cli/issues/30). The
+    // server has no single filter covering both, so the name lookup runs first
+    // and the id lookup only when the value is shaped like a space id.
+    const spaces = await client.get<ResourceCollection<Space>>(
+        `${apiLocation}/spaces?partialName=${encodeURIComponent(spaceNameOrId)}&skip=0&take=${BasicRepository.TakeAll}`
+    );
+
+    const spaceMatchedByName = spaces.Items.find((space) => space.Name === spaceNameOrId);
+    if (spaceMatchedByName) {
+        knownSpaces[spaceNameOrId] = spaceMatchedByName.Id;
+        client.debug(`Resolved space name '${spaceNameOrId}' to Id ${spaceMatchedByName.Id}`);
+        return spaceMatchedByName.Id;
+    }
+
+    if (spaceIdFormat.test(spaceNameOrId)) {
+        const spaceMatchedById = await tryGetSpaceById(client, spaceNameOrId);
+        if (spaceMatchedById) {
+            knownSpaces[spaceNameOrId] = spaceMatchedById.Id;
+            client.debug(`Resolved space id '${spaceNameOrId}' to space '${spaceMatchedById.Name}'`);
+            return spaceMatchedById.Id;
+        }
+    }
+
+    if (spaces.TotalResults === 0) {
+        client.error(`No spaces exist with name or id '${spaceNameOrId}'`);
+        throw new Error(`No spaces exist with name or id '${spaceNameOrId}'`);
+    }
+
+    client.error(`Unable to resolve space name or id '${spaceNameOrId}'`);
+    throw new Error(`Unable to resolve space name or id '${spaceNameOrId}'`);
+}
+
+async function tryGetSpaceById(client: Client, spaceId: string): Promise<Space | undefined> {
+    try {
+        return await client.get<Space>(`${apiLocation}/spaces/${encodeURIComponent(spaceId)}`);
+    } catch (error) {
+        client.debug(`No space exists with id '${spaceId}'`);
+        return undefined;
+    }
+}