Skip to content

Commit

Permalink
refactor: optimize delay between batches on mobile to allow UI intera…
Browse files Browse the repository at this point in the history
…ctivity during load (#2129)
  • Loading branch information
moughxyz committed Jan 4, 2023
1 parent 69b2af7 commit 59fc682
Show file tree
Hide file tree
Showing 32 changed files with 171 additions and 67 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"build:services": "yarn workspaces foreach -pt --topological-dev --verbose -R --from @standardnotes/services run build",
"build:api": "yarn workspaces foreach -pt --topological-dev --verbose -R --from @standardnotes/api run build",
"start:server:web": "lerna run start --scope=@standardnotes/web",
"start:server:e2e": "lerna run start:test-server --scope=@standardnotes/snjs",
"e2e": "lerna run start:test-server --scope=@standardnotes/snjs",
"reset": "find . -type dir -name node_modules | xargs rm -rf && rm -rf yarn.lock && yarn install",
"release:prod": "lerna version --conventional-commits --yes -m \"chore(release): publish\"",
"publish:prod": "lerna publish from-git --yes --no-verify-access --loglevel verbose",
Expand Down
3 changes: 2 additions & 1 deletion packages/services/src/Domain/Strings/Messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,5 +182,6 @@ export const ErrorAlertStrings = {

export const KeychainRecoveryStrings = {
Title: 'Restore Keychain',
Text: "We've detected that your keychain has been wiped. This can happen when restoring your device from a backup. Please enter your account password to restore your account keys.",
Text: (email: string) =>
`We've detected that your keychain has been wiped. This can happen when restoring your device from a backup. Please enter your account password for "${email}" to restore your account keys.`,
}
2 changes: 2 additions & 0 deletions packages/services/src/Domain/User/UserClientInterface.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Base64String } from '@standardnotes/sncrypto-common'
import { UserRequestType } from '@standardnotes/common'
import { DeinitSource } from '../Application/DeinitSource'

Expand All @@ -8,4 +9,5 @@ export interface UserClientInterface {
}>
signOut(force?: boolean, source?: DeinitSource): Promise<void>
submitUserRequest(requestType: UserRequestType): Promise<boolean>
populateSessionFromDemoShareToken(token: Base64String): Promise<void>
}
6 changes: 6 additions & 0 deletions packages/services/src/Domain/User/UserService.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Base64String } from '@standardnotes/sncrypto-common'
import { EncryptionProviderInterface, SNRootKey, SNRootKeyParams } from '@standardnotes/encryption'
import { HttpResponse, SignInResponse, User } from '@standardnotes/responses'
import { KeyParamsOrigination, UserRequestType } from '@standardnotes/common'
Expand Down Expand Up @@ -470,6 +471,11 @@ export class UserService extends AbstractService<AccountEvent, AccountEventData>
}
}

public async populateSessionFromDemoShareToken(token: Base64String): Promise<void> {
await this.sessionManager.populateSessionFromDemoShareToken(token)
await this.notifyEvent(AccountEvent.SignedInOrRegistered)
}

private async setPasscodeWithoutWarning(passcode: string, origination: KeyParamsOrigination) {
const identifier = UuidGenerator.GenerateUuid()
const key = await this.protocolService.createRootKey(identifier, passcode, origination)
Expand Down
2 changes: 1 addition & 1 deletion packages/snjs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ Once the server infrastructure is ready, and you've built all packages, you can

In app repo:
```
yarn start:server:e2e
yarn e2e
```

Once you are finished you can close the running local server on E2E repo by typing:
Expand Down
1 change: 1 addition & 0 deletions packages/snjs/lib/Application/Application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1581,6 +1581,7 @@ export class SNApplication implements ApplicationInterface, AppGroupManagedAppli
this.identifier,
{
loadBatchSize: this.options.loadBatchSize,
sleepBetweenBatches: this.options.sleepBetweenBatches,
},
this.internalEventBus,
)
Expand Down
8 changes: 7 additions & 1 deletion packages/snjs/lib/Application/Options/Defaults.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
import { ApplicationSyncOptions } from './OptionalOptions'
import { ApplicationDisplayOptions, ApplicationSyncOptions } from './OptionalOptions'

export interface ApplicationOptionsWhichHaveDefaults {
loadBatchSize: ApplicationSyncOptions['loadBatchSize']
sleepBetweenBatches: ApplicationSyncOptions['sleepBetweenBatches']
allowNoteSelectionStatePersistence: ApplicationDisplayOptions['allowNoteSelectionStatePersistence']
allowMultipleSelection: ApplicationDisplayOptions['allowMultipleSelection']
}

