Simplify resource manager (#4)

- ResourceLoader interface is just a function that returns either the resource or a promise of the resource
- Removed stream & emitter functionality

Author: mjc1283

packages/js-web-sdk/packages/js-web-sdk/src/DatafileLoaders.ts

@@ -1,5 +1,5 @@
 import { OptimizelyDatafile } from './Datafile'
-import { ResourceLoader, ResourceEmitter } from './ResourceStream'
+import { ResourceLoader } from './ResourceManager'
 const fetch = require('node-fetch')
 
 export class ProvidedDatafileLoader implements ResourceLoader<OptimizelyDatafile> {
@@ -9,13 +9,8 @@ export class ProvidedDatafileLoader implements ResourceLoader<OptimizelyDatafile
     this.datafile = config.datafile
   }
 
-  load(emitter: ResourceEmitter<OptimizelyDatafile>): void {
-    emitter.data({
-      resourceKey: 'datafile',
-      resource: this.datafile,
-      metadata: { source: 'fresh' },
-    })
-    emitter.ready()
+  public load() {
+    return this.datafile;
   }
 }
 
@@ -30,74 +25,49 @@ type FetchUrlCacheEntry = {
 export class FetchUrlDatafileLoader implements ResourceLoader<OptimizelyDatafile> {
   private sdkKey: string
   private localStorageKey: string
-  private preferCached: boolean
-  private backgroundLoadIfCacheHit: boolean
+
+  // 1 week in ms = 7 days * 24 hours * 60 minutes * 60 seconds * 1000 ms
+  private static MAX_CACHE_AGE_MS: number = 7 * 24 * 60 * 60 * 1000
 
   constructor(config: {
     sdkKey: string
     localStorageKey?: string
-    preferCached?: boolean
-    backgroundLoadIfCacheHit?: boolean
   }) {
     this.sdkKey = config.sdkKey
     this.localStorageKey = config.localStorageKey || 'optly_fs_datafile'
-
-    this.backgroundLoadIfCacheHit = !!config.backgroundLoadIfCacheHit
-    this.preferCached = !!config.preferCached
   }
 
-  load(emitter: ResourceEmitter<OptimizelyDatafile>): void {
+  public load() {
     const cacheResult = this.getFromCache()
-    if (cacheResult && this.shouldUseCache(cacheResult)) {
-      emitter.data({
-        resourceKey: 'datafile',
-        resource: cacheResult.datafile,
-        metadata: { source: 'cache' },
-      })
-      if (this.preferCached) {
-        emitter.ready()
-      }
-      if (!this.backgroundLoadIfCacheHit) {
-        // no need to load anything else, we're done
-        return
-      }
-    }
-    this.fetchDatafile().then(
+    const freshDatafileFetch = this.fetchDatafile().then(
       datafile => {
-        emitter.data({
-          resourceKey: 'datafile',
-          resource: datafile,
-          metadata: { source: 'fresh' },
-        })
-        emitter.ready()
-        const cacheEntry: FetchUrlCacheEntry = {
-          datafile,
-          metadata: {
-            timestampCached: new Date().getTime(),
-          },
-        }
-        this.saveToCache(cacheEntry)
-      },
-      response => {
-        emitter.error({
-          resourceKey: 'datafile',
-          reason: 'failed to load',
-        })
-      },
+        this.saveToCache(datafile)
+        return datafile;
+      }
     )
+    if (cacheResult && this.shouldUseCache(cacheResult)) {
+      return cacheResult.datafile;
+    }
+    return freshDatafileFetch;
   }
 
-  saveToCache(toSave: FetchUrlCacheEntry): void {
+  saveToCache(datafileToSave: OptimizelyDatafile): void {
     if (typeof window !== 'undefined') {
+      const cacheEntry: FetchUrlCacheEntry = {
+        datafile: datafileToSave,
+        metadata: {
+          timestampCached: Date.now(),
+        },
+      }
       // use setTimeout as to not block on a potentially expensive JSON.stringify
       setTimeout(() => {
-        window.localStorage.setItem(this.localStorageKey, JSON.stringify(toSave))
+        window.localStorage.setItem(this.localStorageKey, JSON.stringify(cacheEntry))
       }, 0)
     }
   }
 
   shouldUseCache(cacheResult: FetchUrlCacheEntry): boolean {
-    return true
+    return (Date.now() - cacheResult.metadata.timestampCached) <= FetchUrlDatafileLoader.MAX_CACHE_AGE_MS
   }
 
   async fetchDatafile(): Promise<OptimizelyDatafile> {

packages/js-web-sdk/packages/js-web-sdk/src/OptimizelySDKWrapper.ts

@@ -4,8 +4,7 @@ import { StaticUserIdLoader, UserId } from './UserIdLoaders'
 import { find } from './utils'
 import { ProvidedDatafileLoader, FetchUrlDatafileLoader } from './DatafileLoaders'
 import { ProvidedAttributesLoader } from './UserAttributesLoaders'
-import { ResourceManager } from './ResourceManager'
-import { ResourceLoader } from './ResourceStream'
+import { ResourceLoader, ResourceManager } from './ResourceManager'
 
 // export types
 export { OptimizelyDatafile }
@@ -126,22 +125,14 @@ export class OptimizelySDKWrapper implements IOptimizelySDKWrapper {
       userId: this.setupUserIdLoader(config),
     })
 
-    if (this.resourceManager.allResourcesReady()) {
+    if (this.resourceManager.allResourcesLoaded()) {
       this.onInitialized()
       this.initializingPromise = Promise.resolve()
     } else {
-      // TODO handle unsubscribe
-      this.initializingPromise = new Promise((resolve, reject) => {
-        this.resourceManager.stream.subscribe({
-          ready: () => {
-            this.onInitialized()
-            resolve()
-          },
-          error: ({ resourceKey, reason }) => {
-            reject(`"${resourceKey}" failed to load: ${reason}`)
-          },
+      this.initializingPromise = this.resourceManager.allResourcePromises()
+        .then(() => {
+          this.onInitialized();
         })
-      })
     }
   }
 
@@ -479,8 +470,6 @@ export class OptimizelySDKWrapper implements IOptimizelySDKWrapper {
     } else if (config.sdkKey) {
       datafileLoader = new FetchUrlDatafileLoader({
         sdkKey: config.sdkKey,
-        preferCached: true,
-        backgroundLoadIfCacheHit: true,
       })
     } else if (config.UNSTABLE_datafileLoader) {
       datafileLoader = config.UNSTABLE_datafileLoader
@@ -515,9 +504,9 @@ export class OptimizelySDKWrapper implements IOptimizelySDKWrapper {
   }
 
   private onInitialized() {
-    const datafile = this.resourceManager.datafile.getValue()
-    this.userId = this.resourceManager.userId.getValue() || null
-    this.attributes = this.resourceManager.attributes.getValue() || {}
+    const datafile = this.resourceManager.datafile.value
+    this.userId = this.resourceManager.userId.value || null
+    this.attributes = this.resourceManager.attributes.value || {}
     if (datafile) {
       this.datafile = datafile
     }

packages/js-web-sdk/packages/js-web-sdk/src/ResourceManager.ts

@@ -2,116 +2,78 @@ import { OptimizelyDatafile } from './OptimizelySDKWrapper'
 import { UserAttributes } from '@optimizely/optimizely-sdk'
 import { UserId } from './UserIdLoaders'
 
-import {
-  ResourceEmitter,
-  ResourceStream,
-  SingleResourceStream,
-  ResourceLoader,
-  CombinedResourceStream,
-} from './ResourceStream'
-
-interface Resource<K> {
-  getValue(): K | undefined
-  hasValue(): boolean
-  isReady(): boolean
-  hasFailed(): boolean
-  getMetadata(): ResourceEmitter.DataMessage.Metadata | undefined
-  getFailureReason(): string
+export interface ResourceLoader<K> {
+  load: () => K | Promise<K>
 }
 
-type ResourceLoaderState<K> = {
-  resource: K | undefined
-  metadata: ResourceEmitter.DataMessage.Metadata | undefined
-  isReady: boolean
-  failed: boolean
-  failureReason: string
-}
-
-class BaseResource<K> implements Resource<K> {
-  public stream: ResourceStream<K>
-  protected state: ResourceLoaderState<K>
+class Resource<K> {
+  private loader: ResourceLoader<K>
 
-  constructor(loader: ResourceLoader<K>) {
-    this.state = {
-      resource: undefined,
-      metadata: undefined,
-      isReady: false,
-      failed: false,
-      failureReason: '',
-    }
+  private _value?: K
 
-    this.stream = new SingleResourceStream(loader)
-    this.stream.subscribe({
-      data: msg => {
-        this.state.resource = msg.resource
-        this.state.metadata = msg.metadata
-      },
-      ready: () => {
-        this.state.isReady = true
-      },
-      error: msg => {
-        this.state.failed = true
-        this.state.failureReason = msg.reason
-      },
-    })
-  }
+  private _hasLoaded: boolean
 
-  getValue(): K | undefined {
-    return this.state.resource
-  }
+  public readonly promise: Promise<K>
 
-  hasValue(): boolean {
-    return this.getValue() !== void 0
+  public get value(): K | undefined {
+    return this._value
   }
 
-  isReady(): boolean {
-    return this.state.isReady
+  public get hasLoaded(): boolean {
+    return this._hasLoaded
   }
 
-  hasFailed(): boolean {
-    return this.state.failed
-    throw new Error('Method not implemented.')
+  constructor(loader: ResourceLoader<K>) {
+    this.loader = loader;
+    this._hasLoaded = false;
+    this.promise = this.load();
   }
 
-  getMetadata(): ResourceEmitter.DataMessage.Metadata | undefined {
-    return this.state.metadata
+  private updateStateFromLoadResult(value: K) {
+    this._value = value;
+    this._hasLoaded = true;
   }
 
-  getFailureReason(): string {
-    return this.state.failureReason
+  private load(): Promise<K> {
+    const maybeValue = this.loader.load()
+    // TODO: test does this work with polyfilled promise?
+    if (maybeValue instanceof Promise) {
+      return maybeValue.then(value => {
+        this.updateStateFromLoadResult(value);
+        return value
+      })
+    }
+    this.updateStateFromLoadResult(maybeValue);
+    return Promise.resolve(maybeValue);
   }
 }
 
-export class ResourceManager {
-  protected resourceKeys = ['datafile', 'attributes', 'userId']
+type OptimizelyResource = OptimizelyDatafile | UserAttributes | UserId
 
-  public datafile: BaseResource<OptimizelyDatafile>
-  public attributes: BaseResource<UserAttributes>
-  public userId: BaseResource<UserId>
+export class ResourceManager {
+  public datafile: Resource<OptimizelyDatafile>
+  public attributes: Resource<UserAttributes>
+  public userId: Resource<UserId>
 
-  public stream: ResourceStream<any>
+  private resources: Resource<OptimizelyResource>[]
 
   constructor(loaders: {
     datafile: ResourceLoader<OptimizelyDatafile>
     attributes: ResourceLoader<UserAttributes>
     userId: ResourceLoader<UserId>
   }) {
-    this.datafile = new BaseResource(loaders.datafile)
-    this.attributes = new BaseResource(loaders.attributes)
-    this.userId = new BaseResource(loaders.userId)
-
-    this.stream = new CombinedResourceStream([this.datafile.stream, this.attributes.stream, this.userId.stream])
-  }
+    this.datafile = new Resource(loaders.datafile)
+    this.attributes = new Resource(loaders.attributes)
+    this.userId = new Resource(loaders.userId)
 
-  allResourcesHaveValues(): boolean {
-    return this.everyResource(resource => resource.hasValue())
+    this.resources = [this.datafile, this.attributes, this.userId];
   }
 
-  allResourcesReady(): boolean {
-    return this.everyResource(resource => resource.isReady())
+  public allResourcesLoaded(): boolean {
+    return this.resources.every(resource => resource.hasLoaded);
   }
 
-  private everyResource(fn: (resource: Resource<any>) => boolean): boolean {
-    return this.resourceKeys.every(key => fn(this[key]))
+  public allResourcePromises(): Promise<OptimizelyResource[]> {
+    return Promise.all([...this.resources.map(resource => resource.promise)])
   }
 }