Permalink
Browse files

Reserve unknown in Ruby (#3763)

* Reserve unknown in ruby

* Revert ruby tests. Wait for cpp impl for conformance test

* Add conformance test for preserving unknown

* Add unknown field conformance test to csharp failure list.

* Fix comments

* Fix comment

* Fix comments

* Fix typo

* Use stringsink_string directly

* Mark hd unused

* Remove unused encodeunknown_handlerfunc
  • Loading branch information...
TeBoring committed Oct 26, 2017
1 parent a08b03d commit 23adfeb00327fbdccacc99806acb1a9a073bd8e0
View
@@ -176,3 +176,7 @@ js/testproto_libs2.js
ruby/lib/
ruby/tests/generated_code_pb.rb
ruby/tests/test_import_pb.rb
ruby/Gemfile.lock
ruby/compatibility_tests/v3.0.0/protoc
ruby/compatibility_tests/v3.0.0/tests/generated_code_pb.rb
ruby/compatibility_tests/v3.0.0/tests/test_import_pb.rb
@@ -57,10 +57,10 @@ function doTest($request)
return $response;
}
} elseif ($request->getMessageType() == "protobuf_test_messages.proto2.TestAllTypesProto2") {
$response->setSkipped("PHP doesn't support proto2");
return $response;
$response->setSkipped("PHP doesn't support proto2");
return $response;
} else {
trigger_error("Protobuf request doesn't have specific payload type", E_USER_ERROR);
trigger_error("Protobuf request doesn't have specific payload type", E_USER_ERROR);
}
} elseif ($request->getPayload() == "json_payload") {
try {
@@ -290,6 +290,28 @@ void ConformanceTestSuite::RunValidInputTest(
TextFormat::ParseFromString(equivalent_text_format, reference_message))
<< "Failed to parse data for test case: " << test_name
<< ", data: " << equivalent_text_format;
const string equivalent_wire_format = reference_message->SerializeAsString();
RunValidBinaryInputTest(test_name, level, input, input_format,
equivalent_wire_format, requested_output, isProto3);
}
void ConformanceTestSuite::RunValidBinaryInputTest(
const string& test_name, ConformanceLevel level, const string& input,
WireFormat input_format, const string& equivalent_wire_format,
WireFormat requested_output, bool isProto3) {
auto newTestMessage = [&isProto3]() {
Message* newMessage;
if (isProto3) {
newMessage = new TestAllTypesProto3;
} else {
newMessage = new TestAllTypesProto2;
}
return newMessage;
};
Message* reference_message = newTestMessage();
GOOGLE_CHECK(
reference_message->ParseFromString(equivalent_wire_format))
<< "Failed to parse wire data for test case: " << test_name;
ConformanceRequest request;
ConformanceResponse response;
@@ -493,6 +515,19 @@ void ConformanceTestSuite::RunValidProtobufTest(
}
}
void ConformanceTestSuite::RunValidBinaryProtobufTest(
const string& test_name, ConformanceLevel level,
const string& input_protobuf, bool isProto3) {
string rname = ".Proto3";
if (!isProto3) {
rname = ".Proto2";
}
RunValidBinaryInputTest(
ConformanceLevelToString(level) + rname + ".ProtobufInput." + test_name +
".ProtobufOutput", level, input_protobuf, conformance::PROTOBUF,
input_protobuf, conformance::PROTOBUF, isProto3);
}
void ConformanceTestSuite::RunValidProtobufTestWithMessage(
const string& test_name, ConformanceLevel level, const Message *input,
const string& equivalent_text_format, bool isProto3) {
@@ -811,6 +846,14 @@ void ConformanceTestSuite::TestOneofMessage (MessageType &message,
"OneofZeroEnum", RECOMMENDED, &message, "oneof_enum: FOO", isProto3);
}
template <class MessageType>
void ConformanceTestSuite::TestUnknownMessage(MessageType& message,
bool isProto3) {
message.ParseFromString("\xA8\x1F\x01");
RunValidBinaryProtobufTest("UnknownVarint", REQUIRED,
message.SerializeAsString(), isProto3);
}
bool ConformanceTestSuite::RunSuite(ConformanceTestRunner* runner,
std::string* output) {
runner_ = runner;
@@ -1847,6 +1890,14 @@ bool ConformanceTestSuite::RunSuite(ConformanceTestRunner* runner,
"StringFieldSingleQuoteBoth", RECOMMENDED,
R"({'optionalString': 'Hello world!'})");
// Unknown fields.
{
TestAllTypesProto3 messageProto3;
TestAllTypesProto2 messageProto2;
TestUnknownMessage(messageProto3, true);
TestUnknownMessage(messageProto2, false);
}
// Wrapper types.
RunValidJsonTest(
"OptionalBoolWrapper", REQUIRED,
@@ -167,6 +167,13 @@ class ConformanceTestSuite {
const string& equivalent_text_format,
conformance::WireFormat requested_output,
bool isProto3);
void RunValidBinaryInputTest(const string& test_name,
ConformanceLevel level,
const string& input,
conformance::WireFormat input_format,
const string& equivalent_wire_format,
conformance::WireFormat requested_output,
bool isProto3);
void RunValidJsonTest(const string& test_name,
ConformanceLevel level,
const string& input_json,
@@ -180,6 +187,10 @@ class ConformanceTestSuite {
const string& input_protobuf,
const string& equivalent_text_format,
bool isProto3);
void RunValidBinaryProtobufTest(const string& test_name,
ConformanceLevel level,
const string& input_protobuf,
bool isProto3);
void RunValidProtobufTestWithMessage(
const string& test_name, ConformanceLevel level,
const Message *input,
@@ -212,6 +223,9 @@ class ConformanceTestSuite {
template <class MessageType>
void TestOneofMessage (MessageType &message,
bool isProto3);
template <class MessageType>
void TestUnknownMessage (MessageType &message,
bool isProto3);
void TestValidDataForType(
google::protobuf::FieldDescriptor::Type,
std::vector<std::pair<std::string, std::string>> values);
@@ -1,2 +1,3 @@
Recommended.Proto3.JsonInput.BytesFieldBase64Url.JsonOutput
Recommended.Proto3.JsonInput.BytesFieldBase64Url.ProtobufOutput
Required.Proto3.ProtobufInput.UnknownVarint.ProtobufOutput
@@ -59,8 +59,6 @@ Required.Proto3.JsonInput.FieldMask.ProtobufOutput
Required.Proto3.JsonInput.FloatFieldInfinity.JsonOutput
Required.Proto3.JsonInput.FloatFieldNan.JsonOutput
Required.Proto3.JsonInput.FloatFieldNegativeInfinity.JsonOutput
Required.Proto3.JsonInput.FloatFieldTooLarge
Required.Proto3.JsonInput.FloatFieldTooSmall
Required.Proto3.JsonInput.OneofFieldDuplicate
Required.Proto3.JsonInput.OptionalBoolWrapper.JsonOutput
Required.Proto3.JsonInput.OptionalBoolWrapper.ProtobufOutput
@@ -44,6 +44,56 @@ VALUE noleak_rb_str_cat(VALUE rb_str, const char *str, long len) {
return rb_str;
}
// The code below also comes from upb's prototype Ruby binding, developed by
// haberman@.
/* stringsink *****************************************************************/
static void *stringsink_start(void *_sink, const void *hd, size_t size_hint) {
stringsink *sink = _sink;
sink->len = 0;
return sink;
}
static size_t stringsink_string(void *_sink, const void *hd, const char *ptr,
size_t len, const upb_bufhandle *handle) {
stringsink *sink = _sink;
size_t new_size = sink->size;
UPB_UNUSED(hd);
UPB_UNUSED(handle);
while (sink->len + len > new_size) {
new_size *= 2;
}
if (new_size != sink->size) {
sink->ptr = realloc(sink->ptr, new_size);
sink->size = new_size;
}
memcpy(sink->ptr + sink->len, ptr, len);
sink->len += len;
return len;
}
void stringsink_init(stringsink *sink) {
upb_byteshandler_init(&sink->handler);
upb_byteshandler_setstartstr(&sink->handler, stringsink_start, NULL);
upb_byteshandler_setstring(&sink->handler, stringsink_string, NULL);
upb_bytessink_reset(&sink->sink, &sink->handler, sink);
sink->size = 32;
sink->ptr = malloc(sink->size);
sink->len = 0;
}
void stringsink_uninit(stringsink *sink) {
free(sink->ptr);
}
// -----------------------------------------------------------------------------
// Parsing.
// -----------------------------------------------------------------------------
@@ -613,6 +663,20 @@ static void add_handlers_for_oneof_field(upb_handlers *h,
upb_handlerattr_uninit(&attr);
}
static bool unknown_field_handler(void* closure, const void* hd,
const char* buf, size_t size) {
UPB_UNUSED(hd);
MessageHeader* msg = (MessageHeader*)closure;
if (msg->unknown_fields == NULL) {
msg->unknown_fields = malloc(sizeof(stringsink));
stringsink_init(msg->unknown_fields);
}
stringsink_string(msg->unknown_fields, NULL, buf, size, NULL);
return true;
}
static void add_handlers_for_message(const void *closure, upb_handlers *h) {
const upb_msgdef* msgdef = upb_handlers_msgdef(h);
@@ -634,6 +698,9 @@ static void add_handlers_for_message(const void *closure, upb_handlers *h) {
desc->layout = create_layout(desc->msgdef);
}
upb_handlerattr attr = UPB_HANDLERATTR_INITIALIZER;
upb_handlers_setunknown(h, unknown_field_handler, &attr);
for (upb_msg_field_begin(&i, desc->msgdef);
!upb_msg_field_done(&i);
upb_msg_field_next(&i)) {
@@ -831,65 +898,6 @@ VALUE Message_decode_json(VALUE klass, VALUE data) {
// -----------------------------------------------------------------------------
// Serializing.
// -----------------------------------------------------------------------------
//
// The code below also comes from upb's prototype Ruby binding, developed by
// haberman@.
/* stringsink *****************************************************************/
// This should probably be factored into a common upb component.
typedef struct {
upb_byteshandler handler;
upb_bytessink sink;
char *ptr;
size_t len, size;
} stringsink;
static void *stringsink_start(void *_sink, const void *hd, size_t size_hint) {
stringsink *sink = _sink;
sink->len = 0;
return sink;
}
static size_t stringsink_string(void *_sink, const void *hd, const char *ptr,
size_t len, const upb_bufhandle *handle) {
stringsink *sink = _sink;
size_t new_size = sink->size;
UPB_UNUSED(hd);
UPB_UNUSED(handle);
while (sink->len + len > new_size) {
new_size *= 2;
}
if (new_size != sink->size) {
sink->ptr = realloc(sink->ptr, new_size);
sink->size = new_size;
}
memcpy(sink->ptr + sink->len, ptr, len);
sink->len += len;
return len;
}
void stringsink_init(stringsink *sink) {
upb_byteshandler_init(&sink->handler);
upb_byteshandler_setstartstr(&sink->handler, stringsink_start, NULL);
upb_byteshandler_setstring(&sink->handler, stringsink_string, NULL);
upb_bytessink_reset(&sink->sink, &sink->handler, sink);
sink->size = 32;
sink->ptr = malloc(sink->size);
sink->len = 0;
}
void stringsink_uninit(stringsink *sink) {
free(sink->ptr);
}
/* msgvisitor *****************************************************************/
@@ -1171,6 +1179,11 @@ static void putmsg(VALUE msg_rb, const Descriptor* desc,
}
}
stringsink* unknown = msg->unknown_fields;
if (unknown != NULL) {
upb_sink_putunknown(sink, unknown->ptr, unknown->len);
}
upb_sink_endmsg(sink, &status);
}
@@ -44,6 +44,11 @@ void Message_mark(void* _self) {
}
void Message_free(void* self) {
stringsink* unknown = ((MessageHeader *)self)->unknown_fields;
if (unknown != NULL) {
stringsink_uninit(unknown);
free(unknown);
}
xfree(self);
}
@@ -67,6 +72,8 @@ VALUE Message_alloc(VALUE klass) {
msg->descriptor = desc;
rb_ivar_set(ret, descriptor_instancevar_interned, descriptor);
msg->unknown_fields = NULL;
layout_init(desc->layout, Message_data(msg));
return ret;
@@ -475,8 +475,20 @@ VALUE layout_inspect(MessageLayout* layout, void* storage);
// Message class creation.
// -----------------------------------------------------------------------------
// This should probably be factored into a common upb component.
typedef struct {
upb_byteshandler handler;
upb_bytessink sink;
char *ptr;
size_t len, size;
} stringsink;
void stringsink_uninit(stringsink *sink);
struct MessageHeader {
Descriptor* descriptor; // kept alive by self.class.descriptor reference.
Descriptor* descriptor; // kept alive by self.class.descriptor reference.
stringsink* unknown_fields; // store unknown fields in decoding.
// Data comes after this.
};
Oops, something went wrong.

0 comments on commit 23adfeb

Please sign in to comment.