Skip to content

Fix getItem/setItem/clear edge cases with falsy and empty-string values #30

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mc21fly wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix getItem/setItem/clear edge cases with falsy and empty-string values #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 74 additions & 50 deletions src/memorystorage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,66 +17,90 @@
* @param id Optional string argument used to isolate this memory storage object from others.
*/
function MemoryStorage(id) {
// make sure id is assigned
id = id || 'global'
// try to get existing store
var result = storage[id]
// return it if found
if (result) {return result}
// make sure id is assigned
id = id || "global";
// try to get existing store
var result = storage[id];
// return it if found
if (result) {
return result;
}

// make sure there is no harm in leaving out new in invocations to MemoryStorage
if (! (this instanceof MemoryStorage)) {return new MemoryStorage(id);}
// make sure there is no harm in leaving out new in invocations to MemoryStorage
if (!(this instanceof MemoryStorage)) {
return new MemoryStorage(id);
}

// create a new store and save a ref to it so we can get it back later
result = storage[id] = this;
// create a space to store 'cloaked' key/values: items that have a key
// that collides with Web Storage API method names.
var cloak = {};
// create a new store and save a ref to it so we can get it back later
result = storage[id] = this;
// create a space to store 'cloaked' key/values: items that have a key
// that collides with Web Storage API method names.
var cloak = {};

// Allow client code to read the id
Object.defineProperty(result, 'id', {enumerable:true, configurable:true, value:id})
// Allow client code to read the id
Object.defineProperty(result, "id", { enumerable: true, configurable: true, value: id });

// Create the length property
Object.defineProperty(result, 'length', {enumerable:true, configurable:true, get:function(){
return enumerableKeys().length
}})
// Create the length property
Object.defineProperty(result, "length", {
enumerable: true,
configurable: true,
get: function () {
return enumerableKeys().length;
},
});

// Create API methods
result.getItem = function MemoryStorage_getItem(key) {
return (key in API ? cloak[key] : this[key]) || null
}
result.setItem = function MemoryStorage_setItem(key, val) {
key in API ? cloak[key] = val : this[key] = val
}
result.removeItem = function MemoryStorage_removeItem(key) {
key in API ? delete cloak[key] : delete this[key]
}
result.key = function MemoryStorage_key(idx) {
var keys = enumerableKeys()
return idx >= 0 && idx < keys.length ? keys[idx] : null
}
result.clear = function MemoryStorage_clear() {
var keys = uncloakedKeys()
for (var i=0,key; key=keys[i]; i++) {
delete this[key]
}
keys = cloakedKeys()
for (var i=0,key; key=keys[i]; i++) {
delete cloak[key]
}
}
// Create API methods
result.getItem = function MemoryStorage_getItem(key) {
var value = key in API ? cloak[key] : this[key];
return value === undefined ? null : value;
};
result.setItem = function MemoryStorage_setItem(key, val) {
val = String(val);
key in API ? (cloak[key] = val) : (this[key] = val);
};
result.removeItem = function MemoryStorage_removeItem(key) {
key in API ? delete cloak[key] : delete this[key];
};
result.key = function MemoryStorage_key(idx) {
var keys = enumerableKeys();
return idx >= 0 && idx < keys.length ? keys[idx] : null;
};
result.clear = function MemoryStorage_clear() {
var keys = uncloakedKeys();
for (var i = 0; i < keys.length; i++) {
delete this[keys[i]];
}
keys = cloakedKeys();
for (var i = 0; i < keys.length; i++) {
delete cloak[keys[i]];
}
};

// Wrap in ES6 Proxy if available to support Object.keys() on a MemoryStorage object
return typeof Proxy === 'undefined' ? result : new Proxy(result, {ownKeys: function() {return enumerableKeys()}})
// Wrap in ES6 Proxy if available to support Object.keys() on a MemoryStorage object
return typeof Proxy === "undefined"
? result
: new Proxy(result, {
ownKeys: function () {
return enumerableKeys();
},
});

// helper functions
function uncloakedKeys() {return Object.keys(result).filter(function(x){return !(x in API)})}
function cloakedKeys() {return Object.keys(cloak)}
function enumerableKeys(){return uncloakedKeys().concat(cloakedKeys())}
// helper functions
function uncloakedKeys() {
return Object.keys(result).filter(function (x) {
return !(x in API);
});
}
function cloakedKeys() {
return Object.keys(cloak);
}
function enumerableKeys() {
return uncloakedKeys().concat(cloakedKeys());
}
}

