You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by ganeshmurthy <gi...@git.apache.org> on 2015/11/23 15:55:43 UTC
[GitHub] qpid-dispatch pull request: DISPATCH-179 - Added code to handle th...
GitHub user ganeshmurthy opened a pull request:
https://github.com/apache/qpid-dispatch/pull/36
DISPATCH-179 - Added code to handle the management update operation. …
…Also added code to check the parameters to create and update operations before control reaches core
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ganeshmurthy/qpid-dispatch tross-DISPATCH-179-1-gmurthy
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/qpid-dispatch/pull/36.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #36
----
commit d5fc68a16211026bc78e90baa04cd6908b1d4419
Author: ganeshmurthy <gm...@redhat.com>
Date: 2015-11-23T14:52:41Z
DISPATCH-179 - Added code to handle the management update operation. Also added code to check the parameters to create and update operations before control reaches core
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-dispatch pull request: DISPATCH-179 - Added code to handle th...
Posted by ted-ross <gi...@git.apache.org>.
Github user ted-ross commented on a diff in the pull request:
https://github.com/apache/qpid-dispatch/pull/36#discussion_r45612666
--- Diff: include/qpid/dispatch/router_core.h ---
@@ -322,5 +322,5 @@ void qdr_query_free(qdr_query_t *query);
typedef void (*qdr_manage_response_t) (void *context, const qd_amqp_error_t *status, bool more);
void qdr_manage_handler(qdr_core_t *core, qdr_manage_response_t response_handler);
-
+bool qdr_is_set_manage_handler(qdr_core_t *core);
--- End diff --
I don't think this is needed. The "outer" agent should register its callbacks once at initialization time. It is not safe to register new handlers for each request that comes in and it's not necessary to repeatedly register the same handler.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-dispatch pull request: DISPATCH-179 - Added code to handle th...
Posted by ted-ross <gi...@git.apache.org>.
Github user ted-ross commented on a diff in the pull request:
https://github.com/apache/qpid-dispatch/pull/36#discussion_r45614279
--- Diff: src/router_core/management_agent.c ---
@@ -268,6 +269,62 @@ static void qd_core_agent_read_handler(qd_dispatch_t *qd,
}
+/**
+ * Returns true if all the keys contained in the body map are applicable to a waypoints, false otherwise
+ * @see qdr_waypoint_columns
+ */
+static bool qd_is_valid_waypoint_keys(qd_parsed_field_t *in_body)
+{
+ if(in_body != 0 && qd_parse_is_map(in_body)) {
+ //
+ // A map containing attributes with invalid values for an entity MUST result in a failure response with a statusCode of 400 (Bad Request).
+ //
+ int j=0;
+ qd_parsed_field_t *field = qd_parse_sub_key(in_body, j);
+ while (field) {
+ bool found = false;
+ for(int i = 1; i < QDR_WAYPOINT_COLUMN_COUNT; i++) {
+ if (qd_field_iterator_equal(qd_parse_raw(field), (unsigned char*)qdr_waypoint_columns[i])) {
+ found = true;
+ break;
+ }
+ }
+ if (!found) // Some bad field was specified in the body map. Reject this request
+ return false;
+ j++;
+ field = qd_parse_sub_key(in_body, j);
+ }
+ }
+ else
+ // The body is either empty or the body is not a map, return false
+ return false;
+
+ return true;
+}
+
+
+/**
+ * The body map containing any attributes that are not applicable for the entity being updated MUST result in a failure response with a statusCode of 400 (Bad Request).
+ */
+static bool qd_check_body(qd_router_entity_type_t entity_type,
+ qd_parsed_field_t *in_body,
+ qd_composed_field_t *out_body,
+ qd_management_context_t *ctx)
+{
+ if (entity_type == QD_ROUTER_WAYPOINT) {
+ if(!qd_is_valid_waypoint_keys(in_body)){
--- End diff --
Rather than have a proliferation of nearly identical type-specific validation functions, why not do a more general check on the appropriate attribute-name array? This way, you can write the logic once for all types.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-dispatch pull request: DISPATCH-179 - Added code to handle th...
Posted by ted-ross <gi...@git.apache.org>.
Github user ted-ross commented on a diff in the pull request:
https://github.com/apache/qpid-dispatch/pull/36#discussion_r45614085
--- Diff: src/router_core/management_agent.c ---
@@ -268,6 +269,62 @@ static void qd_core_agent_read_handler(qd_dispatch_t *qd,
}
+/**
+ * Returns true if all the keys contained in the body map are applicable to a waypoints, false otherwise
+ * @see qdr_waypoint_columns
+ */
+static bool qd_is_valid_waypoint_keys(qd_parsed_field_t *in_body)
+{
+ if(in_body != 0 && qd_parse_is_map(in_body)) {
+ //
+ // A map containing attributes with invalid values for an entity MUST result in a failure response with a statusCode of 400 (Bad Request).
+ //
+ int j=0;
+ qd_parsed_field_t *field = qd_parse_sub_key(in_body, j);
+ while (field) {
+ bool found = false;
+ for(int i = 1; i < QDR_WAYPOINT_COLUMN_COUNT; i++) {
+ if (qd_field_iterator_equal(qd_parse_raw(field), (unsigned char*)qdr_waypoint_columns[i])) {
+ found = true;
+ break;
+ }
+ }
+ if (!found) // Some bad field was specified in the body map. Reject this request
+ return false;
+ j++;
+ field = qd_parse_sub_key(in_body, j);
+ }
+ }
+ else
+ // The body is either empty or the body is not a map, return false
+ return false;
+
+ return true;
+}
+
+
+/**
+ * The body map containing any attributes that are not applicable for the entity being updated MUST result in a failure response with a statusCode of 400 (Bad Request).
+ */
+static bool qd_check_body(qd_router_entity_type_t entity_type,
--- End diff --
This function's signature suggests that it is simply checking the message body. In fact, it also has the side effect of composing the error message for the response. I think it would be better to stick with the simple check. The calling logic will then be clearer to understand as it will either go forward with the request or create an error response based on the outcome of this check.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-dispatch pull request: DISPATCH-179 - Added code to handle th...
Posted by ganeshmurthy <gi...@git.apache.org>.
Github user ganeshmurthy closed the pull request at:
https://github.com/apache/qpid-dispatch/pull/36
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-dispatch pull request: DISPATCH-179 - Added code to handle th...
Posted by ted-ross <gi...@git.apache.org>.
Github user ted-ross commented on a diff in the pull request:
https://github.com/apache/qpid-dispatch/pull/36#discussion_r45612985
--- Diff: src/router_core/management_agent_private.h ---
@@ -20,6 +20,6 @@
* under the License.
*/
-void management_agent_handler(void *context, qd_message_t *msg, int link_id);
+void qd_management_agent_handler(void *context, qd_message_t *msg, int link_id);
--- End diff --
I think a more descriptive name is needed here. Something like qd_router_agent_on_message.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org