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

diff --git a/maintenance/jsduck/categories.json b/maintenance/jsduck/categories.json
index aad85da..9fe5009 100644 (file)
--- 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 a9d17ff..20f8efb 100644 (file)
--- 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 6cef4a7..436cb2e 100644 (file)
--- 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 ) );