Bug 976503 - Properly handle incoming messages with a 'si-id' field r…

…=julienw

apps/wappush/js/messagedb.js

@@ -1,6 +1,8 @@
 /* -*- Mode: js; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- /
 /* vim: set shiftwidth=2 tabstop=2 autoindent cindent expandtab: */
 
+/* global IDBKeyRange */
+
 /* exported MessageDB */
 
 'use strict';
@@ -73,6 +75,33 @@ var MessageDB = (function() {
     };
   }
 
+  /**
+   * Process a message executing the associated action. This deals with
+   * messages with a 'delete' action by deleting all coresponding message and
+   * with new messages by storing them. Once processing is done the new status
+   * is returned.
+   *
+   * @param {Object} store The messages' object store.
+   * @param {Object} message The message to be processed.
+   * @param {String} status The status of this message
+   * @param {Function} error A callback invoked if an operation fails.
+   *
+   * @return {String} The status of the message after processing it.
+   */
+  function mdb_processMessage(store, message, status, error) {
+    /* If the message has a 'delete' action delete all messages with the
+     * corresponding 'si-id' field and drop the current message. All other
+     * messages should be stored. */
+    if (message.action === 'delete') {
+      mdb_deleteById(store, message.id, error);
+      status = 'discarded';
+    } else {
+      store.put(message);
+    }
+
+    return status;
+  }
+
   /**
    * Adds a messages to the message database according to the out-of-order
    * delivery rules specified in WAP-167 6.2. Once the transaction is completed
@@ -104,60 +133,64 @@ var MessageDB = (function() {
 
       var store = transaction.objectStore(MESSAGE_STORE_NAME);
 
-      if (message.id) {
-        /* Check if the message is new, updates an existing one or is just
-         * outdated and needs to be discarded. */
-        var index = store.index(ID_INDEX);
-        var req = index.get(message.id);
-
-        req.onsuccess = function mdb_gotExistingMessage(event) {
-          if (event.target.result) {
-            var storedMessage = event.target.result;
-
-            /* If this is a new version of an existing message, expire the
-             * previous message and flag this message as updated. Older versions
-             * are discarded right away with no further action required. */
-            if (storedMessage.created && message.created) {
-              if (storedMessage.created < message.created) {
-                status = 'updated';
-                store.delete(storedMessage.timestamp);
-                store.put(message);
-              } else {
-                status = 'discarded';
-              }
-            }
+      if (!message.id) {
+        // The message has no 'si-id' field, store it
+        store.put(message);
+        return;
+      }
 
-            /* After the normal message replacement if this message has a
-             * delete action remove all messages with the same ID including
-             * the received message itself. The user will not be notified. */
-            if (message.action === 'delete') {
+      if (!message.created) {
+        /* The message has a 'si-id' field but no creation time, process it and
+         * store it if needed */
+        status = mdb_processMessage(store, message, status, error);
+        return;
+      }
+
+      /* The message has both a 'si-id' and a 'created' field, make it go
+       * through the out-of-order delivery logic. This will check if the
+       * message is new, updates an existing one or is just outdated and
+       * needs to be discarded. */
+      var index = store.index(ID_INDEX);
+      var req = index.openCursor(IDBKeyRange.only(message.id));
+
+      req.onsuccess = function mdb_cursorSuccess(event) {
+        var cursor = event.target.result;
+
+        if (cursor) {
+          var storedMessage = cursor.value;
+
+          /* If this is a new version of an existing message, expire the
+           * previous message and flag this message as updated. Older versions
+           * are discarded right away with no further action required. */
+          if (storedMessage.created) {
+            if (storedMessage.created < message.created) {
+              cursor.delete(storedMessage);
+              status = mdb_processMessage(store, message, 'updated', error);
+            } else {
               status = 'discarded';
-              mdb_deleteById(transaction, message.id, error);
             }
-          } else {
-            /* No existing message has a matching ID, notify the user */
-            store.put(message);
+
+            return;
           }
-        };
-      } else {
-        /* The message doesn't have an ID, unconditionally store it and notify
-         * the user. */
-        store.put(message);
-      }
+
+          cursor.continue();
+        } else {
+          /* We found no matching message with a 'created' property set,
+           * process the message and store it if needed */
+          status = mdb_processMessage(store, message, status, error);
+        }
+      };
     }, error);
   }
 
   /**
-   * Deletes all messages with the specified si-id. This method is private and
-   * should only be called from within an existing transaction.
+   * Deletes all messages with the specified si-id.
    *
-   * @param {Object} transaction A transaction within which this operation will
-   *        take place.
+   * @param {Object} store The messages' object store.
    * @param {String} id The si-id of the messages to be deleted.
    * @param {Function} error A callback invoked if an operation fails.
    */
