Skip to content

Commit

Permalink
chore: Don't duplicate the URL parsing Regexes (#34788)
Browse files Browse the repository at this point in the history
This code duplication meant that we missed updating the regexes to the
ones that support UUIDs, and so the caching functionality broke.

This PR removes the duplication and imports the regex constants from a
single place.

/test sanity


<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/9850040595>
> Commit: d8261a1
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9850040595&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Sanity`
> Spec:
> <hr>Tue, 09 Jul 2024 03:00:41 UTC
<!-- end of auto-generated comment: Cypress test results  -->


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Refactor**
- Improved maintainability by importing path constants from a
centralized location.
- Replaced hardcoded `pageId` and `applicationId` with variables for
better code clarity and flexibility.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
sharat87 committed Jul 9, 2024
1 parent 0941014 commit d64ddee
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 52 deletions.
90 changes: 45 additions & 45 deletions app/client/src/ce/utils/serviceWorkerUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,65 +18,64 @@ import { Request as NFRequest, Response as NFResponse } from "node-fetch";
delete: jest.fn(),
};

const applicationId = "b0123456789abcdef0000000";
const pageId = "a0123456789abcdef0000000";

describe("serviceWorkerUtils", () => {
describe("matchBuilderPath", () => {
it("should match the standard builder path", () => {
const pathName = "/app/applicationSlug/pageSlug-123/edit";
const pathName = `/app/applicationSlug/pageSlug-${pageId}/edit`;
const options = { end: false };
const result = matchBuilderPath(pathName, options);

expect(result).toBeTruthy();
if (result) {
expect(result.params).toHaveProperty("applicationSlug");
expect(result.params).toHaveProperty("pageSlug");
expect(result.params).toHaveProperty("pageId", "123");
expect(result.params).toHaveProperty("pageId", pageId);
} else {
fail("Expected result to be truthy");
}
});

it("should match the standard builder path for alphanumeric pageId", () => {
const pathName =
"/app/applicationSlug/pageSlug-6616733a6e70274710f21a07/edit";
const pathName = `/app/applicationSlug/pageSlug-${pageId}/edit`;
const options = { end: false };
const result = matchBuilderPath(pathName, options);

expect(result).toBeTruthy();
if (result) {
expect(result.params).toHaveProperty("applicationSlug");
expect(result.params).toHaveProperty("pageSlug");
expect(result.params).toHaveProperty(
"pageId",
"6616733a6e70274710f21a07",
);
expect(result.params).toHaveProperty("pageId", pageId);
} else {
fail("Expected result to be truthy");
}
});

it("should match the custom builder path", () => {
const pathName = "/app/customSlug-custom-456/edit";
const pathName = `/app/customSlug-custom-${pageId}/edit`;
const options = { end: false };
const result = matchBuilderPath(pathName, options);

expect(result).toBeTruthy();
if (result) {
expect(result.params).toHaveProperty("customSlug");
expect(result.params).toHaveProperty("pageId", "456");
expect(result.params).toHaveProperty("pageId", pageId);
} else {
fail("Expected result to be truthy");
}
});

it("should match the deprecated builder path", () => {
const pathName = "/applications/appId123/pages/456/edit";
const pathName = `/applications/${applicationId}/pages/${pageId}/edit`;
const options = { end: false };
const result = matchBuilderPath(pathName, options);

expect(result).toBeTruthy();
if (result) {
expect(result.params).toHaveProperty("applicationId", "appId123");
expect(result.params).toHaveProperty("pageId", "456");
expect(result.params).toHaveProperty("applicationId", applicationId);
expect(result.params).toHaveProperty("pageId", pageId);
} else {
fail("Expected result to be truthy");
}
Expand All @@ -99,15 +98,14 @@ describe("serviceWorkerUtils", () => {
});

it("should match when the path is edit widgets", () => {
const pathName =
"/app/applicationSlug/pageSlug-123/edit/widgets/t36hb2zukr";
const pathName = `/app/applicationSlug/pageSlug-${pageId}/edit/widgets/t36hb2zukr`;
const options = { end: false };
const result = matchBuilderPath(pathName, options);

if (result) {
expect(result.params).toHaveProperty("applicationSlug");
expect(result.params).toHaveProperty("pageSlug");
expect(result.params).toHaveProperty("pageId", "123");
expect(result.params).toHaveProperty("pageId", pageId);
} else {
fail("Expected result to be truthy");
}
Expand All @@ -116,40 +114,40 @@ describe("serviceWorkerUtils", () => {

describe("matchViewerPath", () => {
it("should match the standard viewer path", () => {
const pathName = "/app/applicationSlug/pageSlug-123";
const pathName = `/app/applicationSlug/pageSlug-${pageId}`;
const result = matchViewerPath(pathName);

expect(result).toBeTruthy();
if (result) {
expect(result.params).toHaveProperty("applicationSlug");
expect(result.params).toHaveProperty("pageSlug");
expect(result.params).toHaveProperty("pageId", "123");
expect(result.params).toHaveProperty("pageId", pageId);
} else {
fail("Expected result to be truthy");
}
});

it("should match the custom viewer path", () => {
const pathName = "/app/customSlug-custom-456";
const pathName = `/app/customSlug-custom-${pageId}`;
const result = matchViewerPath(pathName);

expect(result).toBeTruthy();
if (result) {
expect(result.params).toHaveProperty("customSlug");
expect(result.params).toHaveProperty("pageId", "456");
expect(result.params).toHaveProperty("pageId", pageId);
} else {
fail("Expected result to be truthy");
}
});

it("should match the deprecated viewer path", () => {
const pathName = "/applications/appId123/pages/456";
const pathName = `/applications/${applicationId}/pages/${pageId}`;
const result = matchViewerPath(pathName);

expect(result).toBeTruthy();
if (result) {
expect(result.params).toHaveProperty("applicationId", "appId123");
expect(result.params).toHaveProperty("pageId", "456");
expect(result.params).toHaveProperty("applicationId", applicationId);
expect(result.params).toHaveProperty("pageId", pageId);
} else {
fail("Expected result to be truthy");
}
Expand Down Expand Up @@ -189,11 +187,11 @@ describe("serviceWorkerUtils", () => {
describe("getApplicationParamsFromUrl", () => {
it("should parse URL and return correct params for builder path", () => {
const url = new URL(
"https://app.appsmith.com/app/my-app/page-123/edit?branch=main",
`https://app.appsmith.com/app/my-app/page-${pageId}/edit?branch=main`,
);
const expectedParams: TApplicationParams = {
origin: "https://app.appsmith.com",
pageId: "123",
pageId,
applicationId: undefined,
branchName: "main",
appMode: APP_MODE.EDIT,
Expand All @@ -204,11 +202,11 @@ describe("serviceWorkerUtils", () => {

it("should parse URL and return correct params for viewer path", () => {
const url = new URL(
"https://app.appsmith.com/app/my-app/page-123?branch=main",
`https://app.appsmith.com/app/my-app/page-${pageId}?branch=main`,
);
const expectedParams: TApplicationParams = {
origin: "https://app.appsmith.com",
pageId: "123",
pageId,
applicationId: undefined,
branchName: "main",
appMode: APP_MODE.PUBLISHED,
Expand All @@ -224,12 +222,12 @@ describe("serviceWorkerUtils", () => {

it("should parse deprecated builder path and return correct params", () => {
const url = new URL(
"https://app.appsmith.com/applications/app-123/pages/page-123/edit?branch=main",
`https://app.appsmith.com/applications/${applicationId}/pages/${pageId}/edit?branch=main`,
);
const expectedParams: TApplicationParams = {
origin: "https://app.appsmith.com",
pageId: "page-123",
applicationId: "app-123",
pageId,
applicationId,
branchName: "main",
appMode: APP_MODE.EDIT,
};
Expand All @@ -239,12 +237,12 @@ describe("serviceWorkerUtils", () => {

it("should parse deprecated viewer path and return correct params", () => {
const url = new URL(
"https://app.appsmith.com/applications/app-123/pages/page-123?branch=main",
`https://app.appsmith.com/applications/${applicationId}/pages/${pageId}?branch=main`,
);
const expectedParams: TApplicationParams = {
origin: "https://app.appsmith.com",
pageId: "page-123",
applicationId: "app-123",
pageId,
applicationId,
branchName: "main",
appMode: APP_MODE.PUBLISHED,
};
Expand All @@ -254,11 +252,11 @@ describe("serviceWorkerUtils", () => {

it("should parse custom builder path and return correct params", () => {
const url = new URL(
"https://app.appsmith.com/app/custom-app-123/edit?branch=main",
`https://app.appsmith.com/app/custom-app-${pageId}/edit?branch=main`,
);
const expectedParams: TApplicationParams = {
origin: "https://app.appsmith.com",
pageId: "123",
pageId,
applicationId: undefined,
branchName: "main",
appMode: APP_MODE.EDIT,
Expand All @@ -269,11 +267,11 @@ describe("serviceWorkerUtils", () => {

it("should parse custom viewer path and return correct params", () => {
const url = new URL(
"https://app.appsmith.com/app/custom-app-123?branch=main",
`https://app.appsmith.com/app/custom-app-${pageId}?branch=main`,
);
const expectedParams: TApplicationParams = {
origin: "https://app.appsmith.com",
pageId: "123",
pageId,
applicationId: undefined,
branchName: "main",
appMode: APP_MODE.PUBLISHED,
Expand All @@ -283,10 +281,12 @@ describe("serviceWorkerUtils", () => {
});

it("should parse URL and return params with empty branch name if branch query param is not present", () => {
const url = new URL("https://app.appsmith.com/app/my-app/page-123/edit");
const url = new URL(
`https://app.appsmith.com/app/my-app/page-${pageId}/edit`,
);
const expectedParams: TApplicationParams = {
origin: "https://app.appsmith.com",
pageId: "123",
pageId,
applicationId: undefined,
branchName: "",
appMode: APP_MODE.EDIT,
Expand Down Expand Up @@ -314,16 +314,16 @@ describe("serviceWorkerUtils", () => {
it("should create request for EDIT mode with applicationId", () => {
const params: TApplicationParams = {
origin: "https://app.appsmith.com",
pageId: "page123",
applicationId: "app123",
pageId,
applicationId,
branchName: "main",
appMode: APP_MODE.EDIT,
};

const request = getConsolidatedApiPrefetchRequest(params);
expect(request).toBeInstanceOf(Request);
expect(request?.url).toBe(
`https://app.appsmith.com/api/v1/consolidated-api/edit?defaultPageId=page123&applicationId=app123`,
`https://app.appsmith.com/api/v1/consolidated-api/edit?defaultPageId=${pageId}&applicationId=${applicationId}`,
);
expect(request?.method).toBe("GET");
expect(request?.headers.get("Branchname")).toBe("main");
Expand All @@ -332,16 +332,16 @@ describe("serviceWorkerUtils", () => {
it("should create request for PUBLISHED mode with applicationId", () => {
const params: TApplicationParams = {
origin: "https://app.appsmith.com",
pageId: "page123",
applicationId: "app123",
pageId,
applicationId,
branchName: "main",
appMode: APP_MODE.PUBLISHED,
};

const request = getConsolidatedApiPrefetchRequest(params);
expect(request).toBeInstanceOf(Request);
expect(request?.url).toBe(
`https://app.appsmith.com/api/v1/consolidated-api/view?defaultPageId=page123&applicationId=app123`,
`https://app.appsmith.com/api/v1/consolidated-api/view?defaultPageId=${pageId}&applicationId=${applicationId}`,
);
expect(request?.method).toBe("GET");
expect(request?.headers.get("Branchname")).toBe("main");
Expand Down
15 changes: 8 additions & 7 deletions app/client/src/ce/utils/serviceWorkerUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ import { Mutex } from "async-mutex";
import { APP_MODE } from "entities/App";
import type { Match, TokensToRegexpOptions } from "path-to-regexp";
import { match } from "path-to-regexp";

const BUILDER_PATH = `/app/:applicationSlug/:pageSlug(.*\-):pageId/edit`;
const BUILDER_CUSTOM_PATH = `/app/:customSlug(.*\-):pageId/edit`;
const VIEWER_PATH = `/app/:applicationSlug/:pageSlug(.*\-):pageId`;
const VIEWER_CUSTOM_PATH = `/app/:customSlug(.*\-):pageId`;
const BUILDER_PATH_DEPRECATED = `/applications/:applicationId/pages/:pageId/edit`;
const VIEWER_PATH_DEPRECATED = `/applications/:applicationId/pages/:pageId`;
import {
BUILDER_PATH,
BUILDER_CUSTOM_PATH,
VIEWER_PATH,
VIEWER_CUSTOM_PATH,
BUILDER_PATH_DEPRECATED,
VIEWER_PATH_DEPRECATED,
} from "@appsmith/constants/routes/appRoutes";

interface TMatchResult {
pageId?: string;
Expand Down

0 comments on commit d64ddee

Please sign in to comment.