From 8c8bd634fc74a6b1eab490f98944934960835afd Mon Sep 17 00:00:00 2001 From: Alex Kanitz Date: Fri, 13 Nov 2026 08:26:25 +0100 Subject: [PATCH 1/8] fix: react to provider set after the first update The Service Registry services and service components, the TRS tools list, the TRS-Filer tool form and the DRS objects list read `provider` only in firstUpdated. React sets it on a server-rendered element after that, so they never loaded data. They now switch provider whenever `provider` or `baseUrl` changes: a custom provider takes precedence over `baseUrl` (as documented), and the base URL is used once the provider is removed. An unchanged provider or base URL no longer triggers a second load right after the first update. Adds tests for these components and the test scripts their packages lacked (unique ports 8003-8006). Closes #565 Co-Authored-By: Claude Opus 5.5 --- .../ecc-client-elixir-trs-filer/package.json | 1 + .../tool-create/tests/tool-create.test.ts | 82 +++++++++++++ .../src/components/tool-create/tool-create.ts | 24 +++- packages/ecc-client-ga4gh-drs/package.json | 1 + .../src/components/objects-list/objects.ts | 24 +++- .../objects-list/tests/objects.test.ts | 80 +++++++++++++ .../package.json | 1 + .../src/components/service/service.ts | 34 ++++-- .../components/service/tests/service.test.ts | 108 +++++++++++++++++ .../src/components/services/services.ts | 24 +++- .../services/tests/services.test.ts | 109 ++++++++++++++++++ packages/ecc-client-ga4gh-trs/package.json | 1 + .../src/components/tools/tests/tools.test.ts | 85 ++++++++++++++ .../src/components/tools/tools.ts | 24 +++- web-test.runner-config.mjs | 3 +- 15 files changed, 584 insertions(+), 17 deletions(-) create mode 100644 packages/ecc-client-elixir-trs-filer/src/components/tool-create/tests/tool-create.test.ts create mode 100644 packages/ecc-client-ga4gh-drs/src/components/objects-list/tests/objects.test.ts create mode 100644 packages/ecc-client-ga4gh-service-registry/src/components/service/tests/service.test.ts create mode 100644 packages/ecc-client-ga4gh-service-registry/src/components/services/tests/services.test.ts create mode 100644 packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts diff --git a/packages/ecc-client-elixir-trs-filer/package.json b/packages/ecc-client-elixir-trs-filer/package.json index e423dd70..98a8586b 100644 --- a/packages/ecc-client-elixir-trs-filer/package.json +++ b/packages/ecc-client-elixir-trs-filer/package.json @@ -38,6 +38,7 @@ "build": "node ../../scripts/tailwind/index.js && node ../../scripts/build.js -p ecc-client-elixir-trs-filer-", "dev": "concurrently -r \"node ../../scripts/tailwind/index.js --watch\" \"node ../../scripts/build.js -p ecc-client-elixir-trs-filer- --watch\" \"wds\"", "clean": "rm -rf dist node_modules custom-elements-manifest.config.js src/output.css src/tailwind.ts", + "test": "web-test-runner --group default --config ../../web-test.runner-config.mjs --port 8005", "test:watch": "wtr --watch", "lint": "npx eslint .", "lint:fix": "npm run lint -- --fix", diff --git a/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tests/tool-create.test.ts b/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tests/tool-create.test.ts new file mode 100644 index 00000000..3c837f43 --- /dev/null +++ b/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tests/tool-create.test.ts @@ -0,0 +1,82 @@ +/* eslint-disable import/no-extraneous-dependencies */ +import { expect, fixture, html, waitUntil } from "@open-wc/testing"; +import sinon from "sinon"; +import "../index.js"; +import type { ECCClientElixirTrsToolCreate } from "../tool-create.js"; +import type { TrsFilerProvider } from "../../../providers/index.js"; + +const BASE_URL = "https://trs.example.org/ga4gh/trs/v2"; + +/** + * A provider whose tool class list is a stub; the form loads nothing else + * before a submit. + */ +const stubProvider = () => { + const stubs = { + getToolClasses: sinon + .stub() + .resolves([{ id: "workflow", name: "Workflow" }]), + }; + return { stubs, provider: stubs as unknown as TrsFilerProvider }; +}; + +describe("ecc-client-elixir-trs-tool-create provider", () => { + let fetchStub: sinon.SinonStub; + + beforeEach(() => { + fetchStub = sinon.stub(window, "fetch").callsFake( + async () => + new Response(JSON.stringify([]), { + headers: { "Content-Type": "application/json" }, + }) + ); + }); + + afterEach(() => { + sinon.restore(); + }); + + it("loads from a provider set after the first update", async () => { + const el = await fixture( + html`` + ); + const { stubs, provider } = stubProvider(); + + el.provider = provider; + await waitUntil(() => stubs.getToolClasses.callCount === 1); + + expect(fetchStub.callCount).to.equal(0); + }); + + it("loads from a provider that replaces the first one", async () => { + const first = stubProvider(); + const second = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => first.stubs.getToolClasses.callCount === 1); + + el.provider = second.provider; + await waitUntil(() => second.stubs.getToolClasses.callCount === 1); + + expect(first.stubs.getToolClasses.callCount).to.equal(1); + }); + + it("keeps a provider when the base URL is set too", async () => { + const { stubs, provider } = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => stubs.getToolClasses.callCount === 1); + + el.baseUrl = BASE_URL; + await el.updateComplete; + + expect(stubs.getToolClasses.callCount).to.equal(1); + expect(fetchStub.callCount).to.equal(0); + }); +}); diff --git a/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts b/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts index 4cbd0499..3bf790b0 100644 --- a/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts +++ b/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts @@ -337,12 +337,32 @@ export class ECCClientElixirTrsToolCreate extends LitElement { this.initializeFormWithDefaults(); } - if (changedProperties.has("baseUrl") && this.baseUrl) { - this._provider = new RestTrsFilerProvider(this.baseUrl); + if ( + (changedProperties.has("provider") || changedProperties.has("baseUrl")) && + this.useProvider() + ) { this.loadToolClasses(); } } + /** + * Switches to the custom provider or, without one, to a REST provider for + * the base URL; returns whether the provider in use changed + */ + private useProvider(): boolean { + let provider: TrsFilerProvider | null = this.provider ?? null; + if (!provider && this.baseUrl) { + provider = + this._provider instanceof RestTrsFilerProvider && + this._provider.baseUrl === this.baseUrl + ? this._provider + : new RestTrsFilerProvider(this.baseUrl); + } + if (!provider || provider === this._provider) return false; + this._provider = provider; + return true; + } + private async loadToolClasses(): Promise { if (!this._provider) return; diff --git a/packages/ecc-client-ga4gh-drs/package.json b/packages/ecc-client-ga4gh-drs/package.json index e671a5b5..5cd30ea2 100644 --- a/packages/ecc-client-ga4gh-drs/package.json +++ b/packages/ecc-client-ga4gh-drs/package.json @@ -38,6 +38,7 @@ "build": "node ../../scripts/tailwind/index.js && node ../../scripts/build.js -p ecc-client-ga4gh-drs-", "dev": "concurrently -r \"node ../../scripts/tailwind/index.js --watch\" \"node ../../scripts/build.js -p ecc-client-ga4gh-drs- --watch\" \"wds\"", "clean": "rm -rf dist node_modules custom-elements-manifest.config.js src/output.css src/tailwind.ts", + "test": "web-test-runner --group default --config ../../web-test.runner-config.mjs --port 8006", "test:watch": "wtr --watch", "lint": "npx eslint .", "lint:fix": "npm run lint -- --fix", diff --git a/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts b/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts index c0fdd991..7f47c1fa 100644 --- a/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts +++ b/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts @@ -77,12 +77,32 @@ export class ECCClientGa4ghDrsObjects extends LitElement { this.loadData(); } - if (changedProperties.has("baseUrl") && this.baseUrl) { - this._provider = new RestDrsProvider(this.baseUrl); + if ( + (changedProperties.has("provider") || changedProperties.has("baseUrl")) && + this.useProvider() + ) { this.loadData(); } } + /** + * Switches to the custom provider or, without one, to a REST provider for + * the base URL; returns whether the provider in use changed + */ + private useProvider(): boolean { + let provider: DrsProvider | null = this.provider ?? null; + if (!provider && this.baseUrl) { + provider = + this._provider instanceof RestDrsProvider && + this._provider.baseUrl === this.baseUrl + ? this._provider + : new RestDrsProvider(this.baseUrl); + } + if (!provider || provider === this._provider) return false; + this._provider = provider; + return true; + } + private async loadData(): Promise { if (!this._provider) return; diff --git a/packages/ecc-client-ga4gh-drs/src/components/objects-list/tests/objects.test.ts b/packages/ecc-client-ga4gh-drs/src/components/objects-list/tests/objects.test.ts new file mode 100644 index 00000000..9c63e2d2 --- /dev/null +++ b/packages/ecc-client-ga4gh-drs/src/components/objects-list/tests/objects.test.ts @@ -0,0 +1,80 @@ +/* eslint-disable import/no-extraneous-dependencies */ +import { expect, fixture, html, waitUntil } from "@open-wc/testing"; +import sinon from "sinon"; +import "../index.js"; +import type { ECCClientGa4ghDrsObjects } from "../objects.js"; +import type { DrsProvider } from "../../../providers/drs-provider.js"; + +const BASE_URL = "https://drs.example.org/ga4gh/drs/v1"; + +/** + * A provider whose object list is a stub; the list uses no other method + * before an object is selected. + */ +const stubProvider = () => { + const stubs = { + getObjects: sinon.stub().resolves({ objects: [] }), + }; + return { stubs, provider: stubs as unknown as DrsProvider }; +}; + +describe("ecc-client-ga4gh-drs-objects provider", () => { + let fetchStub: sinon.SinonStub; + + beforeEach(() => { + fetchStub = sinon.stub(window, "fetch").callsFake( + async () => + new Response(JSON.stringify({ objects: [] }), { + headers: { "Content-Type": "application/json" }, + }) + ); + }); + + afterEach(() => { + sinon.restore(); + }); + + it("loads from a provider set after the first update", async () => { + const el = await fixture( + html`` + ); + const { stubs, provider } = stubProvider(); + + el.provider = provider; + await waitUntil(() => stubs.getObjects.callCount === 1); + + expect(fetchStub.callCount).to.equal(0); + }); + + it("loads from a provider that replaces the first one", async () => { + const first = stubProvider(); + const second = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => first.stubs.getObjects.callCount > 0); + const firstCalls = first.stubs.getObjects.callCount; + + el.provider = second.provider; + await waitUntil(() => second.stubs.getObjects.callCount === 1); + + expect(first.stubs.getObjects.callCount).to.equal(firstCalls); + }); + + it("keeps a provider when the base URL is set too", async () => { + const { stubs, provider } = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => stubs.getObjects.callCount > 0); + + el.baseUrl = BASE_URL; + await el.updateComplete; + + expect(fetchStub.callCount).to.equal(0); + }); +}); diff --git a/packages/ecc-client-ga4gh-service-registry/package.json b/packages/ecc-client-ga4gh-service-registry/package.json index 148976e2..137974e1 100644 --- a/packages/ecc-client-ga4gh-service-registry/package.json +++ b/packages/ecc-client-ga4gh-service-registry/package.json @@ -38,6 +38,7 @@ "build": "node ../../scripts/tailwind/index.js && node ../../scripts/build.js -p ecc-client-ga4gh-service-registry-", "dev": "concurrently -r \"node ../../scripts/tailwind/index.js --watch\" \"node ../../scripts/build.js -p ecc-client-ga4gh-service-registry- --watch\" \"wds\"", "clean": "rm -rf dist node_modules custom-elements-manifest.config.js src/output.css src/tailwind.ts", + "test": "web-test-runner --group default --config ../../web-test.runner-config.mjs --port 8003", "test:watch": "wtr --watch", "lint": "npx eslint .", "lint:fix": "npm run lint -- --fix", diff --git a/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts b/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts index f3b09aae..4041e348 100644 --- a/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts +++ b/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts @@ -74,17 +74,35 @@ export class ECCClientGa4ghServiceRegistryService extends LitElement { } protected updated(changedProperties: Map): void { + const providerChanged = + (changedProperties.has("provider") || changedProperties.has("baseUrl")) && + this.useProvider(); if ( - (changedProperties.has("baseUrl") && this.baseUrl) || - (changedProperties.has("serviceId") && this.serviceId) + (providerChanged || + (changedProperties.has("serviceId") && this.serviceId)) && + this.serviceId && + this._provider ) { - if (this.baseUrl) { - this._provider = new RestServiceRegistryProvider(this.baseUrl); - } - if (this.serviceId && this._provider) { - this.loadData(); - } + this.loadData(); + } + } + + /** + * Switches to the custom provider or, without one, to a REST provider for + * the base URL; returns whether the provider in use changed + */ + private useProvider(): boolean { + let provider: ServiceRegistryProvider | null = this.provider ?? null; + if (!provider && this.baseUrl) { + provider = + this._provider instanceof RestServiceRegistryProvider && + this._provider.baseUrl === this.baseUrl + ? this._provider + : new RestServiceRegistryProvider(this.baseUrl); } + if (!provider || provider === this._provider) return false; + this._provider = provider; + return true; } private async loadData(): Promise { diff --git a/packages/ecc-client-ga4gh-service-registry/src/components/service/tests/service.test.ts b/packages/ecc-client-ga4gh-service-registry/src/components/service/tests/service.test.ts new file mode 100644 index 00000000..32d5db22 --- /dev/null +++ b/packages/ecc-client-ga4gh-service-registry/src/components/service/tests/service.test.ts @@ -0,0 +1,108 @@ +/* eslint-disable import/no-extraneous-dependencies */ +import { expect, fixture, html, waitUntil } from "@open-wc/testing"; +import sinon from "sinon"; +import "../index.js"; +import type { ECCClientGa4ghServiceRegistryService } from "../service.js"; +import type { + ExternalService, + ServiceRegistryProvider, +} from "../../../providers/index.js"; + +const BASE_URL = "https://registry.example.org/ga4gh/registry/v1"; + +const SERVICE: ExternalService = { + id: "service-1", + name: "Service 1", + type: { group: "org.ga4gh", artifact: "tes", version: "1.1.0" }, + organization: { name: "Example", url: "https://example.org" }, + version: "1.0.0", + url: "https://tes.example.org", +}; + +/** A provider whose methods are stubs. */ +const stubProvider = () => + ({ + getServices: sinon.stub().resolves([SERVICE]), + getServiceById: sinon.stub().resolves(SERVICE), + getServiceTypes: sinon.stub().resolves([SERVICE.type]), + getServiceInfo: sinon.stub().resolves(SERVICE), + } satisfies ServiceRegistryProvider); + +describe("ecc-client-ga4gh-service-registry-service provider", () => { + let fetchStub: sinon.SinonStub; + + beforeEach(() => { + fetchStub = sinon.stub(window, "fetch").callsFake( + async () => + new Response(JSON.stringify(SERVICE), { + headers: { "Content-Type": "application/json" }, + }) + ); + }); + + afterEach(() => { + sinon.restore(); + }); + + it("loads from a provider set after the first update", async () => { + const el = await fixture( + html`` + ); + const provider = stubProvider(); + + el.provider = provider; + await waitUntil(() => provider.getServiceById.callCount === 1); + + expect(provider.getServiceById.firstCall.args[0]).to.equal("service-1"); + expect(fetchStub.callCount).to.equal(0); + }); + + it("loads from a provider that replaces the first one", async () => { + const first = stubProvider(); + const second = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => first.getServiceById.callCount > 0); + + el.provider = second; + await waitUntil(() => second.getServiceById.callCount === 1); + }); + + it("keeps a provider when the base URL is set too", async () => { + const provider = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => provider.getServiceById.callCount > 0); + + el.baseUrl = BASE_URL; + await el.updateComplete; + + expect(fetchStub.callCount).to.equal(0); + }); + + it("loads another service from the same provider", async () => { + const provider = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => provider.getServiceById.callCount > 0); + + el.serviceId = "service-2"; + await waitUntil( + () => provider.getServiceById.lastCall.args[0] === "service-2" + ); + }); +}); diff --git a/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts b/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts index 9918a1f3..a3e08880 100644 --- a/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts +++ b/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts @@ -78,13 +78,33 @@ export class ECCClientGa4ghServiceRegistryServices extends LitElement { } protected updated(changedProperties: Map): void { - if (changedProperties.has("baseUrl") && this.baseUrl) { - this._provider = new RestServiceRegistryProvider(this.baseUrl); + if ( + (changedProperties.has("provider") || changedProperties.has("baseUrl")) && + this.useProvider() + ) { this.loadData(); this.loadServiceTypes(); } } + /** + * Switches to the custom provider or, without one, to a REST provider for + * the base URL; returns whether the provider in use changed + */ + private useProvider(): boolean { + let provider: ServiceRegistryProvider | null = this.provider ?? null; + if (!provider && this.baseUrl) { + provider = + this._provider instanceof RestServiceRegistryProvider && + this._provider.baseUrl === this.baseUrl + ? this._provider + : new RestServiceRegistryProvider(this.baseUrl); + } + if (!provider || provider === this._provider) return false; + this._provider = provider; + return true; + } + private async loadServiceTypes(): Promise { if (!this._provider) return; diff --git a/packages/ecc-client-ga4gh-service-registry/src/components/services/tests/services.test.ts b/packages/ecc-client-ga4gh-service-registry/src/components/services/tests/services.test.ts new file mode 100644 index 00000000..bf179853 --- /dev/null +++ b/packages/ecc-client-ga4gh-service-registry/src/components/services/tests/services.test.ts @@ -0,0 +1,109 @@ +/* eslint-disable import/no-extraneous-dependencies */ +import { expect, fixture, html, waitUntil } from "@open-wc/testing"; +import sinon from "sinon"; +import "../index.js"; +import type { ECCClientGa4ghServiceRegistryServices } from "../services.js"; +import type { + ExternalService, + ServiceRegistryProvider, +} from "../../../providers/index.js"; + +const BASE_URL = "https://registry.example.org/ga4gh/registry/v1"; + +const SERVICE: ExternalService = { + id: "service-1", + name: "Service 1", + type: { group: "org.ga4gh", artifact: "tes", version: "1.1.0" }, + organization: { name: "Example", url: "https://example.org" }, + version: "1.0.0", + url: "https://tes.example.org", +}; + +/** A provider whose methods are stubs. */ +const stubProvider = () => + ({ + getServices: sinon.stub().resolves([SERVICE]), + getServiceById: sinon.stub().resolves(SERVICE), + getServiceTypes: sinon.stub().resolves([SERVICE.type]), + getServiceInfo: sinon.stub().resolves(SERVICE), + } satisfies ServiceRegistryProvider); + +describe("ecc-client-ga4gh-service-registry-services provider", () => { + let fetchStub: sinon.SinonStub; + + beforeEach(() => { + fetchStub = sinon.stub(window, "fetch").callsFake( + async () => + new Response(JSON.stringify([]), { + headers: { "Content-Type": "application/json" }, + }) + ); + }); + + afterEach(() => { + sinon.restore(); + }); + + it("loads from a provider set after the first update", async () => { + const el = await fixture( + html`` + ); + const provider = stubProvider(); + + el.provider = provider; + await waitUntil(() => provider.getServiceTypes.callCount === 1); + + expect(provider.getServices.callCount).to.equal(1); + expect(fetchStub.callCount).to.equal(0); + }); + + it("loads from a provider that replaces the first one", async () => { + const first = stubProvider(); + const second = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => first.getServiceTypes.callCount === 1); + + el.provider = second; + await waitUntil(() => second.getServiceTypes.callCount === 1); + + expect(second.getServices.callCount).to.equal(1); + expect(first.getServices.callCount).to.equal(1); + }); + + it("keeps a provider when the base URL is set too", async () => { + const provider = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => provider.getServiceTypes.callCount === 1); + + el.baseUrl = BASE_URL; + await el.updateComplete; + + expect(provider.getServices.callCount).to.equal(1); + expect(fetchStub.callCount).to.equal(0); + }); + + it("uses the base URL once the provider is removed", async () => { + const provider = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => provider.getServiceTypes.callCount === 1); + + el.provider = undefined; + await waitUntil(() => fetchStub.callCount === 2); + + expect(fetchStub.firstCall.args[0]).to.equal(`${BASE_URL}/services`); + expect(provider.getServices.callCount).to.equal(1); + }); +}); diff --git a/packages/ecc-client-ga4gh-trs/package.json b/packages/ecc-client-ga4gh-trs/package.json index 3548ce1d..1dab4bb7 100644 --- a/packages/ecc-client-ga4gh-trs/package.json +++ b/packages/ecc-client-ga4gh-trs/package.json @@ -38,6 +38,7 @@ "build": "node ../../scripts/tailwind/index.js && node ../../scripts/build.js -p ecc-client-ga4gh-trs-", "dev": "concurrently -r \"node ../../scripts/tailwind/index.js --watch\" \"node ../../scripts/build.js -p ecc-client-ga4gh-trs- --watch\" \"wds\"", "clean": "rm -rf dist node_modules custom-elements-manifest.config.js src/output.css src/tailwind.ts", + "test": "web-test-runner --group default --config ../../web-test.runner-config.mjs --port 8004", "test:watch": "wtr --watch", "lint": "npx eslint .", "lint:fix": "npm run lint -- --fix", diff --git a/packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts b/packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts new file mode 100644 index 00000000..741b3984 --- /dev/null +++ b/packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts @@ -0,0 +1,85 @@ +/* eslint-disable import/no-extraneous-dependencies */ +import { expect, fixture, html, waitUntil } from "@open-wc/testing"; +import sinon from "sinon"; +import "../index.js"; +import type { ECCClientGa4ghTrsTools } from "../tools.js"; +import type { ToolClass, TrsProvider } from "../../../providers/index.js"; + +const BASE_URL = "https://trs.example.org/ga4gh/trs/v2"; + +const TOOL_CLASS: ToolClass = { id: "workflow", name: "Workflow" }; + +/** + * A provider whose list methods are stubs; the tools list uses no other + * methods. + */ +const stubProvider = () => { + const stubs = { + getToolClasses: sinon.stub().resolves([TOOL_CLASS]), + getToolsList: sinon.stub().resolves([]), + }; + return { stubs, provider: stubs as unknown as TrsProvider }; +}; + +describe("ecc-client-ga4gh-trs-tools provider", () => { + let fetchStub: sinon.SinonStub; + + beforeEach(() => { + fetchStub = sinon.stub(window, "fetch").callsFake( + async () => + new Response(JSON.stringify([]), { + headers: { "Content-Type": "application/json" }, + }) + ); + }); + + afterEach(() => { + sinon.restore(); + }); + + it("loads from a provider set after the first update", async () => { + const el = await fixture( + html`` + ); + const { stubs, provider } = stubProvider(); + + el.provider = provider; + await waitUntil(() => stubs.getToolClasses.callCount === 1); + + expect(stubs.getToolsList.callCount).to.equal(1); + expect(fetchStub.callCount).to.equal(0); + }); + + it("loads from a provider that replaces the first one", async () => { + const first = stubProvider(); + const second = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => first.stubs.getToolClasses.callCount === 1); + + el.provider = second.provider; + await waitUntil(() => second.stubs.getToolClasses.callCount === 1); + + expect(second.stubs.getToolsList.callCount).to.equal(1); + expect(first.stubs.getToolClasses.callCount).to.equal(1); + }); + + it("keeps a provider when the base URL is set too", async () => { + const { stubs, provider } = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => stubs.getToolClasses.callCount === 1); + + el.baseUrl = BASE_URL; + await el.updateComplete; + + expect(stubs.getToolClasses.callCount).to.equal(1); + expect(fetchStub.callCount).to.equal(0); + }); +}); diff --git a/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts b/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts index 82db5984..48b34020 100644 --- a/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts +++ b/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts @@ -103,13 +103,33 @@ export class ECCClientGa4ghTrsTools extends LitElement { this.loadData(); } - if (changedProperties.has("baseUrl") && this.baseUrl) { - this._provider = new RestTrsProvider(this.baseUrl); + if ( + (changedProperties.has("provider") || changedProperties.has("baseUrl")) && + this.useProvider() + ) { this.loadData(); this.loadToolClasses(); } } + /** + * Switches to the custom provider or, without one, to a REST provider for + * the base URL; returns whether the provider in use changed + */ + private useProvider(): boolean { + let provider: TrsProvider | null = this.provider ?? null; + if (!provider && this.baseUrl) { + provider = + this._provider instanceof RestTrsProvider && + this._provider.baseUrl === this.baseUrl + ? this._provider + : new RestTrsProvider(this.baseUrl); + } + if (!provider || provider === this._provider) return false; + this._provider = provider; + return true; + } + private async loadToolClasses(): Promise { if (!this._provider) return; diff --git a/web-test.runner-config.mjs b/web-test.runner-config.mjs index 99f70fae..8e14a13b 100644 --- a/web-test.runner-config.mjs +++ b/web-test.runner-config.mjs @@ -3,7 +3,8 @@ import fg from "fast-glob"; import { playwrightLauncher } from "@web/test-runner-playwright"; // Packages run tests in parallel (turbo --parallel), so each package's test -// scripts pass a unique --port: design 8000, wes 8001. +// scripts pass a unique --port: design 8000, wes 8001, cloud registry 8002, +// service registry 8003, trs 8004, trs-filer 8005, drs 8006. export default { rootDir: ".", files: "src/**/*.test.ts", // "default" group From 4ec93f6d77a242907a3f63e433ce024eaa7c1ae5 Mon Sep 17 00:00:00 2001 From: Alex Kanitz Date: Fri, 13 Nov 2026 08:30:41 +0100 Subject: [PATCH 2/8] docs: describe setting providers after rendering Drop the comments of the live previews that explained why they pass `baseUrl` instead of `provider`, as the components now react to both. Co-Authored-By: Claude Opus 5.5 --- .../packages/service-registry/ServicePreview.tsx | 3 --- .../packages/service-registry/ServicesPreview.tsx | 3 --- .../components/packages/trs-filer/ToolCreatePreview.tsx | 3 --- .../documentation/components/packages/trs/ToolsPreview.tsx | 3 --- apps/documentation/pages/docs/providers.mdx | 7 +++++++ 5 files changed, 7 insertions(+), 12 deletions(-) diff --git a/apps/documentation/components/packages/service-registry/ServicePreview.tsx b/apps/documentation/components/packages/service-registry/ServicePreview.tsx index bb40e0d3..cca9ba1c 100644 --- a/apps/documentation/components/packages/service-registry/ServicePreview.tsx +++ b/apps/documentation/components/packages/service-registry/ServicePreview.tsx @@ -8,9 +8,6 @@ export default function ServicePreview() { console.log('Service data updated:', event.detail.service); }; - // `baseUrl` rather than `provider`: the component reads `provider` only on - // its first update, which runs before React sets it on the server-rendered - // element, but it reloads when `baseUrl` changes return ( ; } - // `baseUrl` rather than `provider`: the component reads `provider` only on - // its first update, which runs before React sets it on the server-rendered - // element, but it reloads when `baseUrl` changes return ( diff --git a/apps/documentation/components/packages/trs/ToolsPreview.tsx b/apps/documentation/components/packages/trs/ToolsPreview.tsx index 0f4c7cb2..08ba2818 100644 --- a/apps/documentation/components/packages/trs/ToolsPreview.tsx +++ b/apps/documentation/components/packages/trs/ToolsPreview.tsx @@ -4,9 +4,6 @@ import PreviewLayout from '@/components/common/PreviewLayout'; import { endpoints } from '@/utils/endpoints'; export default function ToolsPreview() { - // `baseUrl` rather than `provider`: the component reads `provider` only on - // its first update, which runs before React sets it on the server-rendered - // element, but it reloads when `baseUrl` changes return ( diff --git a/apps/documentation/pages/docs/providers.mdx b/apps/documentation/pages/docs/providers.mdx index e15d4442..a0026f9c 100644 --- a/apps/documentation/pages/docs/providers.mdx +++ b/apps/documentation/pages/docs/providers.mdx @@ -43,6 +43,13 @@ const provider = new RestProvider('https://api.example.com'); // 4. Handle events/states as needed ``` +You can set or replace a component's provider at any time, also after the +component has rendered (as with React, which sets it on a server-rendered +element only after the first update): the component then loads its data from +the new provider. Create the provider once and pass the same instance on every +render (e.g. with `useMemo` in React), as each new instance makes the component +load its data again. + ## Common Use Cases ### Environment-Based Configuration From 7d90fd1c9a28c127ecda0dc94fea7f333ea00b07 Mon Sep 17 00:00:00 2001 From: Alex Kanitz Date: Fri, 13 Nov 2026 09:51:30 +0100 Subject: [PATCH 3/8] fix: drop stale loads after a provider switch A load from a provider that was replaced before it finished no longer overwrites the data, error or loading state of the new provider. The TRS tools and DRS objects lists start on their first page with a new provider. Removing the provider without a base URL clears the data and shows the missing-provider error (the TRS-Filer form clears its tool classes) instead of keeping the removed provider. Tests cover the base URL path, slow loads from a replaced provider and the removal of the provider. Co-Authored-By: Claude Opus 5.5 --- apps/documentation/pages/docs/providers.mdx | 3 +- .../tool-create/tests/tool-create.test.ts | 84 ++++++++++-- .../src/components/tool-create/tool-create.ts | 19 ++- .../src/components/objects-list/objects.ts | 26 +++- .../objects-list/tests/objects.test.ts | 107 +++++++++++++-- .../src/components/service/service.ts | 25 +++- .../components/service/tests/service.test.ts | 129 ++++++++++++++---- .../src/components/services/services.ts | 34 +++-- .../services/tests/services.test.ts | 106 +++++++++++--- .../src/components/tools/tests/tools.test.ts | 107 +++++++++++++-- .../src/components/tools/tools.ts | 37 +++-- 11 files changed, 575 insertions(+), 102 deletions(-) diff --git a/apps/documentation/pages/docs/providers.mdx b/apps/documentation/pages/docs/providers.mdx index a0026f9c..3758da4f 100644 --- a/apps/documentation/pages/docs/providers.mdx +++ b/apps/documentation/pages/docs/providers.mdx @@ -46,7 +46,8 @@ const provider = new RestProvider('https://api.example.com'); You can set or replace a component's provider at any time, also after the component has rendered (as with React, which sets it on a server-rendered element only after the first update): the component then loads its data from -the new provider. Create the provider once and pass the same instance on every +the new provider, and a load that the previous provider has not finished yet +is dropped. Create the provider once and pass the same instance on every render (e.g. with `useMemo` in React), as each new instance makes the component load its data again. diff --git a/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tests/tool-create.test.ts b/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tests/tool-create.test.ts index 3c837f43..c1f8d02e 100644 --- a/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tests/tool-create.test.ts +++ b/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tests/tool-create.test.ts @@ -6,20 +6,38 @@ import type { ECCClientElixirTrsToolCreate } from "../tool-create.js"; import type { TrsFilerProvider } from "../../../providers/index.js"; const BASE_URL = "https://trs.example.org/ga4gh/trs/v2"; +const OTHER_URL = "https://other.example.org/ga4gh/trs/v2"; + +type ToolClass = { id: string; name: string }; /** * A provider whose tool class list is a stub; the form loads nothing else * before a submit. */ -const stubProvider = () => { +const stubProvider = ( + classes: ToolClass[] | Promise = [ + { id: "workflow", name: "Workflow" }, + ] +) => { const stubs = { - getToolClasses: sinon - .stub() - .resolves([{ id: "workflow", name: "Workflow" }]), + getToolClasses: sinon.stub().callsFake(async () => classes), }; return { stubs, provider: stubs as unknown as TrsFilerProvider }; }; +/** A promise and the function that resolves it. */ +const deferred = () => { + let settleWith: (value: T) => void = () => undefined; + const promise = new Promise((resolve) => { + settleWith = resolve; + }); + return { promise, resolve: settleWith }; +}; + +/** The tool classes the form offers. */ +const offered = (el: ECCClientElixirTrsToolCreate) => + (el as unknown as { toolClasses: ToolClass[] }).toolClasses; + describe("ecc-client-elixir-trs-tool-create provider", () => { let fetchStub: sinon.SinonStub; @@ -43,27 +61,47 @@ describe("ecc-client-elixir-trs-tool-create provider", () => { const { stubs, provider } = stubProvider(); el.provider = provider; - await waitUntil(() => stubs.getToolClasses.callCount === 1); + await waitUntil(() => offered(el)[0]?.id === "workflow"); + expect(stubs.getToolClasses.callCount).to.equal(1); expect(fetchStub.callCount).to.equal(0); }); it("loads from a provider that replaces the first one", async () => { const first = stubProvider(); - const second = stubProvider(); + const second = stubProvider([{ id: "tool", name: "Tool" }]); const el = await fixture( html`` ); - await waitUntil(() => first.stubs.getToolClasses.callCount === 1); + await waitUntil(() => offered(el)[0]?.id === "workflow"); el.provider = second.provider; - await waitUntil(() => second.stubs.getToolClasses.callCount === 1); + await waitUntil(() => offered(el)[0]?.id === "tool"); expect(first.stubs.getToolClasses.callCount).to.equal(1); }); + it("drops a slower load from a replaced provider", async () => { + const slow = deferred(); + const first = stubProvider(slow.promise); + const second = stubProvider([{ id: "tool", name: "Tool" }]); + const el = await fixture( + html`` + ); + + el.provider = second.provider; + await waitUntil(() => offered(el)[0]?.id === "tool"); + slow.resolve([{ id: "workflow", name: "Workflow" }]); + await slow.promise; + await el.updateComplete; + + expect(offered(el)[0].id).to.equal("tool"); + }); + it("keeps a provider when the base URL is set too", async () => { const { stubs, provider } = stubProvider(); const el = await fixture( @@ -79,4 +117,34 @@ describe("ecc-client-elixir-trs-tool-create provider", () => { expect(stubs.getToolClasses.callCount).to.equal(1); expect(fetchStub.callCount).to.equal(0); }); + + it("loads from the base URL and reloads when it changes", async () => { + const el = await fixture( + html`` + ); + await waitUntil(() => fetchStub.callCount === 1); + + el.baseUrl = OTHER_URL; + await waitUntil(() => fetchStub.callCount === 2); + + expect(fetchStub.firstCall.args[0]).to.equal(`${BASE_URL}/toolClasses`); + expect(fetchStub.secondCall.args[0]).to.equal(`${OTHER_URL}/toolClasses`); + }); + + it("offers no tool classes once the provider is removed without a base URL", async () => { + const { provider } = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => offered(el).length === 1); + + el.provider = undefined; + await el.updateComplete; + + expect(offered(el)).to.deep.equal([]); + }); }); diff --git a/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts b/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts index 3bf790b0..ed6ce048 100644 --- a/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts +++ b/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts @@ -341,13 +341,18 @@ export class ECCClientElixirTrsToolCreate extends LitElement { (changedProperties.has("provider") || changedProperties.has("baseUrl")) && this.useProvider() ) { - this.loadToolClasses(); + if (this._provider) { + this.loadToolClasses(); + } else { + this.toolClasses = []; + } } } /** * Switches to the custom provider or, without one, to a REST provider for - * the base URL; returns whether the provider in use changed + * the base URL (to none without either); returns whether the provider in + * use changed */ private useProvider(): boolean { let provider: TrsFilerProvider | null = this.provider ?? null; @@ -358,16 +363,20 @@ export class ECCClientElixirTrsToolCreate extends LitElement { ? this._provider : new RestTrsFilerProvider(this.baseUrl); } - if (!provider || provider === this._provider) return false; + if (provider === this._provider) return false; this._provider = provider; return true; } private async loadToolClasses(): Promise { - if (!this._provider) return; + const provider = this._provider; + if (!provider) return; try { - this.toolClasses = await this._provider.getToolClasses(); + const toolClasses = await provider.getToolClasses(); + // a load from a provider that was replaced in the meantime is dropped + if (provider !== this._provider) return; + this.toolClasses = toolClasses; } catch (error) { console.error("Failed to load tool classes:", error); this.dispatchEvent( diff --git a/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts b/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts index 7f47c1fa..02718c55 100644 --- a/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts +++ b/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts @@ -81,13 +81,23 @@ export class ECCClientGa4ghDrsObjects extends LitElement { (changedProperties.has("provider") || changedProperties.has("baseUrl")) && this.useProvider() ) { - this.loadData(); + // another provider starts on its first page + this.currentPage = 1; + this.totalPages = 0; + if (this._provider) { + this.loadData(); + } else { + this.objects = []; + this.error = + "Please provide either a base URL for the DRS API or a custom provider."; + } } } /** * Switches to the custom provider or, without one, to a REST provider for - * the base URL; returns whether the provider in use changed + * the base URL (to none without either); returns whether the provider in + * use changed */ private useProvider(): boolean { let provider: DrsProvider | null = this.provider ?? null; @@ -98,23 +108,26 @@ export class ECCClientGa4ghDrsObjects extends LitElement { ? this._provider : new RestDrsProvider(this.baseUrl); } - if (!provider || provider === this._provider) return false; + if (provider === this._provider) return false; this._provider = provider; return true; } private async loadData(): Promise { - if (!this._provider) return; + const provider = this._provider; + if (!provider) return; this.loading = true; this.error = null; try { // API treats offset as page number, not actual offset - const result = await this._provider.getObjects( + const result = await provider.getObjects( this.pageSize, this.currentPage - 1 ); + // a load from a provider that was replaced in the meantime is dropped + if (provider !== this._provider) return; this.objects = result.objects; // Update total objects and pages from API response @@ -148,10 +161,11 @@ export class ECCClientGa4ghDrsObjects extends LitElement { }) ); } catch (err) { + if (provider !== this._provider) return; this.error = err instanceof Error ? err.message : "Failed to load objects"; } finally { - this.loading = false; + if (provider === this._provider) this.loading = false; } } diff --git a/packages/ecc-client-ga4gh-drs/src/components/objects-list/tests/objects.test.ts b/packages/ecc-client-ga4gh-drs/src/components/objects-list/tests/objects.test.ts index 9c63e2d2..03c9b264 100644 --- a/packages/ecc-client-ga4gh-drs/src/components/objects-list/tests/objects.test.ts +++ b/packages/ecc-client-ga4gh-drs/src/components/objects-list/tests/objects.test.ts @@ -3,21 +3,45 @@ import { expect, fixture, html, waitUntil } from "@open-wc/testing"; import sinon from "sinon"; import "../index.js"; import type { ECCClientGa4ghDrsObjects } from "../objects.js"; -import type { DrsProvider } from "../../../providers/drs-provider.js"; +import type { + DrsObject, + DrsProvider, +} from "../../../providers/drs-provider.js"; const BASE_URL = "https://drs.example.org/ga4gh/drs/v1"; +const OTHER_URL = "https://other.example.org/ga4gh/drs/v1"; +const MISSING = "Please provide either a base URL"; + +const object = (id: string) => ({ id } as unknown as DrsObject); + +type Page = { objects: DrsObject[] }; /** * A provider whose object list is a stub; the list uses no other method * before an object is selected. */ -const stubProvider = () => { +const stubProvider = ( + page: Page | Promise = { objects: [object("a")] } +) => { const stubs = { - getObjects: sinon.stub().resolves({ objects: [] }), + getObjects: sinon.stub().callsFake(async () => page), }; return { stubs, provider: stubs as unknown as DrsProvider }; }; +/** A promise and the function that resolves it. */ +const deferred = () => { + let settleWith: (value: T) => void = () => undefined; + const promise = new Promise((resolve) => { + settleWith = resolve; + }); + return { promise, resolve: settleWith }; +}; + +/** The component's private state that the tests read and set. */ +const internals = (el: ECCClientGa4ghDrsObjects) => + el as unknown as { objects: DrsObject[]; currentPage: number }; + describe("ecc-client-ga4gh-drs-objects provider", () => { let fetchStub: sinon.SinonStub; @@ -38,31 +62,57 @@ describe("ecc-client-ga4gh-drs-objects provider", () => { const el = await fixture( html`` ); + expect(el.shadowRoot?.textContent).to.contain(MISSING); const { stubs, provider } = stubProvider(); el.provider = provider; - await waitUntil(() => stubs.getObjects.callCount === 1); + await waitUntil(() => internals(el).objects.length === 1); + await el.updateComplete; + expect(stubs.getObjects.callCount).to.equal(1); + expect(el.shadowRoot?.textContent).not.to.contain(MISSING); expect(fetchStub.callCount).to.equal(0); }); - it("loads from a provider that replaces the first one", async () => { + it("loads the first page from a provider that replaces the first one", async () => { const first = stubProvider(); - const second = stubProvider(); + const second = stubProvider({ objects: [object("b")] }); const el = await fixture( html`` ); - await waitUntil(() => first.stubs.getObjects.callCount > 0); + await waitUntil(() => internals(el).objects[0]?.id === "a"); + internals(el).currentPage = 3; const firstCalls = first.stubs.getObjects.callCount; el.provider = second.provider; - await waitUntil(() => second.stubs.getObjects.callCount === 1); + await waitUntil(() => internals(el).objects[0]?.id === "b"); + expect(internals(el).currentPage).to.equal(1); + expect(second.stubs.getObjects.firstCall.args[1]).to.equal(0); expect(first.stubs.getObjects.callCount).to.equal(firstCalls); }); + it("drops a slower load from a replaced provider", async () => { + const slow = deferred(); + const first = stubProvider(slow.promise); + const second = stubProvider({ objects: [object("b")] }); + const el = await fixture( + html`` + ); + + el.provider = second.provider; + await waitUntil(() => internals(el).objects[0]?.id === "b"); + slow.resolve({ objects: [object("a")] }); + await slow.promise; + await el.updateComplete; + + expect(internals(el).objects[0].id).to.equal("b"); + }); + it("keeps a provider when the base URL is set too", async () => { const { stubs, provider } = stubProvider(); const el = await fixture( @@ -70,11 +120,50 @@ describe("ecc-client-ga4gh-drs-objects provider", () => { .provider=${provider} >` ); - await waitUntil(() => stubs.getObjects.callCount > 0); + await waitUntil(() => internals(el).objects.length === 1); + const calls = stubs.getObjects.callCount; el.baseUrl = BASE_URL; await el.updateComplete; + expect(stubs.getObjects.callCount).to.equal(calls); expect(fetchStub.callCount).to.equal(0); }); + + it("loads from the base URL and reloads when it changes", async () => { + const el = await fixture( + html`` + ); + await waitUntil(() => + fetchStub + .getCalls() + .some((call) => String(call.args[0]).startsWith(`${BASE_URL}/objects`)) + ); + + el.baseUrl = OTHER_URL; + await waitUntil(() => + fetchStub + .getCalls() + .some((call) => String(call.args[0]).startsWith(`${OTHER_URL}/objects`)) + ); + }); + + it("shows an error once the provider is removed without a base URL", async () => { + const { provider } = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => internals(el).objects.length === 1); + + el.provider = undefined; + await el.updateComplete; + await el.updateComplete; + + expect(internals(el).objects).to.deep.equal([]); + expect(el.shadowRoot?.textContent).to.contain(MISSING); + }); }); diff --git a/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts b/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts index 4041e348..1464646f 100644 --- a/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts +++ b/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts @@ -77,7 +77,11 @@ export class ECCClientGa4ghServiceRegistryService extends LitElement { const providerChanged = (changedProperties.has("provider") || changedProperties.has("baseUrl")) && this.useProvider(); - if ( + if (providerChanged && !this._provider) { + this.service = null; + this.error = + "Please provide either a base URL for the Service Registry API or a custom provider."; + } else if ( (providerChanged || (changedProperties.has("serviceId") && this.serviceId)) && this.serviceId && @@ -89,7 +93,8 @@ export class ECCClientGa4ghServiceRegistryService extends LitElement { /** * Switches to the custom provider or, without one, to a REST provider for - * the base URL; returns whether the provider in use changed + * the base URL (to none without either); returns whether the provider in + * use changed */ private useProvider(): boolean { let provider: ServiceRegistryProvider | null = this.provider ?? null; @@ -100,19 +105,26 @@ export class ECCClientGa4ghServiceRegistryService extends LitElement { ? this._provider : new RestServiceRegistryProvider(this.baseUrl); } - if (!provider || provider === this._provider) return false; + if (provider === this._provider) return false; this._provider = provider; return true; } private async loadData(): Promise { - if (!this._provider || !this.serviceId) return; + const provider = this._provider; + const { serviceId } = this; + if (!provider || !serviceId) return; + // a load for a provider or service that was replaced in the meantime is + // dropped + const replaced = () => + provider !== this._provider || serviceId !== this.serviceId; this.loading = true; this.error = null; try { - const service = await this._provider.getServiceById(this.serviceId); + const service = await provider.getServiceById(serviceId); + if (replaced()) return; this.service = service; // Emit an event with the updated service data @@ -124,6 +136,7 @@ export class ECCClientGa4ghServiceRegistryService extends LitElement { }) ); } catch (err) { + if (replaced()) return; this.error = err instanceof Error ? err.message : "Failed to load service"; console.error({ @@ -131,7 +144,7 @@ export class ECCClientGa4ghServiceRegistryService extends LitElement { breakPoint: "ECCClientGa4ghServiceRegistryService.loadData", }); } finally { - this.loading = false; + if (!replaced()) this.loading = false; } } diff --git a/packages/ecc-client-ga4gh-service-registry/src/components/service/tests/service.test.ts b/packages/ecc-client-ga4gh-service-registry/src/components/service/tests/service.test.ts index 32d5db22..2c20424b 100644 --- a/packages/ecc-client-ga4gh-service-registry/src/components/service/tests/service.test.ts +++ b/packages/ecc-client-ga4gh-service-registry/src/components/service/tests/service.test.ts @@ -9,32 +9,52 @@ import type { } from "../../../providers/index.js"; const BASE_URL = "https://registry.example.org/ga4gh/registry/v1"; +const OTHER_URL = "https://other.example.org/ga4gh/registry/v1"; +const MISSING = "Please provide either a base URL"; -const SERVICE: ExternalService = { - id: "service-1", - name: "Service 1", +const service = (id: string): ExternalService => ({ + id, + name: `Service ${id}`, type: { group: "org.ga4gh", artifact: "tes", version: "1.1.0" }, organization: { name: "Example", url: "https://example.org" }, version: "1.0.0", url: "https://tes.example.org", -}; +}); -/** A provider whose methods are stubs. */ -const stubProvider = () => +/** + * A provider whose methods are stubs; `getServiceById` returns the given + * service, or one with the requested ID. + */ +const stubProvider = (result?: Promise) => ({ - getServices: sinon.stub().resolves([SERVICE]), - getServiceById: sinon.stub().resolves(SERVICE), - getServiceTypes: sinon.stub().resolves([SERVICE.type]), - getServiceInfo: sinon.stub().resolves(SERVICE), + getServices: sinon.stub().rejects(new Error("not used")), + getServiceById: sinon + .stub() + .callsFake(async (id: string) => result ?? service(id)), + getServiceTypes: sinon.stub().rejects(new Error("not used")), + getServiceInfo: sinon.stub().rejects(new Error("not used")), } satisfies ServiceRegistryProvider); +/** A promise and the function that resolves it. */ +const deferred = () => { + let settleWith: (value: T) => void = () => undefined; + const promise = new Promise((resolve) => { + settleWith = resolve; + }); + return { promise, resolve: settleWith }; +}; + +/** The service the component shows. */ +const shown = (el: ECCClientGa4ghServiceRegistryService) => + (el as unknown as { service: ExternalService | null }).service; + describe("ecc-client-ga4gh-service-registry-service provider", () => { let fetchStub: sinon.SinonStub; beforeEach(() => { fetchStub = sinon.stub(window, "fetch").callsFake( async () => - new Response(JSON.stringify(SERVICE), { + new Response(JSON.stringify(service("a")), { headers: { "Content-Type": "application/json" }, }) ); @@ -47,15 +67,18 @@ describe("ecc-client-ga4gh-service-registry-service provider", () => { it("loads from a provider set after the first update", async () => { const el = await fixture( html`` ); + expect(el.shadowRoot?.textContent).to.contain(MISSING); const provider = stubProvider(); el.provider = provider; - await waitUntil(() => provider.getServiceById.callCount === 1); + await waitUntil(() => shown(el)?.id === "a"); + await el.updateComplete; - expect(provider.getServiceById.firstCall.args[0]).to.equal("service-1"); + expect(provider.getServiceById.callCount).to.equal(1); + expect(el.shadowRoot?.textContent).not.to.contain(MISSING); expect(fetchStub.callCount).to.equal(0); }); @@ -64,45 +87,105 @@ describe("ecc-client-ga4gh-service-registry-service provider", () => { const second = stubProvider(); const el = await fixture( html`` ); - await waitUntil(() => first.getServiceById.callCount > 0); + await waitUntil(() => shown(el)?.id === "a"); + const firstCalls = first.getServiceById.callCount; el.provider = second; await waitUntil(() => second.getServiceById.callCount === 1); + + expect(first.getServiceById.callCount).to.equal(firstCalls); + }); + + it("drops a slower load from a replaced provider", async () => { + const slow = deferred(); + const first = stubProvider(slow.promise); + const second = stubProvider(); + const el = await fixture( + html`` + ); + + el.provider = second; + await waitUntil(() => shown(el)?.id === "b"); + slow.resolve(service("stale")); + await slow.promise; + await el.updateComplete; + + expect(shown(el)?.id).to.equal("b"); }); it("keeps a provider when the base URL is set too", async () => { const provider = stubProvider(); const el = await fixture( html`` ); - await waitUntil(() => provider.getServiceById.callCount > 0); + await waitUntil(() => shown(el)?.id === "a"); + const calls = provider.getServiceById.callCount; el.baseUrl = BASE_URL; await el.updateComplete; + expect(provider.getServiceById.callCount).to.equal(calls); expect(fetchStub.callCount).to.equal(0); }); + it("loads from the base URL and reloads when it changes", async () => { + const el = await fixture( + html`` + ); + await waitUntil(() => fetchStub.callCount > 0); + + el.baseUrl = OTHER_URL; + await waitUntil(() => + fetchStub + .getCalls() + .some((call) => call.args[0] === `${OTHER_URL}/services/a`) + ); + + expect(fetchStub.firstCall.args[0]).to.equal(`${BASE_URL}/services/a`); + }); + it("loads another service from the same provider", async () => { const provider = stubProvider(); const el = await fixture( html`` ); - await waitUntil(() => provider.getServiceById.callCount > 0); + await waitUntil(() => shown(el)?.id === "a"); + + el.serviceId = "b"; + await waitUntil(() => shown(el)?.id === "b"); + }); - el.serviceId = "service-2"; - await waitUntil( - () => provider.getServiceById.lastCall.args[0] === "service-2" + it("shows an error once the provider is removed without a base URL", async () => { + const provider = stubProvider(); + const el = await fixture( + html`` ); + await waitUntil(() => shown(el)?.id === "a"); + + el.provider = undefined; + await el.updateComplete; + await el.updateComplete; + + expect(shown(el)).to.equal(null); + expect(el.shadowRoot?.textContent).to.contain(MISSING); }); }); diff --git a/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts b/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts index a3e08880..1af54750 100644 --- a/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts +++ b/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts @@ -82,14 +82,22 @@ export class ECCClientGa4ghServiceRegistryServices extends LitElement { (changedProperties.has("provider") || changedProperties.has("baseUrl")) && this.useProvider() ) { - this.loadData(); - this.loadServiceTypes(); + if (this._provider) { + this.loadData(); + this.loadServiceTypes(); + } else { + this.services = []; + this.serviceTypes = []; + this.error = + "Please provide either a base URL for the Service Registry API or a custom provider."; + } } } /** * Switches to the custom provider or, without one, to a REST provider for - * the base URL; returns whether the provider in use changed + * the base URL (to none without either); returns whether the provider in + * use changed */ private useProvider(): boolean { let provider: ServiceRegistryProvider | null = this.provider ?? null; @@ -100,29 +108,36 @@ export class ECCClientGa4ghServiceRegistryServices extends LitElement { ? this._provider : new RestServiceRegistryProvider(this.baseUrl); } - if (!provider || provider === this._provider) return false; + if (provider === this._provider) return false; this._provider = provider; return true; } private async loadServiceTypes(): Promise { - if (!this._provider) return; + const provider = this._provider; + if (!provider) return; try { - this.serviceTypes = await this._provider.getServiceTypes(); + const serviceTypes = await provider.getServiceTypes(); + // a load from a provider that was replaced in the meantime is dropped + if (provider !== this._provider) return; + this.serviceTypes = serviceTypes; } catch (error) { console.error("Failed to load service types:", error); } } private async loadData(): Promise { - if (!this._provider) return; + const provider = this._provider; + if (!provider) return; this.loading = true; this.error = null; try { - const services = await this._provider.getServices(); + const services = await provider.getServices(); + // a load from a provider that was replaced in the meantime is dropped + if (provider !== this._provider) return; this.services = services; // Emit an event with the updated services @@ -134,6 +149,7 @@ export class ECCClientGa4ghServiceRegistryServices extends LitElement { }) ); } catch (err) { + if (provider !== this._provider) return; this.error = err instanceof Error ? err.message : "Failed to load services"; console.error({ @@ -141,7 +157,7 @@ export class ECCClientGa4ghServiceRegistryServices extends LitElement { breakPoint: "ECCClientGa4ghServiceRegistryServices.loadData", }); } finally { - this.loading = false; + if (provider === this._provider) this.loading = false; } } diff --git a/packages/ecc-client-ga4gh-service-registry/src/components/services/tests/services.test.ts b/packages/ecc-client-ga4gh-service-registry/src/components/services/tests/services.test.ts index bf179853..f6745fff 100644 --- a/packages/ecc-client-ga4gh-service-registry/src/components/services/tests/services.test.ts +++ b/packages/ecc-client-ga4gh-service-registry/src/components/services/tests/services.test.ts @@ -9,25 +9,42 @@ import type { } from "../../../providers/index.js"; const BASE_URL = "https://registry.example.org/ga4gh/registry/v1"; +const OTHER_URL = "https://other.example.org/ga4gh/registry/v1"; +const MISSING = "Please provide either a base URL"; -const SERVICE: ExternalService = { - id: "service-1", - name: "Service 1", +const service = (id: string): ExternalService => ({ + id, + name: `Service ${id}`, type: { group: "org.ga4gh", artifact: "tes", version: "1.1.0" }, organization: { name: "Example", url: "https://example.org" }, version: "1.0.0", url: "https://tes.example.org", -}; +}); /** A provider whose methods are stubs. */ -const stubProvider = () => +const stubProvider = ( + services: ExternalService[] | Promise = [service("a")] +) => ({ - getServices: sinon.stub().resolves([SERVICE]), - getServiceById: sinon.stub().resolves(SERVICE), - getServiceTypes: sinon.stub().resolves([SERVICE.type]), - getServiceInfo: sinon.stub().resolves(SERVICE), + getServices: sinon.stub().callsFake(async () => services), + getServiceById: sinon.stub().rejects(new Error("not used")), + getServiceTypes: sinon.stub().resolves([]), + getServiceInfo: sinon.stub().rejects(new Error("not used")), } satisfies ServiceRegistryProvider); +/** A promise and the function that resolves it. */ +const deferred = () => { + let settleWith: (value: T) => void = () => undefined; + const promise = new Promise((resolve) => { + settleWith = resolve; + }); + return { promise, resolve: settleWith }; +}; + +/** The services the component shows. */ +const shown = (el: ECCClientGa4ghServiceRegistryServices) => + (el as unknown as { services: ExternalService[] }).services; + describe("ecc-client-ga4gh-service-registry-services provider", () => { let fetchStub: sinon.SinonStub; @@ -48,48 +65,88 @@ describe("ecc-client-ga4gh-service-registry-services provider", () => { const el = await fixture( html`` ); + expect(el.shadowRoot?.textContent).to.contain(MISSING); const provider = stubProvider(); el.provider = provider; - await waitUntil(() => provider.getServiceTypes.callCount === 1); + await waitUntil(() => shown(el).length === 1); + await el.updateComplete; expect(provider.getServices.callCount).to.equal(1); + expect(provider.getServiceTypes.callCount).to.equal(1); + expect(el.shadowRoot?.textContent).not.to.contain(MISSING); expect(fetchStub.callCount).to.equal(0); }); it("loads from a provider that replaces the first one", async () => { const first = stubProvider(); - const second = stubProvider(); + const second = stubProvider([service("b")]); const el = await fixture( html`` ); - await waitUntil(() => first.getServiceTypes.callCount === 1); + await waitUntil(() => shown(el)[0]?.id === "a"); el.provider = second; - await waitUntil(() => second.getServiceTypes.callCount === 1); + await waitUntil(() => shown(el)[0]?.id === "b"); expect(second.getServices.callCount).to.equal(1); expect(first.getServices.callCount).to.equal(1); }); + it("drops a slower load from a replaced provider", async () => { + const slow = deferred(); + const first = stubProvider(slow.promise); + const second = stubProvider([service("b")]); + const el = await fixture( + html`` + ); + + el.provider = second; + await waitUntil(() => shown(el)[0]?.id === "b"); + slow.resolve([service("a")]); + await slow.promise; + await el.updateComplete; + + expect(shown(el)[0].id).to.equal("b"); + }); + it("keeps a provider when the base URL is set too", async () => { const provider = stubProvider(); const el = await fixture( html`` ); - await waitUntil(() => provider.getServiceTypes.callCount === 1); + await waitUntil(() => shown(el).length === 1); - el.baseUrl = BASE_URL; + el.baseUrl = OTHER_URL; await el.updateComplete; expect(provider.getServices.callCount).to.equal(1); expect(fetchStub.callCount).to.equal(0); }); + it("loads from the base URL and reloads when it changes", async () => { + const el = await fixture( + html`` + ); + await waitUntil(() => fetchStub.callCount === 2); + + el.baseUrl = OTHER_URL; + await waitUntil(() => fetchStub.callCount === 4); + + const urls = fetchStub.getCalls().map((call) => call.args[0]); + expect(urls).to.include(`${BASE_URL}/services`); + expect(urls).to.include(`${OTHER_URL}/services`); + }); + it("uses the base URL once the provider is removed", async () => { const provider = stubProvider(); const el = await fixture( @@ -98,7 +155,7 @@ describe("ecc-client-ga4gh-service-registry-services provider", () => { .baseUrl=${BASE_URL} >` ); - await waitUntil(() => provider.getServiceTypes.callCount === 1); + await waitUntil(() => shown(el).length === 1); el.provider = undefined; await waitUntil(() => fetchStub.callCount === 2); @@ -106,4 +163,21 @@ describe("ecc-client-ga4gh-service-registry-services provider", () => { expect(fetchStub.firstCall.args[0]).to.equal(`${BASE_URL}/services`); expect(provider.getServices.callCount).to.equal(1); }); + + it("shows an error once the provider is removed without a base URL", async () => { + const provider = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => shown(el).length === 1); + + el.provider = undefined; + await el.updateComplete; + await el.updateComplete; + + expect(shown(el)).to.deep.equal([]); + expect(el.shadowRoot?.textContent).to.contain(MISSING); + }); }); diff --git a/packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts b/packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts index 741b3984..5cf04923 100644 --- a/packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts +++ b/packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts @@ -3,24 +3,47 @@ import { expect, fixture, html, waitUntil } from "@open-wc/testing"; import sinon from "sinon"; import "../index.js"; import type { ECCClientGa4ghTrsTools } from "../tools.js"; -import type { ToolClass, TrsProvider } from "../../../providers/index.js"; +import type { Tool, ToolClass, TrsProvider } from "../../../providers/index.js"; const BASE_URL = "https://trs.example.org/ga4gh/trs/v2"; +const OTHER_URL = "https://other.example.org/ga4gh/trs/v2"; +const MISSING = "Please provide either a base URL"; const TOOL_CLASS: ToolClass = { id: "workflow", name: "Workflow" }; +const tool = (id: string): Tool => ({ + id, + url: `${BASE_URL}/tools/${id}`, + organization: "Example", + toolclass: TOOL_CLASS, + versions: [], +}); + /** * A provider whose list methods are stubs; the tools list uses no other * methods. */ -const stubProvider = () => { +const stubProvider = (tools: Tool[] | Promise = [tool("a")]) => { const stubs = { getToolClasses: sinon.stub().resolves([TOOL_CLASS]), - getToolsList: sinon.stub().resolves([]), + getToolsList: sinon.stub().callsFake(async () => tools), }; return { stubs, provider: stubs as unknown as TrsProvider }; }; +/** A promise and the function that resolves it. */ +const deferred = () => { + let settleWith: (value: T) => void = () => undefined; + const promise = new Promise((resolve) => { + settleWith = resolve; + }); + return { promise, resolve: settleWith }; +}; + +/** The component's private state that the tests read and set. */ +const internals = (el: ECCClientGa4ghTrsTools) => + el as unknown as { tools: Tool[]; currentPage: number; lastPage: number }; + describe("ecc-client-ga4gh-trs-tools provider", () => { let fetchStub: sinon.SinonStub; @@ -41,30 +64,57 @@ describe("ecc-client-ga4gh-trs-tools provider", () => { const el = await fixture( html`` ); + expect(el.shadowRoot?.textContent).to.contain(MISSING); const { stubs, provider } = stubProvider(); el.provider = provider; - await waitUntil(() => stubs.getToolClasses.callCount === 1); + await waitUntil(() => internals(el).tools.length === 1); + await el.updateComplete; expect(stubs.getToolsList.callCount).to.equal(1); + expect(stubs.getToolClasses.callCount).to.equal(1); + expect(el.shadowRoot?.textContent).not.to.contain(MISSING); expect(fetchStub.callCount).to.equal(0); }); - it("loads from a provider that replaces the first one", async () => { + it("loads the first page from a provider that replaces the first one", async () => { const first = stubProvider(); - const second = stubProvider(); + const second = stubProvider([tool("b")]); + const el = await fixture( + html`` + ); + await waitUntil(() => internals(el).tools[0]?.id === "a"); + internals(el).currentPage = 3; + internals(el).lastPage = 3; + const firstCalls = first.stubs.getToolsList.callCount; + + el.provider = second.provider; + await waitUntil(() => internals(el).tools[0]?.id === "b"); + + expect(internals(el).currentPage).to.equal(1); + expect(second.stubs.getToolsList.firstCall.args[1]).to.equal(0); + expect(first.stubs.getToolsList.callCount).to.equal(firstCalls); + }); + + it("drops a slower load from a replaced provider", async () => { + const slow = deferred(); + const first = stubProvider(slow.promise); + const second = stubProvider([tool("b")]); const el = await fixture( html`` ); - await waitUntil(() => first.stubs.getToolClasses.callCount === 1); el.provider = second.provider; - await waitUntil(() => second.stubs.getToolClasses.callCount === 1); + await waitUntil(() => internals(el).tools[0]?.id === "b"); + slow.resolve([tool("a")]); + await slow.promise; + await el.updateComplete; - expect(second.stubs.getToolsList.callCount).to.equal(1); - expect(first.stubs.getToolClasses.callCount).to.equal(1); + expect(internals(el).tools[0].id).to.equal("b"); }); it("keeps a provider when the base URL is set too", async () => { @@ -82,4 +132,41 @@ describe("ecc-client-ga4gh-trs-tools provider", () => { expect(stubs.getToolClasses.callCount).to.equal(1); expect(fetchStub.callCount).to.equal(0); }); + + it("loads from the base URL and reloads when it changes", async () => { + const el = await fixture( + html`` + ); + await waitUntil(() => + fetchStub + .getCalls() + .some((call) => call.args[0] === `${BASE_URL}/toolClasses`) + ); + + el.baseUrl = OTHER_URL; + await waitUntil(() => + fetchStub + .getCalls() + .some((call) => call.args[0] === `${OTHER_URL}/toolClasses`) + ); + }); + + it("shows an error once the provider is removed without a base URL", async () => { + const { provider } = stubProvider(); + const el = await fixture( + html`` + ); + await waitUntil(() => internals(el).tools.length === 1); + + el.provider = undefined; + await el.updateComplete; + await el.updateComplete; + + expect(internals(el).tools).to.deep.equal([]); + expect(el.shadowRoot?.textContent).to.contain(MISSING); + }); }); diff --git a/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts b/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts index 48b34020..5b8daa2c 100644 --- a/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts +++ b/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts @@ -107,14 +107,25 @@ export class ECCClientGa4ghTrsTools extends LitElement { (changedProperties.has("provider") || changedProperties.has("baseUrl")) && this.useProvider() ) { - this.loadData(); - this.loadToolClasses(); + // another provider starts on its first page + this.currentPage = 1; + this.lastPage = -1; + if (this._provider) { + this.loadData(); + this.loadToolClasses(); + } else { + this.tools = []; + this.toolClasses = []; + this.error = + "Please provide either a base URL for the TRS API or a custom provider."; + } } } /** * Switches to the custom provider or, without one, to a REST provider for - * the base URL; returns whether the provider in use changed + * the base URL (to none without either); returns whether the provider in + * use changed */ private useProvider(): boolean { let provider: TrsProvider | null = this.provider ?? null; @@ -125,16 +136,20 @@ export class ECCClientGa4ghTrsTools extends LitElement { ? this._provider : new RestTrsProvider(this.baseUrl); } - if (!provider || provider === this._provider) return false; + if (provider === this._provider) return false; this._provider = provider; return true; } private async loadToolClasses(): Promise { - if (!this._provider) return; + const provider = this._provider; + if (!provider) return; try { - this.toolClasses = await this._provider.getToolClasses(); + const toolClasses = await provider.getToolClasses(); + // a load from a provider that was replaced in the meantime is dropped + if (provider !== this._provider) return; + this.toolClasses = toolClasses; // Store a mapping of name to ID for lookup during filtering this.toolClassMap = Object.fromEntries( @@ -146,7 +161,8 @@ export class ECCClientGa4ghTrsTools extends LitElement { } private async loadData(): Promise { - if (!this._provider) return; + const provider = this._provider; + if (!provider) return; this.loading = true; this.error = null; @@ -164,12 +180,14 @@ export class ECCClientGa4ghTrsTools extends LitElement { // We want to send the tool class name directly to the API, not the ID // No conversion needed here, the name is already stored in filterParams.toolClass - const tools = await this._provider.getToolsList( + const tools = await provider.getToolsList( this.pageSize < 0 ? 0 : this.pageSize, offset < 0 ? 0 : offset, apiFilterParams, this.searchQuery ); + // a load from a provider that was replaced in the meantime is dropped + if (provider !== this._provider) return; this.tools = tools; @@ -195,13 +213,14 @@ export class ECCClientGa4ghTrsTools extends LitElement { this.loadData(); } } catch (err) { + if (provider !== this._provider) return; this.error = err instanceof Error ? err.message : "Failed to load tools"; console.error({ error: this.error, breakPoint: "ECCClientGa4ghTrs.loadData", }); } finally { - this.loading = false; + if (provider === this._provider) this.loading = false; } } From b3e431acc62bcc4c0b45e45492ad33ea372f18c2 Mon Sep 17 00:00:00 2001 From: Alex Kanitz Date: Fri, 13 Nov 2026 11:16:51 +0100 Subject: [PATCH 4/8] fix: stop loading when the provider is removed Removing the provider while a load runs left the loading state set, as the dropped load no longer clears it. Also show the missing service ID error when the service ID is cleared, and drop filtered lists of a replaced provider in the Service Registry list. Co-Authored-By: Claude Opus 5.5 --- .../src/components/objects-list/objects.ts | 1 + .../src/components/service/service.ts | 5 ++++ .../src/components/services/services.ts | 8 ++++-- .../src/components/tools/tests/tools.test.ts | 27 ++++++++++++++++++- .../src/components/tools/tools.ts | 1 + 5 files changed, 39 insertions(+), 3 deletions(-) diff --git a/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts b/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts index 02718c55..fe044f32 100644 --- a/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts +++ b/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts @@ -88,6 +88,7 @@ export class ECCClientGa4ghDrsObjects extends LitElement { this.loadData(); } else { this.objects = []; + this.loading = false; this.error = "Please provide either a base URL for the DRS API or a custom provider."; } diff --git a/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts b/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts index 1464646f..1106d25f 100644 --- a/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts +++ b/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts @@ -79,8 +79,13 @@ export class ECCClientGa4ghServiceRegistryService extends LitElement { this.useProvider(); if (providerChanged && !this._provider) { this.service = null; + this.loading = false; this.error = "Please provide either a base URL for the Service Registry API or a custom provider."; + } else if (changedProperties.has("serviceId") && !this.serviceId) { + this.service = null; + this.loading = false; + this.error = "Please provide a service ID."; } else if ( (providerChanged || (changedProperties.has("serviceId") && this.serviceId)) && diff --git a/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts b/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts index 1af54750..d90d3508 100644 --- a/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts +++ b/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts @@ -87,6 +87,7 @@ export class ECCClientGa4ghServiceRegistryServices extends LitElement { this.loadServiceTypes(); } else { this.services = []; + this.loading = false; this.serviceTypes = []; this.error = "Please provide either a base URL for the Service Registry API or a custom provider."; @@ -182,7 +183,8 @@ export class ECCClientGa4ghServiceRegistryServices extends LitElement { } const query = this.searchQuery.toLowerCase(); - if (!this._provider) return; + const provider = this._provider; + if (!provider) return; this.loading = true; this.error = null; @@ -190,7 +192,9 @@ export class ECCClientGa4ghServiceRegistryServices extends LitElement { try { // For now, we'll just filter client-side // In a real implementation, you might want to do this on the server - this._provider.getServices().then((services) => { + provider.getServices().then((services) => { + // a load from a provider that was replaced in the meantime is dropped + if (provider !== this._provider) return; this.services = services.filter( (service) => service.id.toLowerCase().includes(query) || diff --git a/packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts b/packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts index 5cf04923..b4c38aa6 100644 --- a/packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts +++ b/packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts @@ -42,7 +42,12 @@ const deferred = () => { /** The component's private state that the tests read and set. */ const internals = (el: ECCClientGa4ghTrsTools) => - el as unknown as { tools: Tool[]; currentPage: number; lastPage: number }; + el as unknown as { + tools: Tool[]; + currentPage: number; + lastPage: number; + loading: boolean; + }; describe("ecc-client-ga4gh-trs-tools provider", () => { let fetchStub: sinon.SinonStub; @@ -169,4 +174,24 @@ describe("ecc-client-ga4gh-trs-tools provider", () => { expect(internals(el).tools).to.deep.equal([]); expect(el.shadowRoot?.textContent).to.contain(MISSING); }); + + it("stops loading once the provider is removed during a load", async () => { + const slow = deferred(); + const { provider } = stubProvider(slow.promise); + const el = await fixture( + html`` + ); + expect(internals(el).loading).to.equal(true); + + el.provider = undefined; + await el.updateComplete; + slow.resolve([tool("a")]); + await slow.promise; + await el.updateComplete; + + expect(internals(el).loading).to.equal(false); + expect(internals(el).tools).to.deep.equal([]); + }); }); diff --git a/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts b/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts index 5b8daa2c..61a4654d 100644 --- a/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts +++ b/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts @@ -115,6 +115,7 @@ export class ECCClientGa4ghTrsTools extends LitElement { this.loadToolClasses(); } else { this.tools = []; + this.loading = false; this.toolClasses = []; this.error = "Please provide either a base URL for the TRS API or a custom provider."; From 734ff5203a42e99722318ba377686566369addb5 Mon Sep 17 00:00:00 2001 From: Alex Kanitz Date: Fri, 13 Nov 2026 15:35:23 +0100 Subject: [PATCH 5/8] test: run at most three test suites at once With nine packages testing in three browsers each, all at once, Firefox on the CI runners failed to open test pages in time (Node 22 job, twice in a row). Run at most three packages' tests at the same time. Co-Authored-By: Claude Opus 5.5 --- package.json | 2 +- web-test.runner-config.mjs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index d45cea53..6ba6c9a9 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "lint:fix": "eslint . --fix && turbo run lint:fix && prettier .", "new": "turbo gen", "prepare": "husky install", - "test": "turbo run test --parallel", + "test": "turbo run test --concurrency=3", "test:watch": "turbo run test --watch" }, "devDependencies": { diff --git a/web-test.runner-config.mjs b/web-test.runner-config.mjs index 4cf766ab..3493cbe1 100644 --- a/web-test.runner-config.mjs +++ b/web-test.runner-config.mjs @@ -2,7 +2,8 @@ import { esbuildPlugin } from "@web/dev-server-esbuild"; import fg from "fast-glob"; import { playwrightLauncher } from "@web/test-runner-playwright"; -// Packages run tests in parallel (turbo --parallel), so each package's test +// Packages run tests in parallel (at most three at once, as more overload +// the browsers on CI runners), so each package's test // scripts pass a unique --port: design 8000, wes 8001, cloud registry 8002, // tes 8003, service registry 8004, trs 8005, trs-filer 8006, drs 8007. export default { From 7f28a3e0d81a5d2abbc725662e9be983ac37b1fe Mon Sep 17 00:00:00 2001 From: Alex Kanitz Date: Fri, 13 Nov 2026 16:30:21 +0100 Subject: [PATCH 6/8] fix: clear old provider data on failed loads From the automated review: when the new provider's load fails, the components no longer show the data, filter choices or tool classes of the previous provider; a failure of a replaced provider no longer fires the TRS-Filer form's failure event. Removing the provider fires the data-changed event with the cleared value. The components reuse only the REST provider they created for the base URL, not a custom provider that happens to be a REST provider for the same URL. Co-Authored-By: Claude Opus 5.5 --- .../src/components/tool-create/tool-create.ts | 17 ++++--- .../src/components/objects-list/objects.ts | 23 ++++++--- .../src/components/service/service.ts | 23 ++++++--- .../src/components/services/services.ts | 24 ++++++--- .../services/tests/services.test.ts | 50 ++++++++++++++++++- .../src/components/tools/tests/tools.test.ts | 8 +++ .../src/components/tools/tools.ts | 28 ++++++++--- 7 files changed, 142 insertions(+), 31 deletions(-) diff --git a/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts b/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts index ed6ce048..08080d62 100644 --- a/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts +++ b/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts @@ -227,6 +227,9 @@ export class ECCClientElixirTrsToolCreate extends LitElement { private _provider: TrsFilerProvider | null = null; + /** The REST provider for `baseUrl`, used without a custom provider */ + private restProvider: RestTrsFilerProvider | null = null; + @state() private activeFileIndex: { [versionIndex: number]: number } = {}; @state() private activeDescriptorType: { [versionIndex: number]: DescriptorType | "ALL"; @@ -253,7 +256,8 @@ export class ECCClientElixirTrsToolCreate extends LitElement { if (this.provider) { this._provider = this.provider; } else if (this.baseUrl) { - this._provider = new RestTrsFilerProvider(this.baseUrl); + this.restProvider = new RestTrsFilerProvider(this.baseUrl); + this._provider = this.restProvider; } else { this._provider = null; } @@ -357,11 +361,10 @@ export class ECCClientElixirTrsToolCreate extends LitElement { private useProvider(): boolean { let provider: TrsFilerProvider | null = this.provider ?? null; if (!provider && this.baseUrl) { - provider = - this._provider instanceof RestTrsFilerProvider && - this._provider.baseUrl === this.baseUrl - ? this._provider - : new RestTrsFilerProvider(this.baseUrl); + if (this.restProvider?.baseUrl !== this.baseUrl) { + this.restProvider = new RestTrsFilerProvider(this.baseUrl); + } + provider = this.restProvider; } if (provider === this._provider) return false; this._provider = provider; @@ -378,6 +381,8 @@ export class ECCClientElixirTrsToolCreate extends LitElement { if (provider !== this._provider) return; this.toolClasses = toolClasses; } catch (error) { + if (provider !== this._provider) return; + this.toolClasses = []; console.error("Failed to load tool classes:", error); this.dispatchEvent( new CustomEvent("ecc-tool-create-failed", { diff --git a/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts b/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts index fe044f32..6cdf1374 100644 --- a/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts +++ b/packages/ecc-client-ga4gh-drs/src/components/objects-list/objects.ts @@ -52,6 +52,9 @@ export class ECCClientGa4ghDrsObjects extends LitElement { private _provider: DrsProvider | null = null; + /** The REST provider for `baseUrl`, used without a custom provider */ + private restProvider: RestDrsProvider | null = null; + protected async firstUpdated(): Promise { if (!this.baseUrl && !this.provider) { this.error = @@ -62,7 +65,8 @@ export class ECCClientGa4ghDrsObjects extends LitElement { if (this.provider) { this._provider = this.provider; } else if (this.baseUrl) { - this._provider = new RestDrsProvider(this.baseUrl); + this.restProvider = new RestDrsProvider(this.baseUrl); + this._provider = this.restProvider; } else { this._provider = null; } @@ -89,6 +93,13 @@ export class ECCClientGa4ghDrsObjects extends LitElement { } else { this.objects = []; this.loading = false; + this.dispatchEvent( + new CustomEvent("ecc-objects-changed", { + detail: { objects: [] }, + bubbles: true, + composed: true, + }) + ); this.error = "Please provide either a base URL for the DRS API or a custom provider."; } @@ -103,11 +114,10 @@ export class ECCClientGa4ghDrsObjects extends LitElement { private useProvider(): boolean { let provider: DrsProvider | null = this.provider ?? null; if (!provider && this.baseUrl) { - provider = - this._provider instanceof RestDrsProvider && - this._provider.baseUrl === this.baseUrl - ? this._provider - : new RestDrsProvider(this.baseUrl); + if (this.restProvider?.baseUrl !== this.baseUrl) { + this.restProvider = new RestDrsProvider(this.baseUrl); + } + provider = this.restProvider; } if (provider === this._provider) return false; this._provider = provider; @@ -163,6 +173,7 @@ export class ECCClientGa4ghDrsObjects extends LitElement { ); } catch (err) { if (provider !== this._provider) return; + this.objects = []; this.error = err instanceof Error ? err.message : "Failed to load objects"; } finally { diff --git a/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts b/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts index 1106d25f..524ba9ec 100644 --- a/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts +++ b/packages/ecc-client-ga4gh-service-registry/src/components/service/service.ts @@ -48,6 +48,9 @@ export class ECCClientGa4ghServiceRegistryService extends LitElement { private _provider: ServiceRegistryProvider | null = null; + /** The REST provider for `baseUrl`, used without a custom provider */ + private restProvider: RestServiceRegistryProvider | null = null; + protected async firstUpdated(): Promise { if (!this.baseUrl && !this.provider) { this.error = @@ -63,7 +66,8 @@ export class ECCClientGa4ghServiceRegistryService extends LitElement { if (this.provider) { this._provider = this.provider; } else if (this.baseUrl) { - this._provider = new RestServiceRegistryProvider(this.baseUrl); + this.restProvider = new RestServiceRegistryProvider(this.baseUrl); + this._provider = this.restProvider; } else { this._provider = null; } @@ -79,6 +83,13 @@ export class ECCClientGa4ghServiceRegistryService extends LitElement { this.useProvider(); if (providerChanged && !this._provider) { this.service = null; + this.dispatchEvent( + new CustomEvent("ecc-service-changed", { + detail: { service: null }, + bubbles: true, + composed: true, + }) + ); this.loading = false; this.error = "Please provide either a base URL for the Service Registry API or a custom provider."; @@ -104,11 +115,10 @@ export class ECCClientGa4ghServiceRegistryService extends LitElement { private useProvider(): boolean { let provider: ServiceRegistryProvider | null = this.provider ?? null; if (!provider && this.baseUrl) { - provider = - this._provider instanceof RestServiceRegistryProvider && - this._provider.baseUrl === this.baseUrl - ? this._provider - : new RestServiceRegistryProvider(this.baseUrl); + if (this.restProvider?.baseUrl !== this.baseUrl) { + this.restProvider = new RestServiceRegistryProvider(this.baseUrl); + } + provider = this.restProvider; } if (provider === this._provider) return false; this._provider = provider; @@ -142,6 +152,7 @@ export class ECCClientGa4ghServiceRegistryService extends LitElement { ); } catch (err) { if (replaced()) return; + this.service = null; this.error = err instanceof Error ? err.message : "Failed to load service"; console.error({ diff --git a/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts b/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts index d90d3508..43237606 100644 --- a/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts +++ b/packages/ecc-client-ga4gh-service-registry/src/components/services/services.ts @@ -56,6 +56,9 @@ export class ECCClientGa4ghServiceRegistryServices extends LitElement { private _provider: ServiceRegistryProvider | null = null; + /** The REST provider for `baseUrl`, used without a custom provider */ + private restProvider: RestServiceRegistryProvider | null = null; + protected async firstUpdated(): Promise { if (!this.baseUrl && !this.provider) { this.error = @@ -66,7 +69,8 @@ export class ECCClientGa4ghServiceRegistryServices extends LitElement { if (this.provider) { this._provider = this.provider; } else if (this.baseUrl) { - this._provider = new RestServiceRegistryProvider(this.baseUrl); + this.restProvider = new RestServiceRegistryProvider(this.baseUrl); + this._provider = this.restProvider; } else { this._provider = null; } @@ -89,6 +93,13 @@ export class ECCClientGa4ghServiceRegistryServices extends LitElement { this.services = []; this.loading = false; this.serviceTypes = []; + this.dispatchEvent( + new CustomEvent("ecc-services-changed", { + detail: { services: [] }, + bubbles: true, + composed: true, + }) + ); this.error = "Please provide either a base URL for the Service Registry API or a custom provider."; } @@ -103,11 +114,10 @@ export class ECCClientGa4ghServiceRegistryServices extends LitElement { private useProvider(): boolean { let provider: ServiceRegistryProvider | null = this.provider ?? null; if (!provider && this.baseUrl) { - provider = - this._provider instanceof RestServiceRegistryProvider && - this._provider.baseUrl === this.baseUrl - ? this._provider - : new RestServiceRegistryProvider(this.baseUrl); + if (this.restProvider?.baseUrl !== this.baseUrl) { + this.restProvider = new RestServiceRegistryProvider(this.baseUrl); + } + provider = this.restProvider; } if (provider === this._provider) return false; this._provider = provider; @@ -124,6 +134,7 @@ export class ECCClientGa4ghServiceRegistryServices extends LitElement { if (provider !== this._provider) return; this.serviceTypes = serviceTypes; } catch (error) { + if (provider === this._provider) this.serviceTypes = []; console.error("Failed to load service types:", error); } } @@ -151,6 +162,7 @@ export class ECCClientGa4ghServiceRegistryServices extends LitElement { ); } catch (err) { if (provider !== this._provider) return; + this.services = []; this.error = err instanceof Error ? err.message : "Failed to load services"; console.error({ diff --git a/packages/ecc-client-ga4gh-service-registry/src/components/services/tests/services.test.ts b/packages/ecc-client-ga4gh-service-registry/src/components/services/tests/services.test.ts index f6745fff..1b0ff015 100644 --- a/packages/ecc-client-ga4gh-service-registry/src/components/services/tests/services.test.ts +++ b/packages/ecc-client-ga4gh-service-registry/src/components/services/tests/services.test.ts @@ -1,5 +1,5 @@ /* eslint-disable import/no-extraneous-dependencies */ -import { expect, fixture, html, waitUntil } from "@open-wc/testing"; +import { expect, fixture, html, oneEvent, waitUntil } from "@open-wc/testing"; import sinon from "sinon"; import "../index.js"; import type { ECCClientGa4ghServiceRegistryServices } from "../services.js"; @@ -7,6 +7,7 @@ import type { ExternalService, ServiceRegistryProvider, } from "../../../providers/index.js"; +import { RestServiceRegistryProvider } from "../../../providers/rest-sr-provider.js"; const BASE_URL = "https://registry.example.org/ga4gh/registry/v1"; const OTHER_URL = "https://other.example.org/ga4gh/registry/v1"; @@ -164,6 +165,32 @@ describe("ecc-client-ga4gh-service-registry-services provider", () => { expect(provider.getServices.callCount).to.equal(1); }); + it("uses a REST provider of its own once a custom REST provider is removed", async () => { + // a custom provider that is a REST provider for the same base URL + class CustomProvider extends RestServiceRegistryProvider { + requests = 0; + + protected fetch(input: RequestInfo | URL, init?: RequestInit) { + this.requests += 1; + return super.fetch(input, init); + } + } + const provider = new CustomProvider(BASE_URL); + const el = await fixture( + html`` + ); + await waitUntil(() => fetchStub.callCount === 2); + const { requests } = provider; + + el.provider = undefined; + await waitUntil(() => fetchStub.callCount === 4); + + expect(provider.requests).to.equal(requests); + }); + it("shows an error once the provider is removed without a base URL", async () => { const provider = stubProvider(); const el = await fixture( @@ -172,6 +199,7 @@ describe("ecc-client-ga4gh-service-registry-services provider", () => { >` ); await waitUntil(() => shown(el).length === 1); + const changed = oneEvent(el, "ecc-services-changed"); el.provider = undefined; await el.updateComplete; @@ -179,5 +207,25 @@ describe("ecc-client-ga4gh-service-registry-services provider", () => { expect(shown(el)).to.deep.equal([]); expect(el.shadowRoot?.textContent).to.contain(MISSING); + expect((await changed).detail.services).to.deep.equal([]); + }); + + it("shows no services of a replaced provider when the new one fails", async () => { + const first = stubProvider(); + const second = stubProvider(); + second.getServices.callsFake(async () => { + throw new Error("Unavailable"); + }); + const el = await fixture( + html`` + ); + await waitUntil(() => shown(el).length === 1); + + el.provider = second; + await waitUntil(() => el.shadowRoot?.textContent?.includes("Unavailable")); + + expect(shown(el)).to.deep.equal([]); }); }); diff --git a/packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts b/packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts index b4c38aa6..c12c3219 100644 --- a/packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts +++ b/packages/ecc-client-ga4gh-trs/src/components/tools/tests/tools.test.ts @@ -156,6 +156,14 @@ describe("ecc-client-ga4gh-trs-tools provider", () => { .getCalls() .some((call) => call.args[0] === `${OTHER_URL}/toolClasses`) ); + + const urls = fetchStub.getCalls().map((call) => String(call.args[0])); + expect(urls.some((url) => url.startsWith(`${BASE_URL}/tools?`))).to.equal( + true + ); + expect(urls.some((url) => url.startsWith(`${OTHER_URL}/tools?`))).to.equal( + true + ); }); it("shows an error once the provider is removed without a base URL", async () => { diff --git a/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts b/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts index 61a4654d..4d4ec4dd 100644 --- a/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts +++ b/packages/ecc-client-ga4gh-trs/src/components/tools/tools.ts @@ -77,6 +77,9 @@ export class ECCClientGa4ghTrsTools extends LitElement { private _provider: TrsProvider | null = null; + /** The REST provider for `baseUrl`, used without a custom provider */ + private restProvider: RestTrsProvider | null = null; + protected async firstUpdated(): Promise { if (!this.baseUrl && !this.provider) { this.error = @@ -87,7 +90,8 @@ export class ECCClientGa4ghTrsTools extends LitElement { if (this.provider) { this._provider = this.provider; } else if (this.baseUrl) { - this._provider = new RestTrsProvider(this.baseUrl); + this.restProvider = new RestTrsProvider(this.baseUrl); + this._provider = this.restProvider; } else { this._provider = null; } @@ -117,6 +121,14 @@ export class ECCClientGa4ghTrsTools extends LitElement { this.tools = []; this.loading = false; this.toolClasses = []; + this.toolClassMap = {}; + this.dispatchEvent( + new CustomEvent("ecc-tools-changed", { + detail: { tools: [] }, + bubbles: true, + composed: true, + }) + ); this.error = "Please provide either a base URL for the TRS API or a custom provider."; } @@ -131,11 +143,10 @@ export class ECCClientGa4ghTrsTools extends LitElement { private useProvider(): boolean { let provider: TrsProvider | null = this.provider ?? null; if (!provider && this.baseUrl) { - provider = - this._provider instanceof RestTrsProvider && - this._provider.baseUrl === this.baseUrl - ? this._provider - : new RestTrsProvider(this.baseUrl); + if (this.restProvider?.baseUrl !== this.baseUrl) { + this.restProvider = new RestTrsProvider(this.baseUrl); + } + provider = this.restProvider; } if (provider === this._provider) return false; this._provider = provider; @@ -157,6 +168,10 @@ export class ECCClientGa4ghTrsTools extends LitElement { this.toolClasses.map((tc) => [tc.name, tc.id]) ); } catch (error) { + if (provider === this._provider) { + this.toolClasses = []; + this.toolClassMap = {}; + } console.error("Failed to load tool classes:", error); } } @@ -215,6 +230,7 @@ export class ECCClientGa4ghTrsTools extends LitElement { } } catch (err) { if (provider !== this._provider) return; + this.tools = []; this.error = err instanceof Error ? err.message : "Failed to load tools"; console.error({ error: this.error, From 7699902dd6549d6688f979bc5d55cdee726c949e Mon Sep 17 00:00:00 2001 From: Alex Kanitz Date: Fri, 13 Nov 2026 17:07:57 +0100 Subject: [PATCH 7/8] fix: report a removed provider in the tool form From the automated review: the TRS-Filer form fires ecc-tool-create-validation-failed when its provider is removed without a base URL, as it does on the first update. The providers overview no longer says that loads of a replaced provider are dropped, as not all components do so. The service test checks the replacement provider's result. Co-Authored-By: Claude Opus 5.5 --- apps/documentation/pages/docs/providers.mdx | 3 +-- .../components/tool-create/tests/tool-create.test.ts | 4 +++- .../src/components/tool-create/tool-create.ts | 10 ++++++++++ .../src/components/service/tests/service.test.ts | 6 ++++-- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/apps/documentation/pages/docs/providers.mdx b/apps/documentation/pages/docs/providers.mdx index 3758da4f..a0026f9c 100644 --- a/apps/documentation/pages/docs/providers.mdx +++ b/apps/documentation/pages/docs/providers.mdx @@ -46,8 +46,7 @@ const provider = new RestProvider('https://api.example.com'); You can set or replace a component's provider at any time, also after the component has rendered (as with React, which sets it on a server-rendered element only after the first update): the component then loads its data from -the new provider, and a load that the previous provider has not finished yet -is dropped. Create the provider once and pass the same instance on every +the new provider. Create the provider once and pass the same instance on every render (e.g. with `useMemo` in React), as each new instance makes the component load its data again. diff --git a/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tests/tool-create.test.ts b/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tests/tool-create.test.ts index c1f8d02e..24b8b102 100644 --- a/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tests/tool-create.test.ts +++ b/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tests/tool-create.test.ts @@ -1,5 +1,5 @@ /* eslint-disable import/no-extraneous-dependencies */ -import { expect, fixture, html, waitUntil } from "@open-wc/testing"; +import { expect, fixture, html, oneEvent, waitUntil } from "@open-wc/testing"; import sinon from "sinon"; import "../index.js"; import type { ECCClientElixirTrsToolCreate } from "../tool-create.js"; @@ -141,10 +141,12 @@ describe("ecc-client-elixir-trs-tool-create provider", () => { >` ); await waitUntil(() => offered(el).length === 1); + const failed = oneEvent(el, "ecc-tool-create-validation-failed"); el.provider = undefined; await el.updateComplete; expect(offered(el)).to.deep.equal([]); + expect((await failed).detail.error).to.contain("Please provide either"); }); }); diff --git a/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts b/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts index 08080d62..d6976ed0 100644 --- a/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts +++ b/packages/ecc-client-elixir-trs-filer/src/components/tool-create/tool-create.ts @@ -349,6 +349,16 @@ export class ECCClientElixirTrsToolCreate extends LitElement { this.loadToolClasses(); } else { this.toolClasses = []; + this.dispatchEvent( + new CustomEvent("ecc-tool-create-validation-failed", { + detail: { + error: + "Please provide either a base URL for the TRS API or a custom provider.", + }, + bubbles: true, + composed: true, + }) + ); } } } diff --git a/packages/ecc-client-ga4gh-service-registry/src/components/service/tests/service.test.ts b/packages/ecc-client-ga4gh-service-registry/src/components/service/tests/service.test.ts index 2c20424b..d6bb05b7 100644 --- a/packages/ecc-client-ga4gh-service-registry/src/components/service/tests/service.test.ts +++ b/packages/ecc-client-ga4gh-service-registry/src/components/service/tests/service.test.ts @@ -84,7 +84,8 @@ describe("ecc-client-ga4gh-service-registry-service provider", () => { it("loads from a provider that replaces the first one", async () => { const first = stubProvider(); - const second = stubProvider(); + const replacement = { ...service("a"), name: "Replacement" }; + const second = stubProvider(Promise.resolve(replacement)); const el = await fixture( html` { const firstCalls = first.getServiceById.callCount; el.provider = second; - await waitUntil(() => second.getServiceById.callCount === 1); + await waitUntil(() => shown(el)?.name === "Replacement"); + expect(second.getServiceById.callCount).to.equal(1); expect(first.getServiceById.callCount).to.equal(firstCalls); }); From 29510bd267db81013f8e9444241c76caba5979da Mon Sep 17 00:00:00 2001 From: Alex Kanitz Date: Sat, 14 Nov 2026 18:25:16 +0100 Subject: [PATCH 8/8] test: cover the REST providers This pull request adds test commands to the TRS, TRS-Filer, DRS and Service Registry packages, so CI now uploads their coverage, and their untested providers pulled the project coverage down by 4.5 % (Codecov allows 1 %). Test every method of the four REST providers: the requested URL, error responses, requests through an overridden fetch, paging and filters, the descriptor path fallback and the TRS-Filer creation methods. Co-Authored-By: Claude Opus 5.5 --- .../tests/rest-trs-filer-provider.test.ts | 343 ++++++++++++++++++ .../providers/tests/rest-drs-provider.test.ts | 103 ++++++ .../providers/tests/rest-sr-provider.test.ts | 83 +++++ .../providers/tests/rest-trs-provider.test.ts | 236 ++++++++++++ 4 files changed, 765 insertions(+) create mode 100644 packages/ecc-client-elixir-trs-filer/src/providers/tests/rest-trs-filer-provider.test.ts create mode 100644 packages/ecc-client-ga4gh-drs/src/providers/tests/rest-drs-provider.test.ts create mode 100644 packages/ecc-client-ga4gh-service-registry/src/providers/tests/rest-sr-provider.test.ts create mode 100644 packages/ecc-client-ga4gh-trs/src/providers/tests/rest-trs-provider.test.ts diff --git a/packages/ecc-client-elixir-trs-filer/src/providers/tests/rest-trs-filer-provider.test.ts b/packages/ecc-client-elixir-trs-filer/src/providers/tests/rest-trs-filer-provider.test.ts new file mode 100644 index 00000000..9fa6c2fa --- /dev/null +++ b/packages/ecc-client-elixir-trs-filer/src/providers/tests/rest-trs-filer-provider.test.ts @@ -0,0 +1,343 @@ +/* eslint-disable import/no-extraneous-dependencies */ +import { expect } from "@open-wc/testing"; +import sinon from "sinon"; +import { RestTrsFilerProvider } from "../rest-trs-filer-provider.js"; +import type { ToolRegister } from "../trs-filer-provider.js"; + +const BASE_URL = "https://trs.example.org/ga4gh/trs/v2"; +const TOOL = `${BASE_URL}/tools/tool%201`; +const TOOL_REGISTER: ToolRegister = { + organization: "Example", + toolclass: { id: "workflow", name: "Workflow" }, + versions: [], +}; +const VERSION = { name: "v1" }; +const TOOL_CLASS = { name: "Workflow" }; + +/** Provider that adds a bearer token to every request, as in the docs. */ +class TokenTrsFilerProvider extends RestTrsFilerProvider { + protected fetch(input: RequestInfo | URL, init: RequestInit = {}) { + const headers = new Headers(init.headers); + headers.set("Authorization", "Bearer test-token"); + return super.fetch(input, { ...init, headers }); + } +} + +type Call = [ + string, + (provider: RestTrsFilerProvider) => Promise, + string +]; + +/** Every provider method, with the URL it requests and its error message. */ +const CALLS: Call[] = [ + ["getToolClasses", (p) => p.getToolClasses(), `${BASE_URL}/toolClasses`], + ["getTool", (p) => p.getTool(BASE_URL, "tool 1"), TOOL], + [ + "getToolVersions", + (p) => p.getToolVersions(BASE_URL, "tool 1"), + `${TOOL}/versions`, + ], + [ + "getToolVersion", + (p) => p.getToolVersion(BASE_URL, "tool 1", "tool 1:v1"), + `${TOOL}/versions/v1`, + ], + [ + "getToolFiles", + (p) => p.getToolFiles(BASE_URL, "tool 1", "v1", "CWL"), + `${TOOL}/versions/v1/CWL/files`, + ], + [ + "getToolDescriptor", + (p) => p.getToolDescriptor(BASE_URL, "tool 1", "tool 1:v1", "WDL"), + `${TOOL}/versions/v1/WDL/descriptor`, + ], + [ + "getContainerfile", + (p) => p.getContainerfile(BASE_URL, "tool 1", "v1"), + `${TOOL}/versions/v1/containerfile`, + ], + [ + "getToolTests", + (p) => p.getToolTests(BASE_URL, "tool 1", "tool 1:v1", "NFL"), + `${TOOL}/versions/v1/NFL/tests`, + ], +]; + +const ok = (body: unknown = {}) => + new Response(JSON.stringify(body), { status: 200 }); +const failed = () => + new Response("{}", { status: 404, statusText: "Not Found" }); + +describe("RestTrsFilerProvider", () => { + let fetchStub: sinon.SinonStub; + + beforeEach(() => { + fetchStub = sinon.stub(window, "fetch").callsFake(async () => ok()); + }); + + afterEach(() => { + sinon.restore(); + }); + + CALLS.forEach(([name, call, url]) => { + it(`${name} requests ${url.slice(BASE_URL.length)}`, async () => { + fetchStub.callsFake(async () => ok({ name })); + + const result = await call(new RestTrsFilerProvider(BASE_URL)); + + expect(fetchStub.callCount).to.equal(1); + expect(fetchStub.firstCall.args[0]).to.equal(url); + expect(result).to.deep.equal({ name }); + }); + + it(`${name} throws on an error response`, async () => { + fetchStub.callsFake(async () => failed()); + + let error: Error | undefined; + try { + await call(new RestTrsFilerProvider(BASE_URL)); + } catch (e) { + error = e as Error; + } + + expect(error?.message).to.match(/^Failed to fetch .*: Not Found$/); + }); + + it(`${name} sends its request through an overridden fetch`, async () => { + await call(new TokenTrsFilerProvider(BASE_URL)); + + const headers = new Headers(fetchStub.firstCall.args[1]?.headers); + expect(headers.get("Authorization")).to.equal("Bearer test-token"); + }); + }); + + it("getToolFiles asks for a ZIP archive", async () => { + await new RestTrsFilerProvider(BASE_URL).getToolFiles( + BASE_URL, + "tool 1", + "v1", + "CWL", + "zip" + ); + + expect(fetchStub.firstCall.args[0]).to.equal( + `${TOOL}/versions/v1/CWL/files?format=zip` + ); + }); + + describe("getToolsList", () => { + it("requests the first page without parameters", async () => { + await new RestTrsFilerProvider(BASE_URL).getToolsList(0, 0, {}, ""); + + expect(fetchStub.firstCall.args[0]).to.equal(`${BASE_URL}/tools?`); + }); + + it("adds paging, the query and set filters", async () => { + await new RestTrsFilerProvider(BASE_URL).getToolsList( + 10, + 20, + { + offset: "5", + toolClass: "Workflow", + checker: false, + author: "", + organization: undefined, + }, + "my tool" + ); + + expect(fetchStub.firstCall.args[0]).to.equal( + `${BASE_URL}/tools?limit=10&offset=20&` + + "&name=my%20tool&toolClass=Workflow&checker=false" + ); + }); + + it("throws on an error response", async () => { + fetchStub.callsFake(async () => failed()); + + let error: Error | undefined; + try { + await new RestTrsFilerProvider(BASE_URL).getToolsList(10, 0, {}, ""); + } catch (e) { + error = e as Error; + } + + expect(error?.message).to.equal("Failed to fetch tools: Not Found"); + }); + }); + + describe("getToolDescriptorByPath", () => { + const DESCRIPTOR = `${TOOL}/versions/v1/CWL/descriptor`; + const byPath = (provider: RestTrsFilerProvider) => + provider.getToolDescriptorByPath( + BASE_URL, + "tool 1", + "tool 1:v1", + "CWL", + "dir/main.cwl" + ); + + it("requests the unencoded path first", async () => { + fetchStub.callsFake(async () => ok({ content: "plain" })); + + const result = await byPath(new RestTrsFilerProvider(BASE_URL)); + + expect(fetchStub.callCount).to.equal(1); + expect(fetchStub.firstCall.args[0]).to.equal( + `${DESCRIPTOR}/dir/main.cwl` + ); + expect(result).to.deep.equal({ content: "plain" }); + }); + + it("falls back to the encoded path after an error response", async () => { + fetchStub.onFirstCall().callsFake(async () => failed()); + fetchStub.onSecondCall().callsFake(async () => ok({ content: "enc" })); + + const result = await byPath(new RestTrsFilerProvider(BASE_URL)); + + expect(fetchStub.callCount).to.equal(2); + expect(fetchStub.secondCall.args[0]).to.equal( + `${DESCRIPTOR}/dir%2Fmain.cwl` + ); + expect(result).to.deep.equal({ content: "enc" }); + }); + + it("falls back to the encoded path after a network error", async () => { + fetchStub.onFirstCall().rejects(new TypeError("Network down")); + fetchStub.onSecondCall().callsFake(async () => ok({ content: "enc" })); + + const result = await byPath(new RestTrsFilerProvider(BASE_URL)); + + expect(fetchStub.secondCall.args[0]).to.equal( + `${DESCRIPTOR}/dir%2Fmain.cwl` + ); + expect(result).to.deep.equal({ content: "enc" }); + }); + + it("throws the first error if both paths fail", async () => { + fetchStub.callsFake(async () => failed()); + + let error: Error | undefined; + try { + await byPath(new RestTrsFilerProvider(BASE_URL)); + } catch (e) { + error = e as Error; + } + + expect(error?.message).to.equal( + "Failed to fetch tool descriptor by path: Not Found" + ); + }); + + it("throws the network error if the encoded path fails too", async () => { + fetchStub.onFirstCall().rejects(new TypeError("Network down")); + fetchStub.onSecondCall().callsFake(async () => failed()); + + let error: Error | undefined; + try { + await byPath(new RestTrsFilerProvider(BASE_URL)); + } catch (e) { + error = e as Error; + } + + expect(error?.message).to.equal("Network down"); + }); + }); + + type Write = [ + string, + (provider: RestTrsFilerProvider) => Promise, + string, + string, + unknown, + string + ]; + + /** Every creation method: URL, HTTP method, body and error message. */ + const WRITES: Write[] = [ + [ + "createTool", + (p) => p.createTool(TOOL_REGISTER), + `${BASE_URL}/tools`, + "POST", + TOOL_REGISTER, + "Failed to create tool", + ], + [ + "createToolWithId", + (p) => p.createToolWithId("tool 1", TOOL_REGISTER), + TOOL, + "PUT", + TOOL_REGISTER, + "Failed to create/update tool", + ], + [ + "createToolVersion", + (p) => p.createToolVersion("tool 1", VERSION), + `${TOOL}/versions`, + "POST", + VERSION, + "Failed to create tool version", + ], + [ + "createToolVersionWithId", + (p) => p.createToolVersionWithId("tool 1", "v 1", VERSION), + `${TOOL}/versions/v%201`, + "PUT", + VERSION, + "Failed to create/update tool version", + ], + [ + "createToolClass", + (p) => p.createToolClass(TOOL_CLASS), + `${BASE_URL}/toolClasses`, + "POST", + TOOL_CLASS, + "Failed to create tool class", + ], + [ + "createToolClassWithId", + (p) => p.createToolClassWithId("class 1", TOOL_CLASS), + `${BASE_URL}/toolClasses/class%201`, + "PUT", + TOOL_CLASS, + "Failed to create/update tool class", + ], + ]; + + WRITES.forEach(([name, call, url, method, body, message]) => { + it(`${name} sends ${method} ${url.slice(BASE_URL.length)}`, async () => { + sinon.stub(console, "log"); + fetchStub.callsFake(async () => new Response("new-id", { status: 200 })); + + const result = await call(new TokenTrsFilerProvider(BASE_URL)); + + const [requestUrl, init] = fetchStub.firstCall.args; + const headers = new Headers(init.headers); + expect(requestUrl).to.equal(url); + expect(init.method).to.equal(method); + expect(JSON.parse(init.body)).to.deep.equal(body); + expect(headers.get("Content-Type")).to.equal("application/json"); + expect(headers.get("Authorization")).to.equal("Bearer test-token"); + expect(result).to.equal("new-id"); + }); + + it(`${name} throws on an error response`, async () => { + sinon.stub(console, "log"); + fetchStub.callsFake( + async () => new Response("", { status: 409, statusText: "Conflict" }) + ); + + let error: Error | undefined; + try { + await call(new RestTrsFilerProvider(BASE_URL)); + } catch (e) { + error = e as Error; + } + + expect(error?.message).to.equal(`${message}: Conflict`); + }); + }); +}); diff --git a/packages/ecc-client-ga4gh-drs/src/providers/tests/rest-drs-provider.test.ts b/packages/ecc-client-ga4gh-drs/src/providers/tests/rest-drs-provider.test.ts new file mode 100644 index 00000000..640cdbb3 --- /dev/null +++ b/packages/ecc-client-ga4gh-drs/src/providers/tests/rest-drs-provider.test.ts @@ -0,0 +1,103 @@ +/* eslint-disable import/no-extraneous-dependencies */ +import { expect } from "@open-wc/testing"; +import sinon from "sinon"; +import { RestDrsProvider } from "../rest-drs-provider.js"; + +const BASE_URL = "https://drs.example.org/ga4gh/drs/v1"; + +/** Provider that adds a bearer token to every request, as in the docs. */ +class TokenDrsProvider extends RestDrsProvider { + protected fetch(input: RequestInfo | URL, init: RequestInit = {}) { + const headers = new Headers(init.headers); + headers.set("Authorization", "Bearer test-token"); + return super.fetch(input, { ...init, headers }); + } +} + +type Call = [string, (provider: RestDrsProvider) => Promise, string]; + +/** Every provider method, with the URL it requests. */ +const CALLS: Call[] = [ + ["getObjects", (p) => p.getObjects(), `${BASE_URL}/objects`], + ["getObject", (p) => p.getObject("obj 1"), `${BASE_URL}/objects/obj%201`], + [ + "getAccessURL", + (p) => p.getAccessURL("obj 1", "s3/a"), + `${BASE_URL}/objects/obj%201/access/s3%2Fa`, + ], + ["getServiceInfo", (p) => p.getServiceInfo(), `${BASE_URL}/service-info`], +]; + +const ok = (body: unknown = {}) => + new Response(JSON.stringify(body), { status: 200 }); + +describe("RestDrsProvider", () => { + let fetchStub: sinon.SinonStub; + + beforeEach(() => { + fetchStub = sinon.stub(window, "fetch").callsFake(async () => ok()); + }); + + afterEach(() => { + sinon.restore(); + }); + + CALLS.forEach(([name, call, url]) => { + it(`${name} requests ${url.slice(BASE_URL.length)}`, async () => { + await call(new RestDrsProvider(BASE_URL)); + + expect(fetchStub.callCount).to.equal(1); + expect(fetchStub.firstCall.args[0]).to.equal(url); + }); + + it(`${name} throws on an error response`, async () => { + fetchStub.callsFake( + async () => new Response("{}", { status: 500, statusText: "Oops" }) + ); + + let error: Error | undefined; + try { + await call(new RestDrsProvider(BASE_URL)); + } catch (e) { + error = e as Error; + } + + expect(error?.message).to.match(/^Failed to fetch .*: Oops$/); + }); + + it(`${name} sends its request through an overridden fetch`, async () => { + await call(new TokenDrsProvider(BASE_URL)); + + const headers = new Headers(fetchStub.firstCall.args[1]?.headers); + expect(headers.get("Authorization")).to.equal("Bearer test-token"); + }); + }); + + it("getObjects adds paging and returns objects and pagination", async () => { + const pagination = { offset: 20, limit: 10, total: 42 }; + fetchStub.callsFake(async () => + ok({ objects: [{ id: "obj-1" }], pagination }) + ); + + const result = await new RestDrsProvider(BASE_URL).getObjects(10, 20); + + expect(fetchStub.firstCall.args[0]).to.equal( + `${BASE_URL}/objects?offset=20&limit=10` + ); + expect(result).to.deep.equal({ objects: [{ id: "obj-1" }], pagination }); + }); + + it("getObjects returns no objects for a response without any", async () => { + const result = await new RestDrsProvider(BASE_URL).getObjects(); + + expect(result).to.deep.equal({ objects: [], pagination: undefined }); + }); + + it("getObject asks for expanded bundles", async () => { + await new RestDrsProvider(BASE_URL).getObject("obj-1", true); + + expect(fetchStub.firstCall.args[0]).to.equal( + `${BASE_URL}/objects/obj-1?expand=true` + ); + }); +}); diff --git a/packages/ecc-client-ga4gh-service-registry/src/providers/tests/rest-sr-provider.test.ts b/packages/ecc-client-ga4gh-service-registry/src/providers/tests/rest-sr-provider.test.ts new file mode 100644 index 00000000..03dfeb81 --- /dev/null +++ b/packages/ecc-client-ga4gh-service-registry/src/providers/tests/rest-sr-provider.test.ts @@ -0,0 +1,83 @@ +/* eslint-disable import/no-extraneous-dependencies */ +import { expect } from "@open-wc/testing"; +import sinon from "sinon"; +import { RestServiceRegistryProvider } from "../rest-sr-provider.js"; + +const BASE_URL = "https://registry.example.org/ga4gh/registry/v1"; + +/** Provider that adds a bearer token to every request, as in the docs. */ +class TokenServiceRegistryProvider extends RestServiceRegistryProvider { + protected fetch(input: RequestInfo | URL, init: RequestInit = {}) { + const headers = new Headers(init.headers); + headers.set("Authorization", "Bearer test-token"); + return super.fetch(input, { ...init, headers }); + } +} + +type Call = [ + string, + (provider: RestServiceRegistryProvider) => Promise, + string +]; + +/** Every provider method, with the URL it requests. */ +const CALLS: Call[] = [ + ["getServices", (p) => p.getServices(), `${BASE_URL}/services`], + [ + "getServiceById", + (p) => p.getServiceById("org/service 1"), + `${BASE_URL}/services/org%2Fservice%201`, + ], + ["getServiceTypes", (p) => p.getServiceTypes(), `${BASE_URL}/services/types`], + ["getServiceInfo", (p) => p.getServiceInfo(), `${BASE_URL}/service-info`], +]; + +describe("RestServiceRegistryProvider", () => { + let fetchStub: sinon.SinonStub; + + beforeEach(() => { + fetchStub = sinon + .stub(window, "fetch") + .callsFake(async () => new Response("{}", { status: 200 })); + }); + + afterEach(() => { + sinon.restore(); + }); + + CALLS.forEach(([name, call, url]) => { + it(`${name} requests ${url.slice(BASE_URL.length)}`, async () => { + fetchStub.callsFake( + async () => new Response(JSON.stringify({ name }), { status: 200 }) + ); + + const result = await call(new RestServiceRegistryProvider(BASE_URL)); + + expect(fetchStub.callCount).to.equal(1); + expect(fetchStub.firstCall.args[0]).to.equal(url); + expect(result).to.deep.equal({ name }); + }); + + it(`${name} throws on an error response`, async () => { + fetchStub.callsFake( + async () => new Response("{}", { status: 500, statusText: "Oops" }) + ); + + let error: Error | undefined; + try { + await call(new RestServiceRegistryProvider(BASE_URL)); + } catch (e) { + error = e as Error; + } + + expect(error?.message).to.match(/^Failed to fetch .*: Oops$/); + }); + + it(`${name} sends its request through an overridden fetch`, async () => { + await call(new TokenServiceRegistryProvider(BASE_URL)); + + const headers = new Headers(fetchStub.firstCall.args[1]?.headers); + expect(headers.get("Authorization")).to.equal("Bearer test-token"); + }); + }); +}); diff --git a/packages/ecc-client-ga4gh-trs/src/providers/tests/rest-trs-provider.test.ts b/packages/ecc-client-ga4gh-trs/src/providers/tests/rest-trs-provider.test.ts new file mode 100644 index 00000000..91f48a70 --- /dev/null +++ b/packages/ecc-client-ga4gh-trs/src/providers/tests/rest-trs-provider.test.ts @@ -0,0 +1,236 @@ +/* eslint-disable import/no-extraneous-dependencies */ +import { expect } from "@open-wc/testing"; +import sinon from "sinon"; +import { RestTrsProvider } from "../rest-trs-provider.js"; + +const BASE_URL = "https://trs.example.org/ga4gh/trs/v2"; +const TOOL = `${BASE_URL}/tools/tool%201`; + +/** Provider that adds a bearer token to every request, as in the docs. */ +class TokenTrsProvider extends RestTrsProvider { + protected fetch(input: RequestInfo | URL, init: RequestInit = {}) { + const headers = new Headers(init.headers); + headers.set("Authorization", "Bearer test-token"); + return super.fetch(input, { ...init, headers }); + } +} + +type Call = [string, (provider: RestTrsProvider) => Promise, string]; + +/** Every provider method, with the URL it requests and its error message. */ +const CALLS: Call[] = [ + ["getToolClasses", (p) => p.getToolClasses(), `${BASE_URL}/toolClasses`], + ["getTool", (p) => p.getTool(BASE_URL, "tool 1"), TOOL], + [ + "getToolVersions", + (p) => p.getToolVersions(BASE_URL, "tool 1"), + `${TOOL}/versions`, + ], + [ + "getToolVersion", + (p) => p.getToolVersion(BASE_URL, "tool 1", "tool 1:v1"), + `${TOOL}/versions/v1`, + ], + [ + "getToolFiles", + (p) => p.getToolFiles(BASE_URL, "tool 1", "v1", "CWL"), + `${TOOL}/versions/v1/CWL/files`, + ], + [ + "getToolDescriptor", + (p) => p.getToolDescriptor(BASE_URL, "tool 1", "tool 1:v1", "WDL"), + `${TOOL}/versions/v1/WDL/descriptor`, + ], + [ + "getContainerfile", + (p) => p.getContainerfile(BASE_URL, "tool 1", "v1"), + `${TOOL}/versions/v1/containerfile`, + ], + [ + "getToolTests", + (p) => p.getToolTests(BASE_URL, "tool 1", "tool 1:v1", "NFL"), + `${TOOL}/versions/v1/NFL/tests`, + ], +]; + +const ok = (body: unknown = {}) => + new Response(JSON.stringify(body), { status: 200 }); +const failed = () => + new Response("{}", { status: 404, statusText: "Not Found" }); + +describe("RestTrsProvider", () => { + let fetchStub: sinon.SinonStub; + + beforeEach(() => { + fetchStub = sinon.stub(window, "fetch").callsFake(async () => ok()); + }); + + afterEach(() => { + sinon.restore(); + }); + + CALLS.forEach(([name, call, url]) => { + it(`${name} requests ${url.slice(BASE_URL.length)}`, async () => { + fetchStub.callsFake(async () => ok({ name })); + + const result = await call(new RestTrsProvider(BASE_URL)); + + expect(fetchStub.callCount).to.equal(1); + expect(fetchStub.firstCall.args[0]).to.equal(url); + expect(result).to.deep.equal({ name }); + }); + + it(`${name} throws on an error response`, async () => { + fetchStub.callsFake(async () => failed()); + + let error: Error | undefined; + try { + await call(new RestTrsProvider(BASE_URL)); + } catch (e) { + error = e as Error; + } + + expect(error?.message).to.match(/^Failed to fetch .*: Not Found$/); + }); + + it(`${name} sends its request through an overridden fetch`, async () => { + await call(new TokenTrsProvider(BASE_URL)); + + const headers = new Headers(fetchStub.firstCall.args[1]?.headers); + expect(headers.get("Authorization")).to.equal("Bearer test-token"); + }); + }); + + it("getToolFiles asks for a ZIP archive", async () => { + await new RestTrsProvider(BASE_URL).getToolFiles( + BASE_URL, + "tool 1", + "v1", + "CWL", + "zip" + ); + + expect(fetchStub.firstCall.args[0]).to.equal( + `${TOOL}/versions/v1/CWL/files?format=zip` + ); + }); + + describe("getToolsList", () => { + it("requests the first page without parameters", async () => { + await new RestTrsProvider(BASE_URL).getToolsList(0, 0, {}, ""); + + expect(fetchStub.firstCall.args[0]).to.equal(`${BASE_URL}/tools?`); + }); + + it("adds paging, the query and set filters", async () => { + await new RestTrsProvider(BASE_URL).getToolsList( + 10, + 20, + { + offset: "5", + toolClass: "Workflow", + checker: false, + author: "", + organization: undefined, + }, + "my tool" + ); + + expect(fetchStub.firstCall.args[0]).to.equal( + `${BASE_URL}/tools?limit=10&offset=20&` + + "&name=my%20tool&toolClass=Workflow&checker=false" + ); + }); + + it("throws on an error response", async () => { + fetchStub.callsFake(async () => failed()); + + let error: Error | undefined; + try { + await new RestTrsProvider(BASE_URL).getToolsList(10, 0, {}, ""); + } catch (e) { + error = e as Error; + } + + expect(error?.message).to.equal("Failed to fetch tools: Not Found"); + }); + }); + + describe("getToolDescriptorByPath", () => { + const DESCRIPTOR = `${TOOL}/versions/v1/CWL/descriptor`; + const byPath = (provider: RestTrsProvider) => + provider.getToolDescriptorByPath( + BASE_URL, + "tool 1", + "tool 1:v1", + "CWL", + "dir/main.cwl" + ); + + it("requests the unencoded path first", async () => { + fetchStub.callsFake(async () => ok({ content: "plain" })); + + const result = await byPath(new RestTrsProvider(BASE_URL)); + + expect(fetchStub.callCount).to.equal(1); + expect(fetchStub.firstCall.args[0]).to.equal( + `${DESCRIPTOR}/dir/main.cwl` + ); + expect(result).to.deep.equal({ content: "plain" }); + }); + + it("falls back to the encoded path after an error response", async () => { + fetchStub.onFirstCall().callsFake(async () => failed()); + fetchStub.onSecondCall().callsFake(async () => ok({ content: "enc" })); + + const result = await byPath(new RestTrsProvider(BASE_URL)); + + expect(fetchStub.callCount).to.equal(2); + expect(fetchStub.secondCall.args[0]).to.equal( + `${DESCRIPTOR}/dir%2Fmain.cwl` + ); + expect(result).to.deep.equal({ content: "enc" }); + }); + + it("falls back to the encoded path after a network error", async () => { + fetchStub.onFirstCall().rejects(new TypeError("Network down")); + fetchStub.onSecondCall().callsFake(async () => ok({ content: "enc" })); + + const result = await byPath(new RestTrsProvider(BASE_URL)); + + expect(fetchStub.secondCall.args[0]).to.equal( + `${DESCRIPTOR}/dir%2Fmain.cwl` + ); + expect(result).to.deep.equal({ content: "enc" }); + }); + + it("throws the first error if both paths fail", async () => { + fetchStub.callsFake(async () => failed()); + + let error: Error | undefined; + try { + await byPath(new RestTrsProvider(BASE_URL)); + } catch (e) { + error = e as Error; + } + + expect(error?.message).to.equal( + "Failed to fetch tool descriptor by path: Not Found" + ); + }); + + it("throws the network error if the encoded path fails too", async () => { + fetchStub.onFirstCall().rejects(new TypeError("Network down")); + fetchStub.onSecondCall().callsFake(async () => failed()); + + let error: Error | undefined; + try { + await byPath(new RestTrsProvider(BASE_URL)); + } catch (e) { + error = e as Error; + } + + expect(error?.message).to.equal("Network down"); + }); + }); +});