Skip to content

Commit 726b84e

Browse files
authored
refactor(storage-resize-images): some refactoring (#2362)
* refactor(storage-resize-images): some refactoring * chore(storage-resize-images): format * chore(storage-resize-image): modify some test imports and remove some unnecessary code * chore(storage-resize-images): format * refactor(storage-resize-images): replacing retry-queue.ts
1 parent f35b23d commit 726b84e

14 files changed

+283
-6913
lines changed

storage-resize-images/functions/__tests__/convert-image.test.ts

+43-8
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ const env = {
1414

1515
mockedEnv(env);
1616

17-
import { convertType } from "../src/resize-image";
17+
import { convertType } from "../src/util";
1818
import { config } from "../src/config";
1919

2020
jest.mock("../src/config");
@@ -32,43 +32,78 @@ beforeAll(async () => {
3232

3333
describe("convertType", () => {
3434
it("converts to png image type", async () => {
35-
const buffer = await convertType(bufferJPG, "png");
35+
const buffer = await convertType(
36+
bufferJPG,
37+
"png",
38+
config.outputOptions,
39+
config.animated
40+
);
3641

3742
expect(imageType(buffer).mime).toBe("image/png");
3843
});
3944

4045
it("converts to jpeg image type", async () => {
41-
const buffer = await convertType(bufferPNG, "jpeg");
46+
const buffer = await convertType(
47+
bufferPNG,
48+
"jpeg",
49+
config.outputOptions,
50+
config.animated
51+
);
4252

4353
expect(imageType(buffer).mime).toBe("image/jpeg");
4454
});
4555

4656
it("converts to webp image type", async () => {
47-
const buffer = await convertType(bufferPNG, "webp");
57+
const buffer = await convertType(
58+
bufferPNG,
59+
"webp",
60+
config.outputOptions,
61+
config.animated
62+
);
4863

4964
expect(imageType(buffer).mime).toBe("image/webp");
5065
});
5166

5267
it("converts to tiff image type", async () => {
53-
const buffer = await convertType(bufferPNG, "tiff");
68+
const buffer = await convertType(
69+
bufferPNG,
70+
"tiff",
71+
config.outputOptions,
72+
config.animated
73+
);
5474

5575
expect(imageType(buffer).mime).toBe("image/tiff");
5676
});
5777

5878
it("converts to gif image type", async () => {
59-
const buffer = await convertType(bufferGIF, "gif");
79+
const buffer = await convertType(
80+
bufferGIF,
81+
"gif",
82+
config.outputOptions,
83+
config.animated
84+
);
6085

6186
expect(imageType(buffer).mime).toBe("image/gif");
6287
});
6388

6489
it("remains jpeg image type when different image type is not supported", async () => {
65-
const buffer = await convertType(bufferJPG, "raw");
90+
const buffer = await convertType(
91+
bufferJPG,
92+
"raw",
93+
config.outputOptions,
94+
config.animated
95+
);
6696

6797
expect(imageType(buffer).mime).toBe("image/jpeg");
6898
});
6999

70100
it("remains gif image type when different image type is not supported", async () => {
71-
const buffer = await convertType(bufferGIF, "raw");
101+
const buffer = await convertType(
102+
bufferGIF,
103+
"raw",
104+
config.outputOptions,
105+
config.animated
106+
);
72107

73108
expect(imageType(buffer).mime).toBe("image/gif");
74109
});

storage-resize-images/functions/__tests__/filters.test.ts

-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import * as path from "path";
22
import * as logs from "../src/logs";
3-
import { supportedContentTypes } from "../src/resize-image";
43
import { ObjectMetadata } from "firebase-functions/v1/storage";
54

65
jest.mock("../src/config", () => {

storage-resize-images/functions/__tests__/resize.test.ts

+4-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import * as fs from "fs";
22

33
import mockedEnv from "mocked-env";
44
import sizeOf from "image-size";
5-
import * as sharp from "sharp";
5+
import sharp from "sharp";
66

77
const environment = {
88
LOCATION: "us-central1",
@@ -16,11 +16,12 @@ const environment = {
1616
let restoreEnv;
1717
restoreEnv = mockedEnv(environment);
1818

19+
import { resize } from "../src/resize-image";
20+
1921
import {
20-
resize,
2122
supportedContentTypes,
2223
supportedImageContentTypeMap,
23-
} from "../src/resize-image";
24+
} from "../src/global";
2425

2526
import * as path from "path";
2627

storage-resize-images/functions/__tests__/retry-queue.test.ts

+53-31
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,17 @@
1-
import { RetryQueue } from "../src/retry-queue";
1+
import PQueue from "p-queue";
22

33
describe("RetryQueue", () => {
4-
let queue: RetryQueue;
5-
64
beforeEach(() => {
7-
queue = new RetryQueue();
85
jest.useRealTimers();
96
});
107

118
test("should process tasks in order of priority", async () => {
9+
// Force sequential execution by overriding concurrency limit
10+
const queue = new PQueue({ concurrency: 1, autoStart: false });
11+
1212
jest.useFakeTimers();
1313
let executionOrder: number[] = [];
1414

15-
// Force sequential execution by overriding concurrency limit
16-
// @ts-ignore - accessing private property for testing
17-
queue["concurrencyLimit"] = 1;
18-
1915
// Create three tasks with different priorities
2016
const task1 = jest.fn().mockImplementation(async () => {
2117
executionOrder.push(1);
@@ -33,9 +29,12 @@ describe("RetryQueue", () => {
3329
});
3430

3531
// Set up promises to track task completion
36-
const promise2 = queue.enqueue(task2, 2);
37-
const promise3 = queue.enqueue(task3, 3);
38-
const promise1 = queue.enqueue(task1, 1);
32+
const promise1 = queue.add(task2, { priority: 2 });
33+
const promise2 = queue.add(task3, { priority: 1 });
34+
const promise3 = queue.add(task1, { priority: 3 });
35+
36+
// Start the queue
37+
queue.start();
3938

4039
// Advance timers to allow the sorting to complete
4140
jest.advanceTimersByTime(10);
@@ -59,13 +58,18 @@ describe("RetryQueue", () => {
5958
});
6059

6160
test("should handle task failures correctly", async () => {
61+
const queue = new PQueue({ concurrency: 3, autoStart: false });
62+
6263
const successTask = jest.fn().mockResolvedValue("success");
6364
const failureTask = jest.fn().mockRejectedValue(new Error("Task failed"));
6465

65-
const successPromise = queue.enqueue(successTask, 1);
66-
const failurePromise = queue.enqueue(failureTask, 2);
66+
const successPromise = queue.add(successTask, { priority: 2 });
67+
const failurePromise = queue.add(failureTask, { priority: 1 });
68+
69+
// Start the queue
70+
queue.start();
6771

68-
// Success task should resolve
72+
// Succes, priority: task should resolve
6973
await expect(successPromise).resolves.toBe("success");
7074

7175
// Failure task should reject
@@ -76,6 +80,8 @@ describe("RetryQueue", () => {
7680
});
7781

7882
test("should respect concurrency limit", async () => {
83+
const queue = new PQueue({ concurrency: 3, autoStart: false });
84+
7985
// Create a mock implementation to track concurrent execution
8086
let concurrentCount = 0;
8187
let maxConcurrentCount = 0;
@@ -97,9 +103,12 @@ describe("RetryQueue", () => {
97103
// Queue multiple slow tasks
98104
const promises = [];
99105
for (let i = 0; i < 10; i++) {
100-
promises.push(queue.enqueue(createTask(10), (i % 3) + 1)); // Vary priority
106+
promises.push(queue.add(createTask(10), { priority: i + 1 }));
101107
}
102108

109+
// Start the queue
110+
queue.start();
111+
103112
// Wait for all tasks to complete
104113
await Promise.all(promises);
105114

@@ -109,10 +118,9 @@ describe("RetryQueue", () => {
109118
});
110119

111120
test("should continue processing queue after task completion", async () => {
121+
const queue = new PQueue({ concurrency: 1, autoStart: false });
122+
112123
jest.useFakeTimers();
113-
// Force sequential execution
114-
// @ts-ignore - accessing private property for testing
115-
queue["concurrencyLimit"] = 1;
116124

117125
const executionOrder: number[] = [];
118126

@@ -135,9 +143,12 @@ describe("RetryQueue", () => {
135143
};
136144

137145
// Enqueue tasks
138-
const promise1 = queue.enqueue(slowTask, 1); // Higher priority
139-
const promise2 = queue.enqueue(fastTask1, 2);
140-
const promise3 = queue.enqueue(fastTask2, 3);
146+
const promise1 = queue.add(slowTask, { priority: 3 }); // Higher priority
147+
const promise2 = queue.add(fastTask1, { priority: 2 });
148+
const promise3 = queue.add(fastTask2, { priority: 1 });
149+
150+
// Start the queue
151+
queue.start();
141152

142153
// Advance timers to allow sorting to complete
143154
jest.advanceTimersByTime(10);
@@ -156,18 +167,18 @@ describe("RetryQueue", () => {
156167
});
157168

158169
test("should handle empty queue gracefully", async () => {
170+
const queue = new PQueue({ concurrency: 3 });
159171
// Just make sure no errors are thrown
160172
expect(() => {
161173
// @ts-ignore - accessing private method for testing
162-
queue.processQueue();
174+
queue.start();
163175
}).not.toThrow();
164176
});
165177

166178
test("should handle many tasks with same priority in order of addition", async () => {
179+
const queue = new PQueue({ concurrency: 1, autoStart: false });
180+
167181
jest.useFakeTimers();
168-
// Force sequential execution
169-
// @ts-ignore - accessing private property for testing
170-
queue["concurrencyLimit"] = 1;
171182

172183
const executionOrder: number[] = [];
173184

@@ -182,9 +193,12 @@ describe("RetryQueue", () => {
182193
// Queue tasks with same priority
183194
const promises = [];
184195
for (let i = 1; i <= 5; i++) {
185-
promises.push(queue.enqueue(createNumberedTask(i), 1)); // All same priority
196+
promises.push(queue.add(createNumberedTask(i), { priority: 1 }));
186197
}
187198

199+
// Start the queue
200+
queue.start();
201+
188202
// Advance timers to allow sorting to complete
189203
jest.advanceTimersByTime(10);
190204

@@ -202,20 +216,28 @@ describe("RetryQueue", () => {
202216
});
203217

204218
test("should handle large number of tasks efficiently", async () => {
219+
const queue = new PQueue({ concurrency: 3, autoStart: false });
220+
205221
const taskCount = 50; // Reduced for faster test execution
206222
const completedTasks: number[] = [];
207223

208224
// Create many tasks
209225
const promises = [];
210226
for (let i = 0; i < taskCount; i++) {
211227
promises.push(
212-
queue.enqueue(async () => {
213-
completedTasks.push(i);
214-
return i;
215-
}, i % 10) // Cycle through 10 priority levels
216-
);
228+
queue.add(
229+
async () => {
230+
completedTasks.push(i);
231+
return i;
232+
},
233+
{ priority: i }
234+
)
235+
); // Cycle through 10 priority levels
217236
}
218237

238+
// Start the queue
239+
queue.start();
240+
219241
// Wait for all tasks to complete
220242
await Promise.all(promises);
221243

storage-resize-images/functions/__tests__/unit/modifyImage.test.ts

-7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// import { getModifiedFilePath } from "../../src/resize-image";
21
import * as path from "path";
32
import { config } from "dotenv";
43

@@ -10,12 +9,6 @@ const envLocalPath = path.resolve(
109
config({ path: envLocalPath, debug: true, override: true });
1110

1211
import { getModifiedFilePath } from "../../src/resize-image";
13-
import { platform } from "os";
14-
15-
const convertToPosixPath = (filePath: string, locale?: "win32" | "posix") => {
16-
const sep = locale ? path[locale].sep : path.sep;
17-
return filePath.split(sep).join(path.posix.sep);
18-
};
1912

2013
const oldGetModifiedFilePath = (
2114
fileDir,

0 commit comments

Comments
 (0)