Merge pull request #3366 from ceph/wip-formatter

formatter: improve pretty output, rename factory method

Reviewed-by: Dan Mick <dan.mick@redhat.com>

src/client/Client.cc

@@ -119,9 +119,7 @@ Client::CommandHook::CommandHook(Client *client) :
 bool Client::CommandHook::call(std::string command, cmdmap_t& cmdmap,
 			       std::string format, bufferlist& out)
 {
-  Formatter *f = new_formatter(format);
-  if (!f)
-    f = new_formatter("json-pretty");
+  Formatter *f = Formatter::create(format);
   f->open_object_section("result");
   m_client->client_lock.Lock();
   if (command == "mds_requests")

src/common/Formatter.cc

@@ -63,12 +63,12 @@ Formatter::Formatter() { }
 
 Formatter::~Formatter() { }
 
-Formatter *
-new_formatter(const std::string &type)
+Formatter *Formatter::create(const std::string &type,
+			     const std::string& default_type)
 {
   std::string mytype = type;
   if (mytype == "")
-    mytype = "json-pretty";
+    mytype = default_type;
 
   if (mytype == "json")
     return new JSONFormatter(false);
@@ -123,6 +123,8 @@ void JSONFormatter::flush(std::ostream& os)
 {
   finish_pending_string();
   os << m_ss.str();
+  if (m_pretty)
+    os << "\n";
   m_ss.clear();
   m_ss.str("");
 }
@@ -146,7 +148,7 @@ void JSONFormatter::print_comma(json_formatter_stack_entry_d& entry)
     } else {
       m_ss << ",";
     }
-  } else if (entry.is_array && m_pretty) {
+  } else if (m_pretty) {
     m_ss << "\n";
     for (unsigned i = 1; i < m_stack.size(); i++)
       m_ss << "    ";
@@ -173,10 +175,7 @@ void JSONFormatter::print_name(const char *name)
   print_comma(entry);
   if (!entry.is_array) {
     if (m_pretty) {
-      if (entry.size)
-        m_ss << "  ";
-      else
-        m_ss << " ";
+      m_ss << "    ";
     }
     m_ss << "\"" << name << "\"";
     if (m_pretty)
@@ -230,6 +229,11 @@ void JSONFormatter::close_section()
   finish_pending_string();
 
   struct json_formatter_stack_entry_d& entry = m_stack.back();
+  if (m_pretty && entry.size) {
+    m_ss << "\n";
+    for (unsigned i = 1; i < m_stack.size(); i++)
+      m_ss << "    ";
+  }
   m_ss << (entry.is_array ? ']' : '}');
   m_stack.pop_back();
 }
@@ -312,6 +316,8 @@ void XMLFormatter::flush(std::ostream& os)
 {
   finish_pending_string();
   os << m_ss.str();
+  if (m_pretty)
+    os << "\n";
   m_ss.clear();
   m_ss.str("");
 }

src/common/Formatter.h

@@ -1,3 +1,5 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
 #ifndef CEPH_FORMATTER_H
 #define CEPH_FORMATTER_H
 
@@ -25,6 +27,12 @@ namespace ceph {
 
   class Formatter {
   public:
+    static Formatter *create(const std::string& type,
+			     const std::string& default_type);
+    static Formatter *create(const std::string& type) {
+      return create(type, "json-pretty");
+    }
+
     Formatter();
     virtual ~Formatter();
 
@@ -72,8 +80,6 @@ namespace ceph {
     }
   };
 
-  Formatter *new_formatter(const std::string &type);
-
   class JSONFormatter : public Formatter {
   public:
     JSONFormatter(bool p = false);

src/common/admin_socket.cc

@@ -448,9 +448,7 @@ class HelpHook : public AdminSocketHook {
 public:
   HelpHook(AdminSocket *as) : m_as(as) {}
   bool call(string command, cmdmap_t &cmdmap, string format, bufferlist& out) {
-    Formatter *f = new_formatter(format);
-    if (!f)
-      f = new_formatter("json-pretty");
+    Formatter *f = Formatter::create(format);
     f->open_object_section("help");
     for (map<string,string>::iterator p = m_as->m_help.begin();
 	 p != m_as->m_help.end();

src/common/ceph_context.cc

@@ -225,9 +225,7 @@ public:
 void CephContext::do_command(std::string command, cmdmap_t& cmdmap,
 			     std::string format, bufferlist *out)
 {
-  Formatter *f = new_formatter(format);
-  if (!f)
-    f = new_formatter("json-pretty");
+  Formatter *f = Formatter::create(format);
   stringstream ss;
   for (cmdmap_t::iterator it = cmdmap.begin(); it != cmdmap.end(); ++it) {
     if (it->first != "prefix") {

src/mds/MDS.cc

@@ -221,9 +221,7 @@ bool MDS::asok_command(string command, cmdmap_t& cmdmap, string format,
 {
   dout(1) << "asok_command: " << command << " (starting...)" << dendl;
 
-  Formatter *f = new_formatter(format);
-  if (!f)
-    f = new_formatter("json-pretty");
+  Formatter *f = Formatter::create(format);
   if (command == "status") {
 
     const OSDMap *osdmap = objecter->get_osdmap_read();

src/mon/AuthMonitor.cc

@@ -557,7 +557,7 @@ bool AuthMonitor::preprocess_command(MMonCommand *m)
 
   string format;
   cmd_getval(g_ceph_context, cmdmap, "format", format, string("plain"));
-  boost::scoped_ptr<Formatter> f(new_formatter(format));
+  boost::scoped_ptr<Formatter> f(Formatter::create(format));
 
   if (prefix == "auth export") {
     KeyRing keyring;
@@ -685,7 +685,7 @@ bool AuthMonitor::prepare_command(MMonCommand *m)
 
   string format;
   cmd_getval(g_ceph_context, cmdmap, "format", format, string("plain"));
-  boost::scoped_ptr<Formatter> f(new_formatter(format));
+  boost::scoped_ptr<Formatter> f(Formatter::create(format));
 
   MonSession *session = m->get_session();
   if (!session) {

src/mon/MDSMonitor.cc

@@ -647,7 +647,7 @@ bool MDSMonitor::preprocess_command(MMonCommand *m)
   cmd_getval(g_ceph_context, cmdmap, "prefix", prefix);
   string format;
   cmd_getval(g_ceph_context, cmdmap, "format", format, string("plain"));
-  boost::scoped_ptr<Formatter> f(new_formatter(format));
+  boost::scoped_ptr<Formatter> f(Formatter::create(format));
 
   MonSession *session = m->get_session();
   if (!session) {

src/mon/Monitor.cc

@@ -267,7 +267,7 @@ void Monitor::do_admin_command(string command, cmdmap_t& cmdmap, string format,
 {
   Mutex::Locker l(lock);
 
-  boost::scoped_ptr<Formatter> f(new_formatter(format));
+  boost::scoped_ptr<Formatter> f(Formatter::create(format));
 
   string args;
   for (cmdmap_t::iterator p = cmdmap.begin();
@@ -2371,7 +2371,7 @@ void Monitor::handle_command(MMonCommand *m)
   cmd_getval(g_ceph_context, cmdmap, "prefix", prefix);
   if (prefix == "get_command_descriptions") {
     bufferlist rdata;
-    Formatter *f = new_formatter("json");
+    Formatter *f = Formatter::create("json");
     format_command_descriptions(leader_supported_mon_commands,
 				leader_supported_mon_commands_size, f, &rdata);
     delete f;
@@ -2386,7 +2386,7 @@ void Monitor::handle_command(MMonCommand *m)
 
   string format;
   cmd_getval(g_ceph_context, cmdmap, "format", format, string("plain"));
-  boost::scoped_ptr<Formatter> f(new_formatter(format));
+  boost::scoped_ptr<Formatter> f(Formatter::create(format));
 
   get_str_vec(prefix, fullcmd);
   module = fullcmd[0];
@@ -2587,7 +2587,7 @@ void Monitor::handle_command(MMonCommand *m)
 
     // this must be formatted, in its current form
     if (!f)
-      f.reset(new_formatter("json-pretty"));
+      f.reset(Formatter::create("json-pretty"));
     f->open_object_section("report");
     f->dump_stream("cluster_fingerprint") << fingerprint;
     f->dump_string("version", ceph_version_to_str());

src/mon/MonmapMonitor.cc

@@ -216,7 +216,7 @@ bool MonmapMonitor::preprocess_command(MMonCommand *m)
       string format;
       cmd_getval(g_ceph_context, cmdmap, "format", format, string("plain"));
       stringstream ds;
-      boost::scoped_ptr<Formatter> f(new_formatter(format));
+      boost::scoped_ptr<Formatter> f(Formatter::create(format));
       if (f) {
         f->open_object_section("monmap");
         p->dump(f.get());

src/mon/OSDMonitor.cc

@@ -2334,7 +2334,7 @@ bool OSDMonitor::preprocess_command(MMonCommand *m)
 
   string format;
   cmd_getval(g_ceph_context, cmdmap, "format", format, string("plain"));
-  boost::scoped_ptr<Formatter> f(new_formatter(format));
+  boost::scoped_ptr<Formatter> f(Formatter::create(format));
 
   if (prefix == "osd stat") {
     osdmap.print_summary(f.get(), ds);
@@ -2479,10 +2479,7 @@ bool OSDMonitor::preprocess_command(MMonCommand *m)
     }
     string format;
     cmd_getval(g_ceph_context, cmdmap, "format", format, string("json-pretty"));
-    boost::scoped_ptr<Formatter> f(new_formatter(format));
-    if (!f)
-      f.reset(new_formatter("json-pretty"));
-
+    boost::scoped_ptr<Formatter> f(Formatter::create(format));
     f->open_object_section("osd_location");
     f->dump_int("osd", osd);
     f->dump_stream("ip") << osdmap.get_addr(osd);
@@ -2508,9 +2505,7 @@ bool OSDMonitor::preprocess_command(MMonCommand *m)
     }
     string format;
     cmd_getval(g_ceph_context, cmdmap, "format", format, string("json-pretty"));
-    boost::scoped_ptr<Formatter> f(new_formatter(format));
-    if (!f)
-      f.reset(new_formatter("json-pretty"));
+    boost::scoped_ptr<Formatter> f(Formatter::create(format));
     f->open_object_section("osd_metadata");
     r = dump_osd_metadata(osd, f.get(), &ss);
     if (r < 0)
@@ -3007,10 +3002,7 @@ stats_out:
 	     prefix == "osd crush rule ls") {
     string format;
     cmd_getval(g_ceph_context, cmdmap, "format", format, string("json-pretty"));
-    Formatter *fp = new_formatter(format);
-    if (!fp)
-      fp = new_formatter("json-pretty");
-    boost::scoped_ptr<Formatter> f(fp);
+    boost::scoped_ptr<Formatter> f(Formatter::create(format));
     f->open_array_section("rules");
     osdmap.crush->list_rules(f.get());
     f->close_section();
@@ -3023,10 +3015,7 @@ stats_out:
     cmd_getval(g_ceph_context, cmdmap, "name", name);
     string format;
     cmd_getval(g_ceph_context, cmdmap, "format", format, string("json-pretty"));
-    Formatter *fp = new_formatter(format);
-    if (!fp)
-      fp = new_formatter("json-pretty");
-    boost::scoped_ptr<Formatter> f(fp);
+    boost::scoped_ptr<Formatter> f(Formatter::create(format));
     if (name == "") {
       f->open_array_section("rules");
       osdmap.crush->dump_rules(f.get());
@@ -3047,10 +3036,7 @@ stats_out:
   } else if (prefix == "osd crush dump") {
     string format;
     cmd_getval(g_ceph_context, cmdmap, "format", format, string("json-pretty"));
-    Formatter *fp = new_formatter(format);
-    if (!fp)
-      fp = new_formatter("json-pretty");
-    boost::scoped_ptr<Formatter> f(fp);
+    boost::scoped_ptr<Formatter> f(Formatter::create(format));
     f->open_object_section("crush_map");
     osdmap.crush->dump(f.get());
     f->close_section();
@@ -3061,10 +3047,7 @@ stats_out:
   } else if (prefix == "osd crush show-tunables") {
     string format;
     cmd_getval(g_ceph_context, cmdmap, "format", format, string("json-pretty"));
-    Formatter *fp = new_formatter(format);
-    if (!fp)
-      fp = new_formatter("json-pretty");
-    boost::scoped_ptr<Formatter> f(fp);
+    boost::scoped_ptr<Formatter> f(Formatter::create(format));
     f->open_object_section("crush_map_tunables");
     osdmap.crush->dump_tunables(f.get());
     f->close_section();
@@ -4102,7 +4085,7 @@ bool OSDMonitor::prepare_command_impl(MMonCommand *m,
 
   string format;
   cmd_getval(g_ceph_context, cmdmap, "format", format, string("plain"));
-  boost::scoped_ptr<Formatter> f(new_formatter(format));
+  boost::scoped_ptr<Formatter> f(Formatter::create(format));
 
   string prefix;
   cmd_getval(g_ceph_context, cmdmap, "prefix", prefix);

src/mon/PGMonitor.cc

@@ -1482,7 +1482,7 @@ bool PGMonitor::preprocess_command(MMonCommand *m)
 
   string format;
   cmd_getval(g_ceph_context, cmdmap, "format", format, string("plain"));
-  boost::scoped_ptr<Formatter> f(new_formatter(format));
+  boost::scoped_ptr<Formatter> f(Formatter::create(format));
 
   if (prefix == "pg stat") {
     if (f) {

src/os/MemStore.cc

@@ -101,7 +101,7 @@ int MemStore::_save()
 
 void MemStore::dump_all()
 {
-  Formatter *f = new_formatter("json-pretty");
+  Formatter *f = Formatter::create("json-pretty");
   f->open_object_section("store");
   dump(f);
   f->close_section();

src/osd/OSD.cc

@@ -1626,9 +1626,7 @@ public:
 bool OSD::asok_command(string command, cmdmap_t& cmdmap, string format,
 		       ostream& ss)
 {
-  Formatter *f = new_formatter(format);
-  if (!f)
-    f = new_formatter("json-pretty");
+  Formatter *f = Formatter::create(format);
   if (command == "status") {
     f->open_object_section("status");
     f->dump_stream("cluster_fsid") << superblock.cluster_fsid;
@@ -4897,7 +4895,7 @@ void OSD::do_command(Connection *con, ceph_tid_t tid, vector<string>& cmd, buffe
   }
 
   cmd_getval(cct, cmdmap, "format", format);
-  f.reset(new_formatter(format));
+  f.reset(Formatter::create(format));
 
   if (prefix == "version") {
     if (f) {
@@ -8220,7 +8218,7 @@ void OSD::ShardedOpWQ::_process(uint32_t thread_index, heartbeat_handle_d *hb )
   }
 
   lgeneric_subdout(osd->cct, osd, 30) << "dequeue status: ";
-  Formatter *f = new_formatter("json");
+  Formatter *f = Formatter::create("json");
   f->open_object_section("q");
   dump(f);
   f->close_section();

src/osd/ReplicatedPG.cc

@@ -603,10 +603,7 @@ int ReplicatedPG::do_command(cmdmap_t cmdmap, ostream& ss,
   string format;
 
   cmd_getval(cct, cmdmap, "format", format);
-  boost::scoped_ptr<Formatter> f(new_formatter(format));
-  // demand that we have a formatter
-  if (!f)
-    f.reset(new_formatter("json"));
+  boost::scoped_ptr<Formatter> f(Formatter::create(format, "json"));
 
   string command;
   cmd_getval(cct, cmdmap, "cmd", command);
@@ -12268,7 +12265,7 @@ bool ReplicatedPG::agent_maybe_evict(ObjectContextRef& obc)
 	     << ", evict_effort " << agent_state->evict_effort
 	     << dendl;
     dout(30) << "agent_state:\n";
-    Formatter *f = new_formatter("");
+    Formatter *f = Formatter::create("");
     f->open_object_section("agent_state");
     agent_state->dump(f);
     f->close_section();

src/osdc/Objecter.cc

@@ -4225,9 +4225,7 @@ Objecter::RequestStateHook::RequestStateHook(Objecter *objecter) :
 bool Objecter::RequestStateHook::call(std::string command, cmdmap_t& cmdmap,
 				      std::string format, bufferlist& out)
 {
-  Formatter *f = new_formatter(format);
-  if (!f)
-    f = new_formatter("json-pretty");
+  Formatter *f = Formatter::create(format);
   RWLock::RLocker rl(m_objecter->rwlock);
   m_objecter->dump_requests(f);
   f->flush(out);

src/test/bench/small_io_bench_fs.cc

@@ -32,7 +32,7 @@ struct MorePrinting : public DetailedStatCollector::AdditionalPrinting {
   MorePrinting(CephContext *cct) : cct(cct) {}
   void operator()(std::ostream *out) {
     bufferlist bl;
-    Formatter *f = new_formatter("json-pretty");
+    Formatter *f = Formatter::create("json-pretty");
     cct->get_perfcounters_collection()->dump_formatted(f, 0);
     f->flush(bl);
     delete f;

src/test/common/get_command_descriptions.cc

@@ -44,7 +44,7 @@ static void usage(ostream &out)
 static void json_print(const MonCommand *mon_commands, int size)
 {
   bufferlist rdata;
-  Formatter *f = new_formatter("json");
+  Formatter *f = Formatter::create("json");
   Monitor::format_command_descriptions(mon_commands, size, f, &rdata);
   delete f;
   string data(rdata.c_str(), rdata.length());

src/test/common/test_sloppy_crc_map.cc

@@ -7,7 +7,7 @@
 
 void dump(const SloppyCRCMap& scm)
 {
-  Formatter *f = new_formatter("json-pretty");
+  Formatter *f = Formatter::create("json-pretty");
   f->open_object_section("map");
   scm.dump(f);
   f->close_section();

src/test/common/test_tableformatter.cc

@@ -195,7 +195,7 @@ TEST(tableformatter, extendingheader)
 TEST(tableformatter, stream)
 {
   std::stringstream sout;
-  TableFormatter* formatter = (TableFormatter*) new_formatter("table");
+  TableFormatter* formatter = (TableFormatter*) Formatter::create("table");
   formatter->dump_stream("integer") << 10;
   formatter->dump_stream("float") << 10.0;
   formatter->dump_stream("string") << "string";
@@ -215,7 +215,7 @@ TEST(tableformatter, stream)
 TEST(tableformatter, multiline_keyval)
 {
   std::stringstream sout;
-  TableFormatter* formatter = (TableFormatter*) new_formatter("table-kv");
+  TableFormatter* formatter = (TableFormatter*) Formatter::create("table-kv");
   formatter->dump_int("integer", 10);
   formatter->dump_float("float", 10.0);
   formatter->dump_string("string", "string");

src/test/crush/TestCrushWrapper.cc

@@ -845,7 +845,7 @@ TEST(CrushWrapper, dump_rules) {
 
   // no ruleset by default
   {
-    Formatter *f = new_formatter("json-pretty");
+    Formatter *f = Formatter::create("json-pretty");
     c->dump_rules(f);
     stringstream ss;
     f->flush(ss);
@@ -859,7 +859,7 @@ TEST(CrushWrapper, dump_rules) {
   EXPECT_EQ(0, ruleset);
 
   {
-    Formatter *f = new_formatter("xml");
+    Formatter *f = Formatter::create("xml");
     c->dump_rules(f);
     stringstream ss;
     f->flush(ss);
@@ -868,7 +868,7 @@ TEST(CrushWrapper, dump_rules) {
   }
 
   {
-    Formatter *f = new_formatter("xml");
+    Formatter *f = Formatter::create("xml");
     c->dump_rule(ruleset, f);
     stringstream ss;
     f->flush(ss);

src/test/crush/indep.cc

@@ -68,7 +68,7 @@ CrushWrapper *build_indep_map(CephContext *cct, int num_rack, int num_host,
   c->set_rule_name(ruleno, "data");
 
   if (false) {
-    Formatter *f = new_formatter("json-pretty");
+    Formatter *f = Formatter::create("json-pretty");
     f->open_object_section("crush_map");
     c->dump(f);
     f->close_section();

src/test/osd/TestOSDMap.cc

@@ -324,7 +324,7 @@ TEST_F(OSDMapTest, PrimaryAffinity) {
 
   /*
   osdmap.print(cout);
-  Formatter *f = new_formatter("json-pretty");
+  Formatter *f = Formatter::create("json-pretty");
   f->open_object_section("CRUSH");
   osdmap.crush->dump(f);
   f->close_section();