Merge pull request #38 from xfxyjwf/fix17

Fix a bug that causes DynamicMessage.setField() to not work for repeated enum fields.

2 files changed, 38 insertions, 3 deletions

diff --git a/java/src/main/java/com/google/protobuf/DynamicMessage.java b/java/src/main/java/com/google/protobuf/DynamicMessage.java
index 6bba8eb8..c9ce667b 100644
--- a/java/src/main/java/com/google/protobuf/DynamicMessage.java
+++ b/java/src/main/java/com/google/protobuf/DynamicMessage.java
@@ -38,6 +38,7 @@ import com.google.protobuf.Descriptors.OneofDescriptor;
 import java.io.InputStream;
 import java.io.IOException;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 
 /**
@@ -483,8 +484,13 @@ public final class DynamicMessage extends AbstractMessage {
     public Builder setField(FieldDescriptor field, Object value) {
       verifyContainingType(field);
       ensureIsMutable();
+      // TODO(xiaofeng): This check should really be put in FieldSet.setField()
+      // where all other such checks are done. However, currently
+      // FieldSet.setField() permits Integer value for enum fields probably
+      // because of some internal features we support. Should figure it out
+      // and move this check to a more appropriate place.
       if (field.getType() == FieldDescriptor.Type.ENUM) {
-        verifyEnumType(field, value);
+        ensureEnumValueDescriptor(field, value);
       }
       OneofDescriptor oneofDescriptor = field.getContainingOneof();
       if (oneofDescriptor != null) {
@@ -572,8 +578,9 @@ public final class DynamicMessage extends AbstractMessage {
       }
     }
 
-    /** Verifies that the value is EnumValueDescriptor and matchs Enum Type. */
-    private void verifyEnumType(FieldDescriptor field, Object value) {
+    /** Verifies that the value is EnumValueDescriptor and matches Enum Type. */
+    private void ensureSingularEnumValueDescriptor(
+        FieldDescriptor field, Object value) {
       if (value == null) {
         throw new NullPointerException();
       }
@@ -587,6 +594,18 @@ public final class DynamicMessage extends AbstractMessage {
       }
     }
 
+    /** Verifies the value for an enum field. */
+    private void ensureEnumValueDescriptor(
+        FieldDescriptor field, Object value) {
+      if (field.isRepeated()) {
+        for (Object item : (List) value) {
+          ensureSingularEnumValueDescriptor(field, item);
+        }
+      } else {
+         ensureSingularEnumValueDescriptor(field, value);
+      }
+    }
+
     private void ensureIsMutable() {
       if (fields.isImmutable()) {
         fields = fields.clone();
diff --git a/java/src/test/java/com/google/protobuf/DynamicMessageTest.java b/java/src/test/java/com/google/protobuf/DynamicMessageTest.java
index 2f17f0b0..55144e7c 100644
--- a/java/src/test/java/com/google/protobuf/DynamicMessageTest.java
+++ b/java/src/test/java/com/google/protobuf/DynamicMessageTest.java
@@ -30,6 +30,7 @@
 
 package com.google.protobuf;
 
+import com.google.protobuf.Descriptors.EnumDescriptor;
 import com.google.protobuf.Descriptors.FieldDescriptor;
 import com.google.protobuf.Descriptors.OneofDescriptor;
 
@@ -307,4 +308,19 @@ public class DynamicMessageTest extends TestCase {
     message = builder.build();
     assertSame(null, message.getOneofFieldDescriptor(oneof));
   }
+
+  // Regression test for a bug that makes setField() not work for repeated
+  // enum fields.
+  public void testSettersForRepeatedEnumField() throws Exception {
+    DynamicMessage.Builder builder =
+        DynamicMessage.newBuilder(TestAllTypes.getDescriptor());
+    FieldDescriptor repeatedEnumField =
+        TestAllTypes.getDescriptor().findFieldByName(
+            "repeated_nested_enum");
+    EnumDescriptor enumDescriptor = TestAllTypes.NestedEnum.getDescriptor();
+    builder.setField(repeatedEnumField, enumDescriptor.getValues());
+    DynamicMessage message = builder.build();
+    assertEquals(
+        enumDescriptor.getValues(), message.getField(repeatedEnumField));
+  }
 }