diff --git a/package-lock.json b/package-lock.json index b47a31b8..2e9a2e44 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3065,18 +3065,6 @@ "node": ">=0.9.9" } }, - "node_modules/cordova-lib/node_modules/cordova-common/node_modules/elementtree": { - "version": "0.1.7", - "dev": true, - "inBundle": true, - "license": "Apache-2.0", - "dependencies": { - "sax": "1.1.4" - }, - "engines": { - "node": ">= 0.4.0" - } - }, "node_modules/cordova-lib/node_modules/cordova-common/node_modules/q": { "version": "1.4.1", "dev": true, @@ -3087,12 +3075,6 @@ "teleport": ">=0.2.0" } }, - "node_modules/cordova-lib/node_modules/cordova-common/node_modules/sax": { - "version": "1.1.4", - "dev": true, - "inBundle": true, - "license": "ISC" - }, "node_modules/cordova-lib/node_modules/cordova-common/node_modules/semver": { "version": "5.1.0", "dev": true, @@ -3120,15 +3102,6 @@ "inBundle": true, "license": "MIT" }, - "node_modules/cordova-lib/node_modules/cordova-common/node_modules/unorm": { - "version": "1.6.0", - "dev": true, - "inBundle": true, - "license": "MIT or GPL-2.0", - "engines": { - "node": ">= 0.4.0" - } - }, "node_modules/cordova-lib/node_modules/cordova-registry-mapper": { "version": "1.1.15", "dev": true, @@ -3160,6 +3133,7 @@ "node_modules/cordova-lib/node_modules/elementtree": { "version": "0.1.6", "dev": true, + "inBundle": true, "dependencies": { "sax": "0.3.5" }, @@ -3431,6 +3405,7 @@ "node_modules/cordova-lib/node_modules/sax": { "version": "0.3.5", "dev": true, + "inBundle": true, "license": "MIT", "engines": { "node": "*" @@ -3473,6 +3448,7 @@ "node_modules/cordova-lib/node_modules/unorm": { "version": "1.3.3", "dev": true, + "inBundle": true, "engines": { "node": ">= 0.4.0" } @@ -18421,24 +18397,11 @@ "unorm": "^1.3.3" }, "dependencies": { - "elementtree": { - "version": "0.1.7", - "bundled": true, - "dev": true, - "requires": { - "sax": "1.1.4" - } - }, "q": { "version": "1.4.1", "bundled": true, "dev": true }, - "sax": { - "version": "1.1.4", - "bundled": true, - "dev": true - }, "semver": { "version": "5.1.0", "bundled": true, @@ -18453,11 +18416,6 @@ "version": "1.8.3", "bundled": true, "dev": true - }, - "unorm": { - "version": "1.6.0", - "bundled": true, - "dev": true } } }, @@ -18483,6 +18441,7 @@ }, "elementtree": { "version": "0.1.6", + "bundled": true, "dev": true, "requires": { "sax": "0.3.5" @@ -18687,6 +18646,7 @@ }, "sax": { "version": "0.3.5", + "bundled": true, "dev": true }, "shelljs": { @@ -18712,6 +18672,7 @@ }, "unorm": { "version": "1.3.3", + "bundled": true, "dev": true }, "util-deprecate": { diff --git a/src/scripts/extensions/authenticationHelper.ts b/src/scripts/extensions/authenticationHelper.ts index 1f739317..8e256811 100644 --- a/src/scripts/extensions/authenticationHelper.ts +++ b/src/scripts/extensions/authenticationHelper.ts @@ -39,59 +39,60 @@ export class AuthenticationHelper { return new Promise((resolve) => { let updateInterval = 0; - let storedUserInformation = this.clipperData.getValue(ClipperStorageKeys.userInformation); - if (storedUserInformation) { - let currentInfo: any; - try { - currentInfo = JSON.parse(storedUserInformation); - } catch (e) { - this.logger.logJsonParseUnexpected(storedUserInformation); - } + this.clipperData.getValue(ClipperStorageKeys.userInformation).then((storedUserInformation) => { + if (storedUserInformation) { + let currentInfo: any; + try { + currentInfo = JSON.parse(storedUserInformation); + } catch (e) { + this.logger.logJsonParseUnexpected(storedUserInformation); + } - if (currentInfo && currentInfo.data && ObjectUtils.isNumeric(currentInfo.data.accessTokenExpiration)) { - // Expiration is in seconds, not milliseconds. Give additional leniency to account for response time. - updateInterval = Math.max((currentInfo.data.accessTokenExpiration * 1000) - 180000, 0); + if (currentInfo && currentInfo.data && ObjectUtils.isNumeric(currentInfo.data.accessTokenExpiration)) { + // Expiration is in seconds, not milliseconds. Give additional leniency to account for response time. + updateInterval = Math.max((currentInfo.data.accessTokenExpiration * 1000) - 180000, 0); + } } - } - - let getUserInformationFunction = () => { - return this.retrieveUserInformation(clipperId, undefined); - }; - - let getInfoEvent: Log.Event.PromiseEvent = new Log.Event.PromiseEvent(Log.Event.Label.GetExistingUserInformation); - getInfoEvent.setCustomProperty(Log.PropertyName.Custom.UserInformationStored, !!storedUserInformation); - this.clipperData.getFreshValue(ClipperStorageKeys.userInformation, getUserInformationFunction, updateInterval).then(async (response: TimeStampedData) => { - let isValidUser = this.isValidUserInformation(response.data); - getInfoEvent.setCustomProperty(Log.PropertyName.Custom.FreshUserInfoAvailable, isValidUser); - let writeableCookies = this.isThirdPartyCookiesEnabled(response.data); - getInfoEvent.setCustomProperty(Log.PropertyName.Custom.WriteableCookies, writeableCookies); - - getInfoEvent.setCustomProperty(Log.PropertyName.Custom.UserUpdateReason, UpdateReason[updateReason]); - - if (isValidUser) { - const dataBoundaryHelper = new UserDataBoundaryHelper(); - let userDataBoundary: string = await dataBoundaryHelper.getUserDataBoundary(response.data); - // The default logging has been configured to EU Pipeline. Once we find the - // userdataboundary and if it is different from EUDB , reinit the logger with WW Pipeline - if (userDataBoundary === DataBoundary[DataBoundary.GLOBAL] || userDataBoundary === DataBoundary[DataBoundary.PUBLIC]) { - LogManager.reInitLoggerForDataBoundaryChange(userDataBoundary); + let getUserInformationFunction = () => { + return this.retrieveUserInformation(clipperId, undefined); + }; + + let getInfoEvent: Log.Event.PromiseEvent = new Log.Event.PromiseEvent(Log.Event.Label.GetExistingUserInformation); + getInfoEvent.setCustomProperty(Log.PropertyName.Custom.UserInformationStored, !!storedUserInformation); + this.clipperData.getFreshValue(ClipperStorageKeys.userInformation, getUserInformationFunction, updateInterval).then(async (response: TimeStampedData) => { + let isValidUser = this.isValidUserInformation(response.data); + getInfoEvent.setCustomProperty(Log.PropertyName.Custom.FreshUserInfoAvailable, isValidUser); + + let writeableCookies = this.isThirdPartyCookiesEnabled(response.data); + getInfoEvent.setCustomProperty(Log.PropertyName.Custom.WriteableCookies, writeableCookies); + + getInfoEvent.setCustomProperty(Log.PropertyName.Custom.UserUpdateReason, UpdateReason[updateReason]); + + if (isValidUser) { + const dataBoundaryHelper = new UserDataBoundaryHelper(); + let userDataBoundary: string = await dataBoundaryHelper.getUserDataBoundary(response.data); + // The default logging has been configured to EU Pipeline. Once we find the + // userdataboundary and if it is different from EUDB , reinit the logger with WW Pipeline + if (userDataBoundary === DataBoundary[DataBoundary.GLOBAL] || userDataBoundary === DataBoundary[DataBoundary.PUBLIC]) { + LogManager.reInitLoggerForDataBoundaryChange(userDataBoundary); + } + getInfoEvent.setCustomProperty(Log.PropertyName.Custom.DataBoundary, userDataBoundary); + response.data.dataBoundary = userDataBoundary; + this.user.set({ user: response.data, lastUpdated: response.lastUpdated, updateReason: updateReason, writeableCookies: writeableCookies }); + } else { + this.user.set({ updateReason: updateReason, writeableCookies: writeableCookies }); } - getInfoEvent.setCustomProperty(Log.PropertyName.Custom.DataBoundary, userDataBoundary); - response.data.dataBoundary = userDataBoundary; - this.user.set({ user: response.data, lastUpdated: response.lastUpdated, updateReason: updateReason, writeableCookies: writeableCookies }); - } else { - this.user.set({ updateReason: updateReason, writeableCookies: writeableCookies }); - } - }, (error: OneNoteApi.GenericError) => { - getInfoEvent.setStatus(Log.Status.Failed); - getInfoEvent.setFailureInfo(error); - this.user.set({ updateReason: updateReason }); - }).then(() => { - this.logger.logEvent(getInfoEvent); + }, (error: OneNoteApi.GenericError) => { + getInfoEvent.setStatus(Log.Status.Failed); + getInfoEvent.setFailureInfo(error); + this.user.set({ updateReason: updateReason }); + }).then(() => { + this.logger.logEvent(getInfoEvent); - resolve(this.user.get()); + resolve(this.user.get()); + }); }); }); } diff --git a/src/scripts/extensions/extensionBase.ts b/src/scripts/extensions/extensionBase.ts index c9bcd9b6..2cdb0739 100644 --- a/src/scripts/extensions/extensionBase.ts +++ b/src/scripts/extensions/extensionBase.ts @@ -56,30 +56,31 @@ export abstract class ExtensionBase { + if (!clipperId) { + // New install + clipperFirstRun = true; + clipperId = ExtensionBase.generateClipperId(); + this.clipperData.setValue(ClipperStorageKeys.clipperId, clipperId); + + // Ensure fresh installs don't trigger thats What's New experience + this.updateLastSeenVersionInStorageToCurrent(); + } - this.clientInfo = new SmartValue({ - clipperType: clipperType, - clipperVersion: ExtensionBase.getExtensionVersion(), - clipperId: clipperId - }); + this.clientInfo = new SmartValue({ + clipperType: clipperType, + clipperVersion: ExtensionBase.getExtensionVersion(), + clipperId: clipperId + }); - if (clipperFirstRun) { - this.onFirstRun(); - } + if (clipperFirstRun) { + this.onFirstRun(); + } - this.initializeUserFlighting(); + this.initializeUserFlighting(); - this.listenForOpportunityToShowPageNavTooltip(); + this.listenForOpportunityToShowPageNavTooltip(); + }); } protected abstract addPageNavListener(callback: (tab: TTab) => void); @@ -174,14 +175,14 @@ export abstract class ExtensionBase { + let lastSeenVersionStr = await this.clipperData.getValue(ClipperStorageKeys.lastSeenVersion); return lastSeenVersionStr ? new Version(lastSeenVersionStr) : undefined; } protected getNewUpdates(lastSeenVersion: Version, currentVersion: Version): Promise { - return new Promise((resolve, reject) => { - let localeOverride = this.clipperData.getValue(ClipperStorageKeys.displayLanguageOverride); + return new Promise(async(resolve, reject) => { + let localeOverride = await this.clipperData.getValue(ClipperStorageKeys.displayLanguageOverride); let localeToGet = localeOverride || navigator.language || (navigator).userLanguage; let changelogUrl = UrlUtils.addUrlQueryValue(Constants.Urls.changelogUrl, Constants.Urls.QueryParams.changelogLocale, localeToGet); HttpWithRetries.get(changelogUrl).then((response: Response) => { @@ -263,17 +264,23 @@ export abstract class ExtensionBase { if (wasInvoked) { this.tooltip.setTooltipInformation(ClipperStorageKeys.lastSeenTooltipTimeBase, tooltipType, Date.now().toString()); let numSeenStorageKey = ClipperStorageKeys.numTimesTooltipHasBeenSeenBase; - let numTimesSeen = this.tooltip.getTooltipInformation(numSeenStorageKey, tooltipType) + 1; - this.tooltip.setTooltipInformation(ClipperStorageKeys.numTimesTooltipHasBeenSeenBase, tooltipType, numTimesSeen.toString()); + this.tooltip.getTooltipInformation(numSeenStorageKey, tooltipType).then((value) => { + let numTimesSeen = value + 1; + this.tooltip.setTooltipInformation(ClipperStorageKeys.numTimesTooltipHasBeenSeenBase, tooltipType, numTimesSeen.toString()); + }); } tooltipImpressionEvent.setCustomProperty(Log.PropertyName.Custom.FeatureEnabled, wasInvoked); - worker.getLogger().logEvent(tooltipImpressionEvent); + Promise.all([lastSeenTooltipTimeBase, numTimesTooltipHasBeenSeenBase]).then((values) => { + tooltipImpressionEvent.setCustomProperty(Log.PropertyName.Custom.LastSeenTooltipTime, values[0]); + tooltipImpressionEvent.setCustomProperty(Log.PropertyName.Custom.NumTimesTooltipHasBeenSeen, values[1]); + worker.getLogger().logEvent(tooltipImpressionEvent); + }); }); } @@ -320,17 +327,6 @@ export abstract class ExtensionBase { - // Fallback behavior for if the last seen version in storage is somehow in a corrupted format - let lastSeenVersion: Version; - try { - lastSeenVersion = this.getLastSeenVersion(); - } catch (e) { - this.updateLastSeenVersionInStorageToCurrent(); - return; - } - - let extensionVersion = new Version(ExtensionBase.getExtensionVersion()); - if (this.clientInfo.get().clipperType !== ClientType.FirefoxExtension) { let tooltips = [TooltipType.Pdf, TooltipType.Product, TooltipType.Recipe]; // Returns the Type of tooltip to show IF the delay is over and the tab has the correct content type @@ -346,10 +342,20 @@ export abstract class ExtensionBase { + // We don't show updates more recent than the local version for now, as it is easy + // for a changelog to be released before a version is actually out + if (this.shouldShowWhatsNewTooltip(tab, lastSeenVersion, extensionVersion)) { + this.showWhatsNewTooltip(tab, lastSeenVersion, extensionVersion); + return; + } + }); + } catch (e) { + this.updateLastSeenVersionInStorageToCurrent(); return; } }); diff --git a/src/scripts/extensions/extensionWorkerBase.ts b/src/scripts/extensions/extensionWorkerBase.ts index 9a895534..54d08bbc 100644 --- a/src/scripts/extensions/extensionWorkerBase.ts +++ b/src/scripts/extensions/extensionWorkerBase.ts @@ -273,40 +273,42 @@ export abstract class ExtensionWorkerBase { protected getLocalizedStrings(locale: string, callback?: Function) { this.logger.setContextProperty(Log.Context.Custom.BrowserLanguage, locale); - let storedLocale = this.clipperData.getValue(ClipperStorageKeys.locale); - let localeInStorageIsDifferent = !storedLocale || storedLocale !== locale; - - let getLocaleEvent = new Log.Event.BaseEvent(Log.Event.Label.GetLocale); - getLocaleEvent.setCustomProperty(Log.PropertyName.Custom.StoredLocaleDifferentThanRequested, localeInStorageIsDifferent); - this.logger.logEvent(getLocaleEvent); - - let fetchStringDataFunction = () => { return LocalizationHelper.makeLocStringsFetchRequest(locale); }; - let updateInterval = localeInStorageIsDifferent ? 0 : ClipperCachedHttp.getDefaultExpiry(); - - let getLocalizedStringsEvent = new Log.Event.PromiseEvent(Log.Event.Label.GetLocalizedStrings); - getLocalizedStringsEvent.setCustomProperty(Log.PropertyName.Custom.ForceRetrieveFreshLocStrings, localeInStorageIsDifferent); - this.clipperData.getFreshValue(ClipperStorageKeys.locStrings, fetchStringDataFunction, updateInterval).then((response) => { - this.clipperData.setValue(ClipperStorageKeys.locale, locale); - if (callback) { - callback(response ? response.data : undefined); - } - }, (error: OneNoteApi.GenericError) => { - getLocalizedStringsEvent.setStatus(Log.Status.Failed); - getLocalizedStringsEvent.setFailureInfo(error); - // Still proceed, as we have backup strings on the client - if (callback) { - callback(undefined); - } - }).then(() => { - this.logger.logEvent(getLocalizedStringsEvent); + this.clipperData.getValue(ClipperStorageKeys.locale).then((storedLocale) => { + let localeInStorageIsDifferent = !storedLocale || storedLocale !== locale; + + let getLocaleEvent = new Log.Event.BaseEvent(Log.Event.Label.GetLocale); + getLocaleEvent.setCustomProperty(Log.PropertyName.Custom.StoredLocaleDifferentThanRequested, localeInStorageIsDifferent); + this.logger.logEvent(getLocaleEvent); + + let fetchStringDataFunction = () => { return LocalizationHelper.makeLocStringsFetchRequest(locale); }; + let updateInterval = localeInStorageIsDifferent ? 0 : ClipperCachedHttp.getDefaultExpiry(); + + let getLocalizedStringsEvent = new Log.Event.PromiseEvent(Log.Event.Label.GetLocalizedStrings); + getLocalizedStringsEvent.setCustomProperty(Log.PropertyName.Custom.ForceRetrieveFreshLocStrings, localeInStorageIsDifferent); + this.clipperData.getFreshValue(ClipperStorageKeys.locStrings, fetchStringDataFunction, updateInterval).then((response) => { + this.clipperData.setValue(ClipperStorageKeys.locale, locale); + if (callback) { + callback(response ? response.data : undefined); + } + }, (error: OneNoteApi.GenericError) => { + getLocalizedStringsEvent.setStatus(Log.Status.Failed); + getLocalizedStringsEvent.setFailureInfo(error); + // Still proceed, as we have backup strings on the client + if (callback) { + callback(undefined); + } + }).then(() => { + this.logger.logEvent(getLocalizedStringsEvent); + }); }); } protected getLocalizedStringsForBrowser(callback: Function) { - let localeOverride = this.clipperData.getValue(ClipperStorageKeys.displayLanguageOverride); - // navigator.userLanguage is only available in IE, and Typescript will not recognize this property - let locale = localeOverride || navigator.language || (navigator).userLanguage; - this.getLocalizedStrings(locale, callback); + this.clipperData.getValue(ClipperStorageKeys.displayLanguageOverride).then((localeOverride) => { + // navigator.userLanguage is only available in IE, and Typescript will not recognize this property + let locale = localeOverride || navigator.language || (navigator).userLanguage; + this.getLocalizedStrings(locale, callback); + }); } protected getUserSessionIdQueryParamValue(): string { @@ -472,10 +474,10 @@ export abstract class ExtensionWorkerBase { }); this.uiCommunicator.registerFunction(Constants.FunctionKeys.getMultipleStorageValues, (keys: string[]) => { - return new Promise<{ [key: string]: string }>((resolve) => { + return new Promise<{ [key: string]: string }>(async(resolve) => { let values: { [key: string]: string } = {}; for (let key of keys) { - values[key] = this.clipperData.getValue(key); + values[key] = await this.clipperData.getValue(key); } resolve(values); }); diff --git a/src/scripts/extensions/tooltipHelper.ts b/src/scripts/extensions/tooltipHelper.ts index eb4f21b7..d0d58c94 100644 --- a/src/scripts/extensions/tooltipHelper.ts +++ b/src/scripts/extensions/tooltipHelper.ts @@ -15,13 +15,13 @@ export class TooltipHelper { this.validTypes = [TooltipType.Pdf, TooltipType.Product, TooltipType.Recipe, TooltipType.Video]; } - public getTooltipInformation(storageKeyBase: string, tooltipType: TooltipType): number { + public async getTooltipInformation(storageKeyBase: string, tooltipType: TooltipType): Promise { if (ObjectUtils.isNullOrUndefined(storageKeyBase) || ObjectUtils.isNullOrUndefined(tooltipType)) { throw new Error("Invalid argument passed to getTooltipInformation"); } let storageKey = TooltipHelper.getStorageKeyForTooltip(storageKeyBase, tooltipType); - let tooltipInfoAsString = this.storage.getValue(storageKey); + let tooltipInfoAsString = await this.storage.getValue(storageKey); let info = parseInt(tooltipInfoAsString, 10 /* radix */); return !isNaN(info) ? info : 0; } @@ -35,26 +35,25 @@ export class TooltipHelper { this.storage.setValue(storageKey, value); } - public tooltipDelayIsOver(tooltipType: TooltipType, curTime: number): boolean { + public async tooltipDelayIsOver(tooltipType: TooltipType, curTime: number): Promise { if (ObjectUtils.isNullOrUndefined(tooltipType) || ObjectUtils.isNullOrUndefined(curTime)) { throw new Error("Invalid argument passed to tooltipDelayIsOver"); } // If the user has clipped this content type - let lastClipTime = this.getTooltipInformation(ClipperStorageKeys.lastClippedTooltipTimeBase, tooltipType); + let lastClipTime = await this.getTooltipInformation(ClipperStorageKeys.lastClippedTooltipTimeBase, tooltipType); if (lastClipTime !== 0) { return false; } // If the user has seen enough of our tooltips :P - let numTimesTooltipHasBeenSeen = this.getTooltipInformation(ClipperStorageKeys.numTimesTooltipHasBeenSeenBase, tooltipType); + let numTimesTooltipHasBeenSeen = await this.getTooltipInformation(ClipperStorageKeys.numTimesTooltipHasBeenSeenBase, tooltipType); if (numTimesTooltipHasBeenSeen >= Constants.Settings.maximumNumberOfTimesToShowTooltips) { return false; } // If not enough time has passed since the user saw this specific tooltip, return false - let lastSeenTooltipTime = this.getTooltipInformation(ClipperStorageKeys.lastSeenTooltipTimeBase, tooltipType); - if (this.tooltipHasBeenSeenInLastTimePeriod(tooltipType, curTime, Constants.Settings.timeBetweenSameTooltip)) { + if (await this.tooltipHasBeenSeenInLastTimePeriod(tooltipType, curTime, Constants.Settings.timeBetweenSameTooltip)) { return false; } @@ -62,7 +61,7 @@ export class TooltipHelper { let indexOfThisTooltip = this.validTypes.indexOf(tooltipType); let validTypesWithCurrentTypeRemoved = this.validTypes.slice(); validTypesWithCurrentTypeRemoved.splice(indexOfThisTooltip, 1); - if (this.hasAnyTooltipBeenSeenInLastTimePeriod(curTime, validTypesWithCurrentTypeRemoved, Constants.Settings.timeBetweenDifferentTooltips)) { + if (await this.hasAnyTooltipBeenSeenInLastTimePeriod(curTime, validTypesWithCurrentTypeRemoved, Constants.Settings.timeBetweenDifferentTooltips)) { return false; } return true; @@ -79,8 +78,8 @@ export class TooltipHelper { /** * Returns true if the lastSeenTooltipTime of @tooltipType is within @timePeriod of @curTime */ - public tooltipHasBeenSeenInLastTimePeriod(tooltipType: TooltipType, curTime: number, timePeriod: number): boolean { - let lastSeenTooltipTime = this.getTooltipInformation(ClipperStorageKeys.lastSeenTooltipTimeBase, tooltipType); + public async tooltipHasBeenSeenInLastTimePeriod(tooltipType: TooltipType, curTime: number, timePeriod: number): Promise { + let lastSeenTooltipTime = await this.getTooltipInformation(ClipperStorageKeys.lastSeenTooltipTimeBase, tooltipType); if (lastSeenTooltipTime === 0) { return false; } @@ -91,10 +90,13 @@ export class TooltipHelper { /** * Returns true if any of the @tooltipTypesToCheck have been seen in the last @timePeriod, given the current @time */ - public hasAnyTooltipBeenSeenInLastTimePeriod(curTime: number, typesToCheck: TooltipType[], timePeriod: number): boolean { - return typesToCheck.some((tooltipType) => { - let tooltipWasSeen = this.tooltipHasBeenSeenInLastTimePeriod(tooltipType, curTime, timePeriod); - return tooltipWasSeen; - }); + public async hasAnyTooltipBeenSeenInLastTimePeriod(curTime: number, typesToCheck: TooltipType[], timePeriod: number): Promise { + for (const tooltipType of typesToCheck) { + const seen = await this.tooltipHasBeenSeenInLastTimePeriod(tooltipType, curTime, timePeriod); + if (seen) { + return true; + } + } + return false; } } diff --git a/src/scripts/http/cachedHttp.ts b/src/scripts/http/cachedHttp.ts index 43bbe9b3..de8cc912 100644 --- a/src/scripts/http/cachedHttp.ts +++ b/src/scripts/http/cachedHttp.ts @@ -25,7 +25,7 @@ export class CachedHttp { * Given a key, checks the cache for a fresh copy of that value. If the cached value does * not exist, or is not fresh, fetches a fresh copy of the data from the specified endpoint. */ - public getFreshValue(key: string, getRemoteValue: GetResponseAsync, updateInterval: number): Promise { + public async getFreshValue(key: string, getRemoteValue: GetResponseAsync, updateInterval: number): Promise { if (!key) { throw new Error("key must be a non-empty string, but was: " + key); } else if (!getRemoteValue) { @@ -34,7 +34,7 @@ export class CachedHttp { updateInterval = 0; } - let value = this.cache.getValue(key); + let value = await this.cache.getValue(key); let keyIsPresent = !!value; if (keyIsPresent) { let valueAsJson: TimeStampedData; diff --git a/src/scripts/storage/clipperData.ts b/src/scripts/storage/clipperData.ts index 56036c34..cbdf9aef 100644 --- a/src/scripts/storage/clipperData.ts +++ b/src/scripts/storage/clipperData.ts @@ -33,18 +33,20 @@ export class ClipperData implements Storage { return this.cachedHttp.getFreshValue(key, getRemoteValue, updateInterval); } - public getValue(key: string): string { + public async getValue(key: string): Promise { return this.storage.getValue(key); } - public getValues(keys: string[]): {} { + public async getValues(keys: string[]): Promise<{}> { return this.storage.getValues(keys); } public setValue(key: string, value: string): void { - if (this.storageGateStrategy.shouldSet(key, value)) { - this.storage.setValue(key, value); - } + this.storageGateStrategy.shouldSet(key, value).then((result) => { + if (result) { + this.storage.setValue(key, value); + } + }); } public removeKey(key: string): boolean { diff --git a/src/scripts/storage/clipperStorageGateStrategy.ts b/src/scripts/storage/clipperStorageGateStrategy.ts index 86e0c004..daf9aaab 100644 --- a/src/scripts/storage/clipperStorageGateStrategy.ts +++ b/src/scripts/storage/clipperStorageGateStrategy.ts @@ -15,10 +15,10 @@ export class ClipperStorageGateStrategy implements StorageGateStrategy { this.storage = storage; } - public shouldSet(key: string, value: string): boolean { + public async shouldSet(key: string, value: string): Promise { // An undefined value is the same thing as removing a key, so we allow it regardless if (value && this.keysThatRequireUserInfo.indexOf(key) > -1) { - let userInfo = this.storage.getValue(ClipperStorageKeys.userInformation); + let userInfo = await this.storage.getValue(ClipperStorageKeys.userInformation); return !!userInfo; } return true; diff --git a/src/scripts/storage/localStorage.ts b/src/scripts/storage/localStorage.ts index 970954dd..437be076 100644 --- a/src/scripts/storage/localStorage.ts +++ b/src/scripts/storage/localStorage.ts @@ -1,7 +1,7 @@ import {Storage} from "./storage"; export class LocalStorage implements Storage { - public getValue(key: string): string { + public async getValue(key: string): Promise { let result: string; if (window.localStorage) { result = window.localStorage.getItem(key); @@ -13,7 +13,7 @@ export class LocalStorage implements Storage { return result; } - public getValues(keys: string[]): {} { + public async getValues(keys: string[]): Promise<{}> { let values = {}; if (window.localStorage && keys) { for (let i = 0; i < keys.length; i++) { diff --git a/src/scripts/storage/storage.ts b/src/scripts/storage/storage.ts index 159dfa3b..955030f9 100644 --- a/src/scripts/storage/storage.ts +++ b/src/scripts/storage/storage.ts @@ -1,6 +1,6 @@ export interface Storage { - getValue(key: string): string; - getValues(keys: string[]): {}; + getValue(key: string): Promise; + getValues(keys: string[]): Promise<{}>; setValue(key: string, value: string): void; removeKey(key: string): boolean; } diff --git a/src/scripts/storage/storageGateStrategy.ts b/src/scripts/storage/storageGateStrategy.ts index 8973053c..0f160bbd 100644 --- a/src/scripts/storage/storageGateStrategy.ts +++ b/src/scripts/storage/storageGateStrategy.ts @@ -1,3 +1,3 @@ export interface StorageGateStrategy { - shouldSet(key: string, value: string): boolean; + shouldSet(key: string, value: string): Promise; } diff --git a/src/tests/http/cachedHttp_tests.ts b/src/tests/http/cachedHttp_tests.ts index f25b2ae7..73f7c4a2 100644 --- a/src/tests/http/cachedHttp_tests.ts +++ b/src/tests/http/cachedHttp_tests.ts @@ -82,9 +82,11 @@ export class CachedHttpTests extends TestModule { strictEqual(timeStampedData.data, expected, "The storage item should be returned"); strictEqual(timeStampedData.lastUpdated, timeOfStorage, "The returned item should have its lastUpdated value be preserved"); - let newStoredValue = JSON.parse(this.mockStorage.getValue(key)) as TimeStampedData; - strictEqual(newStoredValue.data, expected, "The storage item should be preserved"); - strictEqual(newStoredValue.lastUpdated, timeOfStorage, "The storage item's lastUpdated should be preserved"); + this.mockStorage.getValue(key).then((value) => { + let newStoredValue = JSON.parse(value) as TimeStampedData; + strictEqual(newStoredValue.data, expected, "The storage item should be preserved"); + strictEqual(newStoredValue.lastUpdated, timeOfStorage, "The storage item's lastUpdated should be preserved"); + }); }, (error) => { ok(false, "reject should not be called"); }).then(() => { @@ -117,9 +119,11 @@ export class CachedHttpTests extends TestModule { strictEqual(timeStampedData.data, expected, "The remote item should be returned"); ok(timeStampedData.lastUpdated > timeOfStorage, "The returned item's lastUpdated should be greater than that of the stale value's one"); - let newStoredValue = JSON.parse(this.mockStorage.getValue(key)) as TimeStampedData; - strictEqual(newStoredValue.data, expected, "The storage item should be updated to the remote value"); - ok(newStoredValue.lastUpdated > timeOfStorage, "The storage item's lastUpdated should be updated to be greater than the old value"); + this.mockStorage.getValue(key).then((value) => { + let newStoredValue = JSON.parse(value) as TimeStampedData; + strictEqual(newStoredValue.data, expected, "The storage item should be updated to the remote value"); + ok(newStoredValue.lastUpdated > timeOfStorage, "The storage item's lastUpdated should be updated to be greater than the old value"); + }); }, (error) => { ok(false, "reject should not be called"); }).then(() => { @@ -147,9 +151,11 @@ export class CachedHttpTests extends TestModule { ok(timeStampedData.lastUpdated > 0); strictEqual(timeStampedData.data, JSON.parse(response), "The parsed response text should be returned as part of the time stamped data"); - let timeStampedValueInStorage: TimeStampedData = JSON.parse(this.mockStorage.getValue(key)); - deepEqual(timeStampedValueInStorage, timeStampedData, + this.mockStorage.getValue(key).then((value) => { + let timeStampedValueInStorage: TimeStampedData = JSON.parse(value); + deepEqual(timeStampedValueInStorage, timeStampedData, "The latest time stamped data should be cached in the storage object"); + }); }, (error) => { ok(false, "reject should not be called"); }).then(() => { diff --git a/src/tests/storage/clipperData_tests.ts b/src/tests/storage/clipperData_tests.ts index f8852cb2..1f09c563 100644 --- a/src/tests/storage/clipperData_tests.ts +++ b/src/tests/storage/clipperData_tests.ts @@ -60,9 +60,11 @@ export class ClipperDataTests extends TestModule { let clipperData = new ClipperData(this.mockStorage); clipperData.getFreshValue(key, getRemoteValue, 0).then((timeStampedData) => { - let actualStored: TimeStampedData = JSON.parse(this.mockStorage.getValue(key)); - deepEqual(actualStored.data, {}, - "Notebooks should be cached if userInformation exists in storage"); + this.mockStorage.getValue(key).then((value) => { + let actualStored: TimeStampedData = JSON.parse(value); + deepEqual(actualStored.data, {}, + "Notebooks should be cached if userInformation exists in storage"); + }); }, (error) => { ok(false, "reject should not be called"); }).then(() => { diff --git a/src/tests/storage/mockStorage.ts b/src/tests/storage/mockStorage.ts index 249a6750..966b4d2f 100644 --- a/src/tests/storage/mockStorage.ts +++ b/src/tests/storage/mockStorage.ts @@ -20,11 +20,11 @@ export class MockStorage implements Storage { this.setValue(MockStorage.existingKey, JSON.stringify(existingValue)); } - public getValue(key: string): string { + public async getValue(key: string): Promise { return this.storedValues[key]; } - public getValues(keys: string[]): {} { + public async getValues(keys: string[]): Promise<{}> { let values = {}; for (let i = 0; i < keys.length; i++) { values[keys[i]] = this.storedValues[keys[i]];