changes to accomodate review comments on previous version

Signed-off-by: Babu Shanmugam <anbu@enovance.com>

diff --git a/src/Makefile.am b/src/Makefile.am
index 3a46006e0b1165b25d814b1a23cf9af9d1cb496f..1f32fdf5ba8837d5bbc4899b403d672cb96824fe 100644 (file)
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -959,21 +959,21 @@ unittest_rgw_meta_LDFLAGS = libglobal.la
 unittest_rgw_meta_LDADD = librgw.a ${UNITTEST_LDADD} ${UNITTEST_STATIC_LDADD} -lcryptopp -lcurl -luuid -lexpat  librados.la libcls_version_client.a \
  libcls_log_client.a libcls_statelog_client.a libcls_refcount_client.a libcls_rgw_client.a libcls_lock_client.a
 unittest_rgw_meta_CXXFLAGS = ${AM_CXXFLAGS} ${UNITTEST_CXXFLAGS}
-check_PROGRAMS += unittest_rgw_meta
+bin_DEBUGPROGRAMS += unittest_rgw_meta
 
 unittest_rgw_log_SOURCES = test/test_rgw_admin_log.cc
 unittest_rgw_log_LDFLAGS = libglobal.la 
 unittest_rgw_log_LDADD = librgw.a ${UNITTEST_LDADD} ${UNITTEST_STATIC_LDADD} -lcryptopp -lcurl -luuid -lexpat  librados.la libcls_version_client.a \
  libcls_log_client.a libcls_statelog_client.a libcls_refcount_client.a libcls_rgw_client.a libcls_lock_client.a
 unittest_rgw_log_CXXFLAGS = ${AM_CXXFLAGS} ${UNITTEST_CXXFLAGS}
-check_PROGRAMS += unittest_rgw_log
+bin_DEBUGPROGRAMS += unittest_rgw_log
 
 unittest_rgw_opstate_SOURCES = test/test_rgw_admin_opstate.cc
 unittest_rgw_opstate_LDFLAGS = libglobal.la 
 unittest_rgw_opstate_LDADD = librgw.a ${UNITTEST_LDADD} ${UNITTEST_STATIC_LDADD} -lcryptopp -lcurl -luuid -lexpat  librados.la libcls_version_client.a \
  libcls_log_client.a libcls_statelog_client.a libcls_refcount_client.a libcls_rgw_client.a libcls_lock_client.a
 unittest_rgw_opstate_CXXFLAGS = ${AM_CXXFLAGS} ${UNITTEST_CXXFLAGS}
-check_PROGRAMS += unittest_rgw_opstate
+bin_DEBUGPROGRAMS += unittest_rgw_opstate
 endif
 
 ceph_test_librbd_SOURCES = test/librbd/test_librbd.cc test/librados/test.cc

diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h
index 5fc31ffccbb69cb4416eab6c7e346561ea41c939..cd8dac7112f44210c81646707d0691d5eaef2da9 100644 (file)
--- a/src/rgw/rgw_common.h
+++ b/src/rgw/rgw_common.h
@@ -605,7 +605,7 @@ struct RGWBucketInfo
      ::encode(owner, bl);
      ::encode(flags, bl);
      ::encode(region, bl);
-     ::encode((uint32_t)creation_time, bl);
+     ::encode((uint64_t)creation_time, bl);
      ENCODE_FINISH(bl);
   }
   void decode(bufferlist::iterator& bl) {
@@ -618,7 +618,7 @@ struct RGWBucketInfo
      if (struct_v >= 5)
        ::decode(region, bl);
      if (struct_v >= 6)
-       ::decode((uint32_t&)creation_time, bl);
+       ::decode((uint64_t&)creation_time, bl);
      DECODE_FINISH(bl);
   }
   void dump(Formatter *f) const;

