Bug 731024 - Part 4: Handle additional types. Test for livemarks. r=nalexander
authorRichard Newman <rnewman@mozilla.com>
Tue, 27 Mar 2012 10:47:26 -0700
changeset 90431 56789e683c23ecb10aaba419d05e4568e173574c
parent 90430 a856b9136fd1351e7fb4272cbe21517ff0fab967
child 90432 752737597c228b8185517bf029fe1cea77e574c3
push id22358
push userkhuey@mozilla.com
push dateWed, 28 Mar 2012 14:41:10 +0000
treeherdermozilla-central@c3fd0768d46a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander
bugs731024
milestone14.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 731024 - Part 4: Handle additional types. Test for livemarks. r=nalexander
mobile/android/base/sync/Utils.java
mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksDataAccessor.java
mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java
mobile/android/base/sync/repositories/android/AndroidBrowserRepositorySession.java
mobile/android/base/sync/repositories/domain/BookmarkRecord.java
--- a/mobile/android/base/sync/Utils.java
+++ b/mobile/android/base/sync/Utils.java
@@ -36,21 +36,24 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
 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;
 import org.mozilla.apache.commons.codec.binary.Base32;
 import org.mozilla.apache.commons.codec.binary.Base64;
 
 import android.content.Context;
 import android.content.SharedPreferences;
@@ -262,9 +265,43 @@ public class Utils {
     }
     for (int i = 0; i < size; ++i) {
       if (!same(a.get(i), b.get(i))) {
         return false;
       }
     }
     return true;
   }
+
+  /**
+   * Takes a URI, extracting URI components.
+   * @param scheme the URI scheme on which to match.
+   */
+  public static Map<String, String> 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<String, String> out = new HashMap<String, String>();
+    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;
+  }
 }
--- a/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksDataAccessor.java
+++ b/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksDataAccessor.java
@@ -182,36 +182,37 @@ public class AndroidBrowserBookmarksData
     record.title = AndroidBrowserBookmarksRepositorySession.SPECIAL_GUIDS_MAP.get(guid);
     record.type = "folder";
     record.androidParentID = parentId;
     return(RepoUtils.getAndroidIdFromUri(insert(record)));
   }
 
   @Override
   protected ContentValues getContentValues(Record record) {
+    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();
-    BookmarkRecord rec = (BookmarkRecord) record;
     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);
     if (rec.tags == null) {
       rec.tags = new JSONArray();
     }
     cv.put(BrowserContract.Bookmarks.TAGS,        rec.tags.toJSONString());
     cv.put(BrowserContract.Bookmarks.KEYWORD,     rec.keyword);
     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;
   }
 
   /**
    * Returns a cursor over non-deleted records that list the given androidID as a parent.
    */
