Skip to content

Commit

Permalink
CMR-10147: Removing query parameters from self url when generating Ch…
Browse files Browse the repository at this point in the history
…ild links (#362)

* CMR-10147: Removing query parameters from self links

* CMR-10147: Adding spec tests

* CMR-147: Addressing PR comments, adding common getBaseUrl function

* CMR-10147: Adding timeout back to test as it fails in github

* CMR-10147: Adjusting test

* CMR-10147: Fixing test to use entryId

* CMR-10147: Removing timeout from test
  • Loading branch information
william-valencia authored Oct 15, 2024
1 parent 57e86a3 commit 92565e4
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 11 deletions.
36 changes: 36 additions & 0 deletions src/__tests__/providerCatalog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,42 @@ describe("GET /:provider", () => {
});
});
});

describe(`given the provider has a collection`, () => {
it("has a child link for that collection without query parameters", async function () {
sandbox
.stub(Provider, "getProviders")
.resolves([null, [{ "provider-id": "TEST", "short-name": "TEST" }]]);

const mockCollections = generateSTACCollections(1);
sandbox.stub(Collections, "getCollectionIds").resolves({
count: mockCollections.length,
cursor: "foundCursor",
items: mockCollections.map((coll) => ({
id: `${coll.id}`,
title: coll.title ?? faker.random.words(4),
})),
});

const { body: catalog } = await request(stacApp).get("/stac/TEST?param=value");

const children = catalog.links.filter((l: Link) => l.rel === "child");
expect(children).to.have.length(mockCollections.length);

mockCollections.forEach((collection) => {
const childLink = children.find((l: Link) => l.href.endsWith(collection.id));

expect(childLink, JSON.stringify(children, null, 2)).to.have.property(
"type",
"application/json"
);
expect(childLink.href, JSON.stringify(childLink, null, 2)).to.not.contain("?param=value");
expect(childLink.href, JSON.stringify(childLink, null, 2)).to.match(
/^https?:\/\/.*TEST\/collections/
);
});
});
});
});

describe("given CMR providers endpoint responds with an error", () => {
Expand Down
19 changes: 18 additions & 1 deletion src/__tests__/rootCatalog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe("GET /stac", () => {
});

describe("given CMR responds with providers", () => {
before(() => {
beforeEach(() => {
sandbox
.stub(Providers, "getProviders")
.resolves([null, [{ "provider-id": "TEST", "short-name": "TEST" }]]);
Expand All @@ -76,6 +76,23 @@ describe("GET /stac", () => {
expect(providerLink.title).to.equal(provider["provider-id"]);
});
});

it("should have an entry for each provider in the links without query parameters", async () => {
const { statusCode, body } = await request(app).get("/stac?param=value");

expect(statusCode).to.equal(200);
const [, expectedProviders] = cmrProvidersResponse;

expectedProviders!.forEach((provider) => {
const providerLink = body.links.find((l: Link) => l.href.includes(provider["provider-id"]));

expect(providerLink.href).to.match(/^(http)s?:\/\/.*\w+/);
expect(providerLink.href).to.not.contain("?param=value");
expect(providerLink.rel).to.equal("child");
expect(providerLink.type).to.equal("application/json");
expect(providerLink.title).to.equal(provider["provider-id"]);
});
});
});

describe("given CMR providers endpoint responds with an error", () => {
Expand Down
9 changes: 3 additions & 6 deletions src/routes/browse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Links } from "../@types/StacCatalog";
import { getCollections } from "../domains/collections";
import { buildQuery, stringifyQuery } from "../domains/stac";
import { ItemNotFound } from "../models/errors";
import { mergeMaybe, stacContext } from "../utils";
import { getBaseUrl, mergeMaybe, stacContext } from "../utils";

const collectionLinks = (req: Request, nextCursor?: string | null): Links => {
const { stacRoot, self, path } = stacContext(req);
Expand Down Expand Up @@ -64,13 +64,10 @@ export const collectionsHandler = async (req: Request, res: Response): Promise<v

const { stacRoot, self } = stacContext(req);

// Remove query parameters from the URL, keeping only the base path
const baseUrl = self.replace(/\?.*$/, "");

collections.forEach((collection) => {
collection.links.push({
rel: "self",
href: `${baseUrl}/${encodeURIComponent(collection.id)}`,
href: `${getBaseUrl(self)}/${encodeURIComponent(collection.id)}`,
type: "application/json",
});
collection.links.push({
Expand All @@ -92,7 +89,7 @@ export const collectionsHandler = async (req: Request, res: Response): Promise<v
if (!itemsLink) {
collection.links.push({
rel: "items",
href: `${baseUrl}/${encodeURIComponent(collection.id)}/items`,
href: `${getBaseUrl(self)}/${encodeURIComponent(collection.id)}/items`,
type: "application/json",
});
}
Expand Down
4 changes: 2 additions & 2 deletions src/routes/catalog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Links, STACCatalog } from "../@types/StacCatalog";
import { getAllCollectionIds } from "../domains/collections";
import { conformance } from "../domains/providers";
import { ServiceUnavailableError } from "../models/errors";
import { mergeMaybe, stacContext } from "../utils";
import { getBaseUrl, mergeMaybe, stacContext } from "../utils";

const STAC_VERSION = process.env.STAC_VERSION ?? "1.0.0";

Expand Down Expand Up @@ -105,7 +105,7 @@ export const providerCatalogHandler = async (req: Request, res: Response) => {
const selfLinks = generateSelfLinks(req);
const childLinks = (collections ?? []).map(({ id, title }) => ({
rel: "child",
href: `${self}/collections/${encodeURIComponent(id)}`,
href: `${getBaseUrl(self)}/collections/${encodeURIComponent(id)}`,
title,
type: "application/json",
}));
Expand Down
4 changes: 2 additions & 2 deletions src/routes/rootCatalog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Link, STACCatalog } from "../@types/StacCatalog";

import { conformance } from "../domains/providers";
import { Provider } from "../models/CmrModels";
import { stacContext } from "../utils";
import { getBaseUrl, stacContext } from "../utils";

const STAC_VERSION = process.env.STAC_VERSION ?? "1.0.0";

Expand Down Expand Up @@ -45,7 +45,7 @@ const providerLinks = (req: Request, providers: Provider[]): Link[] => {
rel: "child",
title,
type: "application/json",
href: `${self}/${providerId}`,
href: `${getBaseUrl(self)}/${providerId}`,
}));
};

Expand Down
8 changes: 8 additions & 0 deletions src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,11 @@ export const generatePossibleCollectionIds = (id: string, separator: string, rep

return tokensCopy.join(separator);
});

/**
* Returns the base URL
* This is used to remove Query Parameters that may have been added to the URL.
*/
export const getBaseUrl = (url: string) => {
return url.replace(/\?.*$/, "");
};

0 comments on commit 92565e4

Please sign in to comment.