// API methods and properties will be cloaked
var API = {clear:1, getItem:1, id:1, key:1, length:1, removeItem:1, setItem:1}
var API = { clear: 1, getItem: 1, id: 1, key: 1, length: 1, removeItem: 1, setItem: 1 };

// Used to store all memorystorage objects
var storage = {};
240 changes: 140 additions & 100 deletions tests/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,112 +1,152 @@
QUnit.test("W3C Web Storage API Compliance Test", function( assert ) {
var store = new MemoryStorage('local');

store.clear();
assert.ok(store.length===0, "store cleared");
store.setItem('test0', 'data0');
assert.ok(store.length===1, "first item added to store");
assert.ok(store.key(0)==='test0', "key registered");
assert.ok(store.key(99)===null, "key() should return null when index out of bounds")
assert.ok(store.getItem('test0')==='data0', "value retrieved with getItem matches stored value");
assert.ok(store['test0']==='data0', "value retrieved with index operators matches stored value");
store['test0'] = 'changed';
assert.ok(store['test0']==='changed', "value updated correctly with index operators.");
store['test1'] = 'data1';
assert.ok(store.length===2, 'value added correctly with index operators');
store.setItem('test2', 'data2');
assert.ok(store.length===3, 'three items added to store');
if (typeof Proxy === "undefined") {
assert.ok(Object.keys(store).length == (7+3), "store has 10 enumerable properties (id, 6 api methods + 3 stored items)");
}
else {
assert.ok(Object.keys(store).length === 3, "store has 3 enumerable properties (no api methods + 3 stored items)");
assert.ok(Object.keys(store).sort().join(',') === "test0,test1,test2",
"keys are enumerable with Object.keys()");
}
assert.ok(store.getItem('test1')==='data1' && store.getItem('test2')==='data2', "retrieved values matches stored values");
var keyOrderBefore = '';
for (var i=0; i<store.length; i++) {
keyOrderBefore += store.key(i);
}
store.setItem('test2', 'data2.2');
var keyOrderAfter = '';
for (var i=0; i<store.length; i++) {
keyOrderAfter += store.key(i);
}
assert.ok(keyOrderBefore === keyOrderAfter, 'Key order not affected by mutation');
QUnit.test("W3C Web Storage API Compliance Test", function (assert) {
var store = new MemoryStorage("local");

store.removeItem('test1');
assert.ok(store.length===2, "item removed correctly with removeItem");
store.removeItem('test1');
assert.ok(store.length===2, "double removal has no effect");
assert.ok(store.getItem('test1')===null, "get removed item returns null");
store.clear();
assert.ok(store.length === 0, "store cleared");
store.setItem("test0", "data0");
assert.ok(store.length === 1, "first item added to store");
assert.ok(store.key(0) === "test0", "key registered");
assert.ok(store.key(99) === null, "key() should return null when index out of bounds");
assert.ok(store.getItem("test0") === "data0", "value retrieved with getItem matches stored value");
assert.ok(store["test0"] === "data0", "value retrieved with index operators matches stored value");
store["test0"] = "changed";
assert.ok(store["test0"] === "changed", "value updated correctly with index operators.");
store["test1"] = "data1";
assert.ok(store.length === 2, "value added correctly with index operators");
store.setItem("test2", "data2");
assert.ok(store.length === 3, "three items added to store");
if (typeof Proxy === "undefined") {
assert.ok(
Object.keys(store).length == 7 + 3,
"store has 10 enumerable properties (id, 6 api methods + 3 stored items)",
);
} else {
assert.ok(
Object.keys(store).length === 3,
"store has 3 enumerable properties (no api methods + 3 stored items)",
);
assert.ok(
Object.keys(store).sort().join(",") === "test0,test1,test2",
"keys are enumerable with Object.keys()",
);
}
assert.ok(
store.getItem("test1") === "data1" && store.getItem("test2") === "data2",
"retrieved values matches stored values",
);
var keyOrderBefore = "";
for (var i = 0; i < store.length; i++) {
keyOrderBefore += store.key(i);
}
store.setItem("test2", "data2.2");
var keyOrderAfter = "";
for (var i = 0; i < store.length; i++) {
keyOrderAfter += store.key(i);
}
assert.ok(keyOrderBefore === keyOrderAfter, "Key order not affected by mutation");

store.clear();
store.setItem('getItem', 'test');
assert.ok(typeof store.getItem === 'function', "store API methods cannot be overwritten.");
assert.ok(store.getItem('getItem') === 'test', "getItem successfully retrieves item with API name.");
assert.ok(store.length===1, 'items with store API key names should be counted in the length property');
assert.ok(store.key(0)==='getItem', 'items with store API key names should be enumerable with key()');
if (typeof Proxy !== "undefined") {
assert.ok(Object.keys(store).length === 1, "store API key names are counted in the key enumeration");
assert.ok(Object.keys(store).join('') === "getItem",
"store API key names are included in the key enumeration");
}
store.removeItem('getItem');
assert.ok(store.getItem('getItem') === null, "After removal of item with API name, getItem returns null.");

var glob = new MemoryStorage();
assert.ok(glob.length===0, "local store items are not visible globally");
glob.setItem('glob0', 'data0');
assert.ok(glob.length===1 && glob.getItem('glob0')==='data0', "globally stored items are retrieved ok");
assert.ok(store.getItem('glob0')===null, "global items are not visible in the local store");
glob.removeItem('glob0');
assert.ok(glob.length===0, "global length is updated correctly");
assert.ok(glob.key(0)===null, "global keys are removed correctly");
assert.ok(glob.getItem('glob0')===null, "global values are removed correctly");
store.removeItem("test1");
assert.ok(store.length === 2, "item removed correctly with removeItem");
store.removeItem("test1");
assert.ok(store.length === 2, "double removal has no effect");
assert.ok(store.getItem("test1") === null, "get removed item returns null");

assert.ok(store.length===0, "store is cleared");
assert.ok(store.key(0)===null, "no keys in cleared store");
assert.ok(store.getItem('test0')===null, "no values in cleared store");
store.clear();
store.setItem("getItem", "test");
assert.ok(typeof store.getItem === "function", "store API methods cannot be overwritten.");
assert.ok(store.getItem("getItem") === "test", "getItem successfully retrieves item with API name.");
assert.ok(store.length === 1, "items with store API key names should be counted in the length property");
assert.ok(store.key(0) === "getItem", "items with store API key names should be enumerable with key()");
if (typeof Proxy !== "undefined") {
assert.ok(Object.keys(store).length === 1, "store API key names are counted in the key enumeration");
assert.ok(Object.keys(store).join("") === "getItem", "store API key names are included in the key enumeration");
}
store.removeItem("getItem");
assert.ok(store.getItem("getItem") === null, "After removal of item with API name, getItem returns null.");

var glob = new MemoryStorage();
assert.ok(glob.length === 0, "local store items are not visible globally");
glob.setItem("glob0", "data0");
assert.ok(glob.length === 1 && glob.getItem("glob0") === "data0", "globally stored items are retrieved ok");
assert.ok(store.getItem("glob0") === null, "global items are not visible in the local store");
glob.removeItem("glob0");
assert.ok(glob.length === 0, "global length is updated correctly");
assert.ok(glob.key(0) === null, "global keys are removed correctly");
assert.ok(glob.getItem("glob0") === null, "global values are removed correctly");

assert.ok(store.length === 0, "store is cleared");
assert.ok(store.key(0) === null, "no keys in cleared store");
assert.ok(store.getItem("test0") === null, "no values in cleared store");
});

QUnit.test("Multiple Instances Test", function( assert ) {
var store1 = new MemoryStorage('local');
var store2 = new MemoryStorage('local');
store1.clear();
store1.setItem('test0', 'data0');
assert.ok(store2.getItem('test0') === store1.getItem('test0'), "Item added to store1 is also visible in store2");
store1['test0'] = 'changed';
assert.ok((store2.getItem('test0') === store1.getItem('test0')) && store1.getItem('test0') === 'changed', "Item changed in store1 is also changed in store2");
store1['test1'] = 'data1';
assert.ok(store2.length === store1.length, "Store lengths remain consistent");
for (var i=0; i<store1.length; i++) {
assert.ok(store1.key(i) === store2.key(i), 'Order of keys is consistent across stores');
assert.ok(store1[store1.key(i)] === store2.getItem(store2.key(i)), 'Order and contents of values are consistent across stores');
}
store1.clear();
assert.ok(store2.length===0, "Clearing store1 also clears store2");
QUnit.test("Multiple Instances Test", function (assert) {
var store1 = new MemoryStorage("local");
var store2 = new MemoryStorage("local");
store1.clear();
store1.setItem("test0", "data0");
assert.ok(store2.getItem("test0") === store1.getItem("test0"), "Item added to store1 is also visible in store2");
store1["test0"] = "changed";
assert.ok(
store2.getItem("test0") === store1.getItem("test0") && store1.getItem("test0") === "changed",
"Item changed in store1 is also changed in store2",
);
store1["test1"] = "data1";
assert.ok(store2.length === store1.length, "Store lengths remain consistent");
for (var i = 0; i < store1.length; i++) {
assert.ok(store1.key(i) === store2.key(i), "Order of keys is consistent across stores");
assert.ok(
store1[store1.key(i)] === store2.getItem(store2.key(i)),
"Order and contents of values are consistent across stores",
);
}
store1.clear();
assert.ok(store2.length === 0, "Clearing store1 also clears store2");
});

QUnit.test("Beyond W3C API Test", function (assert) {
var store = new MemoryStorage("local");
store.clear();
store.my = { object: "Yes!" };
assert.ok(typeof store.getItem("my") === "object", "Object returned with getItem when object was stored");
assert.ok(store.getItem("my").object === "Yes!", "Contents of objects survive storing/retrieving");
store.tree = { nested: { objects: { works: "Sure!" } } };
assert.ok(store.tree.nested.objects.works === "Sure!", "Deep nested trees stored correctly");
store.clear();
});

QUnit.test("Beyond W3C API Test", function( assert ) {
var store = new MemoryStorage('local');
store.clear();
store.my = {object: 'Yes!'};
assert.ok(typeof store.getItem('my') === 'object', 'Object returned with getItem when object was stored');
assert.ok(store.getItem('my').object === 'Yes!', 'Contents of objects survive storing/retrieving');
store.tree = {nested: {objects: {works: 'Sure!'}}};
assert.ok(store.tree.nested.objects.works === 'Sure!', 'Deep nested trees stored correctly');
store.clear();
QUnit.test("New-less Construction Test", function (assert) {
var store1 = MemoryStorage("local");
assert.ok(store1, "Existing store is defined and not null");
assert.ok(store1 instanceof MemoryStorage, "Existing store is instanceof MemoryStorage");

var store2 = MemoryStorage("new-store");
assert.ok(store2, "New store is defined and not null");
assert.ok(store2 instanceof MemoryStorage, "New store is instanceof MemoryStorage");
});

QUnit.test("New-less Construction Test", function( assert ) {
var store1 = MemoryStorage('local');
assert.ok(store1, 'Existing store is defined and not null');
assert.ok(store1 instanceof MemoryStorage, 'Existing store is instanceof MemoryStorage');

var store2 = MemoryStorage('new-store');
assert.ok(store2, 'New store is defined and not null');
assert.ok(store2 instanceof MemoryStorage, 'New store is instanceof MemoryStorage');
QUnit.test("getItem returns empty string, not null, for a stored empty value", function (assert) {
var s = new MemoryStorage("test-empty-string");
s.setItem("key", "");
assert.strictEqual(s.getItem("key"), "", "empty string should be returned as-is");
});

QUnit.test("getItem still returns null for a genuinely missing key", function (assert) {
var s = new MemoryStorage("test-missing-key");
assert.strictEqual(s.getItem("doesNotExist"), null, "missing key should return null");
});

QUnit.test("setItem coerces non-string values to strings", function (assert) {
var s = new MemoryStorage("test-coercion");
s.setItem("key", 123);
assert.strictEqual(typeof s.getItem("key"), "string", "stored value should be a string");
assert.strictEqual(s.getItem("key"), "123", "numeric value should be coerced to '123'");
});

QUnit.test("clear removes all keys even if one key is an empty string", function (assert) {
var s = new MemoryStorage("test-clear-empty-key");
s.setItem("a", "1");
s.setItem("", "empty key");
s.setItem("b", "2");
s.clear();
assert.strictEqual(Object.keys(s).length, 0, "all keys should be removed, including empty-string key");
});