-
Notifications
You must be signed in to change notification settings - Fork 114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✏️ Rewrite KVStore, ClientStats, and CommHelper #1040
Merged
shankari
merged 26 commits into
e-mission:service_rewrite_2023
from
JGreenlee:rewrite-services-sept2023
Oct 13, 2023
Merged
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
73a8259
add storage.ts, to replace KVStore
JGreenlee b99e001
use storage.ts everywhere instead of KVStore
JGreenlee 6ffbf18
remove KVStore / ngStorage.js
JGreenlee cdabde4
use JSDOM as jest env so localStorage works
JGreenlee fd4f9d5
add tests for storage.ts, plus the needed mocks
JGreenlee 8ada4f8
add clientStats.js, to replace ClientStats service
JGreenlee 6e59af5
use clientStats.ts everywhere, not old ClientStats
JGreenlee 25c7077
remove the old ClientStats / clientstats.js
JGreenlee 1a47634
fix getAppVersion in clientstats
JGreenlee 598d1fb
add some cordova mocks: cordova,device,appversion
JGreenlee 47aceaf
make getAppVersion work async
JGreenlee c7b06b7
add tests for clientStats.ts
JGreenlee c16bde5
rewrite CommHelper service into commHelper.ts
JGreenlee 1dc7451
use commHelper.ts everywhere, not old CommHelper
JGreenlee f78dab1
remove the old CommHelper service
JGreenlee 5826213
Merge branch 'react_navigation_new_onboarding' of https://github.com/…
JGreenlee 77fe30a
add commHelper.test.ts
JGreenlee a5fcbfe
remove backwards-compat munge in storage.ts
JGreenlee 40ce229
don't displayErrorMsg in storage.ts
JGreenlee 37ac065
remove unneeded comment
JGreenlee cfd7829
show error if 'db' not defined in clientStats.ts
JGreenlee ea2b8c5
re-implement functions of commHelper
JGreenlee 641e8aa
don't "processErrorMessages" in commHelper
JGreenlee 03dc94e
update comment in commHelper.test.ts
JGreenlee acb36aa
cordovaMocks: use cordova-ios version from json
JGreenlee e546d73
commHelper: promisify 'registerUser'
JGreenlee File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
import packageJsonBuild from '../../package.cordovabuild.json'; | ||
|
||
export const mockCordova = () => { | ||
window['cordova'] ||= {}; | ||
window['cordova'].platformId ||= 'ios'; | ||
window['cordova'].platformVersion ||= packageJsonBuild.dependencies['cordova-ios']; | ||
window['cordova'].plugins ||= {}; | ||
} | ||
|
||
export const mockDevice = () => { | ||
window['device'] ||= {}; | ||
window['device'].platform ||= 'ios'; | ||
window['device'].version ||= '14.0.0'; | ||
} | ||
|
||
export const mockGetAppVersion = () => { | ||
const mockGetAppVersion = { | ||
getAppName: () => new Promise((rs, rj) => setTimeout(() => rs('Mock App'), 10)), | ||
getPackageName: () => new Promise((rs, rj) => setTimeout(() => rs('com.example.mockapp'), 10)), | ||
getVersionCode: () => new Promise((rs, rj) => setTimeout(() => rs('123'), 10)), | ||
getVersionNumber: () => new Promise((rs, rj) => setTimeout(() => rs('1.2.3'), 10)), | ||
} | ||
window['cordova'] ||= {}; | ||
window['cordova'].getAppVersion = mockGetAppVersion; | ||
} | ||
|
||
export const mockBEMUserCache = () => { | ||
const _cache = {}; | ||
const messages = []; | ||
const mockBEMUserCache = { | ||
getLocalStorage: (key: string, isSecure: boolean) => { | ||
return new Promise((rs, rj) => | ||
setTimeout(() => { | ||
rs(_cache[key]); | ||
}, 100) | ||
); | ||
}, | ||
putLocalStorage: (key: string, value: any) => { | ||
return new Promise<void>((rs, rj) => | ||
setTimeout(() => { | ||
_cache[key] = value; | ||
rs(); | ||
}, 100) | ||
); | ||
}, | ||
removeLocalStorage: (key: string) => { | ||
return new Promise<void>((rs, rj) => | ||
setTimeout(() => { | ||
delete _cache[key]; | ||
rs(); | ||
}, 100) | ||
); | ||
}, | ||
clearAll: () => { | ||
return new Promise<void>((rs, rj) => | ||
setTimeout(() => { | ||
for (let p in _cache) delete _cache[p]; | ||
rs(); | ||
}, 100) | ||
); | ||
}, | ||
listAllLocalStorageKeys: () => { | ||
return new Promise<string[]>((rs, rj) => | ||
setTimeout(() => { | ||
rs(Object.keys(_cache)); | ||
}, 100) | ||
); | ||
}, | ||
listAllUniqueKeys: () => { | ||
return new Promise<string[]>((rs, rj) => | ||
setTimeout(() => { | ||
rs(Object.keys(_cache)); | ||
shankari marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, 100) | ||
); | ||
}, | ||
putMessage: (key: string, value: any) => { | ||
return new Promise<void>((rs, rj) => | ||
setTimeout(() => { | ||
messages.push({ key, value }); | ||
rs(); | ||
}, 100) | ||
); | ||
}, | ||
getAllMessages: (key: string, withMetadata?: boolean) => { | ||
return new Promise<any[]>((rs, rj) => | ||
setTimeout(() => { | ||
rs(messages.filter(m => m.key == key).map(m => m.value)); | ||
}, 100) | ||
); | ||
} | ||
} | ||
window['cordova'] ||= {}; | ||
window['cordova'].plugins ||= {}; | ||
window['cordova'].plugins.BEMUserCache = mockBEMUserCache; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export const mockLogger = () => { | ||
window['Logger'] = { log: console.log }; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
import { mockBEMUserCache, mockDevice, mockGetAppVersion } from "../__mocks__/cordovaMocks"; | ||
import { addStatError, addStatEvent, addStatReading, getAppVersion, statKeys } from "../js/plugin/clientStats"; | ||
|
||
mockDevice(); | ||
// this mocks cordova-plugin-app-version, generating a "Mock App", version "1.2.3" | ||
mockGetAppVersion(); | ||
// clientStats.ts uses BEMUserCache to store the stats, so we need to mock that too | ||
shankari marked this conversation as resolved.
Show resolved
Hide resolved
|
||
mockBEMUserCache(); | ||
const db = window['cordova']?.plugins?.BEMUserCache; | ||
|
||
it('gets the app version', async () => { | ||
const ver = await getAppVersion(); | ||
expect(ver).toEqual('1.2.3'); | ||
}); | ||
|
||
it('stores a client stats reading', async () => { | ||
const reading = { a: 1, b: 2 }; | ||
await addStatReading(statKeys.REMINDER_PREFS, reading); | ||
const storedMessages = await db.getAllMessages('stats/client_time', false); | ||
expect(storedMessages).toContainEqual({ | ||
name: statKeys.REMINDER_PREFS, | ||
ts: expect.any(Number), | ||
reading, | ||
client_app_version: '1.2.3', | ||
client_os_version: '14.0.0' | ||
}); | ||
}); | ||
|
||
it('stores a client stats event', async () => { | ||
await addStatEvent(statKeys.BUTTON_FORCE_SYNC); | ||
const storedMessages = await db.getAllMessages('stats/client_nav_event', false); | ||
expect(storedMessages).toContainEqual({ | ||
name: statKeys.BUTTON_FORCE_SYNC, | ||
ts: expect.any(Number), | ||
reading: null, | ||
client_app_version: '1.2.3', | ||
client_os_version: '14.0.0' | ||
}); | ||
}); | ||
|
||
it('stores a client stats error', async () => { | ||
const errorStr = 'test error'; | ||
await addStatError(statKeys.MISSING_KEYS, errorStr); | ||
const storedMessages = await db.getAllMessages('stats/client_error', false); | ||
expect(storedMessages).toContainEqual({ | ||
name: statKeys.MISSING_KEYS, | ||
ts: expect.any(Number), | ||
reading: errorStr, | ||
client_app_version: '1.2.3', | ||
client_os_version: '14.0.0' | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import { mockLogger } from '../__mocks__/globalMocks'; | ||
import { fetchUrlCached } from '../js/commHelper'; | ||
|
||
mockLogger(); | ||
|
||
// mock for JavaScript 'fetch' | ||
// we emulate a 100ms delay when i) fetching data and ii) parsing it as text | ||
global.fetch = (url: string) => new Promise((rs, rj) => { | ||
setTimeout(() => rs({ | ||
text: () => new Promise((rs, rj) => { | ||
setTimeout(() => rs('mock data for ' + url), 100); | ||
}) | ||
})); | ||
}) as any; | ||
|
||
it('fetches text from a URL and caches it so the next call is faster', async () => { | ||
const tsBeforeCalls = Date.now(); | ||
const text1 = await fetchUrlCached('https://raw.githubusercontent.com/e-mission/e-mission-phone/master/README.md'); | ||
const tsBetweenCalls = Date.now(); | ||
const text2 = await fetchUrlCached('https://raw.githubusercontent.com/e-mission/e-mission-phone/master/README.md'); | ||
const tsAfterCalls = Date.now(); | ||
expect(text1).toEqual(expect.stringContaining('mock data')); | ||
expect(text2).toEqual(expect.stringContaining('mock data')); | ||
expect(tsAfterCalls - tsBetweenCalls).toBeLessThan(tsBetweenCalls - tsBeforeCalls); | ||
}); | ||
|
||
/* The following functions from commHelper.ts are not tested because they are just wrappers | ||
around the native functions in BEMServerComm. | ||
If we wanted to test them, we would need to mock the native functions in BEMServerComm. | ||
It would be better to do integration tests that actually call the native functions. | ||
* - getRawEntries | ||
* - getRawEntriesForLocalDate | ||
* - getPipelineRangeTs | ||
* - getPipelineCompleteTs | ||
* - getMetrics | ||
* - getAggregateData | ||
* - registerUser | ||
* - updateUser | ||
* - getUser | ||
* - putOne | ||
*/ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
import { mockBEMUserCache } from "../__mocks__/cordovaMocks"; | ||
import { mockLogger } from "../__mocks__/globalMocks"; | ||
import { storageClear, storageGet, storageRemove, storageSet } from "../js/plugin/storage"; | ||
|
||
// mocks used - storage.ts uses BEMUserCache and logging. | ||
// localStorage is already mocked for us by Jest :) | ||
mockLogger(); | ||
mockBEMUserCache(); | ||
|
||
it('stores a value and retrieves it back', async () => { | ||
await storageSet('test1', 'test value 1'); | ||
const retVal = await storageGet('test1'); | ||
expect(retVal).toEqual('test value 1'); | ||
}); | ||
|
||
it('stores a value, removes it, and checks that it is gone', async () => { | ||
await storageSet('test2', 'test value 2'); | ||
await storageRemove('test2'); | ||
const retVal = await storageGet('test2'); | ||
expect(retVal).toBeUndefined(); | ||
}); | ||
|
||
it('can store objects too', async () => { | ||
const obj = { a: 1, b: 2 }; | ||
await storageSet('test6', obj); | ||
const retVal = await storageGet('test6'); | ||
expect(retVal).toEqual(obj); | ||
}); | ||
|
||
it('can also store complex nested objects with arrays', async () => { | ||
const obj = { a: 1, b: { c: [1, 2, 3] } }; | ||
await storageSet('test7', obj); | ||
const retVal = await storageGet('test7'); | ||
expect(retVal).toEqual(obj); | ||
}); | ||
|
||
it('preserves values if local gets cleared', async () => { | ||
await storageSet('test3', 'test value 3'); | ||
await storageClear({ local: true }); | ||
const retVal = await storageGet('test3'); | ||
expect(retVal).toEqual('test value 3'); | ||
}); | ||
|
||
it('preserves values if native gets cleared', async () => { | ||
await storageSet('test4', 'test value 4'); | ||
await storageClear({ native: true }); | ||
const retVal = await storageGet('test4'); | ||
expect(retVal).toEqual('test value 4'); | ||
}); | ||
|
||
it('does not preserve values if both local and native are cleared', async () => { | ||
await storageSet('test5', 'test value 5'); | ||
await storageClear({ local: true, native: true }); | ||
const retVal = await storageGet('test5'); | ||
expect(retVal).toBeUndefined(); | ||
}); | ||
|
||
it('preserves values if local gets cleared, then retrieved, then native gets cleared', async () => { | ||
await storageSet('test8', 'test value 8'); | ||
await storageClear({ local: true }); | ||
await storageGet('test8'); | ||
await storageClear({ native: true }); | ||
const retVal = await storageGet('test8'); | ||
expect(retVal).toEqual('test value 8'); | ||
}); | ||
|
||
it('preserves values if native gets cleared, then retrieved, then local gets cleared', async () => { | ||
shankari marked this conversation as resolved.
Show resolved
Hide resolved
|
||
await storageSet('test9', 'test value 9'); | ||
await storageClear({ native: true }); | ||
await storageGet('test9'); | ||
await storageClear({ local: true }); | ||
const retVal = await storageGet('test9'); | ||
expect(retVal).toEqual('test value 9'); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume that the
setTimeout
is to delay the response.future fix: you could pass in the timeout as a parameter to the mock (assuming that something like
mockBEMUserCache(delay)
works and then we could test what happens if the plugins are slow.