Skip to content

Commit

Permalink
Remove SQLiteAdapter's synchronous mode - use JSI mode instead
Browse files Browse the repository at this point in the history
  • Loading branch information
radex committed May 7, 2021
1 parent 015db9c commit 50f5d1b
Show file tree
Hide file tree
Showing 12 changed files with 32 additions and 525 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG-Unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
### BREAKING CHANGES

- [SQLite] `experimentalUseJSI: true` option has been renamed to `jsi: true`
- Depreacated `new Database({ actionsEnabled: false })` options is now removed. Actions are always enabled.
- Deprecated `new Database({ actionsEnabled: false })` options is now removed. Actions are always enabled.
- Deprecated `new SQLiteAdapter({ synchronous: true })` option is now removed. Use `{ jsi: true }` instead.

### Deprecations

Expand Down
3 changes: 0 additions & 3 deletions native/ios/WatermelonDB/DatabaseBridge.m
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ @interface RCT_EXTERN_REMAP_MODULE(DatabaseBridge, DatabaseBridge, NSObject)
args \
resolve:(RCTPromiseResolveBlock)resolve \
reject:(RCTPromiseRejectBlock)reject \
) \
RCT_EXTERN__BLOCKING_SYNCHRONOUS_METHOD(WMELON_CONCAT(name, Synchronous):(nonnull NSNumber *)connectionTag \
args \
)

