Refactored all classes as modules, and added thread-safetyness to imageService

package.json

@@ -14,6 +14,7 @@
     "typescript": "^5.0.0"
   },
   "dependencies": {
+    "async-mutex": "^0.5.0",
     "compression": "^1.7.4",
     "express": "^4.19.2",
     "express-list-endpoints": "^6.0.0",

src/app.ts

@@ -1,7 +1,7 @@
 import compression from "compression";
 import express from "express";
 import listEndpoints from "express-list-endpoints";
-import ImageController from "src/controllers/ImageController";
+import * as imageController from "src/controllers/imageController";
 
 const app = express();
 
@@ -13,6 +13,6 @@ app.get("/", (_, res) => {
   res.json({ endpoints });
 });
 
-app.get("/image", ImageController.get);
+app.get("/image", imageController.get);
 
 export default app;

src/controllers/ImageController.ts

@@ -1,17 +0,0 @@
-import { Request, Response } from "express";
-import logger from "src/logger";
-import ImageService from "src/services/ImageService";
-
-class ImageController {
-  async get(_: Request, res: Response) {
-    try {
-      const image = await ImageService.get();
-      res.json(image);
-    } catch (error: any) {
-      logger.error(error);
-      res.status(500).json({ error: `Internal server error: ${error}` });
-    }
-  }
-}
-
-export default new ImageController();

src/controllers/imageController.ts

@@ -0,0 +1,13 @@
+import { Request, Response } from "express";
+import logger from "src/logger";
+import * as imageService from "src/services/imageService";
+
+export async function get(_: Request, res: Response) {
+  try {
+    const image = await imageService.get();
+    res.json(image);
+  } catch (error: any) {
+    logger.error(error);
+    res.status(500).json({ error: `Internal server error: ${error}` });
+  }
+}

src/services/BotApiService.ts

@@ -1,22 +0,0 @@
-import { env } from "bun";
-import logger from "src/logger";
-import { BotApiResponse } from "src/types/BotApiResponse";
-
-class BotApiService {
-  readonly BOT_API_URI = env.BOT_API_URI;
-
-  async getAll(): Promise<BotApiResponse> {
-    const get_url = `${this.BOT_API_URI}/images`;
-    const response: BotApiResponse = (await fetch(get_url).then(async (res) => {
-      if (!res.ok) {
-        logger.error(`${res.status}: ${res.statusText}, ${await res.text()}`);
-        throw new Error("Error fetching images");
-      } else {
-        return res.json();
-      }
-    })) as BotApiResponse;
-    return response;
-  }
-}
-
-export default new BotApiService();

src/services/GelbooruApiService.ts

@@ -1,32 +0,0 @@
-import { env } from "bun";
-import logger from "src/logger";
-import GelbooruApiResponse from "src/types/GelbooruApiResponse";
-import GelbooruServiceResponse from "src/types/GelbooruServiceResponse";
-
-class GelbooruApiService {
-  async get(): Promise<GelbooruServiceResponse> {
-    const LIMIT = env.GELBOORU_IMAGES_PER_REQUEST || 100;
-    const TAGS = encodeURIComponent(env.GELBOORU_TAGS || "");
-    const url: string = `https://gelbooru.com/index.php?page=dapi&s=post&q=index&limit=${LIMIT}&json=1&tags=${TAGS}`;
-
-    const response: GelbooruApiResponse = (await fetch(url).then(
-      async (res) => {
-        if (!res.ok) {
-          logger.error(`${res.status}: ${res.statusText}, ${await res.text()}`);
-          throw new Error("Error fetching images");
-        } else {
-          return res.json();
-        }
-      }
-    )) as GelbooruApiResponse;
-
-    return {
-      posts: response.post.map((post) => ({
-        url: post.file_url,
-        tags: post.tags.split(" "),
-      })),
-    };
-  }
-}
-
-export default new GelbooruApiService();

src/services/ImageService.ts

@@ -1,39 +0,0 @@
-import GelbooruServiceResponse from "src/types/GelbooruServiceResponse";
-import Image from "src/types/Image";
-import BotApiService from "src/services/BotApiService";
-import GelbooruApiService from "src/services/GelbooruApiService";
-import logger from "src/logger";
-
-class ImageService {
-  postsQueue: Image[] = [];
-
-  async get(): Promise<Image> {
-    while (this.postsQueue.length === 0) {
-      const validPosts = await this.getNewValidImages();
-      this.postsQueue = validPosts;
-      logger.info(`Got ${validPosts.length} images from remote`);
-    }
-    return this.postsQueue.pop() as Image;
-  }
-
-  private async getNewValidImages(): Promise<Image[]> {
-    const gelbooruResponse: GelbooruServiceResponse =
-      await GelbooruApiService.get();
-    const posts = gelbooruResponse.posts;
-
-    const botResponse = await BotApiService.getAll();
-    const imagesUrls = botResponse.images.map((image) => image.url);
-
-    const validPosts = Promise.all(
-      posts
-        .filter((post) => !imagesUrls.some((url) => url === post.url))
-        .map((post): Image => {
-          return { url: post.url, tags: post.tags };
-        })
-    );
-
-    return validPosts;
-  }
-}
-
-export default new ImageService();

src/services/botApiService.ts

@@ -0,0 +1,18 @@
+import { env } from "bun";
+import logger from "src/logger";
+import { BotApiResponse } from "src/types/BotApiResponse";
+
+const BOT_API_URI = env.BOT_API_URI;
+
+export async function getAll(): Promise<BotApiResponse> {
+  const get_url = `${BOT_API_URI}/images`;
+  const response: BotApiResponse = (await fetch(get_url).then(async (res) => {
+    if (!res.ok) {
+      logger.error(`${res.status}: ${res.statusText}, ${await res.text()}`);
+      throw new Error("Error fetching images");
+    } else {
+      return res.json();
+    }
+  })) as BotApiResponse;
+  return response;
+}

src/services/gelbooruApiService.ts

@@ -0,0 +1,28 @@
+import { env } from "bun";
+import logger from "src/logger";
+import GelbooruApiResponse from "src/types/GelbooruApiResponse";
+import GelbooruServiceResponse from "src/types/GelbooruServiceResponse";
+
+export async function get(): Promise<GelbooruServiceResponse> {
+  const LIMIT = env.GELBOORU_IMAGES_PER_REQUEST || 100;
+  const TAGS = encodeURIComponent(env.GELBOORU_TAGS || "");
+  const url: string = `https://gelbooru.com/index.php?page=dapi&s=post&q=index&limit=${LIMIT}&json=1&tags=${TAGS}`;
+
+  const response: GelbooruApiResponse = (await fetch(url).then(
+    async (res) => {
+      if (!res.ok) {
+        logger.error(`${res.status}: ${res.statusText}, ${await res.text()}`);
+        throw new Error("Error fetching images");
+      } else {
+        return res.json();
+      }
+    }
+  )) as GelbooruApiResponse;
+
+  return {
+    posts: response.post.map((post) => ({
+      url: post.file_url,
+      tags: post.tags.split(" "),
+    })),
+  };
+}

src/services/imageService.ts

@@ -0,0 +1,51 @@
+import GelbooruServiceResponse from "src/types/GelbooruServiceResponse";
+import Image from "src/types/Image";
+import logger from "src/logger";
+import { Mutex } from "async-mutex";
+import * as gelbooruApiService from 'src/services/gelbooruApiService';
+import * as botApiService from 'src/services/botApiService';
+
+const mutex: Mutex = new Mutex();
+const postsQueue: Image[] = [];
+
+// We wrap the function into a Mutex because it's not thread-safe
+export async function get(): Promise<Image> {
+  return await mutex.runExclusive(() => unsafeGet())
+}
+
+async function unsafeGet(): Promise<Image> {
+  while (postsQueue.length === 0) {
+    const validPosts = await getNewValidImages();
+    validPosts.map(post => postsQueue.push(post));
+    logger.info(`Got ${validPosts.length} images from remote`);
+  }
+  return popImage();
+}
+
+function popImage(): Image {
+  const image = postsQueue.pop();
+  if (image) {
+    return image
+  } else {
+    throw Error("Can't pop from an empty list");
+  }
+}
+
+async function getNewValidImages(): Promise<Image[]> {
+  const gelbooruResponse: GelbooruServiceResponse =
+    await gelbooruApiService.get();
+  const posts = gelbooruResponse.posts;
+
+  const botResponse = await botApiService.getAll();
+  const imagesUrls = botResponse.images.map((image) => image.url);
+
+  const validPosts = Promise.all(
+    posts
+      .filter((post) => !imagesUrls.some((url) => url === post.url))
+      .map((post): Image => {
+        return { url: post.url, tags: post.tags };
+      })
+  );
+
+  return validPosts;
+}

test/ImageController/ImageController.test.ts

@@ -1,16 +1,11 @@
-import { afterEach, describe, expect, it, mock } from "bun:test";
+import { afterAll, afterEach, describe, expect, it, jest, mock, spyOn } from "bun:test";
 import app from "src/app";
-import ImageService from "src/services/ImageService";
 import request from "supertest";
+import * as imageService from "src/services/imageService";
 
-const imageServiceOriginal = ImageService;
-
-afterEach(() => {
-  mock.restore();
-  mock.module("src/services/ImageService", () => ({
-    default: imageServiceOriginal,
-  }));
-});
+afterAll(() => {
+  jest.restoreAllMocks();
+})
 
 describe("endpoint returns the correct status codes", () => {
   it("should return 200 if successful", async () => {
@@ -21,16 +16,10 @@ describe("endpoint returns the correct status codes", () => {
   });
 
   it("should return 500 if any error happens", async () => {
-    mock.module("src/services/ImageService", () => {
-      return {
-        default: {
-          get: () => {
-            throw new Error("Controlled error");
-          },
-        },
-      };
-    });
+    spyOn(imageService, "get").mockImplementation(() => { throw new Error("Controlled error") });
+
     const res = await request(app).get("/image");
+
     expect(res.status).toBe(500);
   });
 });

test/ImageService/ImageService.test.ts

@@ -1,27 +1,18 @@
-import { afterEach, describe, expect, it, mock } from "bun:test";
-import BotApiService from "src/services/BotApiService";
-import GelbooruApiService from "src/services/GelbooruApiService";
-import ImageService from "src/services/ImageService";
-import { BotApiResponse } from "src/types/BotApiResponse";
-import GelbooruApiResponse from "src/types/GelbooruServiceResponse";
+import { afterAll, describe, expect, it, jest, mock, spyOn } from "bun:test";
 import Image from "src/types/Image";
+import * as gelbooruApiService from "src/services/gelbooruApiService";
+import * as botApiService from "src/services/botApiService";
+import * as imageService from "src/services/imageService";
 
-const imageServiceOriginal = ImageService;
-const gelbooruApiServiceOriginal = GelbooruApiService;
-const botApiServiceOriginal = BotApiService;
+afterAll(() => {
+  jest.restoreAllMocks();
+})
 
-afterEach(() => {
-  mock.restore();
-  mock.module("src/services/ImageService", () => ({
-    default: imageServiceOriginal,
-  }));
-  mock.module("src/services/GelbooruApiService", () => ({
-    default: gelbooruApiServiceOriginal,
-  }));
-  mock.module("src/services/BotApiService", () => ({
-    default: botApiServiceOriginal,
-  }));
-});
+describe("the service is thread-safe", () => {
+  it("should run normally when 2 processes call the get() method with 1 remaining image in the queue", async () => {
+    // TODO
+  })
+})
 
 describe("endpoint gets a non repeated image", () => {
   it("should return an image that is not in the response of the /images endpoint of the bot API", async () => {
@@ -30,25 +21,10 @@ describe("endpoint gets a non repeated image", () => {
     const UNIQUE_URL =
       "https://fastly.picsum.photos/id/2/10/20.jpg?hmac=zy6lz21CuRIstr9ETx9h5AuoH50s_L2uIEct3dROpY8";
 
-    mock.module("src/services/GelbooruApiService", () => {
-      let alreadyCalled = false;
-      return {
-        default: {
-          get: (): GelbooruApiResponse => {
-            if (alreadyCalled) {
-              return { posts: [{ url: UNIQUE_URL, tags: [] }] };
-            } else {
-              alreadyCalled = true;
-              return { posts: [{ url: REPEATED_URL, tags: [] }] };
-            }
-          },
-        },
-      };
-    });
-
-    mock.module("src/services/BotApiService", () => ({
-      default: {
-        getAll: (): BotApiResponse => ({
+    const gelbooruApiServiceGet = spyOn(gelbooruApiService, "get");
+    gelbooruApiServiceGet.mockImplementationOnce(async () => ({ posts: [{ url: UNIQUE_URL, tags: [] }] }));
+    gelbooruApiServiceGet.mockImplementation(async () => ({ posts: [{ url: REPEATED_URL, tags: [] }] }));
+    spyOn(botApiService, "getAll").mockImplementation(async () => ({
       images: [
         {
           _id: "0",
@@ -58,11 +34,10 @@ describe("endpoint gets a non repeated image", () => {
           __v: 0,
         },
       ],
-        }),
-      },
     }));
 
-    const image: Image = await ImageService.get();
+    const image: Image = await imageService.get();
+
     expect(image.url).not.toBe(REPEATED_URL);
   });
 });

yarn.lock

@@ -1,6 +1,6 @@
 # THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
 # yarn lockfile v1
-# bun ./bun.lockb --hash: 2D4E664D3091262E-6e6ac083a9fdd796-C67F3D33680268C0-c674fd6e89aeea1f
+# bun ./bun.lockb --hash: 9A5A612BBFD3E7ED-76a31de2b7c4bbe8-8DB93E498B9CBB30-9cf1116024820c4f
 
 
 "@colors/colors@1.6.0", "@colors/colors@^1.6.0":
@@ -177,6 +177,13 @@ async@^3.2.3:
   resolved "https://registry.npmjs.org/async/-/async-3.2.5.tgz"
   integrity sha512-baNZyqaaLhyLVKm/DlvdW051MSgO6b8eVfIezl9E5PqWxFgzLm/wQntEW4zOytVburDEr0JlALEpdOFwvErLsg==
 
+async-mutex@^0.5.0:
+  version "0.5.0"
+  resolved "https://registry.npmjs.org/async-mutex/-/async-mutex-0.5.0.tgz"
+  integrity sha512-1A94B18jkJ3DYq284ohPxoXbfTA5HsQ7/Mf4DEhcyLx3Bz27Rh59iScbB6EPiP+B+joue6YCxcMXSbFC1tZKwA==
+  dependencies:
+    tslib "^2.4.0"
+
 asynckit@^0.4.0:
   version "0.4.0"
   resolved "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz"
@@ -908,6 +915,11 @@ triple-beam@^1.3.0:
   resolved "https://registry.npmjs.org/triple-beam/-/triple-beam-1.4.1.tgz"
   integrity sha512-aZbgViZrg1QNcG+LULa7nhZpJTZSLm/mXnHXnbAbjmN5aSa0y7V+wvv6+4WaBtpISJzThKy+PIPxc1Nq1EJ9mg==
 
+tslib@^2.4.0:
+  version "2.6.2"
+  resolved "https://registry.npmjs.org/tslib/-/tslib-2.6.2.tgz"
+  integrity sha512-AEYxH93jGFPn/a2iVAwW87VuUIkR1FVUKB77NwMF7nBTDkDrrT/Hpt/IrCJ0QXhW27jTBDcf5ZY7w6RiqTMw2Q==
+
 type-is@~1.6.18:
   version "1.6.18"
   resolved "https://registry.npmjs.org/type-is/-/type-is-1.6.18.tgz"