From 696bb03e6e04beed7fbfea0c141a02c9dc8ec87d Mon Sep 17 00:00:00 2001 From: radex Date: Mon, 16 Dec 2019 18:00:06 +0100 Subject: [PATCH 01/17] Speculative unsubscribed checks (I caught SOMETHING in SharedSubscribable invariant) --- src/observation/subscribeToQueryReloading/index.js | 2 +- src/observation/subscribeToSimpleQuery/index.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/observation/subscribeToQueryReloading/index.js b/src/observation/subscribeToQueryReloading/index.js index 6836be439..5692af50d 100644 --- a/src/observation/subscribeToQueryReloading/index.js +++ b/src/observation/subscribeToQueryReloading/index.js @@ -24,7 +24,7 @@ export default function subscribeToQueryReloading( function reloadingObserverFetch(): void { if (shouldEmitStatus) { - subscriber((false: any)) + !unsubscribed && subscriber((false: any)) } collection._fetchQuery(query, result => { diff --git a/src/observation/subscribeToSimpleQuery/index.js b/src/observation/subscribeToSimpleQuery/index.js index 8d81bd700..7b356e461 100644 --- a/src/observation/subscribeToSimpleQuery/index.js +++ b/src/observation/subscribeToSimpleQuery/index.js @@ -74,7 +74,7 @@ export default function subscribeToSimpleQuery( // Send initial matching records const matchingRecords: Record[] = initialRecords - const emitCopy = () => subscriber(matchingRecords.slice(0)) + const emitCopy = () => !unsubscribed && subscriber(matchingRecords.slice(0)) emitCopy() // Observe changes to the collection From f7f7453f7587707f70e3fcb089ba099afd5687cc Mon Sep 17 00:00:00 2001 From: radex Date: Tue, 17 Dec 2019 11:21:33 +0100 Subject: [PATCH 02/17] CR tweaks --- src/adapters/lokijs/worker/lokiWorker.js | 45 +++++++++++-------- .../subscribeToQueryReloading/index.js | 2 +- .../subscribeToSimpleQuery/index.js | 2 +- 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/adapters/lokijs/worker/lokiWorker.js b/src/adapters/lokijs/worker/lokiWorker.js index 0fa1298c8..1f4d0fac2 100644 --- a/src/adapters/lokijs/worker/lokiWorker.js +++ b/src/adapters/lokijs/worker/lokiWorker.js @@ -5,7 +5,14 @@ import logError from '../../../utils/common/logError' import invariant from '../../../utils/common/invariant' import LokiExecutor from './executor' -import { actions, type WorkerAction, type WorkerResponse } from '../common' +import { + actions, + type WorkerAction, + type WorkerResponse, + type WorkerExecutorType, + type WorkerExecutorPayload, + type WorkerResponseData, +} from '../common' const ExecutorProto = LokiExecutor.prototype const executorMethods = { @@ -79,18 +86,11 @@ export default class LokiWorker { if (type === actions.SETUP || type === actions.UNSAFE_RESET_DATABASE) { this.processActionAsync(action) } else { - // run action - invariant(this.executor, `Cannot run actions because executor is not set up`) - - const runExecutorAction = executorMethods[type].bind(this.executor) - const response = runExecutorAction(...payload) - + const response = this._runExecutorAction(type, payload) this.onActionDone({ id, result: { value: response } }) } } catch (error) { - // Main process only receives error message — this logError is to retain call stack - logError(error) - this.onActionDone({ id: action.id, result: { error } }) + this._onError(action, error) } } @@ -110,18 +110,25 @@ export default class LokiWorker { this.onActionDone({ id, result: { value: null } }) } else { - // run action - invariant(this.executor, `Cannot run actions because executor is not set up`) - - const runExecutorAction = executorMethods[type].bind(this.executor) - const response = await runExecutorAction(...payload) - + const response = await this._runExecutorAction(type, payload) this.onActionDone({ id, result: { value: response } }) } } catch (error) { - // Main process only receives error message — this logError is to retain call stack - logError(error) - this.onActionDone({ id: action.id, result: { error } }) + this._onError(action, error) } } + + _runExecutorAction(type: WorkerExecutorType, payload: WorkerExecutorPayload): WorkerResponseData { + // run action + invariant(this.executor, `Cannot run actions because executor is not set up`) + + const runExecutorAction = executorMethods[type].bind(this.executor) + return runExecutorAction(...payload) + } + + _onError(action: WorkerAction, error: any): void { + // Main process only receives error message (when using web workers) — this logError is to retain call stack + logError(error) + this.onActionDone({ id: action.id, result: { error } }) + } } diff --git a/src/observation/subscribeToQueryReloading/index.js b/src/observation/subscribeToQueryReloading/index.js index 5692af50d..1cccfafaa 100644 --- a/src/observation/subscribeToQueryReloading/index.js +++ b/src/observation/subscribeToQueryReloading/index.js @@ -28,7 +28,7 @@ export default function subscribeToQueryReloading( } collection._fetchQuery(query, result => { - if (!result.value) { + if (result.error) { logError(result.error.toString()) return } diff --git a/src/observation/subscribeToSimpleQuery/index.js b/src/observation/subscribeToSimpleQuery/index.js index 7b356e461..c4e289e6e 100644 --- a/src/observation/subscribeToSimpleQuery/index.js +++ b/src/observation/subscribeToSimpleQuery/index.js @@ -65,7 +65,7 @@ export default function subscribeToSimpleQuery( return } - if (!result.value) { + if (result.error) { logError(result.error.toString()) return } From c7a358f512bf2c1645781e3c57d60c958b2aa600 Mon Sep 17 00:00:00 2001 From: radex Date: Mon, 13 Jan 2020 12:35:38 +0100 Subject: [PATCH 03/17] v0.16.0-8 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 571fa8309..848f67ce0 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@nozbe/watermelondb", "description": "Build powerful React Native and React web apps that scale from hundreds to tens of thousands of records and remain fast", - "version": "0.16.0-7", + "version": "0.16.0-8", "scripts": { "build": "NODE_ENV=production node ./scripts/make.js", "dev": "NODE_ENV=development node ./scripts/make.js", From 619fbe427823f80d343b67fd33a89804611260bc Mon Sep 17 00:00:00 2001 From: radex Date: Mon, 20 Jan 2020 11:59:04 +0100 Subject: [PATCH 04/17] [Loki] Don't silently fail IDBChecker --- src/adapters/lokijs/worker/lokiExtensions.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/adapters/lokijs/worker/lokiExtensions.js b/src/adapters/lokijs/worker/lokiExtensions.js index b612dd5e0..948ad93d7 100644 --- a/src/adapters/lokijs/worker/lokiExtensions.js +++ b/src/adapters/lokijs/worker/lokiExtensions.js @@ -2,6 +2,7 @@ /* eslint-disable no-undef */ import Loki, { LokiMemoryAdapter } from 'lokijs' +import { logger } from '../../../utils/common' const isIDBAvailable = () => { return new Promise(resolve => { @@ -17,12 +18,15 @@ const isIDBAvailable = () => { db.close() resolve(true) } - checkRequest.onerror = () => { + checkRequest.onerror = e => { + logger.error( + '[WatermelonDB] IndexedDB checker failed to open. Most likely, user is in Private Mode. It could also be a quota exceeded error. Will fall back to in-memory database.', + e, + ) resolve(false) } checkRequest.onblocked = () => { - // eslint-disable-next-line no-console - console.error('WatermelonIDBChecker call is blocked') + logger.error('[WatermelonDB] IndexedDB checker call is blocked') } }) } @@ -31,7 +35,7 @@ async function getLokiAdapter( name: ?string, adapter: ?LokiMemoryAdapter, useIncrementalIDB: boolean, - onIndexedDBVersionChange: ?(() => void), + onIndexedDBVersionChange: ?() => void, ): mixed { if (adapter) { return adapter @@ -55,7 +59,7 @@ export async function newLoki( name: ?string, adapter: ?LokiMemoryAdapter, useIncrementalIDB: boolean, - onIndexedDBVersionChange: ?(() => void), + onIndexedDBVersionChange: ?() => void, ): Loki { const loki = new Loki(name, { adapter: await getLokiAdapter(name, adapter, useIncrementalIDB, onIndexedDBVersionChange), From 055e60b5325f6f2eaa3ccb02bedca8ffd059464a Mon Sep 17 00:00:00 2001 From: radex Date: Mon, 20 Jan 2020 11:59:17 +0100 Subject: [PATCH 05/17] Clean up logger calls a little --- src/Database/ActionQueue.js | 6 +++--- src/adapters/common.js | 2 +- src/adapters/lokijs/worker/executor.js | 30 +++++++++++++++----------- src/adapters/sqlite/index.js | 20 ++++++++++------- 4 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/Database/ActionQueue.js b/src/Database/ActionQueue.js index 347a70eb8..131370148 100644 --- a/src/Database/ActionQueue.js +++ b/src/Database/ActionQueue.js @@ -34,14 +34,14 @@ export default class ActionQueue implements ActionInterface { const queue = this._queue const current = queue[0] logger.warn( - `The action you're trying to perform (${description || + `[WatermelonDB] The action you're trying to perform (${description || 'unnamed'}) can't be performed yet, because there are ${ queue.length } actions in the queue. Current action: ${current.description || 'unnamed'}. Ignore this message if everything is working fine. But if your actions are not running, it's because the current action is stuck. Remember that if you're calling an action from an action, you must use subAction(). See docs for more details.`, ) - logger.log(`Enqueued action:`, work) - logger.log(`Running action:`, current.work) + logger.log(`[WatermelonDB] Enqueued action:`, work) + logger.log(`[WatermelonDB] Running action:`, current.work) } this._queue.push({ work, resolve, reject, description }) diff --git a/src/adapters/common.js b/src/adapters/common.js index 3b62eb53c..530365785 100644 --- a/src/adapters/common.js +++ b/src/adapters/common.js @@ -57,7 +57,7 @@ export function sanitizeQueryResult( export function devSetupCallback(result: Result): void { if (result.error) { logger.error( - `[DB] Uh-oh. Database failed to load, we're in big trouble. This might happen if you didn't set up native code correctly (iOS, Android), or if you didn't recompile native app after WatermelonDB update. It might also mean that IndexedDB or SQLite refused to open.`, + `[WatermelonDB] Uh-oh. Database failed to load, we're in big trouble. This might happen if you didn't set up native code correctly (iOS, Android), or if you didn't recompile native app after WatermelonDB update. It might also mean that IndexedDB or SQLite refused to open.`, result.error, ) } diff --git a/src/adapters/lokijs/worker/executor.js b/src/adapters/lokijs/worker/executor.js index 76bc042cd..e678690e5 100644 --- a/src/adapters/lokijs/worker/executor.js +++ b/src/adapters/lokijs/worker/executor.js @@ -35,7 +35,7 @@ export default class LokiExecutor { experimentalUseIncrementalIndexedDB: boolean - onIndexedDBVersionChange: ?(() => void) + onIndexedDBVersionChange: ?() => void _testLokiAdapter: ?LokiMemoryAdapter @@ -197,7 +197,7 @@ export default class LokiExecutor { await deleteDatabase(this.loki) this.cachedRecords.clear() - logger.log('[DB][Worker] Database is now reset') + logger.log('[WatermelonDB][Loki] Database is now reset') await this._openDatabase() this._setUpSchema() @@ -233,7 +233,7 @@ export default class LokiExecutor { // *** Internals *** async _openDatabase(): Promise { - logger.log('[DB][Worker] Initializing IndexedDB') + logger.log('[WatermelonDB][Loki] Initializing IndexedDB') this.loki = await newLoki( this.dbName, @@ -242,11 +242,11 @@ export default class LokiExecutor { this.onIndexedDBVersionChange, ) - logger.log('[DB][Worker] Database loaded') + logger.log('[WatermelonDB][Loki] Database loaded') } _setUpSchema(): void { - logger.log('[DB][Worker] Setting up schema') + logger.log('[WatermelonDB][Loki] Setting up schema') // Add collections values(this.schema.tables).forEach(tableSchema => { @@ -262,7 +262,7 @@ export default class LokiExecutor { // Set database version this._databaseVersion = this.schema.version - logger.log('[DB][Worker] Database collections set up') + logger.log('[WatermelonDB][Loki] Database collections set up') } _addCollection(tableSchema: TableSchema): void { @@ -295,28 +295,32 @@ export default class LokiExecutor { if (dbVersion === schemaVersion) { // All good! } else if (dbVersion === 0) { - logger.log('[DB][Worker] Empty database, setting up') + logger.log('[WatermelonDB][Loki] Empty database, setting up') await this.unsafeResetDatabase() } else if (dbVersion > 0 && dbVersion < schemaVersion) { - logger.log('[DB][Worker] Database has old schema version. Migration is required.') + logger.log('[WatermelonDB][Loki] Database has old schema version. Migration is required.') const migrationSteps = this._getMigrationSteps(dbVersion) if (migrationSteps) { - logger.log(`[DB][Worker] Migrating from version ${dbVersion} to ${this.schema.version}...`) + logger.log( + `[WatermelonDB][Loki] Migrating from version ${dbVersion} to ${this.schema.version}...`, + ) try { await this._migrate(migrationSteps) } catch (error) { - logger.error('[DB][Worker] Migration failed', error) + logger.error('[WatermelonDB][Loki] Migration failed', error) throw error } } else { logger.warn( - '[DB][Worker] Migrations not available for this version range, resetting database instead', + '[WatermelonDB][Loki] Migrations not available for this version range, resetting database instead', ) await this.unsafeResetDatabase() } } else { - logger.warn('[DB][Worker] Database has newer version than app schema. Resetting database.') + logger.warn( + '[WatermelonDB][Loki] Database has newer version than app schema. Resetting database.', + ) await this.unsafeResetDatabase() } } @@ -349,7 +353,7 @@ export default class LokiExecutor { // Set database version this._databaseVersion = this.schema.version - logger.log(`[DB][Worker] Migration successful`) + logger.log(`[WatermelonDB][Loki] Migration successful`) } _executeCreateTableMigration({ schema }: CreateTableMigrationStep): void { diff --git a/src/adapters/sqlite/index.js b/src/adapters/sqlite/index.js index fcf21a105..e2e6e13bb 100644 --- a/src/adapters/sqlite/index.js +++ b/src/adapters/sqlite/index.js @@ -282,13 +282,15 @@ export default class SQLiteAdapter implements DatabaseAdapter, SQLDatabaseAdapte } async _setUpWithMigrations(databaseVersion: SchemaVersion): Promise { - logger.log('[DB] Database needs migrations') + logger.log('[WatermelonDB][SQLite] Database needs migrations') invariant(databaseVersion > 0, 'Invalid database schema version') const migrationSteps = this._migrationSteps(databaseVersion) if (migrationSteps) { - logger.log(`[DB] Migrating from version ${databaseVersion} to ${this.schema.version}...`) + logger.log( + `[WatermelonDB][SQLite] Migrating from version ${databaseVersion} to ${this.schema.version}...`, + ) try { await toPromise(callback => @@ -301,21 +303,23 @@ export default class SQLiteAdapter implements DatabaseAdapter, SQLDatabaseAdapte callback, ), ) - logger.log('[DB] Migration successful') + logger.log('[WatermelonDB][SQLite] Migration successful') } catch (error) { - logger.error('[DB] Migration failed', error) + logger.error('[WatermelonDB][SQLite] Migration failed', error) throw error } } else { logger.warn( - '[DB] Migrations not available for this version range, resetting database instead', + '[WatermelonDB][SQLite] Migrations not available for this version range, resetting database instead', ) await this._setUpWithSchema() } } async _setUpWithSchema(): Promise { - logger.log(`[DB] Setting up database with schema version ${this.schema.version}`) + logger.log( + `[WatermelonDB][SQLite] Setting up database with schema version ${this.schema.version}`, + ) await toPromise(callback => this._dispatcher.setUpWithSchema( this._tag, @@ -325,7 +329,7 @@ export default class SQLiteAdapter implements DatabaseAdapter, SQLDatabaseAdapte callback, ), ) - logger.log(`[DB] Schema set up successfully`) + logger.log(`[WatermelonDB][SQLite] Schema set up successfully`) } find(table: TableName, id: RecordId, callback: ResultCallback): void { @@ -407,7 +411,7 @@ export default class SQLiteAdapter implements DatabaseAdapter, SQLDatabaseAdapte this.schema.version, result => { if (result.value) { - logger.log('[DB] Database is now reset') + logger.log('[WatermelonDB][SQLite] Database is now reset') } callback(result) }, From d3c4b8be196731126971243232490bb86f760d14 Mon Sep 17 00:00:00 2001 From: radex Date: Mon, 20 Jan 2020 12:09:23 +0100 Subject: [PATCH 06/17] [Loki] Even better error handling for failed database open --- src/adapters/lokijs/worker/lokiExtensions.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/adapters/lokijs/worker/lokiExtensions.js b/src/adapters/lokijs/worker/lokiExtensions.js index 948ad93d7..b24d49add 100644 --- a/src/adapters/lokijs/worker/lokiExtensions.js +++ b/src/adapters/lokijs/worker/lokiExtensions.js @@ -18,11 +18,15 @@ const isIDBAvailable = () => { db.close() resolve(true) } - checkRequest.onerror = e => { + checkRequest.onerror = event => { logger.error( - '[WatermelonDB] IndexedDB checker failed to open. Most likely, user is in Private Mode. It could also be a quota exceeded error. Will fall back to in-memory database.', - e, + '[WatermelonDB][Loki] IndexedDB checker failed to open. Most likely, user is in Private Mode. It could also be a quota exceeded error. Will fall back to in-memory database.', + event, ) + const error: ?Error = event?.target?.error + if (error && error.name === 'QuotaExceededError') { + logger.log('[WatermelonDB][Loki] Looks like disk quota was exceeded: ', error) + } resolve(false) } checkRequest.onblocked = () => { From 9ba05c3af586d786692fa0922e2a6f0893a62df9 Mon Sep 17 00:00:00 2001 From: radex Date: Mon, 20 Jan 2020 12:19:08 +0100 Subject: [PATCH 07/17] [loki] Some extra diagnostics for indexeddb --- src/adapters/lokijs/worker/lokiExtensions.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/adapters/lokijs/worker/lokiExtensions.js b/src/adapters/lokijs/worker/lokiExtensions.js index b24d49add..9615deb79 100644 --- a/src/adapters/lokijs/worker/lokiExtensions.js +++ b/src/adapters/lokijs/worker/lokiExtensions.js @@ -19,11 +19,15 @@ const isIDBAvailable = () => { resolve(true) } checkRequest.onerror = event => { + const error: ?Error = event?.target?.error + // this is what Firefox in Private Mode returns: + // DOMException: "A mutation operation was attempted on a database that did not allow mutations." + // code: 11, name: InvalidStateError logger.error( '[WatermelonDB][Loki] IndexedDB checker failed to open. Most likely, user is in Private Mode. It could also be a quota exceeded error. Will fall back to in-memory database.', event, + error, ) - const error: ?Error = event?.target?.error if (error && error.name === 'QuotaExceededError') { logger.log('[WatermelonDB][Loki] Looks like disk quota was exceeded: ', error) } From be42fccbbcb330dc8fbe6db6c633d95244a18fda Mon Sep 17 00:00:00 2001 From: radex Date: Mon, 20 Jan 2020 14:32:37 +0100 Subject: [PATCH 08/17] Add onQuotaExceededError + update changelog --- CHANGELOG.md | 17 ++++++++++++----- src/adapters/lokijs/index.js | 6 +++++- src/adapters/lokijs/worker/executor.js | 4 ++++ src/adapters/lokijs/worker/lokiExtensions.js | 15 ++++++++++++--- 4 files changed, 33 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30c67b92d..c9cd7dd53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,11 +17,10 @@ All notable changes to this project will be documented in this file. When enabled, database operations will block JavaScript thread. Adapter actions will resolve in the next microtask, which simplifies building flicker-free interfaces. Adapter will fall back to async operation when synchronous adapter is not available (e.g. when doing remote debugging) -- [Model] Added experimental `model.experimentalSubscribe((isDeleted) => { ... })` method as a vanilla JS alternative to Rx based `model.observe()`. Unlike the latter, it does not notify the subscriber immediately upon subscription. -- [Collection] Added internal `collection.experimentalSubscribe((changeSet) => { ... })` method as a vanilla JS alternative to Rx based `collection.changes` (you probably shouldn't be using this API anyway) -- [Database] Added experimental `database.experimentalSubscribe(['table1', 'table2'], () => { ... })` method as a vanilla JS alternative to Rx-based `database.withChangesForTables()`. Unlike the latter, `experimentalSubscribe` notifies the subscriber only once after a batch that makes a change in multiple collections subscribed to. It also doesn't notify the subscriber immediately upon subscription, and doesn't send details about the changes, only a signal. -- Added `experimentalDisableObserveCountThrottling()` to `@nozbe/watermelondb/observation/observeCount` that globally disables count observation throttling. We think that throttling on WatermelonDB level is not a good feature and will be removed in a future release - and will be better implemented on app level if necessary -- [Query] Added experimental `query.experimentalSubscribe(records => { ... })`, `query.experimentalSubscribeWithColumns(['col1', 'col2'], records => { ... })`, and `query.experimentalSubscribeToCount(count => { ... })` methods +- [LokiJS] Added new `onQuotaExceededError?: (error: Error) => void` option to `LokiJSAdapter` constructor. + This is called when underlying IndexedDB encountered a quota exceeded error (ran out of allotted disk space for app) + This means that app can't save more data or that it will fall back to using in-memory database only + Note that this only works when `useWebWorker: false` ### Changes @@ -35,6 +34,14 @@ All notable changes to this project will be documented in this file. - Fixed a potential issue when using `database.unsafeResetDatabase()` - [iOS] Fixed issue with clearing database under experimental synchronous mode +### New features (Experimental) + +- [Model] Added experimental `model.experimentalSubscribe((isDeleted) => { ... })` method as a vanilla JS alternative to Rx based `model.observe()`. Unlike the latter, it does not notify the subscriber immediately upon subscription. +- [Collection] Added internal `collection.experimentalSubscribe((changeSet) => { ... })` method as a vanilla JS alternative to Rx based `collection.changes` (you probably shouldn't be using this API anyway) +- [Database] Added experimental `database.experimentalSubscribe(['table1', 'table2'], () => { ... })` method as a vanilla JS alternative to Rx-based `database.withChangesForTables()`. Unlike the latter, `experimentalSubscribe` notifies the subscriber only once after a batch that makes a change in multiple collections subscribed to. It also doesn't notify the subscriber immediately upon subscription, and doesn't send details about the changes, only a signal. +- Added `experimentalDisableObserveCountThrottling()` to `@nozbe/watermelondb/observation/observeCount` that globally disables count observation throttling. We think that throttling on WatermelonDB level is not a good feature and will be removed in a future release - and will be better implemented on app level if necessary +- [Query] Added experimental `query.experimentalSubscribe(records => { ... })`, `query.experimentalSubscribeWithColumns(['col1', 'col2'], records => { ... })`, and `query.experimentalSubscribeToCount(count => { ... })` methods + ## 0.15 - 2019-11-08 ### Highlights diff --git a/src/adapters/lokijs/index.js b/src/adapters/lokijs/index.js index 8030b8d8e..2db26fb71 100644 --- a/src/adapters/lokijs/index.js +++ b/src/adapters/lokijs/index.js @@ -36,10 +36,14 @@ export type LokiAdapterOptions = $Exact<{ // may lead to lower memory consumption, lower latency, and easier debugging useWebWorker?: boolean, experimentalUseIncrementalIndexedDB?: boolean, - // Called when internal IDB version changed (most likely the database was deleted in another browser tab) + // Called when internal IndexedDB version changed (most likely the database was deleted in another browser tab) // Pass a callback to force log out in this copy of the app as well // Note that this only works when using incrementalIDB and not using web workers onIndexedDBVersionChange?: () => void, + // Called when underlying IndexedDB encountered a quota exceeded error (ran out of allotted disk space for app) + // This means that app can't save more data or that it will fall back to using in-memory database only + // Note that this only works when `useWebWorker: false` + onQuotaExceededError?: (error: Error) => void, // -- internal -- _testLokiAdapter?: LokiMemoryAdapter, }> diff --git a/src/adapters/lokijs/worker/executor.js b/src/adapters/lokijs/worker/executor.js index e678690e5..744ddbb9b 100644 --- a/src/adapters/lokijs/worker/executor.js +++ b/src/adapters/lokijs/worker/executor.js @@ -37,6 +37,8 @@ export default class LokiExecutor { onIndexedDBVersionChange: ?() => void + onQuotaExceededError: ?(error: Error) => void + _testLokiAdapter: ?LokiMemoryAdapter cachedRecords: Map, Set> = new Map() @@ -48,6 +50,7 @@ export default class LokiExecutor { this.migrations = migrations this.experimentalUseIncrementalIndexedDB = options.experimentalUseIncrementalIndexedDB || false this.onIndexedDBVersionChange = options.onIndexedDBVersionChange + this.onQuotaExceededError = options.onQuotaExceededError this._testLokiAdapter = _testLokiAdapter } @@ -240,6 +243,7 @@ export default class LokiExecutor { this._testLokiAdapter, this.experimentalUseIncrementalIndexedDB, this.onIndexedDBVersionChange, + this.onQuotaExceededError, ) logger.log('[WatermelonDB][Loki] Database loaded') diff --git a/src/adapters/lokijs/worker/lokiExtensions.js b/src/adapters/lokijs/worker/lokiExtensions.js index 9615deb79..9f2861af6 100644 --- a/src/adapters/lokijs/worker/lokiExtensions.js +++ b/src/adapters/lokijs/worker/lokiExtensions.js @@ -4,7 +4,7 @@ import Loki, { LokiMemoryAdapter } from 'lokijs' import { logger } from '../../../utils/common' -const isIDBAvailable = () => { +const isIDBAvailable = (onQuotaExceededError: ?(error: Error) => void) => { return new Promise(resolve => { // $FlowFixMe if (typeof indexedDB === 'undefined') { @@ -30,6 +30,7 @@ const isIDBAvailable = () => { ) if (error && error.name === 'QuotaExceededError') { logger.log('[WatermelonDB][Loki] Looks like disk quota was exceeded: ', error) + onQuotaExceededError && onQuotaExceededError(error) } resolve(false) } @@ -44,10 +45,11 @@ async function getLokiAdapter( adapter: ?LokiMemoryAdapter, useIncrementalIDB: boolean, onIndexedDBVersionChange: ?() => void, + onQuotaExceededError: ?(error: Error) => void, ): mixed { if (adapter) { return adapter - } else if (await isIDBAvailable()) { + } else if (await isIDBAvailable(onQuotaExceededError)) { if (useIncrementalIDB) { const IncrementalIDBAdapter = require('lokijs/src/incremental-indexeddb-adapter') return new IncrementalIDBAdapter({ @@ -68,9 +70,16 @@ export async function newLoki( adapter: ?LokiMemoryAdapter, useIncrementalIDB: boolean, onIndexedDBVersionChange: ?() => void, + onQuotaExceededError: ?(error: Error) => void, ): Loki { const loki = new Loki(name, { - adapter: await getLokiAdapter(name, adapter, useIncrementalIDB, onIndexedDBVersionChange), + adapter: await getLokiAdapter( + name, + adapter, + useIncrementalIDB, + onIndexedDBVersionChange, + onQuotaExceededError, + ), autosave: true, autosaveInterval: 250, verbose: true, From 0c97065a7a351a8ab454f0a756ee0b507d173aed Mon Sep 17 00:00:00 2001 From: radex Date: Mon, 20 Jan 2020 15:01:25 +0100 Subject: [PATCH 09/17] add failing test --- src/Database/test.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/Database/test.js b/src/Database/test.js index 3774261db..1e49f3273 100644 --- a/src/Database/test.js +++ b/src/Database/test.js @@ -303,6 +303,33 @@ describe('Observation', () => { expect(subscriber1).toHaveBeenCalledTimes(1) unsubscribe1() }) + it.only('has new objects cached before calling subscribers (regression test)', async () => { + const { database, projects, tasks } = mockDatabase({ actionsEnabled: true }) + + const project = projects.prepareCreate() + const task = tasks.prepareCreate(t => { + t.project.set(project) + }) + + let observerCalled = 0 + let taskPromise = null + const observer = jest.fn(() => { + observerCalled += 1 + if (observerCalled === 1) { + // nothing happens + } else { + taskPromise = projects.find(task.id) + } + }) + database.withChangesForTables(['mock_projects']).subscribe(observer) + expect(observer).toHaveBeenCalledTimes(1) + + await database.action(() => database.batch(project, task)) + expect(observer).toHaveBeenCalledTimes(2) + + // check if task is already cached + expect(await taskPromise).toBe(task) + }) }) const delayPromise = () => new Promise(resolve => setTimeout(resolve, 100)) From e218e65fe8a22ed67960589266c2f2c9acfd807f Mon Sep 17 00:00:00 2001 From: radex Date: Mon, 20 Jan 2020 15:16:55 +0100 Subject: [PATCH 10/17] Fix "was sent over the bridge, but it's not cached" issue --- src/Collection/index.js | 4 +++- src/Database/index.js | 10 +++++++--- src/Database/test.js | 6 +++--- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/Collection/index.js b/src/Collection/index.js index b2651218f..302264583 100644 --- a/src/Collection/index.js +++ b/src/Collection/index.js @@ -137,7 +137,7 @@ export default class Collection { ) } - changeSet(operations: CollectionChangeSet): void { + _applyChangesToCache(operations: CollectionChangeSet): void { operations.forEach(({ record, type }) => { if (type === CollectionChangeTypes.created) { record._isCommitted = true @@ -146,7 +146,9 @@ export default class Collection { this._cache.delete(record) } }) + } + _notify(operations: CollectionChangeSet): void { this._subscribers.forEach(subscriber => { subscriber(operations) }) diff --git a/src/Database/index.js b/src/Database/index.js index ee5d0b34a..bd2093492 100644 --- a/src/Database/index.js +++ b/src/Database/index.js @@ -106,11 +106,15 @@ export default class Database { await this.adapter.batch(batchOperations) - // NOTE: Collections must be notified first to ensure that batched - // elements are marked as cached + // NOTE: We must make two passes to ensure all changes to caches are applied before subscribers are called Object.entries(changeNotifications).forEach(notification => { const [table, changeSet]: [TableName, CollectionChangeSet] = (notification: any) - this.collections.get(table).changeSet(changeSet) + this.collections.get(table)._applyChangesToCache(changeSet) + }) + + Object.entries(changeNotifications).forEach(notification => { + const [table, changeSet]: [TableName, CollectionChangeSet] = (notification: any) + this.collections.get(table)._notify(changeSet) }) const affectedTables = Object.keys(changeNotifications) diff --git a/src/Database/test.js b/src/Database/test.js index 1e49f3273..165ea4a4b 100644 --- a/src/Database/test.js +++ b/src/Database/test.js @@ -303,7 +303,7 @@ describe('Observation', () => { expect(subscriber1).toHaveBeenCalledTimes(1) unsubscribe1() }) - it.only('has new objects cached before calling subscribers (regression test)', async () => { + it('has new objects cached before calling subscribers (regression test)', async () => { const { database, projects, tasks } = mockDatabase({ actionsEnabled: true }) const project = projects.prepareCreate() @@ -317,8 +317,8 @@ describe('Observation', () => { observerCalled += 1 if (observerCalled === 1) { // nothing happens - } else { - taskPromise = projects.find(task.id) + } else if (observerCalled === 2) { + taskPromise = tasks.find(task.id) } }) database.withChangesForTables(['mock_projects']).subscribe(observer) From 64900df21f97ab7573187f114b7b15f25470d120 Mon Sep 17 00:00:00 2001 From: radex Date: Mon, 20 Jan 2020 15:19:03 +0100 Subject: [PATCH 11/17] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9cd7dd53..43134f4a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ All notable changes to this project will be documented in this file. ### Fixes +- Fixed a possible cause for "Record ID xxx#yyy was sent over the bridge, but it's not cached" error - [LokiJS] Fixed an issue preventing database from saving when using `experimentalUseIncrementalIndexedDB` - Fixed a potential issue when using `database.unsafeResetDatabase()` - [iOS] Fixed issue with clearing database under experimental synchronous mode From bed9c5fc2e031952c7ae4764f518721d57faac13 Mon Sep 17 00:00:00 2001 From: radex Date: Mon, 20 Jan 2020 15:21:12 +0100 Subject: [PATCH 12/17] experimentalUseIncrementalIndexedDB` option has been renamed to `useIncrementalIndexedDB --- CHANGELOG.md | 4 +++- docs-master/Installation.md | 2 +- src/adapters/lokijs/index.js | 9 +++++++-- src/adapters/lokijs/worker/executor.js | 6 +++--- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43134f4a0..5a75e2237 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,9 @@ All notable changes to this project will be documented in this file. ### ⚠️ Breaking -(Low breakage risk) +- `experimentalUseIncrementalIndexedDB` has been renamed to `useIncrementalIndexedDB` + +#### Low breakage risk - [adapters] Adapter API has changed from returning Promise to taking callbacks as the last argument. This won't affect you unless you call on adapter methods directly. `database.adapter` returns a new `DatabaseAdapterCompat` which has the same shape as old adapter API. You can use `database.adapter.underlyingAdapter` to get back `SQLiteAdapter` / `LokiJSAdapter` - [Collection] `Collection.fetchQuery` and `Collection.fetchCount` are removed. Please use `Query.fetch()` and `Query.fetchCount()`. diff --git a/docs-master/Installation.md b/docs-master/Installation.md index 0b142bea2..9945a684b 100644 --- a/docs-master/Installation.md +++ b/docs-master/Installation.md @@ -246,7 +246,7 @@ const adapter = new LokiJSAdapter({ schema, // These two options are recommended for new projects: useWebWorker: false, - experimentalUseIncrementalIndexedDB: true, + useIncrementalIndexedDB: true, // It's recommended you implement this method: // onIndexedDBVersionChange: () => { // // database was deleted in another browser tab (user logged out), so we must make sure we delete diff --git a/src/adapters/lokijs/index.js b/src/adapters/lokijs/index.js index 2db26fb71..d7f52b926 100644 --- a/src/adapters/lokijs/index.js +++ b/src/adapters/lokijs/index.js @@ -35,7 +35,7 @@ export type LokiAdapterOptions = $Exact<{ // (true by default) Although web workers may have some throughput benefits, disabling them // may lead to lower memory consumption, lower latency, and easier debugging useWebWorker?: boolean, - experimentalUseIncrementalIndexedDB?: boolean, + useIncrementalIndexedDB?: boolean, // Called when internal IndexedDB version changed (most likely the database was deleted in another browser tab) // Pass a callback to force log out in this copy of the app as well // Note that this only works when using incrementalIDB and not using web workers @@ -71,7 +71,12 @@ export default class LokiJSAdapter implements DatabaseAdapter { invariant( // $FlowFixMe options.migrationsExperimental === undefined, - 'LokiJSAdapter migrationsExperimental has been renamed to migrations', + 'LokiJSAdapter `migrationsExperimental` option has been renamed to `migrations`', + ) + invariant( + // $FlowFixMe + options.experimentalUseIncrementalIndexedDB === undefined, + 'LokiJSAdapter `experimentalUseIncrementalIndexedDB` option has been renamed to `useIncrementalIndexedDB`', ) validateAdapter(this) } diff --git a/src/adapters/lokijs/worker/executor.js b/src/adapters/lokijs/worker/executor.js index 744ddbb9b..a2ef5bf67 100644 --- a/src/adapters/lokijs/worker/executor.js +++ b/src/adapters/lokijs/worker/executor.js @@ -33,7 +33,7 @@ export default class LokiExecutor { loki: Loki - experimentalUseIncrementalIndexedDB: boolean + useIncrementalIndexedDB: boolean onIndexedDBVersionChange: ?() => void @@ -48,7 +48,7 @@ export default class LokiExecutor { this.dbName = dbName this.schema = schema this.migrations = migrations - this.experimentalUseIncrementalIndexedDB = options.experimentalUseIncrementalIndexedDB || false + this.useIncrementalIndexedDB = options.useIncrementalIndexedDB || false this.onIndexedDBVersionChange = options.onIndexedDBVersionChange this.onQuotaExceededError = options.onQuotaExceededError this._testLokiAdapter = _testLokiAdapter @@ -241,7 +241,7 @@ export default class LokiExecutor { this.loki = await newLoki( this.dbName, this._testLokiAdapter, - this.experimentalUseIncrementalIndexedDB, + this.useIncrementalIndexedDB, this.onIndexedDBVersionChange, this.onQuotaExceededError, ) From ef2620e764f46e85f1d392e646b6d2f5e92ae93c Mon Sep 17 00:00:00 2001 From: radex Date: Mon, 20 Jan 2020 15:24:07 +0100 Subject: [PATCH 13/17] prepare for a breaking change --- src/adapters/lokijs/index.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/adapters/lokijs/index.js b/src/adapters/lokijs/index.js index d7f52b926..5cc15d1b6 100644 --- a/src/adapters/lokijs/index.js +++ b/src/adapters/lokijs/index.js @@ -1,7 +1,7 @@ // @flow import type { LokiMemoryAdapter } from 'lokijs' -import { invariant } from '../../utils/common' +import { invariant, logger } from '../../utils/common' import type { ResultCallback } from '../../utils/fp/Result' import type { RecordId } from '../../Model' @@ -68,6 +68,16 @@ export default class LokiJSAdapter implements DatabaseAdapter { this._dbName = dbName if (process.env.NODE_ENV !== 'production') { + if (options.useWebWorker === undefined) { + logger.warn( + 'LokiJSAdapter `useWebWorker` option will become required in a future version of WatermelonDB. Pass `{ useWebWorker: false }` to adopt the new behavior, or `{ useWebWorker: true }` to supress this warning with no changes', + ) + } + if (options.useIncrementalIndexedDB === undefined) { + logger.warn( + 'LokiJSAdapter `useIncrementalIndexedDB` option will become required in a future version of WatermelonDB. Pass `{ useIncrementalIndexedDB: true }` to adopt the new behavior, or `{ useIncrementalIndexedDB: false }` to supress this warning with no changes', + ) + } invariant( // $FlowFixMe options.migrationsExperimental === undefined, From 86fc11a1ae6dcc772a8f0d620ecf69fe25e6014b Mon Sep 17 00:00:00 2001 From: radex Date: Mon, 20 Jan 2020 15:33:18 +0100 Subject: [PATCH 14/17] fix test --- src/adapters/__tests__/commonTests.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/adapters/__tests__/commonTests.js b/src/adapters/__tests__/commonTests.js index 3da7a328a..dee74dfd5 100644 --- a/src/adapters/__tests__/commonTests.js +++ b/src/adapters/__tests__/commonTests.js @@ -37,7 +37,11 @@ export default () => [ // expect(() => makeAdapter({})).toThrowError(/missing migrations/) expect(() => makeAdapter({ migrationsExperimental: [] })).toThrow( - /migrationsExperimental has been renamed/, + /LokiJSAdapter `migrationsExperimental` option has been renamed to `migrations`/, + ) + + expect(() => makeAdapter({ experimentalUseIncrementalIndexedDB: false })).toThrow( + /LokiJSAdapter `experimentalUseIncrementalIndexedDB` option has been renamed/, ) expect(() => adapterWithMigrations({ migrations: [] })).toThrow(/use schemaMigrations()/) From 70ccf269e4d54bfd89d6808a5779eaa3c50c7a0f Mon Sep 17 00:00:00 2001 From: radex Date: Mon, 20 Jan 2020 15:59:21 +0100 Subject: [PATCH 15/17] fix tests on ios --- src/adapters/__tests__/commonTests.js | 10 ++++++---- src/adapters/sqlite/index.js | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/adapters/__tests__/commonTests.js b/src/adapters/__tests__/commonTests.js index dee74dfd5..c54ce0cd1 100644 --- a/src/adapters/__tests__/commonTests.js +++ b/src/adapters/__tests__/commonTests.js @@ -37,12 +37,14 @@ export default () => [ // expect(() => makeAdapter({})).toThrowError(/missing migrations/) expect(() => makeAdapter({ migrationsExperimental: [] })).toThrow( - /LokiJSAdapter `migrationsExperimental` option has been renamed to `migrations`/, + /`migrationsExperimental` option has been renamed to `migrations`/, ) - expect(() => makeAdapter({ experimentalUseIncrementalIndexedDB: false })).toThrow( - /LokiJSAdapter `experimentalUseIncrementalIndexedDB` option has been renamed/, - ) + if (AdapterClass.name === 'LokiJSAdapter') { + expect(() => makeAdapter({ experimentalUseIncrementalIndexedDB: false })).toThrow( + /LokiJSAdapter `experimentalUseIncrementalIndexedDB` option has been renamed/, + ) + } expect(() => adapterWithMigrations({ migrations: [] })).toThrow(/use schemaMigrations()/) diff --git a/src/adapters/sqlite/index.js b/src/adapters/sqlite/index.js index e2e6e13bb..ea137a6a9 100644 --- a/src/adapters/sqlite/index.js +++ b/src/adapters/sqlite/index.js @@ -221,7 +221,7 @@ export default class SQLiteAdapter implements DatabaseAdapter, SQLDatabaseAdapte invariant( // $FlowFixMe options.migrationsExperimental === undefined, - 'SQLiteAdapter migrationsExperimental has been renamed to migrations', + 'SQLiteAdapter `migrationsExperimental` option has been renamed to `migrations`', ) invariant( NativeDatabaseBridge, From be19591521b6dafe2898dcfb1b529b20acc9a099 Mon Sep 17 00:00:00 2001 From: radex Date: Mon, 20 Jan 2020 16:01:35 +0100 Subject: [PATCH 16/17] v0.16.0-9 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 848f67ce0..856c1ca29 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@nozbe/watermelondb", "description": "Build powerful React Native and React web apps that scale from hundreds to tens of thousands of records and remain fast", - "version": "0.16.0-8", + "version": "0.16.0-9", "scripts": { "build": "NODE_ENV=production node ./scripts/make.js", "dev": "NODE_ENV=development node ./scripts/make.js", From 02a7b3b8a1d3ce1706e9097f9438b314af5ad19a Mon Sep 17 00:00:00 2001 From: radex Date: Wed, 22 Jan 2020 13:55:00 +0100 Subject: [PATCH 17/17] minor changes --- src/adapters/lokijs/index.js | 10 ++++------ src/adapters/sqlite/index.js | 3 +-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/adapters/lokijs/index.js b/src/adapters/lokijs/index.js index 5cc15d1b6..94c9bf3d3 100644 --- a/src/adapters/lokijs/index.js +++ b/src/adapters/lokijs/index.js @@ -68,24 +68,22 @@ export default class LokiJSAdapter implements DatabaseAdapter { this._dbName = dbName if (process.env.NODE_ENV !== 'production') { - if (options.useWebWorker === undefined) { + if (!('useWebWorker' in options)) { logger.warn( 'LokiJSAdapter `useWebWorker` option will become required in a future version of WatermelonDB. Pass `{ useWebWorker: false }` to adopt the new behavior, or `{ useWebWorker: true }` to supress this warning with no changes', ) } - if (options.useIncrementalIndexedDB === undefined) { + if (!('useIncrementalIndexedDB' in options)) { logger.warn( 'LokiJSAdapter `useIncrementalIndexedDB` option will become required in a future version of WatermelonDB. Pass `{ useIncrementalIndexedDB: true }` to adopt the new behavior, or `{ useIncrementalIndexedDB: false }` to supress this warning with no changes', ) } invariant( - // $FlowFixMe - options.migrationsExperimental === undefined, + !('migrationsExperimental' in options), 'LokiJSAdapter `migrationsExperimental` option has been renamed to `migrations`', ) invariant( - // $FlowFixMe - options.experimentalUseIncrementalIndexedDB === undefined, + !('experimentalUseIncrementalIndexedDB' in options), 'LokiJSAdapter `experimentalUseIncrementalIndexedDB` option has been renamed to `useIncrementalIndexedDB`', ) validateAdapter(this) diff --git a/src/adapters/sqlite/index.js b/src/adapters/sqlite/index.js index ea137a6a9..89fc06880 100644 --- a/src/adapters/sqlite/index.js +++ b/src/adapters/sqlite/index.js @@ -219,8 +219,7 @@ export default class SQLiteAdapter implements DatabaseAdapter, SQLDatabaseAdapte if (process.env.NODE_ENV !== 'production') { invariant( - // $FlowFixMe - options.migrationsExperimental === undefined, + !('migrationsExperimental' in options), 'SQLiteAdapter `migrationsExperimental` option has been renamed to `migrations`', ) invariant(