From 973f3e3c8b9c93b3d096f54efc042281f8eb8d5f Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Sat, 2 Sep 2023 19:44:29 +0200
Subject: [PATCH] Simplify ServersImporter using file.text() instead of a
 FileReader

---
 src/servers/services/ServersImporter.ts       | 30 ++++++-------------
 src/servers/services/provideServices.ts       |  3 +-
 test/servers/services/ServersImporter.test.ts | 23 ++++++--------
 3 files changed, 19 insertions(+), 37 deletions(-)

diff --git a/src/servers/services/ServersImporter.ts b/src/servers/services/ServersImporter.ts
index 2d47b7b5..a4dd0b7e 100644
--- a/src/servers/services/ServersImporter.ts
+++ b/src/servers/services/ServersImporter.ts
@@ -8,32 +8,20 @@ const validateServers = (servers: any): servers is ServerData[] =>
   Array.isArray(servers) && servers.every(validateServer);
 
 export class ServersImporter {
-  public constructor(private readonly csvToJson: CsvToJson, private readonly fileReaderFactory: () => FileReader) {}
+  public constructor(private readonly csvToJson: CsvToJson) {}
 
-  public readonly importServersFromFile = async (file?: File | null): Promise<ServerData[]> => {
+  public async importServersFromFile(file: File | null | undefined): Promise<ServerData[]> {
     if (!file) {
       throw new Error('No file provided');
     }
 
-    const reader = this.fileReaderFactory();
+    const content = await file.text();
+    const servers = await this.csvToJson(content);
 
-    return new Promise((resolve, reject) => {
-      reader.addEventListener('loadend', async (e: ProgressEvent<FileReader>) => {
-        try {
-          // TODO Read as stream, otherwise, if the file is too big, this will block the browser tab
-          const content = e.target?.result?.toString() ?? '';
-          const servers = await this.csvToJson(content);
+    if (!validateServers(servers)) {
+      throw new Error('Provided file does not have the right format.');
+    }
 
-          if (!validateServers(servers)) {
-            throw new Error('Provided file does not have the right format.');
-          }
-
-          resolve(servers);
-        } catch (error) {
-          reject(error);
-        }
-      });
-      reader.readAsText(file);
-    });
-  };
+    return servers;
+  }
 }
diff --git a/src/servers/services/provideServices.ts b/src/servers/services/provideServices.ts
index f4b6b63e..cee6552b 100644
--- a/src/servers/services/provideServices.ts
+++ b/src/servers/services/provideServices.ts
@@ -62,8 +62,7 @@ export const provideServices = (bottle: Bottle, connect: ConnectDecorator) => {
   bottle.decorator('ServerError', connect(['servers', 'selectedServer']));
 
   // Services
-  bottle.constant('fileReaderFactory', () => new FileReader());
-  bottle.service('ServersImporter', ServersImporter, 'csvToJson', 'fileReaderFactory');
+  bottle.service('ServersImporter', ServersImporter, 'csvToJson');
   bottle.service('ServersExporter', ServersExporter, 'Storage', 'window', 'jsonToCsv');
 
   // Actions
diff --git a/test/servers/services/ServersImporter.test.ts b/test/servers/services/ServersImporter.test.ts
index 7f457c30..c5180fec 100644
--- a/test/servers/services/ServersImporter.test.ts
+++ b/test/servers/services/ServersImporter.test.ts
@@ -5,18 +5,13 @@ import { ServersImporter } from '../../../src/servers/services/ServersImporter';
 describe('ServersImporter', () => {
   const servers: RegularServer[] = [fromPartial<RegularServer>({}), fromPartial<RegularServer>({})];
   const csvjsonMock = vi.fn().mockResolvedValue(servers);
-  const readAsText = vi.fn();
-  const fileReaderMock = fromPartial<FileReader>({
-    readAsText,
-    addEventListener: ((_eventName: string, listener: (e: ProgressEvent<FileReader>) => void) => listener(
-      fromPartial({ target: { result: '' } }),
-    )) as any,
-  });
-  const importer = new ServersImporter(csvjsonMock, () => fileReaderMock);
+  const text = vi.fn().mockReturnValue('');
+  const fileMock = () => fromPartial<File>({ text });
+  const importer = new ServersImporter(csvjsonMock);
 
   describe('importServersFromFile', () => {
-    it('rejects with error if no file was provided', async () => {
-      await expect(importer.importServersFromFile()).rejects.toEqual(
+    it.each([[null], [undefined]])('rejects with error if no file was provided', async (file) => {
+      await expect(importer.importServersFromFile(file)).rejects.toEqual(
         new Error('No file provided'),
       );
     });
@@ -26,7 +21,7 @@ describe('ServersImporter', () => {
 
       csvjsonMock.mockRejectedValue(expectedError);
 
-      await expect(importer.importServersFromFile(fromPartial({ type: 'text/html' }))).rejects.toEqual(expectedError);
+      await expect(importer.importServersFromFile(fileMock())).rejects.toEqual(expectedError);
     });
 
     it.each([
@@ -55,7 +50,7 @@ describe('ServersImporter', () => {
     ])('rejects with error if provided file does not parse to valid list of servers', async (parsedObject) => {
       csvjsonMock.mockResolvedValue(parsedObject);
 
-      await expect(importer.importServersFromFile(fromPartial({ type: 'text/html' }))).rejects.toEqual(
+      await expect(importer.importServersFromFile(fileMock())).rejects.toEqual(
         new Error('Provided file does not have the right format.'),
       );
     });
@@ -76,10 +71,10 @@ describe('ServersImporter', () => {
 
       csvjsonMock.mockResolvedValue(expectedServers);
 
-      const result = await importer.importServersFromFile(fromPartial({}));
+      const result = await importer.importServersFromFile(fileMock());
 
       expect(result).toEqual(expectedServers);
-      expect(readAsText).toHaveBeenCalledTimes(1);
+      expect(text).toHaveBeenCalledTimes(1);
       expect(csvjsonMock).toHaveBeenCalledTimes(1);
     });
   });