Fixed the missing handling setter annotations bug

firebase-database/CHANGELOG.md

@@ -1,5 +1,5 @@
 # Unreleased
-
+* [fixed] Fixed the `@Exclude` annotation doesn't been propagated to Kotlin's corresponding bridge methods. [#5626](//github.com/firebase/firebase-android-sdk/pull/5706)
 
 # 20.3.0
 * [changed] Added Kotlin extensions (KTX) APIs from `com.google.firebase:firebase-database-ktx`

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

@@ -494,6 +494,7 @@ public BeanMapper(Class<T> clazz) {
       // getMethods/getFields only returns public methods/fields we need to traverse the
       // class hierarchy to find the appropriate setter or field.
       Class<? super T> currentClass = clazz;
+      Map<String, Method> bridgeMethods = new HashMap<>();
       do {
         // Add any setters
         for (Method method : currentClass.getDeclaredMethods()) {
@@ -504,12 +505,19 @@ public BeanMapper(Class<T> clazz) {
               if (!existingPropertyName.equals(propertyName)) {
                 throw new DatabaseException(
                     "Found setter with invalid " + "case-sensitive name: " + method.getName());
+              } else if (method.isBridge()) {
+                // We ignore bridge setters when creating a bean, but include them in the map
+                // for the purpose of the `isSetterOverride()` check
+                bridgeMethods.put(propertyName, method);
               } else {
                 Method existingSetter = setters.get(propertyName);
+                Method correspondingBridgeMethod = bridgeMethods.get(propertyName);
                 if (existingSetter == null) {
                   method.setAccessible(true);
                   setters.put(propertyName, method);
-                } else if (!isSetterOverride(method, existingSetter)) {
+                } else if (!isSetterOverride(method, existingSetter)
+                    && !(correspondingBridgeMethod != null
+                        && isSetterOverride(method, correspondingBridgeMethod))) {
                   // We require that setters with conflicting property names are
                   // overrides from a base class
                   throw new DatabaseException(
@@ -703,6 +711,10 @@ private static boolean shouldIncludeGetter(Method method) {
       if (method.getParameterTypes().length != 0) {
         return false;
       }
+      // Bridge methods
+      if (method.isBridge()) {
+        return false;
+      }
       // Excluded methods
       if (method.isAnnotationPresent(Exclude.class)) {
         return false;

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

@@ -0,0 +1,30 @@
+/*
+ * Copyright 2024 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.firebase.database
+
+interface GenericInterface<T> {
+  var value: T?
+}
+
+class GenericExcludedSetterBeanKotlin : GenericInterface<String> {
+
+  @set:Exclude
+  override var value: String? = null
+    set(value) {
+      field = "wrong_setter"
+    }
+}

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

@@ -20,6 +20,7 @@
 import static org.junit.Assert.fail;
 
 import androidx.annotation.Keep;
+import androidx.annotation.Nullable;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.firebase.database.core.utilities.encoding.CustomClassMapper;
@@ -849,6 +850,22 @@ public void setValue(String value) {
     }
   }
 
+  private static class GenericExcludedSetterBean implements GenericInterface<String> {
+    private String value = null;
+
+    @Nullable
+    @Override
+    public String getValue() {
+      return value;
+    }
+
+    @Exclude
+    @Override
+    public void setValue(@Nullable String value) {
+      this.value = "wrong setter";
+    }
+  }
+
   private abstract static class GenericTypeIndicatorSubclass<T> extends GenericTypeIndicator<T> {}
 
   private abstract static class NonGenericTypeIndicatorSubclass
@@ -2000,12 +2017,26 @@ public void genericSettersFromSubclassConflictsWithBaseClass() {
     serialize(bean);
   }
 
-  // This should work, but generics and subclassing are tricky to get right. For now we will just
-  // throw and we can add support for generics & subclassing if it becomes a high demand feature
-  @Test(expected = DatabaseException.class)
-  public void settersCanOverrideGenericSettersParsingNot() {
+  @Test
+  public void settersCanOverrideGenericSettersParsing() {
     NonConflictingGenericSetterSubBean bean =
         deserialize("{'value': 'value'}", NonConflictingGenericSetterSubBean.class);
     assertEquals("subsetter:value", bean.value);
   }
+
+  @Test
+  public void excludedOverriddenGenericSetterSetsValueNotJava() {
+    GenericExcludedSetterBean bean =
+        deserialize("{'value': 'foo'}", GenericExcludedSetterBean.class);
+    assertEquals("foo", bean.value);
+  }
+
+  // Unlike Java, in Kotlin, annotations do not get propagated to bridge methods.
+  // That's why there are 2 separate tests for Java and Kotlin
+  @Test
+  public void excludedOverriddenGenericSetterSetsValueNotKotlin() {
+    GenericExcludedSetterBeanKotlin bean =
+        deserialize("{'value': 'foo'}", GenericExcludedSetterBeanKotlin.class);
+    assertEquals("foo", bean.getValue());
+  }
 }

firebase-firestore/CHANGELOG.md

@@ -1,5 +1,5 @@
 # Unreleased
-
+* [fixed] Fixed the missing handling setter annotations bug introduced by [#5626](//github.com/firebase/firebase-android-sdk/pull/5626). [#5706](//github.com/firebase/firebase-android-sdk/pull/5706)
 
 # 24.10.2
 * [changed] Internal test improvements.

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

@@ -648,6 +648,7 @@ private static class BeanMapper<T> {
       // getMethods/getFields only returns public methods/fields we need to traverse the
       // class hierarchy to find the appropriate setter or field.
       Class<? super T> currentClass = clazz;
+      Map<String, Method> bridgeMethods = new HashMap<>();
       do {
         // Add any setters
         for (Method method : currentClass.getDeclaredMethods()) {
@@ -661,13 +662,20 @@ private static class BeanMapper<T> {
                         + currentClass.getName()
                         + " with invalid case-sensitive name: "
                         + method.getName());
+              } else if (method.isBridge()) {
+                // We ignore bridge setters when creating a bean, but include them in the map
+                // for the purpose of the `isSetterOverride()` check
+                bridgeMethods.put(propertyName, method);
               } else {
                 Method existingSetter = setters.get(propertyName);
+                Method correspondingBridgeMethod = bridgeMethods.get(propertyName);
                 if (existingSetter == null) {
                   method.setAccessible(true);
                   setters.put(propertyName, method);
                   applySetterAnnotations(method);
-                } else if (!isSetterOverride(method, existingSetter)) {
+                } else if (!isSetterOverride(method, existingSetter)
+                    && !(correspondingBridgeMethod != null
+                        && isSetterOverride(method, correspondingBridgeMethod))) {
                   // We require that setters with conflicting property names are
                   // overrides from a base class
                   if (currentClass == clazz) {
@@ -1003,6 +1011,10 @@ private static boolean shouldIncludeGetter(Method method) {
       if (method.getParameterTypes().length != 0) {
         return false;
       }
+      // Bridge methods
+      if (method.isBridge()) {
+        return false;
+      }
       // Excluded methods
       if (method.isAnnotationPresent(Exclude.class)) {
         return false;

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

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2024 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.firebase.firestore.util
+
+import com.google.firebase.firestore.Exclude
+
+interface GenericInterface<T> {
+  var value: T?
+}
+
+class GenericExcludedSetterBeanKotlin : GenericInterface<String> {
+
+  @set:Exclude
+  override var value: String? = null
+    set(value) {
+      field = "wrong_setter"
+    }
+}

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

@@ -21,6 +21,7 @@
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.fail;
 
+import androidx.annotation.Nullable;
 import com.google.firebase.firestore.DocumentId;
 import com.google.firebase.firestore.DocumentReference;
 import com.google.firebase.firestore.Exclude;
@@ -907,6 +908,22 @@ public void setValue(String value) {
     }
   }
 
+  private static class GenericExcludedSetterBean implements GenericInterface<String> {
+    private String value = null;
+
+    @Nullable
+    @Override
+    public String getValue() {
+      return value;
+    }
+
+    @Exclude
+    @Override
+    public void setValue(@Nullable String value) {
+      this.value = "wrong setter";
+    }
+  }
+
   private static <T> T deserialize(String jsonString, Class<T> clazz) {
     return deserialize(jsonString, clazz, /*docRef=*/ null);
   }
@@ -2218,18 +2235,27 @@ public void genericSettersFromSubclassConflictsWithBaseClass() {
         () -> serialize(bean));
   }
 
-  // This should work, but generics and subclassing are tricky to get right. For now we will just
-  // throw and we can add support for generics & subclassing if it becomes a high demand feature
   @Test
-  public void settersCanOverrideGenericSettersParsingNot() {
-    assertExceptionContains(
-        "Class com.google.firebase.firestore.util.MapperTest$NonConflictingGenericSetterSubBean "
-            + "has multiple setter overloads",
-        () -> {
-          NonConflictingGenericSetterSubBean bean =
-              deserialize("{'value': 'value'}", NonConflictingGenericSetterSubBean.class);
-          assertEquals("subsetter:value", bean.value);
-        });
+  public void settersCanOverrideGenericSettersParsing() {
+    NonConflictingGenericSetterSubBean bean =
+        deserialize("{'value': 'value'}", NonConflictingGenericSetterSubBean.class);
+    assertEquals("subsetter:value", bean.value);
+  }
+
+  @Test
+  public void excludedOverriddenGenericSetterSetsValueNotJava() {
+    GenericExcludedSetterBean bean =
+        deserialize("{'value': 'foo'}", GenericExcludedSetterBean.class);
+    assertEquals("foo", bean.value);
+  }
+
+  // Unlike Java, in Kotlin, annotations do not get propagated to bridge methods.
+  // That's why there are 2 separate tests for Java and Kotlin
+  @Test
+  public void excludedOverriddenGenericSetterSetsValueNotKotlin() {
+    GenericExcludedSetterBeanKotlin bean =
+        deserialize("{'value': 'foo'}", GenericExcludedSetterBeanKotlin.class);
+    assertEquals("foo", bean.getValue());
   }
 
   @Test