Change Bound's before to inclusive

firebase-firestore/src/main/java/com/google/firebase/firestore/Query.java

@@ -705,7 +705,7 @@ public Query limitToLast(long limit) {
    */
   @NonNull
   public Query startAt(@NonNull DocumentSnapshot snapshot) {
-    Bound bound = boundFromDocumentSnapshot("startAt", snapshot, /*before=*/ true);
+    Bound bound = boundFromDocumentSnapshot("startAt", snapshot, /*inclusive=*/ true);
     return new Query(query.startAt(bound), firestore);
   }
 
@@ -719,7 +719,7 @@ public Query startAt(@NonNull DocumentSnapshot snapshot) {
    */
   @NonNull
   public Query startAt(Object... fieldValues) {
-    Bound bound = boundFromFields("startAt", fieldValues, /*before=*/ true);
+    Bound bound = boundFromFields("startAt", fieldValues, /*inclusive=*/ true);
     return new Query(query.startAt(bound), firestore);
   }
 
@@ -733,7 +733,7 @@ public Query startAt(Object... fieldValues) {
    */
   @NonNull
   public Query startAfter(@NonNull DocumentSnapshot snapshot) {
-    Bound bound = boundFromDocumentSnapshot("startAfter", snapshot, /*before=*/ false);
+    Bound bound = boundFromDocumentSnapshot("startAfter", snapshot, /*inclusive=*/ false);
     return new Query(query.startAt(bound), firestore);
   }
 
@@ -748,7 +748,7 @@ public Query startAfter(@NonNull DocumentSnapshot snapshot) {
    */
   @NonNull
   public Query startAfter(Object... fieldValues) {
-    Bound bound = boundFromFields("startAfter", fieldValues, /*before=*/ false);
+    Bound bound = boundFromFields("startAfter", fieldValues, /*inclusive=*/ false);
     return new Query(query.startAt(bound), firestore);
   }
 
@@ -762,7 +762,7 @@ public Query startAfter(Object... fieldValues) {
    */
   @NonNull
   public Query endBefore(@NonNull DocumentSnapshot snapshot) {
-    Bound bound = boundFromDocumentSnapshot("endBefore", snapshot, /*before=*/ true);
+    Bound bound = boundFromDocumentSnapshot("endBefore", snapshot, /*inclusive=*/ false);
     return new Query(query.endAt(bound), firestore);
   }
 
@@ -776,7 +776,7 @@ public Query endBefore(@NonNull DocumentSnapshot snapshot) {
    */
   @NonNull
   public Query endBefore(Object... fieldValues) {
-    Bound bound = boundFromFields("endBefore", fieldValues, /*before=*/ true);
+    Bound bound = boundFromFields("endBefore", fieldValues, /*inclusive=*/ false);
     return new Query(query.endAt(bound), firestore);
   }
 
@@ -790,7 +790,7 @@ public Query endBefore(Object... fieldValues) {
    */
   @NonNull
   public Query endAt(@NonNull DocumentSnapshot snapshot) {
-    Bound bound = boundFromDocumentSnapshot("endAt", snapshot, /*before=*/ false);
+    Bound bound = boundFromDocumentSnapshot("endAt", snapshot, /*inclusive=*/ true);
     return new Query(query.endAt(bound), firestore);
   }
 
@@ -804,7 +804,7 @@ public Query endAt(@NonNull DocumentSnapshot snapshot) {
    */
   @NonNull
   public Query endAt(Object... fieldValues) {
-    Bound bound = boundFromFields("endAt", fieldValues, /*before=*/ false);
+    Bound bound = boundFromFields("endAt", fieldValues, /*inclusive=*/ true);
     return new Query(query.endAt(bound), firestore);
   }
 
@@ -818,7 +818,7 @@ public Query endAt(Object... fieldValues) {
    * any of the fields in the order by are an uncommitted server timestamp.
    */
   private Bound boundFromDocumentSnapshot(
-      String methodName, DocumentSnapshot snapshot, boolean before) {
+      String methodName, DocumentSnapshot snapshot, boolean inclusive) {
     checkNotNull(snapshot, "Provided snapshot must not be null.");
     if (!snapshot.exists()) {
       throw new IllegalArgumentException(
@@ -857,11 +857,11 @@ private Bound boundFromDocumentSnapshot(
         }
       }
     }
-    return new Bound(components, before);
+    return new Bound(components, inclusive);
   }
 
   /** Converts a list of field values to Bound. */
-  private Bound boundFromFields(String methodName, Object[] values, boolean before) {
+  private Bound boundFromFields(String methodName, Object[] values, boolean inclusive) {
     // Use explicit order by's because it has to match the query the user made
     List<OrderBy> explicitOrderBy = query.getExplicitOrderBy();
     if (values.length > explicitOrderBy.size()) {
@@ -913,7 +913,7 @@ private Bound boundFromFields(String methodName, Object[] values, boolean before
       }
     }
 
-    return new Bound(components, before);
+    return new Bound(components, inclusive);
   }
 
   /**

firebase-firestore/src/main/java/com/google/firebase/firestore/bundle/BundleSerializer.java

@@ -146,8 +146,8 @@ private BundledQuery decodeBundledQuery(JSONObject bundledQuery) throws JSONExce
 
     List<Filter> filters = decodeWhere(structuredQuery.optJSONObject("where"));
     List<OrderBy> orderBys = decodeOrderBy(structuredQuery.optJSONArray("orderBy"));
-    @Nullable Bound startAt = decodeBound(structuredQuery.optJSONObject("startAt"));
-    @Nullable Bound endAt = decodeBound(structuredQuery.optJSONObject("endAt"));
+    @Nullable Bound startAt = decodeStartAtBound(structuredQuery.optJSONObject("startAt"));
+    @Nullable Bound endAt = decodeEndAtBound(structuredQuery.optJSONObject("endAt"));
 
     verifyNoOffset(structuredQuery);
     int limit = decodeLimit(structuredQuery);
@@ -167,24 +167,35 @@ private int decodeLimit(JSONObject structuredQuery) {
     }
   }
 
-  private Bound decodeBound(@Nullable JSONObject bound) throws JSONException {
+  private Bound decodeStartAtBound(@Nullable JSONObject bound) throws JSONException {
     if (bound != null) {
-      List<Value> cursor = new ArrayList<>();
       boolean before = bound.optBoolean("before", false);
-
-      JSONArray values = bound.optJSONArray("values");
-      if (values != null) {
-        for (int i = 0; i < values.length(); ++i) {
-          cursor.add(decodeValue(values.getJSONObject(i)));
-        }
-      }
-
-      return new Bound(cursor, before);
+      List<Value> position = decodePosition(bound);
+      return new Bound(position, before);
     }
+    return null;
+  }
 
+  private Bound decodeEndAtBound(@Nullable JSONObject bound) throws JSONException {
+    if (bound != null) {
+      boolean before = bound.optBoolean("before", false);
+      List<Value> position = decodePosition(bound);
+      return new Bound(position, !before);
+    }
     return null;
   }
 
+  private List<Value> decodePosition(JSONObject bound) throws JSONException {
+    List<Value> cursor = new ArrayList<>();
+    JSONArray values = bound.optJSONArray("values");
+    if (values != null) {
+      for (int i = 0; i < values.length(); ++i) {
+        cursor.add(decodeValue(values.getJSONObject(i)));
+      }
+    }
+    return cursor;
+  }
+
   private List<OrderBy> decodeOrderBy(@Nullable JSONArray orderBys) throws JSONException {
     List<OrderBy> result = new ArrayList<>();
 

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

@@ -38,33 +38,36 @@
  */
 public final class Bound {
 
-  /** Whether this bound is just before or just after the provided position */
-  private final boolean before;
+  /**
+   * Whether this bound includes the provided position (e.g. for {#code startAt()} or {#code
+   * endAt()})
+   */
+  private final boolean inclusive;
 
   /** The index position of this bound */
   private final List<Value> position;
 
-  public Bound(List<Value> position, boolean before) {
+  public Bound(List<Value> position, boolean inclusive) {
     this.position = position;
-    this.before = before;
+    this.inclusive = inclusive;
   }
 
   public List<Value> getPosition() {
     return position;
   }
 
-  public boolean isBefore() {
-    return before;
+  /**
+   * Whether the bound includes the documents that lie directly on the bound. Returns {@code true}
+   * for {@code startAt()} and {@code endAt()} and false for {@code startAfter()} and {@code
+   * endBefore()}.
+   */
+  public boolean isInclusive() {
+    return inclusive;
   }
 
-  public String canonicalString() {
+  public String positionString() {
     // TODO: Make this collision robust.
     StringBuilder builder = new StringBuilder();
-    if (before) {
-      builder.append("b:");
-    } else {
-      builder.append("a:");
-    }
     boolean first = true;
     for (Value indexComponent : position) {
       if (!first) {
@@ -78,6 +81,17 @@ public String canonicalString() {
 
   /** Returns true if a document sorts before a bound using the provided sort order. */
   public boolean sortsBeforeDocument(List<OrderBy> orderBy, Document document) {
+    int comparison = compareToDocument(orderBy, document);
+    return inclusive ? comparison <= 0 : comparison < 0;
+  }
+
+  /** Returns true if a document sorts after a bound using the provided sort order. */
+  public boolean sortsAfterDocument(List<OrderBy> orderBy, Document document) {
+    int comparison = compareToDocument(orderBy, document);
+    return inclusive ? comparison >= 0 : comparison > 0;
+  }
+
+  private int compareToDocument(List<OrderBy> orderBy, Document document) {
     hardAssert(position.size() <= orderBy.size(), "Bound has more components than query's orderBy");
     int comparison = 0;
     for (int i = 0; i < position.size(); i++) {
@@ -105,8 +119,7 @@ public boolean sortsBeforeDocument(List<OrderBy> orderBy, Document document) {
         break;
       }
     }
-
-    return before ? comparison <= 0 : comparison < 0;
+    return comparison;
   }
 
   @Override
@@ -120,21 +133,21 @@ public boolean equals(Object o) {
 
     Bound bound = (Bound) o;
 
-    return before == bound.before && position.equals(bound.position);
+    return inclusive == bound.inclusive && position.equals(bound.position);
   }
 
   @Override
   public int hashCode() {
-    int result = (before ? 1 : 0);
+    int result = (inclusive ? 1 : 0);
     result = 31 * result + position.hashCode();
     return result;
   }
 
   @Override
   public String toString() {
     StringBuilder builder = new StringBuilder();
-    builder.append("Bound(before=");
-    builder.append(before);
+    builder.append("Bound(inclusive=");
+    builder.append(inclusive);
     builder.append(", position=");
     for (int i = 0; i < position.size(); i++) {
       if (i > 0) {

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

@@ -448,7 +448,7 @@ private boolean matchesBounds(Document doc) {
     if (startAt != null && !startAt.sortsBeforeDocument(getOrderBy(), doc)) {
       return false;
     }
-    if (endAt != null && endAt.sortsBeforeDocument(getOrderBy(), doc)) {
+    if (endAt != null && !endAt.sortsAfterDocument(getOrderBy(), doc)) {
       return false;
     }
     return true;
@@ -520,10 +520,12 @@ public Target toTarget() {
 
         // We need to swap the cursors to match the now-flipped query ordering.
         Bound newStartAt =
-            this.endAt != null ? new Bound(this.endAt.getPosition(), !this.endAt.isBefore()) : null;
+            this.endAt != null
+                ? new Bound(this.endAt.getPosition(), !this.endAt.isInclusive())
+                : null;
         Bound newEndAt =
             this.startAt != null
-                ? new Bound(this.startAt.getPosition(), !this.startAt.isBefore())
+                ? new Bound(this.startAt.getPosition(), !this.startAt.isInclusive())
                 : null;
 
         this.memoizedTarget =

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

@@ -180,7 +180,7 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
             Value cursorValue = startAt.getPosition().get(i);
             if (max(segmentValue, cursorValue) == cursorValue) {
               segmentValue = cursorValue;
-              segmentInclusive = startAt.isBefore();
+              segmentInclusive = startAt.isInclusive();
             }
             break;
           }
@@ -256,7 +256,7 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
             Value cursorValue = endAt.getPosition().get(i);
             if (min(segmentValue, cursorValue) == cursorValue) {
               segmentValue = cursorValue;
-              segmentInclusive = !endAt.isBefore();
+              segmentInclusive = endAt.isInclusive();
             }
             break;
           }
@@ -276,7 +276,7 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
       return null;
     }
 
-    return new Bound(values, !inclusive);
+    return new Bound(values, inclusive);
   }
 
   public List<OrderBy> getOrderBy() {
@@ -318,12 +318,14 @@ public String getCanonicalId() {
 
     if (startAt != null) {
       builder.append("|lb:");
-      builder.append(startAt.canonicalString());
+      builder.append(startAt.isInclusive() ? "b:" : "a:");
+      builder.append(startAt.positionString());
     }
 
     if (endAt != null) {
       builder.append("|ub:");
-      builder.append(endAt.canonicalString());
+      builder.append(endAt.isInclusive() ? "a:" : "b:");
+      builder.append(endAt.positionString());
     }
 
     memoizedCannonicalId = builder.toString();

firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteIndexManager.java

@@ -96,6 +96,8 @@ public void addFieldIndex(FieldIndex index) {
         db.query("SELECT MAX(index_id) FROM index_configuration")
             .firstValue(input -> input.isNull(0) ? 0 : input.getInt(0));
 
+    // TODO(indexing): Properly dedupe indices to avoid duplicate index entries (by comparing
+    // collection_group+index_proto)
     db.execute(
         "INSERT OR IGNORE INTO index_configuration ("
             + "index_id, "
@@ -176,7 +178,7 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
     if (fieldIndex == null) return null;
 
     Bound lowerBound = target.getLowerBound(fieldIndex);
-    String lowerBoundOp = lowerBound.isBefore() ? ">=" : ">"; // `startAt()` versus `startAfter()`
+    String lowerBoundOp = lowerBound.isInclusive() ? ">=" : ">";
 
     @Nullable Bound upperBound = target.getUpperBound(fieldIndex);
 
@@ -202,7 +204,7 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
           lowerBoundValues.size() == upperBoundValues.size(),
           "Expected upper and lower bound size to match");
 
-      String upperBoundOp = upperBound.isBefore() ? "<" : "<="; // `endBefore()` versus `endAt()`
+      String upperBoundOp = upperBound.isInclusive() ? "<=" : "<";
 
       // TODO(indexing): To avoid reading the same documents multiple times, we should ideally only
       // send one query that combines all clauses.