You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by qi...@apache.org on 2017/10/03 01:38:48 UTC

[1/2] mesos git commit: Added a test `ProtobufTest.ParseJSONUnrecognizedEnum`.

Repository: mesos
Updated Branches:
  refs/heads/master 772c8f554 -> 2f3ceb451


Added a test `ProtobufTest.ParseJSONUnrecognizedEnum`.

Review: https://reviews.apache.org/r/61174


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2f3ceb45
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2f3ceb45
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2f3ceb45

Branch: refs/heads/master
Commit: 2f3ceb45106e79586f2c32bfd26db0318d608075
Parents: b10a4ea
Author: Qian Zhang <zh...@gmail.com>
Authored: Thu Jul 27 16:15:44 2017 +0800
Committer: Qian Zhang <zh...@gmail.com>
Committed: Tue Oct 3 08:53:11 2017 +0800

----------------------------------------------------------------------
 3rdparty/stout/tests/protobuf_tests.cpp   | 33 ++++++++++++++++++++++++++
 3rdparty/stout/tests/protobuf_tests.proto |  9 +++++++
 2 files changed, 42 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/2f3ceb45/3rdparty/stout/tests/protobuf_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/protobuf_tests.cpp b/3rdparty/stout/tests/protobuf_tests.cpp
index 8877e89..543f96c 100644
--- a/3rdparty/stout/tests/protobuf_tests.cpp
+++ b/3rdparty/stout/tests/protobuf_tests.cpp
@@ -418,6 +418,39 @@ TEST(ProtobufTest, ParseJSONNestedError)
 }
 
 
+// Tests when parsing protobuf from JSON, for the optional enum field which
+// has an unrecognized enum value, after the parsing the field will be unset
+// and its getter will return the default enum value. For the repeated enum
+// field which contains an unrecognized enum value, after the parsing the
+// field will not contain that unrecognized value anymore.
+TEST(ProtobufTest, ParseJSONUnrecognizedEnum)
+{
+  string message =
+    "{"
+    "  \"e1\": \"XXX\","
+    "  \"e2\": \"\","
+    "  \"repeated_enum\": [\"ONE\", \"XXX\", \"\", \"TWO\"]"
+    "}";
+
+  Try<JSON::Object> json = JSON::parse<JSON::Object>(message);
+  ASSERT_SOME(json);
+
+  Try<tests::EnumMessage> parse =
+    protobuf::parse<tests::EnumMessage>(json.get());
+
+  ASSERT_SOME(parse);
+
+  EXPECT_FALSE(parse->has_e1());
+  EXPECT_EQ(tests::UNKNOWN, parse->e1());
+  EXPECT_FALSE(parse->has_e2());
+  EXPECT_EQ(tests::UNKNOWN, parse->e2());
+
+  EXPECT_EQ(2, parse->repeated_enum_size());
+  EXPECT_EQ(tests::ONE, parse->repeated_enum(0));
+  EXPECT_EQ(tests::TWO, parse->repeated_enum(1));
+}
+
+
 TEST(ProtobufTest, Jsonify)
 {
   tests::Message message;

http://git-wip-us.apache.org/repos/asf/mesos/blob/2f3ceb45/3rdparty/stout/tests/protobuf_tests.proto
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/protobuf_tests.proto b/3rdparty/stout/tests/protobuf_tests.proto
index d16726a..cf8aadc 100644
--- a/3rdparty/stout/tests/protobuf_tests.proto
+++ b/3rdparty/stout/tests/protobuf_tests.proto
@@ -24,6 +24,7 @@ package tests;
 // dynamic message at run-time.
 
 enum Enum {
+  UNKNOWN = 0;
   ONE = 1;
   TWO = 2;
 }
@@ -96,3 +97,11 @@ message Message {
 message ArrayMessage {
   repeated SimpleMessage values = 1;
 }
+
+
+// A message for testing optional enum field.
+message EnumMessage {
+  optional Enum e1 = 1;
+  optional Enum e2 = 2;
+  repeated Enum repeated_enum = 3;
+}


[2/2] mesos git commit: Fixed JSON protobuf deserialization to ignore unrecognized enum values.

Posted by qi...@apache.org.
Fixed JSON protobuf deserialization to ignore unrecognized enum values.

Protobuf deserialization will discard any unrecognized enum values.
This patch fixes our custom JSON -> protobuf conversion code to be
consistent with this behavior.

See MESOS-4997 for why this matters when dealing with upgrades.

Fixes MESOS-7828.

Review: https://reviews.apache.org/r/61109


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b10a4ea5
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b10a4ea5
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b10a4ea5

Branch: refs/heads/master
Commit: b10a4ea59231d134662d49417add2ccd7779cde7
Parents: 772c8f5
Author: Qian Zhang <zh...@gmail.com>
Authored: Tue Jul 25 23:03:43 2017 +0800
Committer: Qian Zhang <zh...@gmail.com>
Committed: Tue Oct 3 08:53:11 2017 +0800

----------------------------------------------------------------------
 3rdparty/stout/include/stout/protobuf.hpp | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/b10a4ea5/3rdparty/stout/include/stout/protobuf.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/protobuf.hpp b/3rdparty/stout/include/stout/protobuf.hpp
index 15690b6..baad126 100644
--- a/3rdparty/stout/include/stout/protobuf.hpp
+++ b/3rdparty/stout/include/stout/protobuf.hpp
@@ -451,7 +451,19 @@ struct Parser : boost::static_visitor<Try<Nothing>>
           field->enum_type()->FindValueByName(string.value);
 
         if (descriptor == nullptr) {
-          return Error("Failed to find enum for '" + string.value + "'");
+          if (field->is_required()) {
+            return Error("Failed to find enum for '" + string.value + "'");
+          }
+
+          // Unrecognized enum value will be discarded if this is not a
+          // required enum field, which makes the field's `has..` accessor
+          // return false and its getter return the first value listed in
+          // the enum definition, or the default value if one is specified.
+          //
+          // This is the deserialization behavior of proto2, see the link
+          // below for details:
+          // https://developers.google.com/protocol-buffers/docs/proto#updating
+          break;
         }
 
         if (field->is_repeated()) {