[Asterisk-code-review] ARI: Add support for push configuration of dynamic object (asterisk[13])

[Matt Jordan]

Matt Jordan has posted comments on this change.

Change subject: ARI: Add support for push configuration of dynamic object
......................................................................


Patch Set 1:

(2 comments)

https://gerrit.asterisk.org/#/c/853/1/res/ari/resource_asterisk.c
File res/ari/resource_asterisk.c:

Line 77: 		if (ast_json_array_append(return_set, tuple)) {
       : 			ast_variables_destroy(change_set);
       : 			ast_ari_response_alloc_failed(response);
       : 			return;
> tuple needs releasing here.
Fixed


Line 254: 	sorcery = ast_sorcery_retrieve_by_module_name(args->config_class);
        : 	if (!sorcery) {
        : 		ast_ari_response_error(
        : 			response, 404, "Not Found",
        : 			"configClass '%s' not found",
        : 			args->config_class);
        : 		return;
        : 	}
        : 
        : 	object_type = ast_sorcery_get_object_type(sorcery, args->object_type);
        : 	if (!object_type) {
        : 		ast_ari_response_error(
        : 			response, 404, "Not Found",
        : 			"objectType '%s' not found",
        : 			args->object_type);
        : 		return;
        : 	}
> This code is repeated several times. Would it make sense to abstract it out
I thought about doing that as well, but I don't think it will end up being much prettier. All of the code that uses this needs:
* The sorcery object itself
* The sorcery derived object

So that's two 'out' parameters.

Other code uses the object_type for better error responses and warning messages, which ends up being three 'out' parameters. Or we'd have a struct that contains these three parameters, and have a single 'out' parameter; that, however, will have some rather annoying cleanup semantics since it will contain 3 reference counted attributes.

Really, we're looking at condensing a few validation checks into a subroutine, which will either be rather ugly (3 out parameters) or only somewhat ugly (a single out parameter), but with some goofier cleanup semantics.

I can see what it would really look like if you feel strongly about it, but I think a little bit of repeated - but simple - extraction/validation logic is better than complexity.


-- 
To view, visit https://gerrit.asterisk.org/853
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I28cd5c7bf6f67f8e9e437ff097f8fd171d30ff5c
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes