From 8c3aac50421dca62444221ed0675f93d98b85629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Rebours?= Date: Thu, 27 Jan 2022 22:07:52 +0100 Subject: [PATCH] fix MR comments --- CHANGELOG.md | 4 +- browser-version/lib/storage.react-native.js | 2 +- lib/datastore.js | 11 +-- lib/persistence.js | 4 +- test/persistence.async.test.js | 92 +++++++-------------- 5 files changed, 39 insertions(+), 74 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41068dc..23c07b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - The JSDoc is now much more exhaustive. - An auto-generated JSDoc file is generated: [API.md](./API.md). - Added `Datastore#dropDatabaseAsync` and its callback equivalent. -- The Error given when there the `Datastore#corruptAlertThreshold` is reached now has three properties: `dataLength` which is the amount of lines in the database file (excluding empty lines), `corruptItems` which is the amount of corrupted lines, `corruptionRate` which the rate of corruption between 0 and 1. +- The Error given when the `Datastore#corruptAlertThreshold` is reached now has three properties: `dataLength` which is the amount of lines in the database file (excluding empty lines), `corruptItems` which is the amount of corrupted lines, `corruptionRate` which the rate of corruption between 0 and 1. ### Changed - The `corruptionAlertThreshold` now doesn't take into account empty lines, and the error message is slightly changed. @@ -44,7 +44,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `Executor#buffer` & `Executor#queue` do not have the same signatures as before; - `Executor#push` replaced with `Executor#pushAsync` which is substantially different; - Storage modules : callback-based functions have been replaced with promise-based functions. - - Model module: it has been slightly re-written for clarity, but no changes in its interface was made. + - Model module: it has been slightly re-written for clarity, but no changes in its interface were made. - Typings were updated accordingly. ## Deprecated diff --git a/browser-version/lib/storage.react-native.js b/browser-version/lib/storage.react-native.js index ad7de93..1304822 100755 --- a/browser-version/lib/storage.react-native.js +++ b/browser-version/lib/storage.react-native.js @@ -3,7 +3,7 @@ * * This version is the React-Native version and uses [@react-native-async-storage/async-storage]{@link https://github.com/react-native-async-storage/async-storage}. * @module storageReactNative - * @see module:storagereact-native + * @see module:storageBrowser * @see module:storage * @private */ diff --git a/lib/datastore.js b/lib/datastore.js index 8cdc229..9f9b185 100755 --- a/lib/datastore.js +++ b/lib/datastore.js @@ -312,7 +312,7 @@ class Datastore extends EventEmitter { * @private * @type {null|number} */ - this.autocompactionIntervalId = null + this._autocompactionIntervalId = null } /** @@ -342,11 +342,12 @@ class Datastore extends EventEmitter { */ setAutocompactionInterval (interval) { const minInterval = 5000 + if (typeof interval !== 'number') throw new Error('Interval must be a number') const realInterval = Math.max(interval || 0, minInterval) this.stopAutocompaction() - this.autocompactionIntervalId = setInterval(() => { + this._autocompactionIntervalId = setInterval(() => { this.compactDatafile() }, realInterval) } @@ -355,9 +356,9 @@ class Datastore extends EventEmitter { * Stop autocompaction (do nothing if automatic compaction was not running) */ stopAutocompaction () { - if (this.autocompactionIntervalId) { - clearInterval(this.autocompactionIntervalId) - this.autocompactionIntervalId = null + if (this._autocompactionIntervalId) { + clearInterval(this._autocompactionIntervalId) + this._autocompactionIntervalId = null } } diff --git a/lib/persistence.js b/lib/persistence.js index f6fd43b..0efd7b1 100755 --- a/lib/persistence.js +++ b/lib/persistence.js @@ -115,7 +115,7 @@ class Persistence { * @see Persistence#compactDatafileAsync */ compactDatafile (callback) { - deprecate(callback => this.db.compactDatafile(callback), '@seald-io/nedb: calling Datastore#persistence#compactDatafile is deprecated, please use Datastore#compactDatafile, it will be removed in the next major version.')(callback) + deprecate(_callback => this.db.compactDatafile(_callback), '@seald-io/nedb: calling Datastore#persistence#compactDatafile is deprecated, please use Datastore#compactDatafile, it will be removed in the next major version.')(callback) } /** @@ -123,7 +123,7 @@ class Persistence { * @deprecated */ setAutocompactionInterval (interval) { - deprecate(interval => this.db.setAutocompactionInterval(interval), '@seald-io/nedb: calling Datastore#persistence#setAutocompactionInterval is deprecated, please use Datastore#setAutocompactionInterval, it will be removed in the next major version.')(interval) + deprecate(_interval => this.db.setAutocompactionInterval(_interval), '@seald-io/nedb: calling Datastore#persistence#setAutocompactionInterval is deprecated, please use Datastore#setAutocompactionInterval, it will be removed in the next major version.')(interval) } /** diff --git a/test/persistence.async.test.js b/test/persistence.async.test.js index 10ca915..c02df3b 100755 --- a/test/persistence.async.test.js +++ b/test/persistence.async.test.js @@ -11,6 +11,7 @@ const storage = require('../lib/storage') const { execFile, fork } = require('child_process') const { promisify } = require('util') const { ensureFileDoesntExistAsync } = require('../lib/storage') +const { once } = require('events') const Readable = require('stream').Readable describe('Persistence async', function () { @@ -337,29 +338,6 @@ describe('Persistence async', function () { assert.equal(doc2Reloaded, undefined) }) - it('Calling dropDatabase after the datafile was modified loads the new data', async () => { - await d.loadDatabaseAsync() - await d.insertAsync({ a: 1 }) - await d.insertAsync({ a: 2 }) - const data = d.getAllData() - const doc1 = data.find(doc => doc.a === 1) - const doc2 = data.find(doc => doc.a === 2) - assert.equal(data.length, 2) - assert.equal(doc1.a, 1) - assert.equal(doc2.a, 2) - - await fs.writeFile(testDb, '{"a":3,"_id":"aaa"}', 'utf8') - await d.loadDatabaseAsync() - const dataReloaded = d.getAllData() - const doc1Reloaded = dataReloaded.find(function (doc) { return doc.a === 1 }) - const doc2Reloaded = dataReloaded.find(function (doc) { return doc.a === 2 }) - const doc3Reloaded = dataReloaded.find(function (doc) { return doc.a === 3 }) - assert.equal(dataReloaded.length, 1) - assert.equal(doc3Reloaded.a, 3) - assert.equal(doc1Reloaded, undefined) - assert.equal(doc2Reloaded, undefined) - }) - it('When treating raw data, refuse to proceed if too much data is corrupt, to avoid data loss', async () => { const corruptTestFilename = 'workspace/corruptTest.db' const fakeData = '{"_id":"one","hello":"world"}\n' + 'Some corrupt data\n' + '{"_id":"two","hello":"earth"}\n' + '{"_id":"three","hello":"you"}\n' @@ -369,9 +347,8 @@ describe('Persistence async', function () { // Default corruptAlertThreshold d = new Datastore({ filename: corruptTestFilename }) await assert.rejects(() => d.loadDatabaseAsync(), err => { - assert.ok(Object.prototype.hasOwnProperty.call(err, 'corruptionRate')) - assert.ok(Object.prototype.hasOwnProperty.call(err, 'corruptItems')) - assert.ok(Object.prototype.hasOwnProperty.call(err, 'dataLength')) + assert.notEqual(err, undefined) + assert.notEqual(err, null) assert.equal(err.corruptionRate, 0.25) assert.equal(err.corruptItems, 1) assert.equal(err.dataLength, 4) @@ -384,9 +361,8 @@ describe('Persistence async', function () { await fs.writeFile(corruptTestFilename, fakeData, 'utf8') d = new Datastore({ filename: corruptTestFilename, corruptAlertThreshold: 0 }) await assert.rejects(() => d.loadDatabaseAsync(), err => { - assert.ok(Object.prototype.hasOwnProperty.call(err, 'corruptionRate')) - assert.ok(Object.prototype.hasOwnProperty.call(err, 'corruptItems')) - assert.ok(Object.prototype.hasOwnProperty.call(err, 'dataLength')) + assert.notEqual(err, undefined) + assert.notEqual(err, null) assert.equal(err.corruptionRate, 0.25) assert.equal(err.corruptItems, 1) assert.equal(err.dataLength, 4) @@ -847,40 +823,28 @@ describe('Persistence async', function () { // Loading it in a separate process that we will crash before finishing the loadDatabase const child = fork('test_lac/loadAndCrash.test', [], { stdio: 'inherit' }) - - await Promise.race([ - new Promise((resolve, reject) => child.on('error', reject)), - new Promise((resolve, reject) => { - child.on('exit', async function (code) { - try { - assert.equal(code, 1) // See test_lac/loadAndCrash.test.js - - assert.equal(await exists('workspace/lac.db'), true) - assert.equal(await exists('workspace/lac.db~'), true) - assert.equal((await fs.readFile('workspace/lac.db', 'utf8')).length, datafileLength) - assert.equal((await fs.readFile('workspace/lac.db~', 'utf8')).length, 5000) - - // Reload database without a crash, check that no data was lost and fs state is clean (no temp file) - const db = new Datastore({ filename: 'workspace/lac.db' }) - await db.loadDatabaseAsync() - assert.equal(await exists('workspace/lac.db'), true) - assert.equal(await exists('workspace/lac.db~'), false) - assert.equal((await fs.readFile('workspace/lac.db', 'utf8')).length, datafileLength) - - const docs = await db.findAsync({}) - assert.equal(docs.length, N) - for (i = 0; i < N; i += 1) { - docI = docs.find(d => d._id === 'anid_' + i) - assert.notEqual(docI, undefined) - assert.deepEqual({ hello: 'world', _id: 'anid_' + i }, docI) - } - resolve() - } catch (error) { - reject(error) - } - }) - }) - ]) + const [code] = await once(child, 'exit') + assert.equal(code, 1) // See test_lac/loadAndCrash.test.js + + assert.equal(await exists('workspace/lac.db'), true) + assert.equal(await exists('workspace/lac.db~'), true) + assert.equal((await fs.readFile('workspace/lac.db', 'utf8')).length, datafileLength) + assert.equal((await fs.readFile('workspace/lac.db~', 'utf8')).length, 5000) + + // Reload database without a crash, check that no data was lost and fs state is clean (no temp file) + const db = new Datastore({ filename: 'workspace/lac.db' }) + await db.loadDatabaseAsync() + assert.equal(await exists('workspace/lac.db'), true) + assert.equal(await exists('workspace/lac.db~'), false) + assert.equal((await fs.readFile('workspace/lac.db', 'utf8')).length, datafileLength) + + const docs = await db.findAsync({}) + assert.equal(docs.length, N) + for (i = 0; i < N; i += 1) { + docI = docs.find(d => d._id === 'anid_' + i) + assert.notEqual(docI, undefined) + assert.deepEqual({ hello: 'world', _id: 'anid_' + i }, docI) + } }) // Not run on Windows as there is no clean way to set maximum file descriptors. Not an issue as the code itself is tested. @@ -946,7 +910,7 @@ describe('Persistence async', function () { d.setAutocompactionInterval(5000) await d.insertAsync({ hello: 'world' }) await d.dropDatabaseAsync() - assert.equal(d.autocompactionIntervalId, null) + assert.equal(d._autocompactionIntervalId, null) assert.equal(d.getAllData().length, 0) assert.equal(await exists(testDb), false) })