Refactor server creation, updating and deletion via API

Fixes accidental duplicate servers

Author: Dahlgren

Diff for: lib/manager.js

@@ -14,13 +14,32 @@ var Manager = function (config, logs) {
 
 Manager.prototype = new events.EventEmitter()
 
-Manager.prototype.addServer = function (options) {
+Manager.prototype.addServer = function (options, cb) {
+  if (manager.getServer(Server.generateId(options.title))) {
+    return cb(new Error('Server already exists'))
+  }
+
   var server = this._addServer(options)
-  this.save()
-  return server
+  this.save(function (err) {
+    cb(err, server)
+  })
 }
 
-Manager.prototype.removeServer = function (id) {
+Manager.prototype.updateServer = function (id, options, cb) {
+  var server = manager.getServer(id)
+  var anotherServer = manager.getServer(Server.generateId(options.title))
+
+  if (anotherServer !== null && anotherServer !== server) {
+    return cb(new Error('Server already exists'))
+  }
+
+  server.update(req.body)
+  this.save(function (err) {
+    cb(err, server)
+  })
+}
+
+Manager.prototype.removeServer = function (id, cb) {
   var server = this.serversHash[id]
 
   if (!server) {
@@ -31,13 +50,14 @@ Manager.prototype.removeServer = function (id) {
   if (index > -1) {
     this.serversArr.splice(index, 1)
   }
-  this.save()
 
   if (server.pid) {
     server.stop()
   }
 
-  return server
+  this.save(function (err) {
+    cb(err, server)
+  })
 }
 
 Manager.prototype._addServer = function (data) {
@@ -90,7 +110,7 @@ Manager.prototype.load = function () {
   })
 }
 
-Manager.prototype.save = function () {
+Manager.prototype.save = function (cb) {
   var data = []
   var self = this
 
@@ -128,9 +148,12 @@ Manager.prototype.save = function () {
   fs.writeFile(filePath, JSON.stringify(data), function (err) {
     if (err) {
       console.error('Manager save error: ' + err)
-    } else {
-      self.emit('servers')
+      cb(err)
+      return
     }
+
+    self.emit('servers')
+    cb()
   })
 }
 

Diff for: lib/server.js

@@ -23,6 +23,12 @@ var Server = function (config, logs, options) {
   this.update(options)
 }
 
+Server.generateId = function (title) {
+  return slugify(title)
+    .replace(/[\(|\)]/g, '')
+    .replace(/\./g, '-')
+}
+
 Server.prototype = new events.EventEmitter()
 
 Server.prototype.createServerTitle = function (title) {
@@ -37,10 +43,6 @@ Server.prototype.createServerTitle = function (title) {
   return title
 }
 
-Server.prototype.generateId = function () {
-  return slugify(this.title).replace(/\./g, '-')
-}
-
 Server.prototype.update = function (options) {
   this.additionalConfigurationOptions = options.additionalConfigurationOptions
   this.admin_password = options.admin_password
@@ -62,7 +64,7 @@ Server.prototype.update = function (options) {
   this.von = options.von
   this.verify_signatures = options.verify_signatures
 
-  this.id = this.generateId()
+  this.id = Server.generateId(this.title)
   this.port = parseInt(this.port, 10) // If port is a string then gamedig fails
 }
 

Diff for: routes/servers.js

@@ -13,8 +13,13 @@ module.exports = function (manager, mods) {
       return
     }
 
-    var server = manager.addServer(req.body)
-    res.json(server)
+    manager.addServer(req.body, function (err, server) {
+      if (err) {
+        return res.status(500).send(err)
+      }
+      
+      res.status(201).send(server)
+    })
   })
 
   router.get('/:server', function (req, res) {
@@ -28,15 +33,23 @@ module.exports = function (manager, mods) {
       return
     }
 
-    var server = manager.getServer(req.params.server)
-    server.update(req.body)
-    manager.save()
-    res.json(server)
+    manager.update(req.params.server, req.body, function (err, server) {
+      if (err) {
+        return res.status(500).send(err)
+      }
+      
+      res.status(200).send(server)
+    })
   })
 
   router.delete('/:server', function (req, res) {
-    var server = manager.removeServer(req.params.server)
-    res.json(server)
+    manager.removeServer(req.params.server, function (err, server) {
+      if (err) {
+        return res.status(500).send(err)
+      }
+      
+      res.status(204).send()
+    })
   })
 
   router.post('/:server/start', function (req, res) {

Diff for: test/lib/server.js

@@ -4,9 +4,16 @@ var Server = require('../../lib/server.js')
 
 describe('Server', function () {
   describe('generateId()', function () {
-    it('should include title', function () {
-      var server = new Server(null, null, { title: 'title.with.lot.of.dots' })
-      server.generateId().should.eql('title-with-lot-of-dots')
+    it('should generate id for title with dots', function () {
+      Server.generateId('title.with.lot.of.dots').should.eql('title-with-lot-of-dots')
+    })
+
+    it('should generate id title with brackets', function () {
+      Server.generateId('title [with] [lots of] [brackets]').should.eql('title-with-lots-of-brackets')
+    })
+
+    it('should generate id title with parentheses', function () {
+      Server.generateId('title (with) (lots of) (parentheses)').should.eql('title-with-lots-of-parentheses')
     })
   })