--- a/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java
+++ b/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java
@@ -193,18 +193,18 @@ public class AndroidBrowserBookmarksRepo
       m.put("mobile",  context.getString(R.string.bookmarks_folder_mobile));
       SPECIAL_GUIDS_MAP = Collections.unmodifiableMap(m);
     }
 
     dbHelper = new AndroidBrowserBookmarksDataAccessor(context);
     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) {
     return getTypeFromCursor(cur) == BrowserContract.Bookmarks.TYPE_FOLDER;
   }
 
   private String getGUIDForID(long androidID) {
     String guid = idToGuid.get(androidID);
@@ -474,20 +474,20 @@ public class AndroidBrowserBookmarksRepo
     }
     BookmarkRecord bmk = (BookmarkRecord) record;
 
     if (forbiddenGUID(bmk.guid)) {
       Logger.debug(LOG_TAG, "Ignoring forbidden record with guid: " + bmk.guid);
       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;
   }
   
   @Override
   public void begin(RepositorySessionBeginDelegate delegate) throws InvalidSessionTransitionException {
     // Check for the existence of special folders
     // and insert them if they don't exist.
@@ -573,32 +573,32 @@ public class AndroidBrowserBookmarksRepo
       handleParenting(bmk);
     }
 
     if (Logger.LOG_PERSONAL_INFORMATION) {
       if (bmk.isFolder()) {
         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;
   }
 
   /**
    * If the provided record doesn't have correct parent information,
    * update appropriate bookkeeping to improve the situation.
@@ -680,17 +680,17 @@ public class AndroidBrowserBookmarksRepo
   @Override
   protected void storeRecordDeletion(final Record record) {
     if (SPECIAL_GUIDS_MAP.containsKey(record.guid)) {
       Logger.debug(LOG_TAG, "Told to delete record " + record.guid + ". Ignoring.");
       return;
     }
     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;
     }
     super.storeRecordDeletion(record);
   }
 
   /**
    * When a folder deletion is received, we must ensure -- for database
@@ -763,17 +763,30 @@ public class AndroidBrowserBookmarksRepo
       }
     };
     storeWorkQueue.execute(command);
   }
 
   @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) {
     final String guid = rec.guid;
     if (guid == null) {
       // Oh dear.
       Logger.error(LOG_TAG, "No guid in computeParentFields!");
       return null;
@@ -810,18 +823,17 @@ public class AndroidBrowserBookmarksRepo
       Logger.debug(LOG_TAG, "Returning " + (rec.deleted ? "deleted " : "") +
                             "bookmark record " + rec.guid + " (" + rec.androidID +
                            ", parent " + rec.parentID + ")");
       if (!rec.deleted && Logger.LOG_PERSONAL_INFORMATION) {
         Logger.pii(LOG_TAG, "> Parent name:      " + rec.parentName);
         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 ?
                                  "null" :
                                  rec.children.toJSONString()));
         }
       }
     } catch (Exception e) {
@@ -838,25 +850,30 @@ public class AndroidBrowserBookmarksRepo
     final boolean deleted   = isDeleted(cur);
     BookmarkRecord rec = new BookmarkRecord(guid, collection, lastModified, deleted);
 
     // No point in populating it.
     if (deleted) {
       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);
     rec.children = children;
 
     // Need to restore the parentId since it isn't stored in content provider.
     // We also take this opportunity to fix up parents for special folders,
     // allowing us to map between the hierarchies used by Fennec and Places.
--- a/mobile/android/base/sync/repositories/android/AndroidBrowserRepositorySession.java
+++ b/mobile/android/base/sync/repositories/android/AndroidBrowserRepositorySession.java
@@ -451,17 +451,17 @@ public abstract class AndroidBrowserRepo
           Record toStore = reconcileRecords(record, existingRecord, lastRemoteRetrieval, lastLocalRetrieval);
 
           if (toStore == null) {
             Logger.debug(LOG_TAG, "Reconciling returned null. Not inserting a record.");
             return;
           }
 
           // 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
           // of reconcileRecords.
           Logger.debug(LOG_TAG, "Calling delegate callback with guid " + replaced.guid +
                                 "(" + replaced.androidID + ")");
           delegate.onRecordStoreSucceeded(replaced);
           return;
@@ -565,24 +565,30 @@ public abstract class AndroidBrowserRepo
    * @throws NullCursorException
    * @throws ParentNotFoundException
    */
   protected Record findExistingRecord(Record record) throws MultipleRecordsForGuidException,
     NoGuidForIdException, NullCursorException, ParentNotFoundException {
 
     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<String, String> getRecordToGuidMap() throws NoGuidForIdException, NullCursorException, ParentNotFoundException {
     if (recordToGuid == null) {
       createRecordToGuidMap();
     }
     return recordToGuid;
   }
@@ -597,27 +603,34 @@ public abstract class AndroidBrowserRepo
     Cursor cur = dbHelper.fetchAll();
     try {
       if (!cur.moveToFirst()) {
         return;
       }
       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();
       }
     } finally {
       cur.close();
     }
     Logger.info(LOG_TAG, "END: creating record -> GUID map.");
   }
 
   public void putRecordToGuidMap(String recordString, String guid) throws NoGuidForIdException, NullCursorException, ParentNotFoundException {
+    if (recordString == null) {
+      return;
+    }
+
     if (recordToGuid == null) {
       createRecordToGuidMap();
     }
     recordToGuid.put(recordString, guid);
   }
 
   protected abstract Record prepareRecord(Record record);
   protected void updateBookkeeping(Record record) throws NoGuidForIdException,
