From 0db2b830eb95c12975cb69b945f837194f38d96c Mon Sep 17 00:00:00 2001 From: ncteisen Date: Fri, 3 Aug 2018 11:24:18 -0400 Subject: [PATCH] Refactor rendering to use template method pattern --- .../client_channel/client_channel_channelz.cc | 39 ------------------- .../client_channel/client_channel_channelz.h | 9 ++--- src/core/lib/channel/channelz.cc | 7 ++++ src/core/lib/channel/channelz.h | 20 ++++++---- 4 files changed, 23 insertions(+), 52 deletions(-) diff --git a/src/core/ext/filters/client_channel/client_channel_channelz.cc b/src/core/ext/filters/client_channel/client_channel_channelz.cc index 06b4d2ab956..7e8f59bcd33 100644 --- a/src/core/ext/filters/client_channel/client_channel_channelz.cc +++ b/src/core/ext/filters/client_channel/client_channel_channelz.cc @@ -98,45 +98,6 @@ void ClientChannelNode::PopulateChildRefs(grpc_json* json) { } } -grpc_json* ClientChannelNode::RenderJson() { - // We need to track these three json objects to build our object - grpc_json* top_level_json = grpc_json_create(GRPC_JSON_OBJECT); - grpc_json* json = top_level_json; - grpc_json* json_iterator = nullptr; - // create and fill the ref child - json_iterator = grpc_json_create_child(json_iterator, json, "ref", nullptr, - GRPC_JSON_OBJECT, false); - json = json_iterator; - json_iterator = nullptr; - json_iterator = grpc_json_add_number_string_child(json, json_iterator, - "channelId", uuid()); - // reset json iterators to top level object - json = top_level_json; - json_iterator = nullptr; - // create and fill the data child. - grpc_json* data = grpc_json_create_child(json_iterator, json, "data", nullptr, - GRPC_JSON_OBJECT, false); - json = data; - json_iterator = nullptr; - PopulateConnectivityState(json); - // populate the target. - GPR_ASSERT(target_view() != nullptr); - grpc_json_create_child(nullptr, json, "target", target_view(), - GRPC_JSON_STRING, false); - // fill in the channel trace if applicable - grpc_json* trace_json = trace()->RenderJson(); - if (trace_json != nullptr) { - trace_json->key = "trace"; // this object is named trace in channelz.proto - grpc_json_link_child(json, trace_json, nullptr); - } - // ask CallCountingHelper to populate trace and call count data. - call_counter()->PopulateCallCounts(json); - // reset to the top level - json = top_level_json; - PopulateChildRefs(json); - return top_level_json; -} - grpc_arg ClientChannelNode::CreateChannelArg() { return grpc_channel_arg_pointer_create( const_cast(GRPC_ARG_CHANNELZ_CHANNEL_NODE_CREATION_FUNC), diff --git a/src/core/ext/filters/client_channel/client_channel_channelz.h b/src/core/ext/filters/client_channel/client_channel_channelz.h index 9a3fc1d6f1a..a9e98deedb4 100644 --- a/src/core/ext/filters/client_channel/client_channel_channelz.h +++ b/src/core/ext/filters/client_channel/client_channel_channelz.h @@ -50,7 +50,10 @@ class ClientChannelNode : public ChannelNode { bool is_top_level_channel); virtual ~ClientChannelNode() {} - grpc_json* RenderJson() override; + // Overriding template methods from ChannelNode to render information that + // only ClientChannelNode knows about. + void PopulateConnectivityState(grpc_json* json) override; + void PopulateChildRefs(grpc_json* json) override; // Helper to create a channel arg to ensure this type of ChannelNode is // created. @@ -58,10 +61,6 @@ class ClientChannelNode : public ChannelNode { private: grpc_channel_element* client_channel_; - - // helpers - void PopulateConnectivityState(grpc_json* json); - void PopulateChildRefs(grpc_json* json); }; // Handles channelz bookkeeping for sockets diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc index 1ac7bd3976d..9f548500025 100644 --- a/src/core/lib/channel/channelz.cc +++ b/src/core/lib/channel/channelz.cc @@ -117,6 +117,9 @@ grpc_json* ChannelNode::RenderJson() { GRPC_JSON_OBJECT, false); json = data; json_iterator = nullptr; + // template method. Child classes may override this to add their specific + // functionality. + PopulateConnectivityState(json); // populate the target. GPR_ASSERT(target_.get() != nullptr); grpc_json_create_child(nullptr, json, "target", target_.get(), @@ -129,6 +132,10 @@ grpc_json* ChannelNode::RenderJson() { } // ask CallCountingHelper to populate trace and call count data. call_counter_.PopulateCallCounts(json); + json = top_level_json; + // template method. Child classes may override this to add their specific + // functionality. + PopulateChildRefs(json); return top_level_json; } diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h index 14edc75123b..e2cef233e6d 100644 --- a/src/core/lib/channel/channelz.h +++ b/src/core/lib/channel/channelz.h @@ -124,6 +124,18 @@ class ChannelNode : public BaseNode { grpc_json* RenderJson() override; + // template methods. RenderJSON uses these methods to render its JSON + // representation. These are virtual so that children classes may provide + // their specific mechanism for populating these parts of the channelz + // object. + // + // ChannelNode does not have a notion of connectivity state or child refs, + // so it leaves these implementations blank. + // + // This is utilizing the template method design pattern. + virtual void PopulateConnectivityState(grpc_json* json) {} + virtual void PopulateChildRefs(grpc_json* json) {} + void MarkChannelDestroyed() { GPR_ASSERT(channel_ != nullptr); channel_ = nullptr; @@ -144,14 +156,6 @@ class ChannelNode : public BaseNode { void RecordCallFailed() { call_counter_.RecordCallFailed(); } void RecordCallSucceeded() { call_counter_.RecordCallSucceeded(); } - protected: - // provides view of target for child. - char* target_view() { return target_.get(); } - // provides access to call_counter_ for child. - CallCountingHelper* call_counter() { return &call_counter_; } - // provides access to channel trace for child. - ChannelTrace* trace() { return &trace_; } - private: // to allow the channel trace test to access trace(); friend class testing::ChannelNodePeer;