Refactor rendering to use template method pattern

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 06b4d2ab95..7e8f59bcd3 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<char*>(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 9a3fc1d6f1..a9e98deedb 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 1ac7bd3976..9f54850002 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 14edc75123..e2cef233e6 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;