--- a/mobile/android/base/sync/repositories/domain/BookmarkRecord.java
+++ b/mobile/android/base/sync/repositories/domain/BookmarkRecord.java
@@ -33,31 +33,37 @@
  * 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 ***** */
 
 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;
 import org.mozilla.gecko.sync.NonArrayJSONException;
 import org.mozilla.gecko.sync.Utils;
 import org.mozilla.gecko.sync.repositories.android.RepoUtils;
 
 import android.util.Log;
 
 /**
  * Covers the fields used by all bookmark objects.
  * @author rnewman
  *
  */
 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";
 
   public BookmarkRecord(String guid, String collection, long lastModified, boolean deleted) {
     super(guid, collection, lastModified, deleted);
   }
   public BookmarkRecord(String guid, String collection, long lastModified) {
@@ -78,17 +84,16 @@ public class BookmarkRecord extends Reco
   public String  title;
   public String  bookmarkURI;
   public String  description;
   public String  keyword;
   public String  parentID;
   public String  parentName;
   public long    androidParentID;
   public String  type;
-  public String  pos;
   public long    androidPosition;
 
   public JSONArray children;
   public JSONArray tags;
 
   @Override
   public String toString() {
     return "#<Bookmark " + guid + " (" + androidID + "), parent " +
@@ -126,17 +131,16 @@ public class BookmarkRecord extends Reco
     out.title           = this.title;
     out.bookmarkURI     = this.bookmarkURI;
     out.description     = this.description;
     out.keyword         = this.keyword;
     out.parentID        = this.parentID;
     out.parentName      = this.parentName;
     out.androidParentID = this.androidParentID;
     out.type            = this.type;
-    out.pos             = this.pos;
     out.androidPosition = this.androidPosition;
 
     out.children        = this.copyChildren();
     out.tags            = this.copyTags();
 
     return out;
   }
 
@@ -192,89 +196,155 @@ public class BookmarkRecord extends Reco
     }
     return type.equals("bookmark") ||
            type.equals("microsummary") ||
            type.equals("query");
   }
 
   @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");
       } catch (NonArrayJSONException e) {
         Log.e(LOG_TAG, "Got non-array children in bookmark record " + this.guid, e);
         // Let's see if we can recover later by using the parentid pointers.
         this.children = new JSONArray();
       }
       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;
     }
   }
 
   @Override
   protected void populatePayload(ExtendedJSONObject payload) {
     putPayload(payload, "type", this.type);
     putPayload(payload, "title", this.title);
     putPayload(payload, "description", this.description);
     putPayload(payload, "parentid", this.parentID);
     putPayload(payload, "parentName", this.parentName);
     putPayload(payload, "keyword", this.keyword);
 
-    if (this.tags != null) {
-      payload.put("tags", this.tags);
+    if (isFolder()) {
+      payload.put("children", this.children);
+      return;
+    }
+
+    // bookmark, microsummary, query.
+    if (isBookmarkIsh()) {
+      if (isBookmark()) {
+        payload.put("bmkUri", bookmarkURI);
+      }
+
+      if (isQuery()) {
+        Map<String, String> 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 (isBookmark()) {
-      payload.put("bmkUri", bookmarkURI);
-    } else if (isFolder()) {
-      payload.put("children", this.children);
+    if (isLivemark()) {
+      Map<String, String> parts = Utils.extractURIComponents(PLACES_URI_PREFIX, this.bookmarkURI);
+      putPayload(payload, "siteUri", parts.get("siteUri"));
+      putPayload(payload, "feedUri", parts.get("feedUri"));
+      return;
+    }
+    if (isMicrosummary()) {
+      Map<String, String> parts = Utils.extractURIComponents(PLACES_URI_PREFIX, this.bookmarkURI);
+      putPayload(payload, "generatorUri", parts.get("generatorUri"));
+      putPayload(payload, "staticTitle", parts.get("staticTitle"));
+      return;
     }
-
-    // TODO: fields for other types.
+    if (isSeparator()) {
+      Map<String, String> 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) {
     Logger.trace(LOG_TAG, s);
   }
 
   @Override
   public boolean equalPayloads(Object o) {
@@ -343,16 +413,71 @@ public class BookmarkRecord extends Reco
   // touching the data there (and therefore ordering won't change)
   private boolean jsonArrayStringsEqual(JSONArray a, JSONArray b) {
     // Check for nulls
     if (a == b) return true;
     if (a == null && b != null) return false;
     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();
+  }
 }
 
 
 /*
 // Bookmark:
 {cleartext:
   {id:            "l7p2xqOTMMXw",
    type:          "bookmark",