fix(*): use generics for UploadsIdentifier

Author: bericp1

README.md

@@ -29,7 +29,6 @@ TODO
 -   [ ] Do URL generation for publicly available disks.
 -   [ ] Support temporary URLs (e.g. presigned URLs for S3 buckets) for disks that support it
 -   [ ] Support transfer logic for transferring single uploads from one disk to another and in bulk.
--   [ ] Instead of using an `Upload` type that is `any`, we should use generics
 
 ## Development
 

src/lib/Uploads.test.ts

@@ -1,7 +1,7 @@
 import * as fs from 'fs';
 import { promisify } from 'util';
 import { DiskDriver, DiskManager } from '@carimus/node-disks';
-import { MemoryRepository } from '../support';
+import { MemoryRepository, MemoryRepositoryUploadIdentifier } from '../support';
 import { Uploads } from './Uploads';
 import { UploadMeta } from '../types';
 
@@ -18,14 +18,14 @@ const disks = {
 function setup(): {
     diskManager: DiskManager;
     repository: MemoryRepository;
-    uploads: Uploads;
+    uploads: Uploads<MemoryRepositoryUploadIdentifier>;
 } {
     const diskManager = new DiskManager(disks);
     const repository = new MemoryRepository();
     return {
         diskManager,
         repository,
-        uploads: new Uploads({
+        uploads: new Uploads<MemoryRepositoryUploadIdentifier>({
             disks: diskManager,
             repository,
         }),
@@ -134,21 +134,17 @@ test('Uploads service can duplicate an upload, creating a new repository record
     const duplicateOldMeta = await uploads.duplicate(original);
 
     // Check the repository for the original file data and the new file data
-    const originalFileRecord = await repository.find(original.id);
-    const duplicateFileRecord = await repository.find(duplicate.id);
-    const duplicateOldMetaFileRecord = await repository.find(
-        duplicateOldMeta.id,
-    );
+    const originalFileRecord = await repository.find(original);
+    const duplicateFileRecord = await repository.find(duplicate);
+    const duplicateOldMetaFileRecord = await repository.find(duplicateOldMeta);
     expect(originalFileRecord.meta).toMatchObject(files.normal.meta);
     expect(duplicateFileRecord.meta).toMatchObject(duplicateMeta);
     expect(originalFileRecord).not.toMatchObject(duplicateFileRecord);
     expect(duplicateOldMetaFileRecord.meta).toMatchObject(files.normal.meta);
 
     // Check the disk for the original file and the new file
-    const originalFileInfo = await repository.getUploadedFileInfo(original.id);
-    const duplicateFileInfo = await repository.getUploadedFileInfo(
-        duplicate.id,
-    );
+    const originalFileInfo = await repository.getUploadedFileInfo(original);
+    const duplicateFileInfo = await repository.getUploadedFileInfo(duplicate);
     const originalFileData = await diskManager
         .getDisk(originalFileInfo.disk)
         .read(originalFileInfo.path);
@@ -187,7 +183,7 @@ test('Uploads service can update, updating existing repository records and delet
     const updatedNewMetaFileInfo = await repository.getUploadedFileInfo(
         updatedNewMeta,
     );
-    expect(original.id).toBe(updatedNewMeta.id);
+    expect(original).toBe(updatedNewMeta);
     await expect(
         diskManager.getDisk(originalFileInfo.disk).read(originalFileInfo.path),
     ).rejects.toBeTruthy();
@@ -290,12 +286,15 @@ test('Uploads service can create temp files for local manipulation from uploads'
         .read(fileInfo.path);
 
     // Get the temp file for it and check to make sure their contents match
-    const tempPath = await uploads.withTempFile(upload, async (path) => {
-        const tempFileData = await readFileFromLocalFilesystem(path);
-        expect(tempFileData.toString('base64')).toBe(
-            uploadedFileData.toString('base64'),
-        );
-    });
+    const tempPath = await uploads.withTempFile(
+        upload,
+        async (path: string) => {
+            const tempFileData = await readFileFromLocalFilesystem(path);
+            expect(tempFileData.toString('base64')).toBe(
+                uploadedFileData.toString('base64'),
+            );
+        },
+    );
 
     // Ensure that once the callback is completed, the file doesn't exist since we didn't tell it not to cleanup
     expect(tempPath).toBeTruthy();

src/lib/Uploads.ts

@@ -3,7 +3,6 @@ import * as fs from 'fs';
 import { DiskManager, pipeStreams } from '@carimus/node-disks';
 import { InvalidConfigError, PathNotUniqueError } from '../errors';
 import {
-    Upload,
     UploadedFile,
     UploadMeta,
     UploadRepository,
@@ -19,12 +18,12 @@ import { defaultSanitizeFilename, defaultGeneratePath } from './defaults';
  * TODO Support temporary URLs (e.g. presigned URLs for S3 buckets) for disks that support it
  * TODO Support transfer logic for transferring single uploads from one disk to another and in bulk.
  */
-export class Uploads {
-    private config: UploadsConfig;
-    private repository: UploadRepository;
+export class Uploads<UploadIdentifier> {
+    private config: UploadsConfig<UploadIdentifier>;
+    private repository: UploadRepository<UploadIdentifier>;
     private disks: DiskManager;
 
-    public constructor(config: UploadsConfig) {
+    public constructor(config: UploadsConfig<UploadIdentifier>) {
         this.config = config;
         this.repository = config.repository;
         if (typeof config.disks === 'object') {
@@ -160,7 +159,7 @@ export class Uploads {
         fileData: Buffer | Readable,
         uploadedAs: string,
         meta?: UploadMeta,
-    ): Promise<Upload> {
+    ): Promise<UploadIdentifier> {
         const uploadedFile = await this.place(fileData, uploadedAs);
         return this.repository.create(uploadedFile, meta);
     }
@@ -175,11 +174,11 @@ export class Uploads {
      * @param meta
      */
     public async update(
-        upload: Upload,
+        upload: UploadIdentifier,
         fileData: Buffer | Readable,
         uploadedAs: string,
         meta?: UploadMeta,
-    ): Promise<Upload> {
+    ): Promise<UploadIdentifier> {
         // Get the info about the old file and resolve its disk.
         const oldFile = await this.repository.getUploadedFileInfo(upload);
         const oldDisk = this.disks.getDisk(oldFile.disk);
@@ -206,9 +205,9 @@ export class Uploads {
      * @param meta
      */
     public async duplicate(
-        original: Upload,
+        original: UploadIdentifier,
         meta?: UploadMeta,
-    ): Promise<Upload> {
+    ): Promise<UploadIdentifier> {
         // Ask the repository for info on where and how the original upload file is stored.
         const originalFile = await this.repository.getUploadedFileInfo(
             original,
@@ -231,7 +230,7 @@ export class Uploads {
      * @param onlyFile If true, only thd disk file is deleted and not the upload in the repository.
      */
     public async delete(
-        upload: Upload,
+        upload: UploadIdentifier,
         onlyFile: boolean = false,
     ): Promise<void> {
         // Ask the repository for info on where and how the upload file is stored.
@@ -265,7 +264,7 @@ export class Uploads {
      * @param execute
      */
     public async withTempFile(
-        upload: Upload,
+        upload: UploadIdentifier,
         execute: ((path: string) => Promise<void> | void) | null = null,
     ): Promise<string> {
         // Ask the repository for info on where and how the upload file is stored.

src/support/MemoryRepository.test.ts

@@ -27,35 +27,27 @@ const bar: { file: UploadedFile; meta: UploadMeta } = {
 
 test('MemoryRepository implements create, update, delete, and getUploadedFileInfo', async () => {
     const repo = new MemoryRepository();
-    const upload = await repo.create(foo.file, foo.meta);
-    expect(await repo.getUploadedFileInfo(upload)).toMatchObject(foo.file);
-    const updatedUpload = await repo.update(upload, bar.file, bar.meta);
+    const uploadId = await repo.create(foo.file, foo.meta);
+    expect(await repo.getUploadedFileInfo(uploadId)).toMatchObject(foo.file);
+    const updatedUploadId = await repo.update(uploadId, bar.file, bar.meta);
     // Note that we expect `Upload`s to be long-lived identifiers (or contain such) so we test that here by
     // checking to ensure the file info is updated for our old upload reference too.
-    expect(await repo.getUploadedFileInfo(upload)).toMatchObject(bar.file);
-    expect(await repo.getUploadedFileInfo(updatedUpload)).toMatchObject(
+    expect(await repo.getUploadedFileInfo(uploadId)).toMatchObject(bar.file);
+    expect(await repo.getUploadedFileInfo(updatedUploadId)).toMatchObject(
         bar.file,
     );
     // `delete` on success resolves with undefined.
-    expect(repo.delete(upload)).resolves.toBeUndefined();
+    expect(repo.delete(uploadId)).resolves.toBeUndefined();
     // Since upload identifiers should be long-lived identifiers, this should reject since it's already been deleted.
-    expect(repo.delete(updatedUpload)).rejects.toBeTruthy();
-});
-
-test('MemoryRepository works with IDs as upload identifiers instead of entire records', async () => {
-    const repo = new MemoryRepository();
-    const fooUpload = await repo.create(foo.file, foo.meta);
-    const barUpload = await repo.create(bar.file, bar.meta);
-    expect(await repo.getUploadedFileInfo(fooUpload.id)).toBe(foo.file);
-    expect(await repo.getUploadedFileInfo(barUpload.id)).toBe(bar.file);
+    expect(repo.delete(updatedUploadId)).rejects.toBeTruthy();
 });
 
 test('MemoryRepository update handles optional meta by persisting old meta', async () => {
     const repo = new MemoryRepository();
-    const fooUpload = await repo.create(foo.file, foo.meta);
-    const fooUpload2 = await repo.update(fooUpload, bar.file);
-    expect(fooUpload.file).toMatchObject(foo.file);
-    expect(fooUpload.meta).toMatchObject(foo.meta);
-    expect(fooUpload2.file).toMatchObject(bar.file);
-    expect(fooUpload2.meta).toMatchObject(foo.meta);
+    const fooUploadId = await repo.create(foo.file, foo.meta);
+    expect(await repo.getUploadedFileInfo(fooUploadId)).toMatchObject(foo.file);
+    expect(await repo.getMeta(fooUploadId)).toMatchObject(foo.meta);
+    await repo.update(fooUploadId, bar.file);
+    expect(await repo.getUploadedFileInfo(fooUploadId)).toMatchObject(bar.file);
+    expect(await repo.getMeta(fooUploadId)).toMatchObject(foo.meta);
 });

src/support/MemoryRepository.ts

@@ -1,21 +1,25 @@
 import { UploadedFile, UploadMeta, UploadRepository } from '../types';
 
+export type MemoryRepositoryUploadIdentifier = number;
+
 export interface MemoryRepositoryRecord {
-    id: number;
+    id: MemoryRepositoryUploadIdentifier;
     file: UploadedFile;
     meta: UploadMeta;
 }
 
-type MemoryRepositoryUpload = MemoryRepositoryRecord | number;
-
-export class MemoryRepository implements UploadRepository {
-    private database: Map<number, MemoryRepositoryRecord> = new Map();
-    private counter: number = 0;
+export class MemoryRepository
+    implements UploadRepository<MemoryRepositoryUploadIdentifier> {
+    private database: Map<
+        MemoryRepositoryUploadIdentifier,
+        MemoryRepositoryRecord
+    > = new Map();
+    private counter: MemoryRepositoryUploadIdentifier = 0;
 
     /**
      * Get the next available ID. Mimics auto-increment starting at 1.
      */
-    private getNextID(): number {
+    private getNextID(): MemoryRepositoryUploadIdentifier {
         return ++this.counter;
     }
 
@@ -27,21 +31,22 @@ export class MemoryRepository implements UploadRepository {
     public async create(
         file: UploadedFile,
         meta?: UploadMeta,
-    ): Promise<MemoryRepositoryRecord> {
+    ): Promise<MemoryRepositoryUploadIdentifier> {
         const id = this.getNextID();
         const upload = { id, file, meta: meta || {} };
         this.database.set(id, upload);
-        return upload;
+        return upload.id;
     }
 
     /**
      * Resolve a record or ID to a fresh record straight from the database.
-     * @param upload
+     * @param id
      */
-    private resolve(upload: MemoryRepositoryUpload): MemoryRepositoryRecord {
-        const id = (typeof upload === 'number' ? upload : upload.id) || null;
+    private resolve(
+        id: MemoryRepositoryUploadIdentifier,
+    ): MemoryRepositoryRecord {
         if (!id) {
-            throw new Error(`Bad identifier or malformed record: ${id}`);
+            throw new Error(`Bad identifier: ${id}`);
         }
         const existingUpload = this.database.get(id);
         if (!existingUpload) {
@@ -57,25 +62,27 @@ export class MemoryRepository implements UploadRepository {
      * @param meta
      */
     public async update(
-        upload: MemoryRepositoryUpload,
+        upload: MemoryRepositoryUploadIdentifier,
         file: UploadedFile,
         meta?: UploadMeta,
-    ): Promise<MemoryRepositoryRecord> {
+    ): Promise<MemoryRepositoryUploadIdentifier> {
         const existingUpload = this.resolve(upload);
         const updatedUpload = {
             ...existingUpload,
             file,
             meta: meta || existingUpload.meta || {},
         };
         this.database.set(existingUpload.id, updatedUpload);
-        return updatedUpload;
+        return updatedUpload.id;
     }
 
     /**
      * @inheritDoc
      * @param upload
      */
-    public async delete(upload: MemoryRepositoryUpload): Promise<void> {
+    public async delete(
+        upload: MemoryRepositoryUploadIdentifier,
+    ): Promise<void> {
         const { id } = this.resolve(upload);
         this.database.delete(id);
     }
@@ -85,7 +92,7 @@ export class MemoryRepository implements UploadRepository {
      * @param upload
      */
     public async getUploadedFileInfo(
-        upload: MemoryRepositoryUpload,
+        upload: MemoryRepositoryUploadIdentifier,
     ): Promise<UploadedFile> {
         const existingUpload = this.resolve(upload);
         return existingUpload.file;
@@ -95,7 +102,9 @@ export class MemoryRepository implements UploadRepository {
      * @inheritDoc
      * @param upload
      */
-    public async getMeta(upload: MemoryRepositoryUpload): Promise<UploadMeta> {
+    public async getMeta(
+        upload: MemoryRepositoryUploadIdentifier,
+    ): Promise<UploadMeta> {
         const existingUpload = this.resolve(upload);
         return existingUpload.meta;
     }
@@ -105,7 +114,7 @@ export class MemoryRepository implements UploadRepository {
      * @param upload
      */
     public async find(
-        upload: MemoryRepositoryUpload,
+        upload: MemoryRepositoryUploadIdentifier,
     ): Promise<MemoryRepositoryRecord> {
         return this.resolve(upload);
     }
@@ -115,13 +124,22 @@ export class MemoryRepository implements UploadRepository {
      *
      * @param id
      */
-    public log(id?: number): void {
-        const rawEntries: [number, MemoryRepositoryRecord | undefined][] = id
-            ? [[id, this.database.get(id)]]
-            : [...this.database.entries()];
-        const entries: [number, MemoryRepositoryRecord][] = rawEntries.filter(
-            (entry: [number, MemoryRepositoryRecord | undefined]) => !!entry[1],
-        ) as [number, MemoryRepositoryRecord][];
+    public log(id?: MemoryRepositoryUploadIdentifier): void {
+        const rawEntries: [
+            MemoryRepositoryUploadIdentifier,
+            MemoryRepositoryRecord | undefined
+        ][] = id ? [[id, this.database.get(id)]] : [...this.database.entries()];
+        const entries: [
+            MemoryRepositoryUploadIdentifier,
+            MemoryRepositoryRecord
+        ][] = rawEntries.filter(
+            (
+                entry: [
+                    MemoryRepositoryUploadIdentifier,
+                    MemoryRepositoryRecord | undefined
+                ],
+            ) => !!entry[1],
+        ) as [MemoryRepositoryUploadIdentifier, MemoryRepositoryRecord][];
         console.table(
             entries.map(([id, record]) => {
                 return { id, ...record.file, meta: record.meta };

src/support/index.ts

@@ -1 +1,4 @@
-export { MemoryRepository } from './MemoryRepository';
+export {
+    MemoryRepository,
+    MemoryRepositoryUploadIdentifier,
+} from './MemoryRepository';