From 400b30b2293a8abe3807b8266127aafc73cdb680 Mon Sep 17 00:00:00 2001
From: Dan Mills <thunder@mozilla.com>
Date: Fri, 19 Dec 2008 17:00:12 -0800
Subject: [PATCH] fix typo in history sql queries, clarify valid sort order
 values for collections

---
 .../sync/modules/base_records/collection.js   |  5 +-
 services/sync/modules/engines/history.js      | 51 ++++++++++---------
 2 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/services/sync/modules/base_records/collection.js b/services/sync/modules/base_records/collection.js
index b6282580a37..9efab67b073 100644
--- a/services/sync/modules/base_records/collection.js
+++ b/services/sync/modules/base_records/collection.js
@@ -99,8 +99,9 @@ Collection.prototype = {
     this._rebuildURL();
   },
 
-  // get items sorted by some criteria
-  // date
+  // get items sorted by some criteria. valid values:
+  // oldest (oldest first)
+  // newest (newest first)
   // index
   // depthindex
   get sort() { return this._sort; },
diff --git a/services/sync/modules/engines/history.js b/services/sync/modules/engines/history.js
index 46eceac43ef..bcdf9f69070 100644
--- a/services/sync/modules/engines/history.js
+++ b/services/sync/modules/engines/history.js
@@ -141,15 +141,9 @@ HistoryStore.prototype = {
     }
   },
 
-  _defineStm: function HistStore__defineStm(prop, sql) {
-    let stm = this._db.createStatement(sql);
-    this.__defineGetter__(prop, function() stm);
-    return stm;
-  },
-
   get _visitStm() {
-    let stm = this._defineStm(
-      "_visitStm",
+    this._log.trace("Creating SQL statement: _visitStm");
+    let stm = this._db.createStatement(
       "SELECT * FROM ( " +
         "SELECT visit_type AS type, visit_date AS date " +
         "FROM moz_historyvisits_temp " +
@@ -167,12 +161,13 @@ HistoryStore.prototype = {
         "LIMIT 10 " +
       ") " +
       "ORDER BY 2 DESC LIMIT 10"); /* 2 is visit_date */
+    this.__defineGetter__("_visitStm", function() stm);
     return stm;
   },
 
   get _pidStm() {
-    let stm = this._defineStm(
-      "_pidStm",
+    this._log.trace("Creating SQL statement: _pidStm");
+    let stm = this._db.createStatement(
       "SELECT * FROM " + 
         "(SELECT id FROM moz_places_temp WHERE url = :url LIMIT 1) " +
       "UNION ALL " +
@@ -182,36 +177,42 @@ HistoryStore.prototype = {
         "LIMIT 1 " +
       ") " +
       "LIMIT 1");
+    this.__defineGetter__("_pidStm", function() stm);
     return stm;
   },
 
   get _urlStm() {
-    let stm = this._defineStm(
-      "_urlStm",
+    this._log.trace("Creating SQL statement: _urlStm");
+    let stm = this._db.createStatement(
       "SELECT * FROM " +
         "(SELECT url FROM moz_places_temp WHERE id = :id LIMIT 1) " +
       "UNION ALL " +
       "SELECT * FROM ( " +
-        "SELECT url FROM moz_places WHERE ud = :id " +
+        "SELECT url FROM moz_places WHERE id = :id " +
         "AND id NOT IN (SELECT id from moz_places_temp) " +
         "LIMIT 1 " +
       ") " +
       "LIMIT 1");
+    this._log.trace("_urlStm 1");
+    this.__defineGetter__("_urlStm", function() stm);
+    this._log.trace("_urlStm 2");
     return stm;
   },
 
   get _annoAttrIdStm() {
-    let stm = this._defineStm(
-      "_annoAttrIdStm",
+    this._log.trace("Creating SQL statement: _annoAttrIdStm");
+    let stm = this._db.createStatement(
       "SELECT id from moz_anno_attributes WHERE name = :name");
+    this.__defineGetter__("_annoAttrIdStm", function() stm);
     return stm;
   },
 
   get _findPidByAnnoStm() {
-    let stm = this._defineStm(
-      "_findPidByAnnoStm",
+    this._log.trace("Creating SQL statement: _findPidByAnnoStm");
+    let stm = this._db.createStatement(
       "SELECT place_id AS id FROM moz_annos " +
         "WHERE anno_attribute_id = :attr AND content = :content");
+    this.__defineGetter__("_findPidByAnnoStm", function() stm);
     return stm;
   },
 
@@ -227,12 +228,16 @@ HistoryStore.prototype = {
       return visits;
     }
 
-    this._visitStm.params.placeid = placeid;
-    while (this._visitStm.step()) {
-      visits.push({date: this._visitStm.row.date,
-                   type: this._visitStm.row.type});
+    try {
+      this._visitStm.params.placeid = placeid;
+      while (this._visitStm.step()) {
+        visits.push({date: this._visitStm.row.date,
+                     type: this._visitStm.row.type});
+      }
+    } finally {
+      this._visitStm.reset();
     }
-    this._visitStm.reset();
+
     return visits;
   },
 
@@ -252,7 +257,7 @@ HistoryStore.prototype = {
   },
 
   // See bug 468732 for why we use SQL here
-  _findURLByGUID: function HistStore__findByGUID(guid) {
+  _findURLByGUID: function HistStore__findURLByGUID(guid) {
     try {
       this._annoAttrIdStm.params.name = "weave/guid";
       if (!this._annoAttrIdStm.step())