diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/CustomClassMapper.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/CustomClassMapper.java index df78fa838ca..f3230a6b1e3 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/CustomClassMapper.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/CustomClassMapper.java @@ -96,8 +96,7 @@ public static Map convertToPlainJavaTypes(Map update) */ public static T convertToCustomClass( Object object, Class clazz, DocumentReference docRef) { - // TODO(wuandy): Use DeserializeContext to encapsulate ErrorPath and docRef. - return deserializeToClass(object, clazz, ErrorPath.EMPTY); + return deserializeToClass(object, clazz, new DeserializeContext(ErrorPath.EMPTY, docRef)); } private static Object serialize(T o) { @@ -180,17 +179,18 @@ private static Object serialize(T o, ErrorPath path) { } @SuppressWarnings({"unchecked", "TypeParameterUnusedInFormals"}) - private static T deserializeToType(Object o, Type type, ErrorPath path) { + private static T deserializeToType(Object o, Type type, DeserializeContext context) { if (o == null) { return null; } else if (type instanceof ParameterizedType) { - return deserializeToParameterizedType(o, (ParameterizedType) type, path); + return deserializeToParameterizedType(o, (ParameterizedType) type, context); } else if (type instanceof Class) { - return deserializeToClass(o, (Class) type, path); + return deserializeToClass(o, (Class) type, context); } else if (type instanceof WildcardType) { Type[] lowerBounds = ((WildcardType) type).getLowerBounds(); if (lowerBounds.length > 0) { - throw deserializeError(path, "Generic lower-bounded wildcard types are not supported"); + throw deserializeError( + context.errorPath, "Generic lower-bounded wildcard types are not supported"); } // Upper bounded wildcards are of the form . Multiple upper bounds are allowed @@ -199,62 +199,63 @@ private static T deserializeToType(Object o, Type type, ErrorPath path) { // 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); + return deserializeToType(o, upperBounds[0], context); } 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); + return deserializeToType(o, upperBounds[0], context); } else if (type instanceof GenericArrayType) { - throw deserializeError(path, "Generic Arrays are not supported, please use Lists instead"); + throw deserializeError( + context.errorPath, "Generic Arrays are not supported, please use Lists instead"); } else { - throw deserializeError(path, "Unknown type encountered: " + type); + throw deserializeError(context.errorPath, "Unknown type encountered: " + type); } } @SuppressWarnings("unchecked") - private static T deserializeToClass(Object o, Class clazz, ErrorPath path) { + private static T deserializeToClass(Object o, Class clazz, DeserializeContext context) { if (o == null) { return null; } else if (clazz.isPrimitive() || Number.class.isAssignableFrom(clazz) || Boolean.class.isAssignableFrom(clazz) || Character.class.isAssignableFrom(clazz)) { - return deserializeToPrimitive(o, clazz, path); + return deserializeToPrimitive(o, clazz, context); } else if (String.class.isAssignableFrom(clazz)) { - return (T) convertString(o, path); + return (T) convertString(o, context); } else if (Date.class.isAssignableFrom(clazz)) { - return (T) convertDate(o, path); + return (T) convertDate(o, context); } else if (Timestamp.class.isAssignableFrom(clazz)) { - return (T) convertTimestamp(o, path); + return (T) convertTimestamp(o, context); } else if (Blob.class.isAssignableFrom(clazz)) { - return (T) convertBlob(o, path); + return (T) convertBlob(o, context); } else if (GeoPoint.class.isAssignableFrom(clazz)) { - return (T) convertGeoPoint(o, path); + return (T) convertGeoPoint(o, context); } else if (DocumentReference.class.isAssignableFrom(clazz)) { - return (T) convertDocumentReference(o, path); + return (T) convertDocumentReference(o, context); } else if (clazz.isArray()) { throw deserializeError( - path, "Converting to Arrays is not supported, please use Lists instead"); + context.errorPath, "Converting to Arrays is not supported, please use Lists instead"); } else if (clazz.getTypeParameters().length > 0) { throw deserializeError( - path, + context.errorPath, "Class " + clazz.getName() + " 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, path); + return deserializeToEnum(o, clazz, context); } else { - return convertBean(o, clazz, path); + return convertBean(o, clazz, context); } } @SuppressWarnings({"unchecked", "TypeParameterUnusedInFormals"}) private static T deserializeToParameterizedType( - Object o, ParameterizedType type, ErrorPath path) { + Object o, ParameterizedType type, DeserializeContext context) { // getRawType should always return a Class Class rawType = (Class) type.getRawType(); if (List.class.isAssignableFrom(rawType)) { @@ -263,32 +264,40 @@ private static T deserializeToParameterizedType( List list = (List) o; List result = new ArrayList<>(list.size()); for (int i = 0; i < list.size(); i++) { - result.add(deserializeToType(list.get(i), genericType, path.child("[" + i + "]"))); + result.add( + deserializeToType( + list.get(i), + genericType, + context.newInstanceWithErrorPath(context.errorPath.child("[" + i + "]")))); } return (T) result; } else { - throw deserializeError(path, "Expected a List, but got a " + o.getClass()); + throw deserializeError(context.errorPath, "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 deserializeError( - path, + context.errorPath, "Only Maps with string keys are supported, but found Map with key type " + keyType); } - Map map = expectMap(o, path); + Map map = expectMap(o, context); HashMap result = new HashMap<>(); for (Map.Entry entry : map.entrySet()) { result.put( entry.getKey(), - deserializeToType(entry.getValue(), valueType, path.child(entry.getKey()))); + deserializeToType( + entry.getValue(), + valueType, + context.newInstanceWithErrorPath(context.errorPath.child(entry.getKey())))); } return (T) result; } else if (Collection.class.isAssignableFrom(rawType)) { - throw deserializeError(path, "Collections are not supported, please use Lists instead"); + throw deserializeError( + context.errorPath, "Collections are not supported, please use Lists instead"); } else { - Map map = expectMap(o, path); + Map map = expectMap(o, context); BeanMapper mapper = (BeanMapper) loadOrCreateBeanMapperForClass(rawType); HashMap>, Type> typeMapping = new HashMap<>(); TypeVariable>[] typeVariables = mapper.clazz.getTypeParameters(); @@ -299,31 +308,33 @@ private static T deserializeToParameterizedType( for (int i = 0; i < typeVariables.length; i++) { typeMapping.put(typeVariables[i], types[i]); } - return mapper.deserialize(map, typeMapping, path); + return mapper.deserialize(map, typeMapping, context); } } @SuppressWarnings("unchecked") - private static T deserializeToPrimitive(Object o, Class clazz, ErrorPath path) { + private static T deserializeToPrimitive( + Object o, Class clazz, DeserializeContext context) { if (Integer.class.isAssignableFrom(clazz) || int.class.isAssignableFrom(clazz)) { - return (T) convertInteger(o, path); + return (T) convertInteger(o, context); } else if (Boolean.class.isAssignableFrom(clazz) || boolean.class.isAssignableFrom(clazz)) { - return (T) convertBoolean(o, path); + return (T) convertBoolean(o, context); } else if (Double.class.isAssignableFrom(clazz) || double.class.isAssignableFrom(clazz)) { - return (T) convertDouble(o, path); + return (T) convertDouble(o, context); } else if (Long.class.isAssignableFrom(clazz) || long.class.isAssignableFrom(clazz)) { - return (T) convertLong(o, path); + return (T) convertLong(o, context); } else if (Float.class.isAssignableFrom(clazz) || float.class.isAssignableFrom(clazz)) { - return (T) (Float) convertDouble(o, path).floatValue(); + return (T) (Float) convertDouble(o, context).floatValue(); } else { throw deserializeError( - path, + context.errorPath, String.format("Deserializing values to %s is not supported", clazz.getSimpleName())); } } @SuppressWarnings("unchecked") - private static T deserializeToEnum(Object object, Class clazz, ErrorPath path) { + private static T deserializeToEnum( + Object object, Class clazz, DeserializeContext context) { if (object instanceof String) { String value = (String) object; // We cast to Class without generics here since we can't prove the bound @@ -345,12 +356,12 @@ private static T deserializeToEnum(Object object, Class clazz, ErrorPath return (T) Enum.valueOf((Class) clazz, value); } catch (IllegalArgumentException e) { throw deserializeError( - path, + context.errorPath, "Could not find enum value of " + clazz.getName() + " for value \"" + value + "\""); } } else { throw deserializeError( - path, + context.errorPath, "Expected a String while deserializing to enum " + clazz + " but got a " @@ -371,17 +382,17 @@ private static BeanMapper loadOrCreateBeanMapperForClass(Class clazz) } @SuppressWarnings("unchecked") - private static Map expectMap(Object object, ErrorPath path) { + private static Map expectMap(Object object, DeserializeContext context) { if (object instanceof Map) { // TODO: runtime validation of keys? return (Map) object; } else { throw deserializeError( - path, "Expected a Map while deserializing, but got a " + object.getClass()); + context.errorPath, "Expected a Map while deserializing, but got a " + object.getClass()); } } - private static Integer convertInteger(Object o, ErrorPath path) { + private static Integer convertInteger(Object o, DeserializeContext context) { if (o instanceof Integer) { return (Integer) o; } else if (o instanceof Long || o instanceof Double) { @@ -390,18 +401,19 @@ private static Integer convertInteger(Object o, ErrorPath path) { return ((Number) o).intValue(); } else { throw deserializeError( - path, + context.errorPath, "Numeric value out of 32-bit integer range: " + value + ". Did you mean to use a long or double instead of an int?"); } } else { throw deserializeError( - path, "Failed to convert a value of type " + o.getClass().getName() + " to int"); + context.errorPath, + "Failed to convert a value of type " + o.getClass().getName() + " to int"); } } - private static Long convertLong(Object o, ErrorPath path) { + private static Long convertLong(Object o, DeserializeContext context) { if (o instanceof Integer) { return ((Integer) o).longValue(); } else if (o instanceof Long) { @@ -412,18 +424,19 @@ private static Long convertLong(Object o, ErrorPath path) { return value.longValue(); } else { throw deserializeError( - path, + context.errorPath, "Numeric value out of 64-bit long range: " + value + ". Did you mean to use a double instead of a long?"); } } else { throw deserializeError( - path, "Failed to convert a value of type " + o.getClass().getName() + " to long"); + context.errorPath, + "Failed to convert a value of type " + o.getClass().getName() + " to long"); } } - private static Double convertDouble(Object o, ErrorPath path) { + private static Double convertDouble(Object o, DeserializeContext context) { if (o instanceof Integer) { return ((Integer) o).doubleValue(); } else if (o instanceof Long) { @@ -433,7 +446,7 @@ private static Double convertDouble(Object o, ErrorPath path) { return doubleValue; } else { throw deserializeError( - path, + context.errorPath, "Loss of precision while converting number to " + "double: " + o @@ -443,85 +456,92 @@ private static Double convertDouble(Object o, ErrorPath path) { return (Double) o; } else { throw deserializeError( - path, "Failed to convert a value of type " + o.getClass().getName() + " to double"); + context.errorPath, + "Failed to convert a value of type " + o.getClass().getName() + " to double"); } } - private static Boolean convertBoolean(Object o, ErrorPath path) { + private static Boolean convertBoolean(Object o, DeserializeContext context) { if (o instanceof Boolean) { return (Boolean) o; } else { throw deserializeError( - path, "Failed to convert value of type " + o.getClass().getName() + " to boolean"); + context.errorPath, + "Failed to convert value of type " + o.getClass().getName() + " to boolean"); } } - private static String convertString(Object o, ErrorPath path) { + private static String convertString(Object o, DeserializeContext context) { if (o instanceof String) { return (String) o; } else { throw deserializeError( - path, "Failed to convert value of type " + o.getClass().getName() + " to String"); + context.errorPath, + "Failed to convert value of type " + o.getClass().getName() + " to String"); } } - private static Date convertDate(Object o, ErrorPath path) { + private static Date convertDate(Object o, DeserializeContext context) { 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"); + context.errorPath, + "Failed to convert value of type " + o.getClass().getName() + " to Date"); } } - private static Timestamp convertTimestamp(Object o, ErrorPath path) { + private static Timestamp convertTimestamp(Object o, DeserializeContext context) { if (o instanceof Timestamp) { return (Timestamp) o; } else if (o instanceof Date) { return new Timestamp((Date) o); } else { throw deserializeError( - path, "Failed to convert value of type " + o.getClass().getName() + " to Timestamp"); + context.errorPath, + "Failed to convert value of type " + o.getClass().getName() + " to Timestamp"); } } - private static Blob convertBlob(Object o, ErrorPath path) { + private static Blob convertBlob(Object o, DeserializeContext context) { if (o instanceof Blob) { return (Blob) o; } else { throw deserializeError( - path, "Failed to convert value of type " + o.getClass().getName() + " to Blob"); + context.errorPath, + "Failed to convert value of type " + o.getClass().getName() + " to Blob"); } } - private static GeoPoint convertGeoPoint(Object o, ErrorPath path) { + private static GeoPoint convertGeoPoint(Object o, DeserializeContext context) { if (o instanceof GeoPoint) { return (GeoPoint) o; } else { throw deserializeError( - path, "Failed to convert value of type " + o.getClass().getName() + " to GeoPoint"); + context.errorPath, + "Failed to convert value of type " + o.getClass().getName() + " to GeoPoint"); } } - private static DocumentReference convertDocumentReference(Object o, ErrorPath path) { + private static DocumentReference convertDocumentReference(Object o, DeserializeContext context) { if (o instanceof DocumentReference) { return (DocumentReference) o; } else { throw deserializeError( - path, + context.errorPath, "Failed to convert value of type " + o.getClass().getName() + " to DocumentReference"); } } - private static T convertBean(Object o, Class clazz, ErrorPath path) { + private static T convertBean(Object o, Class clazz, DeserializeContext context) { BeanMapper mapper = loadOrCreateBeanMapperForClass(clazz); if (o instanceof Map) { - return mapper.deserialize(expectMap(o, path), path); + return mapper.deserialize(expectMap(o, context), context); } else { throw deserializeError( - path, + context.errorPath, "Can't convert object of type " + o.getClass().getName() + " to type " + clazz.getName()); } } @@ -542,21 +562,36 @@ private static RuntimeException deserializeError(ErrorPath path, String reason) return new RuntimeException(reason); } + // Helper class to convert from maps to custom objects (Beans), and vice versa. private static class BeanMapper { private final Class clazz; private final Constructor constructor; + // Whether to throw exception if there are properties we don't know how to set to + // custom object fields/setters during deserialization. private final boolean throwOnUnknownProperties; + // Whether to log a message if there are properties we don't know how to set to + // custom object fields/setters during deserialization. private final boolean warnOnUnknownProperties; + // Case insensitive mapping of properties to their case sensitive versions private final Map properties; + // Below are maps to find getter/setter/field from a given property name. + // A property name is the name annotated by @PropertyName, if exists; or their property name + // following the Java Bean convention: field name is kept as-is while getters/setters will have + // their prefixes removed. See method propertyName for details. private final Map getters; private final Map setters; private final Map fields; - // A list of any properties that were annotated with @ServerTimestamp. + // A set of property names that were annotated with @ServerTimestamp. private final HashSet serverTimestamps; + // A set of property names that were annotated with @DocumentId. These properties will be + // populated with document ID values during deserialization, and be skipped during + // serialization. + private final HashSet documentIdPropertyNames; + BeanMapper(Class clazz) { this.clazz = clazz; throwOnUnknownProperties = clazz.isAnnotationPresent(ThrowOnExtraProperties.class); @@ -568,6 +603,7 @@ private static class BeanMapper { fields = new HashMap<>(); serverTimestamps = new HashSet<>(); + documentIdPropertyNames = new HashSet<>(); Constructor constructor; try { @@ -676,6 +712,18 @@ private static class BeanMapper { if (properties.isEmpty()) { throw new RuntimeException("No properties to serialize found on class " + clazz.getName()); } + + // Make sure we can write to @DocumentId annotated properties before proceeding. + for (String docIdProperty : documentIdPropertyNames) { + if (!setters.containsKey(docIdProperty) && !fields.containsKey(docIdProperty)) { + throw new RuntimeException( + "@DocumentId is annotated on property " + + docIdProperty + + " of class " + + clazz.getName() + + " but no field or public setter was found"); + } + } } private void addProperty(String property) { @@ -688,15 +736,17 @@ private void addProperty(String property) { } } - T deserialize(Map values, ErrorPath path) { - return deserialize(values, Collections.emptyMap(), path); + T deserialize(Map values, DeserializeContext context) { + return deserialize(values, Collections.emptyMap(), context); } T deserialize( - Map values, Map>, Type> types, ErrorPath path) { + Map values, + Map>, Type> types, + DeserializeContext context) { if (constructor == null) { throw deserializeError( - path, + context.errorPath, "Class " + clazz.getName() + " does not define a no-argument constructor. If you are using ProGuard, make " @@ -704,9 +754,10 @@ T deserialize( } T instance = newInstance(constructor); + HashSet deserialzedProperties = new HashSet<>(); for (Map.Entry entry : values.entrySet()) { String propertyName = entry.getKey(); - ErrorPath childPath = path.child(propertyName); + ErrorPath childPath = context.errorPath.child(propertyName); if (setters.containsKey(propertyName)) { Method setter = setters.get(propertyName); Type[] params = setter.getGenericParameterTypes(); @@ -715,18 +766,22 @@ T deserialize( } Type resolvedType = resolveType(params[0], types); Object value = - CustomClassMapper.deserializeToType(entry.getValue(), resolvedType, childPath); + CustomClassMapper.deserializeToType( + entry.getValue(), resolvedType, context.newInstanceWithErrorPath(childPath)); invoke(setter, instance, value); + deserialzedProperties.add(propertyName); } else if (fields.containsKey(propertyName)) { Field field = fields.get(propertyName); Type resolvedType = resolveType(field.getGenericType(), types); Object value = - CustomClassMapper.deserializeToType(entry.getValue(), resolvedType, childPath); + CustomClassMapper.deserializeToType( + entry.getValue(), resolvedType, context.newInstanceWithErrorPath(childPath)); try { field.set(instance, value); } catch (IllegalAccessException e) { throw new RuntimeException(e); } + deserialzedProperties.add(propertyName); } else { String message = "No setter/field for " + propertyName + " found on class " + clazz.getName(); @@ -740,9 +795,58 @@ T deserialize( } } } + populateDocumentIdProperties(types, context, instance, deserialzedProperties); + return instance; } + // Populate @DocumentId annotated fields. If there is a conflict (@DocumentId annotation is + // applied to a property that is already deserialized from the firestore document) + // a runtime exception will be thrown. + private void populateDocumentIdProperties( + Map>, Type> types, + DeserializeContext context, + T instance, + HashSet deserialzedProperties) { + for (String docIdPropertyName : documentIdPropertyNames) { + if (deserialzedProperties.contains(docIdPropertyName)) { + String message = + "'" + + docIdPropertyName + + "' was found from document " + + context.documentRef.getPath() + + ", cannot apply @DocumentId on this property for class " + + clazz.getName(); + throw new RuntimeException(message); + } + ErrorPath childPath = context.errorPath.child(docIdPropertyName); + if (setters.containsKey(docIdPropertyName)) { + Method setter = setters.get(docIdPropertyName); + Type[] params = setter.getGenericParameterTypes(); + if (params.length != 1) { + throw deserializeError(childPath, "Setter does not have exactly one parameter"); + } + Type resolvedType = resolveType(params[0], types); + if (resolvedType == String.class) { + invoke(setter, instance, context.documentRef.getId()); + } else { + invoke(setter, instance, context.documentRef); + } + } else { + Field docIdField = fields.get(docIdPropertyName); + try { + if (docIdField.getType() == String.class) { + docIdField.set(instance, context.documentRef.getId()); + } else { + docIdField.set(instance, context.documentRef); + } + } catch (IllegalAccessException e) { + throw new RuntimeException(e); + } + } + } + } + private Type resolveType(Type type, Map>, Type> types) { if (type instanceof TypeVariable) { Type resolvedType = types.get(type); @@ -767,6 +871,11 @@ Map serialize(T object, ErrorPath path) { } Map result = new HashMap<>(); for (String property : properties.values()) { + // Skip @DocumentId annotated properties; + if (documentIdPropertyNames.contains(property)) { + continue; + } + Object propertyValue; if (getters.containsKey(property)) { Method getter = getters.get(property); @@ -809,6 +918,12 @@ private void applyFieldAnnotations(Field field) { } serverTimestamps.add(propertyName(field)); } + + if (field.isAnnotationPresent(DocumentId.class)) { + Class fieldType = field.getType(); + ensureValidDocumentIdType("Field", "is", fieldType); + documentIdPropertyNames.add(propertyName(field)); + } } private void applyGetterAnnotations(Method method) { @@ -824,6 +939,13 @@ private void applyGetterAnnotations(Method method) { } serverTimestamps.add(propertyName(method)); } + + // Even though the value will be skipped, we still check for type matching for consistency. + if (method.isAnnotationPresent(DocumentId.class)) { + Class returnType = method.getReturnType(); + ensureValidDocumentIdType("Method", "returns", returnType); + documentIdPropertyNames.add(propertyName(method)); + } } private void applySetterAnnotations(Method method) { @@ -834,6 +956,24 @@ private void applySetterAnnotations(Method method) { + " is annotated with @ServerTimestamp but should not be. @ServerTimestamp can" + " only be applied to fields and getters, not setters."); } + + if (method.isAnnotationPresent(DocumentId.class)) { + Class paramType = method.getParameterTypes()[0]; + ensureValidDocumentIdType("Method", "accepts", paramType); + documentIdPropertyNames.add(propertyName(method)); + } + } + + private void ensureValidDocumentIdType(String fieldDescription, String operation, Type type) { + if (type != String.class && type != DocumentReference.class) { + throw new IllegalArgumentException( + fieldDescription + + " is annotated with @DocumentId but " + + operation + + " " + + type + + " instead of String or DocumentReference."); + } } private static boolean shouldIncludeGetter(Method method) { @@ -1015,4 +1155,23 @@ public String toString() { } } } + + /** Holds information a deserialization operation needs to complete the job. */ + static class DeserializeContext { + + /** Current path to the field being deserialized, used for better error messages. */ + final ErrorPath errorPath; + + /** Value used to set to {@link DocumentId} annotated fields during deserialization, if any. */ + final DocumentReference documentRef; + + DeserializeContext(ErrorPath path, DocumentReference docRef) { + errorPath = path; + documentRef = docRef; + } + + DeserializeContext newInstanceWithErrorPath(ErrorPath newPath) { + return new DeserializeContext(newPath, documentRef); + } + } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/DocumentId.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/DocumentId.java index 9ac8097bd0b..7eab6674889 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/DocumentId.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/DocumentId.java @@ -23,16 +23,25 @@ import java.lang.annotation.Target; /** - * Annotation used to mark a POJO field to be automatically populated with the document's ID when + * Annotation used to mark a POJO property to be automatically populated with the document's ID when * the POJO is created from a Firestore document (e.g. via {@link DocumentSnapshot#toObject}). * - *

This annotation can only be applied to fields of String or {@link DocumentReference}, + *

This annotation can only be applied to properties of String or {@link DocumentReference}, * otherwise a runtime exception will be thrown. * - *

When writing a POJO to Firestore, the @DocumentId-annotated field will be ignored, to allow + *

A run time exception will be thrown if this annotation is applied to a property that is not + * writeable (eg, a Java Bean getter without a backing field.). + * + *

If there are conflicts between this annotation and property name matches, a runtime exception + * will be thrown. For example: If a POJO has a field `firstName` annotated by @DocumentId, and + * there is a property from the document named `firstName` as well, an exception will be thrown when + * you try to read document into the POJO via {@link DocumentSnapshot#toObject} or {@link + * DocumentReference#get}. + * + *

When writing a POJO to Firestore, the @DocumentId-annotated property will be ignored, to allow * writing to any documents that are not the origin of the POJO. */ @PublicApi @Retention(RetentionPolicy.RUNTIME) -@Target({ElementType.FIELD}) +@Target({ElementType.FIELD, ElementType.METHOD}) @interface DocumentId {} diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/util/MapperTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/util/MapperTest.java index c9ca1d59f01..3cd91f34684 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/util/MapperTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/util/MapperTest.java @@ -24,6 +24,7 @@ import com.google.firebase.firestore.DocumentReference; import com.google.firebase.firestore.Exclude; import com.google.firebase.firestore.PropertyName; +import com.google.firebase.firestore.TestUtil; import com.google.firebase.firestore.ThrowOnExtraProperties; import java.io.Serializable; import java.util.ArrayList; @@ -173,6 +174,13 @@ public String getValue() { } } + @ThrowOnExtraProperties + private static class GetterBeanNoField { + public String getValue() { + return "getter:value"; + } + } + private static class GetterPublicFieldBean { public String value; @@ -899,8 +907,12 @@ public void setValue(String value) { } private static T deserialize(String jsonString, Class clazz) { + return deserialize(jsonString, clazz, /*docRef=*/ null); + } + + private static T deserialize(String jsonString, Class clazz, DocumentReference docRef) { Map json = fromSingleQuotedString(jsonString); - return CustomClassMapper.convertToCustomClass(json, clazz, null); + return CustomClassMapper.convertToCustomClass(json, clazz, docRef); } private static Object serialize(Object object) { @@ -1498,6 +1510,21 @@ public void getterOverridesField() { assertJson("{'value': 'getter:foo'}", serialize(bean)); } + @Test + public void serializeGetterBeanWithNoBackingField() { + GetterBeanNoField bean = new GetterBeanNoField(); + assertJson("{'value': 'getter:value'}", serialize(bean)); + } + + @Test + public void deserializeGetterBeanWithNoBackingFieldThrows() { + assertExceptionContains( + "No setter/field", + () -> { + deserialize("{'value': 'foo'}", GetterBeanNoField.class); + }); + } + @Test public void getterOverridesPublicField() { GetterPublicFieldBean bean = new GetterPublicFieldBean(); @@ -2247,4 +2274,189 @@ public void deserializationFailureIncludesPath() { e.getMessage()); } } + + // Bean definitions with @DocumentId applied to wrong type. + private static class FieldWithDocumentIdOnWrongTypeBean { + @DocumentId public Integer intField; + } + + private static class GetterWithDocumentIdOnWrongTypeBean { + private int intField = 100; + + @DocumentId + public int getIntField() { + return intField; + } + } + + private static class PropertyWithDocumentIdOnWrongTypeBean { + @PropertyName("intField") + @DocumentId + public int intField = 100; + } + + @Test + public void documentIdAnnotateWrongTypeThrows() { + final String expectedErrorMessage = "instead of String or DocumentReference"; + assertExceptionContains( + expectedErrorMessage, () -> serialize(new FieldWithDocumentIdOnWrongTypeBean())); + assertExceptionContains( + expectedErrorMessage, + () -> deserialize("{'intField': 1}", FieldWithDocumentIdOnWrongTypeBean.class)); + + assertExceptionContains( + expectedErrorMessage, () -> serialize(new GetterWithDocumentIdOnWrongTypeBean())); + assertExceptionContains( + expectedErrorMessage, + () -> deserialize("{'intField': 1}", GetterWithDocumentIdOnWrongTypeBean.class)); + + assertExceptionContains( + expectedErrorMessage, () -> serialize(new PropertyWithDocumentIdOnWrongTypeBean())); + assertExceptionContains( + expectedErrorMessage, + () -> deserialize("{'intField': 1}", PropertyWithDocumentIdOnWrongTypeBean.class)); + } + + private static class GetterWithoutBackingFieldOnDocumentIdBean { + @DocumentId + public String getDocId() { + return "doc-id"; + } + } + + @Test + public void documentIdAnnotateReadOnlyThrows() { + final String expectedErrorMessage = "but no field or public setter was found"; + // Serialization. + GetterWithoutBackingFieldOnDocumentIdBean bean = + new GetterWithoutBackingFieldOnDocumentIdBean(); + assertExceptionContains(expectedErrorMessage, () -> serialize(bean)); + + // Deserialization. + assertExceptionContains( + expectedErrorMessage, + () -> deserialize("{'docId': 'id'}", GetterWithoutBackingFieldOnDocumentIdBean.class)); + } + + private static class DocumentIdOnStringField { + @DocumentId public String docId = "doc-id"; + } + + private static class DocumentIdOnStringFieldAsProperty { + @PropertyName("docIdProperty") + @DocumentId + public String docId = "doc-id"; + + @PropertyName("anotherProperty") + public int someOtherProperty = 0; + } + + private static class DocumentIdOnDocRefGetter { + private DocumentReference docRef; + + @DocumentId + public DocumentReference getDocRef() { + return docRef; + } + + public void setDocRef(DocumentReference ref) { + docRef = ref; + } + } + + private static class DocumentIdOnInheritedDocRefSetter extends DocumentIdOnDocRefGetter { + + private DocumentReference inheritedDocRef; + + @DocumentId + public DocumentReference getInheritedDocRef() { + return inheritedDocRef; + } + + public void setInheritedDocRef(DocumentReference ref) { + inheritedDocRef = ref; + } + } + + private static class DocumentIdOnNestedObjects { + @PropertyName("nestedDocIdHolder") + public DocumentIdOnStringField nestedDocIdHolder; + } + + @Test + public void documentIdsDeserialize() { + DocumentReference ref = TestUtil.documentReference("coll/doc123"); + + assertEquals("doc123", deserialize("{}", DocumentIdOnStringField.class, ref).docId); + + DocumentIdOnStringFieldAsProperty target = + deserialize("{'anotherProperty': 100}", DocumentIdOnStringFieldAsProperty.class, ref); + assertEquals("doc123", target.docId); + assertEquals(100, target.someOtherProperty); + + assertEquals(ref, deserialize("{}", DocumentIdOnDocRefGetter.class, ref).getDocRef()); + + DocumentIdOnInheritedDocRefSetter target1 = + deserialize("{}", DocumentIdOnInheritedDocRefSetter.class, ref); + assertEquals(ref, target1.getInheritedDocRef()); + assertEquals(ref, target1.getDocRef()); + + assertEquals( + "doc123", + deserialize("{'nestedDocIdHolder': {}}", DocumentIdOnNestedObjects.class, ref) + .nestedDocIdHolder + .docId); + } + + @Test + public void documentIdsRoundTrip() { + // Implicitly verifies @DocumentId is ignored during serialization. + + DocumentReference ref = TestUtil.documentReference("coll/doc123"); + + assertEquals( + Collections.emptyMap(), serialize(deserialize("{}", DocumentIdOnStringField.class, ref))); + + assertEquals( + Collections.singletonMap("anotherProperty", 100), + serialize( + deserialize("{'anotherProperty': 100}", DocumentIdOnStringFieldAsProperty.class, ref))); + + assertEquals( + Collections.emptyMap(), serialize(deserialize("{}", DocumentIdOnDocRefGetter.class, ref))); + + assertEquals( + Collections.emptyMap(), + serialize(deserialize("{}", DocumentIdOnInheritedDocRefSetter.class, ref))); + + assertEquals( + Collections.singletonMap("nestedDocIdHolder", Collections.emptyMap()), + serialize(deserialize("{'nestedDocIdHolder': {}}", DocumentIdOnNestedObjects.class, ref))); + } + + @Test + public void documentIdsDeserializeConflictThrows() { + final String expectedErrorMessage = "cannot apply @DocumentId on this property"; + DocumentReference ref = TestUtil.documentReference("coll/doc123"); + + assertExceptionContains( + expectedErrorMessage, + () -> deserialize("{'docId': 'toBeOverwritten'}", DocumentIdOnStringField.class, ref)); + + assertExceptionContains( + expectedErrorMessage, + () -> + deserialize( + "{'docIdProperty': 'toBeOverwritten', 'anotherProperty': 100}", + DocumentIdOnStringFieldAsProperty.class, + ref)); + + assertExceptionContains( + expectedErrorMessage, + () -> + deserialize( + "{'nestedDocIdHolder': {'docId': 'toBeOverwritten'}}", + DocumentIdOnNestedObjects.class, + ref)); + } }