From 000f08d26ea43bca1d89945fb6c5422385e2c092 Mon Sep 17 00:00:00 2001 From: Katie Rischpater <98350084+the-bay-kay@users.noreply.github.com> Date: Mon, 6 Nov 2023 14:21:33 -0800 Subject: [PATCH] Refactored combinedPromises - combinedPromises now uses `Promise.allSettled()` - rewrote tests to reflect change --- www/__tests__/unifiedDataLoader.test.ts | 73 +++++++--------- www/js/diary/services.js | 109 ++++++++++++------------ www/js/services/unifiedDataLoader.ts | 109 ++++++------------------ 3 files changed, 114 insertions(+), 177 deletions(-) diff --git a/www/__tests__/unifiedDataLoader.test.ts b/www/__tests__/unifiedDataLoader.test.ts index 285971f0a..c31172d0c 100644 --- a/www/__tests__/unifiedDataLoader.test.ts +++ b/www/__tests__/unifiedDataLoader.test.ts @@ -1,5 +1,5 @@ import { mockLogger } from '../__mocks__/globalMocks'; -import { combineWithDedup, combinedPromises } from '../js/services/unifiedDataLoader'; +import { removeDup, combinedPromises } from '../js/services/unifiedDataLoader'; import { ServerData } from '../js/types/serverData'; mockLogger(); @@ -9,7 +9,7 @@ const testOne: ServerData = { metadata: { key: '', platform: '', - write_ts: 1, // the only value checked by combineWithDedup + write_ts: 1, // the only value checked by removeDup time_zone: '', write_fmt_time: '', write_local_dt: null, @@ -23,25 +23,21 @@ testThree.metadata.write_ts = 3; const testFour = JSON.parse(JSON.stringify(testOne)); testFour.metadata.write_ts = 4; -describe('combineWithDedup can', () => { - it('work with empty arrays', () => { - expect(combineWithDedup([], [])).toEqual([]); - expect(combineWithDedup([], [testOne])).toEqual([testOne]); - expect(combineWithDedup([testOne, testTwo], [])).toEqual([testOne, testTwo]); +describe('removeDup can', () => { + it('work with an empty array', () => { + expect(removeDup([])).toEqual([]); }); - it('work with arrays of len 1', () => { - expect(combineWithDedup([testOne], [testOne])).toEqual([testOne]); - expect(combineWithDedup([testOne], [testTwo])).toEqual([testOne, testTwo]); + + it('work with an array of len 1', () => { + expect(removeDup([testOne])).toEqual([testOne]); }); - it('work with arrays of len > 1', () => { - expect(combineWithDedup([testOne], [testOne, testTwo])).toEqual([testOne, testTwo]); - expect(combineWithDedup([testOne], [testTwo, testTwo])).toEqual([testOne, testTwo]); - expect(combineWithDedup([testOne, testTwo], [testTwo, testTwo])).toEqual([testOne, testTwo]); - expect(combineWithDedup([testOne, testTwo, testThree], [testOne, testTwo])).toEqual([ - testOne, - testTwo, - testThree, - ]); + + it('work with an array of len >=1', () => { + expect(removeDup([testOne, testTwo])).toEqual([testOne, testTwo]); + expect(removeDup([testOne, testOne])).toEqual([testOne]); + expect(removeDup([testOne, testTwo, testThree])).toEqual([testOne, testTwo, testThree]); + expect(removeDup([testOne, testOne, testThree])).toEqual([testOne, testThree]); + expect(removeDup([testOne, testOne, testOne])).toEqual([testOne]); }); }); @@ -55,38 +51,35 @@ const badPromiseGenerator = (input: string) => { it('throws an error on an empty input', async () => { expect(() => { - combinedPromises([], combineWithDedup); + combinedPromises([], removeDup); }).toThrow(); }); it('catches when all promises fails', async () => { - expect(combinedPromises([badPromiseGenerator('')], combineWithDedup)).rejects.toEqual(['']); + expect(combinedPromises([badPromiseGenerator('')], removeDup)).rejects.toEqual(['']); expect( - combinedPromises( - [badPromiseGenerator('bad'), badPromiseGenerator('promise')], - combineWithDedup, - ), + combinedPromises([badPromiseGenerator('bad'), badPromiseGenerator('promise')], removeDup), ).rejects.toEqual(['bad', 'promise']); expect( combinedPromises( [badPromiseGenerator('very'), badPromiseGenerator('bad'), badPromiseGenerator('promise')], - combineWithDedup, + removeDup, ), ).rejects.toEqual(['very', 'bad', 'promise']); expect( - combinedPromises([badPromiseGenerator('bad'), promiseGenerator([testOne])], combineWithDedup), + combinedPromises([badPromiseGenerator('bad'), promiseGenerator([testOne])], removeDup), ).resolves.toEqual([testOne]); expect( - combinedPromises([promiseGenerator([testOne]), badPromiseGenerator('bad')], combineWithDedup), + combinedPromises([promiseGenerator([testOne]), badPromiseGenerator('bad')], removeDup), ).resolves.toEqual([testOne]); }); it('work with arrays of len 1', async () => { const promiseArrayOne = [promiseGenerator([testOne])]; const promiseArrayTwo = [promiseGenerator([testOne, testTwo])]; - const testResultOne = await combinedPromises(promiseArrayOne, combineWithDedup); - const testResultTwo = await combinedPromises(promiseArrayTwo, combineWithDedup); + const testResultOne = await combinedPromises(promiseArrayOne, removeDup); + const testResultTwo = await combinedPromises(promiseArrayTwo, removeDup); expect(testResultOne).toEqual([testOne]); expect(testResultTwo).toEqual([testOne, testTwo]); @@ -105,11 +98,11 @@ it('works with arrays of len 2', async () => { promiseGenerator([testTwo, testThree]), ]; - const testResultOne = await combinedPromises(promiseArrayOne, combineWithDedup); - const testResultTwo = await combinedPromises(promiseArrayTwo, combineWithDedup); - const testResultThree = await combinedPromises(promiseArrayThree, combineWithDedup); - const testResultFour = await combinedPromises(promiseArrayFour, combineWithDedup); - const testResultFive = await combinedPromises(promiseArrayFive, combineWithDedup); + const testResultOne = await combinedPromises(promiseArrayOne, removeDup); + const testResultTwo = await combinedPromises(promiseArrayTwo, removeDup); + const testResultThree = await combinedPromises(promiseArrayThree, removeDup); + const testResultFour = await combinedPromises(promiseArrayFour, removeDup); + const testResultFive = await combinedPromises(promiseArrayFive, removeDup); expect(testResultOne).toEqual([testOne, testTwo]); expect(testResultTwo).toEqual([testOne, testTwo, testThree]); @@ -140,10 +133,10 @@ it('works with arrays of len >= 2', async () => { promiseGenerator([testFour]), ]; - const testResultOne = await combinedPromises(promiseArrayOne, combineWithDedup); - const testResultTwo = await combinedPromises(promiseArrayTwo, combineWithDedup); - const testResultThree = await combinedPromises(promiseArrayThree, combineWithDedup); - const testResultFour = await combinedPromises(promiseArrayFour, combineWithDedup); + const testResultOne = await combinedPromises(promiseArrayOne, removeDup); + const testResultTwo = await combinedPromises(promiseArrayTwo, removeDup); + const testResultThree = await combinedPromises(promiseArrayThree, removeDup); + const testResultFour = await combinedPromises(promiseArrayFour, removeDup); expect(testResultOne).toEqual([testOne, testTwo, testThree]); expect(testResultTwo).toEqual([testOne, testTwo]); @@ -154,4 +147,4 @@ it('works with arrays of len >= 2', async () => { /* TO-DO: Once getRawEnteries can be tested via end-to-end testing, we will be able to test getUnifiedDataForInterval as well. -*/ +*/ \ No newline at end of file diff --git a/www/js/diary/services.js b/www/js/diary/services.js index d9ec7fbe3..7d746fc94 100644 --- a/www/js/diary/services.js +++ b/www/js/diary/services.js @@ -16,7 +16,6 @@ angular $ionicPlatform, $window, $rootScope, - UnifiedDataLoader, Logger, $injector, ) { @@ -268,64 +267,65 @@ angular ' -> ' + moment.unix(tripEndTransition.data.ts).toString(), ); - return UnifiedDataLoader.getUnifiedSensorDataForInterval( - 'background/filtered_location', - tq, - ).then(function (locationList) { - if (locationList.length == 0) { - return undefined; - } - var sortedLocationList = locationList.sort(tsEntrySort); - var retainInRange = function (loc) { - return ( - tripStartTransition.data.ts <= loc.data.ts && loc.data.ts <= tripEndTransition.data.ts - ); - }; + const getMethod = window['cordova'].plugins.BEMUserCache.getSensorDataForInterval; + return getUnifiedDataForInterval('background/filtered_location', tq, getMethod).then( + function (locationList) { + if (locationList.length == 0) { + return undefined; + } + var sortedLocationList = locationList.sort(tsEntrySort); + var retainInRange = function (loc) { + return ( + tripStartTransition.data.ts <= loc.data.ts && + loc.data.ts <= tripEndTransition.data.ts + ); + }; - var filteredLocationList = sortedLocationList.filter(retainInRange); + var filteredLocationList = sortedLocationList.filter(retainInRange); - // Fix for https://github.com/e-mission/e-mission-docs/issues/417 - if (filteredLocationList.length == 0) { - return undefined; - } + // Fix for https://github.com/e-mission/e-mission-docs/issues/417 + if (filteredLocationList.length == 0) { + return undefined; + } - var tripStartPoint = filteredLocationList[0]; - var tripEndPoint = filteredLocationList[filteredLocationList.length - 1]; - Logger.log( - 'tripStartPoint = ' + - JSON.stringify(tripStartPoint) + - 'tripEndPoint = ' + - JSON.stringify(tripEndPoint), - ); - // if we get a list but our start and end are undefined - // let's print out the complete original list to get a clue - // this should help with debugging - // https://github.com/e-mission/e-mission-docs/issues/417 - // if it ever occurs again - if (angular.isUndefined(tripStartPoint) || angular.isUndefined(tripEndPoint)) { - Logger.log('BUG 417 check: locationList = ' + JSON.stringify(locationList)); + var tripStartPoint = filteredLocationList[0]; + var tripEndPoint = filteredLocationList[filteredLocationList.length - 1]; Logger.log( - 'transitions: start = ' + - JSON.stringify(tripStartTransition.data) + - ' end = ' + - JSON.stringify(tripEndTransition.data.ts), + 'tripStartPoint = ' + + JSON.stringify(tripStartPoint) + + 'tripEndPoint = ' + + JSON.stringify(tripEndPoint), ); - } + // if we get a list but our start and end are undefined + // let's print out the complete original list to get a clue + // this should help with debugging + // https://github.com/e-mission/e-mission-docs/issues/417 + // if it ever occurs again + if (angular.isUndefined(tripStartPoint) || angular.isUndefined(tripEndPoint)) { + Logger.log('BUG 417 check: locationList = ' + JSON.stringify(locationList)); + Logger.log( + 'transitions: start = ' + + JSON.stringify(tripStartTransition.data) + + ' end = ' + + JSON.stringify(tripEndTransition.data.ts), + ); + } - const tripProps = points2TripProps(filteredLocationList); - - return { - ...tripProps, - start_loc: { - type: 'Point', - coordinates: [tripStartPoint.data.longitude, tripStartPoint.data.latitude], - }, - end_loc: { - type: 'Point', - coordinates: [tripEndPoint.data.longitude, tripEndPoint.data.latitude], - }, - }; - }); + const tripProps = points2TripProps(filteredLocationList); + + return { + ...tripProps, + start_loc: { + type: 'Point', + coordinates: [tripStartPoint.data.longitude, tripStartPoint.data.latitude], + }, + end_loc: { + type: 'Point', + coordinates: [tripEndPoint.data.longitude, tripEndPoint.data.latitude], + }, + }; + }, + ); }; var linkTrips = function (trip1, trip2) { @@ -354,7 +354,8 @@ angular ' -> ' + moment.unix(tq.endTs).toString(), ); - return UnifiedDataLoader.getUnifiedMessagesForInterval('statemachine/transition', tq).then( + const getMethod = window['cordova'].plugins.BEMUserCache.getMessagesForInterval; + return getUnifiedDataForInterval('statemachine/transition', tq, getMethod).then( function (transitionList) { if (transitionList.length == 0) { Logger.log('No unprocessed trips. yay!'); diff --git a/www/js/services/unifiedDataLoader.ts b/www/js/services/unifiedDataLoader.ts index d4211b4ce..8a91d3587 100644 --- a/www/js/services/unifiedDataLoader.ts +++ b/www/js/services/unifiedDataLoader.ts @@ -1,16 +1,13 @@ -import { logDebug } from '../plugin/logger'; import { getRawEntries } from './commHelper'; import { ServerResponse, ServerData, TimeQuery } from '../types/serverData'; /** - * combineWithDedup is a helper function for combinedPromises - * @param list1 values evaluated from a BEMUserCache promise - * @param list2 same as list1 - * @returns a dedup array generated from the input lists + * removeDup is a helper function for combinedPromises + * @param list An array of values from a BEMUserCache promise + * @returns an array with duplicate values removed */ -export const combineWithDedup = function (list1: Array>, list2: Array) { - const combinedList = list1.concat(list2); - return combinedList.filter(function (value, i, array) { +export const removeDup = function (list: Array>) { + return list.filter(function (value, i, array) { const firstIndexOfValue = array.findIndex(function (element) { return element.metadata.write_ts == value.metadata.write_ts; }); @@ -18,86 +15,32 @@ export const combineWithDedup = function (list1: Array>, list2: }); }; -/** - * combinedPromises is a recursive function that joins multiple promises - * @param promiseList 1 or more promises - * @param combiner a function that takes two arrays and joins them - * @returns A promise which evaluates to a combined list of values or errors - */ export const combinedPromises = function ( promiseList: Array>, - combiner: (list1: Array, list2: Array) => Array, + filter: (list: Array) => Array, ) { if (promiseList.length === 0) { throw new RangeError('combinedPromises needs input array.length >= 1'); } return new Promise(function (resolve, reject) { - var firstResult = []; - var firstError = null; - - var nextResult = []; - var nextError = null; - - var firstPromiseDone = false; - var nextPromiseDone = false; - - const checkAndResolve = function () { - if (firstPromiseDone && nextPromiseDone) { - if (firstError && nextError) { - reject([firstError].concat(nextError)); - } else { - logDebug( - `About to dedup firstResult = ${firstResult.length}` + - ` nextResult = ${nextResult.length}`, - ); - const dedupedList = combiner(firstResult, nextResult); - logDebug(`Deduped list = ${dedupedList.length}`); - resolve(dedupedList); - } - } - }; - - if (promiseList.length === 1) { - return promiseList[0].then( - function (result: Array) { - resolve(result); - }, - function (err) { - reject([err]); - }, - ); - } - - const firstPromise = promiseList[0]; - const nextPromise = combinedPromises(promiseList.slice(1), combiner); - - firstPromise - .then( - function (currentFirstResult: Array) { - firstResult = currentFirstResult; - firstPromiseDone = true; - }, - function (error) { - firstResult = []; - firstError = error; - firstPromiseDone = true; - }, - ) - .then(checkAndResolve); - - nextPromise - .then( - function (currentNextResult: Array) { - nextResult = currentNextResult; - nextPromiseDone = true; - }, - function (error) { - nextResult = []; - nextError = error; - nextPromiseDone = true; - }, - ) - .then(checkAndResolve); + Promise.allSettled(promiseList).then( + (results) => { + var allRej = true; + var values = []; + var rejections = []; + results.forEach((item) => { + if (item.status === 'fulfilled') { + if (allRej) allRej = false; + if (item.value.length != 0) values.push(item.value); + } else rejections.push(item.reason); + }); + if (allRej) reject(rejections); + else resolve(filter(values.flat(1))); + }, + (err) => { + reject(err); + }, + ); }); }; @@ -121,5 +64,5 @@ export const getUnifiedDataForInterval = function ( return serverResponse.phone_data; }); const promiseList = [getPromise, remotePromise]; - return combinedPromises(promiseList, combineWithDedup); -}; + return combinedPromises(promiseList, removeDup); +}; \ No newline at end of file