Address feedback

schmidt-sebastian · schmidt-sebastian · commit b7cee6079af5 · 2018-05-29T14:19:41.000-07:00
diff --git a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/CustomClassMapper.java b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/CustomClassMapper.java
@@ -47,7 +47,7 @@
 import java.util.logging.Logger;
 
 /** Helper class to convert to/from custom POJO classes and plain Java types. */
-public class CustomClassMapper {
+class CustomClassMapper {
   private static final Logger LOGGER = Logger.getLogger(CustomClassMapper.class.getName());
 
   /** Maximum depth before we give up and assume it's a recursive object graph. */
diff --git a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentSnapshot.java b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentSnapshot.java
@@ -49,9 +49,9 @@ public class DocumentSnapshot {
   private final FirestoreImpl firestore;
   private final DocumentReference docRef;
   @Nullable private final Map<String, Value> fields;
-  @Nullable private Timestamp readTime;
-  @Nullable private Timestamp updateTime;
-  @Nullable private Timestamp createTime;
+  @Nullable private final Timestamp readTime;
+  @Nullable private final Timestamp updateTime;
+  @Nullable private final Timestamp createTime;
 
   DocumentSnapshot(
       FirestoreImpl firestore,
diff --git a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Precondition.java b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Precondition.java
@@ -26,8 +26,8 @@ public final class Precondition {
   /** An empty Precondition that adds no enforcements */
   public static final Precondition NONE = new Precondition(null, null);
 
-  private Boolean exists;
-  private Timestamp updateTime;
+  private final Boolean exists;
+  private final Timestamp updateTime;
 
   private Precondition(Boolean exists, Timestamp updateTime) {
     this.exists = exists;
diff --git a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/LocalFirestoreHelper.java b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/LocalFirestoreHelper.java
@@ -699,15 +699,15 @@ public boolean equals(Object o) {
                     .setTimestampValue(
                         com.google.protobuf.Timestamp.newBuilder()
                             .setSeconds(479978400)
-                            .setNanos(123000000))
+                            .setNanos(123000000))  // Dates only support millisecond precision.
                     .build())
             .put(
                 "timestampValue",
                 Value.newBuilder()
                     .setTimestampValue(
                         com.google.protobuf.Timestamp.newBuilder()
                             .setSeconds(479978400)
-                            .setNanos(123000))
+                            .setNanos(123000))  // Timestamps supports microsecond precision.
                     .build())
             .put(
                 "arrayValue",
diff --git a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java
@@ -200,6 +200,19 @@ public void serverTimestamp() throws Exception {
     assertTrue(documentSnapshot.getDate("time").getTime() > 0);
   }
 
+  @Test
+  public void updateMicrosecondTimestamp() throws Exception {
+    DocumentReference documentReference = addDocument("time", Timestamp.ofTimeSecondsAndNanos(0, 123000));
+    DocumentSnapshot documentSnapshot = documentReference.get().get();
+
+    Timestamp timestamp = documentSnapshot.getTimestamp("time");
+    documentReference.update("time", timestamp);
+
+    documentSnapshot = documentReference.get().get();
+    timestamp = documentSnapshot.getTimestamp("time");
+    assertEquals(123000, timestamp.getNanos());
+  }
+
   @Test
   public void fieldDelete() throws Exception {
     Map<String, Object> fields = new HashMap<>();
@@ -245,25 +258,6 @@ public void defaultQuery() throws Exception {
     assertEquals("bar", documents.next().get("foo"));
   }
 
-  @Test
-  public void queryForServerTimestamps() throws Exception {
-    DocumentReference documentReference = addDocument("serverTime", FieldValue.serverTimestamp());
-    DocumentSnapshot documentSnapshot = documentReference.get().get();
-    Timestamp serverTime = documentSnapshot.getTimestamp("serverTime");
-
-    if (TimeUnit.NANOSECONDS.toMicros(serverTime.getNanos()) > 0) {
-      // If serverTime has a microsecond component, this query produces no results as `getDate()`
-      // truncates to milliseconds.
-      Query query = randomColl.whereEqualTo("foo", documentSnapshot.getDate("serverTime"));
-      QuerySnapshot querySnapshot = query.get().get();
-      assertEquals(0, querySnapshot.size());
-    }
-
-    Query query = randomColl.whereEqualTo("serverTime", serverTime);
-    QuerySnapshot querySnapshot = query.get().get();
-    assertEquals(1, querySnapshot.size());
-  }
-
   @Test
   public void noResults() throws Exception {
     QuerySnapshot querySnapshot = randomColl.get().get();
