From b2ae7809f1110286181e8c8e5264bd4fff29fd4f Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 29 May 2018 09:30:28 -0700 Subject: [PATCH 1/4] Return Dates as com.google.cloud.Timestamps --- .../cloud/firestore/CustomClassMapper.java | 378 ++++++++++++------ .../cloud/firestore/DocumentSnapshot.java | 74 ++-- .../google/cloud/firestore/FirestoreImpl.java | 45 ++- .../cloud/firestore/FirestoreOptions.java | 52 ++- .../google/cloud/firestore/Precondition.java | 14 +- .../com/google/cloud/firestore/Query.java | 17 +- .../firestore/QueryDocumentSnapshot.java | 17 +- .../google/cloud/firestore/QuerySnapshot.java | 12 +- .../cloud/firestore/UserDataConverter.java | 8 +- .../com/google/cloud/firestore/Watch.java | 13 +- .../google/cloud/firestore/WriteResult.java | 18 +- .../firestore/annotation/ServerTimestamp.java | 6 +- .../firestore/CollectionReferenceTest.java | 5 +- .../cloud/firestore/ConformanceTest.java | 23 +- .../firestore/DocumentReferenceTest.java | 55 ++- .../google/cloud/firestore/FirestoreTest.java | 5 +- .../cloud/firestore/LocalFirestoreHelper.java | 45 ++- .../com/google/cloud/firestore/QueryTest.java | 9 +- .../cloud/firestore/TransactionTest.java | 11 +- .../com/google/cloud/firestore/WatchTest.java | 6 +- .../cloud/firestore/WriteBatchTest.java | 24 +- .../cloud/firestore/it/ITSystemTest.java | 39 +- 22 files changed, 600 insertions(+), 276 deletions(-) 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 index a896929f39b1..d2e61644511c 100644 --- 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 @@ -16,6 +16,7 @@ package com.google.cloud.firestore; +import com.google.cloud.Timestamp; import com.google.cloud.firestore.annotation.Exclude; import com.google.cloud.firestore.annotation.IgnoreExtraProperties; import com.google.cloud.firestore.annotation.PropertyName; @@ -43,9 +44,14 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.logging.Logger; /** Helper class to convert to/from custom POJO classes and plain Java types. */ -class CustomClassMapper { +public 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. */ + private static final int MAX_DEPTH = 500; private static final ConcurrentMap, BeanMapper> mappers = new ConcurrentHashMap<>(); @@ -87,20 +93,31 @@ public static Map convertToPlainJavaTypes(Map update) * @return The POJO object. */ public static T convertToCustomClass(Object object, Class clazz) { - return deserializeToClass(object, clazz); + return deserializeToClass(object, clazz, ErrorPath.EMPTY); + } + + protected static Object serialize(T o) { + return serialize(o, ErrorPath.EMPTY); } @SuppressWarnings("unchecked") - static Object serialize(T o) { + private static Object serialize(T o, ErrorPath path) { + if (path.getLength() > MAX_DEPTH) { + throw serializeError( + path, + "Exceeded maximum depth of " + + MAX_DEPTH + + ", which likely indicates there's an object cycle"); + } if (o == null) { return null; } else if (o instanceof Number) { if (o instanceof Float) { return ((Float) o).doubleValue(); } else if (o instanceof Short) { - throw new RuntimeException("Shorts are not supported, please use int or long"); + throw serializeError(path, "Shorts are not supported, please use int or long"); } else if (o instanceof Byte) { - throw new RuntimeException("Bytes are not supported, please use int or long"); + throw serializeError(path, "Bytes are not supported, please use int or long"); } else { // Long, Integer, Double return o; @@ -110,16 +127,16 @@ static Object serialize(T o) { } else if (o instanceof Boolean) { return o; } else if (o instanceof Character) { - throw new RuntimeException("Characters are not supported, please use Strings"); + throw serializeError(path, "Characters are not supported, please use Strings."); } else if (o instanceof Map) { Map result = new HashMap<>(); for (Map.Entry entry : ((Map) o).entrySet()) { Object key = entry.getKey(); if (key instanceof String) { String keyString = (String) key; - result.put(keyString, serialize(entry.getValue())); + result.put(keyString, serialize(entry.getValue(), path.child(keyString))); } else { - throw new RuntimeException("Maps with non-string keys are not supported"); + throw serializeError(path, "Maps with non-string keys are not supported"); } } return result; @@ -127,19 +144,20 @@ static Object serialize(T o) { if (o instanceof List) { List list = (List) o; List result = new ArrayList<>(list.size()); - for (Object object : list) { - result.add(serialize(object)); + for (int i = 0; i < list.size(); i++) { + result.add(serialize(list.get(i), path.child("[" + i + "]"))); } return result; } else { - throw new RuntimeException( - "Serializing Collections is not supported, please use Lists instead"); + throw serializeError( + path, "Serializing Collections is not supported, please use Lists instead"); } } else if (o.getClass().isArray()) { - throw new RuntimeException("Serializing Arrays is not supported, please use Lists instead"); + throw serializeError(path, "Serializing Arrays is not supported, please use Lists instead"); } else if (o instanceof Enum) { return ((Enum) o).name(); } else if (o instanceof Date + || o instanceof Timestamp || o instanceof GeoPoint || o instanceof Blob || o instanceof DocumentReference) { @@ -147,65 +165,86 @@ static Object serialize(T o) { } else { Class clazz = (Class) o.getClass(); BeanMapper mapper = loadOrCreateBeanMapperForClass(clazz); - return mapper.serialize(o); + return mapper.serialize(o, path); } } @SuppressWarnings({"unchecked", "TypeParameterUnusedInFormals"}) - private static T deserializeToType(Object o, Type type) { + private static T deserializeToType(Object o, Type type, ErrorPath path) { if (o == null) { return null; } else if (type instanceof ParameterizedType) { - return deserializeToParameterizedType(o, (ParameterizedType) type); + return deserializeToParameterizedType(o, (ParameterizedType) type, path); } else if (type instanceof Class) { - return deserializeToClass(o, (Class) type); + return deserializeToClass(o, (Class) type, path); } else if (type instanceof WildcardType) { - throw new RuntimeException("Generic wildcard types are not supported"); + Type[] lowerBounds = ((WildcardType) type).getLowerBounds(); + if (lowerBounds.length > 0) { + throw deserializeError(path, "Generic lower-bounded wildcard types are not supported"); + } + + // Upper bounded wildcards are of the form . Multiple upper bounds are allowed + // but if any of the bounds are of class type, that bound must come first in this array. Note + // that this array always has at least one element, since the unbounded wildcard always + // has at least an upper bound of Object. + Type[] upperBounds = ((WildcardType) type).getUpperBounds(); + hardAssert(upperBounds.length > 0, "Unexpected type bounds on wildcard " + type); + return deserializeToType(o, upperBounds[0], path); + } else if (type instanceof TypeVariable) { + // As above, TypeVariables always have at least one upper bound of Object. + Type[] upperBounds = ((TypeVariable) type).getBounds(); + hardAssert(upperBounds.length > 0, "Unexpected type bounds on type variable " + type); + return deserializeToType(o, upperBounds[0], path); + } else if (type instanceof GenericArrayType) { - throw new RuntimeException("Generic Arrays are not supported, please use Lists instead"); + throw deserializeError(path, "Generic Arrays are not supported, please use Lists instead"); } else { - throw new IllegalStateException("Unknown type encountered: " + type); + throw deserializeError(path, "Unknown type encountered: " + type); } } @SuppressWarnings("unchecked") - private static T deserializeToClass(Object o, Class clazz) { + private static T deserializeToClass(Object o, Class clazz, ErrorPath path) { if (o == null) { return null; } else if (clazz.isPrimitive() || Number.class.isAssignableFrom(clazz) || Boolean.class.isAssignableFrom(clazz) || Character.class.isAssignableFrom(clazz)) { - return (T) deserializeToPrimitive(o, clazz); + return (T) deserializeToPrimitive(o, clazz, path); } else if (String.class.isAssignableFrom(clazz)) { - return (T) convertString(o); + return (T) convertString(o, path); } else if (Date.class.isAssignableFrom(clazz)) { - return (T) convertDate(o); + return (T) convertDate(o, path); + } else if (Timestamp.class.isAssignableFrom(clazz)) { + return (T) convertTimestamp(o, path); } else if (Blob.class.isAssignableFrom(clazz)) { - return (T) convertBlob(o); + return (T) convertBlob(o, path); } else if (GeoPoint.class.isAssignableFrom(clazz)) { - return (T) convertGeoPoint(o); + return (T) convertGeoPoint(o, path); } else if (DocumentReference.class.isAssignableFrom(clazz)) { - return (T) convertDocumentReference(o); + return (T) convertDocumentReference(o, path); } else if (clazz.isArray()) { - throw new RuntimeException("Converting to Arrays is not supported, please use Lists instead"); + throw deserializeError( + path, "Converting to Arrays is not supported, please use Lists instead"); } else if (clazz.getTypeParameters().length > 0) { - throw new RuntimeException( + throw deserializeError( + path, "Class " + clazz.getName() - + " has generic type " - + "parameters, please use GenericTypeIndicator instead"); + + " has generic type parameters, please use GenericTypeIndicator instead"); } else if (clazz.equals(Object.class)) { return (T) o; } else if (clazz.isEnum()) { - return deserializeToEnum(o, clazz); + return deserializeToEnum(o, clazz, path); } else { - return convertBean(o, clazz); + return convertBean(o, clazz, path); } } @SuppressWarnings({"unchecked", "TypeParameterUnusedInFormals"}) - private static T deserializeToParameterizedType(Object o, ParameterizedType type) { + private static T deserializeToParameterizedType( + Object o, ParameterizedType type, ErrorPath path) { // getRawType should always return a Class Class rawType = (Class) type.getRawType(); if (List.class.isAssignableFrom(rawType)) { @@ -213,33 +252,33 @@ private static T deserializeToParameterizedType(Object o, ParameterizedType if (o instanceof List) { List list = (List) o; List result = new ArrayList<>(list.size()); - for (Object object : list) { - result.add(deserializeToType(object, genericType)); + for (int i = 0; i < list.size(); i++) { + result.add(deserializeToType(list.get(i), genericType, path.child("[" + i + "]"))); } return (T) result; } else { - throw new RuntimeException( - "Expected a List while deserializing, but got a " + o.getClass()); + throw deserializeError(path, "Expected a List, but got a " + o.getClass()); } } else if (Map.class.isAssignableFrom(rawType)) { Type keyType = type.getActualTypeArguments()[0]; Type valueType = type.getActualTypeArguments()[1]; if (!keyType.equals(String.class)) { - throw new RuntimeException( - "Only Maps with string keys are supported, " - + "but found Map with key type " - + keyType); + throw deserializeError( + path, + "Only Maps with string keys are supported, but found Map with key type " + keyType); } - Map map = expectMap(o); + Map map = expectMap(o, path); HashMap result = new HashMap<>(); for (Map.Entry entry : map.entrySet()) { - result.put(entry.getKey(), deserializeToType(entry.getValue(), valueType)); + result.put( + entry.getKey(), + deserializeToType(entry.getValue(), valueType, path.child(entry.getKey()))); } return (T) result; } else if (Collection.class.isAssignableFrom(rawType)) { - throw new RuntimeException("Collections are not supported, please use Lists instead"); + throw deserializeError(path, "Collections are not supported, please use Lists instead"); } else { - Map map = expectMap(o); + Map map = expectMap(o, path); BeanMapper mapper = (BeanMapper) loadOrCreateBeanMapperForClass(rawType); HashMap>, Type> typeMapping = new HashMap<>(); TypeVariable>[] typeVariables = mapper.clazz.getTypeParameters(); @@ -250,35 +289,35 @@ private static T deserializeToParameterizedType(Object o, ParameterizedType for (int i = 0; i < typeVariables.length; i++) { typeMapping.put(typeVariables[i], types[i]); } - return mapper.deserialize(map, typeMapping); + return mapper.deserialize(map, typeMapping, path); } } @SuppressWarnings("unchecked") - private static T deserializeToPrimitive(Object o, Class clazz) { + private static T deserializeToPrimitive(Object o, Class clazz, ErrorPath path) { if (Integer.class.isAssignableFrom(clazz) || int.class.isAssignableFrom(clazz)) { - return (T) convertInteger(o); + return (T) convertInteger(o, path); } else if (Boolean.class.isAssignableFrom(clazz) || boolean.class.isAssignableFrom(clazz)) { - return (T) convertBoolean(o); + return (T) convertBoolean(o, path); } else if (Double.class.isAssignableFrom(clazz) || double.class.isAssignableFrom(clazz)) { - return (T) convertDouble(o); + return (T) convertDouble(o, path); } else if (Long.class.isAssignableFrom(clazz) || long.class.isAssignableFrom(clazz)) { - return (T) convertLong(o); + return (T) convertLong(o, path); } else if (Float.class.isAssignableFrom(clazz) || float.class.isAssignableFrom(clazz)) { - return (T) (Float) convertDouble(o).floatValue(); + return (T) (Float) convertDouble(o, path).floatValue(); } else if (Short.class.isAssignableFrom(clazz) || short.class.isAssignableFrom(clazz)) { - throw new RuntimeException("Deserializing to shorts is not supported"); + throw deserializeError(path, "Deserializing to shorts is not supported"); } else if (Byte.class.isAssignableFrom(clazz) || byte.class.isAssignableFrom(clazz)) { - throw new RuntimeException("Deserializing to bytes is not supported"); + throw deserializeError(path, "Deserializing to bytes is not supported"); } else if (Character.class.isAssignableFrom(clazz) || char.class.isAssignableFrom(clazz)) { - throw new RuntimeException("Deserializing to char is not supported"); + throw deserializeError(path, "Deserializing to chars is not supported"); } else { throw new IllegalArgumentException("Unknown primitive type: " + clazz); } } @SuppressWarnings("unchecked") - private static T deserializeToEnum(Object object, Class clazz) { + private static T deserializeToEnum(Object object, Class clazz, ErrorPath path) { if (object instanceof String) { String value = (String) object; // We cast to Class without generics here since we can't prove the bound @@ -286,11 +325,13 @@ private static T deserializeToEnum(Object object, Class clazz) { try { return (T) Enum.valueOf((Class) clazz, value); } catch (IllegalArgumentException e) { - throw new RuntimeException( + throw deserializeError( + path, "Could not find enum value of " + clazz.getName() + " for value \"" + value + "\""); } } else { - throw new RuntimeException( + throw deserializeError( + path, "Expected a String while deserializing to enum " + clazz + " but got a " @@ -311,17 +352,17 @@ private static BeanMapper loadOrCreateBeanMapperForClass(Class clazz) } @SuppressWarnings("unchecked") - private static Map expectMap(Object object) { + private static Map expectMap(Object object, ErrorPath path) { if (object instanceof Map) { // TODO(dimond): runtime validation of keys? return (Map) object; } else { - throw new RuntimeException( - "Expected a Map while deserializing, but got a " + object.getClass()); + throw deserializeError( + path, "Expected a Map while deserializing, but got a " + object.getClass()); } } - private static Integer convertInteger(Object o) { + private static Integer convertInteger(Object o, ErrorPath path) { if (o instanceof Integer) { return (Integer) o; } else if (o instanceof Long || o instanceof Double) { @@ -329,18 +370,19 @@ private static Integer convertInteger(Object o) { if (value >= Integer.MIN_VALUE && value <= Integer.MAX_VALUE) { return ((Number) o).intValue(); } else { - throw new RuntimeException( + throw deserializeError( + path, "Numeric value out of 32-bit integer range: " + value + ". Did you mean to use a long or double instead of an int?"); } } else { - throw new RuntimeException( - "Failed to convert a value of type " + o.getClass().getName() + " to int"); + throw deserializeError( + path, "Failed to convert a value of type " + o.getClass().getName() + " to int"); } } - private static Long convertLong(Object o) { + private static Long convertLong(Object o, ErrorPath path) { if (o instanceof Integer) { return ((Integer) o).longValue(); } else if (o instanceof Long) { @@ -350,18 +392,19 @@ private static Long convertLong(Object o) { if (value >= Long.MIN_VALUE && value <= Long.MAX_VALUE) { return value.longValue(); } else { - throw new RuntimeException( + throw deserializeError( + path, "Numeric value out of 64-bit long range: " + value + ". Did you mean to use a double instead of a long?"); } } else { - throw new RuntimeException( - "Failed to convert a value of type " + o.getClass().getName() + " to long"); + throw deserializeError( + path, "Failed to convert a value of type " + o.getClass().getName() + " to long"); } } - private static Double convertDouble(Object o) { + private static Double convertDouble(Object o, ErrorPath path) { if (o instanceof Integer) { return ((Integer) o).doubleValue(); } else if (o instanceof Long) { @@ -370,7 +413,8 @@ private static Double convertDouble(Object o) { if (doubleValue.longValue() == value) { return doubleValue; } else { - throw new RuntimeException( + throw deserializeError( + path, "Loss of precision while converting number to " + "double: " + o @@ -379,77 +423,107 @@ private static Double convertDouble(Object o) { } else if (o instanceof Double) { return (Double) o; } else { - throw new RuntimeException( - "Failed to convert a value of type " + o.getClass().getName() + " to double"); + throw deserializeError( + path, "Failed to convert a value of type " + o.getClass().getName() + " to double"); } } - private static Boolean convertBoolean(Object o) { + private static Boolean convertBoolean(Object o, ErrorPath path) { if (o instanceof Boolean) { return (Boolean) o; } else { - throw new RuntimeException( - "Failed to convert value of type " + o.getClass().getName() + " to boolean"); + throw deserializeError( + path, "Failed to convert value of type " + o.getClass().getName() + " to boolean"); } } - private static String convertString(Object o) { + private static String convertString(Object o, ErrorPath path) { if (o instanceof String) { return (String) o; } else { - throw new RuntimeException( - "Failed to convert value of type " + o.getClass().getName() + " to String"); + throw deserializeError( + path, "Failed to convert value of type " + o.getClass().getName() + " to String"); } } - private static Date convertDate(Object o) { + private static Date convertDate(Object o, ErrorPath path) { if (o instanceof Date) { return (Date) o; + } else if (o instanceof Timestamp) { + return ((Timestamp) o).toDate(); + } else { + throw deserializeError( + path, "Failed to convert value of type " + o.getClass().getName() + " to Date"); + } + } + + private static Timestamp convertTimestamp(Object o, ErrorPath path) { + if (o instanceof Timestamp) { + return (Timestamp) o; + } else if (o instanceof Date) { + return Timestamp.of((Date) o); } else { - throw new RuntimeException( - "Failed to convert value of type " + o.getClass().getName() + " to Date"); + throw deserializeError( + path, "Failed to convert value of type " + o.getClass().getName() + " to Timestamp"); } } - private static Blob convertBlob(Object o) { + private static Blob convertBlob(Object o, ErrorPath path) { if (o instanceof Blob) { return (Blob) o; } else { - throw new RuntimeException( - "Failed to convert value of type " + o.getClass().getName() + " to Blob"); + throw deserializeError( + path, "Failed to convert value of type " + o.getClass().getName() + " to Blob"); } } - private static GeoPoint convertGeoPoint(Object o) { + private static GeoPoint convertGeoPoint(Object o, ErrorPath path) { if (o instanceof GeoPoint) { return (GeoPoint) o; } else { - throw new RuntimeException( - "Failed to convert value of type " + o.getClass().getName() + " to GeoPoint"); + throw deserializeError( + path, "Failed to convert value of type " + o.getClass().getName() + " to GeoPoint"); } } - private static DocumentReference convertDocumentReference(Object o) { + private static DocumentReference convertDocumentReference(Object o, ErrorPath path) { if (o instanceof DocumentReference) { return (DocumentReference) o; } else { - throw new RuntimeException( + throw deserializeError( + path, "Failed to convert value of type " + o.getClass().getName() + " to DocumentReference"); } } - private static T convertBean(Object o, Class clazz) { + private static T convertBean(Object o, Class clazz, ErrorPath path) { BeanMapper mapper = loadOrCreateBeanMapperForClass(clazz); if (o instanceof Map) { - return mapper.deserialize(expectMap(o)); + return mapper.deserialize(expectMap(o, path), path); } else { - throw new RuntimeException( + throw deserializeError( + path, "Can't convert object of type " + o.getClass().getName() + " to type " + clazz.getName()); } } - private static class BeanMapper { + private static RuntimeException serializeError(ErrorPath path, String reason) { + reason = "Could not serialize object. " + reason; + if (path.getLength() > 0) { + reason = reason + " (found in field '" + path.toString() + "')"; + } + return new RuntimeException(reason); + } + private static RuntimeException deserializeError(ErrorPath path, String reason) { + reason = "Could not deserialize object. " + reason; + if (path.getLength() > 0) { + reason = reason + " (found in field '" + path.toString() + "')"; + } + return new RuntimeException(reason); + } + + private static class BeanMapper { private final Class clazz; private final Constructor constructor; private final boolean throwOnUnknownProperties; @@ -492,7 +566,11 @@ public BeanMapper(Class clazz) { addProperty(propertyName); method.setAccessible(true); if (getters.containsKey(propertyName)) { - throw new RuntimeException("Found conflicting getters for name: " + method.getName()); + throw new RuntimeException( + "Found conflicting getters for name " + + method.getName() + + " on class " + + clazz.getName()); } getters.put(propertyName, method); applyGetterAnnotations(method); @@ -521,7 +599,10 @@ public BeanMapper(Class clazz) { if (existingPropertyName != null) { if (!existingPropertyName.equals(propertyName)) { throw new RuntimeException( - "Found setter with invalid case-sensitive name: " + method.getName()); + "Found setter on " + + currentClass.getName() + + " with invalid case-sensitive name: " + + method.getName()); } else { Method existingSetter = setters.get(propertyName); if (existingSetter == null) { @@ -531,15 +612,24 @@ public BeanMapper(Class clazz) { } else if (!isSetterOverride(method, existingSetter)) { // We require that setters with conflicting property names are // overrides from a base class - throw new RuntimeException( - "Found a conflicting setters " - + "with name: " - + method.getName() - + " (conflicts with " - + existingSetter.getName() - + " defined on " - + existingSetter.getDeclaringClass().getName() - + ")"); + if (currentClass == clazz) { + // TODO(mikelehen): Should we support overloads? + throw new RuntimeException( + "Class " + + clazz.getName() + + " has multiple setter overloads with name " + + method.getName()); + } else { + throw new RuntimeException( + "Found conflicting setters " + + "with name: " + + method.getName() + + " (conflicts with " + + existingSetter.getName() + + " defined on " + + existingSetter.getDeclaringClass().getName() + + ")"); + } } } } @@ -579,17 +669,19 @@ private void addProperty(String property) { } } - public T deserialize(Map values) { - return deserialize(values, Collections.>, Type>emptyMap()); + public T deserialize(Map values, ErrorPath path) { + return deserialize(values, Collections.>, Type>emptyMap(), path); } - public T deserialize(Map values, Map>, Type> types) { + public T deserialize( + Map values, Map>, Type> types, ErrorPath path) { if (constructor == null) { - throw new RuntimeException( + throw deserializeError( + path, "Class " + clazz.getName() + " does not define a no-argument constructor. If you are using ProGuard, make " - + "sure these constructors are not stripped."); + + "sure these constructors are not stripped"); } T instance; try { @@ -599,14 +691,16 @@ public T deserialize(Map values, Map>, Typ } for (Map.Entry entry : values.entrySet()) { String propertyName = entry.getKey(); + ErrorPath childPath = path.child(propertyName); if (setters.containsKey(propertyName)) { Method setter = setters.get(propertyName); Type[] params = setter.getGenericParameterTypes(); if (params.length != 1) { - throw new IllegalStateException("Setter does not have exactly one parameter"); + throw deserializeError(childPath, "Setter does not have exactly one parameter"); } Type resolvedType = resolveType(params[0], types); - Object value = CustomClassMapper.deserializeToType(entry.getValue(), resolvedType); + Object value = + CustomClassMapper.deserializeToType(entry.getValue(), resolvedType, childPath); try { setter.invoke(instance, value); } catch (IllegalAccessException | InvocationTargetException e) { @@ -615,7 +709,8 @@ public T deserialize(Map values, Map>, Typ } else if (fields.containsKey(propertyName)) { Field field = fields.get(propertyName); Type resolvedType = resolveType(field.getGenericType(), types); - Object value = CustomClassMapper.deserializeToType(entry.getValue(), resolvedType); + Object value = + CustomClassMapper.deserializeToType(entry.getValue(), resolvedType, childPath); try { field.set(instance, value); } catch (IllegalAccessException e) { @@ -630,8 +725,7 @@ public T deserialize(Map values, Map>, Typ if (throwOnUnknownProperties) { throw new RuntimeException(message); } else if (warnOnUnknownProperties) { - // TODO(dimond): Better logging - System.out.print(message); + LOGGER.warning(message); } } } @@ -651,7 +745,7 @@ private Type resolveType(Type type, Map>, Type> types) { } } - public Map serialize(T object) { + public Map serialize(T object, ErrorPath path) { if (!clazz.isAssignableFrom(object.getClass())) { throw new IllegalArgumentException( "Can't serialize object of class " @@ -673,7 +767,7 @@ public Map serialize(T object) { // Must be a field Field field = fields.get(property); if (field == null) { - throw new IllegalStateException("Bean property without field or getter:" + property); + throw new IllegalStateException("Bean property without field or getter: " + property); } try { propertyValue = field.get(object); @@ -687,7 +781,7 @@ public Map serialize(T object) { // Replace null ServerTimestamp-annotated fields with the sentinel. serializedValue = FieldValue.serverTimestamp(); } else { - serializedValue = CustomClassMapper.serialize(propertyValue); + serializedValue = CustomClassMapper.serialize(propertyValue, path.child(property)); } result.put(property, serializedValue); } @@ -697,13 +791,13 @@ public Map serialize(T object) { private void applyFieldAnnotations(Field field) { if (field.isAnnotationPresent(ServerTimestamp.class)) { Class fieldType = field.getType(); - if (fieldType != Date.class) { + if (fieldType != Date.class && fieldType != Timestamp.class) { throw new IllegalArgumentException( "Field " + field.getName() + " is annotated with @ServerTimestamp but is " + fieldType - + " instead of Date."); + + " instead of Date or Timestamp."); } serverTimestamps.add(propertyName(field)); } @@ -712,13 +806,13 @@ private void applyFieldAnnotations(Field field) { private void applyGetterAnnotations(Method method) { if (method.isAnnotationPresent(ServerTimestamp.class)) { Class returnType = method.getReturnType(); - if (returnType != Date.class) { + if (returnType != Date.class && returnType != Timestamp.class) { throw new IllegalArgumentException( "Method " + method.getName() + " is annotated with @ServerTimestamp but returns " + returnType - + " instead of Date."); + + " instead of Date or Timestamp."); } serverTimestamps.add(propertyName(method)); } @@ -875,4 +969,42 @@ private static String serializedName(String methodName) { return new String(chars); } } + + /** + * Immutable class representing the path to a specific field in an object. Used to provide better + * error messages. + */ + static class ErrorPath { + private final int length; + private final ErrorPath parent; + private final String name; + + static final ErrorPath EMPTY = new ErrorPath(null, null, 0); + + ErrorPath(ErrorPath parent, String name, int length) { + this.parent = parent; + this.name = name; + this.length = length; + } + + int getLength() { + return length; + } + + public ErrorPath child(String name) { + return new ErrorPath(this, name, length + 1); + } + + @Override + public String toString() { + if (length == 0) { + return ""; + } else if (length == 1) { + return name; + } else { + // This is not very efficient, but it's only hit if there's an error. + return parent.toString() + "." + name; + } + } + } } 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 index 6382464fa65d..cb373ae5f340 100644 --- 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 @@ -16,12 +16,12 @@ package com.google.cloud.firestore; +import com.google.cloud.Timestamp; import com.google.cloud.firestore.UserDataConverter.EncodingOptions; import com.google.common.base.Preconditions; import com.google.firestore.v1beta1.Document; import com.google.firestore.v1beta1.Value; import com.google.firestore.v1beta1.Write; -import com.google.protobuf.Timestamp; import java.util.ArrayList; import java.util.Date; import java.util.HashMap; @@ -29,10 +29,8 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.concurrent.TimeUnit; import javax.annotation.Nonnull; import javax.annotation.Nullable; -import org.threeten.bp.Instant; /** * A DocumentSnapshot contains data read from a document in a Firestore database. The data can be @@ -51,17 +49,17 @@ public class DocumentSnapshot { private final FirestoreImpl firestore; private final DocumentReference docRef; @Nullable private final Map fields; - @Nullable private Instant readTime; - @Nullable private Instant updateTime; - @Nullable private Instant createTime; + @Nullable private Timestamp readTime; + @Nullable private Timestamp updateTime; + @Nullable private Timestamp createTime; DocumentSnapshot( FirestoreImpl firestore, DocumentReference docRef, @Nullable Map fields, - @Nullable Instant readTime, - @Nullable Instant updateTime, - @Nullable Instant createTime) { + @Nullable Timestamp readTime, + @Nullable Timestamp updateTime, + @Nullable Timestamp createTime) { this.firestore = firestore; this.docRef = docRef; this.fields = fields; @@ -98,19 +96,17 @@ static DocumentSnapshot fromObject( static DocumentSnapshot fromDocument( FirestoreImpl firestore, Timestamp readTime, Document document) { - Timestamp updateTime = document.getUpdateTime(); - Timestamp createTime = document.getCreateTime(); return new DocumentSnapshot( firestore, new DocumentReference(firestore, ResourcePath.create(document.getName())), document.getFieldsMap(), - Instant.ofEpochSecond(readTime.getSeconds(), readTime.getNanos()), - Instant.ofEpochSecond(updateTime.getSeconds(), updateTime.getNanos()), - Instant.ofEpochSecond(createTime.getSeconds(), createTime.getNanos())); + readTime, + Timestamp.fromProto(document.getUpdateTime()), + Timestamp.fromProto(document.getCreateTime())); } static DocumentSnapshot fromMissing( - FirestoreImpl firestore, DocumentReference documentReference, Instant readTime) { + FirestoreImpl firestore, DocumentReference documentReference, Timestamp readTime) { return new DocumentSnapshot(firestore, documentReference, null, readTime, null, null); } @@ -126,11 +122,7 @@ private Object decodeValue(Value v) { case DOUBLE_VALUE: return v.getDoubleValue(); case TIMESTAMP_VALUE: - Timestamp timestamp = v.getTimestampValue(); - long milliseconds = - TimeUnit.SECONDS.toMillis(timestamp.getSeconds()) - + TimeUnit.NANOSECONDS.toMillis(timestamp.getNanos()); - return new Date(milliseconds); + return Timestamp.fromProto(v.getTimestampValue()); case STRING_VALUE: return v.getStringValue(); case BYTES_VALUE: @@ -166,7 +158,7 @@ private Object decodeValue(Value v) { * @return The read time of this snapshot. */ @Nullable - public Instant getReadTime() { + public Timestamp getReadTime() { return readTime; } @@ -178,7 +170,7 @@ public Instant getReadTime() { * exist. */ @Nullable - public Instant getUpdateTime() { + public Timestamp getUpdateTime() { return updateTime; } @@ -189,7 +181,7 @@ public Instant getUpdateTime() { * exist. */ @Nullable - public Instant getCreateTime() { + public Timestamp getCreateTime() { return createTime; } @@ -222,7 +214,9 @@ public Map getData() { Map decodedFields = new HashMap<>(); for (Map.Entry entry : fields.entrySet()) { - decodedFields.put(entry.getKey(), decodeValue(entry.getValue())); + Object decodedValue = decodeValue(entry.getValue()); + decodedValue = convertToDateIfNecessary(decodedValue); + decodedFields.put(entry.getKey(), decodedValue); } return decodedFields; } @@ -287,7 +281,17 @@ public Object get(@Nonnull FieldPath fieldPath) { return null; } - return decodeValue(value); + Object decodedValue = decodeValue(value); + return convertToDateIfNecessary(decodedValue); + } + + private Object convertToDateIfNecessary(Object decodedValue) { + if (decodedValue instanceof Timestamp) { + if (!this.firestore.areTimestampsInSnapshotsEnabled()) { + decodedValue = ((Timestamp) decodedValue).toDate(); + } + } + return decodedValue; } /** Returns the Value Proto at 'fieldPath'. Returns null if the field was not found. */ @@ -363,13 +367,31 @@ public Long getLong(@Nonnull String field) { /** * Returns the value of the field as a Date. * + *

This method ignores the global setting {@link + * FirestoreOptions#areTimestampsInSnapshotsEnabled}. + * * @param field The path to the field. * @throws RuntimeException if the value is not a Date. * @return The value of the field. */ @Nullable public Date getDate(@Nonnull String field) { - return (Date) get(field); + return ((Timestamp) get(field)).toDate(); + } + + /** + * Returns the value of the field as a {@link Timestamp}. + * + *

This method ignores the global setting {@link + * FirestoreOptions#areTimestampsInSnapshotsEnabled}. + * + * @param field The path to the field. + * @throws RuntimeException if the value is not a Date. + * @return The value of the field. + */ + @Nullable + public Timestamp getTimestamp(@Nonnull String field) { + return (Timestamp) get(field); } /** diff --git a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreImpl.java b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreImpl.java index b2da6a5c8d6e..525dad28718e 100644 --- a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreImpl.java +++ b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreImpl.java @@ -25,6 +25,7 @@ import com.google.api.gax.rpc.BidiStreamingCallable; import com.google.api.gax.rpc.ServerStreamingCallable; import com.google.api.gax.rpc.UnaryCallable; +import com.google.cloud.Timestamp; import com.google.cloud.firestore.spi.v1beta1.FirestoreRpc; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; @@ -45,9 +46,9 @@ import java.util.Map; import java.util.Random; import java.util.concurrent.Executor; +import java.util.logging.Logger; import javax.annotation.Nonnull; import javax.annotation.Nullable; -import org.threeten.bp.Instant; /** * Main implementation of the Firestore client. This is the entry point for all Firestore @@ -60,6 +61,7 @@ class FirestoreImpl implements Firestore { private static final String AUTO_ID_ALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; + private static final Logger LOGGER = Logger.getLogger("Firestore"); private static final Tracer tracer = Tracing.getTracer(); private static final io.opencensus.trace.Status TOO_MANY_RETRIES_STATUS = io.opencensus.trace.Status.ABORTED.withDescription("too many retries"); @@ -83,6 +85,34 @@ class FirestoreImpl implements Firestore { + "Please explicitly set your Project ID in FirestoreOptions."); this.databasePath = ResourcePath.create(DatabaseRootName.of(options.getProjectId(), options.getDatabaseId())); + + if (!options.areTimestampsInSnapshotsEnabled()) { + LOGGER.warning( + "The behavior for java.util.Date objects stored in Firestore is going to change " + + "AND YOUR APP MAY BREAK.\n" + + "To hide this warning and ensure your app does not break, you need to add " + + "the following code to your app before calling any other Cloud Firestore " + + "methods:\n" + + "\n" + + "FirestoreOptions options = \n" + + " FirestoreOptions.newBuilder().setTimestampsInSnapshotsEnabled(true).build();\n" + + "Firestore firestore = options.getService();\n" + + "\n" + + "With this change, timestamps stored in Cloud Firestore will be read back as " + + "com.google.cloud.Timestamp objects instead of as system java.util.Date " + + "objects. So you will also need to update code expecting a java.util.Date to " + + "instead expect a Timestamp. For example:\n" + + "\n" + + "// Old:\n" + + "java.util.Date date = snapshot.getDate(\"created_at\");\n" + + "// New:\n" + + "Timestamp timestamp = snapshot.getTimestamp(\"created_at\");\n" + + "java.util.Date date = timestamp.toDate();\n" + + "\n" + + "Please audit all existing usages of java.util.Date when you enable the new " + + "behavior. In a future release, the behavior will be changed to the new " + + "behavior, so if you do not follow these steps, YOUR APP MAY BREAK."); + } } /** Creates a pseudo-random 20-character ID that can be used for Firestore documents. */ @@ -159,7 +189,9 @@ public void onNext(BatchGetDocumentsResponse response) { FirestoreImpl.this, ResourcePath.create(response.getFound().getName())); documentSnapshot = DocumentSnapshot.fromDocument( - FirestoreImpl.this, response.getReadTime(), response.getFound()); + FirestoreImpl.this, + Timestamp.fromProto(response.getReadTime()), + response.getFound()); break; case MISSING: documentReference = @@ -169,9 +201,7 @@ public void onNext(BatchGetDocumentsResponse response) { DocumentSnapshot.fromMissing( FirestoreImpl.this, documentReference, - Instant.ofEpochSecond( - response.getReadTime().getSeconds(), - response.getReadTime().getNanos())); + Timestamp.fromProto(response.getReadTime())); break; default: return; @@ -369,6 +399,11 @@ public void onSuccess(Void ignored) { }); } + /** Returns whether the user has opted into receiving dates as com.google.cloud.Timestamp. */ + boolean areTimestampsInSnapshotsEnabled() { + return this.firestoreOptions.areTimestampsInSnapshotsEnabled(); + } + /** Returns the name of the Firestore project associated with this client. */ String getDatabaseName() { return databasePath.toString(); diff --git a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreOptions.java b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreOptions.java index 01886df6ccf4..87d37100ff07 100644 --- a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreOptions.java +++ b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreOptions.java @@ -40,9 +40,11 @@ public final class FirestoreOptions extends ServiceOptions { private String databaseId = DEFAULT_DATABASE_ID; + private boolean timestampsInSnapshotsEnabled = DEFAULT_TIMESTAMPS_IN_SNAPSHOTS_ENABLED; private Builder() {} private Builder(FirestoreOptions options) { super(options); + timestampsInSnapshotsEnabled = options.timestampsInSnapshotsEnabled; } @Nonnull @@ -106,19 +110,48 @@ public Builder setTransportOptions(@Nonnull TransportOptions transportOptions) { throw new IllegalArgumentException( "Only grpc transport is allowed for " + API_SHORT_NAME + "."); } - return super.setTransportOptions(transportOptions); + super.setTransportOptions(transportOptions); + return this; } - @Override + /** + * Enables the use of {@link com.google.cloud.Timestamp Timestamps} for timestamp fields in + * {@link DocumentSnapshot DocumentSnapshots}. + * + *

Currently, Firestore returns timestamp fields as {@link java.util.Date} but {@link + * java.util.Date Date} only supports millisecond precision, which leads to truncation and + * causes unexpected behavior when using a timestamp from a snapshot as a part of a subsequent + * query. + * + *

Setting {@code setTimestampsInSnapshotsEnabled(true)} will cause Firestore to return + * {@link com.google.cloud.Timestamp Timestamp} values instead of {@link java.util.Date Date}, + * avoiding this kind of problem. To make this work you must also change any code that uses + * {@link java.util.Date Date} to use {@link com.google.cloud.Timestamp Timestamp} instead. + * + *

NOTE: in the future {@link FirestoreOptions#areTimestampsInSnapshotsEnabled} will default + * to true and this option will be removed so you should change your code to use Timestamp now + * and opt-in to this new behavior as soon as you can. + * + * @return A settings object on which the return type for timestamp fields is configured as + * specified by the given {@code value}. + */ @Nonnull - public FirestoreOptions build() { - return new FirestoreOptions(this); + public Builder setTimestampsInSnapshotsEnabled(boolean value) { + this.timestampsInSnapshotsEnabled = value; + return this; } - public Builder setDatabaseId(String databaseId) { + @Nonnull + public Builder setDatabaseId(@Nonnull String databaseId) { this.databaseId = databaseId; return this; } + + @Override + @Nonnull + public FirestoreOptions build() { + return new FirestoreOptions(this); + } } @InternalApi("This class should only be extended within google-cloud-java") @@ -126,6 +159,7 @@ protected FirestoreOptions(Builder builder) { super(FirestoreFactory.class, FirestoreRpcFactory.class, builder, new FirestoreDefaults()); this.databaseId = builder.databaseId; + this.timestampsInSnapshotsEnabled = builder.timestampsInSnapshotsEnabled; } private static class FirestoreDefaults implements ServiceDefaults { @@ -154,6 +188,14 @@ public static GrpcTransportOptions getDefaultGrpcTransportOptions() { return GrpcTransportOptions.newBuilder().build(); } + /** + * Returns whether or not {@link DocumentSnapshot DocumentSnapshots} return timestamp fields as + * {@link com.google.cloud.Timestamp Timestamps}. + */ + public boolean areTimestampsInSnapshotsEnabled() { + return timestampsInSnapshotsEnabled; + } + @Override protected Set getScopes() { return SCOPES; 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 index 8a45c35c1a38..8f28ed96a603 100644 --- 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 @@ -16,10 +16,9 @@ package com.google.cloud.firestore; -import com.google.protobuf.Timestamp; +import com.google.cloud.Timestamp; import java.util.Objects; import javax.annotation.Nonnull; -import org.threeten.bp.Instant; /** Preconditions that can be used to restrict update() calls. */ public final class Precondition { @@ -28,9 +27,9 @@ public final class Precondition { public static final Precondition NONE = new Precondition(null, null); private Boolean exists; - private Instant updateTime; + private Timestamp updateTime; - private Precondition(Boolean exists, Instant updateTime) { + private Precondition(Boolean exists, Timestamp updateTime) { this.exists = exists; this.updateTime = updateTime; } @@ -55,7 +54,7 @@ static Precondition exists(Boolean exists) { * @return A new Precondition */ @Nonnull - public static Precondition updatedAt(Instant updateTime) { + public static Precondition updatedAt(Timestamp updateTime) { return new Precondition(null, updateTime); } @@ -76,10 +75,7 @@ com.google.firestore.v1beta1.Precondition toPb() { } if (updateTime != null) { - Timestamp.Builder timestamp = Timestamp.newBuilder(); - timestamp.setSeconds(updateTime.getEpochSecond()); - timestamp.setNanos(updateTime.getNano()); - precondition.setUpdateTime(timestamp.build()); + precondition.setUpdateTime(updateTime.toProto()); } return precondition.build(); diff --git a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java index 75040e181f40..f1b3259124c1 100644 --- a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java +++ b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java @@ -25,6 +25,7 @@ import com.google.api.core.ApiFuture; import com.google.api.core.SettableApiFuture; import com.google.api.gax.rpc.ApiStreamObserver; +import com.google.cloud.Timestamp; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.firestore.v1beta1.Cursor; @@ -49,7 +50,6 @@ import java.util.concurrent.Executor; import javax.annotation.Nonnull; import javax.annotation.Nullable; -import org.threeten.bp.Instant; /** * A Query which you can read or listen to. You can also construct refined Query objects by adding @@ -903,14 +903,14 @@ public void onCompleted() { private abstract static class QuerySnapshotObserver implements ApiStreamObserver { - private Instant readTime; + private Timestamp readTime; - void onCompleted(Instant readTime) { + void onCompleted(Timestamp readTime) { this.readTime = readTime; this.onCompleted(); } - Instant getReadTime() { + Timestamp getReadTime() { return readTime; } } @@ -933,7 +933,7 @@ private void stream( ApiStreamObserver observer = new ApiStreamObserver() { - Instant readTime; + Timestamp readTime; boolean firstResponse; int numDocuments; @@ -952,14 +952,13 @@ public void onNext(RunQueryResponse response) { } Document document = response.getDocument(); QueryDocumentSnapshot documentSnapshot = - QueryDocumentSnapshot.fromDocument(firestore, response.getReadTime(), document); + QueryDocumentSnapshot.fromDocument( + firestore, Timestamp.fromProto(response.getReadTime()), document); documentObserver.onNext(documentSnapshot); } if (readTime == null) { - readTime = - Instant.ofEpochSecond( - response.getReadTime().getSeconds(), response.getReadTime().getNanos()); + readTime = Timestamp.fromProto(response.getReadTime()); } } diff --git a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/QueryDocumentSnapshot.java b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/QueryDocumentSnapshot.java index 3b2809fb275b..1f1126fab9f5 100644 --- a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/QueryDocumentSnapshot.java +++ b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/QueryDocumentSnapshot.java @@ -16,13 +16,12 @@ package com.google.cloud.firestore; +import com.google.cloud.Timestamp; import com.google.common.base.Preconditions; import com.google.firestore.v1beta1.Document; import com.google.firestore.v1beta1.Value; -import com.google.protobuf.Timestamp; import java.util.Map; import javax.annotation.Nonnull; -import org.threeten.bp.Instant; /** * A QueryDocumentSnapshot contains data read from a document in a Firestore database as part of a @@ -42,23 +41,21 @@ private QueryDocumentSnapshot( FirestoreImpl firestore, DocumentReference docRef, Map fields, - Instant readTime, - Instant updateTime, - Instant createTime) { + Timestamp readTime, + Timestamp updateTime, + Timestamp createTime) { super(firestore, docRef, fields, readTime, updateTime, createTime); } static QueryDocumentSnapshot fromDocument( FirestoreImpl firestore, Timestamp readTime, Document document) { - Timestamp updateTime = document.getUpdateTime(); - Timestamp createTime = document.getCreateTime(); return new QueryDocumentSnapshot( firestore, new DocumentReference(firestore, ResourcePath.create(document.getName())), document.getFieldsMap(), - Instant.ofEpochSecond(readTime.getSeconds(), readTime.getNanos()), - Instant.ofEpochSecond(updateTime.getSeconds(), updateTime.getNanos()), - Instant.ofEpochSecond(createTime.getSeconds(), createTime.getNanos())); + readTime, + Timestamp.fromProto(document.getUpdateTime()), + Timestamp.fromProto(document.getCreateTime())); } /** diff --git a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/QuerySnapshot.java b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/QuerySnapshot.java index b840f40a133e..b73dda84741d 100644 --- a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/QuerySnapshot.java +++ b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/QuerySnapshot.java @@ -16,6 +16,7 @@ package com.google.cloud.firestore; +import com.google.cloud.Timestamp; import com.google.cloud.firestore.DocumentChange.Type; import java.util.ArrayList; import java.util.Collections; @@ -23,7 +24,6 @@ import java.util.List; import java.util.Objects; import javax.annotation.Nonnull; -import org.threeten.bp.Instant; /** * A QuerySnapshot contains the results of a query. It can contain zero or more DocumentSnapshot @@ -32,16 +32,16 @@ public abstract class QuerySnapshot implements Iterable { private final Query query; - private final Instant readTime; + private final Timestamp readTime; - private QuerySnapshot(Query query, Instant readTime) { + private QuerySnapshot(Query query, Timestamp readTime) { this.query = query; this.readTime = readTime; } /** Creates a new QuerySnapshot representing the results of a Query with added documents. */ public static QuerySnapshot withDocuments( - final Query query, Instant readTime, final List documents) { + final Query query, Timestamp readTime, final List documents) { return new QuerySnapshot(query, readTime) { volatile List documentChanges; @@ -96,7 +96,7 @@ public int hashCode() { /** Creates a new QuerySnapshot representing a snapshot of a Query with changed documents. */ public static QuerySnapshot withChanges( final Query query, - Instant readTime, + Timestamp readTime, final DocumentSet documentSet, final List documentChanges) { return new QuerySnapshot(query, readTime) { @@ -163,7 +163,7 @@ public Query getQuery() { * @return The read time of this snapshot. */ @Nonnull - public Instant getReadTime() { + public Timestamp getReadTime() { return readTime; } diff --git a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UserDataConverter.java b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UserDataConverter.java index c2278c7f971a..6be73a8d39d5 100644 --- a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UserDataConverter.java +++ b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UserDataConverter.java @@ -16,12 +16,12 @@ package com.google.cloud.firestore; +import com.google.cloud.Timestamp; import com.google.common.base.Preconditions; import com.google.firestore.v1beta1.ArrayValue; import com.google.firestore.v1beta1.MapValue; import com.google.firestore.v1beta1.Value; import com.google.protobuf.NullValue; -import com.google.protobuf.Timestamp; import java.util.Date; import java.util.List; import java.util.Map; @@ -91,10 +91,14 @@ static Value encodeValue( Date date = (Date) sanitizedObject; long epochSeconds = TimeUnit.MILLISECONDS.toSeconds(date.getTime()); long msOffset = date.getTime() - TimeUnit.SECONDS.toMillis(epochSeconds); - Timestamp.Builder timestampBuilder = Timestamp.newBuilder(); + com.google.protobuf.Timestamp.Builder timestampBuilder = + com.google.protobuf.Timestamp.newBuilder(); timestampBuilder.setSeconds(epochSeconds); timestampBuilder.setNanos((int) TimeUnit.MILLISECONDS.toNanos(msOffset)); return Value.newBuilder().setTimestampValue(timestampBuilder.build()).build(); + } else if (sanitizedObject instanceof Timestamp) { + Timestamp timestamp = (Timestamp) sanitizedObject; + return Value.newBuilder().setTimestampValue(timestamp.toProto()).build(); } else if (sanitizedObject instanceof List) { ArrayValue.Builder res = ArrayValue.newBuilder(); int i = 0; diff --git a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Watch.java b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Watch.java index cca1f379b646..1ce0dead8ea5 100644 --- a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Watch.java +++ b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Watch.java @@ -23,6 +23,7 @@ import com.google.api.gax.retrying.TimedAttemptSettings; import com.google.api.gax.rpc.ApiException; import com.google.api.gax.rpc.ApiStreamObserver; +import com.google.cloud.Timestamp; import com.google.cloud.firestore.DocumentChange.Type; import com.google.common.base.Preconditions; import com.google.firestore.v1beta1.Document; @@ -32,7 +33,6 @@ import com.google.firestore.v1beta1.Target.QueryTarget; import com.google.firestore.v1beta1.TargetChange; import com.google.protobuf.ByteString; -import com.google.protobuf.Timestamp; import io.grpc.Status; import io.grpc.Status.Code; import io.grpc.StatusException; @@ -50,7 +50,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.Nullable; import org.threeten.bp.Duration; -import org.threeten.bp.Instant; /** * Watch provides listen functionality and exposes snapshot listeners. It can be used with any valid @@ -192,7 +191,7 @@ public synchronized void onNext(ListenResponse listenResponse) { if (noTargetIds && change.hasReadTime() && current) { // This means everything is up-to-date, so emit the current set of docs as a snapshot, // if there were changes. - pushSnapshot(change.getReadTime(), change.getResumeToken()); + pushSnapshot(Timestamp.fromProto(change.getReadTime()), change.getResumeToken()); } break; case ADD: @@ -319,7 +318,7 @@ public void run() { * Returns the current count of all documents, including the changes from the current changeMap. */ private int currentSize() { - ChangeSet changeSet = extractChanges(Timestamp.getDefaultInstance()); + ChangeSet changeSet = extractChanges(Timestamp.now()); return documentSet.size() + changeSet.adds.size() - changeSet.deletes.size(); } @@ -468,11 +467,7 @@ private void pushSnapshot(final Timestamp readTime, ByteString nextResumeToken) final List changes = computeSnapshot(readTime); if (!hasPushed || !changes.isEmpty()) { final QuerySnapshot querySnapshot = - QuerySnapshot.withChanges( - query, - Instant.ofEpochSecond(readTime.getSeconds(), readTime.getNanos()), - documentSet, - changes); + QuerySnapshot.withChanges(query, readTime, documentSet, changes); userCallbackExecutor.execute( new Runnable() { @Override diff --git a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/WriteResult.java b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/WriteResult.java index 52bf1f02a126..5e98804c8468 100644 --- a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/WriteResult.java +++ b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/WriteResult.java @@ -16,17 +16,16 @@ package com.google.cloud.firestore; -import com.google.protobuf.Timestamp; +import com.google.cloud.Timestamp; import java.util.Objects; import javax.annotation.Nonnull; -import org.threeten.bp.Instant; /** A WriteResult exposes the update time set by the server. */ public final class WriteResult { - private final Instant updateTime; + private final Timestamp updateTime; - private WriteResult(Instant updateTime) { + private WriteResult(Timestamp updateTime) { this.updateTime = updateTime; } @@ -36,15 +35,16 @@ private WriteResult(Instant updateTime) { * @return The update time of the corresponding write. */ @Nonnull - public Instant getUpdateTime() { + public Timestamp getUpdateTime() { return this.updateTime; } static WriteResult fromProto( - com.google.firestore.v1beta1.WriteResult writeResult, Timestamp commitTime) { - Timestamp timestamp = writeResult.hasUpdateTime() ? writeResult.getUpdateTime() : commitTime; - Instant instant = Instant.ofEpochSecond(timestamp.getSeconds(), timestamp.getNanos()); - return new WriteResult(instant); + com.google.firestore.v1beta1.WriteResult writeResult, + com.google.protobuf.Timestamp commitTime) { + Timestamp timestamp = + Timestamp.fromProto(writeResult.hasUpdateTime() ? writeResult.getUpdateTime() : commitTime); + return new WriteResult(timestamp); } /** diff --git a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/annotation/ServerTimestamp.java b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/annotation/ServerTimestamp.java index a21c23449855..0d7564f0b5e8 100644 --- a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/annotation/ServerTimestamp.java +++ b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/annotation/ServerTimestamp.java @@ -22,9 +22,9 @@ import java.lang.annotation.Target; /** - * Annotation used to mark a field as being populated via Server Timestamps. If a POJO being written - * contains null for a @ServerTimestamp annotated field, it will be replaced with a server-generated - * timestamp. + * Annotation used to mark a timestamp field as being populated via Server Timestamps. If a POJO + * being written contains null for a @ServerTimestamp annotated field, it will be replaced with a + * server-generated timestamp. */ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.METHOD, ElementType.FIELD}) diff --git a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/CollectionReferenceTest.java b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/CollectionReferenceTest.java index 47934c4d2890..e119bbead18c 100644 --- a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/CollectionReferenceTest.java +++ b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/CollectionReferenceTest.java @@ -43,7 +43,10 @@ public class CollectionReferenceTest { @Spy private FirestoreImpl firestoreMock = new FirestoreImpl( - FirestoreOptions.newBuilder().setProjectId("test-project").build(), + FirestoreOptions.newBuilder() + .setProjectId("test-project") + .setTimestampsInSnapshotsEnabled(true) + .build(), Mockito.mock(FirestoreRpc.class)); @Captor private ArgumentCaptor argCaptor; diff --git a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/ConformanceTest.java b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/ConformanceTest.java index 71c37e69cd5e..2d3b118a5bdb 100644 --- a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/ConformanceTest.java +++ b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/ConformanceTest.java @@ -30,6 +30,7 @@ import com.google.api.gax.rpc.BidiStreamingCallable; import com.google.api.gax.rpc.ServerStreamingCallable; import com.google.api.gax.rpc.UnaryCallable; +import com.google.cloud.Timestamp; import com.google.cloud.firestore.Query.Direction; import com.google.cloud.firestore.conformance.TestDefinition; import com.google.cloud.firestore.conformance.TestDefinition.Clause; @@ -91,7 +92,6 @@ import org.mockito.MockitoAnnotations; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; -import org.threeten.bp.Instant; @RunWith(AllTests.class) public class ConformanceTest { @@ -130,7 +130,11 @@ public ConformanceTest() { firestoreMock = Mockito.spy( new FirestoreImpl( - FirestoreOptions.newBuilder().setProjectId("projectID").build(), firestoreRpc)); + FirestoreOptions.newBuilder() + .setProjectId("projectID") + .setTimestampsInSnapshotsEnabled(true) + .build(), + firestoreRpc)); watchQuery = collection("projects/projectID/databases/(default)/documents/C").orderBy("a"); } @@ -166,9 +170,7 @@ private com.google.cloud.firestore.Precondition convertPrecondition(Precondition return com.google.cloud.firestore.Precondition.exists(precondition.getExists()); case UPDATE_TIME: return com.google.cloud.firestore.Precondition.updatedAt( - Instant.ofEpochSecond( - precondition.getUpdateTime().getSeconds(), - precondition.getUpdateTime().getNanos())); + Timestamp.fromProto(precondition.getUpdateTime())); default: return com.google.cloud.firestore.Precondition.NONE; } @@ -273,15 +275,14 @@ private DocumentChange.Type convertKind(Kind kind) { /** Converts a test snapshot to its API representation. */ private QuerySnapshot convertQuerySnapshot(Snapshot testSnapshot) { - Instant readTime = - Instant.ofEpochSecond( - testSnapshot.getReadTime().getSeconds(), testSnapshot.getReadTime().getNanos()); + Timestamp readTime = Timestamp.fromProto(testSnapshot.getReadTime()); final List documentSnapshots = new ArrayList<>(); for (Document document : testSnapshot.getDocsList()) { documentSnapshots.add( - QueryDocumentSnapshot.fromDocument(firestoreMock, testSnapshot.getReadTime(), document)); + QueryDocumentSnapshot.fromDocument( + firestoreMock, Timestamp.fromProto(testSnapshot.getReadTime()), document)); } DocumentSet documentSet = @@ -314,7 +315,9 @@ public int compare(QueryDocumentSnapshot o1, QueryDocumentSnapshot o2) { for (DocChange documentChange : testSnapshot.getChangesList()) { QueryDocumentSnapshot documentSnapshot = QueryDocumentSnapshot.fromDocument( - firestoreMock, testSnapshot.getReadTime(), documentChange.getDoc()); + firestoreMock, + Timestamp.fromProto(testSnapshot.getReadTime()), + documentChange.getDoc()); DocumentChange.Type changeType = convertKind(documentChange.getKind()); documentChanges.add( new DocumentChange( diff --git a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/DocumentReferenceTest.java b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/DocumentReferenceTest.java index e040a43af193..a7adfa19b970 100644 --- a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/DocumentReferenceTest.java +++ b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/DocumentReferenceTest.java @@ -34,6 +34,7 @@ import static com.google.cloud.firestore.LocalFirestoreHelper.SINGLE_FIELD_OBJECT; import static com.google.cloud.firestore.LocalFirestoreHelper.SINGLE_FIELD_PROTO; import static com.google.cloud.firestore.LocalFirestoreHelper.SINGLE_WRITE_COMMIT_RESPONSE; +import static com.google.cloud.firestore.LocalFirestoreHelper.TIMESTAMP; import static com.google.cloud.firestore.LocalFirestoreHelper.UPDATE_PRECONDITION; import static com.google.cloud.firestore.LocalFirestoreHelper.assertCommitEquals; import static com.google.cloud.firestore.LocalFirestoreHelper.commit; @@ -60,6 +61,7 @@ import com.google.api.gax.rpc.ApiStreamObserver; import com.google.api.gax.rpc.ServerStreamingCallable; import com.google.api.gax.rpc.UnaryCallable; +import com.google.cloud.Timestamp; import com.google.cloud.firestore.spi.v1beta1.FirestoreRpc; import com.google.common.collect.ImmutableList; import com.google.firestore.v1beta1.BatchGetDocumentsRequest; @@ -67,7 +69,6 @@ import com.google.firestore.v1beta1.CommitRequest; import com.google.firestore.v1beta1.CommitResponse; import com.google.firestore.v1beta1.Value; -import com.google.protobuf.Timestamp; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -83,7 +84,6 @@ import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.runners.MockitoJUnitRunner; -import org.threeten.bp.Instant; @RunWith(MockitoJUnitRunner.class) public class DocumentReferenceTest { @@ -91,7 +91,10 @@ public class DocumentReferenceTest { @Spy private FirestoreImpl firestoreMock = new FirestoreImpl( - FirestoreOptions.newBuilder().setProjectId("test-project").build(), + FirestoreOptions.newBuilder() + .setProjectId("test-project") + .setTimestampsInSnapshotsEnabled(true) + .build(), Mockito.mock(FirestoreRpc.class)); @Captor private ArgumentCaptor commitCapture; @@ -171,6 +174,8 @@ public void deserializeBasicTypes() throws Exception { streamObserverCapture.capture(), Matchers.any()); + doReturn(true).when(firestoreMock).areTimestampsInSnapshotsEnabled(); + DocumentSnapshot snapshot = documentReference.get().get(); assertEquals(snapshot.getData(), ALL_SUPPORTED_TYPES_MAP); @@ -191,6 +196,7 @@ public void deserializeBasicTypes() throws Exception { assertEquals((Long) 0L, snapshot.getLong("longValue")); assertEquals(true, snapshot.getBoolean("trueValue")); assertEquals(DATE, snapshot.getDate("dateValue")); + assertEquals(TIMESTAMP, snapshot.getTimestamp("timestampValue")); assertEquals(BLOB, snapshot.getBlob("bytesValue")); assertEquals(BLOB.hashCode(), snapshot.getBlob("bytesValue").hashCode()); @@ -208,9 +214,9 @@ public void deserializeBasicTypes() throws Exception { assertFalse(snapshot.contains("objectValue.bar")); assertTrue(snapshot.exists()); - assertEquals(Instant.ofEpochSecond(1, 2), snapshot.getCreateTime()); - assertEquals(Instant.ofEpochSecond(3, 4), snapshot.getUpdateTime()); - assertEquals(Instant.ofEpochSecond(5, 6), snapshot.getReadTime()); + assertEquals(Timestamp.ofTimeSecondsAndNanos(1, 2), snapshot.getCreateTime()); + assertEquals(Timestamp.ofTimeSecondsAndNanos(3, 4), snapshot.getUpdateTime()); + assertEquals(Timestamp.ofTimeSecondsAndNanos(5, 6), snapshot.getReadTime()); assertEquals(get(), getAllCapture.getValue()); } @@ -231,12 +237,39 @@ public void deserializeDocumentReference() throws Exception { assertEquals(documentReference, snapshot.getReference()); } + @Test + public void deserializesDates() throws Exception { + doAnswer(getAllResponse(ALL_SUPPORTED_TYPES_PROTO)) + .when(firestoreMock) + .streamRequest( + getAllCapture.capture(), + streamObserverCapture.capture(), + Matchers.any()); + + DocumentSnapshot snapshot = documentReference.get().get(); + + doReturn(false).when(firestoreMock).areTimestampsInSnapshotsEnabled(); + + assertEquals(DATE, snapshot.get("dateValue")); + assertEquals(TIMESTAMP.toDate(), snapshot.get("timestampValue")); + assertEquals(DATE, snapshot.getData().get("dateValue")); + assertEquals(TIMESTAMP.toDate(), snapshot.getData().get("timestampValue")); + + doReturn(true).when(firestoreMock).areTimestampsInSnapshotsEnabled(); + + assertEquals(Timestamp.of(DATE), snapshot.get("dateValue")); + assertEquals(TIMESTAMP, snapshot.get("timestampValue")); + assertEquals(Timestamp.of(DATE), snapshot.getData().get("dateValue")); + assertEquals(TIMESTAMP, snapshot.getData().get("timestampValue")); + } + @Test public void notFound() throws Exception { final BatchGetDocumentsResponse.Builder getDocumentResponse = BatchGetDocumentsResponse.newBuilder(); getDocumentResponse.setMissing(DOCUMENT_NAME); - getDocumentResponse.setReadTime(Timestamp.newBuilder().setSeconds(5).setNanos(6)); + getDocumentResponse.setReadTime( + com.google.protobuf.Timestamp.newBuilder().setSeconds(5).setNanos(6)); doAnswer(streamingResponse(getDocumentResponse.build())) .when(firestoreMock) @@ -248,7 +281,7 @@ public void notFound() throws Exception { DocumentSnapshot snapshot = documentReference.get().get(); assertEquals(documentReference, snapshot.getReference()); assertFalse(snapshot.exists()); - assertEquals(snapshot.getReadTime(), Instant.ofEpochSecond(5, 6)); + assertEquals(snapshot.getReadTime(), Timestamp.ofTimeSecondsAndNanos(5, 6)); assertNull(snapshot.getData()); } @@ -261,7 +294,7 @@ public void deleteDocument() throws Exception { documentReference.delete().get(); documentReference - .delete(Precondition.updatedAt(Instant.ofEpochSecond(479978400, 123000000))) + .delete(Precondition.updatedAt(Timestamp.ofTimeSecondsAndNanos(479978400, 123000000))) .get(); List commitRequests = commitCapture.getAllValues(); @@ -565,6 +598,7 @@ public void extractFieldMaskFromMerge() throws Exception { "second.negInfValue", "second.nullValue", "second.objectValue.foo", + "second.timestampValue", "second.trueValue"); CommitRequest expectedCommit = commit(set(nestedUpdate, updateMask)); @@ -737,7 +771,8 @@ public void updateDocumentWithPreconditions() throws Exception { .sendRequest( commitCapture.capture(), Matchers.>any()); - Precondition options = Precondition.updatedAt(Instant.ofEpochSecond(479978400, 123000000)); + Precondition options = + Precondition.updatedAt(Timestamp.ofTimeSecondsAndNanos(479978400, 123000000)); documentReference.update(SINGLE_FIELD_MAP, options).get(); documentReference.update(options, "foo", "bar").get(); diff --git a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/FirestoreTest.java b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/FirestoreTest.java index cc384e532b25..67274c7976e2 100644 --- a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/FirestoreTest.java +++ b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/FirestoreTest.java @@ -43,7 +43,10 @@ public class FirestoreTest { @Spy private FirestoreImpl firestoreMock = new FirestoreImpl( - FirestoreOptions.newBuilder().setProjectId("test-project").build(), + FirestoreOptions.newBuilder() + .setProjectId("test-project") + .setTimestampsInSnapshotsEnabled(true) + .build(), Mockito.mock(FirestoreRpc.class)); @Captor private ArgumentCaptor getAllCapture; 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 index 0aa1f82d152a..e9b5fa40caf7 100644 --- 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 @@ -21,6 +21,7 @@ import com.google.api.core.ApiFuture; import com.google.api.core.ApiFutures; import com.google.api.gax.rpc.ApiStreamObserver; +import com.google.cloud.Timestamp; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.firestore.v1beta1.ArrayValue; @@ -50,7 +51,6 @@ import com.google.protobuf.ByteString; import com.google.protobuf.Empty; import com.google.protobuf.NullValue; -import com.google.protobuf.Timestamp; import com.google.type.LatLng; import java.nio.charset.Charset; import java.text.ParseException; @@ -64,10 +64,10 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; -import org.threeten.bp.Instant; public final class LocalFirestoreHelper { @@ -104,6 +104,7 @@ public final class LocalFirestoreHelper { public static final ApiFuture SERVER_TIMESTAMP_COMMIT_RESPONSE; public static final Date DATE; + public static final Timestamp TIMESTAMP; public static final GeoPoint GEO_POINT; public static final Blob BLOB; @@ -171,9 +172,13 @@ public static Answer getAllResponse( name += i + 1; } BatchGetDocumentsResponse.Builder response = BatchGetDocumentsResponse.newBuilder(); - response.getFoundBuilder().setCreateTime(Timestamp.newBuilder().setSeconds(1).setNanos(2)); - response.getFoundBuilder().setUpdateTime(Timestamp.newBuilder().setSeconds(3).setNanos(4)); - response.setReadTime(Timestamp.newBuilder().setSeconds(5).setNanos(6)); + response + .getFoundBuilder() + .setCreateTime(com.google.protobuf.Timestamp.newBuilder().setSeconds(1).setNanos(2)); + response + .getFoundBuilder() + .setUpdateTime(com.google.protobuf.Timestamp.newBuilder().setSeconds(3).setNanos(4)); + response.setReadTime(com.google.protobuf.Timestamp.newBuilder().setSeconds(5).setNanos(6)); response.getFoundBuilder().setName(name).putAllFields(fields[i]); responses[i] = response.build(); } @@ -196,7 +201,8 @@ public static Answer queryResponse(String... documentNames) { final RunQueryResponse.Builder runQueryResponse = RunQueryResponse.newBuilder(); runQueryResponse.setDocument( Document.newBuilder().setName(documentNames[i]).putAllFields(SINGLE_FIELD_PROTO)); - runQueryResponse.setReadTime(Timestamp.newBuilder().setSeconds(1).setNanos(2)); + runQueryResponse.setReadTime( + com.google.protobuf.Timestamp.newBuilder().setSeconds(1).setNanos(2)); responses[i] = runQueryResponse.build(); } return streamingResponse(responses); @@ -568,6 +574,7 @@ public static class AllSupportedTypes { public boolean falseValue = false; public SingleField objectValue = new SingleField(); public Date dateValue = DATE; + public Timestamp timestampValue = TIMESTAMP; public List arrayValue = ImmutableList.of("foo"); public String nullValue = null; public Blob bytesValue = BLOB; @@ -592,6 +599,7 @@ public boolean equals(Object o) { && Objects.equals(doubleValue, that.doubleValue) && Objects.equals(objectValue, that.objectValue) && Objects.equals(dateValue, that.dateValue) + && Objects.equals(timestampValue, that.timestampValue) && Objects.equals(arrayValue, that.arrayValue) && Objects.equals(nullValue, that.nullValue) && Objects.equals(bytesValue, that.bytesValue) @@ -608,6 +616,10 @@ public boolean equals(Object o) { throw new RuntimeException("Failed to parse date", e); } + TIMESTAMP = + Timestamp.ofTimeSecondsAndNanos( + TimeUnit.MILLISECONDS.toSeconds(DATE.getTime()), + 123000); // Firestore truncates to microsecond precision. GEO_POINT = new GeoPoint(50.1430847, -122.9477780); BLOB = Blob.fromBytes(new byte[] {1, 2, 3}); @@ -630,9 +642,9 @@ public boolean equals(Object o) { DatabaseRootName.of("test-project", "(default)"), ImmutableList.of("coll", "doc"))), SINGLE_FIELD_PROTO, - Instant.ofEpochSecond(5, 6), - Instant.ofEpochSecond(3, 4), - Instant.ofEpochSecond(1, 2)); + Timestamp.ofTimeSecondsAndNanos(5, 6), + Timestamp.ofTimeSecondsAndNanos(3, 4), + Timestamp.ofTimeSecondsAndNanos(1, 2)); UPDATED_FIELD_MAP = map("foo", (Object) "foobar"); UPDATED_FIELD_PROTO = map("foo", Value.newBuilder().setStringValue("foobar").build()); @@ -659,7 +671,8 @@ public boolean equals(Object o) { ALL_SUPPORTED_TYPES_MAP.put("trueValue", true); ALL_SUPPORTED_TYPES_MAP.put("falseValue", false); ALL_SUPPORTED_TYPES_MAP.put("objectValue", map("foo", (Object) "bar")); - ALL_SUPPORTED_TYPES_MAP.put("dateValue", DATE); + ALL_SUPPORTED_TYPES_MAP.put("dateValue", Timestamp.of(DATE)); + ALL_SUPPORTED_TYPES_MAP.put("timestampValue", TIMESTAMP); ALL_SUPPORTED_TYPES_MAP.put("arrayValue", ImmutableList.of("foo")); ALL_SUPPORTED_TYPES_MAP.put("nullValue", null); ALL_SUPPORTED_TYPES_MAP.put("bytesValue", BLOB); @@ -684,7 +697,17 @@ public boolean equals(Object o) { "dateValue", Value.newBuilder() .setTimestampValue( - Timestamp.newBuilder().setSeconds(479978400).setNanos(123000000)) + com.google.protobuf.Timestamp.newBuilder() + .setSeconds(479978400) + .setNanos(123000000)) + .build()) + .put( + "timestampValue", + Value.newBuilder() + .setTimestampValue( + com.google.protobuf.Timestamp.newBuilder() + .setSeconds(479978400) + .setNanos(123000)) .build()) .put( "arrayValue", diff --git a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryTest.java b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryTest.java index a958631b843f..9990d4b364aa 100644 --- a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryTest.java +++ b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryTest.java @@ -35,6 +35,7 @@ import com.google.api.gax.rpc.ApiStreamObserver; import com.google.api.gax.rpc.ServerStreamingCallable; +import com.google.cloud.Timestamp; import com.google.cloud.firestore.spi.v1beta1.FirestoreRpc; import com.google.firestore.v1beta1.RunQueryRequest; import com.google.firestore.v1beta1.StructuredQuery; @@ -53,7 +54,6 @@ import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.runners.MockitoJUnitRunner; -import org.threeten.bp.Instant; @RunWith(MockitoJUnitRunner.class) public class QueryTest { @@ -61,7 +61,10 @@ public class QueryTest { @Spy private FirestoreImpl firestoreMock = new FirestoreImpl( - FirestoreOptions.newBuilder().setProjectId("test-project").build(), + FirestoreOptions.newBuilder() + .setProjectId("test-project") + .setTimestampsInSnapshotsEnabled(true) + .build(), Mockito.mock(FirestoreRpc.class)); @Captor private ArgumentCaptor runQuery; @@ -549,7 +552,7 @@ public void getResult() throws Exception { assertFalse(changeIterator.hasNext()); - assertEquals(Instant.ofEpochSecond(1, 2), result.getReadTime()); + assertEquals(Timestamp.ofTimeSecondsAndNanos(1, 2), result.getReadTime()); assertEquals( Arrays.asList( diff --git a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java index b51416968b47..9054d134343e 100644 --- a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java +++ b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java @@ -44,6 +44,7 @@ import com.google.api.gax.rpc.ApiStreamObserver; import com.google.api.gax.rpc.ServerStreamingCallable; import com.google.api.gax.rpc.UnaryCallable; +import com.google.cloud.Timestamp; import com.google.cloud.firestore.spi.v1beta1.FirestoreRpc; import com.google.firestore.v1beta1.Write; import com.google.protobuf.ByteString; @@ -65,7 +66,6 @@ import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.runners.MockitoJUnitRunner; -import org.threeten.bp.Instant; @RunWith(MockitoJUnitRunner.class) public class TransactionTest { @@ -75,7 +75,11 @@ public class TransactionTest { @Spy private FirestoreImpl firestoreMock = new FirestoreImpl( - FirestoreOptions.newBuilder().setProjectId("test-project").build(), firestoreRpc); + FirestoreOptions.newBuilder() + .setProjectId("test-project") + .setTimestampsInSnapshotsEnabled(true) + .build(), + firestoreRpc); @Captor private ArgumentCaptor requestCapture; @Captor private ArgumentCaptor> streamObserverCapture; @@ -535,7 +539,8 @@ public void deleteDocument() throws Exception { public String updateCallback(Transaction transaction) { transaction.delete(documentReference); transaction.delete( - documentReference, Precondition.updatedAt(Instant.ofEpochSecond(1, 2))); + documentReference, + Precondition.updatedAt(Timestamp.ofTimeSecondsAndNanos(1, 2))); return "foo"; } }, diff --git a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WatchTest.java b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WatchTest.java index 8080aa528656..da6d6cd8d020 100644 --- a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WatchTest.java +++ b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WatchTest.java @@ -97,7 +97,11 @@ public class WatchTest { @Spy private FirestoreImpl firestoreMock = new FirestoreImpl( - FirestoreOptions.newBuilder().setProjectId("test-project").build(), firestoreRpc); + FirestoreOptions.newBuilder() + .setProjectId("test-project") + .setTimestampsInSnapshotsEnabled(true) + .build(), + firestoreRpc); @Captor private ArgumentCaptor> streamObserverCapture; diff --git a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WriteBatchTest.java b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WriteBatchTest.java index 6b2f810ca156..299660fa4205 100644 --- a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WriteBatchTest.java +++ b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WriteBatchTest.java @@ -27,11 +27,11 @@ import static org.mockito.Mockito.doReturn; import com.google.api.gax.rpc.UnaryCallable; +import com.google.cloud.Timestamp; import com.google.cloud.firestore.spi.v1beta1.FirestoreRpc; import com.google.firestore.v1beta1.CommitRequest; import com.google.firestore.v1beta1.CommitResponse; import com.google.firestore.v1beta1.Write; -import com.google.protobuf.Timestamp; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -45,7 +45,6 @@ import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.runners.MockitoJUnitRunner; -import org.threeten.bp.Instant; @RunWith(MockitoJUnitRunner.class) public class WriteBatchTest { @@ -53,7 +52,10 @@ public class WriteBatchTest { @Spy private FirestoreImpl firestoreMock = new FirestoreImpl( - FirestoreOptions.newBuilder().setProjectId("test-project").build(), + FirestoreOptions.newBuilder() + .setProjectId("test-project") + .setTimestampsInSnapshotsEnabled(true) + .build(), Mockito.mock(FirestoreRpc.class)); @Captor private ArgumentCaptor commitCapture; @@ -79,13 +81,13 @@ public void updateDocument() throws Exception { com.google.firestore.v1beta1.Precondition.newBuilder().setExists(true).build(), com.google.firestore.v1beta1.Precondition.newBuilder().setExists(true).build(), com.google.firestore.v1beta1.Precondition.newBuilder() - .setUpdateTime(Timestamp.getDefaultInstance()) + .setUpdateTime(com.google.protobuf.Timestamp.getDefaultInstance()) .build(), com.google.firestore.v1beta1.Precondition.newBuilder() - .setUpdateTime(Timestamp.getDefaultInstance()) + .setUpdateTime(com.google.protobuf.Timestamp.getDefaultInstance()) .build()); - Precondition updateTime = Precondition.updatedAt(Instant.ofEpochSecond(0, 0)); + Precondition updateTime = Precondition.updatedAt(Timestamp.ofTimeSecondsAndNanos(0, 0)); batch.update(documentReference, LocalFirestoreHelper.SINGLE_FIELD_MAP); batch.update(documentReference, "foo", "bar"); @@ -96,7 +98,7 @@ public void updateDocument() throws Exception { List writes = new ArrayList<>(); for (int i = 0; i < writeResults.size(); ++i) { - assertEquals(Instant.ofEpochSecond(i, i), writeResults.get(i).getUpdateTime()); + assertEquals(Timestamp.ofTimeSecondsAndNanos(i, i), writeResults.get(i).getUpdateTime()); writes.add( update( LocalFirestoreHelper.SINGLE_FIELD_PROTO, @@ -129,7 +131,7 @@ public void setDocument() throws Exception { List writeResults = batch.commit().get(); for (int i = 0; i < writeResults.size(); ++i) { - assertEquals(Instant.ofEpochSecond(i, i), writeResults.get(i).getUpdateTime()); + assertEquals(Timestamp.ofTimeSecondsAndNanos(i, i), writeResults.get(i).getUpdateTime()); } CommitRequest commitRequest = commitCapture.getValue(); @@ -164,7 +166,7 @@ public void createDocument() throws Exception { List writes = new ArrayList<>(); for (int i = 0; i < writeResults.size(); ++i) { - assertEquals(Instant.ofEpochSecond(i, i), writeResults.get(i).getUpdateTime()); + assertEquals(Timestamp.ofTimeSecondsAndNanos(i, i), writeResults.get(i).getUpdateTime()); writes.add(create(LocalFirestoreHelper.SINGLE_FIELD_PROTO)); } @@ -183,7 +185,7 @@ public void deleteDocument() throws Exception { batch.delete(documentReference); writes.add(delete()); - batch.delete(documentReference, Precondition.updatedAt(Instant.ofEpochSecond(1, 2))); + batch.delete(documentReference, Precondition.updatedAt(Timestamp.ofTimeSecondsAndNanos(1, 2))); com.google.firestore.v1beta1.Precondition.Builder precondition = com.google.firestore.v1beta1.Precondition.newBuilder(); precondition.getUpdateTimeBuilder().setSeconds(1).setNanos(2); @@ -192,7 +194,7 @@ public void deleteDocument() throws Exception { List writeResults = batch.commit().get(); for (int i = 0; i < writeResults.size(); ++i) { - assertEquals(Instant.ofEpochSecond(i, i), writeResults.get(i).getUpdateTime()); + assertEquals(Timestamp.ofTimeSecondsAndNanos(i, i), writeResults.get(i).getUpdateTime()); } CommitRequest commitRequest = commitCapture.getValue(); 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 index c764a23048fc..0d94d3048618 100644 --- 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 @@ -26,6 +26,7 @@ import com.google.api.core.ApiFuture; import com.google.api.core.ApiFutures; +import com.google.cloud.Timestamp; import com.google.cloud.firestore.CollectionReference; import com.google.cloud.firestore.DocumentChange.Type; import com.google.cloud.firestore.DocumentReference; @@ -52,7 +53,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.Date; import java.util.HashMap; import java.util.Iterator; import java.util.List; @@ -60,6 +60,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.Nullable; import org.junit.After; @@ -67,7 +68,6 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TestName; -import org.threeten.bp.Instant; public class ITSystemTest { @@ -86,7 +86,9 @@ public class ITSystemTest { @Before public void before() { - firestore = FirestoreOptions.getDefaultInstance().getService(); + FirestoreOptions firestoreOptions = + FirestoreOptions.newBuilder().setTimestampsInSnapshotsEnabled(true).build(); + firestore = firestoreOptions.getService(); randomColl = firestore.collection( String.format("java-%s-%s", testName.getMethodName(), LocalFirestoreHelper.autoId())); @@ -219,7 +221,7 @@ public void deleteDocument() throws Exception { randomDoc.delete().get(); WriteResult writeResult = randomDoc.set(ALL_SUPPORTED_TYPES_MAP).get(); try { - randomDoc.delete(Precondition.updatedAt(Instant.ofEpochSecond(1))).get(); + randomDoc.delete(Precondition.updatedAt(Timestamp.ofTimeSecondsAndNanos(1, 0))).get(); fail(); } catch (ExecutionException e) { assertTrue(e.getMessage().contains("FAILED_PRECONDITION")); @@ -227,11 +229,11 @@ public void deleteDocument() throws Exception { writeResult = randomDoc.delete(Precondition.updatedAt(writeResult.getUpdateTime())).get(); DocumentSnapshot documentSnapshot = randomDoc.get().get(); assertFalse(documentSnapshot.exists()); - assertTrue(writeResult.getUpdateTime().getEpochSecond() > 0); + assertTrue(writeResult.getUpdateTime().getSeconds() > 0); } @Test - public void emptyQuery() throws Exception { + public void defaultQuery() throws Exception { addDocument("foo", "bar"); addDocument("foo", "bar"); @@ -243,6 +245,25 @@ public void emptyQuery() 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(); @@ -652,7 +673,7 @@ public void addAndRemoveServerTimestamps() throws ExecutionException, Interrupte .create( map("time", FieldValue.serverTimestamp(), "a", map("b", FieldValue.serverTimestamp()))) .get(); - Date time = (Date) getData().get("time"); + Timestamp time = (Timestamp) getData().get("time"); expected.put("time", time); expected.put("a", map("b", time)); assertEquals(expected, getData()); @@ -697,9 +718,9 @@ public void addAndRemoveServerTimestamps() throws ExecutionException, Interrupte assertEquals(expected, getData()); } - private Date updateTime(Map dataWithTime) + private Timestamp updateTime(Map dataWithTime) throws ExecutionException, InterruptedException { - Date time = (Date) getData().get("time"); + Timestamp time = (Timestamp) getData().get("time"); dataWithTime.put("time", time); return time; } From b7cee6079af53a886c72742e85978d8a38f83d62 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 29 May 2018 14:19:41 -0700 Subject: [PATCH 2/4] Address feedback --- .../cloud/firestore/CustomClassMapper.java | 2 +- .../cloud/firestore/DocumentSnapshot.java | 6 ++-- .../google/cloud/firestore/Precondition.java | 4 +-- .../cloud/firestore/LocalFirestoreHelper.java | 4 +-- .../cloud/firestore/it/ITSystemTest.java | 32 ++++++++----------- 5 files changed, 21 insertions(+), 27 deletions(-) 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 index d2e61644511c..5d21a438733a 100644 --- 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 index cb373ae5f340..7fd51f5d2903 100644 --- 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 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 index 8f28ed96a603..e22900e9bf36 100644 --- 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 index e9b5fa40caf7..9c57c3ec99f7 100644 --- 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,7 +699,7 @@ 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", @@ -707,7 +707,7 @@ public boolean equals(Object o) { .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 index 0d94d3048618..9ce4e602a892 100644 --- 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 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(); From 1d1a2c248168fed01a3a03bd5907f40667f53ae0 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 29 May 2018 16:58:38 -0700 Subject: [PATCH 3/4] Adding queryWithMicrosecondPrecision test --- .../cloud/firestore/it/ITSystemTest.java | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) 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 index 9ce4e602a892..7e77aba60e95 100644 --- 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 @@ -201,7 +201,7 @@ public void serverTimestamp() throws Exception { } @Test - public void updateMicrosecondTimestamp() throws Exception { + public void timestampDoesntGetTruncatedDuringUpdate() throws Exception { DocumentReference documentReference = addDocument("time", Timestamp.ofTimeSecondsAndNanos(0, 123000)); DocumentSnapshot documentSnapshot = documentReference.get().get(); @@ -265,6 +265,23 @@ public void noResults() throws Exception { assertNotNull(querySnapshot.getReadTime()); } + @Test + public void queryWithMicrosecondPrecision() throws Exception { + Timestamp microsecondTimestamp = Timestamp.ofTimeSecondsAndNanos(0, 123000); + + DocumentReference documentReference = addDocument("time", microsecondTimestamp); + DocumentSnapshot documentSnapshot = documentReference.get().get(); + + Query query = randomColl.whereEqualTo("time", microsecondTimestamp); + QuerySnapshot querySnapshot = query.get().get(); + assertEquals(1, querySnapshot.size()); + + // Using `.toDate()` truncates to millseconds, and hence the query doesn't match. + query = randomColl.whereEqualTo("time", microsecondTimestamp.toDate()); + querySnapshot = query.get().get(); + assertEquals(0, querySnapshot.size()); + } + @Test public void nestedQuery() throws Exception { randomColl = randomColl.document("foo").collection("bar"); From d50ea25fe0537a4e4bf161e0e27ab978a361ed47 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Fri, 1 Jun 2018 10:59:36 -0700 Subject: [PATCH 4/4] Updating log message to use .get() --- .../main/java/com/google/cloud/firestore/FirestoreImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreImpl.java b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreImpl.java index 525dad28718e..5301f117f1a9 100644 --- a/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreImpl.java +++ b/google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreImpl.java @@ -104,9 +104,9 @@ class FirestoreImpl implements Firestore { + "instead expect a Timestamp. For example:\n" + "\n" + "// Old:\n" - + "java.util.Date date = snapshot.getDate(\"created_at\");\n" + + "java.util.Date date = (java.util.Date) snapshot.get(\"created_at\");\n" + "// New:\n" - + "Timestamp timestamp = snapshot.getTimestamp(\"created_at\");\n" + + "Timestamp timestamp = (Timestamp) snapshot.get(\"created_at\");\n" + "java.util.Date date = timestamp.toDate();\n" + "\n" + "Please audit all existing usages of java.util.Date when you enable the new "