Address feedback2

Author: cherylEnkidu

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

@@ -35,46 +35,28 @@ private AggregateField(@Nullable FieldPath fieldPath, @NonNull String operator)
 
     // Use $operator_$field format if it's an aggregation of a specific field. For example: sum_foo.
     // Use $operator format if there's no field. For example: count.
-    //
-    // Note: If the fieldPath contains characters that need to be escaped, we need to do further
-    // processing. Example:
-    // field:                  contains`invalid
-    // field.getEncodedPath(): `contains\`invalid`
-    // alias:                  `sum_contains\`invalid`
-    if (fieldPath == null) {
-      this.alias = operator;
-    } else {
-      String encodedFieldPath = fieldPath.toString();
-      boolean hasEscapedChars =
-          encodedFieldPath.startsWith("`")
-              && encodedFieldPath.endsWith("`")
-              && encodedFieldPath.length() > 1;
-      String maybeBacktick = hasEscapedChars ? "`" : "";
-      // Strip away the leading and trailing backticks.
-      encodedFieldPath =
-          hasEscapedChars
-              ? encodedFieldPath.substring(1, encodedFieldPath.length() - 1)
-              : encodedFieldPath;
-      this.alias = maybeBacktick + operator + "_" + encodedFieldPath + maybeBacktick;
-    }
+    this.alias = operator + (fieldPath == null ? "" : "_" + fieldPath);
   }
 
   /**
    * Returns the field on which the aggregation takes place. Returns an empty string if there's no
    * field (e.g. for count).
    */
+  @RestrictTo(RestrictTo.Scope.LIBRARY)
   @NonNull
   public String getFieldPath() {
     return fieldPath == null ? "" : fieldPath.toString();
   }
 
   /** Returns the alias used internally for this aggregate field. */
+  @RestrictTo(RestrictTo.Scope.LIBRARY)
   @NonNull
   public String getAlias() {
     return alias;
   }
 
   /** Returns a string representation of this aggregation's operator. For example: "sum" */
+  @RestrictTo(RestrictTo.Scope.LIBRARY)
   @NonNull
   public String getOperator() {
     return operator;

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

@@ -67,25 +67,7 @@ public long getCount() {
   @RestrictTo(RestrictTo.Scope.LIBRARY)
   @Nullable
   public Object get(@Nonnull AggregateField aggregateField) {
-    if (!data.containsKey(aggregateField.getAlias())) {
-      throw new IllegalArgumentException(
-          "'"
-              + aggregateField.getOperator()
-              + "("
-              + aggregateField.getFieldPath()
-              + ")"
-              + "' was not requested in the aggregation query.");
-    }
-    Value value = data.get(aggregateField.getAlias());
-    if (value.hasNullValue()) {
-      return null;
-    } else if (value.hasDoubleValue()) {
-      return value.getDoubleValue();
-    } else if (value.hasIntegerValue()) {
-      return value.getIntegerValue();
-    } else {
-      throw new IllegalStateException("Found aggregation result that is not an integer nor double");
-    }
+    return getInternal(aggregateField);
   }
 
   /**
@@ -98,18 +80,12 @@ public Object get(@Nonnull AggregateField aggregateField) {
   /** @hide */
   @RestrictTo(RestrictTo.Scope.LIBRARY)
   public long get(@Nonnull AggregateField.CountAggregateField countAggregateField) {
-    Object value = get((AggregateField) countAggregateField);
+    Long value = getLong(countAggregateField);
     if (value == null) {
       throw new IllegalArgumentException(
           "RunAggregationQueryResponse alias " + countAggregateField.getAlias() + " is null");
-    } else if (!(value instanceof Long)) {
-      throw new IllegalArgumentException(
-          "RunAggregationQueryResponse alias "
-              + countAggregateField.getAlias()
-              + " has incorrect type: "
-              + value.getClass().getName());
     }
-    return (long) value;
+    return value;
   }
 
   /**
@@ -126,7 +102,7 @@ public long get(@Nonnull AggregateField.CountAggregateField countAggregateField)
   @RestrictTo(RestrictTo.Scope.LIBRARY)
   @Nullable
   public Double get(@Nonnull AggregateField.AverageAggregateField averageAggregateField) {
-    return (Double) get((AggregateField) averageAggregateField);
+    return getDouble(averageAggregateField);
   }
 
   /**
@@ -143,8 +119,8 @@ public Double get(@Nonnull AggregateField.AverageAggregateField averageAggregate
   @RestrictTo(RestrictTo.Scope.LIBRARY)
   @Nullable
   public Double getDouble(@Nonnull AggregateField aggregateField) {
-    Number result = (Number) get(aggregateField);
-    return result == null ? null : result.doubleValue();
+    Number val = getTypedValue(aggregateField, Number.class);
+    return val != null ? val.doubleValue() : null;
   }
 
   /**
@@ -160,8 +136,44 @@ public Double getDouble(@Nonnull AggregateField aggregateField) {
   @RestrictTo(RestrictTo.Scope.LIBRARY)
   @Nullable
   public Long getLong(@Nonnull AggregateField aggregateField) {
-    Number result = (Number) get(aggregateField);
-    return result == null ? null : result.longValue();
+    Number val = getTypedValue(aggregateField, Number.class);
+    return val != null ? val.longValue() : null;
+  }
+
+  @Nullable
+  private Object getInternal(@Nonnull AggregateField aggregateField) {
+    if (!data.containsKey(aggregateField.getAlias())) {
+      throw new IllegalArgumentException(
+          "'"
+              + aggregateField.getOperator()
+              + "("
+              + aggregateField.getFieldPath()
+              + ")"
+              + "' was not requested in the aggregation query.");
+    }
+    Value value = data.get(aggregateField.getAlias());
+    UserDataWriter userDataWriter =
+        new UserDataWriter(
+            query.getQuery().firestore, DocumentSnapshot.ServerTimestampBehavior.DEFAULT);
+    return userDataWriter.convertValue(value);
+  }
+
+  @Nullable
+  private <T> T getTypedValue(@Nonnull AggregateField aggregateField, Class<T> clazz) {
+    Object value = getInternal(aggregateField);
+    return castTypedValue(value, aggregateField, clazz);
+  }
+
+  @Nullable
+  private <T> T castTypedValue(
+      Object value, @Nonnull AggregateField aggregateField, Class<T> clazz) {
+    if (value == null) {
+      return null;
+    } else if (!clazz.isInstance(value)) {
+      throw new RuntimeException(
+          "AggregateField '" + aggregateField.getAlias() + "' is not a " + clazz.getName());
+    }
+    return clazz.cast(value);
   }
 
   /**

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

@@ -31,7 +31,6 @@
 import static com.google.firebase.firestore.util.Assert.fail;
 
 import androidx.annotation.RestrictTo;
-import androidx.annotation.VisibleForTesting;
 import com.google.firebase.Timestamp;
 import com.google.firebase.firestore.model.DatabaseId;
 import com.google.firebase.firestore.model.DocumentKey;
@@ -53,15 +52,13 @@ public class UserDataWriter {
   private final FirebaseFirestore firestore;
   private final DocumentSnapshot.ServerTimestampBehavior serverTimestampBehavior;
 
-  @VisibleForTesting
   public UserDataWriter(
       FirebaseFirestore firestore,
       DocumentSnapshot.ServerTimestampBehavior serverTimestampBehavior) {
     this.firestore = firestore;
     this.serverTimestampBehavior = serverTimestampBehavior;
   }
 
-  @VisibleForTesting
   public Object convertValue(Value value) {
     switch (typeOrder(value)) {
       case TYPE_ORDER_MAP: