Bug 607115 - use a much smaller guid format than we currently use for bookmarks
authorShawn Wilsher <me@shawnwilsher.com>
Tue, 23 Nov 2010 16:04:28 -0800
changeset 59352 952cedf2302e4422a5ff1796ec35faef48bb4505
parent 59351 510ee4557bb934de8587396b137d0604a02f508b
child 59353 0e86c9d9a9946c6a3bdf80c043320550d6a20bae
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
bugs607115
milestone2.0b8pre
Bug 607115 - use a much smaller guid format than we currently use for bookmarks Part 3 - Create a function we can use with SQL queries to generate a guid. r=mak
toolkit/components/places/src/SQLFunctions.cpp
toolkit/components/places/src/SQLFunctions.h
toolkit/components/places/src/nsNavHistory.cpp
toolkit/components/places/tests/unit/test_sql_guid_functions.js
--- a/toolkit/components/places/src/SQLFunctions.cpp
+++ b/toolkit/components/places/src/SQLFunctions.cpp
@@ -210,18 +210,18 @@ namespace places {
 
   //////////////////////////////////////////////////////////////////////////////
   //// MatchAutoCompleteFunction
 
   /* static */
   nsresult
   MatchAutoCompleteFunction::create(mozIStorageConnection *aDBConn)
   {
-    nsRefPtr<MatchAutoCompleteFunction> function(new MatchAutoCompleteFunction);
-    NS_ENSURE_TRUE(function, NS_ERROR_OUT_OF_MEMORY);
+    nsRefPtr<MatchAutoCompleteFunction> function =
+      new MatchAutoCompleteFunction();
 
     nsresult rv = aDBConn->CreateFunction(
       NS_LITERAL_CSTRING("autocomplete_match"), kArgIndexLength, function
     );
     NS_ENSURE_SUCCESS(rv, rv);
 
     return NS_OK;
   }
@@ -348,18 +348,17 @@ namespace places {
     nsCString url;
     (void)aArguments->GetUTF8String(kArgIndexURL, url);
 
     // We only want to filter javascript: URLs if we are not supposed to search
     // for them, and the search does not start with "javascript:".
     if (!HAS_BEHAVIOR(JAVASCRIPT) &&
         !StringBeginsWith(searchString, NS_LITERAL_CSTRING("javascript:")) &&
         StringBeginsWith(url, NS_LITERAL_CSTRING("javascript:"))) {
-      NS_IF_ADDREF(*_result = new IntegerVariant(0));
-      NS_ENSURE_TRUE(*_result, NS_ERROR_OUT_OF_MEMORY);
+      NS_ADDREF(*_result = new IntegerVariant(0));
       return NS_OK;
     }
 
     PRInt32 visitCount = aArguments->AsInt32(kArgIndexVisitCount);
     bool typed = aArguments->AsInt32(kArgIndexTyped) ? true : false;
     bool bookmark = aArguments->AsInt32(kArgIndexBookmark) ? true : false;
     nsCAutoString tags;
     (void)aArguments->GetUTF8String(kArgIndexTags, tags);
@@ -370,18 +369,17 @@ namespace places {
     bool matches = !(
       (HAS_BEHAVIOR(HISTORY) && visitCount == 0) ||
       (HAS_BEHAVIOR(TYPED) && !typed) ||
       (HAS_BEHAVIOR(BOOKMARK) && !bookmark) ||
       (HAS_BEHAVIOR(TAG) && tags.IsVoid()) ||
       (HAS_BEHAVIOR(OPENPAGE) && openPageCount == 0)
     );
     if (!matches) {
-      NS_IF_ADDREF(*_result = new IntegerVariant(0));
-      NS_ENSURE_TRUE(*_result, NS_ERROR_OUT_OF_MEMORY);
+      NS_ADDREF(*_result = new IntegerVariant(0));
       return NS_OK;
     }
 
     // Clean up our URI spec and prepare it for searching.
     nsCString fixedURI;
     fixupURISpec(url, fixedURI);
 
     // Obtain our search function.
@@ -409,18 +407,17 @@ namespace places {
       // If we do not match the URL when we have to, reset matches to false.
       // Otherwise, keep track that we did match the current search.
       if (HAS_BEHAVIOR(URL) && !matchURL)
         matches = false;
       else
         matches = matches || matchURL;
     }
 
-    NS_IF_ADDREF(*_result = new IntegerVariant(matches ? 1 : 0));
-    NS_ENSURE_TRUE(*_result, NS_ERROR_OUT_OF_MEMORY);
+    NS_ADDREF(*_result = new IntegerVariant(matches ? 1 : 0));
     return NS_OK;
     #undef HAS_BEHAVIOR
   }
 
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Frecency Calculation Function
 
@@ -428,17 +425,16 @@ namespace places {
   //// CalculateFrecencyFunction
 
   /* static */
   nsresult
   CalculateFrecencyFunction::create(mozIStorageConnection *aDBConn)
   {
     nsCOMPtr<CalculateFrecencyFunction> function =
       new CalculateFrecencyFunction();
-    NS_ENSURE_TRUE(function, NS_ERROR_OUT_OF_MEMORY);
 
     nsresult rv = aDBConn->CreateFunction(
       NS_LITERAL_CSTRING("calculate_frecency"), 1, function
     );
     NS_ENSURE_SUCCESS(rv, rv);
 
     return NS_OK;
   }
@@ -539,25 +535,23 @@ namespace places {
 
       // If we found some visits for this page, use the calculated weight.
       if (numSampledVisits) {
         // fix for bug #412219
         if (!pointsForSampledVisits) {
           // For URIs with zero points in the sampled recent visits
           // but "browsing" type visits outside the sampling range, set
           // frecency to -visit_count, so they're still shown in autocomplete.
-          NS_IF_ADDREF(*_result = new IntegerVariant(-visitCount));
-          NS_ENSURE_TRUE(*_result, NS_ERROR_OUT_OF_MEMORY);
+          NS_ADDREF(*_result = new IntegerVariant(-visitCount));
         }
         else {
           // Estimate frecency using the last few visits.
           // Use NS_ceilf() so that we don't round down to 0, which
           // would cause us to completely ignore the place during autocomplete.
-          NS_IF_ADDREF(*_result = new IntegerVariant((PRInt32) NS_ceilf(fullVisitCount * NS_ceilf(pointsForSampledVisits) / numSampledVisits)));
-          NS_ENSURE_TRUE(*_result, NS_ERROR_OUT_OF_MEMORY);
+          NS_ADDREF(*_result = new IntegerVariant((PRInt32) NS_ceilf(fullVisitCount * NS_ceilf(pointsForSampledVisits) / numSampledVisits)));
         }
 
         return NS_OK;
       }
     }
 
     // This page is unknown or has no visits.  It could have just been added, so
     // use passed in or default values.
@@ -582,16 +576,54 @@ namespace places {
       bonus += history->GetFrecencyTransitionBonus(nsINavHistoryService::TRANSITION_TYPED, false);
     }
 
     // Assume "now" as our ageInDays, so use the first bucket.
     pointsForSampledVisits = history->GetFrecencyBucketWeight(1) * (bonus / (float)100.0); 
 
     // use NS_ceilf() so that we don't round down to 0, which
     // would cause us to completely ignore the place during autocomplete
-    NS_IF_ADDREF(*_result = new IntegerVariant((PRInt32) NS_ceilf(fullVisitCount * NS_ceilf(pointsForSampledVisits))));
-    NS_ENSURE_TRUE(*_result, NS_ERROR_OUT_OF_MEMORY);
+    NS_ADDREF(*_result = new IntegerVariant((PRInt32) NS_ceilf(fullVisitCount * NS_ceilf(pointsForSampledVisits))));
+
+    return NS_OK;
+  }
+
+////////////////////////////////////////////////////////////////////////////////
+//// GUID Creation Function
+
+  //////////////////////////////////////////////////////////////////////////////
+  //// GenerateGUIDFunction
+
+  /* static */
+  nsresult
+  GenerateGUIDFunction::create(mozIStorageConnection *aDBConn)
+  {
+    nsCOMPtr<GenerateGUIDFunction> function = new GenerateGUIDFunction();
+    nsresult rv = aDBConn->CreateFunction(
+      NS_LITERAL_CSTRING("generate_guid"), 0, function
+    );
+    NS_ENSURE_SUCCESS(rv, rv);
 
     return NS_OK;
   }
 
+  NS_IMPL_THREADSAFE_ISUPPORTS1(
+    GenerateGUIDFunction,
+    mozIStorageFunction
+  )
+
+  //////////////////////////////////////////////////////////////////////////////
+  //// mozIStorageFunction
+
+  NS_IMETHODIMP
+  GenerateGUIDFunction::OnFunctionCall(mozIStorageValueArray *aArguments,
+                                       nsIVariant **_result)
+  {
+    nsCAutoString guid;
+    nsresult rv = GenerateGUID(guid);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    NS_ADDREF(*_result = new UTF8TextVariant(guid));
+    return NS_OK;
+  }
+
 } // namespace places
 } // namespace mozilla
