diff --git a/CHANGELOG.md b/CHANGELOG.md index 479441a..815fc5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,16 @@ -# HEAD +# 1.0.0 + +## API-BREAKING CHANGES + +- Changed `fdb.open()` to return a directory reference *synchronously*. This matches the semantics of the new client API. +- Removed support for the older nan-based native module. The newer napi code works on all officially supported versions of nodejs, as well as node 8.16. If you're using a version of nodejs older than that, its past time to upgrade. +- Changed `db.get()` / `txn.get()` to return `undefined` rather than `null` if the object doesn't exist. This is because null is a valid tuple value. +- Changed `db.getKey()` / `txn.getKey()` to return `undefined` if the requested key is not in the db / transaction's subspace +- Deprecated `tn.scopedTo`. Use `tn.at()`. + +--- + +### Other changes - Pulled out database / transaction scope information (prefix and key value transformers) into a separate class 'subspace' to more closely match the other bindings. This is currently internal-only but it will be exposed when I'm more confident about the API. - Added support in the tuple encoder for non-array values, which are functionally equivalent to their array-wrapped versions. Eg this will now work: @@ -11,12 +23,12 @@ Note that `db.at(['prefix']).set(['key'], 'value')` is equivalent to `db.at(['pr (The mental model is that tuple.pack(arr1) + tuple.pack(arr2) is always equivalent to tuple.pack(arr1 + arr2), so `[]` encodes to an empty byte string, but `[null]` encodes to `[0]`). -- Removed support for the older nan-based native module. The newer napi code works on all officially supported versions of nodejs, as well as node 8.16. So this should be pretty safe at this point. - Updated API to support foundationdb 620 - Updated the binding tester to conform to version 620's changes - Fixed a spec conformance bug in the tuple encoder's handling of extremely large negative integers -- Changed db.get() / txn.get() to return `undefined` rather than `null` if the object doesn't exist. This is because null is a valid tuple value. - Added the directory layer (!!) +- Added doc comments for a lot of methods in Transaction + # 0.10.7 diff --git a/lib/database.ts b/lib/database.ts index dab928f..b80c8b7 100644 --- a/lib/database.ts +++ b/lib/database.ts @@ -98,7 +98,7 @@ export default class Database { return this.doTransaction(tn => tn.snapshot().get(key)) } - getKey(selector: KeyIn | KeySelector): Promise { + getKey(selector: KeyIn | KeySelector): Promise { return this.doTransaction(tn => tn.snapshot().getKey(selector)) } getVersionstampPrefixedValue(key: KeyIn): Promise<{stamp: Buffer, value?: ValOut} | null> { diff --git a/lib/index.ts b/lib/index.ts index 24e83e7..8f1d333 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -129,8 +129,8 @@ export function open(clusterFile?: string, dbOpts?: DatabaseOptions) { // *** Some deprecated stuff to remove: -/** @deprecated Async database connection has been removed from FDB. Call open() directly. */ -export const openSync = deprecate(open, 'Async database connection has been removed from FDB. Call open() directly.') +/** @deprecated This method will be removed in a future version. Call fdb.open() directly - it is syncronous too. */ +export const openSync = deprecate(open, 'This method will be removed in a future version. Call fdb.open() directly - it is syncronous too.') // Previous versions of this library allowed you to create a cluster and then // create database objects from it. This was all removed from the C API. We'll diff --git a/lib/native.ts b/lib/native.ts index e160b8b..df92bf9 100644 --- a/lib/native.ts +++ b/lib/native.ts @@ -35,8 +35,10 @@ export interface NativeTransaction { get(key: NativeValue, isSnapshot: boolean): Promise get(key: NativeValue, isSnapshot: boolean, cb: Callback): void - getKey(key: NativeValue, orEqual: boolean, offset: number, isSnapshot: boolean): Promise - getKey(key: NativeValue, orEqual: boolean, offset: number, isSnapshot: boolean, cb: Callback): void + // getKey always returns a value - but it will return the empty buffer or a + // buffer starting in '\xff' if there's no other keys to find. + getKey(key: NativeValue, orEqual: boolean, offset: number, isSnapshot: boolean): Promise + getKey(key: NativeValue, orEqual: boolean, offset: number, isSnapshot: boolean, cb: Callback): void set(key: NativeValue, val: NativeValue): void clear(key: NativeValue): void diff --git a/lib/transaction.ts b/lib/transaction.ts index 29adce6..18aeeaf 100644 --- a/lib/transaction.ts +++ b/lib/transaction.ts @@ -29,7 +29,6 @@ import { import { UnboundStamp, packVersionstamp, - packPrefixedVersionstamp, packVersionstampPrefixSuffix } from './versionstamp' import Subspace, { GetSubspace } from './subspace' @@ -77,22 +76,66 @@ interface TxnCtx { toBake: null | BakeItem[] } -// NativeValue is string | Buffer because the C code accepts either format. -// But all values returned from methods will actually just be Buffer. +/** + * This class wraps a foundationdb transaction object. All interaction with the + * data in a foundationdb database happens through a transaction. For more + * detail about how to model your queries, see the [transaction chapter of the + * FDB developer + * guide](https://apple.github.io/foundationdb/developer-guide.html?#transaction-basics). + * + * You should never create transactions directly. Instead, open a database and + * call `await db.doTn(async tn => {...})`. + * + * ```javascript + * const db = fdb.open() + * const val = await db.doTn(async tn => { + * // Use the transaction in this block. The transaction will be automatically + * // committed (and potentially retried) after this block returns. + * tn.set('favorite color', 'hotpink') + * return await tn.get('another key') + * }) + * ``` + * + * --- + * + * This class has 4 template parameters - which is kind of messy. They're used + * to make the class typesafe in the face of key and value transformers. These + * parameters should be automatically inferred, but sometimes you will need to + * specify them explicitly. They are: + * + * @param KeyIn The type for keys passed by the user into functions (eg `get(k: + * KeyIn)`). Defaults to string | Buffer. Change this by scoping the transaction + * with a subspace with a key transformer. Eg + * `txn.at(fdb.root.withKeyEncoding(fdb.tuple)).get([1, 2, 3])`. + * @param KeyOut The type of keys returned by methods which return keys - like + * `getKey(..) => Promise`. Unless you have a KV transformer, this will + * be Buffer. + * @param ValIn The type of values passed into transaction functions, like + * `txn.set(key, val: ValIn)`. By default this is string | Buffer. Override this + * by applying a value transformer to your subspace. + * @param ValOut The type of database values returned by functions. Eg, + * `txn.get(...) => Promise`. Defaults to Buffer, but if you + * apply a value transformer this will change. + */ export default class Transaction { - _tn: NativeTransaction + /** @internal */ _tn: NativeTransaction isSnapshot: boolean subspace: Subspace // Copied out from scope for convenience, since these are so heavily used. Not // sure if this is a good idea. - _keyEncoding: Transformer - _valueEncoding: Transformer + private _keyEncoding: Transformer + private _valueEncoding: Transformer - _ctx: TxnCtx + private _ctx: TxnCtx - /** NOTE: Do not call this directly. Instead transactions should be created via db.doTn(...) */ + /** + * NOTE: Do not call this directly. Instead transactions should be created + * via db.doTn(...) + * + * @internal + */ constructor(tn: NativeTransaction, snapshot: boolean, subspace: Subspace, // keyEncoding: Transformer, valueEncoding: Transformer, @@ -115,6 +158,8 @@ export default class Transaction(body: (tn: Transaction) => Promise, opts?: TransactionOptions): Promise { // Logic described here: // https://apple.github.io/foundationdb/api-c.html#c.fdb_transaction_on_error @@ -182,6 +227,7 @@ export default class Transaction {...}). @@ -204,6 +250,12 @@ export default class Transaction get(key: KeyIn, cb: Callback): void get(key: KeyIn, cb?: Callback) { @@ -216,17 +268,38 @@ export default class Transaction val == null ? undefined : this._valueEncoding.unpack(val)) } + /** Checks if the key exists in the database. This is just a shorthand for + * tn.get() !== undefined. + */ exists(key: KeyIn): Promise { const keyBuf = this._keyEncoding.pack(key) - return this._tn.get(keyBuf, this.isSnapshot).then(val => val != null) + return this._tn.get(keyBuf, this.isSnapshot).then(val => val != undefined) } - getKey(_sel: KeyIn | KeySelector): Promise { + /** + * Find and return the first key which matches the specified key selector + * inside the given subspace. Returns undefined if no key matching the + * selector falls inside the current subspace. + * + * If you pass a key instead of a selector, this method will find the first + * key >= the specified key. Aka `getKey(someKey)` is the equivalent of + * `getKey(keySelector.firstGreaterOrEqual(somekey))`. + * + * Note that this method is a little funky in the root subspace: + * + * - We cannot differentiate between "no smaller key found" and "found the + * empty key ('')". To make the API more consistent, we assume you aren't + * using the empty key in your dataset. + * - If your key selector looks forward in the dataset, this method may find + * and return keys in the system portion (starting with '\xff'). + */ + getKey(_sel: KeySelector | KeyIn): Promise { const sel = keySelector.from(_sel) return this._tn.getKey(this._keyEncoding.pack(sel.key), sel.orEqual, sel.offset, this.isSnapshot) - .then(keyOrNull => ( - keyOrNull != null ? this._keyEncoding.unpack(keyOrNull) : null - )) + .then(key => { + if (key.length === 0 || !this.subspace.contains(key)) return undefined + else return this._keyEncoding.unpack(key) + }) } /** Set the specified key/value pair in the database */ @@ -246,7 +319,7 @@ export default class Transaction ({more: r.more, results: this._encodeRangeResult(r.results)})) } + /** + * This method is functionally the same as *getRange*, but values are returned + * in the batches they're delivered in from the database. This method is + * present because it may be marginally faster than `getRange`. + * + * Example: + * + * ``` + * for await (const batch of tn.getRangeBatch(0, 1000)) { + * for (let k = 0; k < batch.length; k++) { + * const [key, val] = batch[k] + * // ... + * } + * } + * ``` + * + * @see Transaction.getRange + */ async *getRangeBatch( _start: KeyIn | KeySelector, // Consider also supporting string / buffers for these. _end?: KeyIn | KeySelector, // If not specified, start is used as a prefix. @@ -326,6 +417,46 @@ export default class Transaction, // Consider also supporting string / buffers for these. end?: KeyIn | KeySelector, @@ -337,6 +468,15 @@ export default class Transaction, end?: KeyIn | KeySelector, // if undefined, start is used as a prefix. @@ -355,7 +495,12 @@ export default class Transaction(item: T, transformer: Transformer, code: Buffer | null) { + private _addBakeItem(item: T, transformer: Transformer, code: Buffer | null) { if (transformer.bakeVersionstamp) { const scope = this._ctx if (scope.toBake == null) scope.toBake = [] @@ -559,19 +705,23 @@ export default class Transaction { const val = await this._tn.get(this._keyEncoding.pack(key), this.isSnapshot) return val == null ? null diff --git a/lib/transformer.ts b/lib/transformer.ts index df7f48d..4c1368f 100644 --- a/lib/transformer.ts +++ b/lib/transformer.ts @@ -1,7 +1,7 @@ // The transformer type is used to transparently translate keys and values // through an encoder and decoder function. -import {asBuf, concat2, strInc} from './util' +import {asBuf, concat2, strInc, startsWith} from './util' import {UnboundStamp} from './versionstamp' export type Transformer = { @@ -46,6 +46,7 @@ export const prefixTransformer = (prefix: string | Buffer, inner: Trans return concat2(_prefix, asBuf(innerVal)) }, unpack(buf: Buffer) { + if (!startsWith(buf, _prefix)) throw Error('Cannot unpack key outside of prefix range.') return inner.unpack(buf.slice(_prefix.length)) }, } diff --git a/scripts/bindingtester.ts b/scripts/bindingtester.ts index 777bd19..9769f0d 100755 --- a/scripts/bindingtester.ts +++ b/scripts/bindingtester.ts @@ -725,9 +725,13 @@ const makeMachine = (db: Database, initialName: Buffer) => { try { await operations[opcode](operand, ...oper) } catch (e) { + if (verbose) console.log('Exception:', e.message) if (opcode.startsWith('DIRECTORY_')) { - if (!(e instanceof DirectoryError)) throw e - if (verbose) console.log('Database exception', e.message) + // For some reason we absorb all errors here rather than just + // directory errors. This is probably a bug in the fuzzer, but eh. See + // seed 3079719521. + + // if (!(e instanceof DirectoryError)) throw e if (([ 'DIRECTORY_CREATE_SUBSPACE', diff --git a/src/transaction.cpp b/src/transaction.cpp index 8062a58..5a0a51b 100644 --- a/src/transaction.cpp +++ b/src/transaction.cpp @@ -184,6 +184,13 @@ static MaybeValue getKey(napi_env env, FDBFuture* future, fdb_error_t* errOut) { *errOut = fdb_future_get_key(future, &key, &len); if (UNLIKELY(*errOut)) return wrap_null(); + // get_key can't / doesn't differentiate between returning the empty key ("") + // or returning *no* key (ie, nothing exists which matches the given + // selector). I'm not sure what the behaviour should be if you call getKey() + // and we return an empty string here - that could either match the empty key, + // or match *no* key. + // There is a note about this here: + // https://apple.github.io/foundationdb/developer-guide.html?#key-selectors napi_value result; TRY(napi_create_buffer_copy(env, (size_t)len, (void *)key, NULL, &result)); return wrap_ok(result); diff --git a/test/kv.ts b/test/kv.ts index 2019f56..a516994 100644 --- a/test/kv.ts +++ b/test/kv.ts @@ -7,7 +7,7 @@ import { bufToNum, withEachDb, } from './util' -import {MutationType, tuple, TupleItem, encoders, Watch} from '../lib' +import {MutationType, tuple, TupleItem, encoders, Watch, keySelector} from '../lib' process.on('unhandledRejection', err => { throw err }) @@ -39,6 +39,16 @@ withEachDb(db => describe('key value functionality', () => { }) }) + it.skip("returns undefined when keys don't exist in get() and getKey", async () => { + await db.doTn(async tn => { + console.log(await tn.getKey('doesnotexist')) + // assert.strictEqual(await tn.get('doesnotexist'), undefined) + assert.strictEqual(await tn.getKey('doesnotexist'), undefined) + }) + // assert.strictEqual(await db.get('doesnotexist'), undefined) + // assert.strictEqual(await db.getKey('doesnotexist'), undefined) + }) + it('reads its writes in separate transactions', async () => { const val = Buffer.from('hi there') @@ -111,6 +121,31 @@ withEachDb(db => describe('key value functionality', () => { assert(txnAttempts > concurrentWrites) }) + describe('getKey', () => { + it('returns the exact key requested', async () => { + await db.set('x', 'y') + assert.strictEqual((await db.getKey('x'))?.toString(), 'x') + }) + + it('returns undefined if there is no key that matches', async () => { + assert.strictEqual(await db.getKey('doesnotexist'), undefined) + }) + + it('returns the key that matches if a key selector is passed', async () => { + await db.set('a', 'y') + await db.set('b', 'y') + assert.strictEqual((await db.getKey(keySelector.firstGreaterThan('a')))?.toString(), 'b') + }) + + it('returns undefined if the key selector matches outside of the subspace range', async () => { + await db.at('b').set('key', 'val') + + const _db = db.at('a') + + assert.strictEqual(await _db.getKey(keySelector('', true, 0)), undefined) + }) + }) + describe('version stamps', () => { it('handles setVersionstampSuffixedKey correctly', async () => { const keyPrefix = Buffer.from('hi there') @@ -279,9 +314,9 @@ withEachDb(db => describe('key value functionality', () => { }) describe('watch', () => { - it('getAndWatch returns null for empty keys', async () => { + it('getAndWatch returns undefined for empty keys', async () => { const watch = await db.getAndWatch('hi') - assert.equal(watch.value, null) + assert.strictEqual(watch.value, undefined) await db.set('hi', 'yo') assert.strictEqual(true, await watch.promise) }) diff --git a/tsconfig.json b/tsconfig.json index 8aae14d..f91c329 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -55,5 +55,6 @@ /* Experimental Options */ // "experimentalDecorators": true, /* Enables experimental support for ES7 decorators. */ // "emitDecoratorMetadata": true, /* Enables experimental support for emitting type metadata for decorators. */ + "stripInternal": true, } }