From a44b9322f0a720edf32f461f546d08831c8cad37 Mon Sep 17 00:00:00 2001 From: Ori Livneh Date: Mon, 30 Nov 2015 12:41:43 -0800 Subject: [PATCH] mediawiki.storage: Provide a wrapper for sessionStorage too T119146 provides a use-case for using sessionStorage. So far mw.storage was localStorage-specific. With a small modification, we can allow the Storage object to passed to the constructor, which allows us to create a wrapper around sessionStorage (mw.storage.session) with minimal code duplication. Bug: T121646 Change-Id: I73bc82d9fa2359148fe1e50b6535bfa0dbe8bd3e --- maintenance/jsduck/categories.json | 1 + resources/src/mediawiki/mediawiki.storage.js | 128 +++++++++++------- .../mediawiki/mediawiki.storage.test.js | 46 +++++-- 3 files changed, 111 insertions(+), 64 deletions(-) diff --git a/maintenance/jsduck/categories.json b/maintenance/jsduck/categories.json index aad85da4a4..9fe5009acd 100644 --- a/maintenance/jsduck/categories.json +++ b/maintenance/jsduck/categories.json @@ -27,6 +27,7 @@ "mw.notification", "mw.Notification_", "mw.storage", + "mw.storage.session", "mw.user", "mw.util", "mw.plugin.*", diff --git a/resources/src/mediawiki/mediawiki.storage.js b/resources/src/mediawiki/mediawiki.storage.js index a9d17ff70d..20f8efb659 100644 --- a/resources/src/mediawiki/mediawiki.storage.js +++ b/resources/src/mediawiki/mediawiki.storage.js @@ -1,65 +1,91 @@ ( function ( mw ) { 'use strict'; - /** - * Library for storing device specific information. It should be used for storing simple - * strings and is not suitable for storing large chunks of data. - * - * @class mw.storage - * @singleton - */ - mw.storage = { - - localStorage: ( function () { - // Catch exceptions to avoid fatal in Chrome's "Block data storage" mode - // which throws when accessing the localStorage property itself, as opposed - // to the standard behaviour of throwing on getItem/setItem. (T148998) + // Catch exceptions to avoid fatal in Chrome's "Block data storage" mode + // which throws when accessing the localStorage property itself, as opposed + // to the standard behaviour of throwing on getItem/setItem. (T148998) + var + localStorage = ( function () { try { return window.localStorage; } catch ( e ) {} }() ), - - /** - * Retrieve value from device storage. - * - * @param {string} key Key of item to retrieve - * @return {string|boolean} False when localStorage not available, otherwise string - */ - get: function ( key ) { + sessionStorage = ( function () { try { - return mw.storage.localStorage.getItem( key ); + return window.sessionStorage; } catch ( e ) {} - return false; - }, + }() ); - /** - * Set a value in device storage. - * - * @param {string} key Key name to store under - * @param {string} value Value to be stored - * @return {boolean} Whether the save succeeded or not - */ - set: function ( key, value ) { - try { - mw.storage.localStorage.setItem( key, value ); - return true; - } catch ( e ) {} - return false; - }, + /** + * A wrapper for an HTML5 Storage interface (`localStorage` or `sessionStorage`) + * that is safe to call on all browsers. + * + * @class mw.SafeStorage + * @private + */ - /** - * Remove a value from device storage. - * - * @param {string} key Key of item to remove - * @return {boolean} Whether the save succeeded or not - */ - remove: function ( key ) { - try { - mw.storage.localStorage.removeItem( key ); - return true; - } catch ( e ) {} - return false; - } + /** + * @ignore + * @param {Object|undefined} store The Storage instance to wrap around + */ + function SafeStorage( store ) { + this.store = store; + } + + /** + * Retrieve value from device storage. + * + * @param {string} key Key of item to retrieve + * @return {string|boolean} False when localStorage not available, otherwise string + */ + SafeStorage.prototype.get = function ( key ) { + try { + return this.store.getItem( key ); + } catch ( e ) {} + return false; + }; + + /** + * Set a value in device storage. + * + * @param {string} key Key name to store under + * @param {string} value Value to be stored + * @return {boolean} Whether the save succeeded or not + */ + SafeStorage.prototype.set = function ( key, value ) { + try { + this.store.setItem( key, value ); + return true; + } catch ( e ) {} + return false; + }; + + /** + * Remove a value from device storage. + * + * @param {string} key Key of item to remove + * @return {boolean} Whether the save succeeded or not + */ + SafeStorage.prototype.remove = function ( key ) { + try { + this.store.removeItem( key ); + return true; + } catch ( e ) {} + return false; }; + /** + * @class + * @singleton + * @extends mw.SafeStorage + */ + mw.storage = new SafeStorage( localStorage ); + + /** + * @class + * @singleton + * @extends mw.SafeStorage + */ + mw.storage.session = new SafeStorage( sessionStorage ); + }( mediaWiki ) ); diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.storage.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.storage.test.js index 6cef4a7c81..436cb2ed75 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.storage.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.storage.test.js @@ -1,36 +1,56 @@ ( function ( mw ) { QUnit.module( 'mediawiki.storage' ); - QUnit.test( 'set/get with localStorage', 3, function ( assert ) { - this.sandbox.stub( mw.storage, 'localStorage', { + QUnit.test( 'set/get with storage support', function ( assert ) { + var stub = { setItem: this.sandbox.spy(), getItem: this.sandbox.stub() - } ); + }; + stub.getItem.withArgs( 'foo' ).returns( 'test' ); + stub.getItem.returns( null ); + this.sandbox.stub( mw.storage, 'store', stub ); mw.storage.set( 'foo', 'test' ); - assert.ok( mw.storage.localStorage.setItem.calledOnce ); + assert.ok( stub.setItem.calledOnce ); - mw.storage.localStorage.getItem.withArgs( 'foo' ).returns( 'test' ); - mw.storage.localStorage.getItem.returns( null ); assert.strictEqual( mw.storage.get( 'foo' ), 'test', 'Check value gets stored.' ); assert.strictEqual( mw.storage.get( 'bar' ), null, 'Unset values are null.' ); } ); - QUnit.test( 'set/get without localStorage', 3, function ( assert ) { - this.sandbox.stub( mw.storage, 'localStorage', { + QUnit.test( 'set/get with storage methods disabled', function ( assert ) { + // This covers browsers where storage is disabled + // (quota full, or security/privacy settings). + // On most browsers, these interface will be accessible with + // their methods throwing. + var stub = { getItem: this.sandbox.stub(), removeItem: this.sandbox.stub(), setItem: this.sandbox.stub() - } ); + }; + stub.getItem.throws(); + stub.setItem.throws(); + stub.removeItem.throws(); + this.sandbox.stub( mw.storage, 'store', stub ); - mw.storage.localStorage.getItem.throws(); assert.strictEqual( mw.storage.get( 'foo' ), false ); - - mw.storage.localStorage.setItem.throws(); assert.strictEqual( mw.storage.set( 'foo', 'test' ), false ); + assert.strictEqual( mw.storage.remove( 'foo', 'test' ), false ); + } ); + + QUnit.test( 'set/get with storage object disabled', function ( assert ) { + // On other browsers, these entire object is disabled. + // `'localStorage' in window` would be true (and pass feature test) + // but trying to read the object as window.localStorage would throw + // an exception. Such case would instantiate SafeStorage with + // undefined after the internal try/catch. + var old = mw.storage.store; + mw.storage.store = undefined; - mw.storage.localStorage.removeItem.throws(); + assert.strictEqual( mw.storage.get( 'foo' ), false ); + assert.strictEqual( mw.storage.set( 'foo', 'test' ), false ); assert.strictEqual( mw.storage.remove( 'foo', 'test' ), false ); + + mw.storage.store = old; } ); }( mediaWiki ) ); -- 2.20.1