From 68d3cc7295079509908c3ba937c89bb489da267e Mon Sep 17 00:00:00 2001
From: Alex Kocharin <alex@kocharin.ru>
Date: Mon, 13 Jan 2014 20:48:51 +0400
Subject: [PATCH] fs interface refactoring

---
 lib/config.js        |  24 ++++++-----
 lib/local-fs.js      | 100 +++++++++++--------------------------------
 lib/local-storage.js |  75 +++++++++++++++++++++-----------
 package.yaml         |   1 +
 4 files changed, 92 insertions(+), 108 deletions(-)

diff --git a/lib/config.js b/lib/config.js
index 128fb927f..57e543774 100644
--- a/lib/config.js
+++ b/lib/config.js
@@ -47,7 +47,7 @@ function Config(config) {
 	for (var i in this.users) {
 		assert(this.users[i].password, 'CONFIG: no password for user: ' + i)
 		assert(
-			typeof(this.users[i].password) === 'string' && 
+			typeof(this.users[i].password) === 'string' &&
 			this.users[i].password.match(/^[a-f0-9]{40}$/)
 		, 'CONFIG: wrong password format for user: ' + i + ', sha1 expected')
 	}
@@ -113,15 +113,10 @@ function Config(config) {
 }
 
 function allow_action(package, who, action) {
-	for (var i in this.packages) {
-		if (minimatch.makeRe(i).exec(package)) {
-			return this.packages[i][action].reduce(function(prev, curr) {
-				if (curr === String(who) || curr === 'all') return true
-				return prev
-			}, false)
-		}
-	}
-	return false
+	return (this.get_package_setting(package, action) || []).reduce(function(prev, curr) {
+		if (curr === String(who) || curr === 'all') return true
+		return prev
+	}, false)
 }
 
 Config.prototype.allow_access = function(package, user) {
@@ -140,6 +135,15 @@ Config.prototype.proxy_publish = function(package, uplink) {
 	return allow_action.call(this, package, uplink, 'proxy_publish')
 }
 
+Config.prototype.get_package_setting = function(package, setting) {
+	for (var i in this.packages) {
+		if (minimatch.makeRe(i).exec(package)) {
+			return this.packages[i][setting]
+		}
+	}
+	return undefined
+}
+
 Config.prototype.authenticate = function(user, password) {
 	if (this.users[user] == null) return false
 	return crypto.createHash('sha1').update(password).digest('hex') === this.users[user].password
diff --git a/lib/local-fs.js b/lib/local-fs.js
index 82d916842..dc17f6027 100644
--- a/lib/local-fs.js
+++ b/lib/local-fs.js
@@ -1,24 +1,10 @@
 var fs = require('fs')
   , fsExt = require('fs-ext')
   , Path = require('path')
+  , mkdirp = require('mkdirp')
   , mystreams = require('./streams')
-  , Logger = require('./logger')
   , FSError = require('./error').FSError
 
-function make_directories(dest, cb) {
-	var dir = Path.dirname(dest)
-	if (dir === '.' || dir === '..') return cb()
-	fs.mkdir(dir, function(err) {
-		if (err && err.code === 'ENOENT') {
-			make_directories(dir, function() {
-				fs.mkdir(dir, cb)
-			})
-		} else {
-			cb()
-		}
-	})
-}
-
 function write(dest, data, cb) {
 	var safe_write = function(cb) {
 		var tmpname = dest + '.tmp' + String(Math.random()).substr(2)
@@ -30,7 +16,8 @@ function write(dest, data, cb) {
 
 	safe_write(function(err) {
 		if (err && err.code === 'ENOENT') {
-			make_directories(dest, function() {
+			mkdirp(Path.dirname(dest), function(err) {
+				if (err) return cb(err)
 				safe_write(cb)
 			})
 		} else {
@@ -137,7 +124,7 @@ function open_flock(name, opmod, flmod, tries, backoff, cb) {
 			}
 		})
 	})
-} 
+}
 
 // this function neither unlocks file nor closes it
 // it'll have to be done manually later
@@ -158,23 +145,10 @@ function lock_and_read(name, callback) {
 	})
 }
 
-function Storage(path) {
-	this.path = path
-	this.logger = Logger.logger.child({sub: 'fs'})
-	try {
-		fs.mkdirSync(path)
-		this.logger.warn({path: path}, 'created new packages directory: @{path}')
-	} catch(err) {
-		if (err.code !== 'EEXIST') throw new Error(err)
-	}
-}
+module.exports.read = read
 
-Storage.prototype.read = function(name, cb) {
-	read(this.path + '/' + name, cb)
-}
-
-Storage.prototype.read_json = function(name, cb) {
-	read(this.path + '/' + name, function(err, res) {
+module.exports.read_json = function(name, cb) {
+	read(name, function(err, res) {
 		if (err) return cb(err)
 
 		var args = []
@@ -187,12 +161,10 @@ Storage.prototype.read_json = function(name, cb) {
 	})
 }
 
-Storage.prototype.lock_and_read = function(name, cb) {
-	lock_and_read(this.path + '/' + name, cb)
-}
+module.exports.lock_and_read = lock_and_read
 
-Storage.prototype.lock_and_read_json = function(name, cb) {
-	lock_and_read(this.path + '/' + name, function(err, fd, res) {
+module.exports.lock_and_read_json = function(name, cb) {
+	lock_and_read(name, function(err, fd, res) {
 		if (err) return cb(err, fd)
 
 		var args = []
@@ -205,49 +177,29 @@ Storage.prototype.lock_and_read_json = function(name, cb) {
 	})
 }
 
-Storage.prototype.path_to = function(file) {
-	return this.path + '/' + file
+module.exports.create = create
+
+module.exports.create_json = function(name, value, cb) {
+	create(name, JSON.stringify(value, null, '\t'), cb)
 }
 
-Storage.prototype.create = function(name, value, cb) {
-	create(this.path + '/' + name, value, cb)
+module.exports.update = update
+
+module.exports.update_json = function(name, value, cb) {
+	update(name, JSON.stringify(value, null, '\t'), cb)
 }
 
-Storage.prototype.create_json = function(name, value, cb) {
-	create(this.path + '/' + name, JSON.stringify(value, null, '\t'), cb)
+module.exports.write = write
+
+module.exports.write_json = function(name, value, cb) {
+	write(name, JSON.stringify(value, null, '\t'), cb)
 }
 
-Storage.prototype.update = function(name, value, cb) {
-	update(this.path + '/' + name, value, cb)
-}
+module.exports.write_stream = write_stream
 
-Storage.prototype.update_json = function(name, value, cb) {
-	update(this.path + '/' + name, JSON.stringify(value, null, '\t'), cb)
-}
+module.exports.read_stream = read_stream
 
-Storage.prototype.write = function(name, value, cb) {
-	write(this.path + '/' + name, value, cb)
-}
+module.exports.unlink = fs.unlink
 
-Storage.prototype.write_json = function(name, value, cb) {
-	write(this.path + '/' + name, JSON.stringify(value, null, '\t'), cb)
-}
-
-Storage.prototype.write_stream = function(name, value, cb) {
-	return write_stream(this.path + '/' + name, value, cb)
-}
-
-Storage.prototype.read_stream = function(name, cb) {
-	return read_stream(this.path + '/' + name, cb)
-}
-
-Storage.prototype.unlink = function(name, cb) {
-	fs.unlink(this.path + '/' + name, cb)
-}
-
-Storage.prototype.rmdir = function(name, cb) {
-	fs.rmdir(this.path + '/' + name, cb)
-}
-
-module.exports = Storage
+module.exports.rmdir = fs.rmdir
 
diff --git a/lib/local-storage.js b/lib/local-storage.js
index f8aa2a681..ac3f61de1 100644
--- a/lib/local-storage.js
+++ b/lib/local-storage.js
@@ -2,7 +2,7 @@ var fs = require('fs')
   , Path = require('path')
   , crypto = require('crypto')
   , assert = require('assert')
-  , FS_Storage = require('./local-fs')
+  , fs_storage = require('./local-fs')
   , UError = require('./error').UserError
   , utils = require('./utils')
   , mystreams = require('./streams')
@@ -16,8 +16,6 @@ var fs = require('fs')
 function Storage(config) {
 	if (!(this instanceof Storage)) return new Storage(config)
 	this.config = config
-	var path = Path.resolve(Path.dirname(this.config.self_path), this.config.storage)
-	this.storage = new FS_Storage(path)
 	this.logger = Logger.logger.child({sub: 'fs'})
 	return this
 }
@@ -38,7 +36,7 @@ function get_boilerplate(name) {
 }
 
 Storage.prototype._internal_error = function(err, file, msg) {
-	this.logger.error( {err: err, file: this.storage.path_to(file)}
+	this.logger.error( {err: err, file: file}
 	                 , msg + ' @{file}: @{!err.message}'
 	                 )
 	return new UError({
@@ -48,7 +46,7 @@ Storage.prototype._internal_error = function(err, file, msg) {
 }
 
 Storage.prototype.add_package = function(name, metadata, callback) {
-	this.storage.create_json(name + '/' + info_file, get_boilerplate(name), function(err) {
+	this.storage(name).create_json(info_file, get_boilerplate(name), function(err) {
 		if (err && err.code === 'EEXISTS') {
 			return callback(new UError({
 				status: 409,
@@ -62,7 +60,7 @@ Storage.prototype.add_package = function(name, metadata, callback) {
 Storage.prototype.remove_package = function(name, callback) {
 	var self = this
 	self.logger.info({name: name}, 'unpublishing @{name} (all)')
-	self.storage.read_json(name + '/' + info_file, function(err, data) {
+	self.storage(name).read_json(info_file, function(err, data) {
 		if (err) {
 			if (err.code === 'ENOENT') {
 				return callback(new UError({
@@ -75,7 +73,7 @@ Storage.prototype.remove_package = function(name, callback) {
 		}
 		self._normalize_package(data)
 
-		self.storage.unlink(name + '/' + info_file, function(err) {
+		self.storage(name).unlink(info_file, function(err) {
 			if (err) return callback(err)
 
 			var files = Object.keys(data._attachments)
@@ -84,14 +82,14 @@ Storage.prototype.remove_package = function(name, callback) {
 				if (files.length === 0) return cb()
 
 				var file = files.shift()
-				self.storage.unlink(name + '/' + file, function() {
+				self.storage(name).unlink(file, function() {
 					unlinkNext(cb)
 				})
 			}
 
 			unlinkNext(function() {
 				// try to unlink the directory, but ignore errors because it can fail
-				self.storage.rmdir(name, function(err) {
+				self.storage(name).rmdir('.', function(err) {
 					callback()
 				})
 			})
@@ -101,15 +99,14 @@ Storage.prototype.remove_package = function(name, callback) {
 
 Storage.prototype._read_create_package = function(name, callback) {
 	var self = this
-	  , file = name + '/' + info_file
-	self.storage.read_json(file, function(err, data) {
+	self.storage(name).read_json(info_file, function(err, data) {
 		// TODO: race condition
 		if (err) {
 			if (err.code === 'ENOENT') {
 				// if package doesn't exist, we create it here
 				data = get_boilerplate(name)
 			} else {
-				return callback(self._internal_error(err, file, 'error reading'))
+				return callback(self._internal_error(err, info_file, 'error reading'))
 			}
 		}
 		self._normalize_package(data)
@@ -294,7 +291,7 @@ Storage.prototype.remove_tarball = function(name, filename, revision, callback)
 		}
 	}, function(err) {
 		if (err) return callback(err)
-		self.storage.unlink(name + '/' + filename, callback)
+		self.storage(name).unlink(filename, callback)
 	})
 }
 
@@ -320,7 +317,7 @@ Storage.prototype.add_tarball = function(name, filename) {
 		}))
 	}
 
-	var wstream = this.storage.write_stream(name + '/' + filename)
+	var wstream = this.storage(name).write_stream(filename)
 
 	wstream.on('error', function(err) {
 		if (err.code === 'EEXISTS') {
@@ -387,7 +384,7 @@ Storage.prototype.get_tarball = function(name, filename, callback) {
 		rstream.close()
 	}
 
-	var rstream = this.storage.read_stream(name + '/' + filename)
+	var rstream = this.storage(name).read_stream(filename)
 	rstream.on('error', function(err) {
 		if (err && err.code === 'ENOENT') {
 			stream.emit('error', new UError({
@@ -410,9 +407,8 @@ Storage.prototype.get_package = function(name, options, callback) {
 	if (typeof(options) === 'function') callback = options, options = {}
 
 	var self = this
-	  , file = name + '/' + info_file
 
-	self.storage.read_json(file, function(err, result) {
+	self.storage(name).read_json(info_file, function(err, result) {
 		if (err) {
 			if (err.code === 'ENOENT') {
 				return callback(new UError({
@@ -420,7 +416,7 @@ Storage.prototype.get_package = function(name, options, callback) {
 					msg: 'no such package available'
 				}))
 			} else {
-				return callback(self._internal_error(err, file, 'error reading'))
+				return callback(self._internal_error(err, info_file, 'error reading'))
 			}
 		}
 		self._normalize_package(result)
@@ -446,12 +442,8 @@ Storage.prototype.get_package = function(name, options, callback) {
 //
 Storage.prototype.update_package = function(name, updateFn, _callback) {
 	var self = this
-	  , file = name + '/' + info_file
-	self.storage.lock_and_read_json(file, function(err, fd, json) {
-		self.logger.debug({file: file}, 'locking @{file}')
-
+	self.storage(name).lock_and_read_json(info_file, function(err, fd, json) {
 		function callback() {
-			self.logger.debug({file: file}, 'unlocking @{file}')
 			var _args = arguments
 			if (fd) {
 				fs.close(fd, function(err) {
@@ -502,8 +494,43 @@ Storage.prototype._write_package = function(name, json, callback) {
 	var rev = json._rev.split('-')
 	json._rev = ((+rev[0] || 0) + 1) + '-' + crypto.pseudoRandomBytes(8).toString('hex')
 
-	this.storage.write_json(name + '/' + info_file, json, callback)
+	this.storage(name).write_json(info_file, json, callback)
 }
 
+Storage.prototype.storage = function(package) {
+	return new Path_Wrapper(
+		Path.join(
+			Path.resolve(
+				Path.dirname(this.config.self_path),
+				this.config.storage
+			),
+			package
+		)
+	)
+}
+
+var Path_Wrapper = (function() {
+	// a wrapper adding paths to fs_storage methods
+	function Wrapper(path) {
+		this.path = path
+	}
+
+	for (var i in fs_storage) {
+		if (fs_storage.hasOwnProperty(i)) {
+			Wrapper.prototype[i] = wrapper(i)
+		}
+	}
+
+	function wrapper(method) {
+		return function(/*...*/) {
+			var args = Array.prototype.slice.apply(arguments)
+			args[0] = Path.join(this.path, args[0] || '')
+			return fs_storage[method].apply(null, args)
+		}
+	}
+
+	return Wrapper
+})()
+
 module.exports = Storage
 
diff --git a/package.yaml b/package.yaml
index 9a8c3b576..925da5605 100644
--- a/package.yaml
+++ b/package.yaml
@@ -28,6 +28,7 @@ dependencies:
   minimatch: '*'
   bunyan: '>= 0.16.4'
   fs-ext: '*'
+  mkdirp: '*'
 
 devDependencies:
   rimraf: '*'