Skip to content

Commit

Permalink
Clone additionalMethods to prevent mutation (#4)
Browse files Browse the repository at this point in the history
Closes #3
  • Loading branch information
vweevers authored Oct 13, 2019
1 parent 192bc9e commit 7b89eba
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 3 deletions.
2 changes: 1 addition & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ module.exports = function supports () {
encodings: manifest.encodings || false,

// Methods that are not part of abstract-leveldown or levelup
additionalMethods: manifest.additionalMethods || {}
additionalMethods: xtend(manifest.additionalMethods)
})
}
26 changes: 26 additions & 0 deletions test/cloneable.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
'use strict'

var supports = require('..')

// Every object in a manifest must have a unique identity, to avoid accidental
// mutation. In supports() we only shallowly clone the manifest object itself
// and additionalMethods. If in the future we add more objects to manifests,
// this test will break and we'll know to start performing a deep clone.
module.exports = function cloneable (t, manifest) {
var copy = supports(manifest)
verifyUnique(t, 'manifest', manifest, copy)
}

function verifyUnique (t, path, a, b) {
if (isObject(a) && isObject(b)) {
t.ok(a !== b, path + ' has unique identity')

Object.keys(a).forEach(function (key) {
verifyUnique(t, path + '.' + key, a[key], b[key])
})
}
}

function isObject (o) {
return typeof o === 'object' && o !== null
}
2 changes: 2 additions & 0 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

var xtend = require('xtend')
var shape = require('./shape')
var cloneable = require('./cloneable')

module.exports = function suite (test, testCommon) {
test('db has manifest', function (t) {
var db = testCommon.factory()
var manifest = db.supports

shape(t, manifest)
cloneable(t, manifest)

var before = xtend(manifest, {
additionalMethods: xtend(manifest.additionalMethods)
Expand Down
23 changes: 21 additions & 2 deletions test/self.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
var test = require('tape')
var supports = require('..')
var shape = require('./shape')
var cloneable = require('./cloneable')

test('no options', function (t) {
shape(t, supports())
cloneable(t, supports())
t.end()
})

Expand Down Expand Up @@ -44,13 +46,30 @@ test('truthy options', function (t) {

test('merges input objects without mutating them', function (t) {
var input1 = { bufferKeys: null, streams: false }
var input2 = { streams: true }
var input2 = { streams: true, additionalMethods: {} }
var manifest = supports(input1, input2)

manifest.foobar = true
manifest.additionalMethods.baz = true

t.same(input1, { bufferKeys: null, streams: false })
t.same(input2, { streams: true })
t.same(input2, { streams: true, additionalMethods: {} })
t.is(manifest.bufferKeys, false)
t.is(manifest.streams, true)
shape(t, manifest)
t.end()
})

test('inherits additionalMethods', function (t) {
var manifest = supports({ additionalMethods: { foo: true } }, {})
t.same(manifest.additionalMethods, { foo: true })
t.end()
})

test('does not merge additionalMethods', function (t) {
var input1 = { additionalMethods: { foo: true } }
var input2 = { additionalMethods: { bar: true } }
var manifest = supports(input1, input2)
t.same(manifest.additionalMethods, { bar: true })
t.end()
})

0 comments on commit 7b89eba

Please sign in to comment.