acelyc111 commented on code in PR #1496:
URL:
https://github.com/apache/incubator-pegasus/pull/1496#discussion_r1222514338
##########
src/runtime/test/host_port_test.cpp:
##########
@@ -202,4 +211,29 @@ TEST(host_port_test, dns_resolver)
}
}
+void send_and_check_host_port_by_different_serialize(dsn_msg_serialize_format
t)
+{
+ host_port hp = host_port("localhost", 8080);
+ auto hp_str = hp.to_string();
+ ::dsn::rpc_address server("localhost", 20101);
+
+ dsn::message_ptr mesg_ptr =
dsn::message_ex::create_request(RPC_TEST_THRIFT_HOST_PORT_PARSER);
+ mesg_ptr->header->context.u.serialize_format = t;
+
+ ::dsn::marshall(mesg_ptr.get(), hp);
+
+ dsn::task_tracker _tracker;
Review Comment:
rename `_tracker` to `tracker`.
##########
src/runtime/test/host_port_test.cpp:
##########
@@ -202,4 +211,29 @@ TEST(host_port_test, dns_resolver)
}
}
+void send_and_check_host_port_by_different_serialize(dsn_msg_serialize_format
t)
+{
+ host_port hp = host_port("localhost", 8080);
+ auto hp_str = hp.to_string();
+ ::dsn::rpc_address server("localhost", 20101);
+
+ dsn::message_ptr mesg_ptr =
dsn::message_ex::create_request(RPC_TEST_THRIFT_HOST_PORT_PARSER);
Review Comment:
how about msg_ptr? it's a common practice to shorten "message" as "msg".
##########
src/common/serialization_helper/thrift_helper.h:
##########
@@ -280,6 +281,105 @@ inline uint32_t
rpc_address::write(apache::thrift::protocol::TProtocol *oprot) c
}
}
+inline uint32_t host_port::read(apache::thrift::protocol::TProtocol *iprot)
+{
+ std::string host;
+ int16_t port;
+ int8_t type_enum_number;
+
+ uint32_t xfer = 0;
+ auto binary_proto = dynamic_cast<apache::thrift::protocol::TBinaryProtocol
*>(iprot);
+ if (binary_proto != nullptr) {
+ // the protocol is binary protocol
+ xfer += iprot->readString(host);
+ xfer += iprot->readI16(port);
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ // the protocol is json protocol
+ std::string fname;
+ xfer += iprot->readStructBegin(fname);
+
+ int16_t fid;
+ ::apache::thrift::protocol::TType ftype;
+ while (true) {
+ xfer += iprot->readFieldBegin(fname, ftype, fid);
+ if (ftype == ::apache::thrift::protocol::T_STOP) {
+ break;
+ }
+ switch (fid) {
+ case 1:
+ if (ftype == ::apache::thrift::protocol::T_STRING) {
+ xfer += iprot->readString(host);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 2:
+ if (ftype == ::apache::thrift::protocol::T_I16) {
+ xfer += iprot->readI16(port);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 3:
+ if (ftype == ::apache::thrift::protocol::T_BYTE) {
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ default:
+ xfer += iprot->skip(ftype);
+ break;
+ }
+ xfer += iprot->readFieldEnd();
+ }
+ xfer += iprot->readStructEnd();
+ }
+
+ _host = host;
+ _port = static_cast<uint16_t>(port);
+ _type = static_cast<dsn_host_type_t>(type_enum_number);
+ CHECK(_type == HOST_TYPE_INVALID || _type == HOST_TYPE_IPV4,
+ "only invalid or ipv4 can be serialized.");
+
+ return xfer;
+}
+
+inline uint32_t host_port::write(apache::thrift::protocol::TProtocol *oprot)
const
+{
+ CHECK(_type == HOST_TYPE_INVALID || _type == HOST_TYPE_IPV4,
+ "only invalid or ipv4 can be serialized.");
+ uint32_t xfer = 0;
+ auto binary_proto = dynamic_cast<apache::thrift::protocol::TBinaryProtocol
*>(oprot);
+ if (binary_proto != nullptr) {
+ // the protocol is binary protocol
+ xfer += oprot->writeString(_host);
+ xfer += oprot->writeI16(static_cast<int16_t>(_port));
+ xfer += oprot->writeByte(static_cast<int8_t>(_type));
+ } else {
+ // the protocol is json protocol
+ xfer += oprot->writeStructBegin("host_port");
+
+ xfer += oprot->writeFieldBegin("_host",
::apache::thrift::protocol::T_STRING, 1);
+ xfer += oprot->writeString(_host);
+ xfer += oprot->writeFieldEnd();
+
+ xfer += oprot->writeFieldBegin("_port",
::apache::thrift::protocol::T_I16, 2);
+ xfer += oprot->writeI16(static_cast<int16_t>(_port));
+ xfer += oprot->writeFieldEnd();
+
+ xfer += oprot->writeFieldBegin("_type",
::apache::thrift::protocol::T_BYTE, 3);
Review Comment:
```suggestion
xfer += oprot->writeFieldBegin("type",
::apache::thrift::protocol::T_BYTE, 3);
```
##########
src/common/serialization_helper/thrift_helper.h:
##########
@@ -280,6 +281,105 @@ inline uint32_t
rpc_address::write(apache::thrift::protocol::TProtocol *oprot) c
}
}
+inline uint32_t host_port::read(apache::thrift::protocol::TProtocol *iprot)
+{
+ std::string host;
+ int16_t port;
+ int8_t type_enum_number;
+
+ uint32_t xfer = 0;
+ auto binary_proto = dynamic_cast<apache::thrift::protocol::TBinaryProtocol
*>(iprot);
+ if (binary_proto != nullptr) {
+ // the protocol is binary protocol
+ xfer += iprot->readString(host);
+ xfer += iprot->readI16(port);
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ // the protocol is json protocol
+ std::string fname;
+ xfer += iprot->readStructBegin(fname);
+
+ int16_t fid;
+ ::apache::thrift::protocol::TType ftype;
+ while (true) {
+ xfer += iprot->readFieldBegin(fname, ftype, fid);
+ if (ftype == ::apache::thrift::protocol::T_STOP) {
+ break;
+ }
+ switch (fid) {
+ case 1:
+ if (ftype == ::apache::thrift::protocol::T_STRING) {
+ xfer += iprot->readString(host);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 2:
+ if (ftype == ::apache::thrift::protocol::T_I16) {
+ xfer += iprot->readI16(port);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 3:
+ if (ftype == ::apache::thrift::protocol::T_BYTE) {
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ default:
+ xfer += iprot->skip(ftype);
+ break;
+ }
+ xfer += iprot->readFieldEnd();
+ }
+ xfer += iprot->readStructEnd();
+ }
+
+ _host = host;
+ _port = static_cast<uint16_t>(port);
+ _type = static_cast<dsn_host_type_t>(type_enum_number);
+ CHECK(_type == HOST_TYPE_INVALID || _type == HOST_TYPE_IPV4,
+ "only invalid or ipv4 can be serialized.");
+
+ return xfer;
+}
+
+inline uint32_t host_port::write(apache::thrift::protocol::TProtocol *oprot)
const
+{
+ CHECK(_type == HOST_TYPE_INVALID || _type == HOST_TYPE_IPV4,
+ "only invalid or ipv4 can be serialized.");
+ uint32_t xfer = 0;
+ auto binary_proto = dynamic_cast<apache::thrift::protocol::TBinaryProtocol
*>(oprot);
+ if (binary_proto != nullptr) {
+ // the protocol is binary protocol
+ xfer += oprot->writeString(_host);
+ xfer += oprot->writeI16(static_cast<int16_t>(_port));
+ xfer += oprot->writeByte(static_cast<int8_t>(_type));
+ } else {
+ // the protocol is json protocol
+ xfer += oprot->writeStructBegin("host_port");
+
+ xfer += oprot->writeFieldBegin("_host",
::apache::thrift::protocol::T_STRING, 1);
+ xfer += oprot->writeString(_host);
+ xfer += oprot->writeFieldEnd();
+
+ xfer += oprot->writeFieldBegin("_port",
::apache::thrift::protocol::T_I16, 2);
Review Comment:
```suggestion
xfer += oprot->writeFieldBegin("port",
::apache::thrift::protocol::T_I16, 2);
```
##########
src/common/serialization_helper/thrift_helper.h:
##########
@@ -280,6 +281,105 @@ inline uint32_t
rpc_address::write(apache::thrift::protocol::TProtocol *oprot) c
}
}
+inline uint32_t host_port::read(apache::thrift::protocol::TProtocol *iprot)
+{
+ std::string host;
+ int16_t port;
+ int8_t type_enum_number;
+
+ uint32_t xfer = 0;
+ auto binary_proto = dynamic_cast<apache::thrift::protocol::TBinaryProtocol
*>(iprot);
+ if (binary_proto != nullptr) {
+ // the protocol is binary protocol
+ xfer += iprot->readString(host);
+ xfer += iprot->readI16(port);
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ // the protocol is json protocol
+ std::string fname;
+ xfer += iprot->readStructBegin(fname);
+
+ int16_t fid;
+ ::apache::thrift::protocol::TType ftype;
+ while (true) {
+ xfer += iprot->readFieldBegin(fname, ftype, fid);
+ if (ftype == ::apache::thrift::protocol::T_STOP) {
+ break;
+ }
+ switch (fid) {
+ case 1:
+ if (ftype == ::apache::thrift::protocol::T_STRING) {
+ xfer += iprot->readString(host);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 2:
+ if (ftype == ::apache::thrift::protocol::T_I16) {
+ xfer += iprot->readI16(port);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 3:
+ if (ftype == ::apache::thrift::protocol::T_BYTE) {
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ default:
+ xfer += iprot->skip(ftype);
+ break;
+ }
+ xfer += iprot->readFieldEnd();
+ }
+ xfer += iprot->readStructEnd();
+ }
+
+ _host = host;
+ _port = static_cast<uint16_t>(port);
+ _type = static_cast<dsn_host_type_t>(type_enum_number);
+ CHECK(_type == HOST_TYPE_INVALID || _type == HOST_TYPE_IPV4,
+ "only invalid or ipv4 can be serialized.");
Review Comment:
```suggestion
"only invalid or ipv4 can be deserialized.");
```
##########
src/runtime/test/host_port_test.cpp:
##########
@@ -202,4 +211,29 @@ TEST(host_port_test, dns_resolver)
}
}
+void send_and_check_host_port_by_different_serialize(dsn_msg_serialize_format
t)
+{
+ host_port hp = host_port("localhost", 8080);
Review Comment:
How about make `hp` as a parameter, and then add more tests?
##########
src/common/serialization_helper/thrift_helper.h:
##########
@@ -280,6 +281,105 @@ inline uint32_t
rpc_address::write(apache::thrift::protocol::TProtocol *oprot) c
}
}
+inline uint32_t host_port::read(apache::thrift::protocol::TProtocol *iprot)
+{
+ std::string host;
+ int16_t port;
+ int8_t type_enum_number;
+
+ uint32_t xfer = 0;
+ auto binary_proto = dynamic_cast<apache::thrift::protocol::TBinaryProtocol
*>(iprot);
+ if (binary_proto != nullptr) {
+ // the protocol is binary protocol
+ xfer += iprot->readString(host);
+ xfer += iprot->readI16(port);
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ // the protocol is json protocol
+ std::string fname;
+ xfer += iprot->readStructBegin(fname);
+
+ int16_t fid;
+ ::apache::thrift::protocol::TType ftype;
+ while (true) {
+ xfer += iprot->readFieldBegin(fname, ftype, fid);
+ if (ftype == ::apache::thrift::protocol::T_STOP) {
+ break;
+ }
+ switch (fid) {
+ case 1:
+ if (ftype == ::apache::thrift::protocol::T_STRING) {
+ xfer += iprot->readString(host);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 2:
+ if (ftype == ::apache::thrift::protocol::T_I16) {
+ xfer += iprot->readI16(port);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 3:
+ if (ftype == ::apache::thrift::protocol::T_BYTE) {
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ default:
+ xfer += iprot->skip(ftype);
+ break;
+ }
+ xfer += iprot->readFieldEnd();
+ }
+ xfer += iprot->readStructEnd();
+ }
+
+ _host = host;
+ _port = static_cast<uint16_t>(port);
+ _type = static_cast<dsn_host_type_t>(type_enum_number);
+ CHECK(_type == HOST_TYPE_INVALID || _type == HOST_TYPE_IPV4,
+ "only invalid or ipv4 can be serialized.");
+
+ return xfer;
+}
+
+inline uint32_t host_port::write(apache::thrift::protocol::TProtocol *oprot)
const
+{
+ CHECK(_type == HOST_TYPE_INVALID || _type == HOST_TYPE_IPV4,
+ "only invalid or ipv4 can be serialized.");
+ uint32_t xfer = 0;
+ auto binary_proto = dynamic_cast<apache::thrift::protocol::TBinaryProtocol
*>(oprot);
+ if (binary_proto != nullptr) {
+ // the protocol is binary protocol
+ xfer += oprot->writeString(_host);
+ xfer += oprot->writeI16(static_cast<int16_t>(_port));
+ xfer += oprot->writeByte(static_cast<int8_t>(_type));
+ } else {
+ // the protocol is json protocol
+ xfer += oprot->writeStructBegin("host_port");
+
+ xfer += oprot->writeFieldBegin("_host",
::apache::thrift::protocol::T_STRING, 1);
Review Comment:
```suggestion
xfer += oprot->writeFieldBegin("host",
::apache::thrift::protocol::T_STRING, 1);
```
##########
src/runtime/test/host_port_test.cpp:
##########
@@ -202,4 +211,29 @@ TEST(host_port_test, dns_resolver)
}
}
+void send_and_check_host_port_by_different_serialize(dsn_msg_serialize_format
t)
+{
+ host_port hp = host_port("localhost", 8080);
+ auto hp_str = hp.to_string();
+ ::dsn::rpc_address server("localhost", 20101);
+
+ dsn::message_ptr mesg_ptr =
dsn::message_ex::create_request(RPC_TEST_THRIFT_HOST_PORT_PARSER);
+ mesg_ptr->header->context.u.serialize_format = t;
+
+ ::dsn::marshall(mesg_ptr.get(), hp);
+
+ dsn::task_tracker _tracker;
+ rpc::call(server, mesg_ptr.get(), &_tracker, [hp_str](error_code ec,
std::string &&resp) {
+ if (ERR_OK == ec) {
Review Comment:
Is it always ERR_OK? add ASSERT_EQ(ERR_OK, ec) if it is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
