Repository: qpid-dispatch
Updated Branches:
  refs/heads/master cc8f70a6c -> a2612ca4f


DISPATCH-373 - Added improved error messages for reads in the core agent.
This closes #86


Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/a2612ca4
Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/a2612ca4
Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/a2612ca4

Branch: refs/heads/master
Commit: a2612ca4fc5296ed7144ac8d5045192cc121eb77
Parents: cc8f70a
Author: Ted Ross <[email protected]>
Authored: Thu Aug 4 14:32:35 2016 -0400
Committer: Ted Ross <[email protected]>
Committed: Thu Aug 4 14:32:35 2016 -0400

----------------------------------------------------------------------
 .../qpid_dispatch_internal/management/agent.py  |  4 ++--
 src/router_core/agent_config_address.c          | 20 +++++++++++++++---
 src/router_core/agent_config_auto_link.c        | 20 +++++++++++++++---
 src/router_core/agent_config_link_route.c       | 22 +++++++++++++++++---
 src/router_core/router_core.c                   |  5 +++--
 src/router_core/router_core_private.h           |  1 +
 tests/system_tests_link_routes.py               |  2 +-
 7 files changed, 60 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/a2612ca4/python/qpid_dispatch_internal/management/agent.py
----------------------------------------------------------------------
diff --git a/python/qpid_dispatch_internal/management/agent.py 
b/python/qpid_dispatch_internal/management/agent.py
index 6e4b7fc..42191da 100644
--- a/python/qpid_dispatch_internal/management/agent.py
+++ b/python/qpid_dispatch_internal/management/agent.py
@@ -803,7 +803,7 @@ class Agent(object):
         """Called when a management request is received."""
         def error(e, trace):
             """Raise an error"""
-            self.log(LOG_ERROR, "Error dispatching %s: %s\n%s"%(request, e, 
trace))
+            self.log(LOG_ERROR, "Error performing %s: 
%s"%(request.properties.get('operation'), e.message))
             self.respond(request, e.status, e.description)
 
         # If there's no reply_to, don't bother to process the request.
@@ -917,7 +917,7 @@ class Agent(object):
             if not requested_type or 
self.management.entity_type.is_a(requested_type):
                 return self.management
             else:
-                raise BadRequestStatus("No name or identity provided")
+                raise BadRequestStatus("%s: No name or identity provided" % 
requested_type)
 
         def attrvals():
             """String form of the id attribute values for error messages"""

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/a2612ca4/src/router_core/agent_config_address.c
----------------------------------------------------------------------
diff --git a/src/router_core/agent_config_address.c 
b/src/router_core/agent_config_address.c
index edb9ba5..753e79d 100644
--- a/src/router_core/agent_config_address.c
+++ b/src/router_core/agent_config_address.c
@@ -42,6 +42,7 @@ const char *qdr_config_address_columns[] =
      "egressPhase",
      0};
 