--- a/toolkit/components/places/src/SQLFunctions.h
+++ b/toolkit/components/places/src/SQLFunctions.h
@@ -217,12 +217,33 @@ public:
    * Registers the function with the specified database connection.
    *
    * @param aDBConn
    *        The database connection to register with.
    */
   static nsresult create(mozIStorageConnection *aDBConn);
 };
 
+/**
+ * SQL function to generate a GUID for a place or bookmark item.  This is just
+ * a wrapper around GenerateGUID in Helpers.h.
+ *
+ * @return a guid for the item.
+ */
+class GenerateGUIDFunction : public mozIStorageFunction
+{
+public:
+  NS_DECL_ISUPPORTS
+  NS_DECL_MOZISTORAGEFUNCTION
+
+  /**
+   * Registers the function with the specified database connection.
+   *
+   * @param aDBConn
+   *        The database connection to register with.
+   */
+  static nsresult create(mozIStorageConnection *aDBConn);
+};
+
 } // namespace places
 } // namespace storage
 
 #endif // mozilla_places_SQLFunctions_h_
--- a/toolkit/components/places/src/nsNavHistory.cpp
+++ b/toolkit/components/places/src/nsNavHistory.cpp
@@ -1099,16 +1099,19 @@ nsNavHistory::InitFunctions()
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = MatchAutoCompleteFunction::create(mDBConn);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = CalculateFrecencyFunction::create(mDBConn);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  rv = GenerateGUIDFunction::create(mDBConn);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   return NS_OK;
 }
 
 mozIStorageStatement*
 nsNavHistory::GetStatement(const nsCOMPtr<mozIStorageStatement>& aStmt)
 {
   // mCanNotify is set to false on shutdown.
   if (!mCanNotify)
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/unit/test_sql_guid_functions.js
@@ -0,0 +1,127 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * This file tests that the guid function generates a guid of the proper length,
+ * with no invalid characters.
+ */
+
+/**
+ * Checks all our invariants about our guids for a given result.
+ *
+ * @param aGuid
+ *        The guid to check.
+ */
+function check_invariants(aGuid)
+{
+  print("TEST-INFO | " + tests[index - 1].name + " | Checking guid '" +
+        aGuid + "'");
+
+  const kGUIDLength = 12;
+  do_check_eq(aGuid.length, kGUIDLength);
+  do_check_true(/[a-zA-Z0-9\-_]{12}/.test(aGuid));
+}
+
+////////////////////////////////////////////////////////////////////////////////
+//// Test Functions
+
+function test_guid_invariants()
+{
+  const kExpectedChars = 64;
+  const kAllowedChars =
+    "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_"
+  do_check_eq(kAllowedChars.length, kExpectedChars);
+  let checkedChars = {};
+  for (let i = 0; i < kAllowedChars; i++) {
+    checkedChars[kAllowedChars[i]] = false;
+  }
+
+  // We run this until we've seen every character that we expect to see.
+  let seenChars = 0;
+  let stmt = DBConn().createStatement("SELECT GENERATE_GUID()");
+  while (seenChars != kExpectedChars) {
+    do_check_true(stmt.executeStep());
+    let guid = stmt.getString(0);
+    check_invariants(guid);
+
+    for (let i = 0; i < guid.length; i++) {
+      if (!checkedChars[guid[i]]) {
+        checkedChars[guid[i]] = true;
+        seenChars++;
+      }
+    }
+    stmt.reset();
+  }
+  stmt.finalize();
+
+  // One last reality check - make sure all of our characters were seen.
+  for (let i = 0; i < kAllowedChars; i++) {
+    do_check_true(checkedChars[kAllowedChars[i]]);
+  }
+
+  run_next_test();
+}
+
+function test_guid_on_background()
+{
+  // We should not assert if we execute this asynchronously.
+  let stmt = DBConn().createAsyncStatement("SELECT GENERATE_GUID()");
+  let checked = false;
+  stmt.executeAsync({
+    handleResult: function(aResult) {
+      try {
+        let row = aResult.getNextRow();
+        check_invariants(row.getResultByIndex(0));
+        do_check_eq(aResult.getNextRow(), null);
+        checked = true;
+      }
+      catch (e) {
+        do_throw(e);
+      }
+    },
+    handleCompletion: function(aReason) {
+      do_check_eq(aReason, Ci.mozIStorageStatementCallback.REASON_FINISHED);
+      do_check_true(checked);
+      run_next_test();
+    }
+  });
+  stmt.finalize();
+}
+
+////////////////////////////////////////////////////////////////////////////////
+//// Test Runner
+
+let tests = [
+  test_guid_invariants,
+  test_guid_on_background,
+];
+let index = 0;
+
+function run_next_test()
+{
+  function _run_next_test() {
+    if (index < tests.length) {
+      do_test_pending();
+      print("TEST-INFO | " + _TEST_FILE + " | Starting " + tests[index].name);
+
+      // Exceptions do not kill asynchronous tests, so they'll time out.
+      try {
+        tests[index++]();
+      }
+      catch (e) {
+        do_throw(e);
+      }
+    }
+
+    do_test_finished();
+  }
+
+  // For sane stacks during failures, we execute this code soon, but not now.
+  do_execute_soon(_run_next_test);
+}
+
+function run_test()
+{
+  do_test_pending();
+  run_next_test();
+}