SyncEngine.java: Replace contents with the fix in #2404

Author: dconeybe

firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java

@@ -21,6 +21,8 @@
 import androidx.annotation.VisibleForTesting;
 import com.google.android.gms.tasks.Task;
 import com.google.android.gms.tasks.TaskCompletionSource;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.firebase.database.collection.ImmutableSortedMap;
 import com.google.firebase.database.collection.ImmutableSortedSet;
 import com.google.firebase.firestore.FirebaseFirestoreException;
@@ -55,13 +57,12 @@
 import com.google.firebase.firestore.util.Util;
 import io.grpc.Status;
 import java.io.IOException;
-import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Queue;
 import java.util.Set;
 
 /**
@@ -130,7 +131,7 @@ interface SyncEngineCallback {
    * The keys of documents that are in limbo for which we haven't yet started a limbo resolution
    * query.
    */
-  private final Queue<DocumentKey> enqueuedLimboResolutions;
+  private final LinkedHashSet<DocumentKey> enqueuedLimboResolutions;
 
   /** Keeps track of the target ID for each document that is in limbo with an active target. */
   private final Map<DocumentKey, Integer> activeLimboTargetsByKey;
@@ -169,7 +170,7 @@ public SyncEngine(
     queryViewsByQuery = new HashMap<>();
     queriesByTarget = new HashMap<>();
 
-    enqueuedLimboResolutions = new ArrayDeque<>();
+    enqueuedLimboResolutions = new LinkedHashSet<>();
     activeLimboTargetsByKey = new HashMap<>();
     activeLimboResolutionsByTarget = new HashMap<>();
     limboDocumentRefs = new ReferenceSet();
@@ -321,8 +322,6 @@ public void handleRemoteEvent(RemoteEvent event) {
       TargetChange targetChange = entry.getValue();
       LimboResolution limboResolution = activeLimboResolutionsByTarget.get(targetId);
       if (limboResolution != null) {
-        Logger.warn("zzyzx", "handleRemoteEvent() limboResolution.key=" + limboResolution.key + " targetChange.getAddedDocuments().size()=" + targetChange.getAddedDocuments().size() + " targetChange.getModifiedDocuments().size()=" + targetChange.getModifiedDocuments().size() + " targetChange.getRemovedDocuments().size()=" + targetChange.getRemovedDocuments().size());
-
         // Since this is a limbo resolution lookup, it's for a single document and it could be
         // added, modified, or removed, but not a combination.
         hardAssert(
@@ -398,7 +397,6 @@ public void handleRejectedListen(int targetId, Status error) {
     LimboResolution limboResolution = activeLimboResolutionsByTarget.get(targetId);
     DocumentKey limboKey = limboResolution != null ? limboResolution.key : null;
     if (limboKey != null) {
-      Logger.warn("zzyzx", "handleRejectedListen() limboKey=" + limboKey + " targetId=" + targetId);
       // Since this query failed, we won't want to manually unlisten to it.
       // So go ahead and remove it from bookkeeping.
       activeLimboTargetsByKey.remove(limboKey);
@@ -605,26 +603,11 @@ private void removeAndCleanupTarget(int targetId, Status status) {
     }
   }
 
-  private String describeKeyInActiveLimboTargetsByKey(DocumentKey key) {
-    ArrayList<String> list = new ArrayList<>();
-    for (DocumentKey currentKey : activeLimboTargetsByKey.keySet()) {
-      list.add(currentKey.toString());
-    }
-    Collections.sort(list);
-    return "activeLimboTargetsByKey.containsKey(key)="
-      + activeLimboTargetsByKey.containsKey(key)
-      + " (by string: "
-      + list.contains(key.toString())
-      + ") activeLimboTargetsByKey="
-      + list;
-  }
-
   private void removeLimboTarget(DocumentKey key) {
+    enqueuedLimboResolutions.remove(key);
     // It's possible that the target already got removed because the query failed. In that case,
     // the key won't exist in `limboTargetsByKey`. Only do the cleanup if we still have the target.
     Integer targetId = activeLimboTargetsByKey.get(key);
-    Logger.warn("zzyzx", "removeLimboTarget() start; key=" + key + " targetId=" + targetId + " " + describeKeyInActiveLimboTargetsByKey(key));
-    enqueuedLimboResolutions.remove(key);
     if (targetId != null) {
       remoteStore.stopListening(targetId);
       activeLimboTargetsByKey.remove(key);
@@ -675,12 +658,10 @@ private void updateTrackedLimboDocuments(List<LimboDocumentChange> limboChanges,
     for (LimboDocumentChange limboChange : limboChanges) {
       switch (limboChange.getType()) {
         case ADDED:
-          Logger.warn("zzyzx", "updateTrackedLimboDocuments() ADDED " + limboChange.getKey());
           limboDocumentRefs.addReference(limboChange.getKey(), targetId);
           trackLimboChange(limboChange);
           break;
         case REMOVED:
-          Logger.warn("zzyzx", "updateTrackedLimboDocuments() REMOVED " + limboChange.getKey());
           Logger.debug(TAG, "Document no longer in limbo: %s", limboChange.getKey());
           DocumentKey limboDocKey = limboChange.getKey();
           limboDocumentRefs.removeReference(limboDocKey, targetId);
@@ -697,8 +678,7 @@ private void updateTrackedLimboDocuments(List<LimboDocumentChange> limboChanges,
 
   private void trackLimboChange(LimboDocumentChange change) {
     DocumentKey key = change.getKey();
-    Logger.warn("zzyzx", "trackLimboChange() for " + key + ": activeLimboTargetsByKey.get(key)==" + activeLimboTargetsByKey.get(key));
-    if (!activeLimboTargetsByKey.containsKey(key)) {
+    if (!activeLimboTargetsByKey.containsKey(key) && !enqueuedLimboResolutions.contains(key)) {
       Logger.debug(TAG, "New document in limbo: %s", key);
       enqueuedLimboResolutions.add(key);
       pumpEnqueuedLimboResolutions();
@@ -714,12 +694,11 @@ private void trackLimboChange(LimboDocumentChange change) {
    * https://github.com/firebase/firebase-js-sdk/issues/2683.
    */
   private void pumpEnqueuedLimboResolutions() {
-    Logger.warn("zzyzx", "pumpEnqueuedLimboResolutions() start; enqueuedLimboResolutions.size()=" + enqueuedLimboResolutions.size() + " activeLimboTargetsByKey.size()=" + activeLimboTargetsByKey.size());
     while (!enqueuedLimboResolutions.isEmpty()
         && activeLimboTargetsByKey.size() < maxConcurrentLimboResolutions) {
-      DocumentKey key = enqueuedLimboResolutions.remove();
+      DocumentKey key = enqueuedLimboResolutions.iterator().next();
+      enqueuedLimboResolutions.remove(key);
       int limboTargetId = targetIdGenerator.nextId();
-      Logger.warn("zzyzx", "pumpEnqueuedLimboResolutions() starting listen for " + key + " (limboTargetId=" + limboTargetId + ")");
       activeLimboResolutionsByTarget.put(limboTargetId, new LimboResolution(key));
       activeLimboTargetsByKey.put(key, limboTargetId);
       remoteStore.listen(
@@ -729,19 +708,18 @@ private void pumpEnqueuedLimboResolutions() {
               ListenSequence.INVALID,
               QueryPurpose.LIMBO_RESOLUTION));
     }
-    Logger.warn("zzyzx", "pumpEnqueuedLimboResolutions() done; enqueuedLimboResolutions.size()=" + enqueuedLimboResolutions.size() + " activeLimboTargetsByKey.size()=" + activeLimboTargetsByKey.size());
   }
 
   @VisibleForTesting
-  public Map<DocumentKey, Integer> getActiveLimboDocumentResolutions() {
+  public ImmutableMap<DocumentKey, Integer> getActiveLimboDocumentResolutions() {
     // Make a defensive copy as the Map continues to be modified.
-    return new HashMap<>(activeLimboTargetsByKey);
+    return ImmutableMap.copyOf(activeLimboTargetsByKey);
   }
 
   @VisibleForTesting
-  public Queue<DocumentKey> getEnqueuedLimboDocumentResolutions() {
-    // Make a defensive copy as the Queue continues to be modified.
-    return new ArrayDeque<>(enqueuedLimboResolutions);
+  public ImmutableSet<DocumentKey> getEnqueuedLimboDocumentResolutions() {
+    // Make a defensive copy as the LinkedHashMap continues to be modified.
+    return ImmutableSet.copyOf(enqueuedLimboResolutions);
   }
 
   public void handleCredentialChange(User user) {