+const char *CONFIG_ADDRESS_TYPE = 
"org.apache.qpid.dispatch.router.config.address";
 
 static void qdr_config_address_insert_column_CT(qdr_address_config_t *addr, 
int col, qd_composed_field_t *body, bool as_map)
 {
@@ -67,7 +68,7 @@ static void 
qdr_config_address_insert_column_CT(qdr_address_config_t *addr, int
     }
 
     case QDR_CONFIG_ADDRESS_TYPE:
-        qd_compose_insert_string(body, 
"org.apache.qpid.dispatch.router.config.address");
+        qd_compose_insert_string(body, CONFIG_ADDRESS_TYPE);
         break;
 
     case QDR_CONFIG_ADDRESS_PREFIX:
@@ -259,8 +260,11 @@ void qdra_config_address_delete_CT(qdr_core_t          
*core,
 {
     qdr_address_config_t *addr = 0;
 
-    if (!name && !identity)
+    if (!name && !identity) {
         query->status = QD_AMQP_BAD_REQUEST;
+        query->status.description = "No name or identity provided";
+        qd_log(core->agent_log, QD_LOG_ERROR, "Error performing DELETE of %s: 
%s", CONFIG_ADDRESS_TYPE, query->status.description);
+    }
     else {
         if (identity)
             addr = qdr_address_config_find_by_identity_CT(core, identity);
@@ -311,11 +315,14 @@ void qdra_config_address_create_CT(qdr_core_t          
*core,
         if (!!addr) {
             query->status = QD_AMQP_BAD_REQUEST;
             query->status.description = "Name conflicts with an existing 
entity";
+            qd_log(core->agent_log, QD_LOG_ERROR, "Error performing CREATE of 
%s: %s", CONFIG_ADDRESS_TYPE, query->status.description);
             break;
         }
 
         if (!qd_parse_is_map(in_body)) {
             query->status = QD_AMQP_BAD_REQUEST;
+            query->status.description = "Body of request must be a map";
+            qd_log(core->agent_log, QD_LOG_ERROR, "Error performing CREATE of 
%s: %s", CONFIG_ADDRESS_TYPE, query->status.description);
             break;
         }
 
@@ -333,6 +340,8 @@ void qdra_config_address_create_CT(qdr_core_t          
*core,
         //
         if (!prefix_field) {
             query->status = QD_AMQP_BAD_REQUEST;
+            query->status.description = "prefix field is mandatory";
+            qd_log(core->agent_log, QD_LOG_ERROR, "Error performing CREATE of 
%s: %s", CONFIG_ADDRESS_TYPE, query->status.description);
             break;
         }
 
@@ -347,6 +356,7 @@ void qdra_config_address_create_CT(qdr_core_t          
*core,
         if (!!addr) {
             query->status = QD_AMQP_BAD_REQUEST;
             query->status.description = "Address prefix conflicts with an 
existing entity";
+            qd_log(core->agent_log, QD_LOG_ERROR, "Error performing CREATE of 
%s: %s", CONFIG_ADDRESS_TYPE, query->status.description);
             break;
         }
 
@@ -369,6 +379,7 @@ void qdra_config_address_create_CT(qdr_core_t          
*core,
         if (in_phase < 0 || in_phase > 9 || out_phase < 0 || out_phase > 9) {
             query->status = QD_AMQP_BAD_REQUEST;
             query->status.description = "Phase values must be between 0 and 9";
+            qd_log(core->agent_log, QD_LOG_ERROR, "Error performing CREATE of 
%s: %s", CONFIG_ADDRESS_TYPE, query->status.description);
             break;
         }
 
@@ -440,8 +451,11 @@ void qdra_config_address_get_CT(qdr_core_t          *core,
 {
     qdr_address_config_t *addr = 0;
 
-    if (!name && !identity)
+    if (!name && !identity) {
         query->status = QD_AMQP_BAD_REQUEST;
+        query->status.description = "No name or identity provided";
+        qd_log(core->agent_log, QD_LOG_ERROR, "Error performing READ of %s: 
%s", CONFIG_ADDRESS_TYPE, query->status.description);
+    }
     else {
         if (identity) //If there is identity, ignore the name
             addr = qdr_address_config_find_by_identity_CT(core, identity);

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/a2612ca4/src/router_core/agent_config_auto_link.c
----------------------------------------------------------------------
diff --git a/src/router_core/agent_config_auto_link.c 
b/src/router_core/agent_config_auto_link.c
index 4f77348..4397b81 100644
--- a/src/router_core/agent_config_auto_link.c
+++ b/src/router_core/agent_config_auto_link.c
@@ -49,6 +49,7 @@ const char *qdr_config_auto_link_columns[] =
      "lastError",
      0};
 
+const char *CONFIG_AUTOLINK_TYPE = 
"org.apache.qpid.dispatch.router.config.autoLink";
 
 static void qdr_config_auto_link_insert_column_CT(qdr_auto_link_t *al, int 
col, qd_composed_field_t *body, bool as_map)
 {
@@ -73,7 +74,7 @@ static void 
qdr_config_auto_link_insert_column_CT(qdr_auto_link_t *al, int col,
         break;
 
     case QDR_CONFIG_AUTO_LINK_TYPE:
-        qd_compose_insert_string(body, 
"org.apache.qpid.dispatch.router.config.autoLink");
+        qd_compose_insert_string(body, CONFIG_AUTOLINK_TYPE);
         break;
 
     case QDR_CONFIG_AUTO_LINK_ADDR:
@@ -297,8 +298,11 @@ void qdra_config_auto_link_delete_CT(qdr_core_t          
*core,
 {
     qdr_auto_link_t *al = 0;
 
-    if (!name && !identity)
+    if (!name && !identity) {
         query->status = QD_AMQP_BAD_REQUEST;
+        query->status.description = "No name or identity provided";
+        qd_log(core->agent_log, QD_LOG_ERROR, "Error performing DELETE of %s: 
%s", CONFIG_AUTOLINK_TYPE, query->status.description);
+    }
     else {
         if (identity)
             al = qdr_auto_link_config_find_by_identity_CT(core, identity);
@@ -337,11 +341,14 @@ void qdra_config_auto_link_create_CT(qdr_core_t          
*core,
         if (!!al) {
             query->status = QD_AMQP_BAD_REQUEST;
             query->status.description = "Name conflicts with an existing 
entity";
+            qd_log(core->agent_log, QD_LOG_ERROR, "Error performing CREATE of 
%s: %s", CONFIG_AUTOLINK_TYPE, query->status.description);
             break;
         }
 
         if (!qd_parse_is_map(in_body)) {
             query->status = QD_AMQP_BAD_REQUEST;
+            query->status.description = "Body of request must be a map";
+            qd_log(core->agent_log, QD_LOG_ERROR, "Error performing CREATE of 
%s: %s", CONFIG_AUTOLINK_TYPE, query->status.description);
             break;
         }
 
@@ -359,6 +366,8 @@ void qdra_config_auto_link_create_CT(qdr_core_t          
*core,
         //
         if (!addr_field || !dir_field) {
             query->status = QD_AMQP_BAD_REQUEST;
+            query->status.description = "addr and dir fields are mandatory";
+            qd_log(core->agent_log, QD_LOG_ERROR, "Error performing CREATE of 
%s: %s", CONFIG_AUTOLINK_TYPE, query->status.description);
             break;
         }
 
@@ -367,6 +376,7 @@ void qdra_config_auto_link_create_CT(qdr_core_t          
*core,
         if (error) {
             query->status = QD_AMQP_BAD_REQUEST;
             query->status.description = error;
+            qd_log(core->agent_log, QD_LOG_ERROR, "Error performing CREATE of 
%s: %s", CONFIG_AUTOLINK_TYPE, query->status.description);
             break;
         }
 
@@ -382,6 +392,7 @@ void qdra_config_auto_link_create_CT(qdr_core_t          
*core,
         if (phase < 0 || phase > 9) {
             query->status = QD_AMQP_BAD_REQUEST;
             query->status.description = "autoLink phase must be between 0 and 
9";
+            qd_log(core->agent_log, QD_LOG_ERROR, "Error performing CREATE of 
%s: %s", CONFIG_AUTOLINK_TYPE, query->status.description);
             break;
         }
 
@@ -450,8 +461,11 @@ void qdra_config_auto_link_get_CT(qdr_core_t        *core,
 {
     qdr_auto_link_t *al = 0;
 
-    if (!name && !identity)
+    if (!name && !identity) {
         query->status = QD_AMQP_BAD_REQUEST;
+        query->status.description = "No name or identity provided";
+        qd_log(core->agent_log, QD_LOG_ERROR, "Error performing READ of %s: 
%s", CONFIG_AUTOLINK_TYPE, query->status.description);
+    }
     else {
         if (identity) //If there is identity, ignore the name
             al = qdr_auto_link_config_find_by_identity_CT(core, identity);

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/a2612ca4/src/router_core/agent_config_link_route.c
----------------------------------------------------------------------
diff --git a/src/router_core/agent_config_link_route.c 
b/src/router_core/agent_config_link_route.c
index e2a38bf..d7da761 100644
--- a/src/router_core/agent_config_link_route.c
+++ b/src/router_core/agent_config_link_route.c
@@ -45,6 +45,9 @@ const char *qdr_config_link_route_columns[] =
      "operStatus",
      0};
 
+const char *CONFIG_LINKROUTE_TYPE = 
"org.apache.qpid.dispatch.router.config.linkRoute";
+
+const qd_amqp_error_t QD_AMQP_NAME_IDENTITY_MISSING = { 400, "No name or 
identity provided" };
 
 static void qdr_config_link_route_insert_column_CT(qdr_link_route_t *lr, int 
col, qd_composed_field_t *body, bool as_map)
 {
@@ -70,7 +73,7 @@ static void 
qdr_config_link_route_insert_column_CT(qdr_link_route_t *lr, int col
     }
 
     case QDR_CONFIG_LINK_ROUTE_TYPE:
-        qd_compose_insert_string(body, 
"org.apache.qpid.dispatch.router.config.linkRoute");
+        qd_compose_insert_string(body, CONFIG_LINKROUTE_TYPE);
         break;
 
     case QDR_CONFIG_LINK_ROUTE_PREFIX:
@@ -311,8 +314,11 @@ void qdra_config_link_route_delete_CT(qdr_core_t          
*core,
 {
     qdr_link_route_t *lr = 0;
 
-    if (!name && !identity)
+    if (!name && !identity) {
         query->status = QD_AMQP_BAD_REQUEST;
+        query->status.description = "No name or identity provided";
+        qd_log(core->agent_log, QD_LOG_ERROR, "Error performing DELETE of %s: 
%s", CONFIG_LINKROUTE_TYPE, query->status.description);
+    }
     else {
         if (identity)
             lr = qdr_link_route_config_find_by_identity_CT(core, identity);
@@ -351,11 +357,14 @@ void qdra_config_link_route_create_CT(qdr_core_t          
*core,
         if (!!lr) {
             query->status = QD_AMQP_BAD_REQUEST;
             query->status.description = "Name conflicts with an existing 
entity";
+            qd_log(core->agent_log, QD_LOG_ERROR, "Error performing CREATE of 
%s: %s", CONFIG_LINKROUTE_TYPE, query->status.description);
             break;
         }
 
         if (!qd_parse_is_map(in_body)) {
             query->status = QD_AMQP_BAD_REQUEST;
+            query->status.description = "Body of request must be a map";
+            qd_log(core->agent_log, QD_LOG_ERROR, "Error performing CREATE of 
%s: %s", CONFIG_LINKROUTE_TYPE, query->status.description);
             break;
         }
 
@@ -373,6 +382,8 @@ void qdra_config_link_route_create_CT(qdr_core_t          
*core,
         //
         if (!prefix_field || !dir_field) {
             query->status = QD_AMQP_BAD_REQUEST;
+            query->status.description = "prefix and dir fields are mandatory";
+            qd_log(core->agent_log, QD_LOG_ERROR, "Error performing CREATE of 
%s: %s", CONFIG_LINKROUTE_TYPE, query->status.description);
             break;
         }
 
@@ -381,6 +392,7 @@ void qdra_config_link_route_create_CT(qdr_core_t          
*core,
         if (error) {
             query->status = QD_AMQP_BAD_REQUEST;
             query->status.description = error;
+            qd_log(core->agent_log, QD_LOG_ERROR, "Error performing CREATE of 
%s: %s", CONFIG_LINKROUTE_TYPE, query->status.description);
             break;
         }
 
@@ -389,6 +401,7 @@ void qdra_config_link_route_create_CT(qdr_core_t          
*core,
         if (error) {
             query->status = QD_AMQP_BAD_REQUEST;
             query->status.description = error;
+            qd_log(core->agent_log, QD_LOG_ERROR, "Error performing CREATE of 
%s: %s", CONFIG_LINKROUTE_TYPE, query->status.description);
             break;
         }
 
@@ -440,8 +453,11 @@ void qdra_config_link_route_get_CT(qdr_core_t          
*core,
 {
     qdr_link_route_t *lr = 0;
 
-    if (!name && !identity)
+    if (!name && !identity) {
         query->status = QD_AMQP_BAD_REQUEST;
+        query->status.description = "No name or identity provided";
+        qd_log(core->agent_log, QD_LOG_ERROR, "Error performing READ of %s: 
%s", CONFIG_LINKROUTE_TYPE, query->status.description);
+    }
     else {
         if (identity) //If there is identity, ignore the name
             lr = qdr_link_route_config_find_by_identity_CT(core, identity);

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/a2612ca4/src/router_core/router_core.c
----------------------------------------------------------------------
diff --git a/src/router_core/router_core.c b/src/router_core/router_core.c
index e8cc06b..937e112 100644
--- a/src/router_core/router_core.c
+++ b/src/router_core/router_core.c
@@ -44,9 +44,10 @@ qdr_core_t *qdr_core(qd_dispatch_t *qd, qd_router_mode_t 
mode, const char *area,
     core->router_id   = id;
 
     //
-    // Set up the logging source for the router core
+    // Set up the logging sources for the router core
     //
-    core->log = qd_log_source("ROUTER_CORE");
+    core->log       = qd_log_source("ROUTER_CORE");
+    core->agent_log = qd_log_source("AGENT");
 
     //
     // Set up the threading support

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/a2612ca4/src/router_core/router_core_private.h
----------------------------------------------------------------------
diff --git a/src/router_core/router_core_private.h 
b/src/router_core/router_core_private.h
index a578810..2079435 100644
--- a/src/router_core/router_core_private.h
+++ b/src/router_core/router_core_private.h
@@ -503,6 +503,7 @@ ALLOC_DECLARE(qdr_conn_identifier_t);
 struct qdr_core_t {
     qd_dispatch_t     *qd;
     qd_log_source_t   *log;
+    qd_log_source_t   *agent_log;
     sys_thread_t      *thread;
     bool               running;
     qdr_action_list_t  action_list;

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/a2612ca4/tests/system_tests_link_routes.py
----------------------------------------------------------------------
diff --git a/tests/system_tests_link_routes.py 
b/tests/system_tests_link_routes.py
index 7f1d117..781eb93 100644
--- a/tests/system_tests_link_routes.py
+++ b/tests/system_tests_link_routes.py
@@ -184,7 +184,7 @@ class LinkRoutePatternTest(TestCase):
             out = self.run_qdmanage(cmd=cmd, 
address=self.routers[1].addresses[0])
         except Exception, e:
             exception_occurred = True
-            self.assertTrue("BadRequestStatus: Bad Request" in e.message)
+            self.assertTrue("BadRequestStatus: No name or identity provided" 
in e.message)
 
         self.assertTrue(exception_occurred)
 


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to