From 732218cc1d53e6535d6a5625c1822da850d8a83a Mon Sep 17 00:00:00 2001 From: Barry Tulchinsky Date: Tue, 24 Dec 2013 15:10:17 -0500 Subject: [PATCH] fixed unnecessary null checking and replaced it with hasOwnproperty method, also added removexattr and fremovexattr and tests for it --- src/fs.js | 155 ++++++++++++++++++++++++++++++++---- tests/spec/fs.xattr.spec.js | 71 ++++++++++++++++- 2 files changed, 208 insertions(+), 18 deletions(-) diff --git a/src/fs.js b/src/fs.js index 95afaa5..ba6e357 100644 --- a/src/fs.js +++ b/src/fs.js @@ -1123,7 +1123,6 @@ define(function(require) { function setxattr_file (context, path, name, value, flag, callback) { path = normalize(path); - var undefined; function set_xattr (error, node) { var xattr = (node ? node.xattrs[name] : null); @@ -1131,10 +1130,10 @@ define(function(require) { if (error) { callback (error); } - else if (flag === XATTR_CREATE && (xattr != null || xattr != undefined)) { + else if (flag == XATTR_CREATE && node.xattrs.hasOwnProperty(name)) { callback(new EExists('attribute already exists')); } - else if (flag === XATTR_REPLACE && (xattr == null || xattr == undefined)) { + else if (flag === XATTR_REPLACE && !node.xattrs.hasOwnProperty(name)) { callback(new ENoAttr('attribute does not exist')); } else { @@ -1149,10 +1148,10 @@ define(function(require) { else if (!name) { callback(new EInvalid('attribute name cannot be an empty string')); } - else if (value == null || value == undefined) { - callback(new EInvalid('value cannot be null or undefined')); + else if (value == null) { + callback(new EInvalid('value must be defined')); } - else if ((flag != null || flag != undefined) && + else if (flag != null && flag !== XATTR_CREATE && flag !== XATTR_REPLACE) { callback(new EInvalid('invalid flag, must be null, XATTR_CREATE or XATTR_REPLACE')); } @@ -1162,7 +1161,6 @@ define(function(require) { } function fsetxattr_file (context, ofd, name, value, flag, callback) { - var undefined; function set_xattr (error, node) { var xattr = (node ? node.xattrs[name] : null); @@ -1170,10 +1168,10 @@ define(function(require) { if (error) { callback(error); } - else if (flag === XATTR_CREATE && (xattr != null || xattr != undefined)) { + else if (flag === XATTR_CREATE && node.xattrs.hasOwnProperty(name)) { callback(new EExists('attribute already exists')); } - else if (flag === XATTR_REPLACE && (xattr == null || xattr == undefined)) { + else if (flag === XATTR_REPLACE && !node.xattrs.hasOwnProperty(name)) { callback(new ENoAttr('attribute does not exist')); } else { @@ -1188,10 +1186,10 @@ define(function(require) { else if (!name) { callback(new EInvalid('attribute name cannot be an empty string')); } - else if (value == null || value == undefined) { - callback(new EInvalid('value cannot be empty')); + else if (value == null) { + callback(new EInvalid('value must be defined')); } - else if ((flag != null || flag != undefined) && + else if (flag != null && flag !== XATTR_CREATE && flag !== XATTR_REPLACE) { callback(new EInvalid('invalid flag, must be null, XATTR_CREATE or XATTR_REPLACE')); } @@ -1202,7 +1200,6 @@ define(function(require) { function getxattr_file (context, path, name, callback) { path = normalize(path); - var undefined; function get_xattr(error, node) { var xattr = (node ? node.xattrs[name] : null); @@ -1210,7 +1207,7 @@ define(function(require) { if (error) { callback (error); } - else if (xattr == null || xattr == undefined) { + else if (!node.xattrs.hasOwnProperty(name)) { callback(new ENoAttr('attribute does not exist')); } else { @@ -1230,7 +1227,6 @@ define(function(require) { } function fgetxattr_file (context, ofd, name, callback) { - var undefined; function get_xattr (error, node) { var xattr = (node ? node.xattrs[name] : null); @@ -1238,7 +1234,7 @@ define(function(require) { if (error) { callback(error); } - else if (xattr == null || xattr == undefined) { + else if (!node.xattrs.hasOwnProperty(name)) { callback(new ENoAttr('attribute does not exist')); } else { @@ -1257,6 +1253,61 @@ define(function(require) { } } + function removexattr_file (context, path, name, callback) { + path = normalize(path); + + function remove_xattr (error, node) { + var xattr = (node ? node.xattrs : null); + + if (error) { + callback(error); + } + else if (!xattr.hasOwnProperty(name)) { + callback(new ENoAttr('attribute does not exist')); + } + else { + delete node.xattrs[name]; + context.put(node.id, node, callback); + } + } + + if (typeof name != 'string') { + callback(new EInvalid('attribute name must be a string')); + } + else if (!name) { + callback(new EInvalid('attribute name cannot be an empty string')); + } + else { + find_node(context, path, remove_xattr); + } + } + + function fremovexattr_file (context, ofd, name, callback) { + + function remove_xattr (error, node) { + if (error) { + callback(error); + } + else if (!node.xattrs.hasOwnProperty(name)) { + callback(new ENoAttr('attribute does not exist')); + } + else { + delete node.xattrs[name]; + context.put(node.id, node, callback); + } + } + + if (typeof name != 'string') { + callback(new EInvalid('attribute name must be a string')); + } + else if (!name) { + callback(new EInvalid('attribute name cannot be an empty string')); + } + else { + context.get(ofd.id, remove_xattr); + } + } + function validate_flags(flags) { if(!_(O_FLAGS).has(flags)) { return null; @@ -1744,6 +1795,45 @@ define(function(require) { } } + function _removexattr (context, path, name, callback) { + if (!nullCheck(path, callback)) return; + + function remove_xattr (error) { + if (error) { + callback(error); + } + else { + callback(null); + } + } + + removexattr_file (context, path, name, remove_xattr); + } + + function _fremovexattr (fs, context, fd, name, callback) { + + function remove_xattr (error) { + if (error) { + callback(error); + } + else { + callback(null); + } + } + + var ofd = fs.openFiles[fd]; + + if (!ofd) { + callback(new EBadFileDescriptor('invalid file descriptor')); + } + else if (!_(ofd.flags).contains(O_WRITE)) { + callback(new EBadFileDescriptor('descriptor does not permit writing')); + } + else { + fremovexattr_file(context, ofd, name, remove_xattr); + } + } + function _lseek(fs, context, fd, offset, whence, callback) { function check_result(error, offset) { if(error) { @@ -2258,8 +2348,39 @@ define(function(require) { _fgetxattr(fs, context, fd, name, callback); } ); - }; + if (error) { + callback(error); + } + }; + FileSystem.prototype.removexattr = function (path, name, callback) { + callback = maybeCallback(callback); + var fs = this; + var error = fs.queueOrRun( + function () { + var context = fs.provider.getReadWriteContext(); + _removexattr(context, path, name, callback); + } + ); + + if (error) { + callback(error); + } + }; + FileSystem.prototype.fremovexattr = function (fd, name, callback) { + callback = maybeCallback(callback); + var fs = this; + var error = fs.queueOrRun( + function () { + var context = fs.provider.getReadWriteContext(); + _fremovexattr(fs, context, fd, name, callback); + } + ); + + if (error) { + callback(error); + } + }; return FileSystem; }); diff --git a/tests/spec/fs.xattr.spec.js b/tests/spec/fs.xattr.spec.js index a45802e..737e8d3 100644 --- a/tests/spec/fs.xattr.spec.js +++ b/tests/spec/fs.xattr.spec.js @@ -303,6 +303,34 @@ define(["IDBFS"], function(IDBFS) { }); }); + it('should error when attempting to remove a non-existing attribute', function (error) { + var complete = false; + var _error; + var that = this; + + that.fs.writeFile('/testfile', '', function (error) { + if (error) throw error; + + that.fs.setxattr('/testfile', 'test', '', function (error) { + if (error) throw error; + + that.fs.removexattr('/testfile', 'testenoattr', function (error) { + _error = error; + complete = true; + }); + }); + }); + + waitsFor(function () { + return complete; + }, 'test to complete', DEFAULT_TIMEOUT); + + runs(function () { + expect(_error).toBeDefined(); + expect(_error.name).toEqual('ENoAttr'); + }); + }); + it('should set and get an empty string as a value', function (error) { var complete = false; var _error; @@ -369,7 +397,7 @@ define(["IDBFS"], function(IDBFS) { }); }); - it('should set/get an object to an extended attribute', function (error) { + it('should set and get an object to an extended attribute', function (error) { var complete = false; var _error; var that = this; @@ -488,5 +516,46 @@ define(["IDBFS"], function(IDBFS) { expect(_value2).toEqual('attribute'); }); }); + + it('should remove an extended attribute from a path', function (error) { + var complete = false; + var _error, _value; + var that = this; + + that.fs.writeFile('/testfile', '', function (error) { + if (error) throw error; + + that.fs.setxattr('/testfile', 'test', 'somevalue', function (error) { + if (error) throw error; + + that.fs.getxattr('/testfile', 'test', function (error, value) { + if (error) throw error; + + _value = value; + + that.fs.removexattr('/testfile', 'test', function (error) { + if (error) throw error; + + that.fs.getxattr('/testfile', 'test', function (error) { + _error = error; + complete = true; + }); + }); + }); + }); + }); + + waitsFor(function () { + return complete; + }, 'test to complete', DEFAULT_TIMEOUT); + + runs(function () { + expect(_error).toBeDefined(); + expect(_value).toBeDefined(); + expect(_value).toEqual('somevalue'); + expect(_error.name).toEqual('ENoAttr'); + }); + + }); }); }); \ No newline at end of file