Skip to content
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.

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

feat: add logic for image reference counting #609

Open
wants to merge 3 commits into
base: staging
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions backend/jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
* https://jestjs.io/docs/configuration
*/

import { MOCK_FIREBASE_STORAGE_DEFAULT_BUCKET } from "./testUtils/imageUpload";

export default {
// All imported modules in your tests should be mocked automatically
// automock: false,
Expand Down Expand Up @@ -204,3 +206,7 @@ export default {
// Increases timeout to let tests run
testTimeout: 1000000,
};

process.env = Object.assign(process.env, {
FIREBASE_STORAGE_DEFAULT_BUCKET: MOCK_FIREBASE_STORAGE_DEFAULT_BUCKET,
});
27 changes: 27 additions & 0 deletions backend/models/imageCount.model.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import type { Document } from "mongoose";
import mongoose, { Schema } from "mongoose";

/**
* This document contains information about a single image.
*/
export interface ImageCount extends Document {
/** the unique identifier for the image */
id: string;
/** the unique file path of the image */
filePath: string;
/** the number of tests that reference this image */
referenceCount: number;
}

const ImageCountSchema: Schema = new Schema({
filePath: {
type: String,
required: true,
},
referenceCount: {
type: Number,
required: true,
},
});

export default mongoose.model<ImageCount>("ImageCount", ImageCountSchema);
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import db from "../../../testUtils/testDb";
import ImageCountService from "../imageCountService";

describe("mongo imageCountService", (): void => {
let imageCountService: ImageCountService;

beforeAll(async () => {
await db.connect();
});

afterAll(async () => {
await db.disconnect();
});

beforeEach(async () => {
imageCountService = new ImageCountService();
});

afterEach(async () => {
await db.clear();
});

it("initialize count", async () => {
const referenceCount = await imageCountService.initializeCount("test path");
expect(referenceCount).toEqual(1);
});

it("increment count", async () => {
await imageCountService.initializeCount("test path");
const referenceCount = await imageCountService.incrementCount("test path");
expect(referenceCount).toEqual(2);
});

it("decrement count", async () => {
await imageCountService.initializeCount("test path");
const referenceCount = await imageCountService.decrementCount("test path");
expect(referenceCount).toEqual(0);

await expect(async () => {
await imageCountService.decrementCount("test path");
}).rejects.toThrowError(`Image test path not found`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import {
} from "../../../testUtils/imageUpload";
import type IImageUploadService from "../../interfaces/imageUploadService";
import ImageUploadService from "../imageUploadService";
import ImageCountService from "../imageCountService";
import type { FileUpload } from "../../../lib/graphql-upload";
import MgImageCount from "../../../models/imageCount.model";

jest.mock("firebase-admin", () => {
const storage = jest.fn().mockReturnValue({
Expand All @@ -24,7 +27,7 @@ jest.mock("firebase-admin", () => {
getSignedUrl: jest
.fn()
.mockReturnValue([
"https://storage.googleapis.com/jump-math-98edf.appspot.com/test-bucket/test.png",
"https://storage.googleapis.com/test-url/test-bucket/test.png",
]),
delete: jest.fn().mockReturnValue({}),
}),
Expand All @@ -36,6 +39,7 @@ jest.mock("firebase-admin", () => {

describe("mongo imageUploadService", (): void => {
let imageUploadService: IImageUploadService;
let imageCountService: ImageCountService;

beforeAll(async () => {
await db.connect();
Expand All @@ -46,14 +50,17 @@ describe("mongo imageUploadService", (): void => {
});

beforeEach(async () => {
imageUploadService = new ImageUploadService(uploadDir);
imageCountService = new ImageCountService();
imageUploadService = new ImageUploadService(uploadDir, imageCountService);
});

afterEach(async () => {
await db.clear();
});

it("deleteImage - invalid filePath", async () => {
await imageCountService.initializeCount(imageMetadata.filePath);

await expect(async () => {
await imageUploadService.deleteImage(imageMetadata);
}).rejects.toThrowError(
Expand All @@ -65,8 +72,38 @@ describe("mongo imageUploadService", (): void => {
const uploadedImage = await imageUploadService.uploadImage(imageUpload);
assertResponseMatchesExpected(uploadedImage);

const res = await imageUploadService.deleteImage(uploadedImage);
// check that the reference count is 1
let referenceCount = await MgImageCount.findOne({
filePath: uploadedImage.filePath,
});
expect(referenceCount?.referenceCount).toEqual(1);

// upload same image and check that the reference count is 2
const id = uploadedImage.filePath.split("_")[1];
await imageUploadService.uploadImage({
file: undefined as unknown as Promise<FileUpload>,
previewUrl: `${uploadedImage.url}_${id}?GoogleAccessId=fi&Expires=1698622126&Signature=gV`,
});
referenceCount = await MgImageCount.findOne({
filePath: uploadedImage.filePath,
});
expect(referenceCount?.referenceCount).toEqual(2);

// delete image and check that the reference count is 1
let res = await imageUploadService.deleteImage(uploadedImage);
assertResponseMatchesExpected(res);
referenceCount = await MgImageCount.findOne({
filePath: uploadedImage.filePath,
});
expect(referenceCount?.referenceCount).toEqual(1);

// delete same image and check that the image is deleted
res = await imageUploadService.deleteImage(uploadedImage);
assertResponseMatchesExpected(res);
referenceCount = await MgImageCount.findOne({
filePath: uploadedImage.filePath,
});
expect(referenceCount).toBeNull();
});

it("uploadImage - invalid image type", async () => {
Expand Down
17 changes: 17 additions & 0 deletions backend/services/implementations/__tests__/testService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ describe("mongo testService", (): void => {
testService.imageUploadService.uploadImage = jest
.fn()
.mockReturnValue(imageMetadata);
testService.imageUploadService.incrementImageCount = jest
.fn()
.mockReturnValue(imageMetadata);
testService.imageUploadService.getImage = jest
.fn()
.mockReturnValue(imageMetadata);
Expand Down Expand Up @@ -117,6 +120,13 @@ describe("mongo testService", (): void => {
expect(test.id).not.toEqual(duplicateTest.id);
expect(`${test.name} [COPY]`).toEqual(duplicateTest.name);

expect(
testService.imageUploadService.incrementImageCount,
).toHaveBeenCalledWith(imageMetadata);
expect(
testService.imageUploadService.incrementImageCount,
).toHaveBeenCalledTimes(1);

const originalTest = await testService.getTestById(test.id);
assertResponseMatchesExpected(mockPublishedTest, originalTest);
expect(test.id).toEqual(originalTest.id);
Expand All @@ -131,6 +141,13 @@ describe("mongo testService", (): void => {
expect(test.id).not.toEqual(unarchivedTest.id);
expect(`${test.name} [COPY]`).toEqual(unarchivedTest.name);

expect(
testService.imageUploadService.incrementImageCount,
).toHaveBeenCalledWith(imageMetadata);
expect(
testService.imageUploadService.incrementImageCount,
).toHaveBeenCalledTimes(1);

const originalTest = await MgTest.findById(test.id);
expect(originalTest?.status).toBe(AssessmentStatus.DELETED);
});
Expand Down
70 changes: 70 additions & 0 deletions backend/services/implementations/imageCountService.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import MgImage from "../../models/imageCount.model";
import { getErrorMessage } from "../../utilities/errorUtils";
import logger from "../../utilities/logger";
import type IImageCountService from "../interfaces/imageCountService";

const Logger = logger(__filename);

class ImageCountService implements IImageCountService {
/* eslint-disable class-methods-use-this */
async initializeCount(filePath: string): Promise<number> {
try {
const image = new MgImage({
filePath,
referenceCount: 1,
});
await image.save();
return image.referenceCount;
} catch (error: unknown) {
Logger.error(
`Failed to increment count. Reason = ${getErrorMessage(error)}`,
);
throw error;
}
}

async incrementCount(filePath: string): Promise<number> {
try {
const image = await MgImage.findOneAndUpdate(
{ filePath },
{ $inc: { referenceCount: 1 } },
{ new: true },
);
if (!image) {
throw new Error(`Image ${filePath} not found`);
}
return image.referenceCount;
} catch (error: unknown) {
Logger.error(
`Failed to increment count. Reason = ${getErrorMessage(error)}`,
);
throw error;
}
}

async decrementCount(filePath: string): Promise<number> {
try {
const image = await MgImage.findOneAndUpdate(
{ filePath, referenceCount: { $gt: 0 } },
{ $inc: { referenceCount: -1 } },
{ new: true },
);
if (!image) {
throw new Error(`Image ${filePath} not found`);
}

if (image.referenceCount === 0) {
await MgImage.deleteOne({ filePath });
}

return image.referenceCount;
} catch (error: unknown) {
Logger.error(
`Failed to decrement count. Reason = ${getErrorMessage(error)}`,
);
throw error;
}
}
}

export default ImageCountService;
33 changes: 30 additions & 3 deletions backend/services/implementations/imageUploadService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type {
ImageMetadata,
ImageMetadataRequest,
} from "../../types/questionMetadataTypes";
import type IImageService from "../interfaces/imageCountService";

const Logger = logger(__filename);

Expand All @@ -32,16 +33,19 @@ const writeFile = (readStream: ReadStream, filePath: string): Promise<void> => {
class ImageUploadService implements IImageUploadService {
uploadDir: string;

imageService: IImageService;

storageService: IFileStorageService;

googleStorageUploadUrl: string;

constructor(uploadDir: string) {
constructor(uploadDir: string, imageService: IImageService) {
this.uploadDir = escapeRegExp(uploadDir);

const defaultBucket = process.env.FIREBASE_STORAGE_DEFAULT_BUCKET || "";
const storageService = new FileStorageService(defaultBucket);
this.storageService = storageService;
this.imageService = imageService;

this.googleStorageUploadUrl = escapeRegExp(
`https://storage.googleapis.com/${defaultBucket}`,
Expand All @@ -53,11 +57,13 @@ class ImageUploadService implements IImageUploadService {
let filePath = this.getFilePath(image);

try {
if (filePath)
if (filePath) {
await this.incrementImageCount({ filePath, url: "" });
return await this.hydrateImage({
filePath,
url: image.previewUrl,
});
}

const { createReadStream, mimetype, filename } = await image.file;
if (!fs.existsSync(this.uploadDir)) {
Expand All @@ -78,6 +84,20 @@ class ImageUploadService implements IImageUploadService {
}
}

async incrementImageCount(image: ImageMetadata): Promise<ImageMetadata> {
try {
await this.imageService.incrementCount(image.filePath);
return image;
} catch (error: unknown) {
Logger.error(
`Failed to increment image count for filePath: ${
image.filePath
}. Reason = ${getErrorMessage(error)}`,
);
throw error;
}
}

async hydrateImage(image: ImageMetadata): Promise<ImageMetadata> {
if (this.getExpirationDate(image) > Date.now()) {
return image;
Expand Down Expand Up @@ -112,7 +132,13 @@ class ImageUploadService implements IImageUploadService {

async deleteImage(image: ImageMetadata): Promise<ImageMetadata> {
try {
await this.storageService.deleteFile(image.filePath);
const decrementCount = await this.imageService.decrementCount(
image.filePath,
);
if (decrementCount === 0) {
await this.storageService.deleteFile(image.filePath);
}

return image;
} catch (error: unknown) {
Logger.error(
Expand All @@ -130,6 +156,7 @@ class ImageUploadService implements IImageUploadService {
): Promise<ImageMetadata> {
try {
await this.storageService.createFile(filePath, filePath, fileContentType);
await this.imageService.initializeCount(filePath);
return await this.getImage(filePath);
} catch (error: unknown) {
Logger.error(
Expand Down
Loading