You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2015/09/16 23:56:51 UTC
[3/5] mesos git commit: Integer Precision for JSON <-> Protobuf
conversions.
Integer Precision for JSON <-> Protobuf conversions.
* Changes JSON::Number to keep track of whether it is floating, signed
integral, or unsigned integral.
* Changes how JSON::Number is stringified, to ensure that stringified
doubles are parsed as JSON doubles.
* Added one test for integer precision between
String <-> JSON <-> Protobuf conversions.
* Added one test for JSON::Number comparisons.
Review: https://reviews.apache.org/r/38031
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4807db6c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4807db6c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4807db6c
Branch: refs/heads/master
Commit: 4807db6ceb7fafe59c28cbcfb31334ca7061dd59
Parents: ae9579d
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Wed Sep 16 13:40:02 2015 -0400
Committer: Joris Van Remoortere <jo...@gmail.com>
Committed: Wed Sep 16 17:45:36 2015 -0400
----------------------------------------------------------------------
.../3rdparty/stout/include/stout/json.hpp | 157 +++++++++++++++++--
.../3rdparty/stout/include/stout/protobuf.hpp | 54 ++-----
.../3rdparty/stout/tests/json_tests.cpp | 74 ++++++---
.../3rdparty/stout/tests/protobuf_tests.cpp | 82 +++++++++-
4 files changed, 289 insertions(+), 78 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/4807db6c/3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp
index f28138c..6e667a1 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp
@@ -21,6 +21,7 @@
#include <limits>
#include <map>
#include <string>
+#include <type_traits>
#include <vector>
#include <boost/type_traits/is_arithmetic.hpp>
@@ -72,11 +73,71 @@ struct String
};
+// NOTE: Due to how PicoJson parses unsigned integers, a roundtrip from Number
+// to JSON and back to Number will result in:
+// - a signed integer, if the value is less than or equal to INT64_MAX;
+// - or a double, if the value is greater than INT64_MAX.
+// See: https://github.com/kazuho/picojson/blob/rel/v1.3.0/picojson.h#L777-L781
struct Number
{
Number() : value(0) {}
- Number(double _value) : value(_value) {}
- double value;
+
+ template <typename T>
+ Number(
+ T _value,
+ typename std::enable_if<std::is_floating_point<T>::value, int>::type = 0)
+ : type(FLOATING), value(_value) {}
+
+ template <typename T>
+ Number(
+ T _value,
+ typename std::enable_if<
+ std::is_integral<T>::value && std::is_signed<T>::value,
+ int>::type = 0)
+ : type(SIGNED_INTEGER), signed_integer(_value) {}
+
+ template <typename T>
+ Number(
+ T _value,
+ typename std::enable_if<
+ std::is_integral<T>::value && std::is_unsigned<T>::value,
+ int>::type = 0)
+ : type(UNSIGNED_INTEGER), unsigned_integer(_value) {}
+
+ template <typename T>
+ T as() const
+ {
+ switch (type) {
+ case FLOATING:
+ return static_cast<T>(value);
+ case SIGNED_INTEGER:
+ return static_cast<T>(signed_integer);
+ case UNSIGNED_INTEGER:
+ return static_cast<T>(unsigned_integer);
+
+ // NOTE: By not setting a default we leverage the compiler
+ // errors when the enumeration is augmented to find all
+ // the cases we need to provide.
+ }
+ }
+
+ enum Type
+ {
+ FLOATING,
+ SIGNED_INTEGER,
+ UNSIGNED_INTEGER,
+ } type;
+
+private:
+ friend struct Value;
+ friend struct Comparator;
+ friend std::ostream& operator<<(std::ostream& stream, const Number& number);
+
+ union {
+ double value;
+ int64_t signed_integer;
+ uint64_t unsigned_integer;
+ };
};
@@ -207,6 +268,8 @@ struct Value : internal::Variant
bool contains(const Value& other) const;
private:
+ friend struct Comparator;
+
// A class which follows the visitor pattern and implements the
// containment rules described in the documentation of 'contains'.
// See 'bool Value::contains(const Value& other) const'.
@@ -257,7 +320,6 @@ inline const Value& Value::as<Value>() const
}
-
template <typename T>
Result<T> Object::find(const std::string& path) const
{
@@ -384,7 +446,53 @@ inline bool Value::ContainmentComparator::operator()(const Number& other) const
if (!self.is<Number>()) {
return false;
}
- return self.as<Number>().value == other.value;
+
+ // NOTE: For the following switch statements, we do not set a default
+ // case in order to leverage the compiler errors when the enumeration
+ // is augmented to find all the cases we need to provide.
+
+ // NOTE: Using '==' is unsafe for unsigned-signed integer comparisons.
+ // The compiler will automatically cast the signed integer to an
+ // unsigned integer. i.e. If the signed integer was negative, it
+ // might be converted to a large positive number.
+ // Using the '==' operator *is* safe for double-integer comparisons.
+
+ const Number& number = self.as<Number>();
+ switch (number.type) {
+ case Number::FLOATING:
+ switch (other.type) {
+ case Number::FLOATING:
+ return number.value == other.value;
+ case Number::SIGNED_INTEGER:
+ return number.value == other.signed_integer;
+ case Number::UNSIGNED_INTEGER:
+ return number.value == other.unsigned_integer;
+ }
+
+ case Number::SIGNED_INTEGER:
+ switch (other.type) {
+ case Number::FLOATING:
+ return number.signed_integer == other.value;
+ case Number::SIGNED_INTEGER:
+ return number.signed_integer == other.signed_integer;
+ case Number::UNSIGNED_INTEGER:
+ // See note above for why this is not a simple '==' expression.
+ return number.signed_integer >= 0 &&
+ number.as<uint64_t>() == other.unsigned_integer;
+ }
+
+ case Number::UNSIGNED_INTEGER:
+ switch (other.type) {
+ case Number::FLOATING:
+ return number.unsigned_integer == other.value;
+ case Number::SIGNED_INTEGER:
+ // See note above for why this is not a simple '==' expression.
+ return other.signed_integer >= 0 &&
+ number.unsigned_integer == other.as<uint64_t>();
+ case Number::UNSIGNED_INTEGER:
+ return number.unsigned_integer == other.unsigned_integer;
+ }
+ }
}
@@ -446,12 +554,11 @@ struct Comparator : boost::static_visitor<bool>
return false;
}
- bool operator()(const Number& number) const
+ bool operator()(const Number& other) const
{
- if (value.is<Number>()) {
- return value.as<Number>().value == number.value;
- }
- return false;
+ // Delegate to the containment comparator.
+ // See Value::ContainmentComparator::operator(Number).
+ return Value::ContainmentComparator(value)(other);
}
bool operator()(const Array& array) const
@@ -502,10 +609,32 @@ inline std::ostream& operator<<(std::ostream& out, const String& string)
inline std::ostream& operator<<(std::ostream& out, const Number& number)
{
- // Use the guaranteed accurate precision, see:
- // http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2005.pdf
- return out << std::setprecision(std::numeric_limits<double>::digits10)
- << number.value;
+ switch (number.type) {
+ case Number::FLOATING: {
+ // Prints a floating point value, with the specified precision, see:
+ // http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2005.pdf
+ // Additionally ensures that a decimal point is in the output.
+ char buffer[50] {}; // More than long enough for the specified precision.
+ snprintf(
+ buffer,
+ sizeof(buffer),
+ "%#.*g",
+ std::numeric_limits<double>::digits10,
+ number.value);
+
+ // Get rid of trailing zeroes before outputting.
+ // Otherwise, printing 1.0 would result in "1.00000000000000".
+ return out << strings::trim(buffer, strings::SUFFIX, "0");
+ }
+ case Number::SIGNED_INTEGER:
+ return out << number.signed_integer;
+ case Number::UNSIGNED_INTEGER:
+ return out << number.unsigned_integer;
+
+ // NOTE: By not setting a default we leverage the compiler
+ // errors when the enumeration is augmented to find all
+ // the cases we need to provide.
+ }
}
@@ -575,6 +704,8 @@ inline Value convert(const picojson::value& value)
array.values.push_back(convert(value));
}
return array;
+ } else if (value.is<int64_t>()) {
+ return Number(value.get<int64_t>());
} else if (value.is<double>()) {
return Number(value.get<double>());
} else if (value.is<std::string>()) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/4807db6c/3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp
index 115cc0c..2285ce9 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp
@@ -391,80 +391,50 @@ struct Parser : boost::static_visitor<Try<Nothing> >
switch (field->type()) {
case google::protobuf::FieldDescriptor::TYPE_DOUBLE:
if (field->is_repeated()) {
- reflection->AddDouble(message, field, number.value);
+ reflection->AddDouble(message, field, number.as<double>());
} else {
- reflection->SetDouble(message, field, number.value);
+ reflection->SetDouble(message, field, number.as<double>());
}
break;
case google::protobuf::FieldDescriptor::TYPE_FLOAT:
if (field->is_repeated()) {
- reflection->AddFloat(
- message,
- field,
- static_cast<float>(number.value));
+ reflection->AddFloat(message, field, number.as<float>());
} else {
- reflection->SetFloat(
- message,
- field,
- static_cast<float>(number.value));
+ reflection->SetFloat(message, field, number.as<float>());
}
break;
case google::protobuf::FieldDescriptor::TYPE_INT64:
case google::protobuf::FieldDescriptor::TYPE_SINT64:
case google::protobuf::FieldDescriptor::TYPE_SFIXED64:
if (field->is_repeated()) {
- reflection->AddInt64(
- message,
- field,
- static_cast<int64_t>(number.value));
+ reflection->AddInt64(message, field, number.as<int64_t>());
} else {
- reflection->SetInt64(
- message,
- field,
- static_cast<int64_t>(number.value));
+ reflection->SetInt64(message, field, number.as<int64_t>());
}
break;
case google::protobuf::FieldDescriptor::TYPE_UINT64:
case google::protobuf::FieldDescriptor::TYPE_FIXED64:
if (field->is_repeated()) {
- reflection->AddUInt64(
- message,
- field,
- static_cast<uint64_t>(number.value));
+ reflection->AddUInt64(message, field, number.as<uint64_t>());
} else {
- reflection->SetUInt64(
- message,
- field,
- static_cast<uint64_t>(number.value));
+ reflection->SetUInt64(message, field, number.as<uint64_t>());
}
break;
case google::protobuf::FieldDescriptor::TYPE_INT32:
case google::protobuf::FieldDescriptor::TYPE_SINT32:
case google::protobuf::FieldDescriptor::TYPE_SFIXED32:
if (field->is_repeated()) {
- reflection->AddInt32(
- message,
- field,
- static_cast<int32_t>(number.value));
+ reflection->AddInt32(message, field, number.as<int32_t>());
} else {
- reflection->SetInt32(
- message,
- field,
- static_cast<int32_t>(number.value));
+ reflection->SetInt32(message, field, number.as<int32_t>());
}
break;
case google::protobuf::FieldDescriptor::TYPE_UINT32:
case google::protobuf::FieldDescriptor::TYPE_FIXED32:
if (field->is_repeated()) {
- reflection->AddUInt32(
- message,
- field,
- static_cast<uint32_t>(number.value));
+ reflection->AddUInt32(message, field, number.as<uint32_t>());
} else {
- reflection->SetUInt32(
- message,
- field,
- static_cast<uint32_t>(number.value));
+ reflection->SetUInt32(message, field, number.as<uint32_t>());
}
break;
default:
http://git-wip-us.apache.org/repos/asf/mesos/blob/4807db6c/3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp
index 850650c..88b6147 100644
--- a/3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp
+++ b/3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp
@@ -50,18 +50,50 @@ TEST(JsonTest, BinaryData)
TEST(JsonTest, NumberFormat)
{
- // Test whole numbers.
- EXPECT_EQ("0", stringify(JSON::Number(0.0)));
- EXPECT_EQ("1", stringify(JSON::Number(1.0)));
+ // Test whole numbers (as doubles).
+ EXPECT_EQ("0.", stringify(JSON::Number(0.0)));
+ EXPECT_EQ("1.", stringify(JSON::Number(1.0)));
// Negative.
- EXPECT_EQ("-1", stringify(JSON::Number(-1.0)));
+ EXPECT_EQ("-1.", stringify(JSON::Number(-1.0)));
+
+ // Test integers.
+ EXPECT_EQ("0", stringify(JSON::Number(0)));
+ EXPECT_EQ("2", stringify(JSON::Number(2)));
+ EXPECT_EQ("-2", stringify(JSON::Number(-2)));
// Expect at least 15 digits of precision.
EXPECT_EQ("1234567890.12345", stringify(JSON::Number(1234567890.12345)));
}
+TEST(JsonTest, NumberComparisons)
+{
+ // Unsigned and signed comparisons.
+ EXPECT_EQ(JSON::Number(1U), JSON::Number((int64_t) 1));
+ EXPECT_EQ(JSON::Number(0U), JSON::Number((int64_t) 0));
+
+ EXPECT_NE(JSON::Number(1U), JSON::Number(-1));
+ EXPECT_NE(JSON::Number((uint64_t) -1), JSON::Number(-1));
+
+ // Signed and unsigned comparisons (opposite order of above).
+ EXPECT_EQ(JSON::Number((int64_t) 1), JSON::Number(1U));
+ EXPECT_EQ(JSON::Number((int64_t) 0), JSON::Number(0U));
+
+ EXPECT_NE(JSON::Number(-1), JSON::Number(1U));
+ EXPECT_NE(JSON::Number(-1), JSON::Number((uint64_t) -1));
+
+ // Make sure we aren't doing an implicit cast from int64_t to uint64_t.
+ // These have the same bit representation (64h'8000_0000_0000_0001).
+ EXPECT_NE(
+ JSON::Number(9223372036854775809U),
+ JSON::Number(-9223372036854775807));
+ EXPECT_NE(
+ JSON::Number(-9223372036854775807),
+ JSON::Number(9223372036854775809U));
+}
+
+
TEST(JsonTest, BooleanFormat)
{
EXPECT_EQ("false", stringify(JSON::False()));
@@ -121,44 +153,50 @@ TEST(JsonTest, NumericAssignment)
s.st_nlink = 1;
JSON::Value v = s.st_nlink;
JSON::Number d = s.st_nlink;
- EXPECT_EQ(get<JSON::Number>(v).value, 1.0);
- EXPECT_EQ(d.value, 1.0);
+ EXPECT_NE(get<JSON::Number>(v).type, JSON::Number::FLOATING);
+ EXPECT_EQ(get<JSON::Number>(v).as<int64_t>(), 1);
+ EXPECT_EQ(d.as<int64_t>(), 1);
s.st_size = 2;
v = s.st_size;
d = s.st_size;
- EXPECT_EQ(get<JSON::Number>(v).value, 2.0);
- EXPECT_EQ(d.value, 2.0);
+ EXPECT_NE(get<JSON::Number>(v).type, JSON::Number::FLOATING);
+ EXPECT_EQ(get<JSON::Number>(v).as<int64_t>(), 2);
+ EXPECT_EQ(d.as<int64_t>(), 2);
s.st_mtime = 3;
v = s.st_mtime;
d = s.st_mtime;
- EXPECT_EQ(get<JSON::Number>(v).value, 3.0);
- EXPECT_EQ(d.value, 3.0);
+ EXPECT_EQ(get<JSON::Number>(v).as<int64_t>(), 3);
+ EXPECT_EQ(d.as<int64_t>(), 3);
size_t st = 4;
v = st;
d = st;
- EXPECT_EQ(get<JSON::Number>(v).value, 4.0);
- EXPECT_EQ(d.value, 4.0);
+ EXPECT_EQ(get<JSON::Number>(v).type, JSON::Number::UNSIGNED_INTEGER);
+ EXPECT_EQ(get<JSON::Number>(v).as<uint64_t>(), 4);
+ EXPECT_EQ(d.as<uint64_t>(), 4);
uint64_t ui64 = 5;
v = ui64;
d = ui64;
- EXPECT_EQ(get<JSON::Number>(v).value, 5.0);
- EXPECT_EQ(d.value, 5.0);
+ EXPECT_EQ(get<JSON::Number>(v).type, JSON::Number::UNSIGNED_INTEGER);
+ EXPECT_EQ(get<JSON::Number>(v).as<uint64_t>(), 5);
+ EXPECT_EQ(d.as<uint64_t>(), 5);
const unsigned int ui = 6;
v = ui;
d = ui;
- EXPECT_EQ(get<JSON::Number>(v).value, 6.0);
- EXPECT_EQ(d.value, 6.0);
+ EXPECT_EQ(get<JSON::Number>(v).type, JSON::Number::UNSIGNED_INTEGER);
+ EXPECT_EQ(get<JSON::Number>(v).as<uint64_t>(), 6);
+ EXPECT_EQ(d.as<uint64_t>(), 6);
int i = 7;
v = i;
d = i;
- EXPECT_EQ(get<JSON::Number>(v).value, 7.0);
- EXPECT_EQ(d.value, 7.0);
+ EXPECT_EQ(get<JSON::Number>(v).type, JSON::Number::SIGNED_INTEGER);
+ EXPECT_EQ(get<JSON::Number>(v).as<int64_t>(), 7);
+ EXPECT_EQ(d.as<int64_t>(), 7);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/4807db6c/3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp
index 87737dd..01d5ec7 100644
--- a/3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp
+++ b/3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp
@@ -108,18 +108,18 @@ TEST(ProtobufTest, JSON)
"{"
" \"b\": true,"
" \"bytes\": \"Ynl0ZXM=\","
- " \"d\": 1,"
+ " \"d\": 1.,"
" \"e\": \"ONE\","
- " \"f\": 1,"
+ " \"f\": 1.,"
" \"int32\": -1,"
" \"int64\": -1,"
" \"nested\": { \"str\": \"nested\"},"
- " \"optional_default\": 42,"
+ " \"optional_default\": 42.,"
" \"repeated_bool\": [true],"
" \"repeated_bytes\": [\"cmVwZWF0ZWRfYnl0ZXM=\"],"
- " \"repeated_double\": [1, 2],"
+ " \"repeated_double\": [1., 2.],"
" \"repeated_enum\": [\"TWO\"],"
- " \"repeated_float\": [1],"
+ " \"repeated_float\": [1.],"
" \"repeated_int32\": [-2],"
" \"repeated_int64\": [-2],"
" \"repeated_nested\": [ { \"str\": \"repeated_nested\" } ],"
@@ -233,3 +233,75 @@ TEST(ProtobufTest, ParseJSONArray)
EXPECT_EQ(message, repeated.Get(0));
EXPECT_EQ(message, repeated.Get(1));
}
+
+
+// Tests that integer precision is maintained between
+// JSON <-> Protobuf conversions.
+TEST(ProtobufTest, JsonLargeIntegers)
+{
+ // These numbers are equal or close to the integer limits.
+ tests::Message message;
+ message.set_int32(-2147483647);
+ message.set_int64(-9223372036854775807);
+ message.set_uint32(4294967295U);
+ message.set_uint64(9223372036854775807);
+ message.set_sint32(-1234567890);
+ message.set_sint64(-1234567890123456789);
+ message.add_repeated_int32(-2000000000);
+ message.add_repeated_int64(-9000000000000000000);
+ message.add_repeated_uint32(3000000000U);
+ message.add_repeated_uint64(7000000000000000000);
+ message.add_repeated_sint32(-1000000000);
+ message.add_repeated_sint64(-8000000000000000000);
+
+ // Parts of the protobuf that are required. Copied from the above test.
+ message.set_b(true);
+ message.set_str("string");
+ message.set_bytes("bytes");
+ message.set_f(1.0);
+ message.set_d(1.0);
+ message.set_e(tests::ONE);
+ message.mutable_nested()->set_str("nested");
+
+ // The keys are in alphabetical order.
+ string expected = strings::remove(
+ "{"
+ " \"b\": true,"
+ " \"bytes\": \"Ynl0ZXM=\","
+ " \"d\": 1.,"
+ " \"e\": \"ONE\","
+ " \"f\": 1.,"
+ " \"int32\": -2147483647,"
+ " \"int64\": -9223372036854775807,"
+ " \"nested\": {\"str\": \"nested\"},"
+ " \"optional_default\": 42.,"
+ " \"repeated_int32\": [-2000000000],"
+ " \"repeated_int64\": [-9000000000000000000],"
+ " \"repeated_sint32\": [-1000000000],"
+ " \"repeated_sint64\": [-8000000000000000000],"
+ " \"repeated_uint32\": [3000000000],"
+ " \"repeated_uint64\": [7000000000000000000],"
+ " \"sint32\": -1234567890,"
+ " \"sint64\": -1234567890123456789,"
+ " \"str\": \"string\","
+ " \"uint32\": 4294967295,"
+ " \"uint64\": 9223372036854775807"
+ "}",
+ " ");
+
+ // Check JSON -> String.
+ JSON::Object object = JSON::Protobuf(message);
+ EXPECT_EQ(expected, stringify(object));
+
+ // Check JSON -> Protobuf.
+ Try<tests::Message> parse = protobuf::parse<tests::Message>(object);
+ ASSERT_SOME(parse);
+
+ // Check Protobuf -> JSON.
+ EXPECT_EQ(object, JSON::Protobuf(parse.get()));
+
+ // Check String -> JSON.
+ Try<JSON::Object> json = JSON::parse<JSON::Object>(expected);
+ ASSERT_SOME(json);
+ EXPECT_EQ(object, json.get());
+}