-  function mdb_deleteById(transaction, id, error) {
-    var store = transaction.objectStore(MESSAGE_STORE_NAME);
+  function mdb_deleteById(store, id, error) {
     var index = store.index(ID_INDEX);
     var req = index.openCursor(id);
 

apps/wappush/js/wappush.js

@@ -181,13 +181,13 @@
             tag: message.timestamp
           };
 
-          var onClick = function wpm_notificationOnClick(timestamp) {
-            app.launch();
-            wpm_displayWapPushMessage(timestamp);
-          };
-
           var notification = new Notification(message.sender, options);
-          notification.addEventListener('click', onClick.bind(options.tag));
+          notification.addEventListener('click',
+            function wpm_onNotificationClick(event) {
+              app.launch();
+              wpm_displayWapPushMessage(event.target.tag);
+            }
+          );
 
           wpm_finish();
         };
@@ -256,7 +256,8 @@
               break;
           }
         } else {
-          SiSlScreenHelper.populateScreen(message);
+          // Notify the user that the message has expired
+          SiSlScreenHelper.populateScreen();
         }
       },
       function wpm_loadError(error) {

apps/wappush/test/unit/messagedb_test.js

@@ -45,9 +45,45 @@ suite('MessageDB', function() {
       timestamp: '3',
       href: 'http://www.mozilla.org',
       id: '[email protected]',
-      created: (new Date('2013-09-03T10:35:34Z')).getTime(),
       action: 'delete',
-      text: 'newer message'
+      text: ''
+    },
+    old_delete: {
+      type: 'text/vnd.wap.si',
+      sender: '+31641600986',
+      timestamp: '4',
+      href: 'http://www.mozilla.org',
+      id: '[email protected]',
+      created: (new Date('2013-09-03T10:35:32Z')).getTime(),
+      action: 'delete',
+      text: ''
+    },
+    low: {
+      type: 'text/vnd.wap.si',
+      sender: '+31641600986',
+      timestamp: '5',
+      href: 'http://www.mozilla.org',
+      id: 'http://www.mozilla.org',
+      action: 'signal-low',
+      text: 'check this out'
+    },
+    medium: {
+      type: 'text/vnd.wap.si',
+      sender: '+31641600986',
+      timestamp: '6',
+      href: 'http://www.mozilla.org',
+      id: 'http://www.mozilla.org',
+      action: 'signal-medium',
+      text: 'check this out'
+    },
+    high: {
+      type: 'text/vnd.wap.si',
+      sender: '+31641600986',
+      timestamp: '7',
+      href: 'http://www.mozilla.org',
+      id: 'http://www.mozilla.org',
+      action: 'signal-high',
+      text: 'check this out'
     }
   };
 
@@ -89,6 +125,37 @@ suite('MessageDB', function() {
     });
   });
 
+  suite('handling multiple messages', function() {
+    test('storing multiple messages with the same si-id', function(done) {
+      MessageDB.put(messages.low, function putSuccess(status_low) {
+        MessageDB.put(messages.medium, function putSuccess(status_medium) {
+          MessageDB.put(messages.high, function putSuccess(status_high) {
+            MessageDB.retrieve(messages.low.timestamp,
+              function retrieveSuccess(low) {
+                MessageDB.retrieve(messages.medium.timestamp,
+                  function retrieveSuccess(medium) {
+                    MessageDB.retrieve(messages.high.timestamp,
+                      function retrieveSuccess(high) {
+                        done(function checks() {
+                          assert.equal(status_low, 'new');
+                          assert.equal(low.id, messages.low.id);
+                          assert.equal(low.action, messages.low.action);
+                          assert.equal(status_medium, 'new');
+                          assert.equal(medium.id, messages.medium.id);
+                          assert.equal(medium.action, messages.medium.action);
+                          assert.equal(status_high, 'new');
+                          assert.equal(high.id, 'http://www.mozilla.org');
+                          assert.equal(high.action, 'signal-high');
+                        });
+                      });
+                  });
+              });
+          });
+        });
+      });
+    });
+  });
+
   suite('out-of-order message handling', function() {
     test('message is updated by newer message', function(done) {
       MessageDB.put(messages.current,
@@ -133,15 +200,43 @@ suite('MessageDB', function() {
   suite('message actions', function() {
     test('delete action removes all message with same id', function(done) {
       MessageDB.put(messages.current,
-        function putSuccess(status) {
+        function putSuccess() {
           MessageDB.put(messages.delete,
+            function putSuccess(status) {
+              MessageDB.retrieve(messages.current.timestamp,
+                function retrieveSuccess(message) {
+                  done(function checks() {
+                    assert.equal(status, 'discarded');
+                    assert.equal(message, null);
+                  });
+                });
+            });
+        });
+    });
+
+    test('delete action messages are never stored', function(done) {
+      MessageDB.put(messages.delete, function putSuccess(status) {
+        MessageDB.retrieve(messages.delete.timestamp,
+          function retrieveSuccess(message) {
+            done(function checks() {
+              assert.equal(status, 'discarded');
+              assert.equal(message, null);
+            });
+          });
+      });
+    });
+
+    test('old delete action is ignored', function(done) {
+      MessageDB.put(messages.current,
+        function putSuccess(status) {
+          MessageDB.put(messages.old_delete,
             function putSuccess(status) {
               assert.equal(status, 'discarded');
 
               MessageDB.retrieve(messages.current.timestamp,
                 function retrieveSuccess(message) {
                   done(function checks() {
-                    assert.equal(message, null);
+                    assert.ok(message);
                   });
                 });
             });