Skip to content

Commit 8478879

Browse files
cherylEnkiducherylEnkidu
committed
Syntax changes
1 parent bd3bbca commit 8478879

File tree

6 files changed

+41
-51
lines changed

6 files changed

+41
-51
lines changed

‎firebase-database/src/main/java/com/google/firebase/database/core/utilities/encoding/CustomClassMapper.java‎

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,6 @@ private static <T> T convertBean(Object o, Class<T> clazz) {
440440
}
441441

442442
private static class BeanMapper<T> {
443-
private static final String BRIDGE_METHOD_KEY_SUFFIX = "$$bridge";
444443
private final Class<T> clazz;
445444
private final Constructor<T> constructor;
446445
private final boolean throwOnUnknownProperties;
@@ -497,7 +496,8 @@ public BeanMapper(Class<T> clazz) {
497496
// getMethods/getFields only returns public methods/fields we need to traverse the
498497
// class hierarchy to find the appropriate setter or field.
499498
Class<? super T> currentClass = clazz;
500-
Set<String> propNamesOfExcludedSetters = new HashSet<>();
499+
Map<String, Method> bridgeMethods = new HashMap<>();
500+
Set<String> propertyNamesOfExcludedSetters = new HashSet<>();
501501
do {
502502
// Add any setters
503503
for (Method method : currentClass.getDeclaredMethods()) {
@@ -511,17 +511,20 @@ public BeanMapper(Class<T> clazz) {
511511
} else if (method.isBridge()) {
512512
// We ignore bridge setters when creating a bean, but include them in the map
513513
// for the purpose of the `isSetterOverride()` check
514-
setters.put(propertyName + BRIDGE_METHOD_KEY_SUFFIX, method);
514+
bridgeMethods.put(propertyName, method);
515515
} else {
516516
Method existingSetter = setters.get(propertyName);
517-
Method correspondingBridge = setters.get(propertyName + BRIDGE_METHOD_KEY_SUFFIX);
517+
Method correspondingBridgeMethod = bridgeMethods.get(propertyName);
518518
if (existingSetter == null) {
519519
setters.put(propertyName, method);
520-
if (!method.isAnnotationPresent(Exclude.class))
520+
if (!method.isAnnotationPresent(Exclude.class)) {
521521
method.setAccessible(true);
522-
else
523-
propNamesOfExcludedSetters.add(propertyName);
524-
} else if (!isSetterOverride(method, existingSetter) && !(correspondingBridge != null && isSetterOverride(method, correspondingBridge))) {
522+
} else {
523+
propertyNamesOfExcludedSetters.add(propertyName);
524+
}
525+
} else if (!isSetterOverride(method, existingSetter)
526+
&& !(correspondingBridgeMethod != null
527+
&& isSetterOverride(method, correspondingBridgeMethod))) {
525528
// We require that setters with conflicting property names are
526529
// overrides from a base class
527530
throw new DatabaseException(
@@ -556,7 +559,9 @@ public BeanMapper(Class<T> clazz) {
556559
currentClass = currentClass.getSuperclass();
557560
} while (currentClass != null && !currentClass.equals(Object.class));
558561

559-
for (String propertyName: propNamesOfExcludedSetters) {
562+
// When subclass setter is annotated with `@Exclude`, the corresponding superclass setter
563+
// also need to be filtered out.
564+
for (String propertyName: propertyNamesOfExcludedSetters) {
560565
setters.remove(propertyName);
561566
}
562567

@@ -750,16 +755,7 @@ private static boolean shouldIncludeSetter(Method method) {
750755
if (method.getParameterTypes().length != 1) {
751756
return false;
752757
}
753-
// removed the following check (although it's present in `shouldIncludeGetter()`) because bridge methods
754-
// are useful in distinguishing logic between conflicting and non-conflicting generic setters
755-
// if (method.isBridge()) {
756-
// return false;
757-
// }
758-
// removed the following check (although it's present in `shouldIncludeGetter()`) because
759-
// @Exclude-annotated methods are useful for `isSetterOverride()`
760-
// if (method.isAnnotationPresent(Exclude.class)) {
761-
// return false;
762-
// }
758+
763759
return true;
764760
}
765761

‎firebase-database/src/test/java/com/google/firebase/database/KotlinEntities.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ interface GenericInterface<T> {
2020
var value: T?
2121
}
2222

23-
class GenericExcludedSetterBeanKt : GenericInterface<String> {
23+
class GenericExcludedSetterBeanKotlin : GenericInterface<String> {
2424

2525
@set:Exclude
2626
override var value: String? = null

‎firebase-database/src/test/java/com/google/firebase/database/MapperTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2026,7 +2026,7 @@ public void settersCanOverrideGenericSettersParsing() {
20262026
}
20272027

20282028
@Test
2029-
public void excludedOverridenGenericSetterSetsValueNot() {
2029+
public void excludedOverriddenGenericSetterSetsValueNotJava() {
20302030
GenericExcludedSetterBean bean =
20312031
deserialize("{'value': 'foo'}", GenericExcludedSetterBean.class);
20322032
assertEquals("foo", bean.value);
@@ -2035,9 +2035,9 @@ public void excludedOverridenGenericSetterSetsValueNot() {
20352035
// Unlike Java, in Kotlin, annotations do not get propagated to bridge methods.
20362036
// That's why there are 2 separate tests for Java and Kotlin
20372037
@Test
2038-
public void excludedOverridenGenericSetterSetsValueNotKotlin() {
2039-
GenericExcludedSetterBeanKt bean =
2040-
deserialize("{'value': 'foo'}", GenericExcludedSetterBeanKt.class);
2038+
public void excludedOverriddenGenericSetterSetsValueNotKotlin() {
2039+
GenericExcludedSetterBeanKotlin bean =
2040+
deserialize("{'value': 'foo'}", GenericExcludedSetterBeanKotlin.class);
20412041
assertEquals("foo", bean.getValue());
20422042
}
20432043
}

‎firebase-firestore/src/main/java/com/google/firebase/firestore/util/CustomClassMapper.java‎

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,6 @@ private static RuntimeException deserializeError(ErrorPath path, String reason)
568568

569569
// Helper class to convert from maps to custom objects (Beans), and vice versa.
570570
private static class BeanMapper<T> {
571-
private static final String BRIDGE_METHOD_KEY_SUFFIX = "$$bridge";
572571
private final Class<T> clazz;
573572
private final Constructor<T> constructor;
574573
// Whether to throw exception if there are properties we don't know how to set to
@@ -650,7 +649,8 @@ private static class BeanMapper<T> {
650649
// getMethods/getFields only returns public methods/fields we need to traverse the
651650
// class hierarchy to find the appropriate setter or field.
652651
Class<? super T> currentClass = clazz;
653-
Set<String> propNamesOfExcludedSetters = new HashSet<>();
652+
Map<String, Method> bridgeMethods = new HashMap<>();
653+
Set<String> propertyNamesOfExcludedSetters = new HashSet<>();
654654
do {
655655
// Add any setters
656656
for (Method method : currentClass.getDeclaredMethods()) {
@@ -667,18 +667,20 @@ private static class BeanMapper<T> {
667667
} else if (method.isBridge()) {
668668
// We ignore bridge setters when creating a bean, but include them in the map
669669
// for the purpose of the `isSetterOverride()` check
670-
setters.put(propertyName + BRIDGE_METHOD_KEY_SUFFIX, method);
670+
bridgeMethods.put(propertyName, method);
671671
} else {
672672
Method existingSetter = setters.get(propertyName);
673-
Method correspondingBridge = setters.get(propertyName + BRIDGE_METHOD_KEY_SUFFIX);
673+
Method correspondingBridgeMethod = bridgeMethods.get(propertyName);
674674
if (existingSetter == null) {
675675
setters.put(propertyName, method);
676-
if (!method.isAnnotationPresent(Exclude.class))
676+
if (!method.isAnnotationPresent(Exclude.class)) {
677677
method.setAccessible(true);
678-
else
679-
propNamesOfExcludedSetters.add(propertyName);
680-
applySetterAnnotations(method);
681-
} else if (!isSetterOverride(method, existingSetter) && !(correspondingBridge != null && isSetterOverride(method, correspondingBridge))) {
678+
} else {
679+
propertyNamesOfExcludedSetters.add(propertyName);
680+
}
681+
} else if (!isSetterOverride(method, existingSetter)
682+
&& !(correspondingBridgeMethod != null
683+
&& isSetterOverride(method, correspondingBridgeMethod))) {
682684
// We require that setters with conflicting property names are
683685
// overrides from a base class
684686
if (currentClass == clazz) {
@@ -723,7 +725,9 @@ private static class BeanMapper<T> {
723725
currentClass = currentClass.getSuperclass();
724726
} while (currentClass != null && !currentClass.equals(Object.class));
725727

726-
for (String propertyName: propNamesOfExcludedSetters) {
728+
// When subclass setter is annotated with `@Exclude`, the corresponding superclass setter
729+
// also need to be filtered out.
730+
for (String propertyName : propertyNamesOfExcludedSetters) {
727731
setters.remove(propertyName);
728732
}
729733

@@ -1049,16 +1053,7 @@ private static boolean shouldIncludeSetter(Method method) {
10491053
if (method.getParameterTypes().length != 1) {
10501054
return false;
10511055
}
1052-
// removed the following check (although it's present in `shouldIncludeGetter()`) because bridge methods
1053-
// are useful in distinguishing logic between conflicting and non-conflicting generic setters
1054-
// if (method.isBridge()) {
1055-
// return false;
1056-
// }
1057-
// removed the following check (although it's present in `shouldIncludeGetter()`) because
1058-
// @Exclude-annotated methods are useful for `isSetterOverride()`
1059-
// if (method.isAnnotationPresent(Exclude.class)) {
1060-
// return false;
1061-
// }
1056+
10621057
return true;
10631058
}
10641059

‎firebase-firestore/src/test/java/com/google/firebase/firestore/util/KotlinEntities.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ interface GenericInterface<T> {
2222
var value: T?
2323
}
2424

25-
class GenericExcludedSetterBeanKt : GenericInterface<String> {
25+
class GenericExcludedSetterBeanKotlin : GenericInterface<String> {
2626

2727
@set:Exclude
2828
override var value: String? = null

‎firebase-firestore/src/test/java/com/google/firebase/firestore/util/MapperTest.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import static org.junit.Assert.fail;
2323

2424
import androidx.annotation.Nullable;
25-
2625
import com.google.firebase.firestore.DocumentId;
2726
import com.google.firebase.firestore.DocumentReference;
2827
import com.google.firebase.firestore.Exclude;
@@ -2244,18 +2243,18 @@ public void settersCanOverrideGenericSettersParsing() {
22442243
}
22452244

22462245
@Test
2247-
public void excludedOverridenGenericSetterSetsValueNot() {
2246+
public void excludedOverriddenGenericSetterSetsValueNotJava() {
22482247
GenericExcludedSetterBean bean =
2249-
deserialize("{'value': 'foo'}", GenericExcludedSetterBean.class);
2248+
deserialize("{'value': 'foo'}", GenericExcludedSetterBean.class);
22502249
assertEquals("foo", bean.value);
22512250
}
22522251

22532252
// Unlike Java, in Kotlin, annotations do not get propagated to bridge methods.
22542253
// That's why there are 2 separate tests for Java and Kotlin
22552254
@Test
2256-
public void excludedOverridenGenericSetterSetsValueNotKotlin() {
2257-
GenericExcludedSetterBeanKt bean =
2258-
deserialize("{'value': 'foo'}", GenericExcludedSetterBeanKt.class);
2255+
public void excludedOverriddenGenericSetterSetsValueNotKotlin() {
2256+
GenericExcludedSetterBeanKotlin bean =
2257+
deserialize("{'value': 'foo'}", GenericExcludedSetterBeanKotlin.class);
22592258
assertEquals("foo", bean.getValue());
22602259
}
22612260

0 commit comments

Comments
 (0)