From 156161dfcde38b72c55ead02cacff7087c93a4d8 Mon Sep 17 00:00:00 2001
From: Thomas Van Lenten <thomasvl@google.com>
Date: Wed, 3 Jan 2018 11:19:53 -0500
Subject: [PATCH] Properly copy maps with string keys but pod values.

Add tests to cover all the common special casing in the runtime code to
ensure things come out correctly.
---
 objectivec/GPBMessage.m              |  3 +-
 objectivec/Tests/GPBMessageTests.m   | 51 ++++++++++++++++++++++++++++
 objectivec/Tests/unittest_objc.proto | 21 ++++++++++++
 3 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/objectivec/GPBMessage.m b/objectivec/GPBMessage.m
index 90485bd1bc..3be20b678e 100644
--- a/objectivec/GPBMessage.m
+++ b/objectivec/GPBMessage.m
@@ -969,7 +969,8 @@ static GPBUnknownFieldSet *GetOrMakeUnknownFields(GPBMessage *self) {
               newValue = [value copyWithZone:zone];
             }
           } else {
-            if (field.mapKeyDataType == GPBDataTypeString) {
+            if ((field.mapKeyDataType == GPBDataTypeString) &&
+                GPBFieldDataTypeIsObject(field)) {
               // NSDictionary
               newValue = [value mutableCopyWithZone:zone];
             } else {
diff --git a/objectivec/Tests/GPBMessageTests.m b/objectivec/Tests/GPBMessageTests.m
index 4d75f1e1a6..a314909657 100644
--- a/objectivec/Tests/GPBMessageTests.m
+++ b/objectivec/Tests/GPBMessageTests.m
@@ -2052,4 +2052,55 @@
   XCTAssertEqual([msg1 hash], [msg1Prime hash]);
 }
 
+- (void)testCopyingMapFileds {
+  TestMessageOfMaps *msg = [TestMessageOfMaps message];
+
+  msg.strToStr[@"foo"] = @"bar";
+
+  [msg.strToInt setInt32:1 forKey:@"mumble"];
+  [msg.intToStr setObject:@"wee" forKey:42];
+  [msg.intToInt setInt32:123 forKey:321];
+
+  [msg.strToBool setBool:YES forKey:@"one"];
+  [msg.boolToStr setObject:@"something" forKey:YES];
+  [msg.boolToBool setBool:YES forKey:NO];
+
+  [msg.intToBool setBool:YES forKey:13];
+  [msg.boolToInt setInt32:111 forKey:NO];
+
+  TestAllTypes *subMsg1 = [TestAllTypes message];
+  subMsg1.optionalInt32 = 1;
+  TestAllTypes *subMsg2 = [TestAllTypes message];
+  subMsg1.optionalInt32 = 2;
+  TestAllTypes *subMsg3 = [TestAllTypes message];
+  subMsg1.optionalInt32 = 3;
+
+  msg.strToMsg[@"baz"] = subMsg1;
+  [msg.intToMsg setObject:subMsg2 forKey:222];
+  [msg.boolToMsg setObject:subMsg3 forKey:YES];
+
+  TestMessageOfMaps *msg2 = [[msg copy] autorelease];
+  XCTAssertNotNil(msg2);
+  XCTAssertEqualObjects(msg2, msg);
+  XCTAssertTrue(msg2 != msg);  // ptr compare
+  XCTAssertTrue(msg.strToStr != msg2.strToStr);  // ptr compare
+  XCTAssertTrue(msg.intToStr != msg2.intToStr);  // ptr compare
+  XCTAssertTrue(msg.intToInt != msg2.intToInt);  // ptr compare
+  XCTAssertTrue(msg.strToBool != msg2.strToBool);  // ptr compare
+  XCTAssertTrue(msg.boolToStr != msg2.boolToStr);  // ptr compare
+  XCTAssertTrue(msg.boolToBool != msg2.boolToBool);  // ptr compare
+  XCTAssertTrue(msg.intToBool != msg2.intToBool);  // ptr compare
+  XCTAssertTrue(msg.boolToInt != msg2.boolToInt);  // ptr compare
+  XCTAssertTrue(msg.strToMsg != msg2.strToMsg);  // ptr compare
+  XCTAssertTrue(msg.intToMsg != msg2.intToMsg);  // ptr compare
+  XCTAssertTrue(msg.boolToMsg != msg2.boolToMsg);  // ptr compare
+
+  XCTAssertTrue(msg.strToMsg[@"baz"] != msg2.strToMsg[@"baz"]);  // ptr compare
+  XCTAssertEqualObjects(msg.strToMsg[@"baz"], msg2.strToMsg[@"baz"]);
+  XCTAssertTrue([msg.intToMsg objectForKey:222] != [msg2.intToMsg objectForKey:222]);  // ptr compare
+  XCTAssertEqualObjects([msg.intToMsg objectForKey:222], [msg2.intToMsg objectForKey:222]);
+  XCTAssertTrue([msg.boolToMsg objectForKey:YES] != [msg2.boolToMsg objectForKey:YES]);  // ptr compare
+  XCTAssertEqualObjects([msg.boolToMsg objectForKey:YES], [msg2.boolToMsg objectForKey:YES]);
+}
+
 @end
diff --git a/objectivec/Tests/unittest_objc.proto b/objectivec/Tests/unittest_objc.proto
index e5577faf0d..b0eb47237e 100644
--- a/objectivec/Tests/unittest_objc.proto
+++ b/objectivec/Tests/unittest_objc.proto
@@ -58,6 +58,27 @@ message TestRecursiveMessageWithRepeatedField {
   map<string, string> str_to_str = 5;
 }
 
+// Message with a few types of maps to cover the different custom flows
+// in the runtime.
+message TestMessageOfMaps {
+  map<string, string> str_to_str = 1;
+
+  map<string, int32> str_to_int = 2;
+  map<int32, string> int_to_str = 3;
+  map<int32, int32> int_to_int = 4;
+
+  map<string, bool> str_to_bool = 5;
+  map<bool, string> bool_to_str = 6;
+  map<bool, bool> bool_to_bool = 7;
+
+  map<int32, bool> int_to_bool = 8;
+  map<bool, int32> bool_to_int = 9;
+
+  map<string, TestAllTypes> str_to_msg = 10;
+  map<int32, TestAllTypes> int_to_msg = 11;
+  map<bool, TestAllTypes> bool_to_msg = 12;
+}
+
 // Recursive message and extension to for testing autocreators at different
 // depths.
 message TestRecursiveExtension {