Skip to content

Commit

Permalink
feat: added PowersyncDatabase options validation
Browse files Browse the repository at this point in the history
  • Loading branch information
stevensJourney committed Aug 19, 2024
1 parent 4fc1de3 commit eeae6b5
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/dull-dancers-judge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@powersync/common': patch
---

Added basic validations for required options in `PowerSyncDatabase` constructor.
11 changes: 9 additions & 2 deletions packages/common/src/client/AbstractPowerSyncDatabase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,21 +182,28 @@ export abstract class AbstractPowerSyncDatabase extends BaseObserver<PowerSyncDB
constructor(options: PowerSyncDatabaseOptions); // Note this is important for extending this class and maintaining API compatibility
constructor(protected options: PowerSyncDatabaseOptions) {
super();
const { database } = options;

const { database, schema } = options;

if (false == schema instanceof Schema) {
throw new Error('The `schema` option should be provided and should be an instance of `Schema`.');
}

if (isDBAdapter(database)) {
this._database = database;
} else if (isSQLOpenFactory(database)) {
this._database = database.openDB();
} else if (isPowerSyncDatabaseOptionsWithSettings(options)) {
this._database = this.openDBAdapter(options);
} else {
throw new Error('The provided `database` option is invalid.');
}

this.bucketStorageAdapter = this.generateBucketStorageAdapter();
this.closed = false;
this.currentStatus = new SyncStatus({});
this.options = { ...DEFAULT_POWERSYNC_DB_OPTIONS, ...options };
this._schema = options.schema;
this._schema = schema;
this.ready = false;
this.sdkVersion = '';
// Start async init
Expand Down
3 changes: 2 additions & 1 deletion packages/common/src/client/SQLOpenFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ export interface SQLOpenFactory {
* Tests if the input is a {@link SQLOpenOptions}
*/
export const isSQLOpenOptions = (test: any): test is SQLOpenOptions => {
return typeof test == 'object' && 'dbFilename' in test;
// typeof null is `object`, but you cannot use the `in` operator on `null.
return test && typeof test == 'object' && 'dbFilename' in test;
};

/**
Expand Down
91 changes: 90 additions & 1 deletion packages/web/tests/open.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AbstractPowerSyncDatabase } from '@powersync/common';
import { AbstractPowerSyncDatabase, Schema } from '@powersync/common';
import {
PowerSyncDatabase,
WASQLiteDBAdapter,
Expand Down Expand Up @@ -127,4 +127,93 @@ describe('Open Methods', () => {
expect(sharedSpy).toBeCalledTimes(0);
expect(dedicatedSpy).toBeCalledTimes(0);
});

/**
* TypeScript should prevent this kind of error. This scenario could happen
* in pure JavaScript with no language server checking types.
*/
it('Should throw if schema setting is not valid', async () => {
const schemaError = 'The `schema` option should be provided';

expect(
() =>
new PowerSyncDatabase({
database: { dbFilename: 'test.sqlite' },
// @ts-expect-error
schema: null
})
).throws(schemaError);

expect(
() =>
new PowerSyncDatabase({
database: { dbFilename: 'test.sqlite' },
// @ts-expect-error
schema: {}
})
).throws(schemaError);

expect(
() =>
new PowerSyncDatabase({
database: { dbFilename: 'test.sqlite' },
// @ts-expect-error
schema: 'schema'
})
).throws(schemaError);

expect(
() =>
new PowerSyncDatabase({
database: { dbFilename: 'test.sqlite' },
// @ts-expect-error
schema: undefined
})
).throws(schemaError);

// An Extended class should be fine
class ExtendedSchema extends Schema {}

const extendedClient = new PowerSyncDatabase({
database: { dbFilename: 'test.sqlite' },
schema: new ExtendedSchema([])
});

await extendedClient.close();
});

/**
* TypeScript should prevent this kind of error. This scenario could happen
* in pure JavaScript with no language server checking types.
*/
it('Should throw if database setting is not valid', async () => {
const dbError = 'The provided `database` option is invalid.';

expect(
() =>
new PowerSyncDatabase({
// @ts-expect-error
database: null,
schema: new Schema([])
})
).throws(dbError);

expect(
() =>
new PowerSyncDatabase({
// @ts-expect-error
database: {},
schema: new Schema([])
})
).throws(dbError);

expect(
() =>
new PowerSyncDatabase({
// @ts-expect-error
database: 'db.sqlite',
schema: new Schema([])
})
).throws(dbError);
});
});

0 comments on commit eeae6b5

Please sign in to comment.