Skip to content

Commit

Permalink
fix(): constructSlug accounts for typekeyword max length of 256 chars (…
Browse files Browse the repository at this point in the history 
…#1410)
  • Loading branch information
@rweber-esri
rweber-esri authored Feb 14, 2024
1 parent a48851a commit c7465a0
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 45 deletions.
48 changes: 30 additions & 18 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

74 changes: 50 additions & 24 deletions packages/common/src/items/slugs.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,63 @@ import { IItem } from "@esri/arcgis-rest-types";
import { slugify } from "../utils";
import { uriSlugToKeywordSlug } from "./_internal/slugConverters";

const TYPEKEYWORD_SLUG_PREFIX = "slug";

const TYPEKEYWORD_MAX_LENGTH = 256;

/**
* Create a slug, namespaced to an org
* Typically used to lookup items by a human readable name in urls
* Create a slug, namespaced to an org and accounting for the 256 character limit
* of individual typekeywords. Typically used to lookup items by a human readable name in urls
*
* @param title
* @param orgKey
* @returns
*/
export function constructSlug(title: string, orgKey: string) {
return `${orgKey.toLowerCase()}|${slugify(title)}`;
// typekeywords have a max length of 256 characters, so we use the slug
// format that gets persisted in typekeywords as our basis
return (
[
// add the typekeyword slug prefix
TYPEKEYWORD_SLUG_PREFIX,
// add the orgKey segment
orgKey.toLowerCase(),
// add the slugified title segment
slugify(title),
]
.join("|")
// allow some padding at the end for incrementing so we don't wind up w/ weird, inconsistent slugs
// when the increment goes from single to multiple digits, i.e. avoid producing the following when
// deduping:
// slug|qa-pre-a-hub|some-really-really-...-really-long
// slug|qa-pre-a-hub|some-really-really-...-really-lo-1
// slug|qa-pre-a-hub|some-really-really-...-really-l-11
// slug|qa-pre-a-hub|some-really-really-...-really-100
.substring(0, TYPEKEYWORD_MAX_LENGTH - 4)
// removing tailing hyphens
.replace(/-+$/, "")
// remove typekeyword slug prefix, it's re-added in setSlugKeyword
.replace(new RegExp(`^${TYPEKEYWORD_SLUG_PREFIX}\\|`), "")
);
}

/**
* Adds/Updates the slug typekeyword
* Returns a new array of keywords
* @param typeKeywords
* @param slug
* @returns
*
* @param typeKeywords A collection of typekeywords
* @param slug The slug to add/update
* @returns An updated collection of typekeywords
*/
export function setSlugKeyword(typeKeywords: string[], slug: string): string[] {
// remove slug entry from array
const removed = typeKeywords.filter((entry: string) => {
return !entry.startsWith("slug|");
});
const updatedTypekeywords = typeKeywords.filter(
(entry: string) => !entry.startsWith(`${TYPEKEYWORD_SLUG_PREFIX}|`)
);

// now add it
removed.push(`slug|${slug}`);
return removed;
updatedTypekeywords.push([TYPEKEYWORD_SLUG_PREFIX, slug].join("|"));
return updatedTypekeywords;
}

/**
Expand Down Expand Up @@ -84,8 +113,11 @@ export function findItemsBySlug(
},
requestOptions: IRequestOptions
): Promise<IItem[]> {
const filter = slugInfo.slug.startsWith(`${TYPEKEYWORD_SLUG_PREFIX}|`)
? slugInfo.slug
: [TYPEKEYWORD_SLUG_PREFIX, slugInfo.slug].join("|");
const opts = {
filter: `typekeywords:"slug|${slugInfo.slug}"`,
filter: `typekeywords:"${filter}"`,
} as ISearchOptions;

if (requestOptions.authentication) {
Expand Down Expand Up @@ -130,22 +162,16 @@ export function getUniqueSlug(
requestOptions: IRequestOptions,
step: number = 0
): Promise<string> {
let combinedSlug = slugInfo.slug;
if (step) {
combinedSlug = `${slugInfo.slug}-${step}`;
}
const combinedSlug = step ? [slugInfo.slug, step].join("-") : slugInfo.slug;
return findItemsBySlug(
{ slug: combinedSlug, exclude: slugInfo.existingId },
requestOptions
)
.then((results) => {
if (results.length) {
step++;
return getUniqueSlug(slugInfo, requestOptions, step);
} else {
return combinedSlug;
}
})
.then((results) =>
!results.length
? combinedSlug
: getUniqueSlug(slugInfo, requestOptions, step + 1)
)
.catch((e) => {
throw Error(`Error in getUniqueSlug ${e}`);
});
Expand Down
70 changes: 67 additions & 3 deletions packages/common/test/items/slugs.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,24 @@ describe("slug utils: ", () => {
expect(slugModule.constructSlug("E2E Test Project", "qa-bas-hub")).toBe(
"qa-bas-hub|e2e-test-project"
);
expect(
slugModule.constructSlug(
"A really really really really really really really really really really really really really really really really really really really really really really really really really really really really really really really really really really long title",
"qa-bas-hub"
)
).toBe(
"qa-bas-hub|a-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-rea",
"does not exceed 256 chars taking into account slug prefix and orgKey"
);
expect(
slugModule.constructSlug(
"A really really really really really really really really really really really really really really really really really really really really really really really really really really really really really really really really reallllly really long title",
"qa-bas-hub"
)
).toBe(
"qa-bas-hub|a-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-really-reallllly",
"does not end with hyphen"
);
});
});
describe("setSlugKeyword:", () => {
Expand Down Expand Up @@ -156,6 +174,30 @@ describe("slug utils: ", () => {
expect(args.authentication).toBe(MOCK_AUTH);
});

it("does not add slug prefix when already present", async () => {
const searchSpy = spyOn(portalModule, "searchItems").and.returnValue(
Promise.resolve({
results: [
{ id: "3ef", title: "Fake", typeKeywords: ["one", "slug|foo-bar"] },
],
})
);

const results = await slugModule.findItemsBySlug(
{ slug: "slug|foo-bar", exclude: "bc3" },
{
authentication: MOCK_AUTH,
}
);
expect(results[0].id).toBe("3ef");
// check if
expect(searchSpy.calls.count()).toBe(1);
const args = searchSpy.calls.argsFor(0)[0] as unknown as ISearchOptions;
expect(args.filter).toBe(`typekeywords:"slug|foo-bar"`);
expect(args.q).toBe(`NOT id:bc3`);
expect(args.authentication).toBe(MOCK_AUTH);
});

it("passes an undefined q query when no exclusion is provided", async () => {
const searchSpy = spyOn(portalModule, "searchItems").and.returnValue(
Promise.resolve({
Expand Down Expand Up @@ -269,14 +311,34 @@ describe("slug utils: ", () => {
expect(args.authentication).toBe(MOCK_AUTH);
expect(slug).toBe("foo-bar");
});
it("excludes item with existingId", async () => {
const searchSpy = spyOn(portalModule, "searchItems").and.returnValue(
Promise.resolve({
results: [],
})
);
const slug = await slugModule.getUniqueSlug(
{ slug: "foo-bar", existingId: "31c" },
{
authentication: MOCK_AUTH,
}
);

expect(searchSpy.calls.count()).toBe(1);
const args = searchSpy.calls.argsFor(0)[0] as unknown as ISearchOptions;
expect(args.filter).toBe(`typekeywords:"slug|foo-bar"`);
expect(args.authentication).toBe(MOCK_AUTH);
expect(args.q).toBe("NOT id:31c");
expect(slug).toBe("foo-bar");
});
it("increments if item found", async () => {
let callCount = 0;
// semantics for jasmine spies are lacking
const searchSpy = spyOn(portalModule, "searchItems").and.callFake(() => {
const response = {
results: [] as portalModule.IItem[],
};
if (callCount === 0) {
if (callCount < 2) {
response.results.push({
id: "3ef",
title: "Fake",
Expand All @@ -293,12 +355,14 @@ describe("slug utils: ", () => {
authentication: MOCK_AUTH,
}
);
expect(slug).toBe("foo-bar-1");
expect(searchSpy.calls.count()).toBe(2);
expect(slug).toBe("foo-bar-2");
expect(searchSpy.calls.count()).toBe(3);
const args = searchSpy.calls.argsFor(0)[0] as unknown as ISearchOptions;
expect(args.filter).toBe(`typekeywords:"slug|foo-bar"`);
const args2 = searchSpy.calls.argsFor(1)[0] as unknown as ISearchOptions;
expect(args2.filter).toBe(`typekeywords:"slug|foo-bar-1"`);
const args3 = searchSpy.calls.argsFor(2)[0] as unknown as ISearchOptions;
expect(args3.filter).toBe(`typekeywords:"slug|foo-bar-2"`);
});
it("re-throws exceptions", async () => {
const searchSpy = spyOn(portalModule, "searchItems").and.returnValue(
Expand Down

0 comments on commit c7465a0

Please sign in to comment.