factor out feed subscription and refreshment into a single code path that handles both; separate retrieval from storage in the identity module (not complete; we still have to handle the case where an identity/person already exists)

--HG--
extra : rebase_source : c2fde49d0d6bb0da213e5722543e6e6631e677a9

modules/feed.js

@@ -181,13 +181,29 @@ SnowlFeed.prototype = {
     throw Cr.NS_ERROR_NOT_IMPLEMENTED;
   },
 
+
+  //**************************************************************************//
+  // Refreshment
+
   _refreshTime: null,
+  _refreshCallback: null,
 
-  refresh: function(refreshTime) {
+  /**
-    // Cache the refresh time so we can use it as the received time when adding
+   * Refresh the feed, retrieving the latest information in it.
-    // messages to the datastore.
+   *
-    this._refreshTime = refreshTime;
+   * @param time      {Date}
+   *        The time the refresh was initiated; determines new messages'
+   *        received time.  We let the caller specify this so a caller
+   *        refreshing multiple feeds can give their messages the same
+   *        received time.
+   * @param callback  {Function}
+   */
+  refresh: function(time, callback) {
+    this._refreshTime = time;
+    this._refreshCallback = callback;
 
+    // FIXME: remove subscribe from this notification's name.
+    Observers.notify("snowl:subscribe:connect:start", this);
     this._log.info("refreshing " + this.machineURI.spec);
 
     new Request({
@@ -210,24 +226,29 @@ SnowlFeed.prototype = {
     // period of time.  We should instead keep trying when a source fails,
     // but with a progressively longer interval (up to the standard one).
     // FIXME: implement the approach described above.
-    this.lastRefreshed = refreshTime;
+    this.lastRefreshed = time;
   },
 
-  onRefreshLoad: function(aEvent) {
+  onRefreshLoad: function(event) {
-    let request = aEvent.target;
+    let request = event.target;
 
     // The load event can fire even with a non 2xx code, so handle as error
     if (request.status < 200 || request.status > 299) {
-      this.onRefreshError(aEvent);
+      this.onRefreshError(event);
       return;
     }
 
     // XXX What's the right way to handle this?
     if (request.responseText.length == 0) {
-      this.onRefreshError(aEvent);
+      this.onRefreshError(event);
       return;
     }
 
+    // XXX Perhaps we should set this._lastStatus = request.status so we don't
+    // need to pass it in this notification and it's available at any time.
+    // FIXME: remove subscribe from this notification's name.
+    Observers.notify("snowl:subscribe:connect:end", this, request.status);
+
     // _authInfo only gets set if we prompted the user to authenticate
     // and the user checked the "remember password" box.  Since we're here,
     // it means the request succeeded, so we save the login.
@@ -249,38 +270,77 @@ SnowlFeed.prototype = {
     this._resetRefresh();
   },
 
-  onRefreshError: function(aEvent) {
+  onRefreshError: function(event) {
-    let request = aEvent.target;
+    let request = event.target;
 
     // Sometimes an attempt to retrieve status text throws NS_ERROR_NOT_AVAILABLE.
     let statusText;
-    try {statusText = request.statusText;} catch(ex) {statusText = "[no status text]"}
+    try { statusText = request.statusText } catch(ex) { statusText = "[no status text]" }
-
     this._log.error("onRefreshError: " + request.status + " (" + statusText + ")");
+    // XXX Perhaps we should set this._lastStatus = request.status so we don't
+    // need to pass it in this notification and it's available at any time.
+    // FIXME: remove subscribe from this notification's name.
+    Observers.notify("snowl:subscribe:connect:end", this, request.status);
 
     this._resetRefresh();
+
+    if (this._subscribeCallback)
+      this._subscribeCallback();
   },
 
-  onRefreshResult: strand(function(aResult) {
+  onRefreshResult: strand(function(result) {
-    // FIXME: figure out why aResult.doc is sometimes null (its content isn't
+    // FIXME: figure out why result.doc is sometimes null (perhaps its content
-    // a valid feed?) and report a more descriptive error message.
+    // isn't a valid feed?) and report a more descriptive error message.
-    if (aResult.doc == null) {
+    if (result.doc == null) {
       this._log.error("onRefreshResult: result.doc is null");
+      // FIXME: factor this out with similar code in onSubscribeError and make
+      // the observers of snowl:subscribe:connect:end understand the status
+      // we return.
+      // FIXME: remove subscribe from this notification's name.
+      Observers.notify("snowl:subscribe:connect:end", this, "result.doc is null");
+      if (this._subscribeCallback)
+        this._subscribeCallback();
       return;
     }
 
-    let feed = aResult.doc.QueryInterface(Components.interfaces.nsIFeed);
+    try {
+      let feed = result.doc.QueryInterface(Ci.nsIFeed);
 
-    this.messages = this._processFeed(feed, this._refreshTime);
+      // Extract the name and human URI (if we don't already have them)
-    if (this.id)
+      // from the feed.
-      this.persistMessages();
+      // ??? Should we update these if they've changed?
-    Observers.notify("snowl:refresh:end", this);
+      if (!this.name)
+        this.name = feed.title.plainText();
+      if (!this.humanURI)
+        this.humanURI = feed.link;
+
+      // FIXME: remove subscribe from this notification's name.
+      Observers.notify("snowl:subscribe:get:start", this);
+      this.messages = this._processFeed(feed, this._refreshTime);
+      // FIXME: remove subscribe from this notification's name.
+      Observers.notify("snowl:subscribe:get:end", this);
+    }
+    catch(ex) {
+      this._log.error("error on subscribe result: " + ex);
+      // FIXME: remove subscribe from this notification's name.
+      // FIXME: make this something besides "connect:end" since we've already
+      // issued one of those notifications by now.
+      Observers.notify("snowl:subscribe:connect:end", this, "error: " + ex);
+    }
+    finally {
+      if (this._subscribeCallback)
+        this._subscribeCallback();
+    }
   }),
 
   _resetRefresh: function() {
     this._refreshTime = null;
   },
 
+
+  //**************************************************************************//
+  // Processing
+
   /**
    * Process a feed into an array of messages.
    *
@@ -298,7 +358,12 @@ SnowlFeed.prototype = {
       // one for it based on its content.
       try {
         let externalID = entry.id || this._generateID(entry);
+if (typeof externalID == "undefined")
+  dump("no external ID\n");
+else
+  dump("external ID: " + externalID + "\n");
         let message = this._processEntry(feed, entry, externalID, received);
+dump("processed entry into message\n");
         messages.push(message);
       }
       catch(ex) {
@@ -334,6 +399,7 @@ SnowlFeed.prototype = {
     let authors = (aEntry.authors.length > 0) ? aEntry.authors
                   : (aFeed.authors.length > 0) ? aFeed.authors
                   : null;
+    // FIXME: process all authors, not just the first one.
     if (authors && authors.length > 0) {
       let author = authors.queryElementAt(0, Ci.nsIFeedPerson);
       // The external ID for an author is her email address, if provided
@@ -342,14 +408,9 @@ SnowlFeed.prototype = {
       // email address if a name is not provided (which it probably was).
       let externalID = author.email || author.name;
       let name = author.name || author.email;
-
+      message.author = new SnowlIdentity(null, this, externalID, name);
-      // Get an existing identity or create a new one.  Creating an identity
+      //identity = SnowlIdentity.get(this.id, externalID) ||
-      // automatically creates a person record with the provided name.
+      //           SnowlIdentity.create(this.id, externalID, name);
-      identity = SnowlIdentity.get(this.id, externalID) ||
-                 SnowlIdentity.create(this.id, externalID, name);
-      message.authorID = identity.personID;
-      // message.authorName
-      // message.authorIcon
     }
 
     // Add parts
@@ -395,122 +456,6 @@ SnowlFeed.prototype = {
     return "urn:" + hasher.finish(true);
   },
 
-
-  //**************************************************************************//
-  // Subscription
-
-  _subscribeCallback: null,
-
-  subscribe: function(callback) {
-    Observers.notify("snowl:subscribe:connect:start", this);
-    this._subscribeCallback = callback;
-    this._log.info("subscribing to " + this.machineURI.spec);
-
-    let request = new Request({
-      loadCallback:           new Callback(this.onSubscribeLoad, this),
-      errorCallback:          new Callback(this.onSubscribeError, this),
-      // The feed processor is going to parse the XML, so override the MIME type
-      // in order to turn off parsing by XMLHttpRequest itself.
-      overrideMimeType:       "text/plain",
-      url:                    this.machineURI,
-      // Register a listener for notification callbacks so we handle
-      // authentication.
-      notificationCallbacks:  this
-    });
-  },
-
-  onSubscribeLoad: function(aEvent) {
-    let request = aEvent.target;
-
-    // The load event can fire even with a non 2xx code, so handle as error
-    if (request.status < 200 || request.status > 299) {
-      this.onSubscribeError(aEvent);
-      return;
-    }
-
-    // XXX What's the right way to handle this?
-    if (request.responseText.length == 0) {
-      this.onSubscribeError(aEvent);
-      return;
-    }
-
-    Observers.notify("snowl:subscribe:connect:end", this, request.status);
-
-    // _authInfo only gets set if we prompted the user to authenticate
-    // and the user checked the "remember password" box.  Since we're here,
-    // it means the request succeeded, so we save the login.
-    if (this._authInfo)
-      this._saveLogin();
-
-    let parser = Cc["@mozilla.org/feed-processor;1"].
-                 createInstance(Ci.nsIFeedProcessor);
-    parser.listener = {
-      self: this,
-      handleResult: function(result) {
-        this.self.onSubscribeResult(result);
-      }
-    };
-    parser.parseFromString(request.responseText, request.channel.URI);
-  },
-
-  onSubscribeError: function(aEvent) {
-    let request = aEvent.target;
-
-    // Sometimes an attempt to retrieve status text throws NS_ERROR_NOT_AVAILABLE.
-    let statusText;
-    try { statusText = request.statusText } catch(ex) { statusText = "[no status text]" }
-
-    this._log.error("onSubscribeError: " + request.status + " (" + statusText + ")");
-    Observers.notify("snowl:subscribe:connect:end", this, request.status);
-
-    if (this._subscribeCallback)
-      this._subscribeCallback();
-  },
-
-  onSubscribeResult: strand(function(aResult) {
-    // FIXME: figure out why aResult.doc is sometimes null (its content isn't
-    // a valid feed?) and report a more descriptive error message.
-    if (aResult.doc == null) {
-      this._log.error("result.doc is null");
-      // FIXME: factor this out with similar code in onSubscribeError and make
-      // the observers of snowl:subscribe:connect:end understand the status
-      // we return.
-      Observers.notify("snowl:subscribe:connect:end", this, "result.doc is null");
-      if (this._subscribeCallback)
-        this._subscribeCallback();
-      return;
-    }
-
-    try {
-      let feed = aResult.doc.QueryInterface(Ci.nsIFeed);
-
-      // Extract the name (if we don't already have one) and human URI from the feed.
-      if (!this.name)
-        this.name = feed.title.plainText();
-      this.humanURI = feed.link;
-
-      this.persist();
-
-      //Observers.notify("snowl:sources:changed");
-
-      // FIXME: use a date provided by the subscriber so refresh times are the same
-      // for all accounts subscribed at the same time (f.e. in an OPML import).
-      Observers.notify("snowl:subscribe:get:start", this);
-      this.messages = this._processFeed(feed, new Date());
-      this.persistMessages();
-      Observers.notify("snowl:subscribe:get:end", this);
-    }
-    catch(ex) {
-      this._log.error("error on subscribe result: " + feed.toSource());
-      this._log.error("error on subscribe result: " + ex);
-      Observers.notify("snowl:subscribe:connect:end", this, "error:" + ex);
-    }
-    finally {
-      if (this._subscribeCallback)
-        this._subscribeCallback();
-    }
-  }),
-
   _saveLogin: function() {
     let lm = Cc["@mozilla.org/login-manager;1"].getService(Ci.nsILoginManager);
 

modules/identity.js

@@ -46,39 +46,129 @@ Cu.import("resource://snowl/modules/datastore.js");
 Cu.import("resource://snowl/modules/source.js");
 Cu.import("resource://snowl/modules/URI.js");
 
-function SnowlIdentity(id, sourceID, externalID, personID) {
+
+function SnowlIdentity(id, source, externalID, name) {
   this.id = id;
-  this.sourceID = sourceID;
+  this.source = source;
   this.externalID = externalID;
-  this.personID = personID;
+  this.person = new SnowlPerson(null, name);
 }
 
-SnowlIdentity.get = function(sourceID, externalID) {
+SnowlIdentity.prototype = {
-  let identity;
+  id:         null,
+  source:     null,
+  externalID: null,
+  person:     null,
 
-  let statement = SnowlDatastore.createStatement(
+  persist: function() {
-    "SELECT id, personID " +
+    this.person.persist(this.source.id);
-    "FROM identities " +
-    "WHERE externalID = :externalID AND sourceID = :sourceID"
-  );
 
-  try {
+    let statement = SnowlDatastore.createStatement(
-    statement.params.sourceID = sourceID;
+      "INSERT INTO identities ( sourceID,  externalID,  personID) " +
-    statement.params.externalID = externalID;
+      "VALUES                 (:sourceID, :externalID, :personID)"
-    if (statement.step()) {
+    );
-      identity = new SnowlIdentity(statement.row.id,
+
-                                   sourceID,
+    try {
-                                   externalID,
+      statement.params.sourceID   = this.source.id;
-                                   statement.row.personID);
+      statement.params.externalID = this.externalID;
+      statement.params.personID   = this.person.id;
+      statement.step();
+      this.id = SnowlDatastore.dbConnection.lastInsertRowID;
+    }
+    finally {
+      statement.reset();
     }
   }
-  finally {
+
-    statement.reset();
+};
+
+
+function SnowlPerson(id, name, placeID, homeURL, iconURL) {
+  this.id      = id;
+  this.name    = name;
+  this.placeID = placeID;
+}
+
+SnowlPerson.prototype = {
+  name:    null,
+  placeID: null,
+  homeURL: null,
+  iconURL: null,
+
+  persist: function(sourceID) {
+    let statement = SnowlDatastore.createStatement(
+      "INSERT INTO people ( name,  homeURL,  iconURL) " +
+      "VALUES             (:name, :homeURL, :iconURL)"
+    );
+
+    try {
+      statement.params.name    = this.name;
+      statement.params.homeURL = this.homeURL;
+      statement.params.iconURL = this.iconURL;
+      statement.step();
+      this.id = SnowlDatastore.dbConnection.lastInsertRowID;
+
+      // XXX lookup favicon in collections table rather than hardcoding
+      let iconURI =
+        this.iconURL ? URI.get(this.iconURL) :
+        this.homeURL ? SnowlSource.faviconSvc.getFaviconForPage(this.homeURL) :
+        URI.get("chrome://snowl/skin/person-16.png");
+  
+      // Create places record, placeID stored into people table record.
+      //SnowlPlaces._log.info("Author name:iconURI.spec - " + name + " : " + iconURI.spec);
+      // FIXME: break the dependency on sourceID, since people should only be
+      // connected to sources through identities.
+      this.placeID = SnowlPlaces.persistPlace("people",
+                                              this.id,
+                                              name,
+                                              null,       // homeURL,
+                                              externalID, // externalID,
+                                              iconURI,
+                                              sourceID);
+      // Store placeID back into messages for DB integrity.
+      SnowlDatastore.dbConnection.executeSimpleSQL(
+        "UPDATE people " +
+        "SET    placeID = " + this.placeID +
+        " WHERE      id = " + this.id);
+    }
+    finally {
+      statement.reset();
+    }
   }
 
-  return identity;
 };
 
+
+
+
+
+
+//SnowlIdentity.get = function(sourceID, externalID) {
+//  let identity;
+//
+//  let statement = SnowlDatastore.createStatement(
+//    "SELECT id, personID " +
+//    "FROM identities " +
+//    "WHERE externalID = :externalID AND sourceID = :sourceID"
+//  );
+//
+//  try {
+//    statement.params.sourceID = sourceID;
+//    statement.params.externalID = externalID;
+//    if (statement.step()) {
+//      identity = new SnowlIdentity(statement.row.id,
+//                                   sourceID,
+//                                   externalID,
+//                                   statement.row.personID);
+//    }
+//  }
+//  finally {
+//    statement.reset();
+//  }
+//
+//  return identity;
+//};
+
 SnowlIdentity.create = function(sourceID, externalID, name, homeURL, iconURL) {
   let identity;
 
@@ -136,14 +226,6 @@ SnowlIdentity.create = function(sourceID, externalID, name, homeURL, iconURL) {
   return identity;
 };
 
-SnowlIdentity.prototype = {};
-
-function SnowlPerson(id, name, placeID) {
-  this.id = id;
-  this.name = name;
-  this.placeID = placeID;
-}
-
 SnowlPerson.__defineGetter__("_getAllStatement",
   function() {
     let statement = SnowlDatastore.createStatement(

modules/message.js

@@ -219,10 +219,12 @@ SnowlMessage.prototype = {
     SnowlDatastore.dbConnection.beginTransaction();
 
     try {
+      this.author.persist();
+
       this._stmtInsertMessage.params.sourceID   = this.sourceID;
       this._stmtInsertMessage.params.externalID = this.externalID;
       this._stmtInsertMessage.params.subject    = this.subject;
-      this._stmtInsertMessage.params.authorID   = this.authorID;
+      this._stmtInsertMessage.params.authorID   = this.author.id;
       this._stmtInsertMessage.params.timestamp  = SnowlDateUtils.jsToJulianDate(this.timestamp);
       this._stmtInsertMessage.params.received   = SnowlDateUtils.jsToJulianDate(this.received);
       this._stmtInsertMessage.params.link       = this.link ? this.link.spec : null;

test/unit/test_feed.js

@@ -22,17 +22,15 @@ function run_test() {
 
   do_test_pending();
 
-  Observers.add("snowl:subscribe:get:end", continue_test);
+  Observers.add("snowl:subscribe:get:end", finish_test);
   feed = new SnowlFeed(null, null, new URI("http://localhost:8080/feed.xml"), undefined, null);
-  feed.subscribe();
-}
-
-function continue_test() {
-  Observers.add("snowl:refresh:end", finish_test);
   feed.refresh(refreshTime);
 }
 
 function finish_test() {
+  feed.persist();
+  feed.persistMessages();
+
   try {
     do_check_eq(SnowlService.accounts.length, 1);
     let account = SnowlService.accounts[0];
@@ -55,6 +53,9 @@ function finish_test() {
     do_check_eq(message.subject, "Atom-Powered Robots Run Amok");
     do_check_eq(message.authorName, "John Doe");
     // TODO: do_check_eq(message.authorID, authorID);
+    // TODO: test that the message's author is a real identity record
+    // with a real person record behind it and the values of those records
+    // are all correct.
     do_check_eq(message.link, "http://example.org/2003/12/13/atom03");
     do_check_eq(message.timestamp.getTime(), 1071340202000);
     do_check_eq(message._read, false);