Merge pull request #44 from h39s/refactorFilters

Refactor filters

package.json

@@ -107,6 +107,7 @@
     ]
   },
   "dependencies": {
+    "ajv": "^8.12.0",
     "d3": "^7.6.1",
     "electron-debug": "^3.2.0",
     "electron-log": "^4.4.7",
@@ -119,6 +120,7 @@
   },
   "devDependencies": {
     "@pmmmwh/react-refresh-webpack-plugin": "^0.5.6",
+    "@rkesters/typescript-json-validator": "^3.0.3",
     "@teamsupercell/typings-for-css-modules-loader": "^2.5.1",
     "@testing-library/jest-dom": "^5.16.4",
     "@testing-library/react": "^13.2.0",

release/app/package.json

@@ -1,6 +1,6 @@
 {
   "name": "aqua",
-  "version": "1.0.1",
+  "version": "1.1.0",
   "description": "An audio equalizer app",
   "license": "MIT",
   "author": {

src/__tests__/cucumber_tests/shared_steps/equalizerApo.ts

@@ -1,5 +1,5 @@
 import { DefineStepFunction } from 'jest-cucumber';
-import { IState } from 'common/constants';
+import { getDefaultFilterWithId, IState } from 'common/constants';
 import { flush } from 'main/flush';
 import { getConfigPath, isEqualizerAPOInstalled } from 'main/registry';
 
@@ -19,7 +19,7 @@ export const givenCanWriteToAquaConfig = (given: DefineStepFunction) => {
       isAutoPreAmpOn: false,
       isGraphViewOn: false,
       preAmp: 0,
-      filters: [],
+      filters: { unique_id: getDefaultFilterWithId() },
     };
     const configDirPath = await getConfigPath();
     flush(emptyState, configDirPath);

src/__tests__/test_presets/presetV1

@@ -0,0 +1 @@
+{"preAmp":0,"filters":{"123":{"id":"123","frequency":2,"gain":-4,"quality":6,"type":"PK"},"456":{"id":"456","frequency":8,"gain":-10,"quality":1.2,"type":"PK"}}}

src/__tests__/test_presets/presetV2

@@ -0,0 +1 @@
+{"preAmp":0,"filters":{"0a04dcf8":{"id":"0a04dcf8","frequency":32,"gain":-4,"quality":1,"type":"PK"}, "d77a7415":{"id":"d77a7415","frequency":16000,"gain":0,"quality":1,"type":"PK"}}}

src/__tests__/unit_tests/common/utils.test.ts

@@ -1,5 +1,15 @@
-import { getDefaultState } from 'common/constants';
-import { computeAvgFreq, roundToPrecision } from 'common/utils';
+import {
+  getDefaultState,
+  IFiltersMap,
+  RESERVED_FILE_NAMES_SET,
+} from 'common/constants';
+import {
+  cloneFilters,
+  computeAvgFreq,
+  isRestrictedPresetName,
+  roundToPrecision,
+} from 'common/utils';
+import { sortHelper } from 'renderer/utils/utils';
 
 describe('utils', () => {
   describe('roundToPrecision', () => {
@@ -16,20 +26,54 @@ describe('utils', () => {
   });
 
   describe('computeAvgFreq', () => {
-    const filters = [...getDefaultState().filters];
+    const filters = Object.values(getDefaultState().filters).sort(sortHelper);
     it('should compute average of first filter and min frequency for index 0', () => {
-      expect(computeAvgFreq(filters, 0)).toBe(6);
+      expect(computeAvgFreq(null, filters[0])).toBe(6);
     });
 
     it('should compute average of last filter and max frequency for last index', () => {
-      expect(computeAvgFreq(filters, filters.length)).toBe(17889);
+      expect(computeAvgFreq(filters[filters.length - 1], null)).toBe(17889);
     });
 
     it('should compute average of neighbouring filters for intermediary indices', () => {
       // Compute harmonic mean between 64Hz and 125Hz
-      expect(computeAvgFreq(filters, 2)).toBe(89);
+      expect(computeAvgFreq(filters[1], filters[2])).toBe(89);
       // Compute harmonic mean between 250Hz and 500Hz
-      expect(computeAvgFreq(filters, 4)).toBe(354);
+      expect(computeAvgFreq(filters[3], filters[4])).toBe(354);
+    });
+  });
+
+  describe('isRestrictedPresetName', () => {
+    it('should return true for restricted names', () => {
+      RESERVED_FILE_NAMES_SET.forEach((name) => {
+        expect(isRestrictedPresetName(name)).toBe(true);
+      });
+    });
+
+    it('should return false for non restricted names', () => {
+      expect(isRestrictedPresetName('greatest preset of all time')).toBe(false);
+    });
+  });
+
+  describe('cloneFilters', () => {
+    const filtersMap: IFiltersMap = getDefaultState().filters;
+    const copy = cloneFilters(filtersMap);
+    it('should have same values', () => {
+      Object.entries(filtersMap).forEach(([id, filter]) => {
+        expect(copy[id]).toStrictEqual(filter);
+      });
+      Object.entries(copy).forEach(([id, filter]) => {
+        expect(filtersMap[id]).toStrictEqual(filter);
+      });
+    });
+
+    it('should have distinct IFilter objects', () => {
+      Object.entries(filtersMap).forEach(([id, filter]) => {
+        filter.id = `${id}*`;
+      });
+      Object.entries(copy).forEach(([id, filter]) => {
+        expect(filter.id).toBe(`${id}`);
+      });
     });
   });
 });

src/__tests__/unit_tests/main/flush.test.ts

@@ -0,0 +1,121 @@
+import {
+  deletePreset,
+  doesPresetExist,
+  fetchPreset,
+  renamePreset,
+  savePreset,
+} from 'main/flush';
+import fs from 'fs';
+import { FilterTypeEnum, IPresetV2 } from 'common/constants';
+import path from 'path';
+
+const PRESETS_DIR = 'src/__tests__/test_presets';
+
+describe('flush', () => {
+  describe('fetchPreset', () => {
+    it('should read succesfully a preset of the IPresetV2 format', () => {
+      const presetName = 'presetV2';
+      const preset = fetchPreset(presetName, PRESETS_DIR);
+      expect(preset).toStrictEqual({
+        preAmp: 0,
+        filters: {
+          '0a04dcf8': {
+            id: '0a04dcf8',
+            frequency: 32,
+            gain: -4,
+            quality: 1,
+            type: FilterTypeEnum.PK,
+          },
+          d77a7415: {
+            id: 'd77a7415',
+            frequency: 16000,
+            gain: 0,
+            quality: 1,
+            type: FilterTypeEnum.PK,
+          },
+        },
+      });
+    });
+
+    it('should read succesfully a preset of the IPresetV1 format and replace it with a IPresetV2 format', () => {
+      const presetName = 'presetV1';
+      const content = fs.readFileSync(path.join(PRESETS_DIR, presetName), {
+        encoding: 'utf8',
+      });
+      const preset = fetchPreset('presetV1', PRESETS_DIR);
+      expect(preset).toStrictEqual({
+        preAmp: 0,
+        filters: {
+          '123': { id: '123', frequency: 2, gain: -4, quality: 6, type: 'PK' },
+          '456': {
+            id: '456',
+            frequency: 8,
+            gain: -10,
+            quality: 1.2,
+            type: FilterTypeEnum.PK,
+          },
+        },
+      });
+      fs.writeFileSync(path.join(PRESETS_DIR, presetName), content, {
+        encoding: 'utf8',
+      });
+    });
+  });
+
+  describe('save and delete preset', () => {
+    it('should save and delete a preset', () => {
+      const presetName = 'newPreset';
+      const preset: IPresetV2 = {
+        preAmp: 0,
+        filters: {
+          '123': {
+            id: '123',
+            frequency: 2,
+            gain: -4,
+            quality: 6,
+            type: FilterTypeEnum.PK,
+          },
+        },
+      };
+      savePreset(presetName, preset, PRESETS_DIR);
+      expect(doesPresetExist(presetName, PRESETS_DIR)).toBe(true);
+      deletePreset(presetName, PRESETS_DIR);
+      expect(doesPresetExist(presetName, PRESETS_DIR)).toBe(false);
+    });
+  });
+
+  describe('doesPresetExist', () => {
+    it('should return true for an existing preset', () => {
+      const presetName = 'presetV1';
+      expect(doesPresetExist(presetName, PRESETS_DIR)).toBe(true);
+    });
+
+    it('should return false for a non-existing preset', () => {
+      const presetName = '404_not_found';
+      expect(doesPresetExist(presetName, PRESETS_DIR)).toBe(false);
+    });
+  });
+
+  describe('renamePreset', () => {
+    it('should rename presets', () => {
+      const oldPresetName = 'presetV1';
+      const newPresetName = 'presetV11';
+      renamePreset(oldPresetName, newPresetName, PRESETS_DIR);
+      expect(() => fetchPreset(oldPresetName, PRESETS_DIR)).toThrow();
+      expect(fetchPreset(newPresetName, PRESETS_DIR)).toStrictEqual({
+        preAmp: 0,
+        filters: {
+          '123': { id: '123', frequency: 2, gain: -4, quality: 6, type: 'PK' },
+          '456': {
+            id: '456',
+            frequency: 8,
+            gain: -10,
+            quality: 1.2,
+            type: FilterTypeEnum.PK,
+          },
+        },
+      });
+      renamePreset(newPresetName, oldPresetName, PRESETS_DIR);
+    });
+  });
+});

src/common/constants.ts

@@ -45,15 +45,17 @@ export const WINDOW_WIDTH = 1428;
 export const WINDOW_HEIGHT = 625;
 export const WINDOW_HEIGHT_EXPANDED = 1036;
 
-export const PRESETS_DIR = 'presets';
-
 export const PREAMP_REGEX = new RegExp('^Preamp: (-\\d\\.\\d) dB$');
 export const FILTER_REGEX = new RegExp(
   '^Filter [1-9]\\d?: ON (PK|LS|HS) Fc ([1-9]\\d{0,3}|[1,2]\\d{4}) Hz Gain (-?[1,2]?\\d\\.\\d) dB Q (\\d\\.\\d\\d)$'
 );
 
 /** ----- Application Interfaces ----- */
 
+export interface IFiltersMap {
+  [key: string]: IFilter;
+} // key is the same id as whats in IFilter
+
 export interface IFilter {
   id: string;
   frequency: number;
@@ -67,14 +69,19 @@ export interface IState {
   isAutoPreAmpOn: boolean;
   isGraphViewOn: boolean;
   preAmp: number;
-  filters: IFilter[];
+  filters: IFiltersMap;
 }
 
-export interface IPreset {
+export interface IPresetV1 {
   preAmp: number;
   filters: IFilter[];
 }
 
+export interface IPresetV2 {
+  preAmp: number;
+  filters: IFiltersMap;
+}
+
 /** ----- Default Values ----- */
 
 const FIXED_FREQUENCIES = [
@@ -88,17 +95,21 @@ const DEFAULT_FILTER_TEMPLATE = {
   type: FilterTypeEnum.PK,
 };
 
-export const getDefaultFilter = () => {
+export const getDefaultFilterWithId = (): IFilter => {
   return {
     id: uid(8),
     ...DEFAULT_FILTER_TEMPLATE,
   };
 };
 
-const getDefaultFilters = (): IFilter[] =>
-  FIXED_FREQUENCIES.map((f) => {
-    return { ...getDefaultFilter(), frequency: f };
+const getDefaultFilters = (): IFiltersMap => {
+  const filters: IFiltersMap = {};
+  FIXED_FREQUENCIES.forEach((f) => {
+    const filter: IFilter = { ...getDefaultFilterWithId(), frequency: f };
+    filters[filter.id] = filter;
   });
+  return filters;
+};
 
 export const getDefaultState = (): IState => {
   return {

src/common/utils.ts

@@ -1,4 +1,5 @@
 import {
+  IFiltersMap,
   IFilter,
   MAX_FREQUENCY,
   MIN_FREQUENCY,
@@ -10,10 +11,12 @@ export const roundToPrecision = (value: number, precision: number) => {
   return Math.round(value * precisionFactor) / precisionFactor;
 };
 
-export const computeAvgFreq = (filters: IFilter[], index: number) => {
-  const lo = index === 0 ? MIN_FREQUENCY : filters[index - 1].frequency;
-  const hi =
-    index === filters.length ? MAX_FREQUENCY : filters[index].frequency;
+export const computeAvgFreq = (
+  leftFilter: IFilter | null,
+  rightFilter: IFilter | null
+) => {
+  const lo = leftFilter ? leftFilter.frequency : MIN_FREQUENCY;
+  const hi = rightFilter ? rightFilter.frequency : MAX_FREQUENCY;
   const exponent = (Math.log10(lo) + Math.log10(hi)) / 2;
   return roundToPrecision(10 ** exponent, 0);
 };
@@ -23,3 +26,11 @@ export const computeAvgFreq = (filters: IFilter[], index: number) => {
 // have manually confirmed this.
 export const isRestrictedPresetName = (newName: string) =>
   RESERVED_FILE_NAMES_SET.has(newName.toUpperCase());
+
+export const cloneFilters = (filters: IFiltersMap) => {
+  const filtersClone: IFiltersMap = {};
+  Object.entries(filters).forEach(([id, filter]) => {
+    filtersClone[id] = { ...filter };
+  });
+  return filtersClone;
+};