workshopper-adventure-storage - npm Package Compare versions

Comparing version 2.0.3 to 3.0.0

index.js

@@ -16,3 +16,3 @@ var fs = require('fs')
    */
-  function reset (cb) {
+  function reset () {
     rimraf.sync(dir)
@@ -26,3 +26,7 @@ }
     mkdirp.sync(dir)
-    fs.writeFileSync(fileName(name), JSON.stringify(data))
+    try {
+      fs.writeFileSync(fileName(name), JSON.stringify(data, null, 2))
+    } catch (e) {
+      // TODO: write this error in a log
+    }
   }
@@ -33,6 +37,16 @@
    */
-  function get (name, cb) {
+  function get (name) {
     var file = fileName(name)
-    var fileData = fs.readFileSync(file, 'utf8')
-    return JSON.parse(fileData)
+    try {
+      var fileData = fs.readFileSync(file, 'utf8')
+    } catch (e) {
+      // TODO: write this error in a log
+      return null
+    }
+    try {
+      return JSON.parse(fileData)
+    } catch (e) {
+      // TODO: write this error in a log
+      return null
+    }
   }
@@ -39,0 +53,0 @@

package.json

 {
   "name": "workshopper-adventure-storage",
-  "version": "2.0.3",
+  "version": "3.0.0",
   "description": "",
@@ -5,0 +5,0 @@ "main": "index.js",

README.md

@@ -40,3 +40,4 @@ # workshopper-adventure-storage
 
-## storage.save('name', data')
+## storage.save('name', data)
+
 JSON encodes and writes a file to the storage directory. The following will
@@ -43,0 +44,0 @@ save the file as `index.json`.

test/index.js

 var storage = require('..')
 var fs = require('fs')
 var expect = require('chai').expect
+var rimraf = require('rimraf')
+var path = require('path')
+var mkdirp = require('mkdirp')
 
 describe('Storage', function () {
-  var fileName = './foo/bar/index.json'
-  var invalidJson = './foo/bar/invalid.json'
-  var testStorage = storage('foo', 'bar')
+  var userDir = path.join(__dirname, '..', '.tmp')
+  var route = [userDir, 'foo', 'bar']
+  var storageDir = path.join.apply(path, route)
+  var fileNameA = path.join(storageDir, 'a.json')
+  var fileNameB = path.join(storageDir, 'b.json')
 
-  /**
-   * Initialize storage and save a file.
-   */
-  beforeEach(function () {
-    testStorage.reset()
-    testStorage.save('index', { foo: 'bar' })
-    fs.writeFileSync(invalidJson, '{')
+  function assertStorageNonExistence () {
+    try {
+      if (fs.accessSync) {
+        fs.accessSync(storageDir)
+      } else {
+        fs.statSync(storageDir)
+      }
+    } catch (e) {
+      expect(e.code).to.be.equals('ENOENT')
+      return
+    }
+    throw new Error('storage folder seems to exist?!')
+  }
+
+  function create () {
+    return storage.apply(null, route)
+  }
+
+  before(function () {
+    // For the test to run in an restricted environment that
+    // also doesn't interfere we run in a clear, separate user
+    // directory
+    rimraf.sync(userDir)
+    storage.userDir = userDir
   })
 
-  it('should have a data directory', function () {
-    expect(testStorage).to.have.property('dir')
-    expect(testStorage.dir).to.equal('foo/bar')
+  describe('in clean state', function () {
+    beforeEach(function () {
+      rimraf.sync(userDir)
+    })
+
+    it('should not automatically create a folder', function () {
+      // If the storage creates a folder automatically we might
+      // theoretically end up with a lot of empty foldes, it
+      // is better that the storages are only created once they
+      // exist
+      create()
+      assertStorageNonExistence()
+    })
+
+    it('should not create a folder when getting data', function () {
+      // It might be tempting to make a system to create a file
+      // if a property is missing. This would be unwanted behavior
+      // (for the same reason as one above)
+      create().get('a')
+      assertStorageNonExistence()
+    })
+
+    it('should allow to get data', function () {
+      // By allowing this we can simply assume & test for null
+      expect(create().get('a')).to.be.equals(null)
+    })
+
+    it('should store files at a predicatable locations', function () {
+      // In order to be update compatible the path should be
+      // in this and future versions of the storage api
+      var storage = create()
+      storage.save('a', 1)
+      storage.save('b', 2)
+      expect(JSON.parse(fs.readFileSync(fileNameA))).to.be.equals(1)
+      expect(JSON.parse(fs.readFileSync(fileNameB))).to.be.equals(2)
+    })
+
+    it('should store readable files', function () {
+      // For debugging we want the files to be stored
+      // in a fashion that is readable to humans
+      create().save('a', {x: 1})
+      expect(fs.readFileSync(fileNameA, 'utf8')).to.be.equals('{\n  "x": 1\n}')
+    })
+
+    it('should allow to store data', function () {
+      var storage = create()
+      storage.save('a', 1)
+      expect(storage.get('a')).to.be.equals(1)
+    })
+
+    it('should allow to reset the store', function () {
+      // Reset as operation could happen at any given
+      // time, this can break if reset assumes that
+      // the folder exists
+      var storage = create()
+      storage.reset()
+      expect(storage.get('a')).to.be.equals(null)
+    })
   })
 
-  describe('#save', function () {
-    it('should save serialized storage to the data directory', function (done) {
-      fs.readFile(fileName, function (err, data) {
-        if (err) return done(err)
-        try {
-          JSON.parse(data)
-        } catch (e) {
-          done(e)
-        }
-        done()
-      })
+  describe('with stored data', function () {
+    var storage = create()
+    beforeEach(function () {
+      rimraf.sync(userDir)
+      storage.save('a', 1)
     })
+
+    it('should return the stored data', function () {
+      expect(storage.get('a')).to.be.equals(1)
+    })
+
+    it('should allow to overwrite the data', function () {
+      storage.save('a', 2)
+      expect(storage.get('a')).to.be.equals(2)
+    })
+
+    it('should allow to reset the data', function () {
+      storage.reset()
+      expect(storage.get('a')).to.be.equals(null)
+    })
   })
 
-  describe('#get', function () {
-    it('should retrive a stored file', function (done) {
-      var file = testStorage.get('index')
-      expect(file.foo).to.equal('bar')
-      done()
+  describe('with corrupt storage', function () {
+    var storage = create()
+    beforeEach(function () {
+      rimraf.sync(userDir)
+      mkdirp.sync(storageDir)
+      fs.writeFileSync(fileNameA, '{')
     })
 
-    it('handle invalid JSON', function (done) {
-      fs.readFile(invalidJson, function (err, data) {
-        if (err) return done(err)
-        try {
-          JSON.parse(data)
-        } catch (e) {
-          return done(err)
-        }
-        done()
-      })
+    it('should still not break get', function () {
+      // By breaking the API with a broken file we gain no
+      // value for the user of workshoppers. A broken file
+      // should not inflict pain on the user of
+      // workshopper-adventure
+      //
+      // TODO: Debugging broken files could be improved by storing the broken
+      //       file in a backup location
+      expect(storage.get('a')).to.be.equal(null)
     })
+
+    it('should allow to overwrite the broken data', function () {
+      storage.save('a', 1)
+      expect(storage.get('a')).to.be.equal(1)
+    })
+
+    it('should allow to reset the state', function () {
+      storage.reset()
+      expect(storage.get('a')).to.be.equal(null)
+    })
   })
 
-  describe('#reset', function () {
-    it('should clean the storage directory', function (done) {
-      testStorage.reset()
-      fs.stat('./foo/bar', function (err, stats) {
-        if (err && err.code === 'ENOENT') return done()
-        done('file still exists!')
-      })
+  describe('inaccessible storage', function () {
+    var storage = create()
+    before(function () {
+      rimraf.sync(userDir)
+      mkdirp.sync(storageDir)
+      fs.writeFileSync(fileNameA, '{"x": 1}')
+      fs.chmodSync(fileNameA, 0)
+      fs.chmodSync(storageDir, 0)
     })
+
+    it('should still not break get', function () {
+      expect(storage.get('a')).to.be.equal(null)
+    })
+
+    it('should just not store if a file isn\'t writable', function () {
+      storage.save('a', 1)
+      expect(storage.get('a')).to.be.equal(null)
+    })
+
+    after(function () {
+      fs.chmodSync(storageDir, 448)
+      fs.chmodSync(fileNameA, 448)
+      rimraf.sync(userDir)
+    })
   })
 })

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Major refactor

Supply chain risk

Package has recently undergone a major refactor. It may be unstable or indicate significant internal changes. Use caution when updating to versions that include significant changes.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

12301

increased by51.21%

Number of package files

9

increased by28.57%

Lines of code

220

increased by122.22%

Number of lines in readme file

80

increased by1.27%

Worsened metrics

Number of medium supply chain risk alerts

1

increased byInfinity%

No dependency changes