jenkins-bot has submitted this change and it was merged.

Change subject: mediawiki.storage: Provide a wrapper for sessionStorage too
......................................................................


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
---
M maintenance/jsduck/categories.json
M resources/src/mediawiki/mediawiki.storage.js
M tests/qunit/suites/resources/mediawiki/mediawiki.storage.test.js
3 files changed, 112 insertions(+), 65 deletions(-)

Approvals:
  BryanDavis: Looks good to me, approved
  jenkins-bot: Verified



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

-- 
To view, visit https://gerrit.wikimedia.org/r/323344
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I73bc82d9fa2359148fe1e50b6535bfa0dbe8bd3e
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: BryanDavis <bda...@wikimedia.org>
Gerrit-Reviewer: Catrope <r...@wikimedia.org>
Gerrit-Reviewer: Edokter <er...@darcoury.nl>
Gerrit-Reviewer: Gilles <gdu...@wikimedia.org>
Gerrit-Reviewer: Helder.wiki <he7...@gmail.com>
Gerrit-Reviewer: Jack Phoenix <j...@countervandalism.net>
Gerrit-Reviewer: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: Ori.livneh <o...@wikimedia.org>
Gerrit-Reviewer: Parent5446 <tylerro...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to