From 58a565ab74b022a1d0a774448f72fdd08731b0cd Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Fri, 28 Oct 2022 15:59:06 +0700 Subject: [PATCH 01/29] Set up MockPushMessagingService in tests --- external/@worldbrain/memex-common | 2 +- src/personal-cloud/background/index.tests.ts | 8 ++++- src/tests/background-integration-tests.ts | 10 +++++- src/tests/integration-tests.ts | 4 ++- src/tests/push-messaging.ts | 38 ++++++++++++++++++++ 5 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 src/tests/push-messaging.ts diff --git a/external/@worldbrain/memex-common b/external/@worldbrain/memex-common index 287df6fc1e..5ae1054b6e 160000 --- a/external/@worldbrain/memex-common +++ b/external/@worldbrain/memex-common @@ -1 +1 @@ -Subproject commit 287df6fc1e5893e5c8bce2cf7189ed6385abae2c +Subproject commit 5ae1054b6ece82d01b1d22a08bb4254c6476bc67 diff --git a/src/personal-cloud/background/index.tests.ts b/src/personal-cloud/background/index.tests.ts index eb6fb77a5e..084660bb51 100644 --- a/src/personal-cloud/background/index.tests.ts +++ b/src/personal-cloud/background/index.tests.ts @@ -31,6 +31,7 @@ import { registerModuleCollections } from '@worldbrain/storex-pattern-modules' import UserStorage from '@worldbrain/memex-common/lib/user-management/storage' import { StorageMiddleware } from '@worldbrain/storex/lib/types/middleware' import { createAuthServices } from 'src/services/local-services' +import { MockPushMessagingService } from 'src/tests/push-messaging' export const BASE_TIMESTAMP = 555 @@ -307,6 +308,7 @@ export async function setupSyncBackgroundTest( let now = BASE_TIMESTAMP const getNow = () => now++ + const pushMessagingService = new MockPushMessagingService() const setups: BackgroundIntegrationTestSetup[] = [] let getSqlStorageMananager: () => Promise | undefined @@ -370,7 +372,10 @@ export async function setupSyncBackgroundTest( storageModules: serverStorage.modules, getSqlStorageMananager, clientSchemaVersion: STORAGE_VERSIONS[25].version, - services, + services: { + activityStreams: services.activityStreams, + pushMessaging: pushMessagingService, + }, view: cloudHub.getView(), disableFailsafes: !options.enableFailsafes, getUserId: async () => userId, @@ -388,6 +393,7 @@ export async function setupSyncBackgroundTest( ...options, services, getServerStorage, + pushMessagingService, personalCloudBackend, }, ) diff --git a/src/tests/background-integration-tests.ts b/src/tests/background-integration-tests.ts index 6960bb248a..5a21f4e40d 100644 --- a/src/tests/background-integration-tests.ts +++ b/src/tests/background-integration-tests.ts @@ -45,6 +45,8 @@ import { PersonalDeviceType } from '@worldbrain/memex-common/lib/personal-cloud/ import { JobScheduler } from 'src/job-scheduler/background/job-scheduler' import { MockAlarmsApi } from 'src/job-scheduler/background/job-scheduler.test' import { createAuthServices } from 'src/services/local-services' +import { MockPushMessagingService } from './push-messaging' +import type { PushMessagingServiceInterface } from '@worldbrain/memex-common/lib/push-messaging/types' fetchMock.restore() export interface BackgroundIntegrationTestSetupOpts { @@ -58,6 +60,7 @@ export interface BackgroundIntegrationTestSetupOpts { startWithSyncDisabled?: boolean useDownloadTranslationLayer?: boolean services?: Services + pushMessagingService?: PushMessagingServiceInterface } export async function setupBackgroundIntegrationTest( @@ -196,7 +199,10 @@ export async function setupBackgroundIntegrationTest( personalCloudBackend: options?.personalCloudBackend ?? new StorexPersonalCloudBackend({ - services, + services: { + activityStreams: services.activityStreams, + pushMessaging: new MockPushMessagingService(), + }, storageManager: serverStorage.manager, storageModules: serverStorage.modules, clientSchemaVersion: STORAGE_VERSIONS[25].version, @@ -266,6 +272,8 @@ export async function setupBackgroundIntegrationTest( setStorex(storageManager) return { + pushMessagingService: + options.pushMessagingService ?? new MockPushMessagingService(), storageManager, persistentStorageManager, backgroundModules, diff --git a/src/tests/integration-tests.ts b/src/tests/integration-tests.ts index bb81df3ea2..0a353a8d3e 100644 --- a/src/tests/integration-tests.ts +++ b/src/tests/integration-tests.ts @@ -17,8 +17,9 @@ import type { MemorySubscriptionsService } from '@worldbrain/memex-common/lib/su import type { ServerStorage } from 'src/storage/types' import type { Browser } from 'webextension-polyfill' import type { MockFetchPageDataProcessor } from 'src/page-analysis/background/mock-fetch-page-data-processor' -import fetchMock from 'fetch-mock' +import type fetchMock from 'fetch-mock' import type { Services } from 'src/services/types' +import type { MockPushMessagingService } from './push-messaging' export interface IntegrationTestSuite { description: string @@ -69,6 +70,7 @@ export interface BackgroundIntegrationTestSetup { browserLocalStorage: MemoryBrowserStorage storageChangeDetector: StorageChangeDetector storageOperationLogger: StorageOperationLogger + pushMessagingService: MockPushMessagingService authService: MemoryAuthService subscriptionService: MemorySubscriptionsService getServerStorage(): Promise diff --git a/src/tests/push-messaging.ts b/src/tests/push-messaging.ts new file mode 100644 index 0000000000..2426dd4144 --- /dev/null +++ b/src/tests/push-messaging.ts @@ -0,0 +1,38 @@ +import type { + PushMessagePayload, + PushMessagingServiceInterface, +} from '@worldbrain/memex-common/lib/push-messaging/types' +import type { AutoPk } from '@worldbrain/memex-common/lib/storage/types' + +export class MockPushMessagingService implements PushMessagingServiceInterface { + sentMessages: Array< + { + payload: PushMessagePayload + } & ( + | { type: 'to-topic'; topic: string } + | { type: 'to-user'; userId: AutoPk } + ) + > = [] + + sendToTopic: PushMessagingServiceInterface['sendToTopic'] = async ( + topic, + payload, + ) => { + this.sentMessages.push({ + type: 'to-topic', + topic, + payload, + }) + } + + sendToUser: PushMessagingServiceInterface['sendToUser'] = async ( + userReference, + payload, + ) => { + this.sentMessages.push({ + type: 'to-user', + userId: userReference.id, + payload, + }) + } +} From 6f181154b4fcdfbe688c5aea333931c7b5931497 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Fri, 28 Oct 2022 15:59:34 +0700 Subject: [PATCH 02/29] Write up base storage module for page activity indicator --- .../background/storage.ts | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 src/page-activity-indicator/background/storage.ts diff --git a/src/page-activity-indicator/background/storage.ts b/src/page-activity-indicator/background/storage.ts new file mode 100644 index 0000000000..4ad2c5e340 --- /dev/null +++ b/src/page-activity-indicator/background/storage.ts @@ -0,0 +1,47 @@ +import Storex from '@worldbrain/storex' +import { + StorageModule, + StorageModuleConfig, +} from '@worldbrain/storex-pattern-modules' +import { STORAGE_VERSIONS } from 'src/storage/constants' + +export default class PageActivityIndicator extends StorageModule { + getConfig(): StorageModuleConfig { + return { + collections: { + followedList: { + version: STORAGE_VERSIONS[27].version, + fields: { + name: { type: 'string' }, + lastSync: { type: 'timestamp' }, + sharedList: { type: 'string' }, + }, + indices: [ + { + field: 'sharedList', + unique: true, + }, + ], + }, + followedListEntry: { + version: STORAGE_VERSIONS[27].version, + fields: { + creator: { type: 'string' }, + sharedList: { type: 'string' }, + entryTitle: { type: 'string' }, + normalizedPageUrl: { type: 'string' }, + hasAnnotations: { type: 'boolean' }, + createdWhen: { type: 'timestamp' }, + updatedWhen: { type: 'timestamp' }, + }, + indices: [ + { field: 'normalizedPageUrl' }, + { field: 'sharedList' }, + ], + relationships: [{ childOf: 'followedList' }], + }, + }, + operations: {}, + } + } +} From 2e81a5f854cfba00379dfeb653e1da06e8004b3d Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Mon, 31 Oct 2022 12:51:14 +0700 Subject: [PATCH 03/29] Factor in FCM sync push trigger to translation layer tests - made me realize this trigger gets sent every single time a write occurs locally, which is sub-optimal --- .../backend/translation-layer/index.test.ts | 204 +++++++++++++++++- 1 file changed, 202 insertions(+), 2 deletions(-) diff --git a/src/personal-cloud/background/backend/translation-layer/index.test.ts b/src/personal-cloud/background/backend/translation-layer/index.test.ts index 03852de606..5d0f33f548 100644 --- a/src/personal-cloud/background/backend/translation-layer/index.test.ts +++ b/src/personal-cloud/background/backend/translation-layer/index.test.ts @@ -36,6 +36,7 @@ import { initSqlUsage, InitSqlUsageParams, } from '@worldbrain/memex-common/lib/personal-cloud/backend/translation-layer/utils' +import type { MockPushMessagingService } from 'src/tests/push-messaging' // This exists due to inconsistencies between Firebase and Dexie when dealing with optional fields // - FB requires them to be `null` and excludes them from query results @@ -337,6 +338,24 @@ async function setup(options?: { runReadwiseTrigger?: boolean }) { options, ) }, + testSyncPushTrigger: (opts: { + wasTriggered: boolean + pushMessagingService?: MockPushMessagingService + }) => { + const pushMessagingService = + opts.pushMessagingService ?? setups[0].pushMessagingService + expect(pushMessagingService.sentMessages[0]).toEqual( + opts.wasTriggered + ? { + type: 'to-user', + userId: TEST_USER.email, + payload: { + type: 'downloadClientUpdates', + }, + } + : undefined, + ) + }, testDownload: async ( expected: PersonalCloudUpdateBatch, downloadOptions?: { @@ -403,7 +422,8 @@ async function setup(options?: { runReadwiseTrigger?: boolean }) { describe('Personal cloud translation layer', () => { describe(`from local schema version 26`, () => { it('should not download updates uploaded from the same device', async () => { - const { setups, testDownload } = await setup() + const { setups, testDownload, testSyncPushTrigger } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].backgroundModules.personalCloud.waitForSync() @@ -415,14 +435,18 @@ describe('Personal cloud translation layer', () => { const { setups, serverIdCapturer, - serverStorageManager, getPersonalWhere, personalDataChanges, personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + + testSyncPushTrigger({ wasTriggered: false }) + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) + await setups[0].backgroundModules.personalCloud.waitForSync() const remoteData = serverIdCapturer.mergeIds(REMOTE_TEST_DATA_V24) @@ -454,6 +478,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'pages', object: LOCAL_TEST_DATA_V24.pages.first }, { type: PersonalCloudUpdateType.Overwrite, collection: 'pages', object: LOCAL_TEST_DATA_V24.pages.second }, ]) + testSyncPushTrigger({ wasTriggered: true }) }) it('should update pages', async () => { @@ -466,7 +491,10 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager.collection('pages').updateObjects( { @@ -513,6 +541,7 @@ describe('Personal cloud translation layer', () => { } }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should delete pages', async () => { @@ -525,7 +554,10 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager.collection('pages').deleteObjects({ url: LOCAL_TEST_DATA_V24.pages.first.url, @@ -560,6 +592,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Delete, collection: 'pages', where: { url: LOCAL_TEST_DATA_V24.pages.first.url } }, ], { skip: 1 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create locators', async () => { @@ -572,8 +605,10 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) // Note we still want to insert the non-PDF pages here to test the different locators behavior await insertTestPages(setups[0].storageManager) await setups[0].storageManager @@ -636,6 +671,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'locators', object: LOCAL_TEST_DATA_V24.locators.fourth_a }, { type: PersonalCloudUpdateType.Overwrite, collection: 'locators', object: LOCAL_TEST_DATA_V24.locators.fourth_b }, ]) + testSyncPushTrigger({ wasTriggered: true }) }) it('should delete locators', async () => { @@ -648,8 +684,10 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('pages') @@ -711,6 +749,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Delete, collection: 'pages', where: { url: LOCAL_TEST_DATA_V24.pages.third.url } }, { type: PersonalCloudUpdateType.Delete, collection: 'locators', where: { id: LOCAL_TEST_DATA_V24.locators.third.id } }, ]) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create bookmarks', async () => { @@ -723,7 +762,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('bookmarks') @@ -758,6 +799,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Overwrite, collection: 'bookmarks', object: LOCAL_TEST_DATA_V24.bookmarks.first }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should delete bookmarks', async () => { @@ -770,7 +812,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('bookmarks') @@ -810,6 +854,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Delete, collection: 'bookmarks', where: changeInfo }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create visits', async () => { @@ -822,7 +867,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('visits') @@ -861,6 +908,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Overwrite, collection: 'visits', object: LOCAL_TEST_DATA_V24.visits.first }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should update visits', async () => { @@ -873,10 +921,12 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() const updatedDuration = LOCAL_TEST_DATA_V24.visits.first.duration * 2 + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('visits') @@ -931,6 +981,7 @@ describe('Personal cloud translation layer', () => { } }, ], { skip: 3 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should delete visits', async () => { @@ -943,8 +994,10 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('visits') @@ -1013,6 +1066,7 @@ describe('Personal cloud translation layer', () => { ], { skip: 2 }, ) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create annotations', async () => { @@ -1025,7 +1079,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('annotations') @@ -1070,6 +1126,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'annotations', object: LOCAL_TEST_DATA_V24.annotations.first }, { type: PersonalCloudUpdateType.Overwrite, collection: 'annotations', object: LOCAL_TEST_DATA_V24.annotations.second }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should update annotation notes', async () => { @@ -1082,7 +1139,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('annotations') @@ -1139,6 +1198,7 @@ describe('Personal cloud translation layer', () => { ], { skip: 3 }, ) + testSyncPushTrigger({ wasTriggered: true }) }) it('should delete annotations', async () => { @@ -1151,7 +1211,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('annotations') @@ -1200,6 +1262,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Delete, collection: 'annotations', where: { url: LOCAL_TEST_DATA_V24.annotations.first.url } }, { type: PersonalCloudUpdateType.Delete, collection: 'annotations', where: { url: LOCAL_TEST_DATA_V24.annotations.second.url } }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create annotation privacy levels', async () => { @@ -1212,7 +1275,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('annotations') @@ -1293,6 +1358,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'annotationPrivacyLevels', object: LOCAL_TEST_DATA_V24.annotationPrivacyLevels.first }, { type: PersonalCloudUpdateType.Overwrite, collection: 'annotationPrivacyLevels', object: LOCAL_TEST_DATA_V24.annotationPrivacyLevels.second }, ], { skip: 4 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should update annotation privacy levels', async () => { @@ -1305,7 +1371,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('annotations') @@ -1380,6 +1448,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'annotationPrivacyLevels', object: { ...LOCAL_TEST_DATA_V24.annotationPrivacyLevels.first, privacyLevel: AnnotationPrivacyLevels.SHARED_PROTECTED } }, { type: PersonalCloudUpdateType.Overwrite, collection: 'annotationPrivacyLevels', object: { ...LOCAL_TEST_DATA_V24.annotationPrivacyLevels.first, privacyLevel: AnnotationPrivacyLevels.SHARED_PROTECTED } }, ], { skip: 3 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should update annotation privacy levels, re-sharing on update to shared privacy level', async () => { @@ -1392,7 +1461,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('annotations') @@ -1482,6 +1553,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'annotationPrivacyLevels', object: { ...LOCAL_TEST_DATA_V24.annotationPrivacyLevels.first, privacyLevel: AnnotationPrivacyLevels.SHARED } }, { type: PersonalCloudUpdateType.Overwrite, collection: 'annotationPrivacyLevels', object: { ...LOCAL_TEST_DATA_V24.annotationPrivacyLevels.first, privacyLevel: AnnotationPrivacyLevels.SHARED } }, ], { skip: 3 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should delete annotation privacy levels', async () => { @@ -1494,7 +1566,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('annotations') @@ -1555,6 +1629,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Delete, collection: 'annotationPrivacyLevels', where: changeInfo }, ], { skip: 5 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create custom lists', async () => { @@ -1567,6 +1642,7 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('customLists') @@ -1601,6 +1677,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'customLists', object: LOCAL_TEST_DATA_V24.customLists.first }, { type: PersonalCloudUpdateType.Overwrite, collection: 'customLists', object: LOCAL_TEST_DATA_V24.customLists.second }, ], { skip: 0 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should update custom lists', async () => { @@ -1613,6 +1690,7 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('customLists') @@ -1664,6 +1742,7 @@ describe('Personal cloud translation layer', () => { ], { skip: 2 }, ) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create custom list descriptions', async () => { @@ -1676,6 +1755,7 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('customLists') @@ -1725,6 +1805,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'customListDescriptions', object: LOCAL_TEST_DATA_V24.customListDescriptions.first }, { type: PersonalCloudUpdateType.Overwrite, collection: 'customListDescriptions', object: LOCAL_TEST_DATA_V24.customListDescriptions.second }, ], { skip: 0 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create custom list descriptions for a shared list, updating the description field of the server-side shared list record', async () => { @@ -1735,6 +1816,7 @@ describe('Personal cloud translation layer', () => { testDownload, getDatabaseContents, getPersonalWhere, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('customLists') @@ -1791,6 +1873,7 @@ describe('Personal cloud translation layer', () => { getPersonalWhere, personalDataChanges, testDownload, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('customLists') @@ -1847,6 +1930,7 @@ describe('Personal cloud translation layer', () => { getPersonalWhere, personalDataChanges, testDownload, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('customLists') @@ -1911,6 +1995,7 @@ describe('Personal cloud translation layer', () => { ], { skip: 4 }, ) + testSyncPushTrigger({ wasTriggered: true }) }) it('should update custom list descriptions for a shared list, updating the description field of the server-side shared list record', async () => { @@ -1921,6 +2006,7 @@ describe('Personal cloud translation layer', () => { getPersonalWhere, personalDataChanges, testDownload, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('customLists') @@ -1982,6 +2068,7 @@ describe('Personal cloud translation layer', () => { getPersonalWhere, personalDataChanges, testDownload, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('customLists') @@ -2028,6 +2115,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Delete, collection: 'customListDescriptions', where: { listId: LOCAL_TEST_DATA_V24.customListDescriptions.first.listId } }, { type: PersonalCloudUpdateType.Delete, collection: 'customListDescriptions', where: { listId: LOCAL_TEST_DATA_V24.customListDescriptions.second.listId } }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create page list entries', async () => { @@ -2040,7 +2128,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('customLists') @@ -2084,6 +2174,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'pageListEntries', object: LOCAL_TEST_DATA_V24.pageListEntries.first }, { type: PersonalCloudUpdateType.Overwrite, collection: 'pageListEntries', object: LOCAL_TEST_DATA_V24.pageListEntries.second }, ], { skip: 3 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should delete page list entries', async () => { @@ -2096,7 +2187,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('customLists') @@ -2146,6 +2239,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Delete, collection: 'pageListEntries', where: changeInfo }, ], { skip: 4 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create shared list metadata', async () => { @@ -2158,6 +2252,7 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('customLists') @@ -2193,6 +2288,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Overwrite, collection: 'sharedListMetadata', object: LOCAL_TEST_DATA_V24.sharedListMetadata.first }, ], { skip: 1 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should delete shared list metadata', async () => { @@ -2205,6 +2301,7 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('customLists') @@ -2247,6 +2344,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Delete, collection: 'sharedListMetadata', where: changeInfo }, ], { skip: 1 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create annotation list entries', async () => { @@ -2259,7 +2357,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('customLists') @@ -2296,6 +2396,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Overwrite, collection: 'annotListEntries', object: LOCAL_TEST_DATA_V24.annotationListEntries.first }, ], { skip: 4 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should delete annotation list entries', async () => { @@ -2308,7 +2409,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('customLists') @@ -2353,6 +2456,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Delete, collection: 'annotListEntries', where: changeInfo }, ], { skip: 4 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create shared annotation metadata', async () => { @@ -2365,7 +2469,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('annotations') @@ -2423,6 +2529,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'sharedAnnotationMetadata', object: LOCAL_TEST_DATA_V24.sharedAnnotationMetadata.first }, { type: PersonalCloudUpdateType.Overwrite, collection: 'sharedAnnotationMetadata', object: LOCAL_TEST_DATA_V24.sharedAnnotationMetadata.second }, ], { skip: 4 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should update shared annotation metadata', async () => { @@ -2435,7 +2542,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('annotations') @@ -2517,6 +2626,7 @@ describe('Personal cloud translation layer', () => { ], { skip: 6 }, ) + testSyncPushTrigger({ wasTriggered: true }) }) it('should delete shared annotation metadata', async () => { @@ -2529,7 +2639,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('annotations') @@ -2593,6 +2705,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Delete, collection: 'sharedAnnotationMetadata', where: changeInfo }, ], { skip: 5 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create page tags', async () => { @@ -2605,7 +2718,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('tags') @@ -2646,6 +2761,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Overwrite, collection: 'tags', object: LOCAL_TEST_DATA_V24.tags.firstPageTag }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should connect existing page tags', async () => { @@ -2658,7 +2774,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('tags') @@ -2715,6 +2833,7 @@ describe('Personal cloud translation layer', () => { ], { skip: 2 }, ) + testSyncPushTrigger({ wasTriggered: true }) }) it('should remove page tags', async () => { @@ -2727,7 +2846,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('tags') @@ -2775,6 +2896,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Delete, collection: 'tags', where: LOCAL_TEST_DATA_V24.tags.firstPageTag }, ], { skip: 3 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('final tag removal for page should remove now-orphaned personalTag', async () => { @@ -2787,7 +2909,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('tags') @@ -2833,6 +2957,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Delete, collection: 'tags', where: LOCAL_TEST_DATA_V24.tags.firstPageTag }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should add annotation tags', async () => { @@ -2845,7 +2970,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('annotations') @@ -2903,6 +3030,7 @@ describe('Personal cloud translation layer', () => { object: LOCAL_TEST_DATA_V24.tags.firstAnnotationTag }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should connect existing annotation tags', async () => { @@ -2915,7 +3043,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('annotations') @@ -2985,6 +3115,7 @@ describe('Personal cloud translation layer', () => { ], { skip: 4 }, ) + testSyncPushTrigger({ wasTriggered: true }) }) it('should remove annotation tags', async () => { @@ -2997,7 +3128,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('annotations') @@ -3056,6 +3189,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Delete, collection: 'tags', where: LOCAL_TEST_DATA_V24.tags.firstAnnotationTag }, ], { skip: 5 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('final tag removal for annotation should remove now-orphaned personalTag', async () => { @@ -3068,7 +3202,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await setups[0].storageManager .collection('annotations') @@ -3122,6 +3258,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Delete, collection: 'tags', where: LOCAL_TEST_DATA_V24.tags.firstAnnotationTag }, ], { skip: 3 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create text export template', async () => { @@ -3134,6 +3271,7 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('templates') @@ -3167,6 +3305,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'templates', object: LOCAL_TEST_DATA_V24.templates.first }, { type: PersonalCloudUpdateType.Overwrite, collection: 'templates', object: LOCAL_TEST_DATA_V24.templates.second }, ], { skip: 0 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should update text export template', async () => { @@ -3179,6 +3318,7 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('templates') @@ -3250,6 +3390,7 @@ describe('Personal cloud translation layer', () => { ], { skip: 2 }, ) + testSyncPushTrigger({ wasTriggered: true }) }) it('should delete text export template', async () => { @@ -3262,6 +3403,7 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('templates') @@ -3299,6 +3441,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Delete, collection: 'templates', where: { id: LOCAL_TEST_DATA_V24.templates.first.id } }, { type: PersonalCloudUpdateType.Delete, collection: 'templates', where: { id: LOCAL_TEST_DATA_V24.templates.second.id } }, ], { skip: 0 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create Memex extension settings', async () => { @@ -3311,6 +3454,7 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('settings') @@ -3350,6 +3494,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'settings', object: LOCAL_TEST_DATA_V24.settings.second }, { type: PersonalCloudUpdateType.Overwrite, collection: 'settings', object: LOCAL_TEST_DATA_V24.settings.third }, ], { skip: 0 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should update Memex extension settings', async () => { @@ -3362,6 +3507,7 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('settings') @@ -3413,6 +3559,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'settings', object: LOCAL_TEST_DATA_V24.settings.third }, { type: PersonalCloudUpdateType.Overwrite, collection: 'settings', object: { ...LOCAL_TEST_DATA_V24.settings.first, value: updatedValue } }, ], { skip: 0 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should delete Memex extension settings', async () => { @@ -3425,6 +3572,7 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() await setups[0].storageManager .collection('settings') @@ -3477,6 +3625,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Delete, collection: 'settings', where: { key: LOCAL_TEST_DATA_V24.settings.first.key } }, { type: PersonalCloudUpdateType.Delete, collection: 'settings', where: { key: LOCAL_TEST_DATA_V24.settings.second.key } }, ], { skip: 0 }) + testSyncPushTrigger({ wasTriggered: true }) }) describe(`translation layer twitter integration`, () => { @@ -3487,6 +3636,7 @@ describe('Personal cloud translation layer', () => { getDatabaseContents, personalDataChanges, getPersonalWhere, + testSyncPushTrigger, } = await setup() await setups[0].storageManager @@ -3533,6 +3683,7 @@ describe('Personal cloud translation layer', () => { getDatabaseContents, personalDataChanges, getPersonalWhere, + testSyncPushTrigger, } = await setup() const testTitleA = 'X on Twitter: "cool stuff"' @@ -3586,6 +3737,7 @@ describe('Personal cloud translation layer', () => { getDatabaseContents, personalDataChanges, getPersonalWhere, + testSyncPushTrigger, } = await setup() const url = 'twitter.com/zzzzz' @@ -3641,7 +3793,9 @@ describe('Personal cloud translation layer', () => { personalDataChanges, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await insertReadwiseAPIKey(serverStorageManager, TEST_USER.id) await setups[0].storageManager @@ -3690,6 +3844,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'annotations', object: LOCAL_TEST_DATA_V24.annotations.first }, { type: PersonalCloudUpdateType.Overwrite, collection: 'annotations', object: LOCAL_TEST_DATA_V24.annotations.second }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should update annotation notes, triggering readwise action create', async () => { @@ -3701,7 +3856,9 @@ describe('Personal cloud translation layer', () => { personalDataChanges, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await insertReadwiseAPIKey(serverStorageManager, TEST_USER.id) await setups[0].storageManager @@ -3762,6 +3919,7 @@ describe('Personal cloud translation layer', () => { ], { skip: 3 }, ) + testSyncPushTrigger({ wasTriggered: true }) }) it('should add annotation tags, triggering readwise action create', async () => { @@ -3773,7 +3931,9 @@ describe('Personal cloud translation layer', () => { personalDataChanges, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await insertReadwiseAPIKey(serverStorageManager, TEST_USER.id) await setups[0].storageManager @@ -3836,6 +3996,7 @@ describe('Personal cloud translation layer', () => { object: LOCAL_TEST_DATA_V24.tags.firstAnnotationTag }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should remove annotation tags, triggering readwise action create', async () => { @@ -3847,7 +4008,9 @@ describe('Personal cloud translation layer', () => { personalDataChanges, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await insertReadwiseAPIKey(serverStorageManager, TEST_USER.id) await setups[0].storageManager @@ -3918,6 +4081,7 @@ describe('Personal cloud translation layer', () => { await testDownload([ { type: PersonalCloudUpdateType.Delete, collection: 'tags', where: LOCAL_TEST_DATA_V24.tags.firstAnnotationTag }, ], { skip: 5 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should add annotation spaces, triggering readwise action create', async () => { @@ -3929,7 +4093,9 @@ describe('Personal cloud translation layer', () => { personalDataChanges, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await insertReadwiseAPIKey(serverStorageManager, TEST_USER.id) await setups[0].storageManager @@ -4000,6 +4166,7 @@ describe('Personal cloud translation layer', () => { object: LOCAL_TEST_DATA_V24.annotationListEntries.first }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should remove annotation spaces, triggering readwise action create', async () => { @@ -4011,7 +4178,9 @@ describe('Personal cloud translation layer', () => { personalDataChanges, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await insertReadwiseAPIKey(serverStorageManager, TEST_USER.id) await setups[0].storageManager @@ -4095,6 +4264,7 @@ describe('Personal cloud translation layer', () => { } }, ], { skip: 6 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should create annotations, triggering readwise highlight upload', async () => { @@ -4102,9 +4272,11 @@ describe('Personal cloud translation layer', () => { setups, serverStorageManager, testFetches, + testSyncPushTrigger, } = await setup({ runReadwiseTrigger: true, }) + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await insertReadwiseAPIKey(serverStorageManager, TEST_USER.id) await setups[0].storageManager @@ -4150,9 +4322,11 @@ describe('Personal cloud translation layer', () => { setups, serverStorageManager, testFetches, + testSyncPushTrigger, } = await setup({ runReadwiseTrigger: true, }) + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await insertReadwiseAPIKey(serverStorageManager, TEST_USER.id) await setups[0].storageManager @@ -4196,9 +4370,11 @@ describe('Personal cloud translation layer', () => { setups, serverStorageManager, testFetches, + testSyncPushTrigger, } = await setup({ runReadwiseTrigger: true, }) + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await insertReadwiseAPIKey(serverStorageManager, TEST_USER.id) await setups[0].storageManager @@ -4246,9 +4422,11 @@ describe('Personal cloud translation layer', () => { setups, serverStorageManager, testFetches, + testSyncPushTrigger, } = await setup({ runReadwiseTrigger: true, }) + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await insertReadwiseAPIKey(serverStorageManager, TEST_USER.id) await setups[0].storageManager @@ -4327,9 +4505,11 @@ describe('Personal cloud translation layer', () => { setups, serverStorageManager, testFetches, + testSyncPushTrigger, } = await setup({ runReadwiseTrigger: true, }) + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await insertReadwiseAPIKey(serverStorageManager, TEST_USER.id) await setups[0].storageManager @@ -4382,9 +4562,11 @@ describe('Personal cloud translation layer', () => { setups, serverStorageManager, testFetches, + testSyncPushTrigger, } = await setup({ runReadwiseTrigger: true, }) + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await insertReadwiseAPIKey(serverStorageManager, TEST_USER.id) await setups[0].storageManager @@ -4470,9 +4652,11 @@ describe('Personal cloud translation layer', () => { setups, serverStorageManager, testFetches, + testSyncPushTrigger, } = await setup({ runReadwiseTrigger: true, }) + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) await insertReadwiseAPIKey(serverStorageManager, TEST_USER.id) const testTagWithSpaces = 'test tag spaces' @@ -4556,6 +4740,7 @@ describe('Personal cloud translation layer', () => { setups, serverStorageManager, testFetches, + testSyncPushTrigger, } = await setup({ runReadwiseTrigger: true, }) @@ -4609,7 +4794,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) // Create + share list await setups[0].storageManager @@ -4727,6 +4914,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'sharedAnnotationMetadata', object: LOCAL_TEST_DATA_V24.sharedAnnotationMetadata.third }, { type: PersonalCloudUpdateType.Overwrite, collection: 'annotationPrivacyLevels', object: LOCAL_TEST_DATA_V24.annotationPrivacyLevels.third }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should index a remote PDF page, create a shared annotation, create and share a new list, then add that page to the list', async () => { @@ -4737,7 +4925,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) // Create PDF page await setups[0].storageManager @@ -4855,6 +5045,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'sharedListMetadata', object: LOCAL_TEST_DATA_V24.sharedListMetadata.first }, { type: PersonalCloudUpdateType.Overwrite, collection: 'pageListEntries', object: LOCAL_TEST_DATA_V24.pageListEntries.third }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should index a local PDF page, create a shared annotation, create and share a new list, then add that page to the list', async () => { @@ -4867,7 +5058,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) // Create PDF page await setups[0].storageManager @@ -4992,6 +5185,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'sharedListMetadata', object: LOCAL_TEST_DATA_V24.sharedListMetadata.first }, { type: PersonalCloudUpdateType.Overwrite, collection: 'pageListEntries', object: LOCAL_TEST_DATA_V24.pageListEntries.fourth }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should index a PDF page, create a shared annotation, create a new list, add that page to the list, then share the list', async () => { @@ -5004,7 +5198,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) // Create PDF page await setups[0].storageManager @@ -5130,6 +5326,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'pageListEntries', object: LOCAL_TEST_DATA_V24.pageListEntries.fourth }, { type: PersonalCloudUpdateType.Overwrite, collection: 'sharedListMetadata', object: LOCAL_TEST_DATA_V24.sharedListMetadata.first }, ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) }) it('should index a page, create a shared list, create a private annotation, add page to list, then share the annotation', async () => { @@ -5142,7 +5339,9 @@ describe('Personal cloud translation layer', () => { personalBlockStats, getDatabaseContents, testDownload, + testSyncPushTrigger, } = await setup() + testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) // Create + share list await setups[0].storageManager @@ -5259,6 +5458,7 @@ describe('Personal cloud translation layer', () => { { type: PersonalCloudUpdateType.Overwrite, collection: 'sharedAnnotationMetadata', object: LOCAL_TEST_DATA_V24.sharedAnnotationMetadata.first }, { type: PersonalCloudUpdateType.Overwrite, collection: 'annotationPrivacyLevels', object: LOCAL_TEST_DATA_V24.annotationPrivacyLevels.first }, ], { skip: 0 }) + testSyncPushTrigger({ wasTriggered: true }) }) }) }) From 6ad0587d99fa1e0dd9a135aa7fd2aca20971cdaa Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Mon, 31 Oct 2022 14:12:16 +0700 Subject: [PATCH 04/29] Replace all usages of sendFcmSyncMessage with PushMessagingService - all the twitter integration tests have regressed. Need to fix these --- external/@worldbrain/memex-common | 2 +- src/background-mv3.ts | 10 ++++--- .../twitter-integration.test.ts | 27 +++++++++---------- src/tests/background-integration-tests.ts | 10 +++---- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/external/@worldbrain/memex-common b/external/@worldbrain/memex-common index 5ae1054b6e..1029cd0bc9 160000 --- a/external/@worldbrain/memex-common +++ b/external/@worldbrain/memex-common @@ -1 +1 @@ -Subproject commit 5ae1054b6ece82d01b1d22a08bb4254c6476bc67 +Subproject commit 1029cd0bc9f277bf1fbee689553cdace1555aa5c diff --git a/src/background-mv3.ts b/src/background-mv3.ts index a5815f204c..29af281885 100644 --- a/src/background-mv3.ts +++ b/src/background-mv3.ts @@ -2,7 +2,6 @@ import browser from 'webextension-polyfill' import XMLHttpRequest from 'xhr-shim' import { getToken } from 'firebase/messaging' import { onBackgroundMessage, getMessaging } from 'firebase/messaging/sw' -import { FCM_SYNC_TRIGGER_MSG } from '@worldbrain/memex-common/lib/personal-cloud/backend/constants' import { setupRpcConnection, setupRemoteFunctionsImplementations, @@ -34,6 +33,7 @@ import type { DexieStorageBackend, IndexedDbImplementation, } from '@worldbrain/storex-backend-dexie' +import type { PushMessagePayload } from '@worldbrain/memex-common/lib/push-messaging/types' // This is here so the correct Service Worker `self` context is available. Maybe there's a better way to set this via tsconfig. declare var self: ServiceWorkerGlobalScope & { @@ -130,8 +130,12 @@ async function main() { ;(storageManager.backend as DexieStorageBackend)._onRegistryInitialized() // Set up incoming FCM handling logic (same thing as SW `push` event) - onBackgroundMessage(getMessaging(), (payload) => { - if (payload.data?.type === FCM_SYNC_TRIGGER_MSG) { + onBackgroundMessage(getMessaging(), (message) => { + const payload = message.data as PushMessagePayload + if (payload == null) { + return + } + if (payload.type === 'downloadClientUpdates') { backgroundModules.personalCloud.triggerSyncContinuation() } }) diff --git a/src/personal-cloud/background/backend/translation-layer/twitter-integration.test.ts b/src/personal-cloud/background/backend/translation-layer/twitter-integration.test.ts index 0c774cd67c..223501f74b 100644 --- a/src/personal-cloud/background/backend/translation-layer/twitter-integration.test.ts +++ b/src/personal-cloud/background/backend/translation-layer/twitter-integration.test.ts @@ -11,7 +11,7 @@ import { import type { TweetData } from '@worldbrain/memex-common/lib/twitter-integration/api/types' import { tweetDataToTitle } from '@worldbrain/memex-common/lib/twitter-integration/utils' import { createUploadStorageUtils } from '@worldbrain/memex-common/lib/personal-cloud/backend/translation-layer/storage-utils' -import { FCM_SYNC_TRIGGER_MSG } from '@worldbrain/memex-common/lib/personal-cloud/backend/constants' +import { TEST_USER } from '@worldbrain/memex-common/lib/authentication/dev' const TEST_TWEET_A: TweetData = { name: 'Test User', @@ -54,17 +54,9 @@ async function setupTest(opts: { describe('Translation-layer Twitter integration tests', () => { it('given a stored twitter action, should device tweet ID from stored locator data, download tweet data from Twitter API, derive title, then update associated content metadata - and send off an FCM message to tell clients to sync', async () => { const fcmTokens = ['test-device-1', 'test-device-2'] - let sentToDevices: { tokens: string[]; message: any } - jest.mock('firebase-admin', () => ({ - messaging: () => ({ - sendToDevice: async (tokens: string[], message: any) => { - sentToDevices = { tokens, message } - }, - }), - })) let capturedException: Error | null = null - const { processor, storage, userId } = await setupTest({ + const { processor, storage, userId, context } = await setupTest({ testTweetData: TEST_TWEET_A, captureException: async (e) => { capturedException = e @@ -131,7 +123,7 @@ describe('Translation-layer Twitter integration tests', () => { .collection('personalTwitterAction') .createObject(twitterAction) - expect(sentToDevices).toEqual({ tokens: [], message: {} }) + expect(context.pushMessagingService.sentMessages).toEqual([]) expect(capturedException).toBeNull() expect( await storage.manager @@ -151,10 +143,15 @@ describe('Translation-layer Twitter integration tests', () => { await processor.processTwitterAction(twitterAction) - expect(sentToDevices).toEqual({ - tokens: fcmTokens, - message: { data: { type: FCM_SYNC_TRIGGER_MSG } }, - }) + expect(context.pushMessagingService.sentMessages).toEqual([ + { + type: 'to-user', + userId: TEST_USER.email, + payload: { + type: 'downloadClientUpdates', + }, + }, + ]) expect(capturedException).toBeNull() expect( await storage.manager diff --git a/src/tests/background-integration-tests.ts b/src/tests/background-integration-tests.ts index 5a21f4e40d..d40a27de69 100644 --- a/src/tests/background-integration-tests.ts +++ b/src/tests/background-integration-tests.ts @@ -43,10 +43,8 @@ import { clearRemotelyCallableFunctions } from 'src/util/webextensionRPC' import { Services } from 'src/services/types' import { PersonalDeviceType } from '@worldbrain/memex-common/lib/personal-cloud/storage/types' import { JobScheduler } from 'src/job-scheduler/background/job-scheduler' -import { MockAlarmsApi } from 'src/job-scheduler/background/job-scheduler.test' import { createAuthServices } from 'src/services/local-services' import { MockPushMessagingService } from './push-messaging' -import type { PushMessagingServiceInterface } from '@worldbrain/memex-common/lib/push-messaging/types' fetchMock.restore() export interface BackgroundIntegrationTestSetupOpts { @@ -60,7 +58,7 @@ export interface BackgroundIntegrationTestSetupOpts { startWithSyncDisabled?: boolean useDownloadTranslationLayer?: boolean services?: Services - pushMessagingService?: PushMessagingServiceInterface + pushMessagingService?: MockPushMessagingService } export async function setupBackgroundIntegrationTest( @@ -176,6 +174,8 @@ export async function setupBackgroundIntegrationTest( ) } + const pushMessagingService = + options?.pushMessagingService ?? new MockPushMessagingService() let nextServerId = 1337 const userMessages = new MemoryUserMessageService() const backgroundModules = createBackgroundModules({ @@ -222,6 +222,7 @@ export async function setupBackgroundIntegrationTest( storageModules: serverStorage.modules, getCurrentUserId: async () => (await auth.authService.getCurrentUser()).id, + services: { pushMessaging: pushMessagingService }, }), generateServerId: () => nextServerId++, }) @@ -272,9 +273,8 @@ export async function setupBackgroundIntegrationTest( setStorex(storageManager) return { - pushMessagingService: - options.pushMessagingService ?? new MockPushMessagingService(), storageManager, + pushMessagingService, persistentStorageManager, backgroundModules, browserLocalStorage, From f5259641e512aba1cd61ee2e0580eb097ae79eca Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Tue, 1 Nov 2022 13:04:11 +0700 Subject: [PATCH 05/29] Write logic for all but followList FCM message - the followList message requires the followed list sync procedure, which will be re-used in other parts --- src/background-mv3.ts | 13 +- src/background-script/setup.ts | 6 + .../background/index.ts | 32 ++ .../background/storage.ts | 131 +++++++- .../background/types.ts | 16 + src/push-messaging/background/index.test.ts | 317 ++++++++++++++++++ src/push-messaging/background/index.ts | 60 ++++ 7 files changed, 562 insertions(+), 13 deletions(-) create mode 100644 src/page-activity-indicator/background/index.ts create mode 100644 src/page-activity-indicator/background/types.ts create mode 100644 src/push-messaging/background/index.test.ts create mode 100644 src/push-messaging/background/index.ts diff --git a/src/background-mv3.ts b/src/background-mv3.ts index 29af281885..56f7109c0f 100644 --- a/src/background-mv3.ts +++ b/src/background-mv3.ts @@ -34,6 +34,7 @@ import type { IndexedDbImplementation, } from '@worldbrain/storex-backend-dexie' import type { PushMessagePayload } from '@worldbrain/memex-common/lib/push-messaging/types' +import PushMessagingClient from './push-messaging/background' // This is here so the correct Service Worker `self` context is available. Maybe there's a better way to set this via tsconfig. declare var self: ServiceWorkerGlobalScope & { @@ -129,15 +130,17 @@ async function main() { // Doing this as all event listeners need to be set up synchronously, before any async logic happens. AND to avoid needing to update storex yet. ;(storageManager.backend as DexieStorageBackend)._onRegistryInitialized() - // Set up incoming FCM handling logic (same thing as SW `push` event) - onBackgroundMessage(getMessaging(), (message) => { + // Set up incoming FCM handling logic (`onBackgroundMessage` wraps the SW `push` event) + const pushMessagingClient = new PushMessagingClient({ + bgModules: backgroundModules, + }) + onBackgroundMessage(getMessaging(), async (message) => { const payload = message.data as PushMessagePayload if (payload == null) { return } - if (payload.type === 'downloadClientUpdates') { - backgroundModules.personalCloud.triggerSyncContinuation() - } + + await pushMessagingClient.handleIncomingMessage(payload) }) await setupBackgroundModules(backgroundModules, storageManager) diff --git a/src/background-script/setup.ts b/src/background-script/setup.ts index d82e632d8d..fd43bd72dc 100644 --- a/src/background-script/setup.ts +++ b/src/background-script/setup.ts @@ -98,6 +98,7 @@ import type { LocalExtensionSettings } from './types' import { normalizeUrl } from '@worldbrain/memex-url-utils/lib/normalize/utils' import { createSyncSettingsStore } from 'src/sync-settings/util' import DeprecatedStorageModules from './deprecated-storage-modules' +import { PageActivityIndicatorBackground } from 'src/page-activity-indicator/background' export interface BackgroundModules { auth: AuthBackground @@ -106,6 +107,7 @@ export interface BackgroundModules { social: SocialBackground pdfBg: PDFBackground // connectivityChecker: ConnectivityCheckerBackground + pageActivityIndicator: PageActivityIndicatorBackground activityIndicator: ActivityIndicatorBackground directLinking: DirectLinkingBackground pages: PageIndexingBackground @@ -602,6 +604,9 @@ export function createBackgroundModules(options: { search, eventLog: new EventLogBackground({ storageManager }), activityIndicator, + pageActivityIndicator: new PageActivityIndicatorBackground({ + storageManager, + }), customLists, tags, bookmarks, @@ -767,6 +772,7 @@ export function getBackgroundStorageModules( __deprecatedModules: DeprecatedStorageModules, ): { [moduleName: string]: StorageModule } { return { + pageActivityIndicator: backgroundModules.pageActivityIndicator.storage, pageFetchBacklog: __deprecatedModules.pageFetchBacklogStorage, annotations: backgroundModules.directLinking.annotationStorage, readwiseAction: __deprecatedModules.readwiseActionQueueStorage, diff --git a/src/page-activity-indicator/background/index.ts b/src/page-activity-indicator/background/index.ts new file mode 100644 index 0000000000..f64b05e79d --- /dev/null +++ b/src/page-activity-indicator/background/index.ts @@ -0,0 +1,32 @@ +import type Storex from '@worldbrain/storex' +import PageActivityIndicatorStorage from './storage' + +export interface PageActivityIndicatorDependencies { + storageManager: Storex +} + +export class PageActivityIndicatorBackground { + storage: PageActivityIndicatorStorage + + constructor(private deps: PageActivityIndicatorDependencies) { + this.storage = new PageActivityIndicatorStorage({ + storageManager: deps.storageManager, + }) + } + + createFollowedList: PageActivityIndicatorStorage['createFollowedList'] = ( + data, + ) => this.storage.createFollowedList(data) + createFollowedListEntry: PageActivityIndicatorStorage['createFollowedListEntry'] = ( + data, + ) => this.storage.createFollowedListEntry(data) + updateFollowedListEntryHasAnnotations: PageActivityIndicatorStorage['updateFollowedListEntryHasAnnotations'] = ( + data, + ) => this.storage.updateFollowedListEntryHasAnnotations(data) + deleteFollowedListEntry: PageActivityIndicatorStorage['deleteFollowedListEntry'] = ( + data, + ) => this.storage.deleteFollowedListEntry(data) + deleteFollowedListAndAllEntries: PageActivityIndicatorStorage['deleteFollowedListAndAllEntries'] = ( + data, + ) => this.storage.deleteFollowedListAndAllEntries(data) +} diff --git a/src/page-activity-indicator/background/storage.ts b/src/page-activity-indicator/background/storage.ts index 4ad2c5e340..19f8b1947f 100644 --- a/src/page-activity-indicator/background/storage.ts +++ b/src/page-activity-indicator/background/storage.ts @@ -1,11 +1,12 @@ -import Storex from '@worldbrain/storex' +import { AutoPk } from '@worldbrain/memex-common/lib/storage/types' import { StorageModule, StorageModuleConfig, } from '@worldbrain/storex-pattern-modules' import { STORAGE_VERSIONS } from 'src/storage/constants' +import { FollowedList, FollowedListEntry } from './types' -export default class PageActivityIndicator extends StorageModule { +export default class PageActivityIndicatorStorage extends StorageModule { getConfig(): StorageModuleConfig { return { collections: { @@ -13,13 +14,14 @@ export default class PageActivityIndicator extends StorageModule { version: STORAGE_VERSIONS[27].version, fields: { name: { type: 'string' }, - lastSync: { type: 'timestamp' }, + creator: { type: 'string' }, sharedList: { type: 'string' }, + lastSync: { type: 'timestamp', optional: true }, }, indices: [ { + pk: true, field: 'sharedList', - unique: true, }, ], }, @@ -27,8 +29,7 @@ export default class PageActivityIndicator extends StorageModule { version: STORAGE_VERSIONS[27].version, fields: { creator: { type: 'string' }, - sharedList: { type: 'string' }, - entryTitle: { type: 'string' }, + entryTitle: { type: 'text' }, normalizedPageUrl: { type: 'string' }, hasAnnotations: { type: 'boolean' }, createdWhen: { type: 'timestamp' }, @@ -36,12 +37,126 @@ export default class PageActivityIndicator extends StorageModule { }, indices: [ { field: 'normalizedPageUrl' }, - { field: 'sharedList' }, + { field: 'followedList' }, ], relationships: [{ childOf: 'followedList' }], }, }, - operations: {}, + operations: { + createFollowedList: { + collection: 'followedList', + operation: 'createObject', + }, + createFollowedListEntry: { + collection: 'followedListEntry', + operation: 'createObject', + }, + updateFollowedListEntryHasAnnotations: { + collection: 'followedListEntry', + operation: 'updateObjects', + args: [ + { + followedList: '$followedList:string', + normalizedPageUrl: '$normalizedPageUrl:string', + }, + { + hasAnnotations: '$hasAnnotations:boolean', + updatedWhen: '$updatedWhen:number', + }, + ], + }, + deleteFollowedList: { + collection: 'followedList', + operation: 'deleteObject', + args: { + sharedList: '$sharedList:string', + }, + }, + deleteFollowedListEntry: { + collection: 'followedListEntry', + operation: 'deleteObjects', + args: { + followedList: '$followedList:string', + normalizedPageUrl: '$normalizedPageUrl:string', + }, + }, + deleteFollowedListEntries: { + collection: 'followedListEntry', + operation: 'deleteObjects', + args: { + followedList: '$followedList:string', + }, + }, + }, } } + + async createFollowedList(data: FollowedList): Promise { + const { object } = await this.operation('createFollowedList', { + name: data.name, + creator: data.creator, + lastSync: data.lastSync, + sharedList: data.sharedList, + }) + return object.id + } + + async createFollowedListEntry( + data: Omit< + FollowedListEntry, + 'updatedWhen' | 'createdWhen' | 'hasAnnotations' + > & + Partial< + Pick< + FollowedListEntry, + 'updatedWhen' | 'createdWhen' | 'hasAnnotations' + > + >, + ): Promise { + const { object } = await this.operation('createFollowedListEntry', { + creator: data.creator, + entryTitle: data.entryTitle, + followedList: data.followedList, + hasAnnotations: data.hasAnnotations ?? false, + normalizedPageUrl: data.normalizedPageUrl, + createdWhen: data.createdWhen ?? Date.now(), + updatedWhen: data.updatedWhen ?? Date.now(), + }) + return object.id + } + + async updateFollowedListEntryHasAnnotations( + data: Pick< + FollowedListEntry, + 'followedList' | 'normalizedPageUrl' | 'hasAnnotations' + > & + Partial>, + ): Promise { + await this.operation('updateFollowedListEntryHasAnnotations', { + followedList: data.followedList, + normalizedPageUrl: data.normalizedPageUrl, + hasAnnotations: data.hasAnnotations, + updatedWhen: data.updatedWhen ?? Date.now(), + }) + } + + async deleteFollowedListEntry( + data: Pick, + ): Promise { + await this.operation('deleteFollowedListEntry', { + followedList: data.followedList, + normalizedPageUrl: data.normalizedPageUrl, + }) + } + + async deleteFollowedListAndAllEntries( + data: Pick, + ): Promise { + await this.operation('deleteFollowedList', { + sharedList: data.sharedList, + }) + await this.operation('deleteFollowedListEntries', { + followedList: data.sharedList, + }) + } } diff --git a/src/page-activity-indicator/background/types.ts b/src/page-activity-indicator/background/types.ts new file mode 100644 index 0000000000..683ad1d950 --- /dev/null +++ b/src/page-activity-indicator/background/types.ts @@ -0,0 +1,16 @@ +export interface FollowedList { + name: string + creator: string + lastSync?: number + sharedList: string +} + +export interface FollowedListEntry { + creator: string + entryTitle: string + followedList: string + hasAnnotations: boolean + normalizedPageUrl: string + createdWhen: number + updatedWhen: number +} diff --git a/src/push-messaging/background/index.test.ts b/src/push-messaging/background/index.test.ts new file mode 100644 index 0000000000..0769fe8608 --- /dev/null +++ b/src/push-messaging/background/index.test.ts @@ -0,0 +1,317 @@ +import { TEST_USER } from '@worldbrain/memex-common/lib/authentication/dev' +import { setupBackgroundIntegrationTest } from 'src/tests/background-integration-tests' +import PushMessagingClient from '.' + +const defaultListName = 'test list a' + +async function setupTest(opts: { + createDefaultList: boolean + createAssocLocalFollowedList?: boolean +}) { + const context = await setupBackgroundIntegrationTest() + const pushMessagingClient = new PushMessagingClient({ + bgModules: context.backgroundModules, + }) + + const serverStorage = await context.getServerStorage() + let defaultListId: string | null = null + if (opts.createDefaultList) { + const { object } = await serverStorage.manager + .collection('sharedList') + .createObject({ + creator: TEST_USER.id, + title: defaultListName, + createdWhen: new Date(), + updatedWhen: new Date(), + }) + defaultListId = object.id + + if (opts.createAssocLocalFollowedList) { + await context.backgroundModules.pageActivityIndicator.createFollowedList( + { + creator: TEST_USER.id, + name: defaultListName, + sharedList: defaultListId, + }, + ) + } + } + + return { pushMessagingClient, serverStorage, defaultListId, ...context } +} + +describe('Push messaging client tests', () => { + it('should trigger sync continuation upon receiving sync trigger message', async () => { + const { pushMessagingClient, backgroundModules } = await setupTest({ + createDefaultList: false, + }) + let hasSyncBeenTriggered = false + + backgroundModules.personalCloud.triggerSyncContinuation = () => { + hasSyncBeenTriggered = true + } + + expect(hasSyncBeenTriggered).toBe(false) + await pushMessagingClient.handleIncomingMessage({ + type: 'downloadClientUpdates', + }) + expect(hasSyncBeenTriggered).toBe(true) + }) + + it(`should create/delete followedListEntry records when receiving created/deleted sharedListEntry messages`, async () => { + const { + pushMessagingClient, + defaultListId, + storageManager, + } = await setupTest({ + createDefaultList: true, + createAssocLocalFollowedList: true, + }) + + const expectedEntryA = { + id: expect.any(Number), + creator: TEST_USER.id, + entryTitle: 'test title a', + followedList: defaultListId, + hasAnnotations: false, + normalizedPageUrl: 'test.com/a', + createdWhen: 1, + updatedWhen: 1, + } + const expectedEntryB = { + id: expect.any(Number), + creator: TEST_USER.id, + entryTitle: 'test title b', + followedList: defaultListId, + hasAnnotations: false, + normalizedPageUrl: 'test.com/b', + createdWhen: 2, + updatedWhen: 2, + } + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await pushMessagingClient.handleIncomingMessage( + { + type: 'createPageListEntry', + creator: TEST_USER.id, + sharedList: defaultListId, + entryTitle: expectedEntryA.entryTitle, + normalizedPageUrl: expectedEntryA.normalizedPageUrl, + }, + { now: expectedEntryA.createdWhen }, + ) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([expectedEntryA]) + + await pushMessagingClient.handleIncomingMessage( + { + type: 'createPageListEntry', + creator: TEST_USER.id, + sharedList: defaultListId, + entryTitle: expectedEntryB.entryTitle, + normalizedPageUrl: expectedEntryB.normalizedPageUrl, + }, + { now: expectedEntryB.createdWhen }, + ) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([expectedEntryA, expectedEntryB]) + + await pushMessagingClient.handleIncomingMessage({ + type: 'deletePageListEntry', + sharedList: defaultListId, + normalizedPageUrl: expectedEntryA.normalizedPageUrl, + }) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([expectedEntryB]) + + await pushMessagingClient.handleIncomingMessage({ + type: 'deletePageListEntry', + sharedList: defaultListId, + normalizedPageUrl: expectedEntryB.normalizedPageUrl, + }) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + }) + + it(`should toggle 'hasAnnotations' flag on assoc. followedListEntry when receiving first/last sharedListEntry added/removed messages`, async () => { + const { + pushMessagingClient, + storageManager, + defaultListId, + } = await setupTest({ + createDefaultList: true, + createAssocLocalFollowedList: true, + }) + + const expectedEntryA = { + id: expect.any(Number), + creator: TEST_USER.id, + entryTitle: 'test title a', + followedList: defaultListId, + hasAnnotations: false, + normalizedPageUrl: 'test.com/a', + createdWhen: 1, + updatedWhen: 1, + } + + // Do create list entry message first, to trigger the creation of a followedListEntry to work with + await pushMessagingClient.handleIncomingMessage( + { + type: 'createPageListEntry', + creator: TEST_USER.id, + sharedList: defaultListId, + entryTitle: expectedEntryA.entryTitle, + normalizedPageUrl: expectedEntryA.normalizedPageUrl, + }, + { now: expectedEntryA.createdWhen }, + ) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([expectedEntryA]) + + await pushMessagingClient.handleIncomingMessage( + { + type: 'createFirstAnnotationListEntry', + normalizedPageUrl: expectedEntryA.normalizedPageUrl, + sharedList: defaultListId, + }, + { now: 2 }, + ) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([{ ...expectedEntryA, hasAnnotations: true, updatedWhen: 2 }]) + + await pushMessagingClient.handleIncomingMessage( + { + type: 'deleteLastAnnotationListEntry', + normalizedPageUrl: expectedEntryA.normalizedPageUrl, + sharedList: defaultListId, + }, + { now: 3 }, + ) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([ + { ...expectedEntryA, hasAnnotations: false, updatedWhen: 3 }, + ]) + + await pushMessagingClient.handleIncomingMessage( + { + type: 'createFirstAnnotationListEntry', + normalizedPageUrl: expectedEntryA.normalizedPageUrl, + sharedList: defaultListId, + }, + { now: 4 }, + ) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([{ ...expectedEntryA, hasAnnotations: true, updatedWhen: 4 }]) + }) + + it(`should create/delete followedList and assoc. followedListEntry records when receiving sharedList followed/unfollowed messages`, async () => { + const { + pushMessagingClient, + storageManager, + defaultListId, + } = await setupTest({ + createDefaultList: true, + createAssocLocalFollowedList: false, + }) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await pushMessagingClient.handleIncomingMessage({ + type: 'followList', + sharedList: defaultListId, + }) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([ + { + name: defaultListName, + creator: TEST_USER.id, + sharedList: defaultListId, + lastSync: expect.any(Number), + }, + ]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([ + { + creator: TEST_USER.id, + entryTitle: 'test title a', + followedList: defaultListId, + hasAnnotations: false, + normalizedPageUrl: 'test.com/a', + createdWhen: expect.any(Number), + updatedWhen: expect.any(Number), + }, + { + creator: TEST_USER.id, + entryTitle: 'test title b', + followedList: defaultListId, + hasAnnotations: false, + normalizedPageUrl: 'test.com/b', + createdWhen: expect.any(Number), + updatedWhen: expect.any(Number), + }, + ]) + + await pushMessagingClient.handleIncomingMessage({ + type: 'unfollowList', + sharedList: defaultListId, + }) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + }) +}) diff --git a/src/push-messaging/background/index.ts b/src/push-messaging/background/index.ts new file mode 100644 index 0000000000..c21d519cf5 --- /dev/null +++ b/src/push-messaging/background/index.ts @@ -0,0 +1,60 @@ +import type { PushMessagePayload } from '@worldbrain/memex-common/lib/push-messaging/types' +import type { BackgroundModules } from 'src/background-script/setup' + +export default class PushMessagingClient { + constructor( + private deps: { + bgModules: Pick< + BackgroundModules, + 'personalCloud' | 'pageActivityIndicator' + > + }, + ) {} + + async handleIncomingMessage( + payload: PushMessagePayload, + opts?: { now?: number }, + ): Promise { + const { bgModules } = this.deps + if (payload.type === 'downloadClientUpdates') { + bgModules.personalCloud.triggerSyncContinuation() + } else if (payload.type === 'createPageListEntry') { + await bgModules.pageActivityIndicator.createFollowedListEntry({ + creator: payload.creator, + entryTitle: payload.entryTitle, + followedList: payload.sharedList, + normalizedPageUrl: payload.normalizedPageUrl, + createdWhen: opts?.now, + updatedWhen: opts?.now, + }) + } else if (payload.type === 'deletePageListEntry') { + await bgModules.pageActivityIndicator.deleteFollowedListEntry({ + normalizedPageUrl: payload.normalizedPageUrl, + followedList: payload.sharedList, + }) + } else if (payload.type === 'createFirstAnnotationListEntry') { + await bgModules.pageActivityIndicator.updateFollowedListEntryHasAnnotations( + { + normalizedPageUrl: payload.normalizedPageUrl, + followedList: payload.sharedList, + updatedWhen: opts?.now, + hasAnnotations: true, + }, + ) + } else if (payload.type === 'deleteLastAnnotationListEntry') { + await bgModules.pageActivityIndicator.updateFollowedListEntryHasAnnotations( + { + normalizedPageUrl: payload.normalizedPageUrl, + followedList: payload.sharedList, + updatedWhen: opts?.now, + hasAnnotations: false, + }, + ) + } else if (payload.type === 'followList') { + } else if (payload.type === 'unfollowList') { + await bgModules.pageActivityIndicator.deleteFollowedListAndAllEntries( + { sharedList: payload.sharedList }, + ) + } + } +} From bfe7d34000421c01297f55d2ca34988a6fa5c858 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Wed, 2 Nov 2022 14:20:09 +0700 Subject: [PATCH 06/29] Write tests to cover base functionality of followedList sync procedure - not yet covering `lastSync` field, as I haven't fully wrapped my head around it --- src/background-script/setup.ts | 14 +- .../background/index.test.data.ts | 161 ++++++++ .../background/index.test.ts | 381 ++++++++++++++++++ .../background/index.ts | 23 ++ .../background/storage.ts | 5 + .../background/types.ts | 10 +- .../background/utils.ts | 26 ++ 7 files changed, 610 insertions(+), 10 deletions(-) create mode 100644 src/page-activity-indicator/background/index.test.data.ts create mode 100644 src/page-activity-indicator/background/index.test.ts create mode 100644 src/page-activity-indicator/background/utils.ts diff --git a/src/background-script/setup.ts b/src/background-script/setup.ts index fd43bd72dc..8b98e1f213 100644 --- a/src/background-script/setup.ts +++ b/src/background-script/setup.ts @@ -99,6 +99,7 @@ import { normalizeUrl } from '@worldbrain/memex-url-utils/lib/normalize/utils' import { createSyncSettingsStore } from 'src/sync-settings/util' import DeprecatedStorageModules from './deprecated-storage-modules' import { PageActivityIndicatorBackground } from 'src/page-activity-indicator/background' +import type { AutoPk } from '@worldbrain/memex-common/lib/storage/types' export interface BackgroundModules { auth: AuthBackground @@ -336,6 +337,9 @@ export function createBackgroundModules(options: { (await options.getServerStorage()).modules.users, }) + const getCurrentUserId = async (): Promise => + (await auth.authService.getCurrentUser())?.id ?? null + const activityStreams = new ActivityStreamsBackground({ storageManager, callFirebaseFunction, @@ -344,10 +348,7 @@ export function createBackgroundModules(options: { if (!options.userMessageService) { const userMessagesService = new FirebaseUserMessageService({ firebase: getFirebase, - auth: { - getCurrentUserId: async () => - (await auth.authService.getCurrentUser())?.id, - }, + auth: { getCurrentUserId }, }) options.userMessageService = userMessagesService userMessagesService.startListening({ @@ -467,8 +468,7 @@ export function createBackgroundModules(options: { }, settingStore: personalCloudSettingStore, localExtSettingStore, - getUserId: async () => - (await auth.authService.getCurrentUser())?.id ?? null, + getUserId: getCurrentUserId, async *userIdChanges() { for await (const nextUser of authChanges(auth.authService)) { yield nextUser @@ -606,6 +606,8 @@ export function createBackgroundModules(options: { activityIndicator, pageActivityIndicator: new PageActivityIndicatorBackground({ storageManager, + getCurrentUserId, + getServerStorage, }), customLists, tags, diff --git a/src/page-activity-indicator/background/index.test.data.ts b/src/page-activity-indicator/background/index.test.data.ts new file mode 100644 index 0000000000..a3b40e9118 --- /dev/null +++ b/src/page-activity-indicator/background/index.test.data.ts @@ -0,0 +1,161 @@ +import type { + SharedAnnotationListEntry, + SharedList, + SharedListEntry, +} from '@worldbrain/memex-common/lib/content-sharing/types' +import type { UserReference } from '@worldbrain/memex-common/lib/web-interface/types/users' +import type { AutoPk } from '@worldbrain/memex-common/lib/storage/types' +import type { ActivityFollow } from '@worldbrain/memex-common/lib/activity-follows/storage/types' +import { TEST_USER } from '@worldbrain/memex-common/lib/authentication/dev' + +export const userReferenceA: UserReference = { + type: 'user-reference', + id: TEST_USER.id, +} +export const userReferenceB: UserReference = { type: 'user-reference', id: 123 } +export const userReferenceC: UserReference = { type: 'user-reference', id: 124 } + +export const users: UserReference[] = [ + userReferenceA, + userReferenceB, + userReferenceC, +] + +export const sharedLists: Array< + SharedList & { id: AutoPk; creator: AutoPk } +> = [ + { + id: 1, + creator: users[0].id, + title: 'test a', + createdWhen: 1, + updatedWhen: 1, + }, + { + id: 2, + creator: users[0].id, + title: 'test b', + createdWhen: 2, + updatedWhen: 2, + }, + { + id: 3, + creator: users[1].id, + title: 'test c', + description: 'great list', + createdWhen: 3, + updatedWhen: 3, + }, + { + id: 4, + creator: users[2].id, + title: 'test discord channel a', + createdWhen: 4, + updatedWhen: 4, + platform: 'discord', + }, +] + +export const listEntries: { + [sharedListId: number]: Array +} = { + [sharedLists[0].id]: [ + { + creator: users[0].id, + normalizedUrl: 'test.com/a', + originalUrl: 'https://test.com/a', + createdWhen: 1, + updatedWhen: 1, + }, + { + creator: users[1].id, + normalizedUrl: 'test.com/b', + originalUrl: 'https://test.com/b', + createdWhen: 1, + updatedWhen: 1, + }, + ], + [sharedLists[1].id]: [ + { + creator: users[0].id, + normalizedUrl: 'test.com/a', + originalUrl: 'https://test.com/a', + createdWhen: 1, + updatedWhen: 1, + }, + { + creator: users[0].id, + normalizedUrl: 'test.com/b', + originalUrl: 'https://test.com/b', + createdWhen: 1, + updatedWhen: 1, + }, + ], + [sharedLists[2].id]: [ + { + creator: users[0].id, + normalizedUrl: 'test.com/a', + originalUrl: 'https://test.com/a', + createdWhen: 1, + updatedWhen: 1, + }, + ], + [sharedLists[3].id]: [ + { + creator: users[0].id, + normalizedUrl: 'test.com/a', + originalUrl: 'https://test.com/a', + createdWhen: 1, + updatedWhen: 1, + }, + ], +} + +export const annotationListEntries: { + [normalizedPageUrl: string]: Array< + SharedAnnotationListEntry & { + sharedList: AutoPk + creator: AutoPk + } + > +} = { + ['test.com/a']: [ + { + creator: users[0].id, + sharedList: sharedLists[0].id, + normalizedPageUrl: 'test.com/a', + updatedWhen: 1, + createdWhen: 1, + uploadedWhen: 1, + }, + { + creator: users[1].id, + sharedList: sharedLists[0].id, + normalizedPageUrl: 'test.com/a', + updatedWhen: 1, + createdWhen: 1, + uploadedWhen: 1, + }, + ], +} + +export const activityFollows: Array = [ + { + collection: 'sharedList', + objectId: sharedLists[0].id.toString(), + user: users[0].id, + createdWhen: 1, + }, + { + collection: 'sharedList', + objectId: sharedLists[1].id.toString(), + user: users[0].id, + createdWhen: 1, + }, + { + collection: 'sharedList', + objectId: sharedLists[3].id.toString(), + user: users[0].id, + createdWhen: 1, + }, +] diff --git a/src/page-activity-indicator/background/index.test.ts b/src/page-activity-indicator/background/index.test.ts new file mode 100644 index 0000000000..b30f839b41 --- /dev/null +++ b/src/page-activity-indicator/background/index.test.ts @@ -0,0 +1,381 @@ +import type { AutoPk } from '@worldbrain/memex-common/lib/storage/types' +import { TEST_USER } from '@worldbrain/memex-common/lib/authentication/dev' +import { setupBackgroundIntegrationTest } from 'src/tests/background-integration-tests' +import * as DATA from './index.test.data' +import { + sharedListEntryToFollowedListEntry, + sharedListToFollowedList, +} from './utils' + +const calcExpectedLists = (wantedListIds?: Set) => + DATA.sharedLists + .filter((list) => wantedListIds?.has(list.id) ?? true) + .map(sharedListToFollowedList) +const calcExpectedListEntries = (wantedListIds?: Set) => + Object.entries(DATA.listEntries).flatMap(([sharedList, entries]) => + wantedListIds?.has(sharedList) ?? true + ? entries.map((entry) => + sharedListEntryToFollowedListEntry({ + ...entry, + sharedList, + }), + ) + : [], + ) + +async function setupTest(opts: { + testData?: { + ownLists?: boolean + follows?: boolean + } +}) { + const context = await setupBackgroundIntegrationTest() + + await context.authService.loginWithEmailAndPassword( + TEST_USER.email, + 'password', + ) + + if (opts?.testData) { + const { manager } = await context.getServerStorage() + + await Promise.all( + DATA.sharedLists + .filter( + (data) => + opts.testData.ownLists || + data.creator !== DATA.userReferenceA.id, + ) + .map((data) => + manager.collection('sharedList').createObject(data), + ), + ) + + await Promise.all( + Object.entries(DATA.listEntries).flatMap(([sharedList, entries]) => + entries.map((data) => + manager + .collection('sharedListEntry') + .createObject({ ...data, sharedList }), + ), + ), + ) + + await Promise.all( + Object.values(DATA.annotationListEntries).flatMap((entries) => + entries.map((data) => + manager + .collection('sharedAnnotationListEntry') + .createObject(data), + ), + ), + ) + + if (opts.testData.follows) { + await Promise.all( + DATA.activityFollows.map((data) => + manager.collection('activityFollow').createObject(data), + ), + ) + } + } + + return { ...context } +} + +describe('Page activity indicator background module tests', () => { + it('should do nothing when no owned or followed sharedLists', async () => { + const { backgroundModules, storageManager } = await setupTest({}) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { from: 1 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + }) + + it('should create followedList and followedListEntries based on owned sharedLists and their entries', async () => { + const { backgroundModules, storageManager } = await setupTest({ + testData: { ownLists: true }, + }) + + const ownListIds = new Set( + DATA.sharedLists + .filter((list) => list.creator === DATA.userReferenceA.id) + .map((list) => list.id), + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { from: 1 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds)) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(ownListIds)) + }) + + it('should delete followedList and followedListEntries when a sharedList no longer exists', async () => { + const { + backgroundModules, + storageManager, + getServerStorage, + } = await setupTest({ + testData: { ownLists: true }, + }) + + const ownListIds = new Set( + DATA.sharedLists + .filter((list) => list.creator === DATA.userReferenceA.id) + .map((list) => list.id), + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { from: 1 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds)) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(ownListIds)) + + // Delete an owned sharedList so that sync will result in removal of local data + const serverStorage = await getServerStorage() + await serverStorage.manager + .collection('sharedList') + .deleteOneObject({ id: DATA.sharedLists[0].id }) + ownListIds.delete(DATA.sharedLists[0].id) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { from: 1 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds)) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(ownListIds)) + }) + + it('should create followedList and followedListEntries based on followed sharedLists and their entries', async () => { + const { backgroundModules, storageManager } = await setupTest({ + testData: { follows: true }, + }) + + const followedListIds = new Set( + DATA.activityFollows.map((follow) => follow.objectId), + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { from: 1 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(followedListIds)) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(followedListIds)) + }) + + it('should delete followedList and followedListEntries when a sharedList is no longer followed', async () => { + const { + backgroundModules, + storageManager, + getServerStorage, + } = await setupTest({ + testData: { follows: true }, + }) + + const followedListIds = new Set( + DATA.activityFollows.map((follow) => follow.objectId), + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { from: 1 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(followedListIds)) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(followedListIds)) + + // Delete a follow so that sync will result in removal of local data + const serverStorage = await getServerStorage() + await serverStorage.manager + .collection('activityFollow') + .deleteOneObject({ objectId: DATA.activityFollows[2].objectId }) + followedListIds.delete(DATA.activityFollows[2].objectId) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { from: 1 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(followedListIds)) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(followedListIds)) + }) + + it('should create followedList and followedListEntries based on both owned and followed sharedLists and their entries', async () => { + const { backgroundModules, storageManager } = await setupTest({ + testData: { follows: true, ownLists: true }, + }) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { from: 1 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists()) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries()) + }) + + it('should delete followedList and followedListEntries when a sharedList is no longer followed and another no longer exists', async () => { + const { + backgroundModules, + storageManager, + getServerStorage, + } = await setupTest({ + testData: { follows: true, ownLists: true }, + }) + + const listIds = new Set(DATA.sharedLists.map((list) => list.id)) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { from: 1 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists()) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries()) + + // Delete a follow + owned sharedList so that sync will result in removal of local data + const serverStorage = await getServerStorage() + await serverStorage.manager + .collection('activityFollow') + .deleteOneObject({ objectId: DATA.activityFollows[2].objectId }) + await serverStorage.manager + .collection('sharedList') + .deleteOneObject({ id: DATA.sharedLists[0].id }) + listIds.delete(DATA.activityFollows[2].objectId) + listIds.delete(DATA.sharedLists[0].id) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { from: 1 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(listIds)) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(listIds)) + }) +}) diff --git a/src/page-activity-indicator/background/index.ts b/src/page-activity-indicator/background/index.ts index f64b05e79d..276a42afa5 100644 --- a/src/page-activity-indicator/background/index.ts +++ b/src/page-activity-indicator/background/index.ts @@ -1,8 +1,16 @@ +import type { AutoPk } from '@worldbrain/memex-common/lib/storage/types' +import type { UserReference } from '@worldbrain/memex-common/lib/web-interface/types/users' import type Storex from '@worldbrain/storex' +import type { ServerStorageModules } from 'src/storage/types' +import type { FollowedList } from './types' import PageActivityIndicatorStorage from './storage' export interface PageActivityIndicatorDependencies { storageManager: Storex + getCurrentUserId: () => Promise + getServerStorage: () => Promise< + Pick + > } export class PageActivityIndicatorBackground { @@ -29,4 +37,19 @@ export class PageActivityIndicatorBackground { deleteFollowedListAndAllEntries: PageActivityIndicatorStorage['deleteFollowedListAndAllEntries'] = ( data, ) => this.storage.deleteFollowedListAndAllEntries(data) + + async syncFollowedListsAndEntries(opts?: { from?: number }): Promise { + const userId = await this.deps.getCurrentUserId() + if (userId == null) { + return + } + const userReference: UserReference = { + id: userId, + type: 'user-reference', + } + const { + activityFollows, + contentSharing, + } = await this.deps.getServerStorage() + } } diff --git a/src/page-activity-indicator/background/storage.ts b/src/page-activity-indicator/background/storage.ts index 19f8b1947f..74cf48defb 100644 --- a/src/page-activity-indicator/background/storage.ts +++ b/src/page-activity-indicator/background/storage.ts @@ -51,6 +51,11 @@ export default class PageActivityIndicatorStorage extends StorageModule { collection: 'followedListEntry', operation: 'createObject', }, + findAllFollowedLists: { + collection: 'followedList', + operation: 'findObjects', + args: {}, + }, updateFollowedListEntryHasAnnotations: { collection: 'followedListEntry', operation: 'updateObjects', diff --git a/src/page-activity-indicator/background/types.ts b/src/page-activity-indicator/background/types.ts index 683ad1d950..5537719910 100644 --- a/src/page-activity-indicator/background/types.ts +++ b/src/page-activity-indicator/background/types.ts @@ -1,14 +1,16 @@ +import type { AutoPk } from '@worldbrain/memex-common/lib/storage/types' + export interface FollowedList { name: string - creator: string + creator: AutoPk lastSync?: number - sharedList: string + sharedList: AutoPk } export interface FollowedListEntry { - creator: string + creator: AutoPk entryTitle: string - followedList: string + followedList: AutoPk hasAnnotations: boolean normalizedPageUrl: string createdWhen: number diff --git a/src/page-activity-indicator/background/utils.ts b/src/page-activity-indicator/background/utils.ts new file mode 100644 index 0000000000..fbdd2d03a7 --- /dev/null +++ b/src/page-activity-indicator/background/utils.ts @@ -0,0 +1,26 @@ +import type { + SharedList, + SharedListEntry, +} from '@worldbrain/memex-common/lib/content-sharing/types' +import type { AutoPk } from '@worldbrain/memex-common/lib/storage/types' +import type { FollowedList, FollowedListEntry } from './types' + +export const sharedListToFollowedList = ( + sharedList: SharedList & { creator: AutoPk; id: AutoPk }, +): FollowedList => ({ + name: sharedList.title, + sharedList: sharedList.id, + creator: sharedList.creator, +}) + +export const sharedListEntryToFollowedListEntry = ( + entry: SharedListEntry & { creator: AutoPk; sharedList: AutoPk }, +): FollowedListEntry => ({ + followedList: entry.sharedList, + createdWhen: entry.createdWhen, + updatedWhen: entry.updatedWhen, + entryTitle: entry.entryTitle ?? '', + normalizedPageUrl: entry.normalizedUrl, + creator: entry.creator, + hasAnnotations: false, +}) From 079b5f7584d22df6eda53c40110f17db2dadb428 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Thu, 3 Nov 2022 20:51:45 +0700 Subject: [PATCH 07/29] Add test to validate lastSync - also auth test --- .../background/index.test.ts | 123 +++++++++++++++++- 1 file changed, 119 insertions(+), 4 deletions(-) diff --git a/src/page-activity-indicator/background/index.test.ts b/src/page-activity-indicator/background/index.test.ts index b30f839b41..4a6b097a84 100644 --- a/src/page-activity-indicator/background/index.test.ts +++ b/src/page-activity-indicator/background/index.test.ts @@ -31,10 +31,12 @@ async function setupTest(opts: { }) { const context = await setupBackgroundIntegrationTest() - await context.authService.loginWithEmailAndPassword( - TEST_USER.email, - 'password', - ) + if (!opts?.skipAuth) { + await context.authService.loginWithEmailAndPassword( + TEST_USER.email, + 'password', + ) + } if (opts?.testData) { const { manager } = await context.getServerStorage() @@ -84,6 +86,35 @@ async function setupTest(opts: { } describe('Page activity indicator background module tests', () => { + it('should do nothing when not logged in', async () => { + const { backgroundModules, storageManager } = await setupTest({ + skipAuth: true, + testData: { follows: true, ownLists: true }, + }) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 1 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + }) + it('should do nothing when no owned or followed sharedLists', async () => { const { backgroundModules, storageManager } = await setupTest({}) @@ -145,6 +176,90 @@ describe('Page activity indicator background module tests', () => { ).toEqual(calcExpectedListEntries(ownListIds)) }) + it('should create followedListEntries based on sharedListEntries created after lastSync time', async () => { + const { + backgroundModules, + storageManager, + getServerStorage, + } = await setupTest({ + testData: { ownLists: true }, + }) + + const ownListIds = new Set( + DATA.sharedLists + .filter((list) => list.creator === DATA.userReferenceA.id) + .map((list) => list.id), + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 1 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds)) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(ownListIds)) + + // Update lastSync time for one followedList, then create two new sharedListEntries: one pre-lastSync time, one post + const sharedList = DATA.sharedLists[0].id + await storageManager + .collection('followedList') + .updateOneObject({ sharedList }, { lastSync: { $set: 2 } }) + const serverStorage = await getServerStorage() + const newListEntries = [ + { + sharedList, + creator: DATA.users[0].id, + normalizedUrl: 'test.com/c', + originalUrl: 'https://test.com/c', + createdWhen: 1, + updatedWhen: 1, + }, + { + sharedList, + creator: DATA.users[0].id, + normalizedUrl: 'test.com/d', + originalUrl: 'https://test.com/d', + createdWhen: 4, + updatedWhen: 4, + }, + ] + for (const entry of newListEntries) { + await serverStorage.manager + .collection('sharedListEntry') + .createObject(entry) + } + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 1 }, + ) + const expected = calcExpectedListEntries(ownListIds) + + // The entry updated post-lastSync time should now be there, but not the one pre-lastSync time + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([ + ...expected, + sharedListEntryToFollowedListEntry(newListEntries[1]), + ]) + }) + it('should delete followedList and followedListEntries when a sharedList no longer exists', async () => { const { backgroundModules, From df76f3f70d2dd200c16e8ec8ce783afda9ff50fe Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Thu, 3 Nov 2022 20:57:56 +0700 Subject: [PATCH 08/29] Write skeleton of sync procedure - doesn't work, though I started discovering and working thru bugs in my tests --- external/@worldbrain/memex-common | 2 +- .../background/index.test.ts | 32 ++++--- .../background/index.ts | 91 +++++++++++++++++-- .../background/storage.ts | 25 +++++ 4 files changed, 130 insertions(+), 20 deletions(-) diff --git a/external/@worldbrain/memex-common b/external/@worldbrain/memex-common index 1029cd0bc9..f7eeb54f08 160000 --- a/external/@worldbrain/memex-common +++ b/external/@worldbrain/memex-common @@ -1 +1 @@ -Subproject commit 1029cd0bc9f277bf1fbee689553cdace1555aa5c +Subproject commit f7eeb54f08081f2df72130b57820c4e9cc12c2b3 diff --git a/src/page-activity-indicator/background/index.test.ts b/src/page-activity-indicator/background/index.test.ts index 4a6b097a84..e96cb13aff 100644 --- a/src/page-activity-indicator/background/index.test.ts +++ b/src/page-activity-indicator/background/index.test.ts @@ -11,9 +11,13 @@ const calcExpectedLists = (wantedListIds?: Set) => DATA.sharedLists .filter((list) => wantedListIds?.has(list.id) ?? true) .map(sharedListToFollowedList) -const calcExpectedListEntries = (wantedListIds?: Set) => - Object.entries(DATA.listEntries).flatMap(([sharedList, entries]) => - wantedListIds?.has(sharedList) ?? true + +const calcExpectedListEntries = (wantedListIds?: Set) => { + const _wantedListIds = wantedListIds + ? new Set([...wantedListIds].map((id) => Number(id))) + : null + return Object.entries(DATA.listEntries).flatMap(([sharedList, entries]) => + _wantedListIds?.has(Number(sharedList)) ?? true ? entries.map((entry) => sharedListEntryToFollowedListEntry({ ...entry, @@ -22,8 +26,10 @@ const calcExpectedListEntries = (wantedListIds?: Set) => ) : [], ) +} async function setupTest(opts: { + skipAuth?: boolean testData?: { ownLists?: boolean follows?: boolean @@ -128,7 +134,7 @@ describe('Page activity indicator background module tests', () => { ).toEqual([]) await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { from: 1 }, + { now: 1 }, ) expect( @@ -162,7 +168,7 @@ describe('Page activity indicator background module tests', () => { ).toEqual([]) await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { from: 1 }, + { now: 1 }, ) expect( @@ -285,7 +291,7 @@ describe('Page activity indicator background module tests', () => { ).toEqual([]) await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { from: 1 }, + { now: 1 }, ) expect( @@ -306,7 +312,7 @@ describe('Page activity indicator background module tests', () => { ownListIds.delete(DATA.sharedLists[0].id) await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { from: 1 }, + { now: 1 }, ) expect( @@ -338,7 +344,7 @@ describe('Page activity indicator background module tests', () => { ).toEqual([]) await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { from: 1 }, + { now: 1 }, ) expect( @@ -375,7 +381,7 @@ describe('Page activity indicator background module tests', () => { ).toEqual([]) await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { from: 1 }, + { now: 1 }, ) expect( @@ -395,7 +401,7 @@ describe('Page activity indicator background module tests', () => { followedListIds.delete(DATA.activityFollows[2].objectId) await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { from: 1 }, + { now: 1 }, ) expect( @@ -423,7 +429,7 @@ describe('Page activity indicator background module tests', () => { ).toEqual([]) await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { from: 1 }, + { now: 1 }, ) expect( @@ -457,7 +463,7 @@ describe('Page activity indicator background module tests', () => { ).toEqual([]) await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { from: 1 }, + { now: 1 }, ) expect( @@ -481,7 +487,7 @@ describe('Page activity indicator background module tests', () => { listIds.delete(DATA.sharedLists[0].id) await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { from: 1 }, + { now: 1 }, ) expect( diff --git a/src/page-activity-indicator/background/index.ts b/src/page-activity-indicator/background/index.ts index 276a42afa5..44093f6b46 100644 --- a/src/page-activity-indicator/background/index.ts +++ b/src/page-activity-indicator/background/index.ts @@ -4,6 +4,11 @@ import type Storex from '@worldbrain/storex' import type { ServerStorageModules } from 'src/storage/types' import type { FollowedList } from './types' import PageActivityIndicatorStorage from './storage' +import { SharedList } from '@worldbrain/memex-common/lib/content-sharing/types' +import { + sharedListEntryToFollowedListEntry, + sharedListToFollowedList, +} from './utils' export interface PageActivityIndicatorDependencies { storageManager: Storex @@ -38,18 +43,92 @@ export class PageActivityIndicatorBackground { data, ) => this.storage.deleteFollowedListAndAllEntries(data) - async syncFollowedListsAndEntries(opts?: { from?: number }): Promise { + private async getAllUserFollowedSharedListsFromServer( + userReference: UserReference, + ): Promise> { + const { + activityFollows, + contentSharing, + } = await this.deps.getServerStorage() + + const [sharedListFollows, ownedSharedLists] = await Promise.all([ + activityFollows.getAllFollowsByCollection({ + collection: 'sharedList', + userReference, + }), + contentSharing.getListsByCreator(userReference), + ]) + + // A user can follow their own shared lists, so filter them out to reduce reads + const ownedSharedListIds = new Set( + ownedSharedLists.map((list) => list.id), + ) + const followedSharedLists = await contentSharing.getListsByReferences( + sharedListFollows + .filter((follow) => !ownedSharedListIds.has(follow.objectId)) + .map((follow) => ({ + type: 'shared-list-reference', + id: follow.objectId, + })), + ) + + return [ + ...ownedSharedLists, + ...followedSharedLists.map((list) => ({ + ...list, + id: list.reference.id, + creator: list.creator.id, + })), + ] + } + + async syncFollowedListsAndEntries(opts?: { now?: number }): Promise { const userId = await this.deps.getCurrentUserId() if (userId == null) { return } - const userReference: UserReference = { + const { contentSharing } = await this.deps.getServerStorage() + + const sharedLists = await this.getAllUserFollowedSharedListsFromServer({ id: userId, type: 'user-reference', + }) + const existingFollowedListsLookup = await this.storage.findAllFollowedLists() + + // TODO: reduce N list entry reads to 1 + for (const sharedList of sharedLists) { + const localFollowedList = existingFollowedListsLookup.get( + sharedList.id, + ) + const sharedListEntries = await contentSharing.getListEntriesByList( + { + listReference: { + type: 'shared-list-reference', + id: sharedList.id, + }, + from: localFollowedList?.lastSync, + }, + ) + for (const entry of sharedListEntries) { + await this.storage.createFollowedListEntry( + sharedListEntryToFollowedListEntry({ + ...entry, + creator: entry.creator.id, + sharedList: entry.sharedList.id, + }), + ) + } + + if (localFollowedList == null) { + await this.storage.createFollowedList( + sharedListToFollowedList(sharedList), + ) + } else { + await this.storage.updateFollowedListLastSync({ + sharedList: sharedList.id, + lastSync: opts?.now ?? Date.now(), + }) + } } - const { - activityFollows, - contentSharing, - } = await this.deps.getServerStorage() } } diff --git a/src/page-activity-indicator/background/storage.ts b/src/page-activity-indicator/background/storage.ts index 74cf48defb..4a833345b7 100644 --- a/src/page-activity-indicator/background/storage.ts +++ b/src/page-activity-indicator/background/storage.ts @@ -56,6 +56,14 @@ export default class PageActivityIndicatorStorage extends StorageModule { operation: 'findObjects', args: {}, }, + updateFollowedListLastSyncTime: { + collection: 'followedList', + operation: 'updateObject', + args: [ + { sharedList: '$sharedList:pk' }, + { lastSync: '$lastSync:number' }, + ], + }, updateFollowedListEntryHasAnnotations: { collection: 'followedListEntry', operation: 'updateObjects', @@ -130,6 +138,23 @@ export default class PageActivityIndicatorStorage extends StorageModule { return object.id } + async findAllFollowedLists(): Promise> { + const followedLists: FollowedList[] = await this.operation( + 'findAllFollowedLists', + {}, + ) + return new Map(followedLists.map((list) => [list.sharedList, list])) + } + + async updateFollowedListLastSync( + data: Pick, + ): Promise { + await this.operation('updateFollowedListLastSyncTime', { + sharedList: data.sharedList, + lastSync: data.lastSync, + }) + } + async updateFollowedListEntryHasAnnotations( data: Pick< FollowedListEntry, From e42cfb9ef065f02b52d94774891fe278eb533ec1 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Mon, 7 Nov 2022 12:50:03 +0700 Subject: [PATCH 09/29] Get all owned list sync tests passing --- .../background/index.test.ts | 85 +++++++++++++------ .../background/index.ts | 29 ++++++- .../background/storage.ts | 21 +++++ .../background/utils.ts | 14 ++- 4 files changed, 121 insertions(+), 28 deletions(-) diff --git a/src/page-activity-indicator/background/index.test.ts b/src/page-activity-indicator/background/index.test.ts index e96cb13aff..2db4f1628e 100644 --- a/src/page-activity-indicator/background/index.test.ts +++ b/src/page-activity-indicator/background/index.test.ts @@ -6,26 +6,41 @@ import { sharedListEntryToFollowedListEntry, sharedListToFollowedList, } from './utils' +import type { FollowedListEntry } from './types' -const calcExpectedLists = (wantedListIds?: Set) => +const calcExpectedLists = ( + wantedListIds?: Set, + opts?: { lastSync?: number }, +) => DATA.sharedLists .filter((list) => wantedListIds?.has(list.id) ?? true) - .map(sharedListToFollowedList) + .map((list) => + sharedListToFollowedList(list, { lastSync: opts?.lastSync }), + ) -const calcExpectedListEntries = (wantedListIds?: Set) => { +const calcExpectedListEntries = ( + wantedListIds?: Set, + opts?: { extraEntries?: FollowedListEntry[] }, +) => { const _wantedListIds = wantedListIds ? new Set([...wantedListIds].map((id) => Number(id))) : null - return Object.entries(DATA.listEntries).flatMap(([sharedList, entries]) => - _wantedListIds?.has(Number(sharedList)) ?? true - ? entries.map((entry) => - sharedListEntryToFollowedListEntry({ - ...entry, - sharedList, - }), - ) - : [], - ) + return expect.arrayContaining([ + ...Object.entries(DATA.listEntries).flatMap(([sharedList, entries]) => + _wantedListIds?.has(Number(sharedList)) ?? true + ? entries.map((entry) => + sharedListEntryToFollowedListEntry( + { + ...entry, + sharedList: Number(sharedList), + }, + { id: expect.any(Number) }, + ), + ) + : [], + ), + ...(opts?.extraEntries ?? []), + ]) } async function setupTest(opts: { @@ -62,9 +77,10 @@ async function setupTest(opts: { await Promise.all( Object.entries(DATA.listEntries).flatMap(([sharedList, entries]) => entries.map((data) => - manager - .collection('sharedListEntry') - .createObject({ ...data, sharedList }), + manager.collection('sharedListEntry').createObject({ + ...data, + sharedList: Number(sharedList), + }), ), ), ) @@ -173,8 +189,21 @@ describe('Page activity indicator background module tests', () => { expect( await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(ownListIds)) + ).toEqual(calcExpectedLists(ownListIds, { lastSync: undefined })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(ownListIds)) + // Do it again to assert idempotency + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 2 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 2 })) expect( await storageManager .collection('followedListEntry') @@ -224,7 +253,7 @@ describe('Page activity indicator background module tests', () => { const sharedList = DATA.sharedLists[0].id await storageManager .collection('followedList') - .updateOneObject({ sharedList }, { lastSync: { $set: 2 } }) + .updateOneObject({ sharedList }, { lastSync: 2 }) const serverStorage = await getServerStorage() const newListEntries = [ { @@ -251,19 +280,23 @@ describe('Page activity indicator background module tests', () => { } await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 1 }, + { now: 2 }, ) - const expected = calcExpectedListEntries(ownListIds) // The entry updated post-lastSync time should now be there, but not the one pre-lastSync time expect( await storageManager .collection('followedListEntry') .findAllObjects({}), - ).toEqual([ - ...expected, - sharedListEntryToFollowedListEntry(newListEntries[1]), - ]) + ).toEqual( + calcExpectedListEntries(ownListIds, { + extraEntries: [ + sharedListEntryToFollowedListEntry(newListEntries[1], { + id: expect.any(Number), + }), + ], + }), + ) }) it('should delete followedList and followedListEntries when a sharedList no longer exists', async () => { @@ -312,12 +345,12 @@ describe('Page activity indicator background module tests', () => { ownListIds.delete(DATA.sharedLists[0].id) await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 1 }, + { now: 2 }, ) expect( await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(ownListIds)) + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 2 })) expect( await storageManager .collection('followedListEntry') diff --git a/src/page-activity-indicator/background/index.ts b/src/page-activity-indicator/background/index.ts index 44093f6b46..ade0b38787 100644 --- a/src/page-activity-indicator/background/index.ts +++ b/src/page-activity-indicator/background/index.ts @@ -6,6 +6,7 @@ import type { FollowedList } from './types' import PageActivityIndicatorStorage from './storage' import { SharedList } from '@worldbrain/memex-common/lib/content-sharing/types' import { + getFollowedListEntryIdentifier, sharedListEntryToFollowedListEntry, sharedListToFollowedList, } from './utils' @@ -95,8 +96,24 @@ export class PageActivityIndicatorBackground { }) const existingFollowedListsLookup = await this.storage.findAllFollowedLists() - // TODO: reduce N list entry reads to 1 + // Do a run over the existing followedLists, to remove any that no longer have assoc. sharedLists existing + for (const followedList of existingFollowedListsLookup.values()) { + if ( + !sharedLists.find((list) => list.id === followedList.sharedList) + ) { + await this.storage.deleteFollowedListAndAllEntries({ + sharedList: followedList.sharedList, + }) + } + } + + // TODO: reduce N list entry server reads to 1 for (const sharedList of sharedLists) { + const existingFollowedListEntryLookup = await this.storage.findAllFollowedListEntries( + { + sharedList: sharedList.id, + }, + ) const localFollowedList = existingFollowedListsLookup.get( sharedList.id, ) @@ -110,6 +127,16 @@ export class PageActivityIndicatorBackground { }, ) for (const entry of sharedListEntries) { + if ( + existingFollowedListEntryLookup.has( + getFollowedListEntryIdentifier({ + ...entry, + sharedList: entry.sharedList.id, + }), + ) + ) { + continue + } await this.storage.createFollowedListEntry( sharedListEntryToFollowedListEntry({ ...entry, diff --git a/src/page-activity-indicator/background/storage.ts b/src/page-activity-indicator/background/storage.ts index 4a833345b7..c47067d702 100644 --- a/src/page-activity-indicator/background/storage.ts +++ b/src/page-activity-indicator/background/storage.ts @@ -5,6 +5,7 @@ import { } from '@worldbrain/storex-pattern-modules' import { STORAGE_VERSIONS } from 'src/storage/constants' import { FollowedList, FollowedListEntry } from './types' +import { getFollowedListEntryIdentifier } from './utils' export default class PageActivityIndicatorStorage extends StorageModule { getConfig(): StorageModuleConfig { @@ -56,6 +57,11 @@ export default class PageActivityIndicatorStorage extends StorageModule { operation: 'findObjects', args: {}, }, + findFollowedListEntries: { + collection: 'followedListEntry', + operation: 'findObjects', + args: { followedList: '$followedList:number' }, + }, updateFollowedListLastSyncTime: { collection: 'followedList', operation: 'updateObject', @@ -146,6 +152,21 @@ export default class PageActivityIndicatorStorage extends StorageModule { return new Map(followedLists.map((list) => [list.sharedList, list])) } + async findAllFollowedListEntries( + data: Pick, + ): Promise> { + const followedListEntries: FollowedListEntry[] = await this.operation( + 'findFollowedListEntries', + { followedList: data.sharedList }, + ) + return new Map( + followedListEntries.map((entry) => [ + getFollowedListEntryIdentifier(entry), + entry, + ]), + ) + } + async updateFollowedListLastSync( data: Pick, ): Promise { diff --git a/src/page-activity-indicator/background/utils.ts b/src/page-activity-indicator/background/utils.ts index fbdd2d03a7..01067fac2a 100644 --- a/src/page-activity-indicator/background/utils.ts +++ b/src/page-activity-indicator/background/utils.ts @@ -7,15 +7,19 @@ import type { FollowedList, FollowedListEntry } from './types' export const sharedListToFollowedList = ( sharedList: SharedList & { creator: AutoPk; id: AutoPk }, + extra?: { lastSync?: number }, ): FollowedList => ({ name: sharedList.title, sharedList: sharedList.id, creator: sharedList.creator, + lastSync: extra?.lastSync, }) export const sharedListEntryToFollowedListEntry = ( entry: SharedListEntry & { creator: AutoPk; sharedList: AutoPk }, -): FollowedListEntry => ({ + extra?: { id: AutoPk }, +): FollowedListEntry & { id?: AutoPk } => ({ + id: extra?.id, followedList: entry.sharedList, createdWhen: entry.createdWhen, updatedWhen: entry.updatedWhen, @@ -24,3 +28,11 @@ export const sharedListEntryToFollowedListEntry = ( creator: entry.creator, hasAnnotations: false, }) + +/** Should be used when dealing with identifying followedListEntries without using the PK field. e.g., relating them back to sharedListEntries */ +export const getFollowedListEntryIdentifier = ( + entry: FollowedListEntry | (SharedListEntry & { sharedList: AutoPk }), +): string => + 'sharedList' in entry + ? `${entry.sharedList}-${entry.normalizedUrl}` + : `${entry.followedList}-${entry.normalizedPageUrl}` From 0b947b01682bc1dca5a9812d7814422707378c57 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Mon, 7 Nov 2022 14:43:19 +0700 Subject: [PATCH 10/29] Get all followed list sync tests passing --- .../background/index.test.ts | 127 +++++++++++++----- 1 file changed, 95 insertions(+), 32 deletions(-) diff --git a/src/page-activity-indicator/background/index.test.ts b/src/page-activity-indicator/background/index.test.ts index 2db4f1628e..e181b3cd1d 100644 --- a/src/page-activity-indicator/background/index.test.ts +++ b/src/page-activity-indicator/background/index.test.ts @@ -9,25 +9,22 @@ import { import type { FollowedListEntry } from './types' const calcExpectedLists = ( - wantedListIds?: Set, + expectedListIds: Set, opts?: { lastSync?: number }, ) => DATA.sharedLists - .filter((list) => wantedListIds?.has(list.id) ?? true) + .filter((list) => expectedListIds.has(list.id)) .map((list) => sharedListToFollowedList(list, { lastSync: opts?.lastSync }), ) const calcExpectedListEntries = ( - wantedListIds?: Set, + expectedListIds: Set, opts?: { extraEntries?: FollowedListEntry[] }, ) => { - const _wantedListIds = wantedListIds - ? new Set([...wantedListIds].map((id) => Number(id))) - : null return expect.arrayContaining([ ...Object.entries(DATA.listEntries).flatMap(([sharedList, entries]) => - _wantedListIds?.has(Number(sharedList)) ?? true + expectedListIds.has(Number(sharedList)) ? entries.map((entry) => sharedListEntryToFollowedListEntry( { @@ -97,9 +94,23 @@ async function setupTest(opts: { if (opts.testData.follows) { await Promise.all( - DATA.activityFollows.map((data) => - manager.collection('activityFollow').createObject(data), - ), + DATA.activityFollows.map(async (data) => { + const exists = await manager + .collection('sharedList') + .findObject({ id: Number(data.objectId) }) + const listData = DATA.sharedLists.find( + (list) => list.id === Number(data.objectId), + ) + if (!exists && listData) { + await manager + .collection('sharedList') + .createObject(listData) + } + await manager.collection('activityFollow').createObject({ + ...data, + objectId: Number(data.objectId), + }) + }), ) } } @@ -364,7 +375,7 @@ describe('Page activity indicator background module tests', () => { }) const followedListIds = new Set( - DATA.activityFollows.map((follow) => follow.objectId), + DATA.activityFollows.map((follow) => Number(follow.objectId)), ) expect( @@ -401,7 +412,7 @@ describe('Page activity indicator background module tests', () => { }) const followedListIds = new Set( - DATA.activityFollows.map((follow) => follow.objectId), + DATA.activityFollows.map((follow) => Number(follow.objectId)), ) expect( @@ -428,18 +439,32 @@ describe('Page activity indicator background module tests', () => { // Delete a follow so that sync will result in removal of local data const serverStorage = await getServerStorage() - await serverStorage.manager - .collection('activityFollow') - .deleteOneObject({ objectId: DATA.activityFollows[2].objectId }) - followedListIds.delete(DATA.activityFollows[2].objectId) + await serverStorage.manager.collection('activityFollow').deleteObjects({ + objectId: Number(DATA.activityFollows[2].objectId), + }) + followedListIds.delete(Number(DATA.activityFollows[2].objectId)) await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 1 }, + { now: 2 }, ) expect( await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(followedListIds)) + ).toEqual(calcExpectedLists(followedListIds, { lastSync: 2 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(followedListIds)) + + // Do it again to assert idempotency + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 3 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(followedListIds, { lastSync: 3 })) expect( await storageManager .collection('followedListEntry') @@ -452,6 +477,12 @@ describe('Page activity indicator background module tests', () => { testData: { follows: true, ownLists: true }, }) + const expectedListIds = new Set([ + DATA.sharedLists[0].id, + DATA.sharedLists[1].id, + DATA.sharedLists[3].id, + ]) + expect( await storageManager.collection('followedList').findAllObjects({}), ).toEqual([]) @@ -467,15 +498,29 @@ describe('Page activity indicator background module tests', () => { expect( await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists()) + ).toEqual(calcExpectedLists(expectedListIds)) expect( await storageManager .collection('followedListEntry') .findAllObjects({}), - ).toEqual(calcExpectedListEntries()) + ).toEqual(calcExpectedListEntries(expectedListIds)) + + // Do it again to assert idempotency + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 2 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 2 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(expectedListIds)) }) - it('should delete followedList and followedListEntries when a sharedList is no longer followed and another no longer exists', async () => { + it('should delete followedList and followedListEntries when a sharedList is no longer followed and another sharedList no longer exists', async () => { const { backgroundModules, storageManager, @@ -484,7 +529,11 @@ describe('Page activity indicator background module tests', () => { testData: { follows: true, ownLists: true }, }) - const listIds = new Set(DATA.sharedLists.map((list) => list.id)) + const expectedListIds = new Set([ + DATA.sharedLists[0].id, + DATA.sharedLists[1].id, + DATA.sharedLists[3].id, + ]) expect( await storageManager.collection('followedList').findAllObjects({}), @@ -501,35 +550,49 @@ describe('Page activity indicator background module tests', () => { expect( await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists()) + ).toEqual(calcExpectedLists(expectedListIds)) expect( await storageManager .collection('followedListEntry') .findAllObjects({}), - ).toEqual(calcExpectedListEntries()) + ).toEqual(calcExpectedListEntries(expectedListIds)) // Delete a follow + owned sharedList so that sync will result in removal of local data const serverStorage = await getServerStorage() - await serverStorage.manager - .collection('activityFollow') - .deleteOneObject({ objectId: DATA.activityFollows[2].objectId }) + await serverStorage.manager.collection('activityFollow').deleteObjects({ + objectId: Number(DATA.activityFollows[2].objectId), + }) await serverStorage.manager .collection('sharedList') .deleteOneObject({ id: DATA.sharedLists[0].id }) - listIds.delete(DATA.activityFollows[2].objectId) - listIds.delete(DATA.sharedLists[0].id) + expectedListIds.delete(Number(DATA.activityFollows[2].objectId)) + expectedListIds.delete(DATA.sharedLists[0].id) await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 1 }, + { now: 2 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 2 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(expectedListIds)) + + // Do it again to assert idempotency + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 3 }, ) expect( await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(listIds)) + ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 3 })) expect( await storageManager .collection('followedListEntry') .findAllObjects({}), - ).toEqual(calcExpectedListEntries(listIds)) + ).toEqual(calcExpectedListEntries(expectedListIds)) }) }) From d72382a6f9820b4700bd5b4f95f92338b02bcea8 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Mon, 7 Nov 2022 19:11:49 +0700 Subject: [PATCH 11/29] Implement hasAnnotations flag syncing --- external/@worldbrain/memex-common | 2 +- .../background/index.test.data.ts | 4 +- .../background/index.test.ts | 224 +++++++++++++++++- .../background/index.ts | 108 +++++++-- .../background/utils.ts | 8 +- 5 files changed, 321 insertions(+), 25 deletions(-) diff --git a/external/@worldbrain/memex-common b/external/@worldbrain/memex-common index f7eeb54f08..f67b1543cb 160000 --- a/external/@worldbrain/memex-common +++ b/external/@worldbrain/memex-common @@ -1 +1 @@ -Subproject commit f7eeb54f08081f2df72130b57820c4e9cc12c2b3 +Subproject commit f67b1543cbb8c222da710cd062902e07c8cdf1c8 diff --git a/src/page-activity-indicator/background/index.test.data.ts b/src/page-activity-indicator/background/index.test.data.ts index a3b40e9118..64c569043a 100644 --- a/src/page-activity-indicator/background/index.test.data.ts +++ b/src/page-activity-indicator/background/index.test.data.ts @@ -112,14 +112,14 @@ export const listEntries: { } export const annotationListEntries: { - [normalizedPageUrl: string]: Array< + [sharedList: number]: Array< SharedAnnotationListEntry & { sharedList: AutoPk creator: AutoPk } > } = { - ['test.com/a']: [ + [sharedLists[0].id]: [ { creator: users[0].id, sharedList: sharedLists[0].id, diff --git a/src/page-activity-indicator/background/index.test.ts b/src/page-activity-indicator/background/index.test.ts index e181b3cd1d..88a0cdbc9d 100644 --- a/src/page-activity-indicator/background/index.test.ts +++ b/src/page-activity-indicator/background/index.test.ts @@ -31,7 +31,18 @@ const calcExpectedListEntries = ( ...entry, sharedList: Number(sharedList), }, - { id: expect.any(Number) }, + { + id: expect.any(Number), + hasAnnotations: DATA.annotationListEntries[ + sharedList + ]?.reduce( + (acc, curr) => + acc || + curr.normalizedPageUrl === + entry.normalizedUrl, + false, + ), + }, ), ) : [], @@ -310,6 +321,217 @@ describe('Page activity indicator background module tests', () => { ) }) + it('should set hasAnnotation flag on existing followedListEntry if new annotation exists on resync', async () => { + const { + backgroundModules, + storageManager, + getServerStorage, + } = await setupTest({ + testData: { ownLists: true }, + }) + + const ownListIds = new Set( + DATA.sharedLists + .filter((list) => list.creator === DATA.userReferenceA.id) + .map((list) => list.id), + ) + const createExpectedListEntry = ( + entry: any, + sharedList: AutoPk, + hasAnnotations: boolean, + ) => + sharedListEntryToFollowedListEntry( + { ...entry, sharedList }, + { hasAnnotations, id: expect.any(Number) }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 1 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds)) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([ + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][1], + DATA.sharedLists[0].id, + false, + ), + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][0], + DATA.sharedLists[0].id, + true, + ), + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[1].id][1], + DATA.sharedLists[1].id, + false, + ), + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[1].id][0], + DATA.sharedLists[1].id, + false, + ), + ]) + + const serverStorage = await getServerStorage() + // Create an annotation list entry for one of the existing list entries + await serverStorage.manager + .collection('sharedAnnotationListEntry') + .createObject({ + creator: DATA.users[2].id, + sharedList: DATA.sharedLists[1].id, + normalizedPageUrl: 'test.com/a', + updatedWhen: 1, + createdWhen: 1, + uploadedWhen: 1, + }) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 2 }, + ) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 2 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([ + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][1], + DATA.sharedLists[0].id, + false, + ), + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][0], + DATA.sharedLists[0].id, + true, + ), + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[1].id][1], + DATA.sharedLists[1].id, + false, + ), + createExpectedListEntry( + { + ...DATA.listEntries[DATA.sharedLists[1].id][0], + updatedWhen: 2, + }, + DATA.sharedLists[1].id, + true, + ), + ]) + + // And another one + await serverStorage.manager + .collection('sharedAnnotationListEntry') + .createObject({ + creator: DATA.users[2].id, + sharedList: DATA.sharedLists[1].id, + normalizedPageUrl: 'test.com/b', + updatedWhen: 3, + createdWhen: 3, + uploadedWhen: 3, + }) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 3 }, + ) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([ + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][1], + DATA.sharedLists[0].id, + false, + ), + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][0], + DATA.sharedLists[0].id, + true, + ), + createExpectedListEntry( + { + ...DATA.listEntries[DATA.sharedLists[1].id][1], + updatedWhen: 3, + }, + DATA.sharedLists[1].id, + true, + ), + createExpectedListEntry( + { + ...DATA.listEntries[DATA.sharedLists[1].id][0], + updatedWhen: 2, + }, + DATA.sharedLists[1].id, + true, + ), + ]) + + // Now delete them, to ensure it works the other way around + await serverStorage.manager + .collection('sharedAnnotationListEntry') + .deleteObjects({ + sharedList: DATA.sharedLists[1].id, + }) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 4 }, + ) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([ + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][1], + DATA.sharedLists[0].id, + false, + ), + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][0], + DATA.sharedLists[0].id, + true, + ), + createExpectedListEntry( + { + ...DATA.listEntries[DATA.sharedLists[1].id][1], + updatedWhen: 4, + }, + DATA.sharedLists[1].id, + false, + ), + createExpectedListEntry( + { + ...DATA.listEntries[DATA.sharedLists[1].id][0], + updatedWhen: 4, + }, + DATA.sharedLists[1].id, + false, + ), + ]) + }) + it('should delete followedList and followedListEntries when a sharedList no longer exists', async () => { const { backgroundModules, diff --git a/src/page-activity-indicator/background/index.ts b/src/page-activity-indicator/background/index.ts index ade0b38787..da31d5f807 100644 --- a/src/page-activity-indicator/background/index.ts +++ b/src/page-activity-indicator/background/index.ts @@ -2,9 +2,11 @@ import type { AutoPk } from '@worldbrain/memex-common/lib/storage/types' import type { UserReference } from '@worldbrain/memex-common/lib/web-interface/types/users' import type Storex from '@worldbrain/storex' import type { ServerStorageModules } from 'src/storage/types' -import type { FollowedList } from './types' import PageActivityIndicatorStorage from './storage' -import { SharedList } from '@worldbrain/memex-common/lib/content-sharing/types' +import type { + SharedList, + SharedListReference, +} from '@worldbrain/memex-common/lib/content-sharing/types' import { getFollowedListEntryIdentifier, sharedListEntryToFollowedListEntry, @@ -107,8 +109,11 @@ export class PageActivityIndicatorBackground { } } - // TODO: reduce N list entry server reads to 1 for (const sharedList of sharedLists) { + const listReference: SharedListReference = { + type: 'shared-list-reference', + id: sharedList.id, + } const existingFollowedListEntryLookup = await this.storage.findAllFollowedListEntries( { sharedList: sharedList.id, @@ -119,31 +124,98 @@ export class PageActivityIndicatorBackground { ) const sharedListEntries = await contentSharing.getListEntriesByList( { - listReference: { - type: 'shared-list-reference', - id: sharedList.id, - }, + listReference, from: localFollowedList?.lastSync, }, ) + const sharedAnnotationListEntries = await contentSharing.getAnnotationListEntries( + { + listReference, + // NOTE: We have to always get all the annotation entries as there's way to determine the true->false case for `followedListEntry.hasAnnotations` if you only have partial results + // from: localFollowedList?.lastSync, + }, + ) + for (const entry of sharedListEntries) { - if ( - existingFollowedListEntryLookup.has( - getFollowedListEntryIdentifier({ - ...entry, - sharedList: entry.sharedList.id, - }), + const hasAnnotations = !!sharedAnnotationListEntries[ + entry.normalizedUrl + ]?.length + const localFollowedListEntry = existingFollowedListEntryLookup.get( + getFollowedListEntryIdentifier({ + ...entry, + sharedList: entry.sharedList.id, + }), + ) + + if (!localFollowedListEntry) { + await this.storage.createFollowedListEntry( + sharedListEntryToFollowedListEntry( + { + ...entry, + creator: entry.creator.id, + sharedList: entry.sharedList.id, + }, + { hasAnnotations }, + ), ) + } else if ( + localFollowedListEntry.hasAnnotations !== hasAnnotations ) { - continue + await this.storage.updateFollowedListEntryHasAnnotations({ + normalizedPageUrl: entry.normalizedUrl, + followedList: entry.sharedList.id, + updatedWhen: opts?.now, + hasAnnotations, + }) } - await this.storage.createFollowedListEntry( - sharedListEntryToFollowedListEntry({ - ...entry, - creator: entry.creator.id, + } + + // This handles the case where a new annotation was created, but the assoc. sharedListEntry didn't get their updatedWhen timestamp updated + const recentAnnotationEntries = Object.values( + sharedAnnotationListEntries, + ) + .flat() + .filter( + (annotationEntry) => + !sharedListEntries.find( + (entry) => + entry.normalizedUrl === + annotationEntry.normalizedPageUrl, + ), + ) + for (const entry of recentAnnotationEntries) { + const localFollowedListEntry = existingFollowedListEntryLookup.get( + getFollowedListEntryIdentifier({ + normalizedUrl: entry.normalizedPageUrl, sharedList: entry.sharedList.id, }), ) + if (localFollowedListEntry?.hasAnnotations) { + continue + } + + await this.storage.updateFollowedListEntryHasAnnotations({ + normalizedPageUrl: entry.normalizedPageUrl, + followedList: entry.sharedList.id, + updatedWhen: opts?.now, + hasAnnotations: true, + }) + } + + // This handles the case where the last annotation for an entry was deleted + for (const localEntry of existingFollowedListEntryLookup.values()) { + if ( + localEntry.hasAnnotations && + !sharedAnnotationListEntries[localEntry.normalizedPageUrl] + ?.length + ) { + await this.storage.updateFollowedListEntryHasAnnotations({ + normalizedPageUrl: localEntry.normalizedPageUrl, + followedList: localEntry.followedList, + updatedWhen: opts?.now, + hasAnnotations: false, + }) + } } if (localFollowedList == null) { diff --git a/src/page-activity-indicator/background/utils.ts b/src/page-activity-indicator/background/utils.ts index 01067fac2a..d99b750a6b 100644 --- a/src/page-activity-indicator/background/utils.ts +++ b/src/page-activity-indicator/background/utils.ts @@ -17,7 +17,7 @@ export const sharedListToFollowedList = ( export const sharedListEntryToFollowedListEntry = ( entry: SharedListEntry & { creator: AutoPk; sharedList: AutoPk }, - extra?: { id: AutoPk }, + extra?: { id?: AutoPk; hasAnnotations?: boolean }, ): FollowedListEntry & { id?: AutoPk } => ({ id: extra?.id, followedList: entry.sharedList, @@ -26,12 +26,14 @@ export const sharedListEntryToFollowedListEntry = ( entryTitle: entry.entryTitle ?? '', normalizedPageUrl: entry.normalizedUrl, creator: entry.creator, - hasAnnotations: false, + hasAnnotations: extra?.hasAnnotations ?? false, }) /** Should be used when dealing with identifying followedListEntries without using the PK field. e.g., relating them back to sharedListEntries */ export const getFollowedListEntryIdentifier = ( - entry: FollowedListEntry | (SharedListEntry & { sharedList: AutoPk }), + entry: + | Pick + | (Pick & { sharedList: AutoPk }), ): string => 'sharedList' in entry ? `${entry.sharedList}-${entry.normalizedUrl}` From 5e50843f01724129b502b2ddb84e3330c9d51da3 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Mon, 7 Nov 2022 19:25:04 +0700 Subject: [PATCH 12/29] Ensure lastSync is always set at the end of sync --- .../background/index.test.ts | 20 ++++++++++--------- .../background/index.ts | 13 +++++++----- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/page-activity-indicator/background/index.test.ts b/src/page-activity-indicator/background/index.test.ts index 88a0cdbc9d..1453198229 100644 --- a/src/page-activity-indicator/background/index.test.ts +++ b/src/page-activity-indicator/background/index.test.ts @@ -211,7 +211,7 @@ describe('Page activity indicator background module tests', () => { expect( await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(ownListIds, { lastSync: undefined })) + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 1 })) expect( await storageManager .collection('followedListEntry') @@ -263,8 +263,7 @@ describe('Page activity indicator background module tests', () => { expect( await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(ownListIds)) - + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 1 })) expect( await storageManager .collection('followedListEntry') @@ -305,6 +304,9 @@ describe('Page activity indicator background module tests', () => { { now: 2 }, ) + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 2 })) // The entry updated post-lastSync time should now be there, but not the one pre-lastSync time expect( await storageManager @@ -360,7 +362,7 @@ describe('Page activity indicator background module tests', () => { expect( await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(ownListIds)) + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 1 })) expect( await storageManager .collection('followedListEntry') @@ -562,7 +564,7 @@ describe('Page activity indicator background module tests', () => { expect( await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(ownListIds)) + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 1 })) expect( await storageManager @@ -615,7 +617,7 @@ describe('Page activity indicator background module tests', () => { expect( await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(followedListIds)) + ).toEqual(calcExpectedLists(followedListIds, { lastSync: 1 })) expect( await storageManager @@ -652,7 +654,7 @@ describe('Page activity indicator background module tests', () => { expect( await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(followedListIds)) + ).toEqual(calcExpectedLists(followedListIds, { lastSync: 1 })) expect( await storageManager .collection('followedListEntry') @@ -720,7 +722,7 @@ describe('Page activity indicator background module tests', () => { expect( await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(expectedListIds)) + ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 1 })) expect( await storageManager .collection('followedListEntry') @@ -772,7 +774,7 @@ describe('Page activity indicator background module tests', () => { expect( await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(expectedListIds)) + ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 1 })) expect( await storageManager .collection('followedListEntry') diff --git a/src/page-activity-indicator/background/index.ts b/src/page-activity-indicator/background/index.ts index da31d5f807..1fb2681987 100644 --- a/src/page-activity-indicator/background/index.ts +++ b/src/page-activity-indicator/background/index.ts @@ -90,6 +90,7 @@ export class PageActivityIndicatorBackground { if (userId == null) { return } + const now = opts?.now ?? Date.now() const { contentSharing } = await this.deps.getServerStorage() const sharedLists = await this.getAllUserFollowedSharedListsFromServer({ @@ -164,7 +165,7 @@ export class PageActivityIndicatorBackground { await this.storage.updateFollowedListEntryHasAnnotations({ normalizedPageUrl: entry.normalizedUrl, followedList: entry.sharedList.id, - updatedWhen: opts?.now, + updatedWhen: now, hasAnnotations, }) } @@ -197,7 +198,7 @@ export class PageActivityIndicatorBackground { await this.storage.updateFollowedListEntryHasAnnotations({ normalizedPageUrl: entry.normalizedPageUrl, followedList: entry.sharedList.id, - updatedWhen: opts?.now, + updatedWhen: now, hasAnnotations: true, }) } @@ -212,7 +213,7 @@ export class PageActivityIndicatorBackground { await this.storage.updateFollowedListEntryHasAnnotations({ normalizedPageUrl: localEntry.normalizedPageUrl, followedList: localEntry.followedList, - updatedWhen: opts?.now, + updatedWhen: now, hasAnnotations: false, }) } @@ -220,12 +221,14 @@ export class PageActivityIndicatorBackground { if (localFollowedList == null) { await this.storage.createFollowedList( - sharedListToFollowedList(sharedList), + sharedListToFollowedList(sharedList, { + lastSync: now, + }), ) } else { await this.storage.updateFollowedListLastSync({ sharedList: sharedList.id, - lastSync: opts?.now ?? Date.now(), + lastSync: now, }) } } From 3f0df26fcde0d3a050c84af5fbfbd52c9e548852 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Thu, 10 Nov 2022 15:17:59 +0700 Subject: [PATCH 13/29] Update MockPushMessagingService with missing methods --- src/tests/push-messaging.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/tests/push-messaging.ts b/src/tests/push-messaging.ts index 2426dd4144..1341663aac 100644 --- a/src/tests/push-messaging.ts +++ b/src/tests/push-messaging.ts @@ -5,6 +5,7 @@ import type { import type { AutoPk } from '@worldbrain/memex-common/lib/storage/types' export class MockPushMessagingService implements PushMessagingServiceInterface { + topicSubscribers = new Map>() sentMessages: Array< { payload: PushMessagePayload @@ -14,6 +15,22 @@ export class MockPushMessagingService implements PushMessagingServiceInterface { ) > = [] + subscribeUserToTopic: PushMessagingServiceInterface['subscribeUserToTopic'] = async ( + userReference, + topic, + ) => { + const existing = this.topicSubscribers.get(topic) ?? new Set() + existing.add(userReference.id) + } + + unsubscribeUserFromTopic: PushMessagingServiceInterface['unsubscribeUserFromTopic'] = async ( + userReference, + topic, + ) => { + const existing = this.topicSubscribers.get(topic) ?? new Set() + existing.delete(userReference.id) + } + sendToTopic: PushMessagingServiceInterface['sendToTopic'] = async ( topic, payload, From 973750b976d0d4ca4d13a8392818baa25828b855 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Fri, 11 Nov 2022 10:17:24 +0700 Subject: [PATCH 14/29] Implement getPageActivityStatus remote fn --- src/background-mv3.ts | 2 + src/background.ts | 2 + .../background/index.test.ts | 1343 +++++++++-------- .../background/index.ts | 27 +- .../background/storage.ts | 16 + .../background/types.ts | 11 + src/util/remote-functions-background.ts | 3 + 7 files changed, 810 insertions(+), 594 deletions(-) diff --git a/src/background-mv3.ts b/src/background-mv3.ts index 56f7109c0f..1689bcc401 100644 --- a/src/background-mv3.ts +++ b/src/background-mv3.ts @@ -170,6 +170,8 @@ async function main() { copyPaster: backgroundModules.copyPaster.remoteFunctions, contentSharing: backgroundModules.contentSharing.remoteFunctions, personalCloud: backgroundModules.personalCloud.remoteFunctions, + pageActivityIndicator: + backgroundModules.pageActivityIndicator.remoteFunctions, pdf: backgroundModules.pdfBg.remoteFunctions, }) rpcManager.unpause() diff --git a/src/background.ts b/src/background.ts index 779069b1b7..6d479ca50e 100644 --- a/src/background.ts +++ b/src/background.ts @@ -202,6 +202,8 @@ export async function main() { featuresBeta: backgroundModules.featuresBeta, tags: backgroundModules.tags.remoteFunctions, collections: backgroundModules.customLists.remoteFunctions, + pageActivityIndicator: + backgroundModules.pageActivityIndicator.remoteFunctions, readablePageArchives: backgroundModules.readable.remoteFunctions, copyPaster: backgroundModules.copyPaster.remoteFunctions, contentSharing: backgroundModules.contentSharing.remoteFunctions, diff --git a/src/page-activity-indicator/background/index.test.ts b/src/page-activity-indicator/background/index.test.ts index 1453198229..b12310a017 100644 --- a/src/page-activity-indicator/background/index.test.ts +++ b/src/page-activity-indicator/background/index.test.ts @@ -130,693 +130,850 @@ async function setupTest(opts: { } describe('Page activity indicator background module tests', () => { - it('should do nothing when not logged in', async () => { - const { backgroundModules, storageManager } = await setupTest({ - skipAuth: true, + it('should be able to derive page activity status from stored followedLists data', async () => { + const { backgroundModules, getServerStorage } = await setupTest({ testData: { follows: true, ownLists: true }, }) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual([]) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) - - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 1 }, - ) + const serverStorage = await getServerStorage() expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual([]) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) - }) - - it('should do nothing when no owned or followed sharedLists', async () => { - const { backgroundModules, storageManager } = await setupTest({}) - + await backgroundModules.pageActivityIndicator[ + 'getPageActivityStatus' + ]('https://test.com/a'), + ).toEqual('no-activity') expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual([]) + await backgroundModules.pageActivityIndicator[ + 'getPageActivityStatus' + ]('https://test.com/b'), + ).toEqual('no-activity') expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) - + await backgroundModules.pageActivityIndicator[ + 'getPageActivityStatus' + ]('https://test.com/c'), + ).toEqual('no-activity') await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( { now: 1 }, ) - - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual([]) expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) - }) - - it('should create followedList and followedListEntries based on owned sharedLists and their entries', async () => { - const { backgroundModules, storageManager } = await setupTest({ - testData: { ownLists: true }, - }) - - const ownListIds = new Set( - DATA.sharedLists - .filter((list) => list.creator === DATA.userReferenceA.id) - .map((list) => list.id), - ) - + await backgroundModules.pageActivityIndicator[ + 'getPageActivityStatus' + ]('https://test.com/a'), + ).toEqual('has-annotations') expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual([]) + await backgroundModules.pageActivityIndicator[ + 'getPageActivityStatus' + ]('https://test.com/b'), + ).toEqual('no-annotations') expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) - - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 1 }, - ) + await backgroundModules.pageActivityIndicator[ + 'getPageActivityStatus' + ]('https://test.com/c'), + ).toEqual('no-activity') - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(ownListIds, { lastSync: 1 })) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual(calcExpectedListEntries(ownListIds)) + // Delete annotations on the server, re-sync, then assert the status changes + await serverStorage.manager + .collection('sharedAnnotationListEntry') + .deleteObjects({}) - // Do it again to assert idempotency await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( { now: 2 }, ) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(ownListIds, { lastSync: 2 })) + await backgroundModules.pageActivityIndicator[ + 'getPageActivityStatus' + ]('https://test.com/a'), + ).toEqual('no-annotations') expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual(calcExpectedListEntries(ownListIds)) - }) - - it('should create followedListEntries based on sharedListEntries created after lastSync time', async () => { - const { - backgroundModules, - storageManager, - getServerStorage, - } = await setupTest({ - testData: { ownLists: true }, - }) - - const ownListIds = new Set( - DATA.sharedLists - .filter((list) => list.creator === DATA.userReferenceA.id) - .map((list) => list.id), - ) - - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual([]) + await backgroundModules.pageActivityIndicator[ + 'getPageActivityStatus' + ]('https://test.com/b'), + ).toEqual('no-annotations') expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) - - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 1 }, - ) + await backgroundModules.pageActivityIndicator[ + 'getPageActivityStatus' + ]('https://test.com/c'), + ).toEqual('no-activity') - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(ownListIds, { lastSync: 1 })) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual(calcExpectedListEntries(ownListIds)) - - // Update lastSync time for one followedList, then create two new sharedListEntries: one pre-lastSync time, one post - const sharedList = DATA.sharedLists[0].id - await storageManager - .collection('followedList') - .updateOneObject({ sharedList }, { lastSync: 2 }) - const serverStorage = await getServerStorage() - const newListEntries = [ - { - sharedList, - creator: DATA.users[0].id, - normalizedUrl: 'test.com/c', - originalUrl: 'https://test.com/c', - createdWhen: 1, - updatedWhen: 1, - }, - { - sharedList, - creator: DATA.users[0].id, - normalizedUrl: 'test.com/d', - originalUrl: 'https://test.com/d', - createdWhen: 4, - updatedWhen: 4, - }, - ] - for (const entry of newListEntries) { - await serverStorage.manager - .collection('sharedListEntry') - .createObject(entry) - } + // Re-add an annotation for good measure + await serverStorage.manager + .collection('sharedAnnotationListEntry') + .createObject({ + ...DATA.annotationListEntries[DATA.sharedLists[0].id][0], + normalizedPageUrl: 'test.com/b', + }) await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( { now: 2 }, ) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(ownListIds, { lastSync: 2 })) - // The entry updated post-lastSync time should now be there, but not the one pre-lastSync time + await backgroundModules.pageActivityIndicator[ + 'getPageActivityStatus' + ]('https://test.com/a'), + ).toEqual('no-annotations') expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual( - calcExpectedListEntries(ownListIds, { - extraEntries: [ - sharedListEntryToFollowedListEntry(newListEntries[1], { - id: expect.any(Number), - }), - ], - }), - ) + await backgroundModules.pageActivityIndicator[ + 'getPageActivityStatus' + ]('https://test.com/b'), + ).toEqual('has-annotations') + expect( + await backgroundModules.pageActivityIndicator[ + 'getPageActivityStatus' + ]('https://test.com/c'), + ).toEqual('no-activity') }) - it('should set hasAnnotation flag on existing followedListEntry if new annotation exists on resync', async () => { - const { - backgroundModules, - storageManager, - getServerStorage, - } = await setupTest({ - testData: { ownLists: true }, - }) + describe('pull sync followed lists tests', () => { + it('should do nothing when not logged in', async () => { + const { backgroundModules, storageManager } = await setupTest({ + skipAuth: true, + testData: { follows: true, ownLists: true }, + }) - const ownListIds = new Set( - DATA.sharedLists - .filter((list) => list.creator === DATA.userReferenceA.id) - .map((list) => list.id), - ) - const createExpectedListEntry = ( - entry: any, - sharedList: AutoPk, - hasAnnotations: boolean, - ) => - sharedListEntryToFollowedListEntry( - { ...entry, sharedList }, - { hasAnnotations, id: expect.any(Number) }, + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 1 }, ) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual([]) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + }) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 1 }, - ) + it('should do nothing when no owned or followed sharedLists', async () => { + const { backgroundModules, storageManager } = await setupTest({}) + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 1 }, + ) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(ownListIds, { lastSync: 1 })) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([ - createExpectedListEntry( - DATA.listEntries[DATA.sharedLists[0].id][1], - DATA.sharedLists[0].id, - false, - ), - createExpectedListEntry( - DATA.listEntries[DATA.sharedLists[0].id][0], - DATA.sharedLists[0].id, - true, - ), - createExpectedListEntry( - DATA.listEntries[DATA.sharedLists[1].id][1], - DATA.sharedLists[1].id, - false, - ), - createExpectedListEntry( - DATA.listEntries[DATA.sharedLists[1].id][0], - DATA.sharedLists[1].id, - false, - ), - ]) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + }) - const serverStorage = await getServerStorage() - // Create an annotation list entry for one of the existing list entries - await serverStorage.manager - .collection('sharedAnnotationListEntry') - .createObject({ - creator: DATA.users[2].id, - sharedList: DATA.sharedLists[1].id, - normalizedPageUrl: 'test.com/a', - updatedWhen: 1, - createdWhen: 1, - uploadedWhen: 1, + it('should create followedList and followedListEntries based on owned sharedLists and their entries', async () => { + const { backgroundModules, storageManager } = await setupTest({ + testData: { ownLists: true }, }) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 2 }, - ) - - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(ownListIds, { lastSync: 2 })) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([ - createExpectedListEntry( - DATA.listEntries[DATA.sharedLists[0].id][1], - DATA.sharedLists[0].id, - false, - ), - createExpectedListEntry( - DATA.listEntries[DATA.sharedLists[0].id][0], - DATA.sharedLists[0].id, - true, - ), - createExpectedListEntry( - DATA.listEntries[DATA.sharedLists[1].id][1], - DATA.sharedLists[1].id, - false, - ), - createExpectedListEntry( - { - ...DATA.listEntries[DATA.sharedLists[1].id][0], - updatedWhen: 2, - }, - DATA.sharedLists[1].id, - true, - ), - ]) + const ownListIds = new Set( + DATA.sharedLists + .filter((list) => list.creator === DATA.userReferenceA.id) + .map((list) => list.id), + ) - // And another one - await serverStorage.manager - .collection('sharedAnnotationListEntry') - .createObject({ - creator: DATA.users[2].id, - sharedList: DATA.sharedLists[1].id, - normalizedPageUrl: 'test.com/b', - updatedWhen: 3, - createdWhen: 3, - uploadedWhen: 3, - }) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 1 }, + ) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 3 }, - ) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 1 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(ownListIds)) + + // Do it again to assert idempotency + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 2 }, + ) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([ - createExpectedListEntry( - DATA.listEntries[DATA.sharedLists[0].id][1], - DATA.sharedLists[0].id, - false, - ), - createExpectedListEntry( - DATA.listEntries[DATA.sharedLists[0].id][0], - DATA.sharedLists[0].id, - true, - ), - createExpectedListEntry( - { - ...DATA.listEntries[DATA.sharedLists[1].id][1], - updatedWhen: 3, - }, - DATA.sharedLists[1].id, - true, - ), - createExpectedListEntry( - { - ...DATA.listEntries[DATA.sharedLists[1].id][0], - updatedWhen: 2, - }, - DATA.sharedLists[1].id, - true, - ), - ]) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 2 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(ownListIds)) + }) - // Now delete them, to ensure it works the other way around - await serverStorage.manager - .collection('sharedAnnotationListEntry') - .deleteObjects({ - sharedList: DATA.sharedLists[1].id, + it('should create followedListEntries based on sharedListEntries created after lastSync time', async () => { + const { + backgroundModules, + storageManager, + getServerStorage, + } = await setupTest({ + testData: { ownLists: true }, }) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 4 }, - ) + const ownListIds = new Set( + DATA.sharedLists + .filter((list) => list.creator === DATA.userReferenceA.id) + .map((list) => list.id), + ) - expect( + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 1 }, + ) + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 1 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(ownListIds)) + + // Update lastSync time for one followedList, then create two new sharedListEntries: one pre-lastSync time, one post + const sharedList = DATA.sharedLists[0].id await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([ - createExpectedListEntry( - DATA.listEntries[DATA.sharedLists[0].id][1], - DATA.sharedLists[0].id, - false, - ), - createExpectedListEntry( - DATA.listEntries[DATA.sharedLists[0].id][0], - DATA.sharedLists[0].id, - true, - ), - createExpectedListEntry( + .collection('followedList') + .updateOneObject({ sharedList }, { lastSync: 2 }) + const serverStorage = await getServerStorage() + const newListEntries = [ { - ...DATA.listEntries[DATA.sharedLists[1].id][1], - updatedWhen: 4, + sharedList, + creator: DATA.users[0].id, + normalizedUrl: 'test.com/c', + originalUrl: 'https://test.com/c', + createdWhen: 1, + updatedWhen: 1, }, - DATA.sharedLists[1].id, - false, - ), - createExpectedListEntry( { - ...DATA.listEntries[DATA.sharedLists[1].id][0], + sharedList, + creator: DATA.users[0].id, + normalizedUrl: 'test.com/d', + originalUrl: 'https://test.com/d', + createdWhen: 4, updatedWhen: 4, }, - DATA.sharedLists[1].id, - false, - ), - ]) - }) + ] + for (const entry of newListEntries) { + await serverStorage.manager + .collection('sharedListEntry') + .createObject(entry) + } + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 2 }, + ) - it('should delete followedList and followedListEntries when a sharedList no longer exists', async () => { - const { - backgroundModules, - storageManager, - getServerStorage, - } = await setupTest({ - testData: { ownLists: true }, + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 2 })) + // The entry updated post-lastSync time should now be there, but not the one pre-lastSync time + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual( + calcExpectedListEntries(ownListIds, { + extraEntries: [ + sharedListEntryToFollowedListEntry(newListEntries[1], { + id: expect.any(Number), + }), + ], + }), + ) }) - const ownListIds = new Set( - DATA.sharedLists - .filter((list) => list.creator === DATA.userReferenceA.id) - .map((list) => list.id), - ) + it('should set hasAnnotation flag on existing followedListEntry if new annotation exists on resync', async () => { + const { + backgroundModules, + storageManager, + getServerStorage, + } = await setupTest({ + testData: { ownLists: true }, + }) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual([]) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) + const ownListIds = new Set( + DATA.sharedLists + .filter((list) => list.creator === DATA.userReferenceA.id) + .map((list) => list.id), + ) + const createExpectedListEntry = ( + entry: any, + sharedList: AutoPk, + hasAnnotations: boolean, + ) => + sharedListEntryToFollowedListEntry( + { ...entry, sharedList }, + { hasAnnotations, id: expect.any(Number) }, + ) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 1 }, - ) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 1 }, + ) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(ownListIds, { lastSync: 1 })) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 1 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([ + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][1], + DATA.sharedLists[0].id, + false, + ), + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][0], + DATA.sharedLists[0].id, + true, + ), + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[1].id][1], + DATA.sharedLists[1].id, + false, + ), + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[1].id][0], + DATA.sharedLists[1].id, + false, + ), + ]) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual(calcExpectedListEntries(ownListIds)) + const serverStorage = await getServerStorage() + // Create an annotation list entry for one of the existing list entries + await serverStorage.manager + .collection('sharedAnnotationListEntry') + .createObject({ + creator: DATA.users[2].id, + sharedList: DATA.sharedLists[1].id, + normalizedPageUrl: 'test.com/a', + updatedWhen: 1, + createdWhen: 1, + uploadedWhen: 1, + }) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 2 }, + ) - // Delete an owned sharedList so that sync will result in removal of local data - const serverStorage = await getServerStorage() - await serverStorage.manager - .collection('sharedList') - .deleteOneObject({ id: DATA.sharedLists[0].id }) - ownListIds.delete(DATA.sharedLists[0].id) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 2 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([ + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][1], + DATA.sharedLists[0].id, + false, + ), + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][0], + DATA.sharedLists[0].id, + true, + ), + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[1].id][1], + DATA.sharedLists[1].id, + false, + ), + createExpectedListEntry( + { + ...DATA.listEntries[DATA.sharedLists[1].id][0], + updatedWhen: 2, + }, + DATA.sharedLists[1].id, + true, + ), + ]) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 2 }, - ) + // And another one + await serverStorage.manager + .collection('sharedAnnotationListEntry') + .createObject({ + creator: DATA.users[2].id, + sharedList: DATA.sharedLists[1].id, + normalizedPageUrl: 'test.com/b', + updatedWhen: 3, + createdWhen: 3, + uploadedWhen: 3, + }) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(ownListIds, { lastSync: 2 })) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual(calcExpectedListEntries(ownListIds)) - }) + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 3 }, + ) - it('should create followedList and followedListEntries based on followed sharedLists and their entries', async () => { - const { backgroundModules, storageManager } = await setupTest({ - testData: { follows: true }, - }) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([ + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][1], + DATA.sharedLists[0].id, + false, + ), + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][0], + DATA.sharedLists[0].id, + true, + ), + createExpectedListEntry( + { + ...DATA.listEntries[DATA.sharedLists[1].id][1], + updatedWhen: 3, + }, + DATA.sharedLists[1].id, + true, + ), + createExpectedListEntry( + { + ...DATA.listEntries[DATA.sharedLists[1].id][0], + updatedWhen: 2, + }, + DATA.sharedLists[1].id, + true, + ), + ]) - const followedListIds = new Set( - DATA.activityFollows.map((follow) => Number(follow.objectId)), - ) + // Now delete them, to ensure it works the other way around + await serverStorage.manager + .collection('sharedAnnotationListEntry') + .deleteObjects({ + sharedList: DATA.sharedLists[1].id, + }) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual([]) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 4 }, + ) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 1 }, - ) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([ + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][1], + DATA.sharedLists[0].id, + false, + ), + createExpectedListEntry( + DATA.listEntries[DATA.sharedLists[0].id][0], + DATA.sharedLists[0].id, + true, + ), + createExpectedListEntry( + { + ...DATA.listEntries[DATA.sharedLists[1].id][1], + updatedWhen: 4, + }, + DATA.sharedLists[1].id, + false, + ), + createExpectedListEntry( + { + ...DATA.listEntries[DATA.sharedLists[1].id][0], + updatedWhen: 4, + }, + DATA.sharedLists[1].id, + false, + ), + ]) + }) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(followedListIds, { lastSync: 1 })) + it('should delete followedList and followedListEntries when a sharedList no longer exists', async () => { + const { + backgroundModules, + storageManager, + getServerStorage, + } = await setupTest({ + testData: { ownLists: true }, + }) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual(calcExpectedListEntries(followedListIds)) - }) + const ownListIds = new Set( + DATA.sharedLists + .filter((list) => list.creator === DATA.userReferenceA.id) + .map((list) => list.id), + ) - it('should delete followedList and followedListEntries when a sharedList is no longer followed', async () => { - const { - backgroundModules, - storageManager, - getServerStorage, - } = await setupTest({ - testData: { follows: true }, - }) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 1 }, + ) - const followedListIds = new Set( - DATA.activityFollows.map((follow) => Number(follow.objectId)), - ) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 1 })) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual([]) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(ownListIds)) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 1 }, - ) + // Delete an owned sharedList so that sync will result in removal of local data + const serverStorage = await getServerStorage() + await serverStorage.manager + .collection('sharedList') + .deleteOneObject({ id: DATA.sharedLists[0].id }) + ownListIds.delete(DATA.sharedLists[0].id) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(followedListIds, { lastSync: 1 })) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual(calcExpectedListEntries(followedListIds)) + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 2 }, + ) - // Delete a follow so that sync will result in removal of local data - const serverStorage = await getServerStorage() - await serverStorage.manager.collection('activityFollow').deleteObjects({ - objectId: Number(DATA.activityFollows[2].objectId), + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 2 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(ownListIds)) }) - followedListIds.delete(Number(DATA.activityFollows[2].objectId)) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 2 }, - ) - - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(followedListIds, { lastSync: 2 })) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual(calcExpectedListEntries(followedListIds)) + it('should create followedList and followedListEntries based on followed sharedLists and their entries', async () => { + const { backgroundModules, storageManager } = await setupTest({ + testData: { follows: true }, + }) - // Do it again to assert idempotency - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 3 }, - ) + const followedListIds = new Set( + DATA.activityFollows.map((follow) => Number(follow.objectId)), + ) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(followedListIds, { lastSync: 3 })) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual(calcExpectedListEntries(followedListIds)) - }) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 1 }, + ) - it('should create followedList and followedListEntries based on both owned and followed sharedLists and their entries', async () => { - const { backgroundModules, storageManager } = await setupTest({ - testData: { follows: true, ownLists: true }, + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(followedListIds, { lastSync: 1 })) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(followedListIds)) }) - const expectedListIds = new Set([ - DATA.sharedLists[0].id, - DATA.sharedLists[1].id, - DATA.sharedLists[3].id, - ]) - - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual([]) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) + it('should delete followedList and followedListEntries when a sharedList is no longer followed', async () => { + const { + backgroundModules, + storageManager, + getServerStorage, + } = await setupTest({ + testData: { follows: true }, + }) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 1 }, - ) + const followedListIds = new Set( + DATA.activityFollows.map((follow) => Number(follow.objectId)), + ) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 1 })) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual(calcExpectedListEntries(expectedListIds)) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 1 }, + ) - // Do it again to assert idempotency - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 2 }, - ) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(followedListIds, { lastSync: 1 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(followedListIds)) + + // Delete a follow so that sync will result in removal of local data + const serverStorage = await getServerStorage() + await serverStorage.manager + .collection('activityFollow') + .deleteObjects({ + objectId: Number(DATA.activityFollows[2].objectId), + }) + followedListIds.delete(Number(DATA.activityFollows[2].objectId)) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 2 }, + ) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 2 })) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual(calcExpectedListEntries(expectedListIds)) - }) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(followedListIds, { lastSync: 2 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(followedListIds)) + + // Do it again to assert idempotency + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 3 }, + ) - it('should delete followedList and followedListEntries when a sharedList is no longer followed and another sharedList no longer exists', async () => { - const { - backgroundModules, - storageManager, - getServerStorage, - } = await setupTest({ - testData: { follows: true, ownLists: true }, + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(followedListIds, { lastSync: 3 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(followedListIds)) }) - const expectedListIds = new Set([ - DATA.sharedLists[0].id, - DATA.sharedLists[1].id, - DATA.sharedLists[3].id, - ]) - - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual([]) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) + it('should create followedList and followedListEntries based on both owned and followed sharedLists and their entries', async () => { + const { backgroundModules, storageManager } = await setupTest({ + testData: { follows: true, ownLists: true }, + }) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 1 }, - ) + const expectedListIds = new Set([ + DATA.sharedLists[0].id, + DATA.sharedLists[1].id, + DATA.sharedLists[3].id, + ]) + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 1 }, + ) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 1 })) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual(calcExpectedListEntries(expectedListIds)) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 1 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(expectedListIds)) + + // Do it again to assert idempotency + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 2 }, + ) - // Delete a follow + owned sharedList so that sync will result in removal of local data - const serverStorage = await getServerStorage() - await serverStorage.manager.collection('activityFollow').deleteObjects({ - objectId: Number(DATA.activityFollows[2].objectId), + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 2 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(expectedListIds)) }) - await serverStorage.manager - .collection('sharedList') - .deleteOneObject({ id: DATA.sharedLists[0].id }) - expectedListIds.delete(Number(DATA.activityFollows[2].objectId)) - expectedListIds.delete(DATA.sharedLists[0].id) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 2 }, - ) + it('should delete followedList and followedListEntries when a sharedList is no longer followed and another sharedList no longer exists', async () => { + const { + backgroundModules, + storageManager, + getServerStorage, + } = await setupTest({ + testData: { follows: true, ownLists: true }, + }) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 2 })) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual(calcExpectedListEntries(expectedListIds)) + const expectedListIds = new Set([ + DATA.sharedLists[0].id, + DATA.sharedLists[1].id, + DATA.sharedLists[3].id, + ]) + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 1 }, + ) - // Do it again to assert idempotency - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 3 }, - ) + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 1 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(expectedListIds)) + + // Delete a follow + owned sharedList so that sync will result in removal of local data + const serverStorage = await getServerStorage() + await serverStorage.manager + .collection('activityFollow') + .deleteObjects({ + objectId: Number(DATA.activityFollows[2].objectId), + }) + await serverStorage.manager + .collection('sharedList') + .deleteOneObject({ id: DATA.sharedLists[0].id }) + expectedListIds.delete(Number(DATA.activityFollows[2].objectId)) + expectedListIds.delete(DATA.sharedLists[0].id) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 3 })) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual(calcExpectedListEntries(expectedListIds)) + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 2 }, + ) + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 2 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(expectedListIds)) + + // Do it again to assert idempotency + await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + { now: 3 }, + ) + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 3 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(expectedListIds)) + }) }) }) diff --git a/src/page-activity-indicator/background/index.ts b/src/page-activity-indicator/background/index.ts index 1fb2681987..e49135bd6b 100644 --- a/src/page-activity-indicator/background/index.ts +++ b/src/page-activity-indicator/background/index.ts @@ -2,11 +2,13 @@ import type { AutoPk } from '@worldbrain/memex-common/lib/storage/types' import type { UserReference } from '@worldbrain/memex-common/lib/web-interface/types/users' import type Storex from '@worldbrain/storex' import type { ServerStorageModules } from 'src/storage/types' -import PageActivityIndicatorStorage from './storage' +import type { RemotePageActivityIndicatorInterface } from './types' import type { SharedList, SharedListReference, } from '@worldbrain/memex-common/lib/content-sharing/types' +import { normalizeUrl } from '@worldbrain/memex-url-utils' +import PageActivityIndicatorStorage from './storage' import { getFollowedListEntryIdentifier, sharedListEntryToFollowedListEntry, @@ -23,11 +25,34 @@ export interface PageActivityIndicatorDependencies { export class PageActivityIndicatorBackground { storage: PageActivityIndicatorStorage + remoteFunctions: RemotePageActivityIndicatorInterface constructor(private deps: PageActivityIndicatorDependencies) { this.storage = new PageActivityIndicatorStorage({ storageManager: deps.storageManager, }) + + this.remoteFunctions = { + getPageActivityStatus: this.getPageActivityStatus, + } + } + + private getPageActivityStatus: RemotePageActivityIndicatorInterface['getPageActivityStatus'] = async ( + fullPageUrl, + ) => { + const normalizedPageUrl = normalizeUrl(fullPageUrl) + const followedListEntries = await this.storage.findFollowedListEntriesByPage( + { normalizedPageUrl }, + ) + if (!followedListEntries.length) { + return 'no-activity' + } + return followedListEntries.reduce( + (acc, curr) => acc || curr.hasAnnotations, + false, + ) + ? 'has-annotations' + : 'no-annotations' } createFollowedList: PageActivityIndicatorStorage['createFollowedList'] = ( diff --git a/src/page-activity-indicator/background/storage.ts b/src/page-activity-indicator/background/storage.ts index c47067d702..2656f72184 100644 --- a/src/page-activity-indicator/background/storage.ts +++ b/src/page-activity-indicator/background/storage.ts @@ -62,6 +62,11 @@ export default class PageActivityIndicatorStorage extends StorageModule { operation: 'findObjects', args: { followedList: '$followedList:number' }, }, + findFollowedListEntriesByPage: { + collection: 'followedListEntry', + operation: 'findObjects', + args: { normalizedPageUrl: '$normalizedPageUrl:string' }, + }, updateFollowedListLastSyncTime: { collection: 'followedList', operation: 'updateObject', @@ -167,6 +172,17 @@ export default class PageActivityIndicatorStorage extends StorageModule { ) } + async findFollowedListEntriesByPage( + data: Pick, + ): Promise { + const followedListEntries: FollowedListEntry[] = await this.operation( + 'findFollowedListEntriesByPage', + { normalizedPageUrl: data.normalizedPageUrl }, + ) + + return followedListEntries + } + async updateFollowedListLastSync( data: Pick, ): Promise { diff --git a/src/page-activity-indicator/background/types.ts b/src/page-activity-indicator/background/types.ts index 5537719910..29667a649d 100644 --- a/src/page-activity-indicator/background/types.ts +++ b/src/page-activity-indicator/background/types.ts @@ -16,3 +16,14 @@ export interface FollowedListEntry { createdWhen: number updatedWhen: number } + +export type PageActivityStatus = + | 'has-annotations' + | 'no-annotations' + | 'no-activity' + +export interface RemotePageActivityIndicatorInterface { + getPageActivityStatus: ( + fullPageUrl: string, + ) => Promise +} diff --git a/src/util/remote-functions-background.ts b/src/util/remote-functions-background.ts index d8f4e637c8..e31bc913c9 100644 --- a/src/util/remote-functions-background.ts +++ b/src/util/remote-functions-background.ts @@ -14,6 +14,7 @@ import { ContentSharingInterface } from 'src/content-sharing/background/types' import type { PDFRemoteInterface } from 'src/pdf/background/types' import type { PersonalCloudRemoteInterface } from 'src/personal-cloud/background/types' import type { AnalyticsInterface } from 'src/analytics/background/types' +import type { RemotePageActivityIndicatorInterface } from 'src/page-activity-indicator/background/types' export interface RemoteFunctionImplementations< Role extends 'provider' | 'caller' @@ -31,6 +32,7 @@ export interface RemoteFunctionImplementations< readablePageArchives: RemoteReaderInterface contentSharing: ContentSharingInterface personalCloud: PersonalCloudRemoteInterface + pageActivityIndicator: RemotePageActivityIndicatorInterface pdf: PDFRemoteInterface } @@ -45,6 +47,7 @@ export const remoteFunctions: RemoteFunctionImplementations<'caller'> = { // features: runInBackground(), featuresBeta: runInBackground(), tags: runInBackground(), + pageActivityIndicator: runInBackground(), collections: runInBackground(), copyPaster: runInBackground(), readablePageArchives: runInBackground(), From 9b9dd50f8a00720498e5e936cf9d042f097679fc Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Fri, 11 Nov 2022 14:27:11 +0700 Subject: [PATCH 15/29] Write PageActivityIndicator component --- external/@worldbrain/memex-common | 2 +- src/page-activity-indicator/ui/indicator.tsx | 109 +++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 src/page-activity-indicator/ui/indicator.tsx diff --git a/external/@worldbrain/memex-common b/external/@worldbrain/memex-common index f67b1543cb..3dc3dcc45f 160000 --- a/external/@worldbrain/memex-common +++ b/external/@worldbrain/memex-common @@ -1 +1 @@ -Subproject commit f67b1543cbb8c222da710cd062902e07c8cdf1c8 +Subproject commit 3dc3dcc45f888b94543d135035450e3bf9654fab diff --git a/src/page-activity-indicator/ui/indicator.tsx b/src/page-activity-indicator/ui/indicator.tsx new file mode 100644 index 0000000000..d3b1d71e13 --- /dev/null +++ b/src/page-activity-indicator/ui/indicator.tsx @@ -0,0 +1,109 @@ +import React from 'react' +import styled from 'styled-components' +import Icon from '@worldbrain/memex-common/lib/common-ui/components/icon' +import { logoNoText } from '@worldbrain/memex-common/lib/common-ui/components/icons' + +export interface Props { + onGoToClick: React.MouseEventHandler + mouseLeaveHideTimeoutMs?: number +} + +interface State { + isShown: boolean + isExpanded: boolean +} + +export default class PageActivityIndicator extends React.PureComponent< + Props, + State +> { + static defaultProps: Pick = { + mouseLeaveHideTimeoutMs: 700, + } + + state: State = { isShown: true, isExpanded: false } + private timeoutId?: number + + private handleMouseEnter: React.MouseEventHandler = (e) => { + clearTimeout(this.timeoutId) + this.setState({ isExpanded: true }) + } + + private handleMouseLeave: React.MouseEventHandler = (e) => { + clearTimeout(this.timeoutId) + this.timeoutId = setTimeout(() => { + this.setState({ isExpanded: false }) + }, this.props.mouseLeaveHideTimeoutMs) + } + + render() { + if (!this.state.isShown) { + return null + } + + return ( + + {this.state.isExpanded ? ( + + this.setState({ isShown: false })} + /> + + Page is annotated + + in Memex Spaces you follow + + + + + ) : ( + + + + )} + + ) + } +} + +const Container = styled.div` + position: fixed; + top: 15px; + right: 15px; + + border-radius: 30px; + padding: 10px; + background-color: #15202b; +` +const MiniContainer = styled.div`` +const ExpandedContainer = styled.div` + display: flex; + flex-direction: row; + justify-content: center; + align-items: center; +` +const TextBox = styled.div` + display: flex; + flex-direction: column; + justify-content: center; + align-items: flex-start; + color: #f3f3f3; +` +const TextMain = styled.span` + font-size: 18px; + font-weight: bold; +` +const TextSecondary = styled.span` + font-size: 16px; +` +const CancelBtn = styled.button`` +const OpenBtn = styled.button`` From 7e65ec190c89ffdcf944b4340f58723a9a03e153 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Fri, 11 Nov 2022 15:12:27 +0700 Subject: [PATCH 16/29] Set up PageActivityIndicator to render w/ ribbon holder - everything works apart from the open sidebar button (sidebar opens but unable to show the shared spaces view) --- src/content-scripts/content_script/global.ts | 24 +++++++--- src/content-scripts/content_script/ribbon.ts | 21 +++++---- src/content-scripts/content_script/sidebar.ts | 9 ++-- .../react/containers/ribbon-holder/index.tsx | 44 ++++++++++++------- .../react/containers/ribbon-holder/logic.ts | 15 ++++++- src/in-page-ui/ribbon/react/index.tsx | 17 +++---- .../shared-state/shared-in-page-ui-state.ts | 4 ++ src/in-page-ui/shared-state/types.ts | 3 ++ .../containers/AnnotationsSidebarInPage.tsx | 7 ++- 9 files changed, 96 insertions(+), 48 deletions(-) diff --git a/src/content-scripts/content_script/global.ts b/src/content-scripts/content_script/global.ts index 09b586b961..94a58ab41d 100644 --- a/src/content-scripts/content_script/global.ts +++ b/src/content-scripts/content_script/global.ts @@ -43,9 +43,10 @@ import { getUnderlyingResourceUrl, isFullUrlPDF } from 'src/util/uri-utils' import { copyPaster, subscription } from 'src/util/remote-functions-background' import { ContentLocatorFormat } from '../../../external/@worldbrain/memex-common/ts/personal-cloud/storage/types' import { setupPdfViewerListeners } from './pdf-detection' -import { RemoteCollectionsInterface } from 'src/custom-lists/background/types' +import type { RemoteCollectionsInterface } from 'src/custom-lists/background/types' import type { RemoteBGScriptInterface } from 'src/background-script/types' import { createSyncSettingsStore } from 'src/sync-settings/util' +import type { RemotePageActivityIndicatorInterface } from 'src/page-activity-indicator/background/types' // import { maybeRenderTutorial } from 'src/in-page-ui/guided-tutorial/content-script' // Content Scripts are separate bundles of javascript code that can be loaded @@ -92,6 +93,9 @@ export async function main( const annotationsBG = runInBackground>() const tagsBG = runInBackground() const collectionsBG = runInBackground() + const pageActivityIndicatorBG = runInBackground< + RemotePageActivityIndicatorInterface + >() const remoteFunctionRegistry = new RemoteFunctionRegistry() const annotationsManager = new AnnotationsManager() const toolbarNotifications = new ToolbarNotifications() @@ -129,8 +133,8 @@ export async function main( delete components[component] }, }) - const pageUrl = await pageInfo.getPageUrl() - const loadAnnotationsPromise = annotationsCache.load(pageUrl) + const fullPageUrl = await pageInfo.getPageUrl() + const loadAnnotationsPromise = annotationsCache.load(fullPageUrl) const annotationFunctionsParams = { inPageUI, @@ -343,9 +347,17 @@ export async function main( } } - const isSidebarEnabled = await sidebarUtils.getSidebarState() - if (isSidebarEnabled && (pageInfo.isPdf ? isPdfViewerRunning : true)) { - await inPageUI.loadComponent('ribbon') + const isSidebarEnabled = + (await sidebarUtils.getSidebarState()) && + (pageInfo.isPdf ? isPdfViewerRunning : true) + const pageActivityStatus = await pageActivityIndicatorBG.getPageActivityStatus( + fullPageUrl, + ) + if (isSidebarEnabled || pageActivityStatus === 'has-annotations') { + await inPageUI.loadComponent('ribbon', { + keepRibbonHidden: !isSidebarEnabled, + showPageActivityIndicator: pageActivityStatus === 'has-annotations', + }) } return inPageUI diff --git a/src/content-scripts/content_script/ribbon.ts b/src/content-scripts/content_script/ribbon.ts index 0a3fab2c34..7e06feb8ad 100644 --- a/src/content-scripts/content_script/ribbon.ts +++ b/src/content-scripts/content_script/ribbon.ts @@ -1,10 +1,11 @@ import browser from 'webextension-polyfill' import { IGNORE_CLICK_OUTSIDE_CLASS } from '../constants' -import { ContentScriptRegistry, RibbonScriptMain } from './types' +import type { ContentScriptRegistry, RibbonScriptMain } from './types' import { setupRibbonUI, destroyRibbonUI } from 'src/in-page-ui/ribbon/react' import { createInPageUI, destroyInPageUI } from 'src/in-page-ui/utils' import { setSidebarState, getSidebarState } from 'src/sidebar-overlay/utils' +import type { ShouldSetUpOptions } from 'src/in-page-ui/shared-state/types' export const main: RibbonScriptMain = async (options) => { const cssFile = browser.runtime.getURL(`/content_script_ribbon.css`) @@ -18,20 +19,23 @@ export const main: RibbonScriptMain = async (options) => { } createMount() - options.inPageUI.events.on('componentShouldSetUp', ({ component }) => { - if (component === 'ribbon') { - setUp() - } - }) + options.inPageUI.events.on( + 'componentShouldSetUp', + ({ component, options }) => { + if (component === 'ribbon') { + setUp(options) + } + }, + ) options.inPageUI.events.on('componentShouldDestroy', ({ component }) => { if (component === 'ribbon') { destroy() } }) - const setUp = async () => { + const setUp = async (setUpOptions: ShouldSetUpOptions = {}) => { createMount() - setupRibbonUI(mount.rootElement, { + setupRibbonUI(mount, { containerDependencies: { ...options, currentTab: (await browser.tabs?.getCurrent()) ?? { @@ -42,6 +46,7 @@ export const main: RibbonScriptMain = async (options) => { getSidebarEnabled: getSidebarState, }, inPageUI: options.inPageUI, + setUpOptions, }) } diff --git a/src/content-scripts/content_script/sidebar.ts b/src/content-scripts/content_script/sidebar.ts index d220d8619c..8a3f9f445b 100644 --- a/src/content-scripts/content_script/sidebar.ts +++ b/src/content-scripts/content_script/sidebar.ts @@ -6,7 +6,8 @@ import { destroyInPageSidebarUI, } from 'src/sidebar/annotations-sidebar/index' import browser from 'webextension-polyfill' -import { InPageUIRootMount } from 'src/in-page-ui/types' +import type { InPageUIRootMount } from 'src/in-page-ui/types' +import type { ShouldSetUpOptions } from 'src/in-page-ui/shared-state/types' export const main: SidebarScriptMain = async (dependencies) => { const cssFile = browser.runtime.getURL(`/content_script_sidebar.css`) @@ -24,7 +25,7 @@ export const main: SidebarScriptMain = async (dependencies) => { 'componentShouldSetUp', ({ component, options }) => { if (component === 'sidebar') { - setUp({ showOnLoad: options.showSidebarOnLoad }) + setUp(options) } }, ) @@ -37,12 +38,12 @@ export const main: SidebarScriptMain = async (dependencies) => { }, ) - const setUp = async (options: { showOnLoad?: boolean } = {}) => { + const setUp = async (options: ShouldSetUpOptions = {}) => { createMount() setupInPageSidebarUI(mount, { ...dependencies, pageUrl: await dependencies.getPageUrl(), - initialState: options.showOnLoad ? 'visible' : 'hidden', + initialState: options.showSidebarOnLoad ? 'visible' : 'hidden', }) } diff --git a/src/in-page-ui/ribbon/react/containers/ribbon-holder/index.tsx b/src/in-page-ui/ribbon/react/containers/ribbon-holder/index.tsx index ea97b2ebab..aa3bb8ea7c 100644 --- a/src/in-page-ui/ribbon/react/containers/ribbon-holder/index.tsx +++ b/src/in-page-ui/ribbon/react/containers/ribbon-holder/index.tsx @@ -9,6 +9,7 @@ import { import { StatefulUIElement } from 'src/util/ui-logic' import RibbonContainer from '../ribbon' import { SharedInPageUIEvents } from 'src/in-page-ui/shared-state/types' +import PageActivityIndicator from 'src/page-activity-indicator/ui/indicator' const styles = require('./styles.css') const RIBBON_HIDE_TIMEOUT = 700 @@ -153,26 +154,37 @@ export default class RibbonHolder extends StatefulUIElement< } showRibbon = () => this.processEvent('show', null) - hideRibbon = () => this.processEvent('hide', null) render() { return ( -
- -
+ <> + {(!this.props.setUpOptions.keepRibbonHidden || + this.state.state === 'visible') && ( +
+ +
+ )} + {this.props.setUpOptions.showPageActivityIndicator && ( + + this.processEvent('openSidebarToSharedSpaces', null) + } + /> + )} + ) } } diff --git a/src/in-page-ui/ribbon/react/containers/ribbon-holder/logic.ts b/src/in-page-ui/ribbon/react/containers/ribbon-holder/logic.ts index ae19d89cf7..6d893f4f6b 100644 --- a/src/in-page-ui/ribbon/react/containers/ribbon-holder/logic.ts +++ b/src/in-page-ui/ribbon/react/containers/ribbon-holder/logic.ts @@ -1,8 +1,9 @@ import { UILogic, UIEvent, UIEventHandler } from 'ui-logic-core' -import { RibbonContainerDependencies } from '../ribbon/types' -import { +import type { RibbonContainerDependencies } from '../ribbon/types' +import type { SharedInPageUIInterface, InPageUIComponentShowState, + ShouldSetUpOptions, } from 'src/in-page-ui/shared-state/types' export interface RibbonHolderState { @@ -11,11 +12,13 @@ export interface RibbonHolderState { } export type RibbonHolderEvents = UIEvent<{ + openSidebarToSharedSpaces: null show: null hide: null }> export interface RibbonHolderDependencies { + setUpOptions: ShouldSetUpOptions inPageUI: SharedInPageUIInterface containerDependencies: RibbonContainerDependencies } @@ -69,6 +72,14 @@ export class RibbonHolderLogic extends UILogic< return { state: { $set: 'hidden' } } } + openSidebarToSharedSpaces: EventHandler< + 'openSidebarToSharedSpaces' + > = async ({}) => { + await this.dependencies.inPageUI.showSidebar({ + action: 'show_shared_spaces', + }) + } + _handleUIStateChange = (event: { newState: InPageUIComponentShowState }) => { diff --git a/src/in-page-ui/ribbon/react/index.tsx b/src/in-page-ui/ribbon/react/index.tsx index 022b5291ea..5db318e4a7 100644 --- a/src/in-page-ui/ribbon/react/index.tsx +++ b/src/in-page-ui/ribbon/react/index.tsx @@ -4,23 +4,20 @@ import { StyleSheetManager, ThemeProvider } from 'styled-components' import { theme } from 'src/common-ui/components/design-library/theme' import RibbonHolder from './containers/ribbon-holder' -import { SharedInPageUIInterface } from 'src/in-page-ui/shared-state/types' -import { RibbonContainerDependencies } from './containers/ribbon/types' +import type { RibbonHolderDependencies } from './containers/ribbon-holder/logic' +import type { InPageUIRootMount } from 'src/in-page-ui/types' export function setupRibbonUI( - target: HTMLElement, - options: { - inPageUI: SharedInPageUIInterface - containerDependencies: RibbonContainerDependencies - }, + mount: InPageUIRootMount, + deps: RibbonHolderDependencies, ) { ReactDOM.render( - + - + , - target, + mount.rootElement, ) } diff --git a/src/in-page-ui/shared-state/shared-in-page-ui-state.ts b/src/in-page-ui/shared-state/shared-in-page-ui-state.ts index 28bdeb6044..d7cc7497c9 100644 --- a/src/in-page-ui/shared-state/shared-in-page-ui-state.ts +++ b/src/in-page-ui/shared-state/shared-in-page-ui-state.ts @@ -148,6 +148,10 @@ export class SharedInPageUIState implements SharedInPageUIInterface { component: InPageUIComponent, options: ShouldSetUpOptions = {}, ) { + // NOTE: The loadComponent call is not async, though if you remove the await then the `componentShouldSetUp` event + // gets sent off too early and the components won't properly receive it. Adding setTimeout(0) to the following call + // doesn't seem to fix it as simply putting await does. + // TODO: fix this - could indicate a deeper timing issue await this.options.loadComponent(component) this._maybeEmitShouldSetUp(component, options) } diff --git a/src/in-page-ui/shared-state/types.ts b/src/in-page-ui/shared-state/types.ts index 1d3d4202a0..624dc5adab 100644 --- a/src/in-page-ui/shared-state/types.ts +++ b/src/in-page-ui/shared-state/types.ts @@ -8,6 +8,7 @@ export type InPageUISidebarAction = | 'edit_annotation_spaces' | 'show_annotation' | 'set_sharing_access' + | 'show_shared_spaces' export type InPageUIRibbonAction = 'comment' | 'tag' | 'list' | 'bookmark' export type InPageUIComponent = 'ribbon' | 'sidebar' | 'tooltip' | 'highlights' export type InPageUIComponentShowState = { @@ -45,7 +46,9 @@ export interface SharedInPageUIEvents { } export interface ShouldSetUpOptions { + keepRibbonHidden?: boolean showSidebarOnLoad?: boolean + showPageActivityIndicator?: boolean } export interface SharedInPageUIInterface { diff --git a/src/sidebar/annotations-sidebar/containers/AnnotationsSidebarInPage.tsx b/src/sidebar/annotations-sidebar/containers/AnnotationsSidebarInPage.tsx index 8669a198ae..d9cd83fa08 100644 --- a/src/sidebar/annotations-sidebar/containers/AnnotationsSidebarInPage.tsx +++ b/src/sidebar/annotations-sidebar/containers/AnnotationsSidebarInPage.tsx @@ -168,7 +168,7 @@ export class AnnotationsSidebarInPage extends AnnotationsSidebarContainer< await (this.logic as SidebarContainerLogic).annotationsLoadComplete if (event.action === 'comment') { - this.processEvent('addNewPageComment', { + await this.processEvent('addNewPageComment', { comment: event.annotationData?.commentText, tags: event.annotationData?.tags, }) @@ -179,9 +179,12 @@ export class AnnotationsSidebarInPage extends AnnotationsSidebarContainer< } else if (event.action === 'edit_annotation_spaces') { this.activateAnnotation(event.annotationUrl, 'edit_spaces') } else if (event.action === 'set_sharing_access') { - this.processEvent('receiveSharingAccessChange', { + await this.processEvent('receiveSharingAccessChange', { sharingAccess: event.annotationSharingAccess, }) + } else if (event.action === 'show_shared_spaces') { + // TODO: Get this working properly opening the shared spaces view + await this.processEvent('expandSharedSpaces', { listIds: [] }) } this.forceUpdate() From c2e96b37773817125bfba3f93f93421d9e56776c Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Mon, 14 Nov 2022 13:25:34 +0700 Subject: [PATCH 17/29] Fix opening sidebar to "Shared Spaces" tab via page activity indicator --- .../components/AnnotationsSidebar.tsx | 13 +++++-------- .../containers/AnnotationsSidebarInPage.tsx | 3 ++- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/sidebar/annotations-sidebar/components/AnnotationsSidebar.tsx b/src/sidebar/annotations-sidebar/components/AnnotationsSidebar.tsx index 21316f0d71..1b064d2e69 100644 --- a/src/sidebar/annotations-sidebar/components/AnnotationsSidebar.tsx +++ b/src/sidebar/annotations-sidebar/components/AnnotationsSidebar.tsx @@ -1062,16 +1062,13 @@ export class AnnotationsSidebar extends React.Component< {' '} - ) : followedLists.allIds.length ? ( - - - {followedLists.allIds.length} - - ) : ( - - 0 + 0} + left="5px" + > + {followedLists.allIds?.length ?? 0} )} diff --git a/src/sidebar/annotations-sidebar/containers/AnnotationsSidebarInPage.tsx b/src/sidebar/annotations-sidebar/containers/AnnotationsSidebarInPage.tsx index d9cd83fa08..b5992f89a5 100644 --- a/src/sidebar/annotations-sidebar/containers/AnnotationsSidebarInPage.tsx +++ b/src/sidebar/annotations-sidebar/containers/AnnotationsSidebarInPage.tsx @@ -183,7 +183,8 @@ export class AnnotationsSidebarInPage extends AnnotationsSidebarContainer< sharingAccess: event.annotationSharingAccess, }) } else if (event.action === 'show_shared_spaces') { - // TODO: Get this working properly opening the shared spaces view + // TODO: Shouldn't need to trigger two events here. Confusing interface + await this.processEvent('expandMyNotes', null) await this.processEvent('expandSharedSpaces', { listIds: [] }) } From 43f33f12b75f68694ea098d32e081943e3d7d017 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Thu, 17 Nov 2022 17:59:44 +0700 Subject: [PATCH 18/29] Fix trans layer tests using dif auth service than test setup - i.e., there was one instance created at translation layer setup logic, then a different one created at the next step down when it runs the setupBackgroundIntegrationTest logic - this meant any interaction on the first didn't change the state of the second instance --- src/personal-cloud/background/index.tests.ts | 1 + src/tests/background-integration-tests.ts | 13 ++++++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/personal-cloud/background/index.tests.ts b/src/personal-cloud/background/index.tests.ts index 084660bb51..39131f228a 100644 --- a/src/personal-cloud/background/index.tests.ts +++ b/src/personal-cloud/background/index.tests.ts @@ -392,6 +392,7 @@ export async function setupSyncBackgroundTest( { ...options, services, + authServices, getServerStorage, pushMessagingService, personalCloudBackend, diff --git a/src/tests/background-integration-tests.ts b/src/tests/background-integration-tests.ts index d40a27de69..88fc85d4cb 100644 --- a/src/tests/background-integration-tests.ts +++ b/src/tests/background-integration-tests.ts @@ -40,7 +40,7 @@ import { } from '@worldbrain/memex-common/lib/personal-cloud/backend/storex' import { STORAGE_VERSIONS } from 'src/storage/constants' import { clearRemotelyCallableFunctions } from 'src/util/webextensionRPC' -import { Services } from 'src/services/types' +import { AuthServices, Services } from 'src/services/types' import { PersonalDeviceType } from '@worldbrain/memex-common/lib/personal-cloud/storage/types' import { JobScheduler } from 'src/job-scheduler/background/job-scheduler' import { createAuthServices } from 'src/services/local-services' @@ -59,6 +59,7 @@ export interface BackgroundIntegrationTestSetupOpts { useDownloadTranslationLayer?: boolean services?: Services pushMessagingService?: MockPushMessagingService + authServices?: AuthServices } export async function setupBackgroundIntegrationTest( @@ -88,10 +89,12 @@ export async function setupBackgroundIntegrationTest( options?.getServerStorage ?? createLazyMemoryServerStorage() const serverStorage = await getServerStorage() - const authServices = createAuthServices({ - backend: 'memory', - getServerStorage, - }) + const authServices = + options?.authServices ?? + createAuthServices({ + backend: 'memory', + getServerStorage, + }) const services = options?.services ?? (await createServices({ From 4b4378e895eee344251dfe9133d891818c0a80d0 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Thu, 17 Nov 2022 18:13:00 +0700 Subject: [PATCH 19/29] Test sync download of followedList creates+deletes --- external/@worldbrain/memex-common | 2 +- .../translation-layer/index.test.data.ts | 41 ++-- .../backend/translation-layer/index.test.ts | 178 ++++++++++++++++-- 3 files changed, 193 insertions(+), 28 deletions(-) diff --git a/external/@worldbrain/memex-common b/external/@worldbrain/memex-common index 661e5ebe82..8f4aea0669 160000 --- a/external/@worldbrain/memex-common +++ b/external/@worldbrain/memex-common @@ -1 +1 @@ -Subproject commit 661e5ebe827a6ee344a6bd7ab204dcf87f1f4cd6 +Subproject commit 8f4aea06695fc93901b85862f65eb6d02ade053b diff --git a/src/personal-cloud/background/backend/translation-layer/index.test.data.ts b/src/personal-cloud/background/backend/translation-layer/index.test.data.ts index 5ca61c8718..00b27895b3 100644 --- a/src/personal-cloud/background/backend/translation-layer/index.test.data.ts +++ b/src/personal-cloud/background/backend/translation-layer/index.test.data.ts @@ -174,6 +174,17 @@ const LOCAL_LISTS_V24 = { }, } +const LOCAL_SHARED_LIST_METADATA_V24 = { + first: { + localId: LOCAL_LISTS_V24.first.id, + remoteId: 'test-1', + }, + second: { + localId: LOCAL_LISTS_V24.second.id, + remoteId: 'test-2', + }, +} + export const LOCAL_TEST_DATA_V24 = { pages: LOCAL_PAGES_V24, bookmarks: { @@ -344,16 +355,7 @@ export const LOCAL_TEST_DATA_V24 = { description: 'test description second list', }, }, - sharedListMetadata: { - first: { - localId: LOCAL_LISTS_V24.first.id, - remoteId: 'test-1', - }, - second: { - localId: LOCAL_LISTS_V24.second.id, - remoteId: 'test-2', - }, - }, + sharedListMetadata: LOCAL_SHARED_LIST_METADATA_V24, pageListEntries: { first: { createdAt: new Date(1625190554480), @@ -392,6 +394,13 @@ export const LOCAL_TEST_DATA_V24 = { createdAt: new Date(1625190554991), }, }, + followedList: { + first: { + creator: TEST_USER.id, + name: LOCAL_LISTS_V24.first.name, + sharedList: LOCAL_SHARED_LIST_METADATA_V24.first.remoteId, + }, + }, templates: { first: { id: 1, @@ -1022,7 +1031,7 @@ export const REMOTE_TEST_DATA_V24 = { first: { id: 1, personalList: REMOTE_LISTS_V24.first.id, - remoteId: LOCAL_TEST_DATA_V24.sharedListMetadata.first.remoteId, + remoteId: LOCAL_SHARED_LIST_METADATA_V24.first.remoteId, user: TEST_USER.id, createdByDevice: REMOTE_DEVICES_V24.first.id, createdWhen: 565, @@ -1031,7 +1040,7 @@ export const REMOTE_TEST_DATA_V24 = { second: { id: 2, personalList: REMOTE_LISTS_V24.second.id, - remoteId: LOCAL_TEST_DATA_V24.sharedListMetadata.second.remoteId, + remoteId: LOCAL_SHARED_LIST_METADATA_V24.second.remoteId, user: TEST_USER.id, createdByDevice: REMOTE_DEVICES_V24.first.id, createdWhen: 565, @@ -1092,6 +1101,14 @@ export const REMOTE_TEST_DATA_V24 = { personalAnnotation: REMOTE_ANNOTATIONS_V24.second.id, }, }, + personalFollowedList: { + first: { + id: 1, + createdByDevice: REMOTE_DEVICES_V24.first.id, + user: TEST_USER.id, + sharedList: LOCAL_SHARED_LIST_METADATA_V24.first.remoteId, + }, + }, personalTextTemplate: { first: { id: 1, diff --git a/src/personal-cloud/background/backend/translation-layer/index.test.ts b/src/personal-cloud/background/backend/translation-layer/index.test.ts index 5d0f33f548..2572f3e98f 100644 --- a/src/personal-cloud/background/backend/translation-layer/index.test.ts +++ b/src/personal-cloud/background/backend/translation-layer/index.test.ts @@ -14,8 +14,6 @@ import { } from './index.test.data' import { DataChangeType, - DataUsageAction, - ContentLocatorFormat, PersonalDeviceType, } from '@worldbrain/memex-common/lib/personal-cloud/storage/types' import { @@ -170,11 +168,17 @@ type DataChange = [ /* info: */ any?, ] +interface DataChangeAssertOpts { + skipChanges?: number + skipAssertTimestamp?: boolean + skipAssertDeviceId?: boolean +} + function dataChanges( remoteData: typeof REMOTE_TEST_DATA_V24, userId: number | string, changes: DataChange[], - options?: { skipChanges?: number; skipAssertTimestamp?: boolean }, + options?: DataChangeAssertOpts, ) { let now = 554 const advance = () => { @@ -197,7 +201,9 @@ function dataChanges( createdWhen: options?.skipAssertTimestamp ? expect.anything() : now, - createdByDevice: remoteData.personalDeviceInfo.first.id, + createdByDevice: options?.skipAssertDeviceId + ? undefined + : remoteData.personalDeviceInfo.first.id, user: userId, type: change[0], collection: change[1], @@ -218,7 +224,7 @@ function blockStats(params: { userId: number | string; usedBlocks: number }) { } } -async function setup(options?: { runReadwiseTrigger?: boolean }) { +async function setup(options?: { withStorageHooks?: boolean }) { const serverIdCapturer = new IdCapturer({ postprocesessMerge: (params) => { // tag connections don't connect with the content they tag through a @@ -261,7 +267,7 @@ async function setup(options?: { runReadwiseTrigger?: boolean }) { getNow, } = await setupSyncBackgroundTest({ deviceCount: 2, - serverChangeWatchSettings: options?.runReadwiseTrigger + serverChangeWatchSettings: options?.withStorageHooks ? storageHooksChangeWatcher : { shouldWatchCollection: (collection) => @@ -272,6 +278,15 @@ async function setup(options?: { runReadwiseTrigger?: boolean }) { }, }) + await setups[0].authService.loginWithEmailAndPassword( + TEST_USER.email, + 'password', + ) + await setups[1].authService.loginWithEmailAndPassword( + TEST_USER.email, + 'password', + ) + let sqlUserId: number | string | undefined if (getSqlStorageMananager) { const initDeps: InitSqlUsageParams = { @@ -305,6 +320,7 @@ async function setup(options?: { runReadwiseTrigger?: boolean }) { return { user: sqlUserId ?? TEST_USER.id } } } + return { serverIdCapturer, setups, @@ -313,7 +329,7 @@ async function setup(options?: { runReadwiseTrigger?: boolean }) { personalDataChanges: ( remoteData: typeof REMOTE_TEST_DATA_V24, changes: DataChange[], - options?: { skipChanges?: number; skipAssertTimestamp?: boolean }, + options?: DataChangeAssertOpts, ) => ({ personalDataChange: dataChanges( remoteData, @@ -4274,7 +4290,7 @@ describe('Personal cloud translation layer', () => { testFetches, testSyncPushTrigger, } = await setup({ - runReadwiseTrigger: true, + withStorageHooks: true, }) testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) @@ -4324,7 +4340,7 @@ describe('Personal cloud translation layer', () => { testFetches, testSyncPushTrigger, } = await setup({ - runReadwiseTrigger: true, + withStorageHooks: true, }) testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) @@ -4372,7 +4388,7 @@ describe('Personal cloud translation layer', () => { testFetches, testSyncPushTrigger, } = await setup({ - runReadwiseTrigger: true, + withStorageHooks: true, }) testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) @@ -4424,7 +4440,7 @@ describe('Personal cloud translation layer', () => { testFetches, testSyncPushTrigger, } = await setup({ - runReadwiseTrigger: true, + withStorageHooks: true, }) testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) @@ -4507,7 +4523,7 @@ describe('Personal cloud translation layer', () => { testFetches, testSyncPushTrigger, } = await setup({ - runReadwiseTrigger: true, + withStorageHooks: true, }) testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) @@ -4564,7 +4580,7 @@ describe('Personal cloud translation layer', () => { testFetches, testSyncPushTrigger, } = await setup({ - runReadwiseTrigger: true, + withStorageHooks: true, }) testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) @@ -4654,7 +4670,7 @@ describe('Personal cloud translation layer', () => { testFetches, testSyncPushTrigger, } = await setup({ - runReadwiseTrigger: true, + withStorageHooks: true, }) testSyncPushTrigger({ wasTriggered: false }) await insertTestPages(setups[0].storageManager) @@ -4742,7 +4758,7 @@ describe('Personal cloud translation layer', () => { testFetches, testSyncPushTrigger, } = await setup({ - runReadwiseTrigger: true, + withStorageHooks: true, }) await insertReadwiseAPIKey(serverStorageManager, TEST_USER.id) const { @@ -5461,5 +5477,137 @@ describe('Personal cloud translation layer', () => { testSyncPushTrigger({ wasTriggered: true }) }) }) + + it('should create followedList next sync download after following a sharedList', async () => { + const { + setups, + serverIdCapturer, + getPersonalWhere, + personalDataChanges, + getDatabaseContents, + testDownload, + testSyncPushTrigger, + } = await setup({ withStorageHooks: true }) + await setups[0].storageManager + .collection('customLists') + .createObject(LOCAL_TEST_DATA_V24.customLists.first) + await setups[0].storageManager + .collection('sharedListMetadata') + .createObject(LOCAL_TEST_DATA_V24.sharedListMetadata.first) + await setups[0].backgroundModules.personalCloud.waitForSync() + + const serverStorage = await setups[0].getServerStorage() + await serverStorage.modules.activityFollows.storeFollow({ + collection: 'sharedList', + userReference: { id: TEST_USER.id, type: 'user-reference' }, + objectId: LOCAL_TEST_DATA_V24.sharedListMetadata.first.remoteId, + }) + + await setups[0].backgroundModules.personalCloud.waitForSync() + const remoteData = serverIdCapturer.mergeIds(REMOTE_TEST_DATA_V24) + const testFollowedLists = remoteData.personalFollowedList + + expect( + await getDatabaseContents(['personalDataChange'], { + getWhere: getPersonalWhere, + }), + ).toEqual({ + ...personalDataChanges( + remoteData, + [ + [ + DataChangeType.Create, + 'personalFollowedList', + testFollowedLists.first.id, + ], + ], + { + skipChanges: 2, + skipAssertDeviceId: true, + skipAssertTimestamp: true, + }, + ), + }) + + // prettier-ignore + await testDownload([ + { type: PersonalCloudUpdateType.Overwrite, collection: 'followedList', object: LOCAL_TEST_DATA_V24.followedList.first }, + ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) + }) + + it('should delete followedList next sync download after unfollowing a sharedList', async () => { + const { + setups, + serverIdCapturer, + getPersonalWhere, + personalDataChanges, + getDatabaseContents, + testDownload, + testSyncPushTrigger, + } = await setup({ withStorageHooks: true }) + await setups[0].storageManager + .collection('customLists') + .createObject(LOCAL_TEST_DATA_V24.customLists.first) + await setups[0].storageManager + .collection('sharedListMetadata') + .createObject(LOCAL_TEST_DATA_V24.sharedListMetadata.first) + await setups[0].backgroundModules.personalCloud.waitForSync() + + const serverStorage = await setups[0].getServerStorage() + await serverStorage.modules.activityFollows.storeFollow({ + collection: 'sharedList', + userReference: { id: TEST_USER.id, type: 'user-reference' }, + objectId: LOCAL_TEST_DATA_V24.sharedListMetadata.first.remoteId, + }) + await setups[0].backgroundModules.personalCloud.waitForSync() + await serverStorage.modules.activityFollows.deleteFollow({ + collection: 'sharedList', + userReference: { id: TEST_USER.id, type: 'user-reference' }, + objectId: LOCAL_TEST_DATA_V24.sharedListMetadata.first.remoteId, + }) + await setups[0].backgroundModules.personalCloud.waitForSync() + + const remoteData = serverIdCapturer.mergeIds(REMOTE_TEST_DATA_V24) + const testFollowedLists = remoteData.personalFollowedList + + expect( + await getDatabaseContents(['personalDataChange'], { + getWhere: getPersonalWhere, + }), + ).toEqual({ + ...personalDataChanges( + remoteData, + [ + [ + DataChangeType.Create, + 'personalFollowedList', + testFollowedLists.first.id, + ], + [ + DataChangeType.Delete, + 'personalFollowedList', + testFollowedLists.first.id, + { + sharedList: + LOCAL_TEST_DATA_V24.sharedListMetadata.first + .remoteId, + }, + ], + ], + { + skipChanges: 2, + skipAssertDeviceId: true, + skipAssertTimestamp: true, + }, + ), + }) + + // prettier-ignore + await testDownload([ + { type: PersonalCloudUpdateType.Delete, collection: 'followedList', where: { sharedList: LOCAL_TEST_DATA_V24.sharedListMetadata.first.remoteId } }, + ], { skip: 2 }) + testSyncPushTrigger({ wasTriggered: true }) + }) }) }) From 4fad565629ba1892304b7a44434440ff436755a6 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Fri, 18 Nov 2022 14:52:38 +0700 Subject: [PATCH 20/29] Remove FCM listeners for page activity indicator --- src/push-messaging/background/index.test.ts | 474 ++++++++++---------- src/push-messaging/background/index.ts | 76 ++-- 2 files changed, 282 insertions(+), 268 deletions(-) diff --git a/src/push-messaging/background/index.test.ts b/src/push-messaging/background/index.test.ts index 0769fe8608..f57fca99a6 100644 --- a/src/push-messaging/background/index.test.ts +++ b/src/push-messaging/background/index.test.ts @@ -58,260 +58,272 @@ describe('Push messaging client tests', () => { expect(hasSyncBeenTriggered).toBe(true) }) - it(`should create/delete followedListEntry records when receiving created/deleted sharedListEntry messages`, async () => { - const { - pushMessagingClient, - defaultListId, - storageManager, - } = await setupTest({ - createDefaultList: true, - createAssocLocalFollowedList: true, - }) - - const expectedEntryA = { - id: expect.any(Number), - creator: TEST_USER.id, - entryTitle: 'test title a', - followedList: defaultListId, - hasAnnotations: false, - normalizedPageUrl: 'test.com/a', - createdWhen: 1, - updatedWhen: 1, - } - const expectedEntryB = { - id: expect.any(Number), - creator: TEST_USER.id, - entryTitle: 'test title b', - followedList: defaultListId, - hasAnnotations: false, - normalizedPageUrl: 'test.com/b', - createdWhen: 2, - updatedWhen: 2, - } - - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) + describe.skip('FCM implementation for page activity indicator', () => { + it(`should create/delete followedListEntry records when receiving created/deleted sharedListEntry messages`, async () => { + const { + pushMessagingClient, + defaultListId, + storageManager, + } = await setupTest({ + createDefaultList: true, + createAssocLocalFollowedList: true, + }) - await pushMessagingClient.handleIncomingMessage( - { - type: 'createPageListEntry', + const expectedEntryA = { + id: expect.any(Number), creator: TEST_USER.id, - sharedList: defaultListId, - entryTitle: expectedEntryA.entryTitle, - normalizedPageUrl: expectedEntryA.normalizedPageUrl, - }, - { now: expectedEntryA.createdWhen }, - ) - - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([expectedEntryA]) - - await pushMessagingClient.handleIncomingMessage( - { - type: 'createPageListEntry', + entryTitle: 'test title a', + followedList: defaultListId, + hasAnnotations: false, + normalizedPageUrl: 'test.com/a', + createdWhen: 1, + updatedWhen: 1, + } + const expectedEntryB = { + id: expect.any(Number), creator: TEST_USER.id, - sharedList: defaultListId, - entryTitle: expectedEntryB.entryTitle, - normalizedPageUrl: expectedEntryB.normalizedPageUrl, - }, - { now: expectedEntryB.createdWhen }, - ) - - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([expectedEntryA, expectedEntryB]) - - await pushMessagingClient.handleIncomingMessage({ - type: 'deletePageListEntry', - sharedList: defaultListId, - normalizedPageUrl: expectedEntryA.normalizedPageUrl, - }) + entryTitle: 'test title b', + followedList: defaultListId, + hasAnnotations: false, + normalizedPageUrl: 'test.com/b', + createdWhen: 2, + updatedWhen: 2, + } - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([expectedEntryB]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) - await pushMessagingClient.handleIncomingMessage({ - type: 'deletePageListEntry', - sharedList: defaultListId, - normalizedPageUrl: expectedEntryB.normalizedPageUrl, - }) + await pushMessagingClient.handleIncomingMessage( + { + type: 'createPageListEntry', + creator: TEST_USER.id, + sharedList: defaultListId, + entryTitle: expectedEntryA.entryTitle, + normalizedPageUrl: expectedEntryA.normalizedPageUrl, + }, + { now: expectedEntryA.createdWhen }, + ) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) - }) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([expectedEntryA]) - it(`should toggle 'hasAnnotations' flag on assoc. followedListEntry when receiving first/last sharedListEntry added/removed messages`, async () => { - const { - pushMessagingClient, - storageManager, - defaultListId, - } = await setupTest({ - createDefaultList: true, - createAssocLocalFollowedList: true, - }) + await pushMessagingClient.handleIncomingMessage( + { + type: 'createPageListEntry', + creator: TEST_USER.id, + sharedList: defaultListId, + entryTitle: expectedEntryB.entryTitle, + normalizedPageUrl: expectedEntryB.normalizedPageUrl, + }, + { now: expectedEntryB.createdWhen }, + ) - const expectedEntryA = { - id: expect.any(Number), - creator: TEST_USER.id, - entryTitle: 'test title a', - followedList: defaultListId, - hasAnnotations: false, - normalizedPageUrl: 'test.com/a', - createdWhen: 1, - updatedWhen: 1, - } + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([expectedEntryA, expectedEntryB]) - // Do create list entry message first, to trigger the creation of a followedListEntry to work with - await pushMessagingClient.handleIncomingMessage( - { - type: 'createPageListEntry', - creator: TEST_USER.id, - sharedList: defaultListId, - entryTitle: expectedEntryA.entryTitle, - normalizedPageUrl: expectedEntryA.normalizedPageUrl, - }, - { now: expectedEntryA.createdWhen }, - ) - - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([expectedEntryA]) - - await pushMessagingClient.handleIncomingMessage( - { - type: 'createFirstAnnotationListEntry', - normalizedPageUrl: expectedEntryA.normalizedPageUrl, - sharedList: defaultListId, - }, - { now: 2 }, - ) - - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([{ ...expectedEntryA, hasAnnotations: true, updatedWhen: 2 }]) - - await pushMessagingClient.handleIncomingMessage( - { - type: 'deleteLastAnnotationListEntry', - normalizedPageUrl: expectedEntryA.normalizedPageUrl, + await pushMessagingClient.handleIncomingMessage({ + type: 'deletePageListEntry', sharedList: defaultListId, - }, - { now: 3 }, - ) - - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([ - { ...expectedEntryA, hasAnnotations: false, updatedWhen: 3 }, - ]) - - await pushMessagingClient.handleIncomingMessage( - { - type: 'createFirstAnnotationListEntry', normalizedPageUrl: expectedEntryA.normalizedPageUrl, - sharedList: defaultListId, - }, - { now: 4 }, - ) - - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([{ ...expectedEntryA, hasAnnotations: true, updatedWhen: 4 }]) - }) + }) - it(`should create/delete followedList and assoc. followedListEntry records when receiving sharedList followed/unfollowed messages`, async () => { - const { - pushMessagingClient, - storageManager, - defaultListId, - } = await setupTest({ - createDefaultList: true, - createAssocLocalFollowedList: false, - }) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([expectedEntryB]) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual([]) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) + await pushMessagingClient.handleIncomingMessage({ + type: 'deletePageListEntry', + sharedList: defaultListId, + normalizedPageUrl: expectedEntryB.normalizedPageUrl, + }) - await pushMessagingClient.handleIncomingMessage({ - type: 'followList', - sharedList: defaultListId, + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) }) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual([ - { - name: defaultListName, - creator: TEST_USER.id, - sharedList: defaultListId, - lastSync: expect.any(Number), - }, - ]) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([ - { + it(`should toggle 'hasAnnotations' flag on assoc. followedListEntry when receiving first/last sharedListEntry added/removed messages`, async () => { + const { + pushMessagingClient, + storageManager, + defaultListId, + } = await setupTest({ + createDefaultList: true, + createAssocLocalFollowedList: true, + }) + + const expectedEntryA = { + id: expect.any(Number), creator: TEST_USER.id, entryTitle: 'test title a', followedList: defaultListId, hasAnnotations: false, normalizedPageUrl: 'test.com/a', - createdWhen: expect.any(Number), - updatedWhen: expect.any(Number), - }, - { - creator: TEST_USER.id, - entryTitle: 'test title b', - followedList: defaultListId, - hasAnnotations: false, - normalizedPageUrl: 'test.com/b', - createdWhen: expect.any(Number), - updatedWhen: expect.any(Number), - }, - ]) + createdWhen: 1, + updatedWhen: 1, + } - await pushMessagingClient.handleIncomingMessage({ - type: 'unfollowList', - sharedList: defaultListId, + // Do create list entry message first, to trigger the creation of a followedListEntry to work with + await pushMessagingClient.handleIncomingMessage( + { + type: 'createPageListEntry', + creator: TEST_USER.id, + sharedList: defaultListId, + entryTitle: expectedEntryA.entryTitle, + normalizedPageUrl: expectedEntryA.normalizedPageUrl, + }, + { now: expectedEntryA.createdWhen }, + ) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([expectedEntryA]) + + await pushMessagingClient.handleIncomingMessage( + { + type: 'createFirstAnnotationListEntry', + normalizedPageUrl: expectedEntryA.normalizedPageUrl, + sharedList: defaultListId, + }, + { now: 2 }, + ) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([ + { ...expectedEntryA, hasAnnotations: true, updatedWhen: 2 }, + ]) + + await pushMessagingClient.handleIncomingMessage( + { + type: 'deleteLastAnnotationListEntry', + normalizedPageUrl: expectedEntryA.normalizedPageUrl, + sharedList: defaultListId, + }, + { now: 3 }, + ) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([ + { ...expectedEntryA, hasAnnotations: false, updatedWhen: 3 }, + ]) + + await pushMessagingClient.handleIncomingMessage( + { + type: 'createFirstAnnotationListEntry', + normalizedPageUrl: expectedEntryA.normalizedPageUrl, + sharedList: defaultListId, + }, + { now: 4 }, + ) + + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([ + { ...expectedEntryA, hasAnnotations: true, updatedWhen: 4 }, + ]) }) - expect( - await storageManager.collection('followedList').findAllObjects({}), - ).toEqual([]) - expect( - await storageManager - .collection('followedListEntry') - .findAllObjects({}), - ).toEqual([]) + it(`should create/delete followedList and assoc. followedListEntry records when receiving sharedList followed/unfollowed messages`, async () => { + const { + pushMessagingClient, + storageManager, + defaultListId, + } = await setupTest({ + createDefaultList: true, + createAssocLocalFollowedList: false, + }) + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await pushMessagingClient.handleIncomingMessage({ + type: 'followList', + sharedList: defaultListId, + }) + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([ + { + name: defaultListName, + creator: TEST_USER.id, + sharedList: defaultListId, + lastSync: expect.any(Number), + }, + ]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([ + { + creator: TEST_USER.id, + entryTitle: 'test title a', + followedList: defaultListId, + hasAnnotations: false, + normalizedPageUrl: 'test.com/a', + createdWhen: expect.any(Number), + updatedWhen: expect.any(Number), + }, + { + creator: TEST_USER.id, + entryTitle: 'test title b', + followedList: defaultListId, + hasAnnotations: false, + normalizedPageUrl: 'test.com/b', + createdWhen: expect.any(Number), + updatedWhen: expect.any(Number), + }, + ]) + + await pushMessagingClient.handleIncomingMessage({ + type: 'unfollowList', + sharedList: defaultListId, + }) + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + }) }) }) diff --git a/src/push-messaging/background/index.ts b/src/push-messaging/background/index.ts index c21d519cf5..bb0976cb25 100644 --- a/src/push-messaging/background/index.ts +++ b/src/push-messaging/background/index.ts @@ -18,43 +18,45 @@ export default class PushMessagingClient { const { bgModules } = this.deps if (payload.type === 'downloadClientUpdates') { bgModules.personalCloud.triggerSyncContinuation() - } else if (payload.type === 'createPageListEntry') { - await bgModules.pageActivityIndicator.createFollowedListEntry({ - creator: payload.creator, - entryTitle: payload.entryTitle, - followedList: payload.sharedList, - normalizedPageUrl: payload.normalizedPageUrl, - createdWhen: opts?.now, - updatedWhen: opts?.now, - }) - } else if (payload.type === 'deletePageListEntry') { - await bgModules.pageActivityIndicator.deleteFollowedListEntry({ - normalizedPageUrl: payload.normalizedPageUrl, - followedList: payload.sharedList, - }) - } else if (payload.type === 'createFirstAnnotationListEntry') { - await bgModules.pageActivityIndicator.updateFollowedListEntryHasAnnotations( - { - normalizedPageUrl: payload.normalizedPageUrl, - followedList: payload.sharedList, - updatedWhen: opts?.now, - hasAnnotations: true, - }, - ) - } else if (payload.type === 'deleteLastAnnotationListEntry') { - await bgModules.pageActivityIndicator.updateFollowedListEntryHasAnnotations( - { - normalizedPageUrl: payload.normalizedPageUrl, - followedList: payload.sharedList, - updatedWhen: opts?.now, - hasAnnotations: false, - }, - ) - } else if (payload.type === 'followList') { - } else if (payload.type === 'unfollowList') { - await bgModules.pageActivityIndicator.deleteFollowedListAndAllEntries( - { sharedList: payload.sharedList }, - ) } + // This is the setup for the old FCM-based implementation for page activity indicator + // } else if (payload.type === 'createPageListEntry') { + // await bgModules.pageActivityIndicator.createFollowedListEntry({ + // creator: payload.creator, + // entryTitle: payload.entryTitle, + // followedList: payload.sharedList, + // normalizedPageUrl: payload.normalizedPageUrl, + // createdWhen: opts?.now, + // updatedWhen: opts?.now, + // }) + // } else if (payload.type === 'deletePageListEntry') { + // await bgModules.pageActivityIndicator.deleteFollowedListEntry({ + // normalizedPageUrl: payload.normalizedPageUrl, + // followedList: payload.sharedList, + // }) + // } else if (payload.type === 'createFirstAnnotationListEntry') { + // await bgModules.pageActivityIndicator.updateFollowedListEntryHasAnnotations( + // { + // normalizedPageUrl: payload.normalizedPageUrl, + // followedList: payload.sharedList, + // updatedWhen: opts?.now, + // hasAnnotations: true, + // }, + // ) + // } else if (payload.type === 'deleteLastAnnotationListEntry') { + // await bgModules.pageActivityIndicator.updateFollowedListEntryHasAnnotations( + // { + // normalizedPageUrl: payload.normalizedPageUrl, + // followedList: payload.sharedList, + // updatedWhen: opts?.now, + // hasAnnotations: false, + // }, + // ) + // } else if (payload.type === 'followList') { + // } else if (payload.type === 'unfollowList') { + // await bgModules.pageActivityIndicator.deleteFollowedListAndAllEntries( + // { sharedList: payload.sharedList }, + // ) + // } } } From 269364809eb53659da6f3f93c9255c72fffbc839 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Mon, 21 Nov 2022 13:11:50 +0700 Subject: [PATCH 21/29] Split followedList pull sync into 2 parts: lists and entries - lists are now synced via the cloud sync, so we can avoid reading sharedLists from the server during this pull sync - though we still need the ability to do it for special cases (new user log in, feature release) - list entry pull sync, which reads remote entries for the local followedLists, will be the one that gets CRON'd --- .../background/index.test.ts | 213 ++++++++++++++---- .../background/index.ts | 56 +++-- 2 files changed, 199 insertions(+), 70 deletions(-) diff --git a/src/page-activity-indicator/background/index.test.ts b/src/page-activity-indicator/background/index.test.ts index b12310a017..ce9e49a876 100644 --- a/src/page-activity-indicator/background/index.test.ts +++ b/src/page-activity-indicator/background/index.test.ts @@ -152,9 +152,27 @@ describe('Page activity indicator background module tests', () => { 'getPageActivityStatus' ]('https://test.com/c'), ).toEqual('no-activity') - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 1 }, - ) + + await backgroundModules.pageActivityIndicator.syncFollowedLists() + expect( + await backgroundModules.pageActivityIndicator[ + 'getPageActivityStatus' + ]('https://test.com/a'), + ).toEqual('no-activity') + expect( + await backgroundModules.pageActivityIndicator[ + 'getPageActivityStatus' + ]('https://test.com/b'), + ).toEqual('no-activity') + expect( + await backgroundModules.pageActivityIndicator[ + 'getPageActivityStatus' + ]('https://test.com/c'), + ).toEqual('no-activity') + + await backgroundModules.pageActivityIndicator.syncFollowedListEntries({ + now: 1, + }) expect( await backgroundModules.pageActivityIndicator[ 'getPageActivityStatus' @@ -176,9 +194,9 @@ describe('Page activity indicator background module tests', () => { .collection('sharedAnnotationListEntry') .deleteObjects({}) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 2 }, - ) + await backgroundModules.pageActivityIndicator.syncFollowedListEntries({ + now: 2, + }) expect( await backgroundModules.pageActivityIndicator[ 'getPageActivityStatus' @@ -203,9 +221,9 @@ describe('Page activity indicator background module tests', () => { normalizedPageUrl: 'test.com/b', }) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 2 }, - ) + await backgroundModules.pageActivityIndicator.syncFollowedListEntries({ + now: 2, + }) expect( await backgroundModules.pageActivityIndicator[ 'getPageActivityStatus' @@ -241,7 +259,7 @@ describe('Page activity indicator background module tests', () => { .findAllObjects({}), ).toEqual([]) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 1 }, ) @@ -271,7 +289,7 @@ describe('Page activity indicator background module tests', () => { .findAllObjects({}), ).toEqual([]) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 1 }, ) @@ -309,7 +327,20 @@ describe('Page activity indicator background module tests', () => { .findAllObjects({}), ).toEqual([]) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedLists() + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds, { lastSync: undefined })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 1 }, ) @@ -325,7 +356,8 @@ describe('Page activity indicator background module tests', () => { ).toEqual(calcExpectedListEntries(ownListIds)) // Do it again to assert idempotency - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedLists() + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 2 }, ) @@ -367,7 +399,20 @@ describe('Page activity indicator background module tests', () => { .findAllObjects({}), ).toEqual([]) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedLists() + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds, { lastSync: undefined })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 1 }, ) @@ -412,7 +457,7 @@ describe('Page activity indicator background module tests', () => { .createObject(entry) } - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 2 }, ) @@ -472,7 +517,20 @@ describe('Page activity indicator background module tests', () => { .findAllObjects({}), ).toEqual([]) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedLists() + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds, { lastSync: undefined })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 1 }, ) @@ -521,7 +579,7 @@ describe('Page activity indicator background module tests', () => { uploadedWhen: 1, }) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 2 }, ) @@ -572,7 +630,7 @@ describe('Page activity indicator background module tests', () => { uploadedWhen: 3, }) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 3 }, ) @@ -616,7 +674,7 @@ describe('Page activity indicator background module tests', () => { sharedList: DATA.sharedLists[1].id, }) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 4 }, ) @@ -680,7 +738,20 @@ describe('Page activity indicator background module tests', () => { .findAllObjects({}), ).toEqual([]) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedLists() + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(ownListIds, { lastSync: undefined })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 1 }, ) @@ -689,7 +760,6 @@ describe('Page activity indicator background module tests', () => { .collection('followedList') .findAllObjects({}), ).toEqual(calcExpectedLists(ownListIds, { lastSync: 1 })) - expect( await storageManager .collection('followedListEntry') @@ -703,15 +773,13 @@ describe('Page activity indicator background module tests', () => { .deleteOneObject({ id: DATA.sharedLists[0].id }) ownListIds.delete(DATA.sharedLists[0].id) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 2 }, - ) + await backgroundModules.pageActivityIndicator.syncFollowedLists() expect( await storageManager .collection('followedList') .findAllObjects({}), - ).toEqual(calcExpectedLists(ownListIds, { lastSync: 2 })) + ).toEqual(calcExpectedLists(ownListIds, { lastSync: 1 })) expect( await storageManager .collection('followedListEntry') @@ -739,7 +807,21 @@ describe('Page activity indicator background module tests', () => { .findAllObjects({}), ).toEqual([]) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedLists() + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual( + calcExpectedLists(followedListIds, { lastSync: undefined }), + ) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 1 }, ) @@ -748,7 +830,6 @@ describe('Page activity indicator background module tests', () => { .collection('followedList') .findAllObjects({}), ).toEqual(calcExpectedLists(followedListIds, { lastSync: 1 })) - expect( await storageManager .collection('followedListEntry') @@ -780,7 +861,21 @@ describe('Page activity indicator background module tests', () => { .findAllObjects({}), ).toEqual([]) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedLists() + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual( + calcExpectedLists(followedListIds, { lastSync: undefined }), + ) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 1 }, ) @@ -804,15 +899,13 @@ describe('Page activity indicator background module tests', () => { }) followedListIds.delete(Number(DATA.activityFollows[2].objectId)) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 2 }, - ) + await backgroundModules.pageActivityIndicator.syncFollowedLists() expect( await storageManager .collection('followedList') .findAllObjects({}), - ).toEqual(calcExpectedLists(followedListIds, { lastSync: 2 })) + ).toEqual(calcExpectedLists(followedListIds, { lastSync: 1 })) expect( await storageManager .collection('followedListEntry') @@ -820,15 +913,16 @@ describe('Page activity indicator background module tests', () => { ).toEqual(calcExpectedListEntries(followedListIds)) // Do it again to assert idempotency - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 3 }, + await backgroundModules.pageActivityIndicator.syncFollowedLists() + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( + { now: 2 }, ) expect( await storageManager .collection('followedList') .findAllObjects({}), - ).toEqual(calcExpectedLists(followedListIds, { lastSync: 3 })) + ).toEqual(calcExpectedLists(followedListIds, { lastSync: 2 })) expect( await storageManager .collection('followedListEntry') @@ -858,7 +952,22 @@ describe('Page activity indicator background module tests', () => { .findAllObjects({}), ).toEqual([]) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedLists() + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual( + calcExpectedLists(expectedListIds, { lastSync: undefined }), + ) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 1 }, ) @@ -874,7 +983,8 @@ describe('Page activity indicator background module tests', () => { ).toEqual(calcExpectedListEntries(expectedListIds)) // Do it again to assert idempotency - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedLists() + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 2 }, ) @@ -916,7 +1026,22 @@ describe('Page activity indicator background module tests', () => { .findAllObjects({}), ).toEqual([]) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( + await backgroundModules.pageActivityIndicator.syncFollowedLists() + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual( + calcExpectedLists(expectedListIds, { lastSync: undefined }), + ) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 1 }, ) @@ -944,15 +1069,13 @@ describe('Page activity indicator background module tests', () => { expectedListIds.delete(Number(DATA.activityFollows[2].objectId)) expectedListIds.delete(DATA.sharedLists[0].id) - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 2 }, - ) + await backgroundModules.pageActivityIndicator.syncFollowedLists() expect( await storageManager .collection('followedList') .findAllObjects({}), - ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 2 })) + ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 1 })) expect( await storageManager .collection('followedListEntry') @@ -960,15 +1083,13 @@ describe('Page activity indicator background module tests', () => { ).toEqual(calcExpectedListEntries(expectedListIds)) // Do it again to assert idempotency - await backgroundModules.pageActivityIndicator.syncFollowedListsAndEntries( - { now: 3 }, - ) + await backgroundModules.pageActivityIndicator.syncFollowedLists() expect( await storageManager .collection('followedList') .findAllObjects({}), - ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 3 })) + ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 1 })) expect( await storageManager .collection('followedListEntry') diff --git a/src/page-activity-indicator/background/index.ts b/src/page-activity-indicator/background/index.ts index e49135bd6b..99a15a65bc 100644 --- a/src/page-activity-indicator/background/index.ts +++ b/src/page-activity-indicator/background/index.ts @@ -2,7 +2,10 @@ import type { AutoPk } from '@worldbrain/memex-common/lib/storage/types' import type { UserReference } from '@worldbrain/memex-common/lib/web-interface/types/users' import type Storex from '@worldbrain/storex' import type { ServerStorageModules } from 'src/storage/types' -import type { RemotePageActivityIndicatorInterface } from './types' +import type { + FollowedList, + RemotePageActivityIndicatorInterface, +} from './types' import type { SharedList, SharedListReference, @@ -110,21 +113,18 @@ export class PageActivityIndicatorBackground { ] } - async syncFollowedListsAndEntries(opts?: { now?: number }): Promise { + async syncFollowedLists(): Promise { const userId = await this.deps.getCurrentUserId() if (userId == null) { return } - const now = opts?.now ?? Date.now() - const { contentSharing } = await this.deps.getServerStorage() - const sharedLists = await this.getAllUserFollowedSharedListsFromServer({ id: userId, type: 'user-reference', }) const existingFollowedListsLookup = await this.storage.findAllFollowedLists() - // Do a run over the existing followedLists, to remove any that no longer have assoc. sharedLists existing + // Remove any local followedLists that don't have an associated remote sharedList (carry over from old implementation, b) for (const followedList of existingFollowedListsLookup.values()) { if ( !sharedLists.find((list) => list.id === followedList.sharedList) @@ -136,22 +136,38 @@ export class PageActivityIndicatorBackground { } for (const sharedList of sharedLists) { + if (!existingFollowedListsLookup.get(sharedList.id)) { + await this.storage.createFollowedList( + sharedListToFollowedList(sharedList), + ) + } + } + } + + async syncFollowedListEntries(opts?: { now?: number }): Promise { + const userId = await this.deps.getCurrentUserId() + if (userId == null) { + return + } + const now = opts?.now ?? Date.now() + const { contentSharing } = await this.deps.getServerStorage() + + const existingFollowedListsLookup = await this.storage.findAllFollowedLists() + + for (const followedList of existingFollowedListsLookup.values()) { const listReference: SharedListReference = { type: 'shared-list-reference', - id: sharedList.id, + id: followedList.sharedList, } const existingFollowedListEntryLookup = await this.storage.findAllFollowedListEntries( { - sharedList: sharedList.id, + sharedList: followedList.sharedList, }, ) - const localFollowedList = existingFollowedListsLookup.get( - sharedList.id, - ) const sharedListEntries = await contentSharing.getListEntriesByList( { listReference, - from: localFollowedList?.lastSync, + from: followedList.lastSync, }, ) const sharedAnnotationListEntries = await contentSharing.getAnnotationListEntries( @@ -244,18 +260,10 @@ export class PageActivityIndicatorBackground { } } - if (localFollowedList == null) { - await this.storage.createFollowedList( - sharedListToFollowedList(sharedList, { - lastSync: now, - }), - ) - } else { - await this.storage.updateFollowedListLastSync({ - sharedList: sharedList.id, - lastSync: now, - }) - } + await this.storage.updateFollowedListLastSync({ + sharedList: followedList.sharedList, + lastSync: now, + }) } } } From fd603b4e7b2f13a8cdccb05086a3d7d75429ab32 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Mon, 21 Nov 2022 13:28:02 +0700 Subject: [PATCH 22/29] Add ability to remove all followedList data - will be used on login change --- .../background/index.test.ts | 52 +++++++++++++++++++ .../background/index.ts | 2 + .../background/storage.ts | 15 ++++++ 3 files changed, 69 insertions(+) diff --git a/src/page-activity-indicator/background/index.test.ts b/src/page-activity-indicator/background/index.test.ts index ce9e49a876..504ab6fe52 100644 --- a/src/page-activity-indicator/background/index.test.ts +++ b/src/page-activity-indicator/background/index.test.ts @@ -1096,5 +1096,57 @@ describe('Page activity indicator background module tests', () => { .findAllObjects({}), ).toEqual(calcExpectedListEntries(expectedListIds)) }) + + it('should be able to delete all followedList and followedListEntries in one swoop', async () => { + const { backgroundModules, storageManager } = await setupTest({ + testData: { follows: true, ownLists: true }, + }) + + const expectedListIds = new Set([ + DATA.sharedLists[0].id, + DATA.sharedLists[1].id, + DATA.sharedLists[3].id, + ]) + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedLists() + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( + { now: 1 }, + ) + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 1 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(expectedListIds)) + + await backgroundModules.pageActivityIndicator.deleteAllFollowedListsData() + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + }) }) }) diff --git a/src/page-activity-indicator/background/index.ts b/src/page-activity-indicator/background/index.ts index 99a15a65bc..cc2151b467 100644 --- a/src/page-activity-indicator/background/index.ts +++ b/src/page-activity-indicator/background/index.ts @@ -73,6 +73,8 @@ export class PageActivityIndicatorBackground { deleteFollowedListAndAllEntries: PageActivityIndicatorStorage['deleteFollowedListAndAllEntries'] = ( data, ) => this.storage.deleteFollowedListAndAllEntries(data) + deleteAllFollowedListsData: PageActivityIndicatorStorage['deleteAllFollowedListsData'] = () => + this.storage.deleteAllFollowedListsData() private async getAllUserFollowedSharedListsFromServer( userReference: UserReference, diff --git a/src/page-activity-indicator/background/storage.ts b/src/page-activity-indicator/background/storage.ts index 2656f72184..410df2ed46 100644 --- a/src/page-activity-indicator/background/storage.ts +++ b/src/page-activity-indicator/background/storage.ts @@ -89,6 +89,16 @@ export default class PageActivityIndicatorStorage extends StorageModule { }, ], }, + deleteAllFollowedLists: { + collection: 'followedList', + operation: 'deleteObjects', + args: {}, + }, + deleteAllFollowedListEntries: { + collection: 'followedListEntry', + operation: 'deleteObjects', + args: {}, + }, deleteFollowedList: { collection: 'followedList', operation: 'deleteObject', @@ -226,4 +236,9 @@ export default class PageActivityIndicatorStorage extends StorageModule { followedList: data.sharedList, }) } + + async deleteAllFollowedListsData(): Promise { + await this.operation('deleteAllFollowedLists', {}) + await this.operation('deleteAllFollowedListEntries', {}) + } } From 9faeead9fbe531e794b01bca35ae29ddedfa662b Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Mon, 21 Nov 2022 13:50:54 +0700 Subject: [PATCH 23/29] Add QnD migration to trigger init followedList sync on next release --- src/background-script/index.ts | 1 + .../quick-and-dirty-migrations/index.ts | 12 +++++++++++- src/background-script/setup.ts | 13 ++++++++----- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/background-script/index.ts b/src/background-script/index.ts index dd1f9f5022..dbe1e129a1 100644 --- a/src/background-script/index.ts +++ b/src/background-script/index.ts @@ -46,6 +46,7 @@ interface Dependencies { storageManager: Storex bgModules: Pick< BackgroundModules, + | 'pageActivityIndicator' | 'tabManagement' | 'notifications' | 'copyPaster' diff --git a/src/background-script/quick-and-dirty-migrations/index.ts b/src/background-script/quick-and-dirty-migrations/index.ts index b89cdbfd2a..cb79eed939 100644 --- a/src/background-script/quick-and-dirty-migrations/index.ts +++ b/src/background-script/quick-and-dirty-migrations/index.ts @@ -30,7 +30,7 @@ export interface MigrationProps { localStorage: Storage.LocalStorageArea bgModules: Pick< BackgroundModules, - 'readwise' | 'syncSettings' | 'personalCloud' + 'readwise' | 'syncSettings' | 'personalCloud' | 'pageActivityIndicator' > localExtSettingStore: SettingStore syncSettingsStore: SyncSettingsStore<'extension'> @@ -49,6 +49,16 @@ export const MIGRATION_PREFIX = '@QnDMigration-' // __IMPORTANT NOTE__ export const migrations: Migrations = { + /* + * This exists to seed initial local followedList collection data based on remote data, for the + * release of the page-activity indicator feature. Future changes to this collection should be + * handled by cloud sync. + */ + [MIGRATION_PREFIX + 'trigger-init-followed-list-pull-sync']: async ({ + bgModules, + }) => { + await bgModules.pageActivityIndicator.syncFollowedLists() + }, [MIGRATION_PREFIX + 'migrate-tags-to-spaces']: async ({ bgModules: { personalCloud }, syncSettingsStore, diff --git a/src/background-script/setup.ts b/src/background-script/setup.ts index 8b98e1f213..2b93540d02 100644 --- a/src/background-script/setup.ts +++ b/src/background-script/setup.ts @@ -415,6 +415,12 @@ export function createBackgroundModules(options: { prefix: 'personalCloud.', }) + const pageActivityIndicator = new PageActivityIndicatorBackground({ + storageManager, + getCurrentUserId, + getServerStorage, + }) + const personalCloud: PersonalCloudBackground = new PersonalCloudBackground({ storageManager, syncSettingsStore, @@ -588,6 +594,7 @@ export function createBackgroundModules(options: { tabManagement, personalCloud, notifications, + pageActivityIndicator, }, }) @@ -604,11 +611,7 @@ export function createBackgroundModules(options: { search, eventLog: new EventLogBackground({ storageManager }), activityIndicator, - pageActivityIndicator: new PageActivityIndicatorBackground({ - storageManager, - getCurrentUserId, - getServerStorage, - }), + pageActivityIndicator, customLists, tags, bookmarks, From 2f843bcf0943b87c11bfc09d6cddf440f406d85e Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Thu, 24 Nov 2022 15:21:15 +0700 Subject: [PATCH 24/29] Add JobScheduler method to add raw Alarm - no funny business - JobScheduler's a mess --- src/job-scheduler/background/job-scheduler.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/job-scheduler/background/job-scheduler.ts b/src/job-scheduler/background/job-scheduler.ts index 0662649091..a2903b4859 100644 --- a/src/job-scheduler/background/job-scheduler.ts +++ b/src/job-scheduler/background/job-scheduler.ts @@ -27,7 +27,7 @@ export class JobScheduler { } private static calcTimeFromNow = (minutes: number, now = Date.now()) => - typeof minutes === 'undefined' ? minutes : now + minutes * 60 * 1000 + minutes == null ? minutes : now + minutes * 60 * 1000 private setTimeoutKey = (key: string, value: number) => this.props.storageAPI.local.set({ @@ -68,7 +68,9 @@ export class JobScheduler { ) { const timeToRun = await this.getTimeoutKey(name) - if (timeToRun < now) { + if (timeToRun === JobScheduler.NOT_SET) { + await job() + } else if (timeToRun < now) { await job() await this.setTimeoutKey( name, @@ -112,6 +114,16 @@ export class JobScheduler { } } + async scheduleJob(job: JobDefinition) { + this.jobs.set(job.name, job) + + this.props.alarmsAPI.create(job.name, { + when: job.when, + delayInMinutes: job.delayInMinutes, + periodInMinutes: job.periodInMinutes, + }) + } + // Schedule all periodic ping attempts at a random minute past the hour, every hour async scheduleJobHourly(job: JobDefinition) { this.jobs.set(job.name, job) From 2d83694f4c4670551a4ffbb89116f6a7ccd13899 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Fri, 25 Nov 2022 12:24:17 +0700 Subject: [PATCH 25/29] Set up periodic followedList sync via cloudflare timestamps --- external/@worldbrain/memex-common | 2 +- src/background-script/setup.ts | 3 + .../background/index.test.ts | 201 +++++++++++++++++- .../background/index.ts | 99 ++++++++- 4 files changed, 294 insertions(+), 11 deletions(-) diff --git a/external/@worldbrain/memex-common b/external/@worldbrain/memex-common index 8f4aea0669..cfbe06d7e7 160000 --- a/external/@worldbrain/memex-common +++ b/external/@worldbrain/memex-common @@ -1 +1 @@ -Subproject commit 8f4aea06695fc93901b85862f65eb6d02ade053b +Subproject commit cfbe06d7e76c721f88f266d6f11f4bf8e2cebacb diff --git a/src/background-script/setup.ts b/src/background-script/setup.ts index 2b93540d02..02416badbf 100644 --- a/src/background-script/setup.ts +++ b/src/background-script/setup.ts @@ -416,9 +416,11 @@ export function createBackgroundModules(options: { }) const pageActivityIndicator = new PageActivityIndicatorBackground({ + fetch, storageManager, getCurrentUserId, getServerStorage, + jobScheduler: jobScheduler.scheduler, }) const personalCloud: PersonalCloudBackground = new PersonalCloudBackground({ @@ -765,6 +767,7 @@ export async function setupBackgroundModules( // await backgroundModules.pdfBg.setupRequestInterceptors() await backgroundModules.analytics.setup() await backgroundModules.jobScheduler.setup() + await backgroundModules.pageActivityIndicator.setup() // Ensure log-in state gotten from FB + trigger share queue processing, but don't wait for it await backgroundModules.auth.authService.refreshUserInfo() diff --git a/src/page-activity-indicator/background/index.test.ts b/src/page-activity-indicator/background/index.test.ts index 504ab6fe52..f65f7f3795 100644 --- a/src/page-activity-indicator/background/index.test.ts +++ b/src/page-activity-indicator/background/index.test.ts @@ -1,5 +1,6 @@ import type { AutoPk } from '@worldbrain/memex-common/lib/storage/types' import { TEST_USER } from '@worldbrain/memex-common/lib/authentication/dev' +import { LIST_TIMESTAMP_WORKER_URLS } from '@worldbrain/memex-common/lib/content-sharing/storage/constants' import { setupBackgroundIntegrationTest } from 'src/tests/background-integration-tests' import * as DATA from './index.test.data' import { @@ -7,6 +8,7 @@ import { sharedListToFollowedList, } from './utils' import type { FollowedListEntry } from './types' +import { PERIODIC_SYNC_JOB_NAME } from '.' const calcExpectedLists = ( expectedListIds: Set, @@ -130,6 +132,26 @@ async function setupTest(opts: { } describe('Page activity indicator background module tests', () => { + it('should schedule periodic sync list entries alarm', async () => { + const { backgroundModules } = await setupTest({}) + + expect(backgroundModules.jobScheduler.scheduler['jobs'].size).toBe(0) + await backgroundModules.pageActivityIndicator.setup() + expect(backgroundModules.jobScheduler.scheduler['jobs'].size).toBe(1) + expect( + backgroundModules.jobScheduler.scheduler['jobs'].get( + PERIODIC_SYNC_JOB_NAME, + ), + ).toEqual({ + name: PERIODIC_SYNC_JOB_NAME, + periodInMinutes: 15, + job: + backgroundModules.pageActivityIndicator[ + 'syncFollowedListEntriesWithNewActivity' + ], + }) + }) + it('should be able to derive page activity status from stored followedLists data', async () => { const { backgroundModules, getServerStorage } = await setupTest({ testData: { follows: true, ownLists: true }, @@ -241,6 +263,52 @@ describe('Page activity indicator background module tests', () => { ).toEqual('no-activity') }) + it('should be able to delete all followedList and followedListEntries in one swoop', async () => { + const { backgroundModules, storageManager } = await setupTest({ + testData: { follows: true, ownLists: true }, + }) + + const expectedListIds = new Set([ + DATA.sharedLists[0].id, + DATA.sharedLists[1].id, + DATA.sharedLists[3].id, + ]) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedLists() + await backgroundModules.pageActivityIndicator.syncFollowedListEntries({ + now: 1, + }) + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 1 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(expectedListIds)) + + await backgroundModules.pageActivityIndicator.deleteAllFollowedListsData() + + expect( + await storageManager.collection('followedList').findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + }) + describe('pull sync followed lists tests', () => { it('should do nothing when not logged in', async () => { const { backgroundModules, storageManager } = await setupTest({ @@ -1097,15 +1165,19 @@ describe('Page activity indicator background module tests', () => { ).toEqual(calcExpectedListEntries(expectedListIds)) }) - it('should be able to delete all followedList and followedListEntries in one swoop', async () => { - const { backgroundModules, storageManager } = await setupTest({ - testData: { follows: true, ownLists: true }, + it('should be able to sync new entries of only those followed lists which have had activity since last sync', async () => { + const { + backgroundModules, + storageManager, + getServerStorage, + fetch, + } = await setupTest({ + testData: { ownLists: true }, }) const expectedListIds = new Set([ DATA.sharedLists[0].id, DATA.sharedLists[1].id, - DATA.sharedLists[3].id, ]) expect( @@ -1120,6 +1192,20 @@ describe('Page activity indicator background module tests', () => { ).toEqual([]) await backgroundModules.pageActivityIndicator.syncFollowedLists() + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual( + calcExpectedLists(expectedListIds, { lastSync: undefined }), + ) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + await backgroundModules.pageActivityIndicator.syncFollowedListEntries( { now: 1 }, ) @@ -1135,18 +1221,119 @@ describe('Page activity indicator background module tests', () => { .findAllObjects({}), ).toEqual(calcExpectedListEntries(expectedListIds)) - await backgroundModules.pageActivityIndicator.deleteAllFollowedListsData() + // Add newer entry for one list + mock out Cloudflare fetch to return newer activity timestamp for that list + const serverStorage = await getServerStorage() + const newSharedListEntry = { + creator: DATA.users[1].id, + sharedList: DATA.sharedLists[0].id, + normalizedUrl: 'test.com/c', + originalUrl: 'https://test.com/c', + createdWhen: 2, + updatedWhen: 2, + } + await serverStorage.manager + .collection('sharedListEntry') + .createObject(newSharedListEntry) + fetch.mock('*', 200, { + response: [[DATA.sharedLists[0].id.toString(), 2]], + sendAsJson: true, + }) + + expect(fetch.calls()).toEqual([]) + await backgroundModules.pageActivityIndicator[ + 'syncFollowedListEntriesWithNewActivity' + ]({ now: 3 }) + + expect(fetch.calls().length).toBe(1) + // TODO: This is the actual req. It does show up, but jest assert fails saying "Received: serializes to the same string" + // .toEqual([ + // LIST_TIMESTAMP_WORKER_URLS.staging + '/', + // { + // method: 'POST', + // body: JSON.stringify({ + // sharedListIds: [...expectedListIds].map((id) => + // id.toString(), + // ), + // }), + // }, + // ], + // ]) + + // Assert only one list had lastSync updated expect( await storageManager .collection('followedList') .findAllObjects({}), - ).toEqual([]) + ).toEqual([ + sharedListToFollowedList(DATA.sharedLists[0], { lastSync: 3 }), + sharedListToFollowedList(DATA.sharedLists[1], { lastSync: 1 }), + ]) expect( await storageManager .collection('followedListEntry') .findAllObjects({}), - ).toEqual([]) + ).toEqual( + calcExpectedListEntries(expectedListIds, { + extraEntries: [ + sharedListEntryToFollowedListEntry(newSharedListEntry, { + id: expect.any(Number), + hasAnnotations: false, + }), + ], + }), + ) + + // Let's add an annotation to that new entry, to assert it also can update from just the annotation change + await serverStorage.manager + .collection('sharedAnnotationListEntry') + .createObject({ + creator: DATA.users[1].id, + sharedList: DATA.sharedLists[0].id, + normalizedPageUrl: 'test.com/c', + updatedWhen: 4, + createdWhen: 4, + uploadedWhen: 4, + }) + fetch.mock('*', 200, { + response: [[DATA.sharedLists[0].id.toString(), 4]], + overwriteRoutes: true, + sendAsJson: true, + }) + + expect(fetch.calls().length).toBe(1) + + await backgroundModules.pageActivityIndicator[ + 'syncFollowedListEntriesWithNewActivity' + ]({ now: 5 }) + + expect(fetch.calls().length).toBe(2) + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([ + sharedListToFollowedList(DATA.sharedLists[0], { lastSync: 5 }), + sharedListToFollowedList(DATA.sharedLists[1], { lastSync: 1 }), + ]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual( + calcExpectedListEntries(expectedListIds, { + extraEntries: [ + sharedListEntryToFollowedListEntry( + { ...newSharedListEntry, updatedWhen: 5 }, + { + id: expect.any(Number), + hasAnnotations: true, + }, + ), + ], + }), + ) }) }) }) diff --git a/src/page-activity-indicator/background/index.ts b/src/page-activity-indicator/background/index.ts index cc2151b467..4d5c41f1a8 100644 --- a/src/page-activity-indicator/background/index.ts +++ b/src/page-activity-indicator/background/index.ts @@ -1,6 +1,7 @@ import type { AutoPk } from '@worldbrain/memex-common/lib/storage/types' import type { UserReference } from '@worldbrain/memex-common/lib/web-interface/types/users' import type Storex from '@worldbrain/storex' +import * as Raven from 'src/util/raven' import type { ServerStorageModules } from 'src/storage/types' import type { FollowedList, @@ -17,15 +18,25 @@ import { sharedListEntryToFollowedListEntry, sharedListToFollowedList, } from './utils' +import type { JobScheduler } from 'src/job-scheduler/background/job-scheduler' +import { LIST_TIMESTAMP_WORKER_URLS } from '@worldbrain/memex-common/lib/content-sharing/storage/constants' +import type { + SharedListTimestamp, + SharedListTimestampGetRequest, +} from '@worldbrain/memex-common/lib/page-activity-indicator/backend/types' export interface PageActivityIndicatorDependencies { + fetch: typeof fetch storageManager: Storex + jobScheduler: JobScheduler getCurrentUserId: () => Promise getServerStorage: () => Promise< Pick > } +export const PERIODIC_SYNC_JOB_NAME = 'followed-list-entry-sync' + export class PageActivityIndicatorBackground { storage: PageActivityIndicatorStorage remoteFunctions: RemotePageActivityIndicatorInterface @@ -40,6 +51,82 @@ export class PageActivityIndicatorBackground { } } + async setup(): Promise { + await this.deps.jobScheduler.scheduleJob({ + name: PERIODIC_SYNC_JOB_NAME, + periodInMinutes: 15, + job: this.syncFollowedListEntriesWithNewActivity, + }) + } + + private syncFollowedListEntriesWithNewActivity = async (opts?: { + now?: number + }) => { + const existingFollowedListsLookup = await this.storage.findAllFollowedLists() + if (!existingFollowedListsLookup.size) { + return + } + + const workerUrl = + process.env.NODE_ENV === 'production' + ? LIST_TIMESTAMP_WORKER_URLS.production + : LIST_TIMESTAMP_WORKER_URLS.staging + const requestBody: SharedListTimestampGetRequest = { + sharedListIds: [...existingFollowedListsLookup.keys()].map((id) => + id.toString(), + ), + } + const response = await this.deps.fetch(workerUrl, { + method: 'POST', + body: JSON.stringify(requestBody), + }) + + if (!response.ok) { + Raven.captureException( + new Error( + `Could not reach Cloudflare worker to check sharedLists' timestamp - response text: ${await response.text()}`, + ), + ) + return + } + + const activityTimestamps: SharedListTimestamp[] = await response.json() + if (!Array.isArray(activityTimestamps)) { + Raven.captureException( + new Error( + `Received unexpected response data from Cloudflare worker - data: ${activityTimestamps}`, + ), + ) + return + } + + if (!activityTimestamps.length) { + return + } + + // Filter out lists which do have updates + const listActivityTimestampLookup = new Map(activityTimestamps) + const followedListsWithUpdates: FollowedList[] = [] + for (const [ + sharedListId, + followedList, + ] of existingFollowedListsLookup) { + if ( + listActivityTimestampLookup.get(sharedListId.toString()) > + followedList.lastSync + ) { + followedListsWithUpdates.push(followedList) + } + } + + if (followedListsWithUpdates.length > 0) { + await this.syncFollowedListEntries({ + forFollowedLists: followedListsWithUpdates, + now: opts?.now, + }) + } + } + private getPageActivityStatus: RemotePageActivityIndicatorInterface['getPageActivityStatus'] = async ( fullPageUrl, ) => { @@ -146,7 +233,11 @@ export class PageActivityIndicatorBackground { } } - async syncFollowedListEntries(opts?: { now?: number }): Promise { + async syncFollowedListEntries(opts?: { + now?: number + /** If defined, will constrain the sync to only these followedLists. Else will sync all. */ + forFollowedLists?: FollowedList[] + }): Promise { const userId = await this.deps.getCurrentUserId() if (userId == null) { return @@ -154,9 +245,11 @@ export class PageActivityIndicatorBackground { const now = opts?.now ?? Date.now() const { contentSharing } = await this.deps.getServerStorage() - const existingFollowedListsLookup = await this.storage.findAllFollowedLists() + const followedLists = + opts?.forFollowedLists ?? + (await this.storage.findAllFollowedLists()).values() - for (const followedList of existingFollowedListsLookup.values()) { + for (const followedList of followedLists) { const listReference: SharedListReference = { type: 'shared-list-reference', id: followedList.sharedList, From 76fb80ba4cacd96a8bf8ffef2fc034855f481e61 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Wed, 30 Nov 2022 16:10:31 +0700 Subject: [PATCH 26/29] Properly fetch the correct worker endpoint --- external/@worldbrain/memex-common | 2 +- src/page-activity-indicator/background/index.ts | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/external/@worldbrain/memex-common b/external/@worldbrain/memex-common index 9a3cd25f44..8163a1b47a 160000 --- a/external/@worldbrain/memex-common +++ b/external/@worldbrain/memex-common @@ -1 +1 @@ -Subproject commit 9a3cd25f44ef881f2c2079b9414510612293f654 +Subproject commit 8163a1b47a7f877db6a8be7c57d012d4fb7b31d1 diff --git a/src/page-activity-indicator/background/index.ts b/src/page-activity-indicator/background/index.ts index 4d5c41f1a8..6e484f0e71 100644 --- a/src/page-activity-indicator/background/index.ts +++ b/src/page-activity-indicator/background/index.ts @@ -24,6 +24,7 @@ import type { SharedListTimestamp, SharedListTimestampGetRequest, } from '@worldbrain/memex-common/lib/page-activity-indicator/backend/types' +import { SHARED_LIST_TIMESTAMP_GET_ROUTE } from '@worldbrain/memex-common/lib/page-activity-indicator/backend/constants' export interface PageActivityIndicatorDependencies { fetch: typeof fetch @@ -76,10 +77,13 @@ export class PageActivityIndicatorBackground { id.toString(), ), } - const response = await this.deps.fetch(workerUrl, { - method: 'POST', - body: JSON.stringify(requestBody), - }) + const response = await this.deps.fetch( + workerUrl + SHARED_LIST_TIMESTAMP_GET_ROUTE, + { + method: 'POST', + body: JSON.stringify(requestBody), + }, + ) if (!response.ok) { Raven.captureException( From 96bc25383a5eabb09a3ee7d9f5012c00489fa95d Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Thu, 1 Dec 2022 15:11:35 +0700 Subject: [PATCH 27/29] Allow syncing of entries for newly synced followedLists - one obv case I missed earlier --- package.json | 3 +- .../background/index.test.ts | 64 +++++++++++++++++++ .../background/index.ts | 3 +- 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 73b9ba0213..e789993672 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "watch:prod": "npm run build:clean && webpack --watch --env.prod", "watch:notif": "webpack --watch --env.notifs", "watch:clean": "npm run cache:clean && npm run watch:notif", + "watch:clean:mv2": "npm run cache:clean && npm run watch:notif:mv2", "cache:clean": "rimraf ./node_modules/.cache", "stats": "webpack --profile -1json > stats.json", "postgres:start": "yarn postgres stop; sudo docker run --name storex-test-postgres -p 127.0.0.1:5435:5432/tcp -e POSTGRES_PASSWORD=storex -d postgres", @@ -309,4 +310,4 @@ }, "main": "index.js", "author": "Oliver Sauter " -} \ No newline at end of file +} diff --git a/src/page-activity-indicator/background/index.test.ts b/src/page-activity-indicator/background/index.test.ts index f65f7f3795..131422b052 100644 --- a/src/page-activity-indicator/background/index.test.ts +++ b/src/page-activity-indicator/background/index.test.ts @@ -1335,5 +1335,69 @@ describe('Page activity indicator background module tests', () => { }), ) }) + + it("should be able to sync entries for newly created followed lists which haven't been synced yet", async () => { + const { + backgroundModules, + storageManager, + getServerStorage, + fetch, + } = await setupTest({ + testData: { ownLists: true }, + }) + + const expectedListIds = new Set([ + DATA.sharedLists[0].id, + DATA.sharedLists[1].id, + ]) + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual([]) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + await backgroundModules.pageActivityIndicator.syncFollowedLists() + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual( + calcExpectedLists(expectedListIds, { lastSync: undefined }), + ) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual([]) + + fetch.mock('*', 200, { + response: [[...expectedListIds].map((id) => [id, 2])], + sendAsJson: true, + }) + + expect(fetch.calls()).toEqual([]) + await backgroundModules.pageActivityIndicator[ + 'syncFollowedListEntriesWithNewActivity' + ]({ now: 1 }) + expect(fetch.calls().length).toBe(1) + + expect( + await storageManager + .collection('followedList') + .findAllObjects({}), + ).toEqual(calcExpectedLists(expectedListIds, { lastSync: 1 })) + expect( + await storageManager + .collection('followedListEntry') + .findAllObjects({}), + ).toEqual(calcExpectedListEntries(expectedListIds)) + }) }) }) diff --git a/src/page-activity-indicator/background/index.ts b/src/page-activity-indicator/background/index.ts index 6e484f0e71..4d77f011de 100644 --- a/src/page-activity-indicator/background/index.ts +++ b/src/page-activity-indicator/background/index.ts @@ -116,8 +116,9 @@ export class PageActivityIndicatorBackground { followedList, ] of existingFollowedListsLookup) { if ( + followedList.lastSync == null || listActivityTimestampLookup.get(sharedListId.toString()) > - followedList.lastSync + followedList.lastSync ) { followedListsWithUpdates.push(followedList) } From ce9eaaffbd1c9e4e9feee83edee61be3aa204a65 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Thu, 1 Dec 2022 15:38:36 +0700 Subject: [PATCH 28/29] Remove relationship on local followedListEntry collection def - kept post-fixing relation field with "-Rel" and I couldn't figure out how to overwrite that behavior again. Felt it make more sense to just remove it and focus on more important tasks --- src/page-activity-indicator/background/storage.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/page-activity-indicator/background/storage.ts b/src/page-activity-indicator/background/storage.ts index 410df2ed46..68db95cc16 100644 --- a/src/page-activity-indicator/background/storage.ts +++ b/src/page-activity-indicator/background/storage.ts @@ -31,6 +31,7 @@ export default class PageActivityIndicatorStorage extends StorageModule { fields: { creator: { type: 'string' }, entryTitle: { type: 'text' }, + followedList: { type: 'string' }, normalizedPageUrl: { type: 'string' }, hasAnnotations: { type: 'boolean' }, createdWhen: { type: 'timestamp' }, @@ -40,7 +41,6 @@ export default class PageActivityIndicatorStorage extends StorageModule { { field: 'normalizedPageUrl' }, { field: 'followedList' }, ], - relationships: [{ childOf: 'followedList' }], }, }, operations: { @@ -60,7 +60,7 @@ export default class PageActivityIndicatorStorage extends StorageModule { findFollowedListEntries: { collection: 'followedListEntry', operation: 'findObjects', - args: { followedList: '$followedList:number' }, + args: { followedList: '$followedList:string' }, }, findFollowedListEntriesByPage: { collection: 'followedListEntry', From 7d33667c50692d727f149f3287f3b8e72513fcd8 Mon Sep 17 00:00:00 2001 From: Jonathan Poltak Samosir Date: Fri, 2 Dec 2022 15:18:18 +0700 Subject: [PATCH 29/29] Update memex-common --- external/@worldbrain/memex-common | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/@worldbrain/memex-common b/external/@worldbrain/memex-common index 8163a1b47a..2981709e0e 160000 --- a/external/@worldbrain/memex-common +++ b/external/@worldbrain/memex-common @@ -1 +1 @@ -Subproject commit 8163a1b47a7f877db6a8be7c57d012d4fb7b31d1 +Subproject commit 2981709e0eb327f6ae06f1ac053913c39fed7632