Skip to content

Commit 955d4ab

Browse files
protobuf-github-botcopybara-github
protobuf-github-bot
authored and
copybara-github
committed
Add MapFieldBuilder and change codegen to generate it and the put{field}BuilderIfAbsent method.
PiperOrigin-RevId: 563738402
1 parent de87585 commit 955d4ab

File tree

7 files changed

+391
-125
lines changed

7 files changed

+391
-125
lines changed

‎java/core/src/main/java/com/google/protobuf/MapFieldBuilder.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@ public class MapFieldBuilder<
5858
/** nullable */
5959
Map<KeyT, MessageT> messageMap = null;
6060

61-
// messageList elements are always MapEntry<KeyT, MessageT>, but we need a List<Message> for
62-
// reflection.
61+
// We need a List<Message> for reflection.
62+
//
63+
// messageList elements are always MapEntry<KeyT, SomeT extends Message>, where SomeT and MessageT
64+
// have the same descriptor (i.e. SomeT can be DynamicMessage)
6365
/** nullable */
6466
List<Message> messageList = null;
6567

@@ -80,8 +82,15 @@ public MapFieldBuilder(Converter<KeyT, MessageOrBuilderT, MessageT> converter) {
8082
@SuppressWarnings("unchecked")
8183
private List<MapEntry<KeyT, MessageT>> getMapEntryList() {
8284
ArrayList<MapEntry<KeyT, MessageT>> list = new ArrayList<>(messageList.size());
85+
Class<?> valueClass = converter.defaultEntry().getValue().getClass();
8386
for (Message entry : messageList) {
84-
list.add((MapEntry<KeyT, MessageT>) entry);
87+
MapEntry<KeyT, ?> typedEntry = (MapEntry<KeyT, ?>) entry;
88+
if (valueClass.isInstance(typedEntry.getValue())) {
89+
list.add((MapEntry<KeyT, MessageT>) typedEntry);
90+
} else {
91+
// This needs to use mergeFrom to allow MapEntry<KeyT, DynamicMessage> to be used.
92+
list.add(converter.defaultEntry().toBuilder().mergeFrom(entry).build());
93+
}
8594
}
8695
return list;
8796
}

‎java/core/src/test/java/com/google/protobuf/MapForProto2Test.java

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ public void testMutableMapLifecycle() {
333333
assertThat(builder.build().getInt32ToInt32Field()).isEqualTo(newMap(1, 2));
334334
try {
335335
intMap.put(2, 3);
336-
assertWithMessage("expected exception").fail();
336+
assertWithMessage("expected exception intMap").fail();
337337
} catch (UnsupportedOperationException e) {
338338
// expected
339339
}
@@ -347,7 +347,7 @@ public void testMutableMapLifecycle() {
347347
.isEqualTo(newMap(1, TestMap.EnumValue.BAR));
348348
try {
349349
enumMap.put(2, TestMap.EnumValue.FOO);
350-
assertWithMessage("expected exception").fail();
350+
assertWithMessage("expected exception enumMap").fail();
351351
} catch (UnsupportedOperationException e) {
352352
// expected
353353
}
@@ -361,27 +361,24 @@ public void testMutableMapLifecycle() {
361361
assertThat(builder.build().getInt32ToStringField()).isEqualTo(newMap(1, "1"));
362362
try {
363363
stringMap.put(2, "2");
364-
assertWithMessage("expected exception").fail();
364+
assertWithMessage("expected exception stringMap").fail();
365365
} catch (UnsupportedOperationException e) {
366366
// expected
367367
}
368368
assertThat(builder.getInt32ToStringField()).isEqualTo(newMap(1, "1"));
369369
builder.getMutableInt32ToStringField().put(2, "2");
370370
assertThat(builder.getInt32ToStringField()).isEqualTo(newMap(1, "1", 2, "2"));
371371

372+
// Message maps are handled differently, and don't freeze old mutable collections.
372373
Map<Integer, TestMap.MessageValue> messageMap = builder.getMutableInt32ToMessageField();
373374
messageMap.put(1, TestMap.MessageValue.getDefaultInstance());
374375
assertThat(builder.build().getInt32ToMessageField())
375376
.isEqualTo(newMap(1, TestMap.MessageValue.getDefaultInstance()));
376-
try {
377-
messageMap.put(2, TestMap.MessageValue.getDefaultInstance());
378-
assertWithMessage("expected exception").fail();
379-
} catch (UnsupportedOperationException e) {
380-
// expected
381-
}
382-
assertThat(builder.getInt32ToMessageField())
383-
.isEqualTo(newMap(1, TestMap.MessageValue.getDefaultInstance()));
384-
builder.getMutableInt32ToMessageField().put(2, TestMap.MessageValue.getDefaultInstance());
377+
// Mutations on old mutable maps don't affect the builder state.
378+
messageMap.put(2, TestMap.MessageValue.getDefaultInstance());
379+
assertThat(builder.getInt32ToMessageField()).isEqualTo(
380+
newMap(1, TestMap.MessageValue.getDefaultInstance()));
381+
builder.putInt32ToMessageField(2, TestMap.MessageValue.getDefaultInstance());
385382
assertThat(builder.getInt32ToMessageField()).isEqualTo(
386383
newMap(1, TestMap.MessageValue.getDefaultInstance(),
387384
2, TestMap.MessageValue.getDefaultInstance()));

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

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ public void testMutableMapLifecycle() {
332332
assertThat(builder.build().getInt32ToInt32Field()).isEqualTo(newMap(1, 2));
333333
try {
334334
intMap.put(2, 3);
335-
assertWithMessage("expected exception").fail();
335+
assertWithMessage("expected exception intMap").fail();
336336
} catch (UnsupportedOperationException e) {
337337
// expected
338338
}
@@ -346,7 +346,7 @@ public void testMutableMapLifecycle() {
346346
.isEqualTo(newMap(1, TestMap.EnumValue.BAR));
347347
try {
348348
enumMap.put(2, TestMap.EnumValue.FOO);
349-
assertWithMessage("expected exception").fail();
349+
assertWithMessage("expected exception enumMap").fail();
350350
} catch (UnsupportedOperationException e) {
351351
// expected
352352
}
@@ -360,26 +360,23 @@ public void testMutableMapLifecycle() {
360360
assertThat(builder.build().getInt32ToStringField()).isEqualTo(newMap(1, "1"));
361361
try {
362362
stringMap.put(2, "2");
363-
assertWithMessage("expected exception").fail();
363+
assertWithMessage("expected exception stringMap").fail();
364364
} catch (UnsupportedOperationException e) {
365365
// expected
366366
}
367367
assertThat(builder.getInt32ToStringField()).isEqualTo(newMap(1, "1"));
368368
builder.putInt32ToStringField(2, "2");
369369
assertThat(builder.getInt32ToStringField()).isEqualTo(newMap(1, "1", 2, "2"));
370370

371+
// Message maps are handled differently, and don't freeze old mutable collections.
371372
Map<Integer, TestMap.MessageValue> messageMap = builder.getMutableInt32ToMessageField();
372373
messageMap.put(1, TestMap.MessageValue.getDefaultInstance());
373-
assertThat( builder.build().getInt32ToMessageField())
374-
.isEqualTo(newMap(1, TestMap.MessageValue.getDefaultInstance()));
375-
try {
376-
messageMap.put(2, TestMap.MessageValue.getDefaultInstance());
377-
assertWithMessage("expected exception").fail();
378-
} catch (UnsupportedOperationException e) {
379-
// expected
380-
}
381-
assertThat(builder.getInt32ToMessageField())
374+
assertThat(builder.build().getInt32ToMessageField())
382375
.isEqualTo(newMap(1, TestMap.MessageValue.getDefaultInstance()));
376+
// Mutations on old mutable maps don't affect the builder state.
377+
messageMap.put(2, TestMap.MessageValue.getDefaultInstance());
378+
assertThat(builder.getInt32ToMessageField()).isEqualTo(
379+
newMap(1, TestMap.MessageValue.getDefaultInstance()));
383380
builder.putInt32ToMessageField(2, TestMap.MessageValue.getDefaultInstance());
384381
assertThat(builder.getInt32ToMessageField()).isEqualTo(
385382
newMap(1, TestMap.MessageValue.getDefaultInstance(),

0 commit comments

Comments
 (0)