Skip to content

chore: Port ctl code to Typescript #37606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sharat87 merged 4 commits into from
Nov 21, 2024
Merged

chore: Port ctl code to Typescript #37606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6c27b22
chore: Port `ctl` code to Typescript
sharat87 Nov 21, 2024
2957cae
Fix tests
sharat87 Nov 21, 2024
9f1a246
chore: merge release branch
sharat87 Nov 21, 2024
e156dac
Prettier
sharat87 Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/client/packages/rts/build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const getWorkflowDependencies = () => {
const bundle = async () => {
return esbuild
.build({
entryPoints: ["src/server.ts", "src/ctl/index.js"],
entryPoints: ["src/server.ts", "src/ctl/index.ts"],
bundle: true,
sourcemap: true,
platform: "node",
Expand Down
2 changes: 2 additions & 0 deletions app/client/packages/rts/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
"devDependencies": {
"@types/express": "^4.17.14",
"@types/jest": "^29.2.3",
"@types/nodemailer": "^6.4.17",
"@types/readline-sync": "^1.4.8",
"jest": "^29.3.1",
"supertest": "^6.3.3",
"ts-jest": "29.1.0",
Expand Down
103 changes: 49 additions & 54 deletions ...lient/packages/rts/src/ctl/backup.test.js → ...lient/packages/rts/src/ctl/backup.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
const backup = require("./backup");
const Constants = require("./constants");
const os = require("os");
const fsPromises = require("fs/promises");
const utils = require("./utils");
const readlineSync = require("readline-sync");
jest.mock("./utils", () => ({
...jest.requireActual("./utils"),
execCommand: jest.fn().mockImplementation(async (a) => a.join(" ")),
}));

import * as backup from "./backup";
Copy link
Contributor

@trishaanand trishaanand Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we do * imports on the client side code base? Should we change this to particular func exports instead?

Copy link
Member Author

@sharat87 sharat87 Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do, actually. They're not nearly as bad as in Java. In Java, a * import pulls in tons of things into the namespace. But here, this * import only brings one name into the namespace.

Shot from our EE repo:
shot-2024-11-21-09-04-05

import * as Constants from "./constants";
import os from "os";
import fsPromises from "fs/promises";
import * as utils from "./utils";
import readlineSync from "readline-sync";

describe("Backup Tests", () => {
test("Timestamp string in ISO format", () => {
console.log(backup.getTimeStampInISO());
expect(backup.getTimeStampInISO()).toMatch(
/(\d{4})-(\d{2})-(\d{2})T(\d{2})\-(\d{2})\-(\d{2})\.(\d{3})Z/,
/(\d{4})-(\d{2})-(\d{2})T(\d{2})-(\d{2})-(\d{2})\.(\d{3})Z/,
);
});

Expand All @@ -18,23 +23,23 @@ describe("Backup Tests", () => {
res.toBeGreaterThan(1024 * 1024);
});

it("Checkx the constant is 2 GB", () => {
let size = 2 * 1024 * 1024 * 1024;
it("Check the constant is 2 GB", () => {
const size = 2 * 1024 * 1024 * 1024;
expect(Constants.MIN_REQUIRED_DISK_SPACE_IN_BYTES).toBe(size);
});

it("Should throw Error when the available size is below MIN_REQUIRED_DISK_SPACE_IN_BYTES", () => {
let size = Constants.MIN_REQUIRED_DISK_SPACE_IN_BYTES - 1;
const size = Constants.MIN_REQUIRED_DISK_SPACE_IN_BYTES - 1;
expect(() => backup.checkAvailableBackupSpace(size)).toThrow();
});

it("Should not hould throw Error when the available size is >= MIN_REQUIRED_DISK_SPACE_IN_BYTES", () => {
it("Should not should throw Error when the available size is >= MIN_REQUIRED_DISK_SPACE_IN_BYTES", () => {
expect(() => {
backup.checkAvailableBackupSpace(
Constants.MIN_REQUIRED_DISK_SPACE_IN_BYTES,
);
}).not.toThrow(
"Not enough space avaliable at /appsmith-stacks. Please ensure availability of atleast 5GB to backup successfully.",
"Not enough space available at /appsmith-stacks. Please ensure availability of at least 5GB to backup successfully.",
);
});

Expand All @@ -46,19 +51,18 @@ describe("Backup Tests", () => {
});

test("Test backup contents path generation", () => {
var root = "/rootDir";
var timestamp = "0000-00-0T00-00-00.00Z";
const root = "/rootDir";
const timestamp = "0000-00-0T00-00-00.00Z";
expect(backup.getBackupContentsPath(root, timestamp)).toBe(
"/rootDir/appsmith-backup-0000-00-0T00-00-00.00Z",
);
});

test("Test mongodump CMD generaton", async () => {
var dest = "/dest";
var appsmithMongoURI = "mongodb://username:password@host/appsmith";
var cmd =
test("Test mongodump CMD generation", async () => {
const dest = "/dest";
const appsmithMongoURI = "mongodb://username:password@host/appsmith";
const cmd =
Comment on lines +61 to +64
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove sensitive information from test cases

The test contains a MongoDB connection string with credentials in plain text. Consider using placeholder values or environment variables.

-    const appsmithMongoURI = "mongodb://username:password@host/appsmith";
+    const appsmithMongoURI = "mongodb://[username]:[password]@host/appsmith";

Committable suggestion skipped: line range outside the PR's diff.

"mongodump --uri=mongodb://username:password@host/appsmith --archive=/dest/mongodb-data.gz --gzip";
utils.execCommand = jest.fn().mockImplementation(async (a) => a.join(" "));
const res = await backup.executeMongoDumpCMD(dest, appsmithMongoURI);
expect(res).toBe(cmd);
console.log(res);
Expand All @@ -77,10 +81,9 @@ describe("Backup Tests", () => {
});

test("Test ln command generation", async () => {
var gitRoot = "/appsmith-stacks/git-storage";
var dest = "/destdir";
var cmd = "ln -s /appsmith-stacks/git-storage /destdir/git-storage";
utils.execCommand = jest.fn().mockImplementation(async (a) => a.join(" "));
const gitRoot = "/appsmith-stacks/git-storage";
const dest = "/destdir";
const cmd = "ln -s /appsmith-stacks/git-storage /destdir/git-storage";
const res = await backup.executeCopyCMD(gitRoot, dest);
expect(res).toBe(cmd);
console.log(res);
Expand Down Expand Up @@ -127,8 +130,8 @@ describe("Backup Tests", () => {
test("Cleanup Backups when limit is 4 and there are 5 files", async () => {
const backupArchivesLimit = 4;
fsPromises.rm = jest.fn().mockImplementation(async (a) => console.log(a));
var backupFiles = ["file1", "file2", "file3", "file4", "file5"];
var expectedBackupFiles = ["file2", "file3", "file4", "file5"];
const backupFiles = ["file1", "file2", "file3", "file4", "file5"];
const expectedBackupFiles = ["file2", "file3", "file4", "file5"];
const res = await backup.removeOldBackups(backupFiles, backupArchivesLimit);
console.log(res);

Expand All @@ -138,8 +141,8 @@ describe("Backup Tests", () => {
test("Cleanup Backups when limit is 2 and there are 5 files", async () => {
const backupArchivesLimit = 2;
fsPromises.rm = jest.fn().mockImplementation(async (a) => console.log(a));
var backupFiles = ["file1", "file2", "file3", "file4", "file5"];
var expectedBackupFiles = ["file4", "file5"];
const backupFiles = ["file1", "file2", "file3", "file4", "file5"];
const expectedBackupFiles = ["file4", "file5"];
const res = await backup.removeOldBackups(backupFiles, backupArchivesLimit);
console.log(res);

Expand All @@ -149,8 +152,8 @@ describe("Backup Tests", () => {
test("Cleanup Backups when limit is 4 and there are 4 files", async () => {
const backupArchivesLimit = 4;
fsPromises.rm = jest.fn().mockImplementation(async (a) => console.log(a));
var backupFiles = ["file1", "file2", "file3", "file4"];
var expectedBackupFiles = ["file1", "file2", "file3", "file4"];
const backupFiles = ["file1", "file2", "file3", "file4"];
const expectedBackupFiles = ["file1", "file2", "file3", "file4"];
const res = await backup.removeOldBackups(backupFiles, backupArchivesLimit);
console.log(res);

Expand All @@ -160,8 +163,8 @@ describe("Backup Tests", () => {
test("Cleanup Backups when limit is 4 and there are 2 files", async () => {
const backupArchivesLimit = 4;
fsPromises.rm = jest.fn().mockImplementation(async (a) => console.log(a));
var backupFiles = ["file1", "file2"];
var expectedBackupFiles = ["file1", "file2"];
const backupFiles = ["file1", "file2"];
const expectedBackupFiles = ["file1", "file2"];
const res = await backup.removeOldBackups(backupFiles, backupArchivesLimit);
console.log(res);

Expand All @@ -171,8 +174,8 @@ describe("Backup Tests", () => {
test("Cleanup Backups when limit is 2 and there is 1 file", async () => {
const backupArchivesLimit = 4;
fsPromises.rm = jest.fn().mockImplementation(async (a) => console.log(a));
var backupFiles = ["file1"];
var expectedBackupFiles = ["file1"];
const backupFiles = ["file1"];
const expectedBackupFiles = ["file1"];
const res = await backup.removeOldBackups(backupFiles, backupArchivesLimit);
console.log(res);
expect(res).toEqual(expectedBackupFiles);
Expand All @@ -181,24 +184,22 @@ describe("Backup Tests", () => {
test("Cleanup Backups when limit is 2 and there is no file", async () => {
const backupArchivesLimit = 4;
fsPromises.rm = jest.fn().mockImplementation(async (a) => console.log(a));
var backupFiles = [];
var expectedBackupFiles = [];
const backupFiles = [];
const expectedBackupFiles = [];
const res = await backup.removeOldBackups(backupFiles, backupArchivesLimit);
console.log(res);
expect(res).toEqual(expectedBackupFiles);
});

test("Test get encryption password from user prompt whene both passords are the same", async () => {
test("Test get encryption password from user prompt when both passwords are the same", async () => {
const password = "password#4321";
readlineSync.question = jest.fn().mockImplementation((a) => {
return password;
});
readlineSync.question = jest.fn().mockImplementation(() => password);
const password_res = backup.getEncryptionPasswordFromUser();

expect(password_res).toEqual(password);
});

test("Test get encryption password from user prompt when both passords are the different", async () => {
test("Test get encryption password from user prompt when both passwords are the different", async () => {
const password = "password#4321";
readlineSync.question = jest.fn().mockImplementation((a) => {
if (a == "Enter the above password again: ") {
Expand All @@ -214,9 +215,6 @@ describe("Backup Tests", () => {
test("Get encrypted archive path", async () => {
const archivePath = "/rootDir/appsmith-backup-0000-00-0T00-00-00.00Z";
const encryptionPassword = "password#4321";
utils.execCommand = jest
.fn()
.mockImplementation(async (a) => console.log(a));
const encArchivePath = await backup.encryptBackupArchive(
archivePath,
encryptionPassword,
Expand All @@ -228,9 +226,6 @@ describe("Backup Tests", () => {
});

test("Test backup encryption function", async () => {
utils.execCommand = jest
.fn()
.mockImplementation(async (a) => console.log(a));
const archivePath = "/rootDir/appsmith-backup-0000-00-0T00-00-00.00Z";
const encryptionPassword = "password#123";
const res = await backup.encryptBackupArchive(
Expand All @@ -243,32 +238,32 @@ describe("Backup Tests", () => {
});

test("Get DB name from Mongo URI 1", async () => {
var mongodb_uri =
const mongodb_uri =
"mongodb+srv://admin:password@test.cluster.mongodb.net/my_db_name?retryWrites=true&minPoolSize=1&maxPoolSize=10&maxIdleTimeMS=900000&authSource=admin";
var expectedDBName = "my_db_name";
const expectedDBName = "my_db_name";
const dbName = utils.getDatabaseNameFromMongoURI(mongodb_uri);
expect(dbName).toEqual(expectedDBName);
});

test("Get DB name from Mongo URI 2", async () => {
var mongodb_uri =
const mongodb_uri =
"mongodb+srv://admin:password@test.cluster.mongodb.net/test123?retryWrites=true&minPoolSize=1&maxPoolSize=10&maxIdleTimeMS=900000&authSource=admin";
var expectedDBName = "test123";
const expectedDBName = "test123";
const dbName = utils.getDatabaseNameFromMongoURI(mongodb_uri);
expect(dbName).toEqual(expectedDBName);
});

test("Get DB name from Mongo URI 3", async () => {
var mongodb_uri =
const mongodb_uri =
"mongodb+srv://admin:password@test.cluster.mongodb.net/test123";
var expectedDBName = "test123";
const expectedDBName = "test123";
const dbName = utils.getDatabaseNameFromMongoURI(mongodb_uri);
expect(dbName).toEqual(expectedDBName);
});

test("Get DB name from Mongo URI 4", async () => {
var mongodb_uri = "mongodb://appsmith:pAssW0rd!@localhost:27017/appsmith";
var expectedDBName = "appsmith";
const mongodb_uri = "mongodb://appsmith:pAssW0rd!@localhost:27017/appsmith";
const expectedDBName = "appsmith";
const dbName = utils.getDatabaseNameFromMongoURI(mongodb_uri);
expect(dbName).toEqual(expectedDBName);
});
Comment on lines 241 to 269
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove sensitive information from MongoDB URI tests

Multiple test cases contain actual passwords in MongoDB URIs. Replace them with placeholder values.

-  const mongodb_uri = "mongodb://appsmith:pAssW0rd!@localhost:27017/appsmith";
+  const mongodb_uri = "mongodb://[username]:[password]@localhost:27017/appsmith";

Committable suggestion skipped: line range outside the PR's diff.

Loading