Bug 454977 - remove INSERT OR REPLACE to avoid error prone paths with views, r=dietrich

This commit is contained in:
Marco Bonardo 2009-01-13 11:48:25 +01:00
parent 2f9c3187e5
commit 28e10ad2a2
7 changed files with 346 additions and 57 deletions

View File

@ -51,11 +51,12 @@ enum {
kHiddenColumn,
kTypedColumn,
kLastVisitColumn,
kFirstVisitColumn,
kColumnCount // keep me last
};
static const char * const gColumnNames[] = {
"URL", "Name", "VisitCount", "Hidden", "Typed", "LastVisitDate"
"URL", "Name", "VisitCount", "Hidden", "Typed", "LastVisitDate", "FirstVisitDate"
};
struct TableReadClosure
@ -142,14 +143,19 @@ AddToHistoryCB(const nsCSubstring &aRowID,
const PRUnichar *title = reinterpret_cast<const PRUnichar*>(titleBytes);
PRInt32 err;
PRInt32 count = values[kVisitCountColumn].ToInteger(&err);
if (err != 0 || count == 0) {
count = 1;
PRInt32 visitCount = values[kVisitCountColumn].ToInteger(&err);
if (err != 0 || visitCount == 0) {
visitCount = 1;
}
PRTime date;
if (PR_sscanf(values[kLastVisitColumn].get(), "%lld", &date) != 1) {
date = -1;
PRTime lastVisitDate;
if (PR_sscanf(values[kLastVisitColumn].get(), "%lld", &lastVisitDate) != 1) {
lastVisitDate = -1;
}
PRTime firstVisitDate;
if (PR_sscanf(values[kFirstVisitColumn].get(), "%lld", &firstVisitDate) != 1) {
firstVisitDate = -1;
}
PRBool isTyped = values[kTypedColumn].EqualsLiteral("1");
@ -163,8 +169,9 @@ AddToHistoryCB(const nsCSubstring &aRowID,
titleStr = nsDependentString(title, titleLength);
else
titleStr.SetIsVoid(PR_TRUE);
history->AddPageWithVisit(uri, titleStr,
PR_FALSE, isTyped, count, transition, date);
history->AddPageWithVisits(uri, titleStr, visitCount, transition,
firstVisitDate, lastVisitDate);
}
return PL_DHASH_NEXT;
}

View File

@ -1218,7 +1218,7 @@ nsNavHistory::InitStatements()
// mDBAddNewPage (see InternalAddNewPage)
rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
"INSERT OR REPLACE INTO moz_places_view "
"INSERT INTO moz_places_view "
"(url, title, rev_host, hidden, typed, frecency) "
"VALUES (?1, ?2, ?3, ?4, ?5, ?6)"),
getter_AddRefs(mDBAddNewPage));
@ -2675,8 +2675,8 @@ nsNavHistory::AddVisit(nsIURI* aURI, PRTime aTime, nsIURI* aReferringURI,
NS_ENSURE_SUCCESS(rv, rv);
PRInt64 pageID = 0;
PRBool hidden; // XXX fix me, this should not be a PRBool, as we later do BindInt32Parameter()
PRBool typed; // XXX fix me, this should not be a PRBool, as we later do BindInt32Parameter()
PRInt32 hidden;
PRInt32 typed;
PRBool newItem = PR_FALSE; // used to send out notifications at the end
if (alreadyVisited) {
// Update the existing entry...
@ -2712,11 +2712,11 @@ nsNavHistory::AddVisit(nsIURI* aURI, PRTime aTime, nsIURI* aReferringURI,
// (aTransitionType == TRANSITION_TYPED) so that they will appear in
// the history UI (sidebar, history menu, url bar autocomplete, etc)
hidden = oldHiddenState;
if (hidden && (!aIsRedirect || aTransitionType == TRANSITION_TYPED) &&
if (hidden == 1 && (!aIsRedirect || aTransitionType == TRANSITION_TYPED) &&
aTransitionType != TRANSITION_EMBED)
hidden = PR_FALSE; // unhide
hidden = 0; // unhide
typed = oldTypedState || (aTransitionType == TRANSITION_TYPED);
typed = (PRInt32)(oldTypedState == 1 || (aTransitionType == TRANSITION_TYPED));
// some items may have a visit count of 0 which will not count for link
// visiting, so be sure to note this transition
@ -2745,14 +2745,15 @@ nsNavHistory::AddVisit(nsIURI* aURI, PRTime aTime, nsIURI* aReferringURI,
// Hide only embedded links and redirects
// See the hidden computation code above for a little more explanation.
hidden = (aTransitionType == TRANSITION_EMBED || aIsRedirect);
hidden = (PRInt32)(aTransitionType == TRANSITION_EMBED || aIsRedirect);
typed = (aTransitionType == TRANSITION_TYPED);
typed = (PRInt32)(aTransitionType == TRANSITION_TYPED);
// set as visited once, no title
nsString voidString;
voidString.SetIsVoid(PR_TRUE);
rv = InternalAddNewPage(aURI, voidString, hidden, typed, 1, PR_TRUE, &pageID);
rv = InternalAddNewPage(aURI, voidString, hidden == 1, typed == 1, 1,
PR_TRUE, &pageID);
NS_ENSURE_SUCCESS(rv, rv);
}
@ -6572,13 +6573,12 @@ nsNavHistory::SetPageTitleInternal(nsIURI* aURI, const nsAString& aTitle)
}
nsresult
nsNavHistory::AddPageWithVisit(nsIURI *aURI,
const nsString &aTitle,
PRBool aHidden,
PRBool aTyped,
PRInt32 aVisitCount,
PRInt32 aLastVisitTransition,
PRTime aLastVisitDate)
nsNavHistory::AddPageWithVisits(nsIURI *aURI,
const nsString &aTitle,
PRInt32 aVisitCount,
PRInt32 aTransitionType,
PRTime aFirstVisitDate,
PRTime aLastVisitDate)
{
PRBool canAdd = PR_FALSE;
nsresult rv = CanAddURI(aURI, &canAdd);
@ -6587,16 +6587,69 @@ nsNavHistory::AddPageWithVisit(nsIURI *aURI,
return NS_OK;
}
PRInt64 pageID;
// we don't want to calculate frecency here for each uri we import
rv = InternalAddNewPage(aURI, aTitle, aHidden, aTyped, aVisitCount, PR_FALSE, &pageID);
// see if this is an update (revisit) or a new page
mozStorageStatementScoper scoper(mDBGetPageVisitStats);
rv = BindStatementURI(mDBGetPageVisitStats, 0, aURI);
NS_ENSURE_SUCCESS(rv, rv);
PRBool alreadyVisited = PR_FALSE;
rv = mDBGetPageVisitStats->ExecuteStep(&alreadyVisited);
NS_ENSURE_SUCCESS(rv, rv);
if (aLastVisitDate != -1) {
PRInt64 visitID;
rv = InternalAddVisit(pageID, 0, 0,
aLastVisitDate, aLastVisitTransition, &visitID);
PRInt64 placeId = 0;
PRInt32 typed = 0;
PRInt32 hidden = 0;
if (alreadyVisited) {
// Update the existing entry
rv = mDBGetPageVisitStats->GetInt64(0, &placeId);
NS_ENSURE_SUCCESS(rv, rv);
// We don't mind visit_count
rv = mDBGetPageVisitStats->GetInt32(2, &typed);
NS_ENSURE_SUCCESS(rv, rv);
rv = mDBGetPageVisitStats->GetInt32(3, &hidden);
NS_ENSURE_SUCCESS(rv, rv);
if (typed == 0 && aTransitionType == TRANSITION_TYPED) {
typed = 1;
// Update with new stats
mozStorageStatementScoper updateScoper(mDBUpdatePageVisitStats);
rv = mDBUpdatePageVisitStats->BindInt64Parameter(0, placeId);
NS_ENSURE_SUCCESS(rv, rv);
rv = mDBUpdatePageVisitStats->BindInt32Parameter(1, hidden);
NS_ENSURE_SUCCESS(rv, rv);
rv = mDBUpdatePageVisitStats->BindInt32Parameter(2, typed);
NS_ENSURE_SUCCESS(rv, rv);
rv = mDBUpdatePageVisitStats->Execute();
NS_ENSURE_SUCCESS(rv, rv);
}
} else {
// Insert the new place entry
rv = InternalAddNewPage(aURI, aTitle, hidden == 1,
aTransitionType == TRANSITION_TYPED, 0,
PR_FALSE, &placeId);
NS_ENSURE_SUCCESS(rv, rv);
}
NS_ASSERTION(placeId != 0, "Cannot add a visit to a not existant page");
if (aFirstVisitDate != -1) {
// Add the first visit
PRInt64 visitId;
rv = InternalAddVisit(placeId, 0, 0,
aFirstVisitDate, aTransitionType, &visitId);
aVisitCount--;
NS_ENSURE_SUCCESS(rv, rv);
}
if (aLastVisitDate != -1) {
// Add remaining visits starting from the last one
for (PRInt64 i = 0; i < aVisitCount; i++) {
PRInt64 visitId;
rv = InternalAddVisit(placeId, 0, 0,
aLastVisitDate - i, aTransitionType, &visitId);
NS_ENSURE_SUCCESS(rv, rv);
}
}
return NS_OK;

View File

@ -349,25 +349,20 @@ public:
// Import-friendly version of AddVisit.
// This method adds a page to history along with a single last visit.
// It is an error to call this method if aURI might already be in history.
// The given aVisitCount should include the given last-visit date.
// aLastVisitDate can be -1 if there is no last visit date to record.
//
// NOTE: This will *replace* existing records for a given URI, creating a
// new place id, and breaking all existing relationships with for that
// id, eg: bookmarks, annotations, tags, etc. This is only for use by
// the import of history.dat on first-run of Places, which currently occurs
// if no places.sqlite file previously exists.
nsresult AddPageWithVisit(nsIURI *aURI,
const nsString &aTitle,
PRBool aHidden, PRBool aTyped,
PRInt32 aVisitCount,
PRInt32 aLastVisitTransition,
PRTime aLastVisitDate);
// This is only for use by the import of history.dat on first-run of Places,
// which currently occurs if no places.sqlite file previously exists.
nsresult AddPageWithVisits(nsIURI *aURI,
const nsString &aTitle,
PRInt32 aVisitCount,
PRInt32 aTransitionType,
PRTime aFirstVisitDate,
PRTime aLastVisitDate);
// Checks the database for any duplicate URLs. If any are found,
// all but the first are removed. This must be called after using
// AddPageWithVisit, to ensure that the database is in a consistent state.
// AddPageWithVisits, to ensure that the database is in a consistent state.
nsresult RemoveDuplicateURIs();
// sets the schema version in the database to match SCHEMA_VERSION

View File

@ -0,0 +1,17 @@
// <!-- <mdb:mork:z v="1.4"/> -->
< <(a=c)> // (f=iso-8859-1)
(8A=Typed)(8B=LastPageVisited)(8C=ByteOrder)
(80=ns:history:db:row:scope:history:all)
(81=ns:history:db:table:kind:history)(82=URL)(83=Referrer)
(84=LastVisitDate)(85=FirstVisitDate)(86=VisitCount)(87=Name)
(88=Hostname)(89=Hidden)>
<(80=LE)(8A=http://www.mozilla.org/)(99=1230666859910484)(96
=1230666845910353)(8C=mozilla.org)(84=1)(95=4)(8E
=M$00o$00z$00i$00l$00l$00a$00.$00o$00r$00g$00 $00-$00 $00H$00o$00m$00e$00 \
$00o$00f$00 $00t$00h$00e$00 $00M$00o$00z$00i$00l$00l$00a$00 $00P$00r$00o$00j$00\
e$00c$00t$00)(8F=http://www.google.it/)(97=1230666851648604)(88=google.it)
(93=3)(91=G$00o$00o$00g$00l$00e$00)>
{1:^80 {(k^81:c)(s=9)[1(^8C=LE)]}
[7(^82^8A)(^84^99)(^85^96)(^88^8C)(^8A=1)(^86=4)(^87^8E)]
[8(^82^8F)(^84^99)(^85^97)(^88^88)(^8A=1)(^86=3)(^87^91)]}

View File

@ -0,0 +1,123 @@
/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* vim:set ts=2 sw=2 sts=2 et: */
/* ***** BEGIN LICENSE BLOCK *****
* Version: MPL 1.1/GPL 2.0/LGPL 2.1
*
* The contents of this file are subject to the Mozilla Public License Version
* 1.1 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
* http://www.mozilla.org/MPL/
*
* Software distributed under the License is distributed on an "AS IS" basis,
* WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
* for the specific language governing rights and limitations under the
* License.
*
* The Original Code is Bug 454977 code.
*
* The Initial Developer of the Original Code is Mozilla Corp.
* Portions created by the Initial Developer are Copyright (C) 2009
* the Initial Developer. All Rights Reserved.
*
* Contributor(s):
* Marco Bonardo <mak77bonardo.net> (Original Author)
*
* Alternatively, the contents of this file may be used under the terms of
* either the GNU General Public License Version 2 or later (the "GPL"), or
* the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
* in which case the provisions of the GPL or the LGPL are applicable instead
* of those above. If you wish to allow use of your version of this file only
* under the terms of either the GPL or the LGPL, and not to allow others to
* use your version of this file under the terms of the MPL, indicate your
* decision by deleting the provisions above and replace them with the notice
* and other provisions required by the GPL or the LGPL. If you do not delete
* the provisions above, a recipient may use your version of this file under
* the terms of any one of the MPL, the GPL or the LGPL.
*
* ***** END LICENSE BLOCK ***** */
// Get services
try {
var hs = Cc["@mozilla.org/browser/nav-history-service;1"].
getService(Ci.nsINavHistoryService);
var mDBConn = hs.QueryInterface(Ci.nsPIPlacesDatabase).DBConnection;
} catch(ex) {
do_throw("Could not get services\n");
}
// Cache actual visit_count value, filled by add_visit, used by check_results
var visit_count = 0;
function add_visit(aURI, aVisitDate, aVisitType) {
var isRedirect = aVisitType == hs.TRANSITION_REDIRECT_PERMANENT ||
aVisitType == hs.TRANSITION_REDIRECT_TEMPORARY;
var visitId = hs.addVisit(aURI, aVisitDate, null,
aVisitType, isRedirect, 0);
do_check_true(visitId > 0);
// Increase visit_count if applicable
if (aVisitType != 0 &&
aVisitType != hs.TRANSITION_EMBED &&
aVisitType != hs.TRANSITION_DOWNLOAD)
visit_count ++;
// Get the place id
var sql = "SELECT place_id FROM moz_historyvisits_view WHERE id = ?1";
var stmt = mDBConn.createStatement(sql);
stmt.bindInt64Parameter(0, visitId);
do_check_true(stmt.executeStep());
var placeId = stmt.getInt64(0);
do_check_true(placeId > 0);
return placeId;
}
/**
* Checks for results consistency, using visit_count as constraint
* @param aExpectedCount
* Number of history results we are expecting (excluded hidden ones)
* @param aExpectedCountWithHidden
* Number of history results we are expecting (included hidden ones)
*/
function check_results(aExpectedCount, aExpectedCountWithHidden) {
var query = hs.getNewQuery();
// used to check visit_count
query.minVisits = visit_count;
query.maxVisits = visit_count;
var options = hs.getNewQueryOptions();
options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY;
result = hs.executeQuery(query, options);
var root = result.root;
root.containerOpen = true;
// Children without hidden ones
do_check_eq(root.childCount, aExpectedCount);
root.containerOpen = false;
// Execute again with includeHidden = true
// This will ensure visit_count is correct
options.includeHidden = true;
result = hs.executeQuery(query, options);
var root = result.root;
root.containerOpen = true;
// Children with hidden ones
do_check_eq(root.childCount, aExpectedCountWithHidden);
root.containerOpen = false;
}
// main
function run_test() {
var testURI = uri("http://test.mozilla.org/");
// Add a visit that force hidden
var placeId = add_visit(testURI, Date.now()*1000, hs.TRANSITION_EMBED);
check_results(0, 1);
// Add a visit that force unhide and check place id
// - We expect that the place gets hidden = 0 while retaining the same
// place_id and a correct visit_count.
do_check_eq(add_visit(testURI, Date.now()*1000, hs.TRANSITION_TYPED), placeId);
check_results(1, 1);
// Add a visit, check that hidden is not overwritten and check place id
// - We expect that the place has still hidden = 0, while retaining the same
// place_id and a correct visit_count.
do_check_eq(add_visit(testURI, Date.now()*1000, hs.TRANSITION_EMBED), placeId);
check_results(1, 1);
}

View File

@ -38,14 +38,11 @@
*
* ***** END LICENSE BLOCK ***** */
// Get history service
try {
var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].getService(Ci.nsINavHistoryService);
var gh = Cc["@mozilla.org/browser/global-history;2"].
getService(Ci.nsIGlobalHistory2);
} catch(ex) {
do_throw("Could not get history service\n");
}
// Get history services
var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].
getService(Ci.nsINavHistoryService);
var gh = histsvc.QueryInterface(Ci.nsIGlobalHistory2);
var bh = histsvc.QueryInterface(Ci.nsIBrowserHistory);
/**
* Adds a test URI visit to the database, and checks for a valid place ID.
@ -231,8 +228,6 @@ function run_test() {
histFile.append("history.dat");
do_check_true(histFile.exists());
var globalHistory = Components.classes["@mozilla.org/browser/global-history;2"]
.getService(Components.interfaces.nsIBrowserHistory);
globalHistory.removeAllPages();
bh.removeAllPages();
do_check_false(histFile.exists());
}

View File

@ -0,0 +1,99 @@
/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* vim:set ts=2 sw=2 sts=2 et: */
/* ***** BEGIN LICENSE BLOCK *****
* Version: MPL 1.1/GPL 2.0/LGPL 2.1
*
* The contents of this file are subject to the Mozilla Public License Version
* 1.1 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
* http://www.mozilla.org/MPL/
*
* Software distributed under the License is distributed on an "AS IS" basis,
* WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
* for the specific language governing rights and limitations under the
* License.
*
* The Original Code is Bug 454977 code.
*
* The Initial Developer of the Original Code is Mozilla Corp.
* Portions created by the Initial Developer are Copyright (C) 2009
* the Initial Developer. All Rights Reserved.
*
* Contributor(s):
* Marco Bonardo <mak77bonardo.net> (Original Author)
*
* Alternatively, the contents of this file may be used under the terms of
* either the GNU General Public License Version 2 or later (the "GPL"), or
* the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
* in which case the provisions of the GPL or the LGPL are applicable instead
* of those above. If you wish to allow use of your version of this file only
* under the terms of either the GPL or the LGPL, and not to allow others to
* use your version of this file under the terms of the MPL, indicate your
* decision by deleting the provisions above and replace them with the notice
* and other provisions required by the GPL or the LGPL. If you do not delete
* the provisions above, a recipient may use your version of this file under
* the terms of any one of the MPL, the GPL or the LGPL.
*
* ***** END LICENSE BLOCK ***** */
var hs = Cc["@mozilla.org/browser/nav-history-service;1"].
getService(Ci.nsINavHistoryService);
var gh = hs.QueryInterface(Ci.nsIGlobalHistory2);
/**
* Checks to see that a URI is in the database.
*
* @param aURI
* The URI to check.
* @returns true if the URI is in the DB, false otherwise.
*/
function uri_in_db(aURI) {
var options = hs.getNewQueryOptions();
options.maxResults = 1;
options.resultType = options.RESULTS_AS_URI
var query = hs.getNewQuery();
query.uri = aURI;
var result = hs.executeQuery(query, options);
var root = result.root;
root.containerOpen = true;
return (root.childCount == 1);
}
// main
function run_test() {
// test import history
var file = do_get_file("toolkit/components/places/tests/unit/history_import_test.dat");
gh.importHistory(file);
var uri1 = uri("http://www.mozilla.org/");
do_check_true(uri_in_db(uri1));
// Check visit count
var options = hs.getNewQueryOptions();
options.sortingMode = options.SORT_BY_DATE_DESCENDING;
options.resultType = options.RESULTS_AS_VISIT;
var query = hs.getNewQuery();
query.minVisits = 4;
query.maxVisits = 4;
query.uri = uri1;
var result = hs.executeQuery(query, options);
var root = result.root;
root.containerOpen = true;
var cc = root.childCount;
do_check_eq(cc, 4);
// Check first and last visits times are correct
var lastVisitDate = root.getChild(0).time;
do_check_eq(lastVisitDate, 1230666859910484);
var firstVisitDate = root.getChild(3).time;
do_check_eq(firstVisitDate, 1230666845910353);
// Check other visits have different times and are between first and last ones
do_check_true(root.getChild(1).time < lastVisitDate &&
root.getChild(1).time > firstVisitDate);
do_check_true(root.getChild(2).time < lastVisitDate &&
root.getChild(2).time > firstVisitDate);
do_check_true(root.getChild(1).time != root.getChild(2).time);
root.containerOpen = false;
}