feat: modify datafile managers to use datafile strings instead of objects

packages/datafile-manager/CHANGELOG.md

@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ## [Unreleased]
 Changes that have landed but are not yet released.
 
+### Changed
+
+- Modified datafile manager to accept, process, and return the datafile's string representation instead of the datafile object.
+- Remove JSON parsing of response received from datafile fetch request
+  - Responsibility of validating the datafile now solely belongs to the project config manager
+
 ## [0.7.0] - July 28, 2020
 
 ### Changed

packages/datafile-manager/__test__/httpPollingDatafileManager.spec.ts

@@ -109,11 +109,11 @@ describe('httpPollingDatafileManager', () => {
 
   describe('when constructed with sdkKey and datafile and autoUpdate: true,', () => {
     beforeEach(() => {
-      manager = new TestDatafileManager({ datafile: { foo: 'abcd' }, sdkKey: '123', autoUpdate: true });
+      manager = new TestDatafileManager({ datafile: JSON.stringify({ foo: 'abcd' }), sdkKey: '123', autoUpdate: true });
     });
 
     it('returns the passed datafile from get', () => {
-      expect(manager.get()).toEqual({ foo: 'abcd' });
+      expect(JSON.parse(manager.get())).toEqual({ foo: 'abcd' });
     });
 
     it('after being started, fetches the datafile, updates itself, and updates itself again after a timeout', async () => {
@@ -134,28 +134,30 @@ describe('httpPollingDatafileManager', () => {
       manager.start();
       expect(manager.responsePromises.length).toBe(1);
       await manager.responsePromises[0];
-      expect(manager.get()).toEqual({ foo: 'bar' });
+      expect(JSON.parse(manager.get())).toEqual({ foo: 'bar' });
       updateFn.mockReset();
 
       await advanceTimersByTime(300000);
 
       expect(manager.responsePromises.length).toBe(2);
       await manager.responsePromises[1];
       expect(updateFn).toBeCalledTimes(1);
-      expect(updateFn).toBeCalledWith({
-        datafile: { fooz: 'barz' },
-      });
-      expect(manager.get()).toEqual({ fooz: 'barz' });
+      expect(updateFn.mock.calls[0][0]).toEqual({ datafile: '{"fooz": "barz"}' });
+      expect(JSON.parse(manager.get())).toEqual({ fooz: 'barz' });
     });
   });
 
   describe('when constructed with sdkKey and datafile and autoUpdate: false,', () => {
     beforeEach(() => {
-      manager = new TestDatafileManager({ datafile: { foo: 'abcd' }, sdkKey: '123', autoUpdate: false });
+      manager = new TestDatafileManager({
+        datafile: JSON.stringify({ foo: 'abcd' }),
+        sdkKey: '123',
+        autoUpdate: false,
+      });
     });
 
     it('returns the passed datafile from get', () => {
-      expect(manager.get()).toEqual({ foo: 'abcd' });
+      expect(JSON.parse(manager.get())).toEqual({ foo: 'abcd' });
     });
 
     it('after being started, fetches the datafile, updates itself once, but does not schedule a future update', async () => {
@@ -167,7 +169,7 @@ describe('httpPollingDatafileManager', () => {
       manager.start();
       expect(manager.responsePromises.length).toBe(1);
       await manager.responsePromises[0];
-      expect(manager.get()).toEqual({ foo: 'bar' });
+      expect(JSON.parse(manager.get())).toEqual({ foo: 'bar' });
       expect(getTimerCount()).toBe(0);
     });
   });
@@ -179,7 +181,7 @@ describe('httpPollingDatafileManager', () => {
 
     describe('initial state', () => {
       it('returns null from get before becoming ready', () => {
-        expect(manager.get()).toBeNull();
+        expect(manager.get()).toEqual('');
       });
     });
 
@@ -205,28 +207,7 @@ describe('httpPollingDatafileManager', () => {
         });
         manager.start();
         await manager.onReady();
-        expect(manager.get()).toEqual({ foo: 'bar' });
-      });
-
-      it('does not update if the response body is not valid json', async () => {
-        manager.queuedResponses.push(
-          {
-            statusCode: 200,
-            body: '{"foo" "',
-            headers: {},
-          },
-          {
-            statusCode: 200,
-            body: '{"foo": "bar"}',
-            headers: {},
-          }
-        );
-        manager.start();
-        await manager.onReady();
-        await advanceTimersByTime(1000);
-        expect(manager.responsePromises.length).toBe(2);
-        await manager.responsePromises[1];
-        expect(manager.get()).toEqual({ foo: 'bar' });
+        expect(JSON.parse(manager.get())).toEqual({ foo: 'bar' });
       });
 
       describe('live updates', () => {
@@ -275,22 +256,22 @@ describe('httpPollingDatafileManager', () => {
 
           manager.start();
           await manager.onReady();
-          expect(manager.get()).toEqual({ foo: 'bar' });
+          expect(JSON.parse(manager.get())).toEqual({ foo: 'bar' });
           expect(updateFn).toBeCalledTimes(0);
 
           await advanceTimersByTime(1000);
           await manager.responsePromises[1];
           expect(updateFn).toBeCalledTimes(1);
-          expect(updateFn.mock.calls[0][0]).toEqual({ datafile: { foo2: 'bar2' } });
-          expect(manager.get()).toEqual({ foo2: 'bar2' });
+          expect(updateFn.mock.calls[0][0]).toEqual({ datafile: '{"foo2": "bar2"}' });
+          expect(JSON.parse(manager.get())).toEqual({ foo2: 'bar2' });
 
           updateFn.mockReset();
 
           await advanceTimersByTime(1000);
           await manager.responsePromises[2];
           expect(updateFn).toBeCalledTimes(1);
-          expect(updateFn.mock.calls[0][0]).toEqual({ datafile: { foo3: 'bar3' } });
-          expect(manager.get()).toEqual({ foo3: 'bar3' });
+          expect(updateFn.mock.calls[0][0]).toEqual({ datafile: '{"foo3": "bar3"}' });
+          expect(JSON.parse(manager.get())).toEqual({ foo3: 'bar3' });
         });
 
         describe('when the update interval time fires before the request is complete', () => {
@@ -351,15 +332,15 @@ describe('httpPollingDatafileManager', () => {
 
           manager.start();
           await manager.onReady();
-          expect(manager.get()).toEqual({ foo: 'bar' });
+          expect(JSON.parse(manager.get())).toEqual({ foo: 'bar' });
 
           advanceTimersByTime(1000);
 
           expect(manager.responsePromises.length).toBe(2);
           manager.stop();
           await manager.responsePromises[1];
           // Should not have updated datafile since manager was stopped
-          expect(manager.get()).toEqual({ foo: 'bar' });
+          expect(JSON.parse(manager.get())).toEqual({ foo: 'bar' });
         });
 
         it('calls abort on the current request if there is a current request when stop is called', async () => {
@@ -399,7 +380,7 @@ describe('httpPollingDatafileManager', () => {
           // Trigger the update, should fetch the next response which should succeed, then we get ready
           advanceTimersByTime(1000);
           await manager.onReady();
-          expect(manager.get()).toEqual({ foo: 'bar' });
+          expect(JSON.parse(manager.get())).toEqual({ foo: 'bar' });
         });
 
         describe('newness checking', () => {
@@ -424,7 +405,7 @@ describe('httpPollingDatafileManager', () => {
 
             manager.start();
             await manager.onReady();
-            expect(manager.get()).toEqual({ foo: 'bar' });
+            expect(JSON.parse(manager.get())).toEqual({ foo: 'bar' });
             // First response promise was for the initial 200 response
             expect(manager.responsePromises.length).toBe(1);
             // Trigger the queued update
@@ -434,7 +415,7 @@ describe('httpPollingDatafileManager', () => {
             await manager.responsePromises[1];
             // Since the response was 304, updateFn should not have been called
             expect(updateFn).toBeCalledTimes(0);
-            expect(manager.get()).toEqual({ foo: 'bar' });
+            expect(JSON.parse(manager.get())).toEqual({ foo: 'bar' });
           });
 
           it('sends if-modified-since using the last observed response last-modified', async () => {
@@ -559,7 +540,7 @@ describe('httpPollingDatafileManager', () => {
       });
       manager.start();
       await manager.onReady();
-      expect(manager.get()).toEqual({ foo: 'bar' });
+      expect(JSON.parse(manager.get())).toEqual({ foo: 'bar' });
     });
 
     it('does not schedule a live update after ready', async () => {
@@ -679,7 +660,7 @@ describe('httpPollingDatafileManager', () => {
       expect(manager.get()).toEqual({ name: 'keyThatExists' });
       expect(updateFn).toBeCalledTimes(0);
       await advanceTimersByTime(50);
-      expect(manager.get()).toEqual({ foo: 'bar' });
+      expect(JSON.parse(manager.get())).toEqual({ foo: 'bar' });
       expect(updateFn).toBeCalledTimes(1);
     });
 
@@ -693,8 +674,9 @@ describe('httpPollingDatafileManager', () => {
       manager.start();
       await manager.onReady();
       await advanceTimersByTime(50);
-      expect(manager.get()).toEqual({ foo: 'bar' });
-      expect(cacheSetSpy).toBeCalledWith('opt-datafile-keyThatExists', { foo: 'bar' });
+      expect(JSON.parse(manager.get())).toEqual({ foo: 'bar' });
+      expect(cacheSetSpy.mock.calls[0][0]).toEqual('opt-datafile-keyThatExists');
+      expect(JSON.parse(cacheSetSpy.mock.calls[0][1])).toEqual({ foo: 'bar' });
     });
   });
 
@@ -721,7 +703,7 @@ describe('httpPollingDatafileManager', () => {
       manager.start();
       await advanceTimersByTime(50);
       await manager.onReady();
-      expect(manager.get()).toEqual({ foo: 'bar' });
+      expect(JSON.parse(manager.get())).toEqual({ foo: 'bar' });
       expect(updateFn).toBeCalledTimes(0);
     });
   });

packages/datafile-manager/src/datafileManager.ts

@@ -16,7 +16,7 @@
 import PersistentKeyValueCache from './persistentKeyValueCache';
 
 export interface DatafileUpdate {
-  datafile: object;
+  datafile: string;
 }
 
 export interface DatafileUpdateListener {
@@ -31,14 +31,14 @@ interface Managed {
 }
 
 export interface DatafileManager extends Managed {
-  get: () => object | null;
+  get: () => string;
   on: (eventName: string, listener: DatafileUpdateListener) => () => void;
   onReady: () => Promise<void>;
 }
 
 export interface DatafileManagerConfig {
   autoUpdate?: boolean;
-  datafile?: object;
+  datafile?: string;
   sdkKey: string;
   updateInterval?: number;
   urlTemplate?: string;

packages/datafile-manager/src/httpPollingDatafileManager.ts

@@ -63,7 +63,7 @@ export default abstract class HttpPollingDatafileManager implements DatafileMana
   // Return any default configuration options that should be applied
   protected abstract getConfigDefaults(): Partial<DatafileManagerConfig>;
 
-  private currentDatafile: object | null;
+  private currentDatafile: string;
 
   private readonly readyPromise: Promise<void>;
 
@@ -131,7 +131,7 @@ export default abstract class HttpPollingDatafileManager implements DatafileMana
         this.resolveReadyPromise();
       }
     } else {
-      this.currentDatafile = null;
+      this.currentDatafile = '';
     }
 
     this.isStarted = false;
@@ -152,7 +152,7 @@ export default abstract class HttpPollingDatafileManager implements DatafileMana
     this.syncOnCurrentRequestComplete = false;
   }
 
-  get(): object | null {
+  get(): string {
     return this.currentDatafile;
   }
 
@@ -222,7 +222,7 @@ export default abstract class HttpPollingDatafileManager implements DatafileMana
     this.trySavingLastModified(response.headers);
 
     const datafile = this.getNextDatafileFromResponse(response);
-    if (datafile !== null) {
+    if (datafile !== '') {
       logger.info('Updating datafile from response');
       this.currentDatafile = datafile;
       this.cache.set(this.cacheKey, datafile);
@@ -305,35 +305,18 @@ export default abstract class HttpPollingDatafileManager implements DatafileMana
     }, nextUpdateDelay);
   }
 
-  private getNextDatafileFromResponse(response: Response): object | null {
+  private getNextDatafileFromResponse(response: Response): string {
     logger.debug('Response status code: %s', response.statusCode);
     if (typeof response.statusCode === 'undefined') {
-      return null;
+      return '';
     }
     if (response.statusCode === 304) {
-      return null;
+      return '';
     }
     if (isSuccessStatusCode(response.statusCode)) {
-      return this.tryParsingBodyAsJSON(response.body);
+      return response.body;
     }
-    return null;
-  }
-
-  private tryParsingBodyAsJSON(body: string): object | null {
-    let parseResult: any;
-    try {
-      parseResult = JSON.parse(body);
-    } catch (err) {
-      logger.error('Error parsing response body: %s', err.message, err);
-      return null;
-    }
-    let datafileObj: object | null = null;
-    if (typeof parseResult === 'object' && parseResult !== null) {
-      datafileObj = parseResult;
-    } else {
-      logger.error('Error parsing response body: was not an object');
-    }
-    return datafileObj;
+    return '';
   }
 
   private trySavingLastModified(headers: Headers): void {