diff --git a/src/rgw/rgw_rest_opstate.cc b/src/rgw/rgw_rest_opstate.cc
index f9af580c9e95faaff1dfb45674d5d076c45c55b8..434b0db5989113d99aa2be73d39c9fc9e9519a00 100644 (file)
--- a/src/rgw/rgw_rest_opstate.cc
+++ b/src/rgw/rgw_rest_opstate.cc
@@ -24,8 +24,8 @@
 #define dout_subsys ceph_subsys_rgw
 
 void RGWOp_Opstate_List::execute() {
-  string client_id = s->info.args.get("client_id"),
-         op_id = s->info.args.get("op_id"),
+  string client_id = s->info.args.get("client-id"),
+         op_id = s->info.args.get("op-id"),
          object = s->info.args.get("object"),
          max_entries_str = s->info.args.get("max-entries");
   unsigned max_entries = OPSTATE_LIST_MAX_ENTRIES;
@@ -43,9 +43,12 @@ void RGWOp_Opstate_List::execute() {
   RGWOpState oc = RGWOpState(store);
   void *handle;
   bool done;
+  list<cls_statelog_entry> entries;
 
   oc.init_list_entries(client_id, op_id, object, &handle);
 
+  http_ret = 0;
+  send_response();
   do {
     int ret = oc.list_entries(handle, max_entries, entries, &done);
 
@@ -59,31 +62,46 @@ void RGWOp_Opstate_List::execute() {
     if (!max_entries_str.empty()) 
       max_entries -= entries.size();
 
+    send_response(entries);
   } while (!done && max_entries > 0);
 
   oc.finish_list_entries(handle);
-  http_ret = 0;
+  send_response_end();
 }
 
 void RGWOp_Opstate_List::send_response() {
+  if (sent_header)
+    return;
+
   set_req_state_err(s, http_ret);
   dump_errno(s);
   end_header(s);
 
+  sent_header = true;
+
+  if (http_ret < 0)
+    return;
+
   s->formatter->open_array_section("entries");
+}
+
+void RGWOp_Opstate_List::send_response(list<cls_statelog_entry> entries) {
   RGWOpState oc(store);
   for (list<cls_statelog_entry>::iterator it = entries.begin();
        it != entries.end(); it++) {
     oc.dump_entry(*it, s->formatter);
     flusher.flush();
   }
+}
+
+void RGWOp_Opstate_List::send_response_end() {
   s->formatter->close_section();
   flusher.flush();
 }
 
 void RGWOp_Opstate_Set::execute() {
-  string client_id = s->info.args.get("client_id"),
-         op_id = s->info.args.get("op_id"),
+  string client_id = s->info.args.get("client-id"),
+         op_id = s->info.args.get("op-id"),
          object = s->info.args.get("object"),
          state_str = s->info.args.get("state");
 
@@ -113,8 +131,8 @@ void RGWOp_Opstate_Set::execute() {
 }
 
 void RGWOp_Opstate_Renew::execute() {
-  string client_id = s->info.args.get("client_id"),
-         op_id = s->info.args.get("op_id"),
+  string client_id = s->info.args.get("client-id"),
+         op_id = s->info.args.get("op-id"),
          object = s->info.args.get("object"),
          state_str = s->info.args.get("state");
 
@@ -143,8 +161,8 @@ void RGWOp_Opstate_Renew::execute() {
 }
 
 void RGWOp_Opstate_Delete::execute() {
-  string client_id = s->info.args.get("client_id"),
-         op_id = s->info.args.get("op_id"),
+  string client_id = s->info.args.get("client-id"),
+         op_id = s->info.args.get("op-id"),
          object = s->info.args.get("object");
 
   if (client_id.empty() ||

diff --git a/src/rgw/rgw_rest_opstate.h b/src/rgw/rgw_rest_opstate.h
index 8aefab400c354b91b7acfebe79567f4bdf1e0288..0d069e0c9c0c99d80d2c76a83ab17a8f4470f854 100644 (file)
--- a/src/rgw/rgw_rest_opstate.h
+++ b/src/rgw/rgw_rest_opstate.h
@@ -15,10 +15,9 @@
 #define CEPH_RGW_REST_OPSTATE_H
 
 class RGWOp_Opstate_List : public RGWRESTOp {
-  int http_ret;
-  list<cls_statelog_entry> entries;
+  bool sent_header;
 public:
-  RGWOp_Opstate_List() : http_ret(0) {}
+  RGWOp_Opstate_List() {}
   ~RGWOp_Opstate_List() {}
 
   int check_caps(RGWUserCaps& caps) {
@@ -29,6 +28,8 @@ public:
   }
   void execute();
   virtual void send_response();
+  virtual void send_response(list<cls_statelog_entry> entries);
+  virtual void send_response_end();
   virtual const char *name() {
     return "opstate_list";
   }

diff --git a/src/test/test_rgw_admin_opstate.cc b/src/test/test_rgw_admin_opstate.cc
index ed7705e0dcace4287149d9947a90f2c8fb121dbd..df99dd814d857d54dacec59d1868079d4c524b8b 100644 (file)
--- a/src/test/test_rgw_admin_opstate.cc
+++ b/src/test/test_rgw_admin_opstate.cc
@@ -533,29 +533,29 @@ TEST(TestRGWAdmin, opstate_set_list_delete) {
   ASSERT_EQ(user_create(uid, display_name), 0);
   ASSERT_EQ(0, caps_add(cname, perm));
 
-  rest_req = "/admin/opstate?client_id=1&op_id=1&object=o";
+  rest_req = "/admin/opstate?client-id=1&op-id=1&object=o";
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
   EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing state*/
 
-  rest_req = "/admin/opstate?client_id=1&op_id=1&state=in-progress";
+  rest_req = "/admin/opstate?client-id=1&op-id=1&state=in-progress";
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
   EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing object*/
 
-  rest_req = "/admin/opstate?client_id=1&state=in-progress&object=o";
+  rest_req = "/admin/opstate?client-id=1&state=in-progress&object=o";
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
-  EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing op_id*/
+  EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing op-id*/
 
-  rest_req = "/admin/opstate?state=in-progress&op_id=1&object=o";
+  rest_req = "/admin/opstate?state=in-progress&op-id=1&object=o";
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
-  EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing client_id*/
+  EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing client-id*/
 
-  rest_req = "/admin/opstate?state=invalid&op_id=1&object=o&client_id=1";
+  rest_req = "/admin/opstate?state=invalid&op-id=1&object=o&client-id=1";
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
   EXPECT_EQ(400U, g_test->get_resp_code()); /*Invalid state*/
 
   state = "in-progress";
   entries.clear();
-  ss << "/admin/opstate?client_id=" << cid_1 << "&op_id=" << oid_1 
+  ss << "/admin/opstate?client-id=" << cid_1 << "&op-id=" << oid_1 
     << "&object=" << obj_1 << "&state=" << state;
   rest_req = ss.str();
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
@@ -581,7 +581,7 @@ TEST(TestRGWAdmin, opstate_set_list_delete) {
   state = "complete";
   ss.str("");
   entries.clear();
-  ss << "/admin/opstate?client_id=" << cid_1 << "&op_id=" << oid_1 
+  ss << "/admin/opstate?client-id=" << cid_1 << "&op-id=" << oid_1 
     << "&object=" << obj_1 << "&state=" << state;
   rest_req = ss.str();
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
@@ -605,7 +605,7 @@ TEST(TestRGWAdmin, opstate_set_list_delete) {
 
   ss.str("");
   entries.clear();
-  ss << "/admin/opstate?client_id=" << cid_2 << "&op_id=" << oid_2 
+  ss << "/admin/opstate?client-id=" << cid_2 << "&op-id=" << oid_2 
     << "&object=" << obj_2 << "&state=" << state;
   rest_req = ss.str();
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
@@ -642,7 +642,7 @@ TEST(TestRGWAdmin, opstate_set_list_delete) {
   
   ss.str("");
   entries.clear();
-  ss << "/admin/opstate?client_id=" << cid_2 << "&op_id=" << oid_2 << "&object=" << obj_2;
+  ss << "/admin/opstate?client-id=" << cid_2 << "&op-id=" << oid_2 << "&object=" << obj_2;
   rest_req = ss.str();
   g_test->send_request(string("GET"), rest_req);
   EXPECT_EQ(200U, g_test->get_resp_code());
@@ -651,7 +651,7 @@ TEST(TestRGWAdmin, opstate_set_list_delete) {
   
   ss.str("");
   entries.clear();
-  ss << "/admin/opstate?client_id=" << cid_2;
+  ss << "/admin/opstate?client-id=" << cid_2;
   rest_req = ss.str();
   g_test->send_request(string("GET"), rest_req);
   EXPECT_EQ(200U, g_test->get_resp_code());
@@ -660,7 +660,7 @@ TEST(TestRGWAdmin, opstate_set_list_delete) {
 
   ss.str("");
   entries.clear();
-  ss << "/admin/opstate?op_id=" << oid_2;
+  ss << "/admin/opstate?op-id=" << oid_2;
   rest_req = ss.str();
   g_test->send_request(string("GET"), rest_req);
   EXPECT_EQ(200U, g_test->get_resp_code());
@@ -669,31 +669,31 @@ TEST(TestRGWAdmin, opstate_set_list_delete) {
 
   ss.str("");
   entries.clear();
-  ss << "/admin/opstate?op_id=" << oid_2 << "&object=" << obj_1 << "&client_id=" << cid_2 ;
+  ss << "/admin/opstate?op-id=" << oid_2 << "&object=" << obj_1 << "&client-id=" << cid_2 ;
   rest_req = ss.str();
   g_test->send_request(string("GET"), rest_req);
   EXPECT_EQ(200U, g_test->get_resp_code());
   EXPECT_EQ(get_opstate_list(entries), 0);
   EXPECT_EQ(entries.size(), 0U);
 
-  rest_req = "/admin/opstate?client_id=1&op_id=1";
+  rest_req = "/admin/opstate?client-id=1&op-id=1";
   g_test->send_request(string("DELETE"), rest_req);
   EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing object*/
 
-  rest_req = "/admin/opstate?client_id=1&object=1";
+  rest_req = "/admin/opstate?client-id=1&object=1";
   g_test->send_request(string("DELETE"), rest_req);
-  EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing op_id*/
+  EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing op-id*/
 
-  rest_req = "/admin/opstate?object=1&op_id=1";
+  rest_req = "/admin/opstate?object=1&op-id=1";
   g_test->send_request(string("DELETE"), rest_req);
-  EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing client_id*/
+  EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing client-id*/
   
-  rest_req = "/admin/opstate?object=1&op_id=1&client_id=1";
+  rest_req = "/admin/opstate?object=1&op-id=1&client-id=1";
   g_test->send_request(string("DELETE"), rest_req);
   EXPECT_EQ(404U, g_test->get_resp_code());
 
   ss.str("");
-  ss << "/admin/opstate?client_id=" << cid_1 << "&op_id=" << oid_1 
+  ss << "/admin/opstate?client-id=" << cid_1 << "&op-id=" << oid_1 
     << "&object=" << obj_1;
   rest_req = ss.str();
   g_test->send_request(string("DELETE"), rest_req);
@@ -709,7 +709,7 @@ TEST(TestRGWAdmin, opstate_set_list_delete) {
   EXPECT_EQ(entries.size(), 1U);
 
   ss.str("");
-  ss << "/admin/opstate?client_id=" << cid_2 << "&op_id=" << oid_2 
+  ss << "/admin/opstate?client-id=" << cid_2 << "&op-id=" << oid_2 
     << "&object=" << obj_2;
   rest_req = ss.str();
   g_test->send_request(string("DELETE"), rest_req);
@@ -741,29 +741,29 @@ TEST(TestRGWAdmin, opstate_renew) {
   ASSERT_EQ(user_create(uid, display_name), 0);
   ASSERT_EQ(0, caps_add(cname, perm));
 
-  rest_req = "/admin/opstate?renew&client_id=1&op_id=1&object=o";
+  rest_req = "/admin/opstate?renew&client-id=1&op-id=1&object=o";
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
   EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing state*/
 
-  rest_req = "/admin/opstate?renew&client_id=1&op_id=1&state=in-progress";
+  rest_req = "/admin/opstate?renew&client-id=1&op-id=1&state=in-progress";
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
   EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing object*/
 
-  rest_req = "/admin/opstate?renew&client_id=1&state=in-progress&object=o";
+  rest_req = "/admin/opstate?renew&client-id=1&state=in-progress&object=o";
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
-  EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing op_id*/
+  EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing op-id*/
 
-  rest_req = "/admin/opstate?renew&state=in-progress&op_id=1&object=o";
+  rest_req = "/admin/opstate?renew&state=in-progress&op-id=1&object=o";
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
-  EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing client_id*/
+  EXPECT_EQ(400U, g_test->get_resp_code()); /*Missing client-id*/
 
-  rest_req = "/admin/opstate?&renew&state=invalid&op_id=1&object=o&client_id=1";
+  rest_req = "/admin/opstate?&renew&state=invalid&op-id=1&object=o&client-id=1";
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
   EXPECT_EQ(400U, g_test->get_resp_code()); /*Invalid state*/
 
   state = "complete";
   ss.str("");
-  ss << "/admin/opstate?client_id=" << cid_1 << "&op_id=" << oid_1 
+  ss << "/admin/opstate?client-id=" << cid_1 << "&op-id=" << oid_1 
     << "&object=" << obj_1 << "&state=" << state;
   rest_req = ss.str();
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
@@ -774,28 +774,28 @@ TEST(TestRGWAdmin, opstate_renew) {
   EXPECT_EQ(200U, g_test->get_resp_code());
 
   ss.str("");
-  ss << "/admin/opstate?renew&client_id=" << cid_1 << "&op_id=1" 
+  ss << "/admin/opstate?renew&client-id=" << cid_1 << "&op-id=1" 
     << "&object=" << obj_1 << "&state=" << state;
   rest_req = ss.str();
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
   EXPECT_EQ(404U, g_test->get_resp_code());
   
   ss.str("");
-  ss << "/admin/opstate?renew&client_id=" << cid_1 << "&op_id=" << oid_1 
+  ss << "/admin/opstate?renew&client-id=" << cid_1 << "&op-id=" << oid_1 
     << "&object=" << obj_1 << "&state=" << state;
   rest_req = ss.str();
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
   EXPECT_EQ(200U, g_test->get_resp_code());
 
   ss.str("");
-  ss << "/admin/opstate?renew&client_id=" << cid_1 << "&op_id=" << oid_1 
+  ss << "/admin/opstate?renew&client-id=" << cid_1 << "&op-id=" << oid_1 
     << "&object=" << obj_1 << "&state=in-progress";
   rest_req = ss.str();
   g_test->send_request(string("POST"), rest_req, read_dummy_post, NULL, sizeof(int));
   EXPECT_EQ(500U, g_test->get_resp_code());
 
   ss.str("");
-  ss << "/admin/opstate?client_id=" << cid_1 << "&op_id=" << oid_1 
+  ss << "/admin/opstate?client-id=" << cid_1 << "&op-id=" << oid_1 
     << "&object=" << obj_1;
   rest_req = ss.str();
   g_test->send_request(string("DELETE"), rest_req);