Clarify docs on MapFieldReflectionAccessorInternal and add tests for the mutability of lists returned from `getAllFields()`

PiperOrigin-RevId: 604508075

java/core/src/main/java/com/google/protobuf/GeneratedMessage.java

@@ -33,6 +33,7 @@ import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.SortedMap;
 import java.util.TreeMap;
 
 /**
@@ -135,7 +136,9 @@ public abstract class GeneratedMessage extends AbstractMessage implements Serial
    */
   private Map<FieldDescriptor, Object> getAllFieldsMutable(boolean getBytesForString) {
     final TreeMap<FieldDescriptor, Object> result = new TreeMap<>();
-    final Descriptor descriptor = internalGetFieldAccessorTable().descriptor;
+    final FieldAccessorTable fieldAccessorTable = internalGetFieldAccessorTable();
+
+    final Descriptor descriptor = fieldAccessorTable.descriptor;
     final List<FieldDescriptor> fields = descriptor.getFields();
 
     for (int i = 0; i < fields.size(); i++) {
@@ -561,7 +564,8 @@ public abstract class GeneratedMessage extends AbstractMessage implements Serial
     /** Internal helper which returns a mutable map. */
     private Map<FieldDescriptor, Object> getAllFieldsMutable() {
       final TreeMap<FieldDescriptor, Object> result = new TreeMap<>();
-      final Descriptor descriptor = internalGetFieldAccessorTable().descriptor;
+      final FieldAccessorTable fieldAccessorTable = internalGetFieldAccessorTable();
+      final Descriptor descriptor = fieldAccessorTable.descriptor;
       final List<FieldDescriptor> fields = descriptor.getFields();
 
       for (int i = 0; i < fields.size(); i++) {
@@ -2057,7 +2061,6 @@ public abstract class GeneratedMessage extends AbstractMessage implements Serial
             oneofs[i] = new SyntheticOneofAccessor(descriptor, i);
           }
         }
-
         initialized = true;
         camelCaseNames = null;
         return this;

java/core/src/main/java/com/google/protobuf/MapFieldReflectionAccessor.java

@@ -14,7 +14,7 @@ import java.util.List;
  * reflection to access both.
  */
 public abstract class MapFieldReflectionAccessor {
-  /** Gets the content of this MapField as a read-only List. */
+  /** Gets the content of this MapField as a list of read-only values. */
   abstract List<Message> getList();
 
   /** Gets a mutable List view of this MapField. */

java/core/src/test/java/com/google/protobuf/GeneratedMessageTest.java

@@ -51,6 +51,7 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import org.junit.After;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -1934,4 +1935,40 @@ public class GeneratedMessageTest {
     assertThat(builder.getRepeatedField(REPEATED_NESTED_MESSAGE_EXTENSION, 0))
         .isEqualTo(NestedMessage.newBuilder().setBb(100).build());
   }
+
+  @Test
+  public void getAllFields_repeatedFieldsAreNotMutable() {
+    TestAllTypes testMsg =
+        TestAllTypes.newBuilder()
+            .addRepeatedInt32(1)
+            .addRepeatedInt32(2)
+            .addRepeatedNestedMessage(NestedMessage.newBuilder().setBb(111).build())
+            .build();
+
+    FieldDescriptor repeatedInt32Field =
+        TestAllTypes.getDescriptor().findFieldByNumber(TestAllTypes.REPEATED_INT32_FIELD_NUMBER);
+    FieldDescriptor repeatedMsgField =
+        TestAllTypes.getDescriptor()
+            .findFieldByNumber(TestAllTypes.REPEATED_NESTED_MESSAGE_FIELD_NUMBER);
+    Map<FieldDescriptor, Object> allFields = testMsg.getAllFields();
+    List<?> list = (List<?>) allFields.get(repeatedInt32Field);
+    assertThat(list).hasSize(2);
+    assertThrows(UnsupportedOperationException.class, list::clear);
+    list = (List<?>) allFields.get(repeatedMsgField);
+    assertThat(list).hasSize(1);
+    assertThrows(UnsupportedOperationException.class, list::clear);
+
+    TestAllTypes.Builder builder = testMsg.toBuilder();
+    allFields = builder.getAllFields();
+    list = (List<?>) allFields.get(repeatedInt32Field);
+    assertThat(list).hasSize(2);
+    assertThrows(UnsupportedOperationException.class, list::clear);
+    builder.clearField(repeatedInt32Field);
+    assertThat(list).hasSize(2);
+    list = (List<?>) allFields.get(repeatedMsgField);
+    assertThat(list).hasSize(1);
+    assertThrows(UnsupportedOperationException.class, list::clear);
+    builder.clearField(repeatedMsgField);
+    assertThat(list).hasSize(1);
+  }
 }

java/core/src/test/java/com/google/protobuf/MapTest.java

@@ -10,6 +10,7 @@ package com.google.protobuf;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.fail;
 
 import com.google.protobuf.Descriptors.Descriptor;
@@ -1592,4 +1593,38 @@ public class MapTest {
       assertThat(expected).hasMessageThat().isNotNull();
     }
   }
+
+  @Test
+  public void getAllFields_mapEntryListMutability() {
+    TestMap testMap =
+        TestMap.newBuilder()
+            .putInt32ToInt32Field(1, 11)
+            .putInt32ToInt32Field(2, 22)
+            .putInt32ToMessageField(1, TestMap.MessageValue.newBuilder().setValue(111).build())
+            .build();
+    FieldDescriptor int2IntMapField =
+        TestMap.getDescriptor().findFieldByNumber(TestMap.INT32_TO_INT32_FIELD_FIELD_NUMBER);
+    FieldDescriptor int2MessageMapField =
+        TestMap.getDescriptor().findFieldByNumber(TestMap.INT32_TO_MESSAGE_FIELD_FIELD_NUMBER);
+    Map<FieldDescriptor, Object> allFields = testMap.getAllFields();
+    List<?> mapEntries = (List<?>) allFields.get(int2IntMapField);
+    assertThat(mapEntries).hasSize(2);
+    assertThrows(UnsupportedOperationException.class, mapEntries::clear);
+    mapEntries = (List<?>) allFields.get(int2MessageMapField);
+    assertThat(mapEntries).hasSize(1);
+    assertThrows(UnsupportedOperationException.class, mapEntries::clear);
+
+    TestMap.Builder builder = testMap.toBuilder();
+    allFields = builder.getAllFields();
+    mapEntries = (List<?>) allFields.get(int2IntMapField);
+    assertThat(mapEntries).hasSize(2);
+    assertThrows(UnsupportedOperationException.class, mapEntries::clear);
+    builder.clearField(int2IntMapField);
+    assertThat(mapEntries).hasSize(2);
+    mapEntries = (List<?>) allFields.get(int2MessageMapField);
+    assertThat(mapEntries).hasSize(1);
+    assertThrows(UnsupportedOperationException.class, mapEntries::clear);
+    builder.clearField(int2MessageMapField);
+    assertThat(mapEntries).hasSize(1);
+  }
 }