From 1e7bc71d4f00c186b2a7621d08a0f2d88544ad30 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Tue, 27 Mar 2012 10:47:26 -0700 Subject: [PATCH] Bug 731024 - Part 4: Handle additional types. Test for livemarks. r=nalexander --- mobile/android/base/sync/Utils.java | 37 ++++ .../AndroidBrowserBookmarksDataAccessor.java | 15 +- ...roidBrowserBookmarksRepositorySession.java | 47 +++-- .../AndroidBrowserRepositorySession.java | 27 ++- .../repositories/domain/BookmarkRecord.java | 199 ++++++++++++++---- 5 files changed, 259 insertions(+), 66 deletions(-) diff --git a/mobile/android/base/sync/Utils.java b/mobile/android/base/sync/Utils.java index d4d16b88bb2..c1fd3142d5b 100644 --- a/mobile/android/base/sync/Utils.java +++ b/mobile/android/base/sync/Utils.java @@ -41,11 +41,14 @@ package org.mozilla.gecko.sync; import java.io.UnsupportedEncodingException; import java.math.BigDecimal; import java.math.BigInteger; +import java.net.URLDecoder; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.util.ArrayList; +import java.util.HashMap; import java.util.Locale; +import java.util.Map; import java.util.TreeMap; import org.json.simple.JSONArray; @@ -267,4 +270,38 @@ public class Utils { } return true; } + + /** + * Takes a URI, extracting URI components. + * @param scheme the URI scheme on which to match. + */ + public static Map extractURIComponents(String scheme, String uri) { + if (uri.indexOf(scheme) != 0) { + throw new IllegalArgumentException("URI scheme does not match: " + scheme); + } + + // Do this the hard way to avoid taking a large dependency on + // HttpClient or getting all regex-tastic. + String components = uri.substring(scheme.length()); + HashMap out = new HashMap(); + String[] parts = components.split("&"); + for (int i = 0; i < parts.length; ++i) { + String part = parts[i]; + if (part.length() == 0) { + continue; + } + String[] pair = part.split("=", 2); + switch (pair.length) { + case 0: + continue; + case 1: + out.put(URLDecoder.decode(pair[0]), null); + break; + case 2: + out.put(URLDecoder.decode(pair[0]), URLDecoder.decode(pair[1])); + break; + } + } + return out; + } } diff --git a/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksDataAccessor.java b/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksDataAccessor.java index 46e3093dabb..a62e07d9867 100644 --- a/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksDataAccessor.java +++ b/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksDataAccessor.java @@ -187,9 +187,16 @@ public class AndroidBrowserBookmarksDataAccessor extends AndroidBrowserRepositor @Override protected ContentValues getContentValues(Record record) { - ContentValues cv = new ContentValues(); BookmarkRecord rec = (BookmarkRecord) record; + + final int recordType = BrowserContractHelpers.typeCodeForString(rec.type); + if (recordType == -1) { + throw new IllegalStateException("Unexpected record type " + rec.type); + } + + ContentValues cv = new ContentValues(); cv.put(BrowserContract.SyncColumns.GUID, rec.guid); + cv.put(BrowserContract.Bookmarks.TYPE, recordType); cv.put(BrowserContract.Bookmarks.TITLE, rec.title); cv.put(BrowserContract.Bookmarks.URL, rec.bookmarkURI); cv.put(BrowserContract.Bookmarks.DESCRIPTION, rec.description); @@ -201,12 +208,6 @@ public class AndroidBrowserBookmarksDataAccessor extends AndroidBrowserRepositor cv.put(BrowserContract.Bookmarks.PARENT, rec.androidParentID); cv.put(BrowserContract.Bookmarks.POSITION, rec.androidPosition); - // Only bookmark and folder types should make it this far. - // Other types should be filtered out and dropped. - cv.put(BrowserContract.Bookmarks.TYPE, rec.type.equalsIgnoreCase(TYPE_FOLDER) ? - BrowserContract.Bookmarks.TYPE_FOLDER : - BrowserContract.Bookmarks.TYPE_BOOKMARK); - // Note that we don't set the modified timestamp: we allow the // content provider to do that for us. return cv; diff --git a/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java b/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java index 66dea741c8c..61b4ee12315 100644 --- a/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java +++ b/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java @@ -198,8 +198,8 @@ public class AndroidBrowserBookmarksRepositorySession extends AndroidBrowserRepo dataAccessor = (AndroidBrowserBookmarksDataAccessor) dbHelper; } - private static long getTypeFromCursor(Cursor cur) { - return RepoUtils.getLongFromCursor(cur, BrowserContract.Bookmarks.TYPE); + private static int getTypeFromCursor(Cursor cur) { + return RepoUtils.getIntFromCursor(cur, BrowserContract.Bookmarks.TYPE); } private static boolean rowIsFolder(Cursor cur) { @@ -479,10 +479,10 @@ public class AndroidBrowserBookmarksRepositorySession extends AndroidBrowserRepo return true; } - if (bmk.isBookmark() || - bmk.isFolder()) { + if (BrowserContractHelpers.isSupportedType(bmk.type)) { return false; } + Logger.debug(LOG_TAG, "Ignoring record with guid: " + bmk.guid + " and type: " + bmk.type); return true; } @@ -578,22 +578,22 @@ public class AndroidBrowserBookmarksRepositorySession extends AndroidBrowserRepo Logger.pii(LOG_TAG, "Inserting folder " + bmk.guid + ", " + bmk.title + " with parent " + bmk.androidParentID + " (" + bmk.parentID + ", " + bmk.parentName + - ", " + bmk.pos + ")"); + ", " + bmk.androidPosition + ")"); } else { Logger.pii(LOG_TAG, "Inserting bookmark " + bmk.guid + ", " + bmk.title + ", " + bmk.bookmarkURI + " with parent " + bmk.androidParentID + " (" + bmk.parentID + ", " + bmk.parentName + - ", " + bmk.pos + ")"); + ", " + bmk.androidPosition + ")"); } } else { if (bmk.isFolder()) { Logger.debug(LOG_TAG, "Inserting folder " + bmk.guid + ", parent " + bmk.androidParentID + - " (" + bmk.parentID + ", " + bmk.pos + ")"); + " (" + bmk.parentID + ", " + bmk.androidPosition + ")"); } else { Logger.debug(LOG_TAG, "Inserting bookmark " + bmk.guid + " with parent " + bmk.androidParentID + - " (" + bmk.parentID + ", " + ", " + bmk.pos + ")"); + " (" + bmk.parentID + ", " + ", " + bmk.androidPosition + ")"); } } return bmk; @@ -685,7 +685,7 @@ public class AndroidBrowserBookmarksRepositorySession extends AndroidBrowserRepo } final BookmarkRecord bookmarkRecord = (BookmarkRecord) record; if (bookmarkRecord.isFolder()) { - Logger.debug(LOG_TAG, "Deleting folder. Ensuring consistency of children."); + Logger.debug(LOG_TAG, "Deleting folder. Ensuring consistency of children. TODO: Bug 724470."); handleFolderDeletion(bookmarkRecord); return; } @@ -768,7 +768,20 @@ public class AndroidBrowserBookmarksRepositorySession extends AndroidBrowserRepo @Override protected String buildRecordString(Record record) { BookmarkRecord bmk = (BookmarkRecord) record; - return bmk.title + bmk.bookmarkURI + bmk.type + bmk.parentName; + String parent = bmk.parentName + "/"; + if (bmk.isBookmark()) { + return "b" + parent + bmk.bookmarkURI + ":" + bmk.title; + } + if (bmk.isFolder()) { + return "f" + parent + bmk.title; + } + if (bmk.isSeparator()) { + return "s" + parent + bmk.androidPosition; + } + if (bmk.isQuery()) { + return "q" + parent + bmk.bookmarkURI; + } + return null; } public static BookmarkRecord computeParentFields(BookmarkRecord rec, String suggestedParentGUID, String suggestedParentName) { @@ -815,8 +828,7 @@ public class AndroidBrowserBookmarksRepositorySession extends AndroidBrowserRepo Logger.pii(LOG_TAG, "> Title: " + rec.title); Logger.pii(LOG_TAG, "> Type: " + rec.type); Logger.pii(LOG_TAG, "> URI: " + rec.bookmarkURI); - Logger.pii(LOG_TAG, "> Android position: " + rec.androidPosition); - Logger.pii(LOG_TAG, "> Position: " + rec.pos); + Logger.pii(LOG_TAG, "> Position: " + rec.androidPosition); if (rec.isFolder()) { Logger.pii(LOG_TAG, "FOLDER: Children are " + (rec.children == null ? @@ -843,15 +855,20 @@ public class AndroidBrowserBookmarksRepositorySession extends AndroidBrowserRepo return logBookmark(rec); } - boolean isFolder = rowIsFolder(cur); + int rowType = getTypeFromCursor(cur); + String typeString = BrowserContractHelpers.typeStringForCode(rowType); + if (typeString == null) { + Logger.warn(LOG_TAG, "Unsupported type code " + rowType); + return null; + } + + rec.type = typeString; rec.title = RepoUtils.getStringFromCursor(cur, BrowserContract.Bookmarks.TITLE); rec.bookmarkURI = RepoUtils.getStringFromCursor(cur, BrowserContract.Bookmarks.URL); rec.description = RepoUtils.getStringFromCursor(cur, BrowserContract.Bookmarks.DESCRIPTION); rec.tags = RepoUtils.getJSONArrayFromCursor(cur, BrowserContract.Bookmarks.TAGS); rec.keyword = RepoUtils.getStringFromCursor(cur, BrowserContract.Bookmarks.KEYWORD); - rec.type = isFolder ? AndroidBrowserBookmarksDataAccessor.TYPE_FOLDER : - AndroidBrowserBookmarksDataAccessor.TYPE_BOOKMARK; rec.androidID = RepoUtils.getLongFromCursor(cur, BrowserContract.Bookmarks._ID); rec.androidPosition = RepoUtils.getLongFromCursor(cur, BrowserContract.Bookmarks.POSITION); diff --git a/mobile/android/base/sync/repositories/android/AndroidBrowserRepositorySession.java b/mobile/android/base/sync/repositories/android/AndroidBrowserRepositorySession.java index 37e1f66ff8e..08c90ca13eb 100644 --- a/mobile/android/base/sync/repositories/android/AndroidBrowserRepositorySession.java +++ b/mobile/android/base/sync/repositories/android/AndroidBrowserRepositorySession.java @@ -456,7 +456,7 @@ public abstract class AndroidBrowserRepositorySession extends StoreTrackingRepos } // TODO: pass in timestamps? - Logger.debug(LOG_TAG, "Replacing " + existingRecord.guid + " with record " + toStore.guid); + Logger.debug(LOG_TAG, "Replacing existing " + existingRecord.guid + " with record " + toStore.guid); Record replaced = replace(toStore, existingRecord); // Note that we don't track records here; deciding that is the job @@ -570,14 +570,20 @@ public abstract class AndroidBrowserRepositorySession extends StoreTrackingRepos Logger.debug(LOG_TAG, "Finding existing record for incoming record with GUID " + record.guid); String recordString = buildRecordString(record); + if (recordString == null) { + Logger.debug(LOG_TAG, "No record string for incoming record " + record.guid); + return null; + } + Logger.debug(LOG_TAG, "Searching with record string " + recordString); String guid = getRecordToGuidMap().get(recordString); - if (guid != null) { - Logger.debug(LOG_TAG, "Found one. Returning computed record."); - return retrieveByGUIDDuringStore(guid); + if (guid == null) { + Logger.debug(LOG_TAG, "findExistingRecord failed to find one for " + record.guid); + return null; } - Logger.debug(LOG_TAG, "findExistingRecord failed to find one for " + record.guid); - return null; + + Logger.debug(LOG_TAG, "Found one. Returning computed record."); + return retrieveByGUIDDuringStore(guid); } public HashMap getRecordToGuidMap() throws NoGuidForIdException, NullCursorException, ParentNotFoundException { @@ -602,7 +608,10 @@ public abstract class AndroidBrowserRepositorySession extends StoreTrackingRepos while (!cur.isAfterLast()) { Record record = retrieveDuringStore(cur); if (record != null) { - recordToGuid.put(buildRecordString(record), record.guid); + final String recordString = buildRecordString(record); + if (recordString != null) { + recordToGuid.put(recordString, record.guid); + } } cur.moveToNext(); } @@ -613,6 +622,10 @@ public abstract class AndroidBrowserRepositorySession extends StoreTrackingRepos } public void putRecordToGuidMap(String recordString, String guid) throws NoGuidForIdException, NullCursorException, ParentNotFoundException { + if (recordString == null) { + return; + } + if (recordToGuid == null) { createRecordToGuidMap(); } diff --git a/mobile/android/base/sync/repositories/domain/BookmarkRecord.java b/mobile/android/base/sync/repositories/domain/BookmarkRecord.java index 79ce5c1da06..2b226ee99d3 100644 --- a/mobile/android/base/sync/repositories/domain/BookmarkRecord.java +++ b/mobile/android/base/sync/repositories/domain/BookmarkRecord.java @@ -38,6 +38,10 @@ package org.mozilla.gecko.sync.repositories.domain; +import java.io.UnsupportedEncodingException; +import java.net.URLEncoder; +import java.util.Map; + import org.json.simple.JSONArray; import org.mozilla.gecko.sync.ExtendedJSONObject; import org.mozilla.gecko.sync.Logger; @@ -53,6 +57,8 @@ import android.util.Log; * */ public class BookmarkRecord extends Record { + public static final String PLACES_URI_PREFIX = "places:"; + private static final String LOG_TAG = "BookmarkRecord"; public static final String COLLECTION_NAME = "bookmarks"; @@ -83,7 +89,6 @@ public class BookmarkRecord extends Record { public String parentName; public long androidParentID; public String type; - public String pos; public long androidPosition; public JSONArray children; @@ -131,7 +136,6 @@ public class BookmarkRecord extends Record { out.parentName = this.parentName; out.androidParentID = this.androidParentID; out.type = this.type; - out.pos = this.pos; out.androidPosition = this.androidPosition; out.children = this.copyChildren(); @@ -197,30 +201,12 @@ public class BookmarkRecord extends Record { @Override protected void initFromPayload(ExtendedJSONObject payload) { - this.type = (String) payload.get("type"); - this.title = (String) payload.get("title"); - this.description = (String) payload.get("description"); - this.parentID = (String) payload.get("parentid"); - this.parentName = (String) payload.get("parentName"); + this.type = payload.getString("type"); + this.title = payload.getString("title"); + this.description = payload.getString("description"); + this.parentID = payload.getString("parentid"); + this.parentName = payload.getString("parentName"); - // bookmark, microsummary, query. - if (isBookmarkIsh()) { - this.keyword = (String) payload.get("keyword"); - try { - this.tags = payload.getArray("tags"); - } catch (NonArrayJSONException e) { - Logger.warn(LOG_TAG, "Got non-array tags in bookmark record " + this.guid, e); - this.tags = new JSONArray(); - } - } - - // bookmark. - if (isBookmark()) { - this.bookmarkURI = (String) payload.get("bmkUri"); - return; - } - - // folder. if (isFolder()) { try { this.children = payload.getArray("children"); @@ -232,20 +218,64 @@ public class BookmarkRecord extends Record { return; } + final String bmkUri = payload.getString("bmkUri"); + + // bookmark, microsummary, query. + if (isBookmarkIsh()) { + this.keyword = payload.getString("keyword"); + try { + this.tags = payload.getArray("tags"); + } catch (NonArrayJSONException e) { + Logger.warn(LOG_TAG, "Got non-array tags in bookmark record " + this.guid, e); + this.tags = new JSONArray(); + } + } + + if (isBookmark()) { + this.bookmarkURI = bmkUri; + return; + } + if (isLivemark()) { - // TODO: siteUri, feedUri. + String siteUri = payload.getString("siteUri"); + String feedUri = payload.getString("feedUri"); + this.bookmarkURI = encodeUnsupportedTypeURI(bmkUri, + "siteUri", siteUri, + "feedUri", feedUri); return; } if (isQuery()) { - // TODO: queryId (optional), folderName. + String queryId = payload.getString("queryId"); + String folderName = payload.getString("folderName"); + this.bookmarkURI = encodeUnsupportedTypeURI(bmkUri, + "queryId", queryId, + "folderName", folderName); return; } if (isMicrosummary()) { - // TODO: generatorUri, staticTitle. + String generatorUri = payload.getString("generatorUri"); + String staticTitle = payload.getString("staticTitle"); + this.bookmarkURI = encodeUnsupportedTypeURI(bmkUri, + "generatorUri", generatorUri, + "staticTitle", staticTitle); return; } if (isSeparator()) { - this.pos = payload.getString("pos"); + Object p = payload.get("pos"); + if (p instanceof Long) { + this.androidPosition = (Long) p; + } else if (p instanceof String) { + try { + this.androidPosition = Long.parseLong((String) p, 10); + } catch (NumberFormatException e) { + return; + } + } else { + Logger.warn(LOG_TAG, "Unsupported position value " + p); + return; + } + String pos = String.valueOf(this.androidPosition); + this.bookmarkURI = encodeUnsupportedTypeURI(null, "pos", pos, null, null); return; } } @@ -259,17 +289,57 @@ public class BookmarkRecord extends Record { putPayload(payload, "parentName", this.parentName); putPayload(payload, "keyword", this.keyword); - if (this.tags != null) { - payload.put("tags", this.tags); - } - - if (isBookmark()) { - payload.put("bmkUri", bookmarkURI); - } else if (isFolder()) { + if (isFolder()) { payload.put("children", this.children); + return; } - // TODO: fields for other types. + // bookmark, microsummary, query. + if (isBookmarkIsh()) { + if (isBookmark()) { + payload.put("bmkUri", bookmarkURI); + } + + if (isQuery()) { + Map parts = Utils.extractURIComponents(PLACES_URI_PREFIX, this.bookmarkURI); + putPayload(payload, "queryId", parts.get("queryId")); + putPayload(payload, "folderName", parts.get("folderName")); + return; + } + + if (this.tags != null) { + payload.put("tags", this.tags); + } + + putPayload(payload, "keyword", this.keyword); + return; + } + + if (isLivemark()) { + Map parts = Utils.extractURIComponents(PLACES_URI_PREFIX, this.bookmarkURI); + putPayload(payload, "siteUri", parts.get("siteUri")); + putPayload(payload, "feedUri", parts.get("feedUri")); + return; + } + if (isMicrosummary()) { + Map parts = Utils.extractURIComponents(PLACES_URI_PREFIX, this.bookmarkURI); + putPayload(payload, "generatorUri", parts.get("generatorUri")); + putPayload(payload, "staticTitle", parts.get("staticTitle")); + return; + } + if (isSeparator()) { + Map parts = Utils.extractURIComponents(PLACES_URI_PREFIX, this.bookmarkURI); + String pos = parts.get("pos"); + if (pos == null) { + return; + } + try { + payload.put("pos", Long.parseLong(pos, 10)); + } catch (NumberFormatException e) { + return; + } + return; + } } private void trace(String s) { @@ -348,6 +418,61 @@ public class BookmarkRecord extends Record { if (a != null && b == null) return false; return RepoUtils.stringsEqual(a.toJSONString(), b.toJSONString()); } + + /** + * URL-encode the provided string. If the input is null, + * the empty string is returned. + * + * @param in the string to encode. + * @return a URL-encoded version of the input. + */ + protected static String encode(String in) { + if (in == null) { + return ""; + } + try { + return URLEncoder.encode(in, "UTF-8"); + } catch (UnsupportedEncodingException e) { + // Will never occur. + return null; + } + } + + /** + * Take the provided URI and two parameters, constructing a URI like + * + * places:uri=$uri&p1=$p1&p2=$p2 + * + * null values in either parameter or value result in the parameter being omitted. + */ + protected static String encodeUnsupportedTypeURI(String originalURI, String p1, String v1, String p2, String v2) { + StringBuilder b = new StringBuilder(PLACES_URI_PREFIX); + boolean previous = false; + if (originalURI != null) { + b.append("uri="); + b.append(encode(originalURI)); + previous = true; + } + if (p1 != null) { + if (previous) { + b.append("&"); + } + b.append(p1); + b.append("="); + b.append(encode(v1)); + previous = true; + } + if (p2 != null) { + if (previous) { + b.append("&"); + } + b.append(p2); + b.append("="); + b.append(encode(v2)); + previous = true; + } + return b.toString(); + } }