WMELON_BRIDGE_METHOD(initialize,
Expand Down
177 changes: 7 additions & 170 deletions native/ios/WatermelonDB/DatabaseBridge.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ final public class DatabaseBridge: NSObject {
@objc let methodQueue = DispatchQueue(label: "com.nozbe.watermelondb.database", qos: .userInteractive)

private enum Connection {
case connected(driver: DatabaseDriver, synchronous: Bool)
case connected(driver: DatabaseDriver)
case waiting(queue: [() -> Void])

var queue: [() -> Void] {
Expand All @@ -34,7 +34,7 @@ extension DatabaseBridge {
do {
try assertNoConnection(tag)
let driver = try DatabaseDriver(dbName: databaseName, schemaVersion: schemaVersion.intValue)
connections[tag.intValue] = .connected(driver: driver, synchronous: false)
connections[tag.intValue] = .connected(driver: driver)
resolve(["code": "ok"])
} catch _ as DatabaseDriver.SchemaNeededError {
connections[tag.intValue] = .waiting(queue: [])
Expand Down Expand Up @@ -83,67 +83,16 @@ extension DatabaseBridge {
}
}

// MARK: - Synchronous connections
// MARK: - JSI Support

extension DatabaseBridge {
@objc(initializeJSI)
func initializeJSI() -> NSDictionary {
return synchronously {
methodQueue.sync {
// swiftlint:disable all
installWatermelonJSI(bridge as? RCTCxxBridge)
}
}

@objc(initializeSynchronous:databaseName:schemaVersion:)
func initializeSynchronous(tag: ConnectionTag,
databaseName: String,
schemaVersion: NSNumber) -> NSDictionary {
return synchronously {
do {
try assertNoConnection(tag)
let driver = try DatabaseDriver(dbName: databaseName, schemaVersion: schemaVersion.intValue)
connections[tag.intValue] = .connected(driver: driver, synchronous: true)
return ["code": "ok"]
} catch _ as DatabaseDriver.SchemaNeededError {
return ["code": "schema_needed"]
} catch let error as DatabaseDriver.MigrationNeededError {
return ["code": "migrations_needed", "databaseVersion": error.databaseVersion]
} catch {
assertionFailure("Unknown error thrown in DatabaseDriver.init")
throw error // rethrow
}
}
}

@objc(setUpWithSchemaSynchronous:databaseName:schema:schemaVersion:)
func setUpWithSchemaSynchronous(tag: ConnectionTag,
databaseName: String,
schema: Database.SQL,
schemaVersion: NSNumber) -> NSDictionary {
return synchronously {
try assertNoConnection(tag)
let driver = DatabaseDriver(dbName: databaseName,
setUpWithSchema: (version: schemaVersion.intValue, sql: schema))
connections[tag.intValue] = .connected(driver: driver, synchronous: true)
return true
}
}

@objc(setUpWithMigrationsSynchronous:databaseName:migrations:fromVersion:toVersion:)
func setUpWithMigrationsSynchronous(tag: ConnectionTag,
databaseName: String,
migrations: Database.SQL,
fromVersion: NSNumber,
toVersion: NSNumber) -> NSDictionary {
return synchronously {
try assertNoConnection(tag)
let driver = try DatabaseDriver(
dbName: databaseName,
setUpWithMigrations: (from: fromVersion.intValue, to: toVersion.intValue, sql: migrations)
)
connections[tag.intValue] = .connected(driver: driver, synchronous: true)
return true
}
return [:]
}
}

Expand Down Expand Up @@ -259,91 +208,6 @@ extension DatabaseBridge {
}
}

// MARK: - Synchronous methods

extension DatabaseBridge {
@objc(findSynchronous:table:id:)
func findSynchronous(tag: ConnectionTag, table: Database.TableName, id: DatabaseDriver.RecordId) -> NSDictionary {
return withDriverSynchronous(tag) {
try $0.find(table: table, id: id) as Any
}
}

@objc(querySynchronous:table:query:)
func querySynchronous(tag: ConnectionTag, table: Database.TableName, query: Database.SQL) -> NSDictionary {
return withDriverSynchronous(tag) {
try $0.cachedQuery(table: table, query: query)
}
}

@objc(countSynchronous:query:)
func countSynchronous(tag: ConnectionTag, query: Database.SQL) -> NSDictionary {
return withDriverSynchronous(tag) {
try $0.count(query)
}
}

@objc(batchJSONSynchronous:operations:)
func batchJSONSynchronous(tag: ConnectionTag, operations serializedOperations: NSString) -> NSDictionary {
return withDriverSynchronous(tag) {
try $0.batch(self.toBatchOperations(serializedOperations))
}
}

@objc(batchSynchronous:operations:)
func batchSynchronous(tag: ConnectionTag, operations: [[Any]]) -> NSDictionary {
return withDriverSynchronous(tag) {
try $0.batch(self.toBatchOperations(operations))
}
}

@objc(getDeletedRecordsSynchronous:table:)
func getDeletedRecordsSynchronous(tag: ConnectionTag, table: Database.TableName) -> NSDictionary {
return withDriverSynchronous(tag) {
try $0.getDeletedRecords(table: table)
}
}

@objc(destroyDeletedRecordsSynchronous:table:records:)
func destroyDeletedRecordsSynchronous(tag: ConnectionTag,
table: Database.TableName,
records: [DatabaseDriver.RecordId]) -> NSDictionary {
return withDriverSynchronous(tag) {
try $0.destroyDeletedRecords(table: table, records: records)
}
}

@objc(unsafeResetDatabaseSynchronous:schema:schemaVersion:)
func unsafeResetDatabaseSynchronous(tag: ConnectionTag,
schema: Database.SQL,
schemaVersion: NSNumber) -> NSDictionary {
return withDriverSynchronous(tag) {
try $0.unsafeResetDatabase(schema: (version: schemaVersion.intValue, sql: schema))
}
}

@objc(getLocalSynchronous:key:)
func getLocalSynchronous(tag: ConnectionTag, key: String) -> NSDictionary {
return withDriverSynchronous(tag) {
try $0.getLocal(key: key) as Any
}
}

@objc(setLocalSynchronous:key:value:)
func setLocalSynchronous(tag: ConnectionTag, key: String, value: String) -> NSDictionary {
return withDriverSynchronous(tag) {
try $0.setLocal(key: key, value: value)
}
}

@objc(removeLocalSynchronous:key:)
func removeLocalSynchronous(tag: ConnectionTag, key: String) -> NSDictionary {
return withDriverSynchronous(tag) {
try $0.removeLocal(key: key)
}
}
}

// MARK: - Helpers

extension DatabaseBridge {
Expand Down Expand Up @@ -416,10 +280,7 @@ extension DatabaseBridge {
}

switch connection {
case .connected(let driver, let synchronous):
guard !synchronous else {
throw "Can't perform async action on synchronous connection \(tagID)".asError()
}
case .connected(let driver):
let result = try action(driver)
resolve(result)
case .waiting(var queue):
Expand All @@ -435,34 +296,10 @@ extension DatabaseBridge {
}
}

private func synchronously(functionName: String = #function, action: () throws -> Any) -> NSDictionary {
return methodQueue.sync {
do {
let result = try action()
return ["status": "success", "result": result]
} catch {
return ["status": "error", "code": "db.\(functionName).error", "message": "\(error)"]
}
}
}

private func withDriverSynchronous(_ connectionTag: ConnectionTag,
functionName: String = #function,
action: (DatabaseDriver) throws -> Any) -> NSDictionary {
return synchronously {
guard let connection = connections[connectionTag.intValue],
case let .connected(driver, synchronous: true) = connection else {
throw "No or invalid connection for tag \(connectionTag)".asError()
}

return try action(driver)
}
}

private func connectDriverAsync(connectionTag: ConnectionTag, driver: DatabaseDriver) {
let tagID = connectionTag.intValue
let queue = connections[tagID]?.queue ?? []
connections[tagID] = .connected(driver: driver, synchronous: false)
connections[tagID] = .connected(driver: driver)

for operation in queue {
operation()
Expand Down
17 changes: 0 additions & 17 deletions src/adapters/sqlite/common.js

This file was deleted.

1 change: 0 additions & 1 deletion src/adapters/sqlite/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ declare module '@nozbe/watermelondb/adapters/sqlite' {
dbName?: string
migrations?: SchemaMigrations
schema: AppSchema
synchronous?: boolean
jsi?: boolean
}

Expand Down
14 changes: 4 additions & 10 deletions src/adapters/sqlite/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,10 @@ export default class SQLiteAdapter implements DatabaseAdapter, SQLDatabaseAdapte
!('experimentalUseJSI' in options),
'SQLiteAdapter `experimentalUseJSI: true` has been renamed to `jsi: true`',
)
if (options.synchronous) {
// Docs semi-recommend synchronous: true, but it adds a lot of junk and I want to get rid
// of this mode completely to simplify code. Ideally, we'd ONLY have JSI, but until RN goes
// all-in on JSI everywhere, this might be a little too risky. I'm adding this warning to
// get feedback via GH if JSI on iOS is ready to be considered stable or not yet.
logger.warn(
'SQLiteAdapter `synchronous: true` option is deprecated and will be replaced with `jsi: true` soon. Please test if your app compiles and works well with `jsi: true`, and if not - file an issue!',
)
}
invariant(
!('synchronous' in options),
'SQLiteAdapter `synchronous: true` was removed. Replace with `jsi: true`, which has the same effect, but with a more modern implementation',
)
invariant(
DatabaseBridge,
`NativeModules.DatabaseBridge is not defined! This means that you haven't properly linked WatermelonDB native module. Refer to docs for more details`,
Expand All @@ -112,7 +107,6 @@ export default class SQLiteAdapter implements DatabaseAdapter, SQLDatabaseAdapte
const clone = new SQLiteAdapter({
dbName: this._dbName,
schema: this.schema,
synchronous: this._dispatcherType === 'synchronous',
jsi: this._dispatcherType === 'jsi',
...(this.migrations ? { migrations: this.migrations } : {}),
...options,
Expand Down
29 changes: 1 addition & 28 deletions src/adapters/sqlite/integrationTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,39 +15,12 @@ const SQLiteAdapterTest = (spec) => {
commonTests().forEach((testCase) => {
const [name, test] = testCase
spec.it(name, async () => {
const adapter = new SQLiteAdapter({ schema: testSchema, synchronous: false })
const adapter = new SQLiteAdapter({ schema: testSchema, jsi: false })
invariant(adapter._dispatcherType === 'asynchronous', 'this should be asynchronous')
await test(new DatabaseAdapterCompat(adapter), SQLiteAdapter, {}, Platform.OS)
})
})
})
spec.describe('SQLiteAdapter (synchronous mode)', () => {
commonTests().forEach((testCase) => {
const [name, test] = testCase
spec.it(name, async () => {
const adapter = new SQLiteAdapter({ schema: testSchema, synchronous: true })

if (Platform.OS === 'ios') {
invariant(adapter._dispatcherType === 'synchronous', 'this should be synchronous')
} else {
invariant(
adapter._dispatcherType === 'asynchronous',
'this should be asynchronous - android does not support synchronous adapter',
)
return // no need to test - we've already run this exact same test
}

// TODO: Remove me. Temporary workaround for the race condition - wait until next macrotask to ensure that database has set up
await new Promise((resolve) => setTimeout(resolve, 0))
await test(
new DatabaseAdapterCompat(adapter),
SQLiteAdapter,
{ synchronous: true },
Platform.OS,
)
})
})
})
spec.describe('SQLiteAdapter (JSI mode)', () => {
commonTests().forEach((testCase) => {
const [name, test] = testCase
Expand Down
Loading

0 comments on commit 50f5d1b

Please sign in to comment.