export const ApplicationOptionsDefaults: ApplicationOptionsWhichHaveDefaults = {
loadBatchSize: 700,
sleepBetweenBatches: 10,
allowMultipleSelection: true,
allowNoteSelectionStatePersistence: true,
}
8 changes: 6 additions & 2 deletions packages/snjs/lib/Application/Options/OptionalOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@ export interface ApplicationSyncOptions {
* The size of the item batch to decrypt and render upon application load.
*/
loadBatchSize: number

sleepBetweenBatches: number
}

// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface ApplicationDisplayOptions {}
export interface ApplicationDisplayOptions {
allowNoteSelectionStatePersistence: boolean
allowMultipleSelection: boolean
}

export interface ApplicationOptionalConfiguratioOptions {
/**
Expand Down
8 changes: 4 additions & 4 deletions packages/snjs/lib/Migrations/Base.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AnyKeyParamsContent } from '@standardnotes/common'
import { AnyKeyParamsContent, KeyParamsContent004 } from '@standardnotes/common'
import { EncryptedPayload, EncryptedTransferPayload, isErrorDecryptingPayload } from '@standardnotes/models'
import { PreviousSnjsVersion1_0_0, PreviousSnjsVersion2_0_0, SnjsVersion } from '../Version'
import { Migration } from '@Lib/Migrations/Migration'
Expand Down Expand Up @@ -165,7 +165,7 @@ export class BaseMigration extends Migration {
}

private async repairMissingKeychain() {
const rawAccountParams = await this.reader.getAccountKeyParams()
const rawAccountParams = (await this.reader.getAccountKeyParams()) as AnyKeyParamsContent

/** Choose an item to decrypt against */
const allItems = (
Expand Down Expand Up @@ -196,14 +196,14 @@ export class BaseMigration extends Migration {
ChallengeReason.Custom,
false,
KeychainRecoveryStrings.Title,
KeychainRecoveryStrings.Text,
KeychainRecoveryStrings.Text((rawAccountParams as KeyParamsContent004).identifier),
)

return new Promise((resolve) => {
this.services.challengeService.addChallengeObserver(challenge, {
onNonvalidatedSubmit: async (challengeResponse) => {
const password = challengeResponse.values[0].value as string
const accountParams = this.services.protocolService.createKeyParams(rawAccountParams as AnyKeyParamsContent)
const accountParams = this.services.protocolService.createKeyParams(rawAccountParams)
const rootKey = await this.services.protocolService.computeRootKey(password, accountParams)

/** TS can't detect we returned early above if itemToDecrypt is null */
Expand Down
12 changes: 10 additions & 2 deletions packages/snjs/lib/Services/Sync/SyncService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,9 @@ export class SNSyncService
? chunks.fullEntries.remainingChunks
: chunks.keys.remainingChunks

let chunkIndex = 0
const ChunkIndexOfContentTypePriorityItems = 0

for (const chunk of remainingChunks) {
const dbEntries = isChunkFullEntry(chunk)
? chunk.entries
Expand All @@ -314,7 +317,14 @@ export class SNSyncService
.filter(isNotUndefined)

await this.processPayloadBatch(payloads, totalProcessedCount, payloadCount)

const shouldSleepOnlyAfterFirstRegularBatch = chunkIndex > ChunkIndexOfContentTypePriorityItems
if (shouldSleepOnlyAfterFirstRegularBatch) {
await sleep(this.options.sleepBetweenBatches, false, 'Sleeping to allow interface to update')
}

totalProcessedCount += payloads.length
chunkIndex++
}

this.databaseLoaded = true
Expand Down Expand Up @@ -353,8 +363,6 @@ export class SNSyncService
if (currentPosition != undefined && payloadCount != undefined) {
this.opStatus.setDatabaseLoadStatus(currentPosition, payloadCount, false)
}

await sleep(1, false)
}

private setLastSyncToken(token: string) {
Expand Down
21 changes: 12 additions & 9 deletions packages/snjs/mocha/application.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,19 @@ describe('application instances', () => {
})

it('two distinct applications should not share model manager state', async () => {
const app1 = await Factory.createAndInitializeApplication('app1')
const app2 = await Factory.createAndInitializeApplication('app2')
expect(app1.payloadManager).to.equal(app1.payloadManager)
expect(app1.payloadManager).to.not.equal(app2.payloadManager)
const context1 = await Factory.createAppContext({ identifier: 'app1' })
const context2 = await Factory.createAppContext({ identifier: 'app2' })
await Promise.all([context1.launch(), context2.launch()])

await Factory.createMappedNote(app1)
expect(app1.itemManager.items.length).length.to.equal(BaseItemCounts.DefaultItems + 1)
expect(app2.itemManager.items.length).to.equal(BaseItemCounts.DefaultItems)
await Factory.safeDeinit(app1)
await Factory.safeDeinit(app2)
expect(context1.application.payloadManager).to.equal(context1.application.payloadManager)
expect(context1.application.payloadManager).to.not.equal(context2.application.payloadManager)

await Factory.createMappedNote(context1.application)
expect(context1.application.itemManager.items.length).length.to.equal(BaseItemCounts.DefaultItems + 1)
expect(context2.application.itemManager.items.length).to.equal(BaseItemCounts.DefaultItems)

await context1.deinit()
await context2.deinit()
})

it('two distinct applications should not share storage manager state', async () => {
Expand Down
6 changes: 4 additions & 2 deletions packages/snjs/mocha/auth.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ describe('basic auth', function () {
beforeEach(async function () {
localStorage.clear()
this.expectedItemCount = BaseItemCounts.DefaultItems
this.application = await Factory.createInitAppWithFakeCrypto()
this.context = await Factory.createAppContext()
await this.context.launch()
this.application = this.context.application
this.email = UuidGenerator.GenerateUuid()
this.password = UuidGenerator.GenerateUuid()
})
Expand Down Expand Up @@ -402,7 +404,7 @@ describe('basic auth', function () {
await this.application.syncService.markAllItemsAsNeedingSyncAndPersist()
await this.application.syncService.sync(syncOptions)

this.application = await Factory.signOutApplicationAndReturnNew(this.application)
this.application = await this.context.signout()

expect(this.application.itemManager.items.length).to.equal(BaseItemCounts.DefaultItems)
expect(this.application.payloadManager.invalidPayloads.length).to.equal(0)
Expand Down
32 changes: 32 additions & 0 deletions packages/snjs/mocha/lib/AppContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,13 @@ export class AppContext {
return newApplication
}

async signout() {
await this.application.user.signOut()
await this.initialize()
await this.launch()
return this.application
}

syncWithIntegrityCheck() {
return this.application.sync.sync({ checkIntegrity: true, awaitAll: true })
}
Expand All @@ -189,11 +196,36 @@ export class AppContext {
})
}

awaitUserPrefsSingletonCreation() {
const preferences = this.application.preferencesService.preferences
if (preferences) {
return
}

let didCompleteRelevantSync = false
return new Promise((resolve) => {
this.application.syncService.addEventObserver((eventName, data) => {
if (!didCompleteRelevantSync) {
if (data?.savedPayloads) {
const matching = data.savedPayloads.find((p) => {
return p.content_type === ContentType.UserPrefs
})
if (matching) {
didCompleteRelevantSync = true
resolve()
}
}
}
})
})
}

async launch({ awaitDatabaseLoad = true, receiveChallenge } = { awaitDatabaseLoad: true }) {
await this.application.prepareForLaunch({
receiveChallenge: receiveChallenge || this.handleChallenge,
})
await this.application.launch(awaitDatabaseLoad)
await this.awaitUserPrefsSingletonCreation()
}

async deinit() {
Expand Down
4 changes: 3 additions & 1 deletion packages/snjs/mocha/model_tests/appmodels.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ describe('app models', () => {

beforeEach(async function () {
this.expectedItemCount = BaseItemCounts.DefaultItems
this.application = await Factory.createInitAppWithFakeCrypto()
this.context = await Factory.createAppContext()
this.application = this.context.application
await this.context.launch()
})

afterEach(async function () {
Expand Down
10 changes: 8 additions & 2 deletions packages/snjs/mocha/model_tests/importing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,24 @@ describe('importing', function () {
let application
let email
let password
let context

beforeEach(function () {
localStorage.clear()
})

const setup = async ({ fakeCrypto }) => {
expectedItemCount = BaseItemCounts.DefaultItems

if (fakeCrypto) {
application = await Factory.createInitAppWithFakeCrypto()
context = await Factory.createAppContext()
} else {
application = await Factory.createInitAppWithRealCrypto()
context = await Factory.createAppContextWithRealCrypto()
}

await context.launch()
application = context.application

email = UuidGenerator.GenerateUuid()
password = UuidGenerator.GenerateUuid()
Factory.handlePasswordChallenges(application, password)
Expand Down
4 changes: 3 additions & 1 deletion packages/snjs/mocha/model_tests/mapping.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ const expect = chai.expect
describe('model manager mapping', () => {
beforeEach(async function () {
this.expectedItemCount = BaseItemCounts.DefaultItems
this.application = await Factory.createInitAppWithFakeCrypto()
this.context = await Factory.createAppContext()
await this.context.launch()
this.application = this.context.application
})

afterEach(async function () {
Expand Down
4 changes: 3 additions & 1 deletion packages/snjs/mocha/model_tests/notes_tags.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ describe('notes and tags', () => {

beforeEach(async function () {
this.expectedItemCount = BaseItemCounts.DefaultItems
this.application = await Factory.createInitAppWithFakeCrypto()
this.context = await Factory.createAppContext()
await this.context.launch()
this.application = this.context.application
})

afterEach(async function () {
Expand Down
4 changes: 3 additions & 1 deletion packages/snjs/mocha/model_tests/notes_tags_folders.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ const expect = chai.expect

describe('tags as folders', () => {
beforeEach(async function () {
this.application = await Factory.createInitAppWithFakeCrypto()
this.context = await Factory.createAppContext()
await this.context.launch()
this.application = this.context.application
})

afterEach(async function () {
Expand Down
6 changes: 5 additions & 1 deletion packages/snjs/mocha/preferences.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ const expect = chai.expect
describe('preferences', function () {
beforeEach(async function () {
localStorage.clear()
this.application = await Factory.createInitAppWithFakeCrypto()

this.context = await Factory.createAppContext()
await this.context.launch()
this.application = this.context.application

this.email = UuidGenerator.GenerateUuid()
this.password = UuidGenerator.GenerateUuid()
})
Expand Down
6 changes: 5 additions & 1 deletion packages/snjs/mocha/singletons.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ describe('singletons', function () {
beforeEach(async function () {
localStorage.clear()
this.expectedItemCount = BaseItemCounts.DefaultItems
this.application = await Factory.createInitAppWithFakeCrypto()

this.context = await Factory.createAppContext()
await this.context.launch()
this.application = this.context.application

this.email = UuidGenerator.GenerateUuid()
this.password = UuidGenerator.GenerateUuid()
this.registerUser = async () => {
Expand Down
7 changes: 5 additions & 2 deletions packages/snjs/mocha/storage.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ describe('storage manager', function () {
beforeEach(async function () {
localStorage.clear()
this.expectedKeyCount = BASE_KEY_COUNT
this.application = await Factory.createInitAppWithFakeCrypto(Environment.Mobile)
this.context = await Factory.createAppContext()
await this.context.launch()
this.application = this.context.application
this.email = UuidGenerator.GenerateUuid()
this.password = UuidGenerator.GenerateUuid()
})
Expand Down Expand Up @@ -221,7 +223,8 @@ describe('storage manager', function () {
expect(decrypted.content).to.be.an.instanceof(Object)
})

it('disabling storage encryption should store items without encryption', async function () {
/** @TODO: Storage encryption disable is no longer available, remove tests and associated functionality */
it.skip('disabling storage encryption should store items without encryption', async function () {
await Factory.registerUserToApplication({
application: this.application,
email: this.email,
Expand Down
4 changes: 3 additions & 1 deletion packages/snjs/mocha/sync_tests/offline.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ describe('offline syncing', () => {

beforeEach(async function () {
this.expectedItemCount = BaseItemCounts.DefaultItems
this.application = await Factory.createInitAppWithFakeCrypto()
this.context = await Factory.createAppContext()
await this.context.launch()
this.application = this.context.application
})

afterEach(async function () {
Expand Down
2 changes: 1 addition & 1 deletion packages/snjs/mocha/sync_tests/online.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('online syncing', function () {

await this.application.sync.sync(syncOptions)

this.application = await Factory.signOutApplicationAndReturnNew(this.application)
this.application = await this.context.signout()

expect(this.application.itemManager.items.length).to.equal(BaseItemCounts.DefaultItems)

Expand Down

0 comments on commit 59fc682

Please sign in to comment.