From 90b1dc7110b7c68a41f6851596f13c881ba158a4 Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Fri, 10 Feb 2017 22:11:25 -0800 Subject: [PATCH 01/18] Add breaking test. --- test/del.js | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/del.js b/test/del.js index da80db9..4e5e5d6 100644 --- a/test/del.js +++ b/test/del.js @@ -122,6 +122,46 @@ test('del', function (t) { } }) +test('del with value', function (t) { + t.plan(4) + + var osm = osmdb({ + log: hyperlog(memdb(), { valueEncoding: 'json' }), + db: memdb(), + store: fdstore(4096, storefile) + }) + + var doc = { type: 'node', lat: 14, lon: -14, changeset: 'foobar' } + + osm.create(doc, function (err, id) { + t.ifError(err) + var v = { + lat: doc.lat, + lon: doc.lon, + changeset: doc.changeset + } + osm.del(id, { value: v }, function (err, node) { + t.ifError(err) + doQuery(id, node.key) + }) + }) + + function doQuery (id, version) { + osm.get(id, function (err, doc) { + t.ifError(err) + var expected = { + changeset: 'foobar', + id: id, + lat: 14, + lon: -14, + version: version, + deleted: true + } + t.deepEqual(doc, expected, 'correct query /w value') + }) + } +}) + function idcmp (a, b) { return a.id < b.id ? -1 : 1 } From fd6bfb385d133c7052eb23f09f9885cf2797691d Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Fri, 10 Feb 2017 22:11:42 -0800 Subject: [PATCH 02/18] Store an optional value on deletions. --- index.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 056ca21..2121f38 100644 --- a/index.js +++ b/index.js @@ -47,6 +47,10 @@ function DB (opts) { var v = row.value.v var d = row.value.d if (v && v.lat !== undefined && v.lon !== undefined) { + var v = row.value.v + var d = row.value.d + var k = row.value.k + if (k && v.lat !== undefined && v.lon !== undefined) { next(null, { type: 'put', point: ptf(v) }) } else if (d && Array.isArray(row.value.points)) { var pts = row.value.points.map(ptf) @@ -266,7 +270,15 @@ DB.prototype._getDocumentDeletionBatchOps = function (id, opts, cb) { fields.members.push.apply(fields.members, v.members) } }) - cb(null, [ { type: 'del', key: id, links: links, fields: fields } ]) + + // Use opts.value to set a value on hyperkv deletions. + if (opts.value) { + fields = xtend(fields, { + v: opts.value + }) + } + + cb(null, [ { type: 'del', key: key, links: links, fields: fields } ]) } } From 9a8a2a29cf945f7bb7aedcd54f3d1558ce0f3778 Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Fri, 10 Feb 2017 22:11:50 -0800 Subject: [PATCH 03/18] Update README. --- README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 4897ccb..071f17d 100644 --- a/README.md +++ b/README.md @@ -195,6 +195,9 @@ Delete a document at `id`. The options `opts` are passed to the underlying [hyperkv][4] instance. +A deletion tombstone may have a value associated with it. It will be set to the +value of `opts.value`, if set. + `cb(err, node)` fires with the underlying `node` in the hyperlog. ### osm.batch(rows, opts={}, cb) @@ -206,7 +209,7 @@ Each `row` in `rows` should have: * `row.type` - `'put'` or `'del'` * `row.key` or `row.id` - the id of the document (generated if not specified) * `row.links` - array of links to ancestor keys -* `row.value` - for puts, the value to store +* `row.value` - the value to store on a `put` or `del` ### osm.get(id, opts={}, cb) From b9b158c9063e731c80da1f7fd8c483ecd087625b Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Thu, 13 Jul 2017 12:52:57 -0700 Subject: [PATCH 04/18] refactor: singular deletion op --- index.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 2121f38..2022d81 100644 --- a/index.js +++ b/index.js @@ -202,8 +202,8 @@ DB.prototype.del = function (key, opts, cb) { }) } -// OsmId, Opts -> [OsmBatchOp] -DB.prototype._getDocumentDeletionBatchOps = function (id, opts, cb) { +// OsmId, Opts -> OsmBatchOp +DB.prototype._getDocumentDeletionBatchOp = function (id, opts, cb) { var self = this if (!opts || !opts.links) { @@ -271,6 +271,8 @@ DB.prototype._getDocumentDeletionBatchOps = function (id, opts, cb) { } }) + var res = { type: 'del', key: id, links: links, fields: fields } + // Use opts.value to set a value on hyperkv deletions. if (opts.value) { fields = xtend(fields, { @@ -278,7 +280,7 @@ DB.prototype._getDocumentDeletionBatchOps = function (id, opts, cb) { }) } - cb(null, [ { type: 'del', key: key, links: links, fields: fields } ]) + cb(null, res) } } @@ -317,9 +319,9 @@ DB.prototype.batch = function (rows, opts, cb) { if (--pending <= 0) done() } else if (row.type === 'del') { var xrow = xtend(opts, row) - self._getDocumentDeletionBatchOps(key, xrow, function (err, xrows) { + self._getDocumentDeletionBatchOp(key, {}, function (err, xrow) { if (err) return release(cb, err) - batch.push.apply(batch, xrows) + batch.push(xrow) if (--pending <= 0) done() }) } else { From 85db6131422ed3ff840fba425d51afd934115267 Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Thu, 13 Jul 2017 18:21:18 -0700 Subject: [PATCH 05/18] fix: fixes deletion writes to include values --- index.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/index.js b/index.js index 2022d81..858342f 100644 --- a/index.js +++ b/index.js @@ -192,7 +192,8 @@ DB.prototype.del = function (key, opts, cb) { { type: 'del', key: key, - links: opts.links + links: opts.links, + fields: opts.value ? { value: opts.value } : null } ] @@ -208,7 +209,7 @@ DB.prototype._getDocumentDeletionBatchOp = function (id, opts, cb) { if (!opts || !opts.links) { // Fetch all versions of the document ID - self.kv.get(id, function (err, docs) { + self.kv.get(id, { fields: true }, function (err, docs) { if (err) return cb(err) docs = mapObj(docs, function (version, doc) { @@ -275,9 +276,7 @@ DB.prototype._getDocumentDeletionBatchOp = function (id, opts, cb) { // Use opts.value to set a value on hyperkv deletions. if (opts.value) { - fields = xtend(fields, { - v: opts.value - }) + res.fields = xtend(res.fields, { v: opts.value }) } cb(null, res) @@ -318,8 +317,8 @@ DB.prototype.batch = function (rows, opts, cb) { batch.push(row) if (--pending <= 0) done() } else if (row.type === 'del') { - var xrow = xtend(opts, row) - self._getDocumentDeletionBatchOp(key, {}, function (err, xrow) { + var opts = row.fields ? { value: row.fields.value } : {} + self._getDocumentDeletionBatchOp(key, opts, function (err, xrow) { if (err) return release(cb, err) batch.push(xrow) if (--pending <= 0) done() From cee8a4340f00cdd55d008ccf4d85c7c5e641a4f7 Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Thu, 13 Jul 2017 18:22:00 -0700 Subject: [PATCH 06/18] fix: osm.get to include deletion values --- index.js | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/index.js b/index.js index 858342f..feeaa33 100644 --- a/index.js +++ b/index.js @@ -337,18 +337,17 @@ DB.prototype.get = function (key, opts, cb) { cb = opts opts = {} } - this.kv.get(key, function (err, docs) { + this.kv.get(key, { fields: true }, function (err, docs) { if (err) return cb(err) docs = mapObj(docs, function (version, doc) { - if (doc.deleted) { - return { - id: key, - version: version, - deleted: true - } - } else { - return doc.value + doc.v = xtend(doc.v, { + id: key, + version: version + }) + if (doc.d) { + doc.v.deleted = true } + return doc.v }) cb(null, docs) From 2309e06795974de27c1c03594a0292cd4c995161 Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Thu, 13 Jul 2017 18:22:24 -0700 Subject: [PATCH 07/18] test: fix up del-with-value test --- test/del.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/test/del.js b/test/del.js index 4e5e5d6..bcc84a4 100644 --- a/test/del.js +++ b/test/del.js @@ -123,13 +123,9 @@ test('del', function (t) { }) test('del with value', function (t) { - t.plan(4) + t.plan(5) - var osm = osmdb({ - log: hyperlog(memdb(), { valueEncoding: 'json' }), - db: memdb(), - store: fdstore(4096, storefile) - }) + var osm = makeOsm() var doc = { type: 'node', lat: 14, lon: -14, changeset: 'foobar' } @@ -147,8 +143,10 @@ test('del with value', function (t) { }) function doQuery (id, version) { - osm.get(id, function (err, doc) { + osm.get(id, function (err, heads) { t.ifError(err) + t.equals(Object.keys(heads).length, 1) + var actual = heads[Object.keys(heads)[0]] var expected = { changeset: 'foobar', id: id, @@ -157,7 +155,7 @@ test('del with value', function (t) { version: version, deleted: true } - t.deepEqual(doc, expected, 'correct query /w value') + t.deepEqual(actual, expected, 'correct query /w value') }) } }) From c90935b1deee3b60a8bdb4f81cc0633321acd69f Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Thu, 13 Jul 2017 18:23:58 -0700 Subject: [PATCH 08/18] refactor: rename key to id --- index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index feeaa33..2320433 100644 --- a/index.js +++ b/index.js @@ -332,16 +332,16 @@ DB.prototype.batch = function (rows, opts, cb) { }) } -DB.prototype.get = function (key, opts, cb) { +DB.prototype.get = function (id, opts, cb) { if (typeof opts === 'function') { cb = opts opts = {} } - this.kv.get(key, { fields: true }, function (err, docs) { + this.kv.get(id, { fields: true }, function (err, docs) { if (err) return cb(err) docs = mapObj(docs, function (version, doc) { doc.v = xtend(doc.v, { - id: key, + id: id, version: version }) if (doc.d) { From fa72da6560cb24befddeee5a5411952f2f0b6060 Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Thu, 13 Jul 2017 19:03:37 -0700 Subject: [PATCH 09/18] lint: formatting --- test/del.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/del.js b/test/del.js index bcc84a4..a9ba2ab 100644 --- a/test/del.js +++ b/test/del.js @@ -148,7 +148,7 @@ test('del with value', function (t) { t.equals(Object.keys(heads).length, 1) var actual = heads[Object.keys(heads)[0]] var expected = { - changeset: 'foobar', + changeset: 'foobar', id: id, lat: 14, lon: -14, From b112ac40055f2bb669b3f121b97033f6d19b050f Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Thu, 13 Jul 2017 19:06:11 -0700 Subject: [PATCH 10/18] refactor: set id and version --- index.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/index.js b/index.js index 2320433..16a242f 100644 --- a/index.js +++ b/index.js @@ -213,15 +213,14 @@ DB.prototype._getDocumentDeletionBatchOp = function (id, opts, cb) { if (err) return cb(err) docs = mapObj(docs, function (version, doc) { - if (doc.deleted) { - return { - id: id, - version: version, - deleted: true - } - } else { - return doc.value + doc.v = xtend(doc.v, { + id: id, + version: version + }) + if (doc.d) { + doc.v.deleted = true } + return doc.v }) handleLinks(docs) From dd620c24ccb9c8cfa9462880e381d3b4293907d0 Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Thu, 13 Jul 2017 19:06:19 -0700 Subject: [PATCH 11/18] test: fix test to look for id and version --- test/fork_count.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/fork_count.js b/test/fork_count.js index 611792a..706c39a 100644 --- a/test/fork_count.js +++ b/test/fork_count.js @@ -74,8 +74,8 @@ test('count forks', function (t) { osm0.get(names.C, function (err, values) { t.ifError(err) var expected = {} - expected[versions.C[1]] = { type: 'node', lat: 62.5, lon: -146.2 } - expected[versions.C[2]] = { type: 'node', lat: 62.4, lon: -146.3 } + expected[versions.C[1]] = { type: 'node', lat: 62.5, lon: -146.2, version: versions.C[1], id: names.C } + expected[versions.C[2]] = { type: 'node', lat: 62.4, lon: -146.3, version: versions.C[2], id: names.C } t.deepEqual(values, expected, 'expected fork values') }) } From e1c95345f6bfa1e53631089c2139245ea32d7d67 Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Thu, 13 Jul 2017 19:06:55 -0700 Subject: [PATCH 12/18] fix: modify opts instead of rewriting it --- index.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 16a242f..386f34e 100644 --- a/index.js +++ b/index.js @@ -196,6 +196,8 @@ DB.prototype.del = function (key, opts, cb) { fields: opts.value ? { value: opts.value } : null } ] + opts.value = undefined + opts.links = undefined self.batch(rows, opts, function (err, nodes) { if (err) cb(err) @@ -316,7 +318,7 @@ DB.prototype.batch = function (rows, opts, cb) { batch.push(row) if (--pending <= 0) done() } else if (row.type === 'del') { - var opts = row.fields ? { value: row.fields.value } : {} + opts = row.fields ? { value: row.fields.value } : {} self._getDocumentDeletionBatchOp(key, opts, function (err, xrow) { if (err) return release(cb, err) batch.push(xrow) From 597352634d57dfd1df0bdec5218d25348237e2ab Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Thu, 13 Jul 2017 19:27:25 -0700 Subject: [PATCH 13/18] fix: include deletion values on queried points --- index.js | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/index.js b/index.js index 386f34e..8a8bcc1 100644 --- a/index.js +++ b/index.js @@ -341,14 +341,7 @@ DB.prototype.get = function (id, opts, cb) { this.kv.get(id, { fields: true }, function (err, docs) { if (err) return cb(err) docs = mapObj(docs, function (version, doc) { - doc.v = xtend(doc.v, { - id: id, - version: version - }) - if (doc.d) { - doc.v.deleted = true - } - return doc.v + return kvDocToOsmDoc(version, id, doc) }) cb(null, docs) @@ -476,10 +469,10 @@ DB.prototype._collectNodeAndReferers = function (version, seenAccum, cb) { } function addDocFromNode (node) { - if (node && node.value && node.value.k && node.value.v) { - addDoc(node.value.k, node.key, node.value.v) - } else if (node && node.value && node.value.d) { - addDoc(node.value.d, node.key, {deleted: true}) + if (node && node.value && (node.value.k || node.value.d)) { + var id = node.value.k || node.value.d + var doc = kvDocToOsmDoc(node.key, id, node.value) + addDoc(id, node.key, doc) } } @@ -630,3 +623,18 @@ function kdbPointToVersion (pt) { function ptf (x) { return [ Number(x.lat), Number(x.lon) ] } + +// Populates an OsmDoc with its version, id, and whether it's a deletion +// tombstone. +// OsmVersion, OsmId, OsmDoc -> OsmDoc +function kvDocToOsmDoc (version, id, doc) { + var res = {} + res = xtend(doc.v, { + id: id, + version: version + }) + if (doc.d) { + res.deleted = true + } + return res +} From 3d2da15e54b18ff3d7f235fddde4d3f7cf817043 Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Thu, 13 Jul 2017 19:27:51 -0700 Subject: [PATCH 14/18] test: add test for querying deleted points too --- test/del.js | 51 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/test/del.js b/test/del.js index a9ba2ab..223cd7f 100644 --- a/test/del.js +++ b/test/del.js @@ -138,11 +138,11 @@ test('del with value', function (t) { } osm.del(id, { value: v }, function (err, node) { t.ifError(err) - doQuery(id, node.key) + doGet(id, node.key) }) }) - function doQuery (id, version) { + function doGet (id, version) { osm.get(id, function (err, heads) { t.ifError(err) t.equals(Object.keys(heads).length, 1) @@ -160,6 +160,53 @@ test('del with value', function (t) { } }) +test('way with a deleted node with value', function (t) { + t.plan(4) + + var osm = makeOsm() + + var doc1 = { type: 'node', lat: 1, lon: -1, changeset: 'foobar' } + var doc2 = { type: 'node', lat: 14, lon: -14, changeset: 'foobar' } + + // osm.create(doc1, function (err, id1) { + // t.ifError(err) + osm.create(doc2, function (err, id2) { + t.ifError(err) + var v = { + lat: doc2.lat, + lon: doc2.lon, + changeset: doc2.changeset + } + osm.del(id2, { value: v }, function (err, node) { + t.ifError(err) + // osm.create({ + // type: 'way', + // refs: [id1, id2] + // }, function (err) { + // t.error(err) + doQuery(id2, node.key) + // }) + }) + }) + // }) + + function doQuery (id, version) { + var q = [[-90,90],[-180,180]] + var expected = { + changeset: 'foobar', + id: id, + lat: 14, + lon: -14, + version: version, + deleted: true + } + osm.query(q, function (err, res) { + t.ifError(err) + t.deepEqual(res, [expected], 'full coverage query') + }) + } +}) + function idcmp (a, b) { return a.id < b.id ? -1 : 1 } From 8638db34d75efa4d2a61fc27f5130809678e49b7 Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Fri, 14 Jul 2017 09:56:01 -0700 Subject: [PATCH 15/18] docs: clarify what deleted docs look like --- README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index 071f17d..810fe67 100644 --- a/README.md +++ b/README.md @@ -216,8 +216,7 @@ Each `row` in `rows` should have: Get a document as `cb(err, docs)` by its OSM `id`. `docs` is an object mapping hyperlog hashes to current document values. If a -document has been deleted, it will only have the properties `{ id: , -version: , deleted: true}`. +document has been deleted, it will have the property `{ deleted: true }` set. ### osm.query(q, opts, cb) From 25c5234d34452e69ff981dbceb0509a1ef983920 Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Fri, 14 Jul 2017 09:57:18 -0700 Subject: [PATCH 16/18] test: clean up query + deleted node test --- test/del.js | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/test/del.js b/test/del.js index 223cd7f..4cc30d9 100644 --- a/test/del.js +++ b/test/del.js @@ -160,35 +160,25 @@ test('del with value', function (t) { } }) -test('way with a deleted node with value', function (t) { +test('query deleted node with value', function (t) { t.plan(4) var osm = makeOsm() - var doc1 = { type: 'node', lat: 1, lon: -1, changeset: 'foobar' } - var doc2 = { type: 'node', lat: 14, lon: -14, changeset: 'foobar' } + var doc = { type: 'node', lat: 14, lon: -14, changeset: 'foobar' } - // osm.create(doc1, function (err, id1) { - // t.ifError(err) - osm.create(doc2, function (err, id2) { + osm.create(doc, function (err, id2) { + t.ifError(err) + var v = { + lat: doc.lat, + lon: doc.lon, + changeset: doc.changeset + } + osm.del(id2, { value: v }, function (err, node) { t.ifError(err) - var v = { - lat: doc2.lat, - lon: doc2.lon, - changeset: doc2.changeset - } - osm.del(id2, { value: v }, function (err, node) { - t.ifError(err) - // osm.create({ - // type: 'way', - // refs: [id1, id2] - // }, function (err) { - // t.error(err) - doQuery(id2, node.key) - // }) - }) + doQuery(id2, node.key) }) - // }) + }) function doQuery (id, version) { var q = [[-90,90],[-180,180]] From e74851bc130f2279a8c31197efcca550902a546d Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Mon, 17 Jul 2017 13:51:11 -0700 Subject: [PATCH 17/18] fix: bad merge --- index.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/index.js b/index.js index 8a8bcc1..9f1add2 100644 --- a/index.js +++ b/index.js @@ -46,9 +46,6 @@ function DB (opts) { if (!row.value) return null var v = row.value.v var d = row.value.d - if (v && v.lat !== undefined && v.lon !== undefined) { - var v = row.value.v - var d = row.value.d var k = row.value.k if (k && v.lat !== undefined && v.lon !== undefined) { next(null, { type: 'put', point: ptf(v) }) From 5c0b88f84fa5dd113093ec2b37e4d3096e529115 Mon Sep 17 00:00:00 2001 From: Stephen Whitmore Date: Mon, 17 Jul 2017 23:19:09 -0700 Subject: [PATCH 18/18] refactor: add comment explaining indexing logic --- index.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index 9f1add2..604e973 100644 --- a/index.js +++ b/index.js @@ -45,11 +45,12 @@ function DB (opts) { map: function (row, next) { if (!row.value) return null var v = row.value.v - var d = row.value.d - var k = row.value.k - if (k && v.lat !== undefined && v.lon !== undefined) { + + // Index the new point + if (row.value.k && v.lat !== undefined && v.lon !== undefined) { next(null, { type: 'put', point: ptf(v) }) - } else if (d && Array.isArray(row.value.points)) { + // Index a deleted point or way + } else if (row.value.d && Array.isArray(row.value.points)) { var pts = row.value.points.map(ptf) next(null, { type: 'put', points: pts }) } else next()