Bug 731024 - Part 1: Unsupported record types causes Android Sync to reorder folders containing livemarks/queries/separators…. r=nalexander

mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java

@@ -152,6 +152,7 @@ public class AndroidBrowserBookmarksRepositorySession extends AndroidBrowserRepo
    *
   */
   public static final Map<String, String> SPECIAL_GUID_PARENTS;
+
   static {
     HashMap<String, String> m = new HashMap<String, String>();
     m.put("places",  null);
@@ -163,6 +164,7 @@ public class AndroidBrowserBookmarksRepositorySession extends AndroidBrowserRepo
     SPECIAL_GUID_PARENTS = Collections.unmodifiableMap(m);
   }
 
+
   /**
    * A map of guids to their localized name strings.
    */

mobile/android/base/sync/repositories/android/BrowserContractHelpers.java

@@ -4,12 +4,14 @@
 
 package org.mozilla.gecko.sync.repositories.android;
 
-import android.net.Uri;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
 
-// This will exist implicitly when we're merged into Fennec, or explicitly
-// due to our own imported copy of Fennec's BrowserContract.java.in.
 import org.mozilla.gecko.db.BrowserContract;
 
+import android.net.Uri;
+
 public class BrowserContractHelpers extends BrowserContract {
   protected static Uri withSyncAndDeleted(Uri u) {
     return u.buildUpon()
@@ -77,4 +79,52 @@ public class BrowserContractHelpers extends BrowserContract {
     Bookmarks.DESCRIPTION,
     Bookmarks.KEYWORD
   };
+
+  // Mapping from Sync types to Fennec types.
+  public static final String[] BOOKMARK_TYPE_CODE_TO_STRING = {
+    // Observe omissions: "microsummary", "item".
+    "folder", "bookmark", "separator", "livemark", "query"
+  };
+  private static final int MAX_BOOKMARK_TYPE_CODE = BOOKMARK_TYPE_CODE_TO_STRING.length - 1;
+  public static final Map<String, Integer> BOOKMARK_TYPE_STRING_TO_CODE;
+  static {
+    HashMap<String, Integer> t = new HashMap<String, Integer>();
+    t.put("folder",    Bookmarks.TYPE_FOLDER);
+    t.put("bookmark",  Bookmarks.TYPE_BOOKMARK);
+    t.put("separator", Bookmarks.TYPE_SEPARATOR);
+    t.put("livemark",  Bookmarks.TYPE_LIVEMARK);
+    t.put("query",     Bookmarks.TYPE_QUERY);
+    BOOKMARK_TYPE_STRING_TO_CODE = Collections.unmodifiableMap(t);
+  }
+
+  /**
+   * Convert a database bookmark type code into the Sync string equivalent.
+   *
+   * @param code one of the <code>Bookmarks.TYPE_*</code> enumerations.
+   * @return the string equivalent, or null if not found.
+   */
+  public static String typeStringForCode(int code) {
+    if (0 <= code && code <= MAX_BOOKMARK_TYPE_CODE) {
+      return BOOKMARK_TYPE_CODE_TO_STRING[code];
+    }
+    return null;
+  }
+
+  /**
+   * Convert a Sync type string into a Fennec type code.
+   *
+   * @param type a type string, such as "livemark".
+   * @return the type code, or -1 if not found.
+   */
+  public static int typeCodeForString(String type) {
+    Integer found = BOOKMARK_TYPE_STRING_TO_CODE.get(type);
+    if (found == null) {
+      return -1;
+    }
+    return found.intValue();
+  }
+
+  public static boolean isSupportedType(String type) {
+    return BOOKMARK_TYPE_STRING_TO_CODE.containsKey(type);
+  }
 }

mobile/android/base/sync/repositories/android/RepoUtils.java

@@ -93,7 +93,7 @@ public class RepoUtils {
     return cur.getLong(cur.getColumnIndex(colId));
   }
 
-  public static long getIntFromCursor(Cursor cur, String colId) {
+  public static int getIntFromCursor(Cursor cur, String colId) {
     return cur.getInt(cur.getColumnIndex(colId));
   }
 

mobile/android/base/sync/repositories/domain/BookmarkRecord.java

@@ -43,7 +43,6 @@ 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.AndroidBrowserBookmarksDataAccessor;
 import org.mozilla.gecko.sync.repositories.android.RepoUtils;
 
 import android.util.Log;
@@ -141,6 +140,61 @@ public class BookmarkRecord extends Record {
     return out;
   }
 
+  public boolean isBookmark() {
+    if (type == null) {
+      return false;
+    }
+    return type.equals("bookmark");
+  }
+
+  public boolean isFolder() {
+    if (type == null) {
+      return false;
+    }
+    return type.equals("folder");
+  }
+
+  public boolean isLivemark() {
+    if (type == null) {
+      return false;
+    }
+    return type.equals("livemark");
+  }
+
+  public boolean isSeparator() {
+    if (type == null) {
+      return false;
+    }
+    return type.equals("separator");
+  }
+
+  public boolean isMicrosummary() {
+    if (type == null) {
+      return false;
+    }
+    return type.equals("microsummary");
+  }
+
+  public boolean isQuery() {
+    if (type == null) {
+      return false;
+    }
+    return type.equals("query");
+  }
+
+  /**
+   * Return true if this record should have the Sync fields
+   * of a bookmark, microsummary, or query.
+   */
+  private boolean isBookmarkIsh() {
+    if (type == null) {
+      return false;
+    }
+    return type.equals("bookmark") ||
+           type.equals("microsummary") ||
+           type.equals("query");
+  }
+
   @Override
   protected void initFromPayload(ExtendedJSONObject payload) {
     this.type        = (String) payload.get("type");
@@ -149,19 +203,24 @@ public class BookmarkRecord extends Record {
     this.parentID    = (String) payload.get("parentid");
     this.parentName  = (String) payload.get("parentName");
 
-    // Bookmark.
-    if (isBookmark()) {
-      this.bookmarkURI = (String) payload.get("bmkUri");
+    // bookmark, microsummary, query.
+    if (isBookmarkIsh()) {
       this.keyword = (String) payload.get("keyword");
     try {
       this.tags = payload.getArray("tags");
     } catch (NonArrayJSONException e) {
-        Log.e(LOG_TAG, "Got non-array tags in bookmark record " + this.guid, e);
+      Logger.warn(LOG_TAG, "Got non-array tags in bookmark record " + this.guid, e);
       this.tags = new JSONArray();
     }
     }
 
-    // Folder.
+    // bookmark.
+    if (isBookmark()) {
+      this.bookmarkURI = (String) payload.get("bmkUri");
+      return;
+    }
+
+    // folder.
     if (isFolder()) {
       try {
         this.children = payload.getArray("children");
@@ -170,19 +229,25 @@ public class BookmarkRecord extends Record {
         // Let's see if we can recover later by using the parentid pointers.
         this.children = new JSONArray();
       }
+      return;
     }
 
-    // TODO: predecessor ID?
-    // TODO: type-specific attributes:
-    /*
-      public String generatorURI;
-      public String staticTitle;
-      public String folderName;
-      public String queryID;
-      public String siteURI;
-      public String feedURI;
-      public String pos;
-     */
+    if (isLivemark()) {
+      // TODO: siteUri, feedUri.
+      return;
+    }
+    if (isQuery()) {
+      // TODO: queryId (optional), folderName.
+      return;
+    }
+    if (isMicrosummary()) {
+      // TODO: generatorUri, staticTitle.
+      return;
+    }
+    if (isSeparator()) {
+      this.pos = payload.getString("pos");
+      return;
+    }
   }
 
   @Override
@@ -192,22 +257,19 @@ public class BookmarkRecord extends Record {
     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 (isBookmark()) {
       payload.put("bmkUri", bookmarkURI);
-      payload.put("keyword", keyword);
-      payload.put("tags", this.tags);
     } else if (isFolder()) {
       payload.put("children", this.children);
     }
-  }
 
-  public boolean isBookmark() {
-    return AndroidBrowserBookmarksDataAccessor.TYPE_BOOKMARK.equalsIgnoreCase(this.type);
-  }
-
-  public boolean isFolder() {
-    return AndroidBrowserBookmarksDataAccessor.TYPE_FOLDER.equalsIgnoreCase(this.type);
+    // TODO: fields for other types.
   }
 
   private void trace(String s) {
@@ -226,6 +288,10 @@ public class BookmarkRecord extends Record {
       return false;
     }
 
+    if (!RepoUtils.stringsEqual(this.type, other.type)) {
+      return false;
+    }
+
     // Check children.
     if (isFolder() && (this.children != other.children)) {
       trace("BookmarkRecord.equals: this folder: " + this.title + ", " + this.guid);
@@ -260,7 +326,6 @@ public class BookmarkRecord extends Record {
         && RepoUtils.stringsEqual(this.bookmarkURI, other.bookmarkURI)
         && RepoUtils.stringsEqual(this.parentID, other.parentID)
         && RepoUtils.stringsEqual(this.parentName, other.parentName)
-        && RepoUtils.stringsEqual(this.type, other.type)
         && RepoUtils.stringsEqual(this.description, other.description)
         && RepoUtils.stringsEqual(this.keyword, other.keyword)
         && jsonArrayStringsEqual(this.tags, other.tags);