Skip to content
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

chore: global sync per minute safety limit #2765

Merged
merged 10 commits into from Jan 11, 2024
Expand Up @@ -18,6 +18,11 @@ export interface ApplicationOptionalConfiguratioOptions {
*/
webSocketUrl?: string

/**
* Amount sync calls allowed per minute.
*/
syncCallsThresholdPerMinute?: number

/**
* 3rd party library function for prompting U2F authenticator device registration
*
Expand Down
11 changes: 11 additions & 0 deletions packages/snjs/lib/Application/Dependencies/Dependencies.ts
Expand Up @@ -176,11 +176,15 @@ import { Logger, isNotUndefined, isDeinitable, LoggerInterface } from '@standard
import { EncryptionOperators } from '@standardnotes/encryption'
import { AsymmetricMessagePayload, AsymmetricMessageSharedVaultInvite } from '@standardnotes/models'
import { PureCryptoInterface } from '@standardnotes/sncrypto-common'
import { SyncFrequencyGuard } from '@Lib/Services/Sync/SyncFrequencyGuard'
import { SyncFrequencyGuardInterface } from '@Lib/Services/Sync/SyncFrequencyGuardInterface'

export class Dependencies {
private factory = new Map<symbol, () => unknown>()
private dependencies = new Map<symbol, unknown>()

private DEFAULT_SYNC_CALLS_THRESHOLD_PER_MINUTE = 200

constructor(private options: FullyResolvedApplicationOptions) {
this.dependencies.set(TYPES.DeviceInterface, options.deviceInterface)
this.dependencies.set(TYPES.AlertService, options.alertService)
Expand Down Expand Up @@ -1341,6 +1345,12 @@ export class Dependencies {
)
})

this.factory.set(TYPES.SyncFrequencyGuard, () => {
return new SyncFrequencyGuard(
this.options.syncCallsThresholdPerMinute ?? this.DEFAULT_SYNC_CALLS_THRESHOLD_PER_MINUTE,
)
})

this.factory.set(TYPES.SyncService, () => {
return new SyncService(
this.get<ItemManager>(TYPES.ItemManager),
Expand All @@ -1358,6 +1368,7 @@ export class Dependencies {
},
this.get<Logger>(TYPES.Logger),
this.get<WebSocketsService>(TYPES.WebSocketsService),
this.get<SyncFrequencyGuardInterface>(TYPES.SyncFrequencyGuard),
this.get<InternalEventBus>(TYPES.InternalEventBus),
)
})
Expand Down
1 change: 1 addition & 0 deletions packages/snjs/lib/Application/Dependencies/Types.ts
Expand Up @@ -29,6 +29,7 @@ export const TYPES = {
SessionManager: Symbol.for('SessionManager'),
SubscriptionManager: Symbol.for('SubscriptionManager'),
HistoryManager: Symbol.for('HistoryManager'),
SyncFrequencyGuard: Symbol.for('SyncFrequencyGuard'),
SyncService: Symbol.for('SyncService'),
ProtectionService: Symbol.for('ProtectionService'),
UserService: Symbol.for('UserService'),
Expand Down
38 changes: 38 additions & 0 deletions packages/snjs/lib/Services/Sync/SyncFrequencyGuard.spec.ts
@@ -0,0 +1,38 @@
import { SyncFrequencyGuard } from './SyncFrequencyGuard'

describe('SyncFrequencyGuard', () => {
const createUseCase = () => new SyncFrequencyGuard(3)

it('should return false when sync calls threshold is not reached', () => {
const useCase = createUseCase()

expect(useCase.isSyncCallsThresholdReachedThisMinute()).toBe(false)
})

it('should return true when sync calls threshold is reached', () => {
const useCase = createUseCase()

useCase.incrementCallsPerMinute()
useCase.incrementCallsPerMinute()
useCase.incrementCallsPerMinute()

expect(useCase.isSyncCallsThresholdReachedThisMinute()).toBe(true)
})

it('should return false when sync calls threshold is reached but a new minute has started', () => {
const spyOnGetCallsPerMinuteKey = jest.spyOn(SyncFrequencyGuard.prototype as any, 'getCallsPerMinuteKey')
spyOnGetCallsPerMinuteKey.mockReturnValueOnce('2020-1-1T1:1')

const useCase = createUseCase()

useCase.incrementCallsPerMinute()
useCase.incrementCallsPerMinute()
useCase.incrementCallsPerMinute()

spyOnGetCallsPerMinuteKey.mockReturnValueOnce('2020-1-1T1:2')

expect(useCase.isSyncCallsThresholdReachedThisMinute()).toBe(false)

spyOnGetCallsPerMinuteKey.mockRestore()
})
})
40 changes: 40 additions & 0 deletions packages/snjs/lib/Services/Sync/SyncFrequencyGuard.ts
@@ -0,0 +1,40 @@
import { SyncFrequencyGuardInterface } from './SyncFrequencyGuardInterface'

export class SyncFrequencyGuard implements SyncFrequencyGuardInterface {
private callsPerMinuteMap: Map<string, number>

constructor(private syncCallsThresholdPerMinute: number) {
this.callsPerMinuteMap = new Map<string, number>()
}

isSyncCallsThresholdReachedThisMinute(): boolean {
const stringDateToTheMinute = this.getCallsPerMinuteKey()
const persistedCallsCount = this.callsPerMinuteMap.get(stringDateToTheMinute) || 0

return persistedCallsCount >= this.syncCallsThresholdPerMinute
}

incrementCallsPerMinute(): void {
const stringDateToTheMinute = this.getCallsPerMinuteKey()
const persistedCallsCount = this.callsPerMinuteMap.get(stringDateToTheMinute)
const newMinuteStarted = persistedCallsCount === undefined

if (newMinuteStarted) {
this.clear()

this.callsPerMinuteMap.set(stringDateToTheMinute, 1)
} else {
this.callsPerMinuteMap.set(stringDateToTheMinute, persistedCallsCount + 1)
}
}

clear(): void {
this.callsPerMinuteMap.clear()
}

private getCallsPerMinuteKey(): string {
const now = new Date()

return `${now.getFullYear()}-${now.getMonth()}-${now.getDate()}T${now.getHours()}:${now.getMinutes()}`
}
}
@@ -0,0 +1,5 @@
export interface SyncFrequencyGuardInterface {
incrementCallsPerMinute(): void
isSyncCallsThresholdReachedThisMinute(): boolean
clear(): void
}
7 changes: 6 additions & 1 deletion packages/snjs/lib/Services/Sync/SyncService.ts
Expand Up @@ -97,6 +97,7 @@ import {
import { CreatePayloadFromRawServerItem } from './Account/Utilities'
import { DecryptedServerConflictMap, TrustedServerConflictMap } from './Account/ServerConflictMap'
import { ContentType } from '@standardnotes/domain-core'
import { SyncFrequencyGuardInterface } from './SyncFrequencyGuardInterface'

const DEFAULT_MAJOR_CHANGE_THRESHOLD = 15
const INVALID_SESSION_RESPONSE_STATUS = 401
Expand Down Expand Up @@ -169,6 +170,7 @@ export class SyncService
private readonly options: ApplicationSyncOptions,
private logger: LoggerInterface,
private sockets: WebSocketsService,
private syncFrequencyGuard: SyncFrequencyGuardInterface,
protected override internalEventBus: InternalEventBusInterface,
) {
super(internalEventBus)
Expand Down Expand Up @@ -643,7 +645,8 @@ export class SyncService
const syncInProgress = this.opStatus.syncInProgress
const databaseLoaded = this.databaseLoaded
const canExecuteSync = !this.syncLock
const shouldExecuteSync = canExecuteSync && databaseLoaded && !syncInProgress
const syncLimitReached = this.syncFrequencyGuard.isSyncCallsThresholdReachedThisMinute()
const shouldExecuteSync = canExecuteSync && databaseLoaded && !syncInProgress && !syncLimitReached

if (shouldExecuteSync) {
this.syncLock = true
Expand Down Expand Up @@ -1296,6 +1299,8 @@ export class SyncService

this.lastSyncDate = new Date()

this.syncFrequencyGuard.incrementCallsPerMinute()

if (operation instanceof AccountSyncOperation && operation.numberOfItemsInvolved >= this.majorChangeThreshold) {
void this.notifyEvent(SyncEvent.MajorDataChange)
}
Expand Down
4 changes: 3 additions & 1 deletion packages/snjs/mocha/lib/AppContext.js
Expand Up @@ -16,13 +16,14 @@ const MaximumSyncOptions = {
let GlobalSubscriptionIdCounter = 1001

export class AppContext {
constructor({ identifier, crypto, email, password, passcode, host } = {}) {
constructor({ identifier, crypto, email, password, passcode, host, syncCallsThresholdPerMinute } = {}) {
this.identifier = identifier || `${Math.random()}`
this.crypto = crypto
this.email = email || UuidGenerator.GenerateUuid()
this.password = password || UuidGenerator.GenerateUuid()
this.passcode = passcode || 'mypasscode'
this.host = host || Defaults.getDefaultHost()
this.syncCallsThresholdPerMinute = syncCallsThresholdPerMinute
}

enableLogging() {
Expand All @@ -46,6 +47,7 @@ export class AppContext {
undefined,
this.host,
this.crypto || new FakeWebCrypto(),
this.syncCallsThresholdPerMinute,
)

this.application.dependencies.get(TYPES.Logger).setLevel('error')
Expand Down
7 changes: 4 additions & 3 deletions packages/snjs/mocha/lib/Applications.js
Expand Up @@ -2,7 +2,7 @@ import WebDeviceInterface from './web_device_interface.js'
import FakeWebCrypto from './fake_web_crypto.js'
import * as Defaults from './Defaults.js'

export function createApplicationWithOptions({ identifier, environment, platform, host, crypto, device }) {
export function createApplicationWithOptions({ identifier, environment, platform, host, crypto, device, syncCallsThresholdPerMinute }) {
if (!device) {
device = new WebDeviceInterface()
device.environment = environment
Expand All @@ -22,11 +22,12 @@ export function createApplicationWithOptions({ identifier, environment, platform
defaultHost: host || Defaults.getDefaultHost(),
appVersion: Defaults.getAppVersion(),
webSocketUrl: Defaults.getDefaultWebSocketUrl(),
syncCallsThresholdPerMinute,
})
}

export function createApplication(identifier, environment, platform, host, crypto) {
return createApplicationWithOptions({ identifier, environment, platform, host, crypto })
export function createApplication(identifier, environment, platform, host, crypto, syncCallsThresholdPerMinute) {
return createApplicationWithOptions({ identifier, environment, platform, host, crypto, syncCallsThresholdPerMinute })
}

export function createApplicationWithFakeCrypto(identifier, environment, platform, host) {
Expand Down
8 changes: 4 additions & 4 deletions packages/snjs/mocha/lib/factory.js
Expand Up @@ -43,16 +43,16 @@ export async function createAndInitSimpleAppContext(
}
}

export async function createAppContextWithFakeCrypto(identifier, email, password) {
return createAppContext({ identifier, crypto: new FakeWebCrypto(), email, password })
export async function createAppContextWithFakeCrypto(identifier, email, password, syncCallsThresholdPerMinute) {
return createAppContext({ identifier, crypto: new FakeWebCrypto(), email, password, syncCallsThresholdPerMinute })
}

export async function createAppContextWithRealCrypto(identifier) {
return createAppContext({ identifier, crypto: new SNWebCrypto() })
}

export async function createAppContext({ identifier, crypto, email, password, host } = {}) {
const context = new AppContext({ identifier, crypto, email, password, host })
export async function createAppContext({ identifier, crypto, email, password, host, syncCallsThresholdPerMinute } = {}) {
const context = new AppContext({ identifier, crypto, email, password, host, syncCallsThresholdPerMinute })
await context.initialize()
return context
}
Expand Down
24 changes: 23 additions & 1 deletion packages/snjs/mocha/sync_tests/online.test.js
Expand Up @@ -13,6 +13,7 @@ describe('online syncing', function () {
let password
let expectedItemCount
let context
let safeGuard

const syncOptions = {
checkIntegrity: true,
Expand All @@ -37,6 +38,10 @@ describe('online syncing', function () {
email: email,
password: password,
})

safeGuard = application.dependencies.get(TYPES.SyncFrequencyGuard)

safeGuard.clear()
})

afterEach(async function () {
Expand All @@ -50,8 +55,11 @@ describe('online syncing', function () {
await Factory.safeDeinit(application)
localStorage.clear()

safeGuard.clear()

application = undefined
context = undefined
safeGuard = undefined
})

function noteObjectsFromObjects(items) {
Expand Down Expand Up @@ -433,6 +441,20 @@ describe('online syncing', function () {
expect(allItems.length).to.equal(expectedItemCount)
})

it('should defer syncing if syncing is breaching the sync calls per minute threshold', async function () {
let syncCount = 0
while(!safeGuard.isSyncCallsThresholdReachedThisMinute()) {
await application.sync.sync({
onPresyncSave: () => {
syncCount++
}
})
}

expect(safeGuard.isSyncCallsThresholdReachedThisMinute()).to.equal(true)
expect(syncCount == 200).to.equal(true)
})

it('items that are never synced and deleted should not be uploaded to server', async function () {
const note = await Factory.createMappedNote(application)
await application.mutator.setItemDirty(note)
Expand Down Expand Up @@ -575,7 +597,7 @@ describe('online syncing', function () {
it('should sync all items including ones that are breaching transfer limit', async function () {
const response = await fetch('/mocha/assets/small_file.md')
const buffer = new Uint8Array(await response.arrayBuffer())
const numberOfNotesToExceedThe1MBTransferLimit = 80
const numberOfNotesToExceedThe1MBTransferLimit = Math.ceil(100_000 / buffer.length) + 1

const testContext = await Factory.createAppContextWithFakeCrypto()
await testContext.launch()
Expand Down