[Asterisk-code-review] RFC: endpoint/stasis: Eliminate duplicate events on endpoint... (asterisk[13])

[Matt Jordan]

Matt Jordan has posted comments on this change.

Change subject: RFC: endpoint/stasis: Eliminate duplicate events on endpoint status change
......................................................................


Patch Set 1:

(2 comments)

https://gerrit.asterisk.org/#/c/570/1/main/stasis_cache.c
File main/stasis_cache.c:

Line 845: #if 0
        : 		else {
        : 			ast_log(LOG_ERROR,
        : 				"Attempting to remove an item from the %s cache that isn't there: %s %s\n",
        : 				stasis_topic_name(caching_topic->topic),
        : 				stasis_message_type_name(msg_type), msg_id);
        : 		}
        : #endif
I don't think this should be removed - looking for something in the cache after the final message has been pushed through Stasis is an error.

If the test is looking for something after it has been removed from the cache, then either the test is in error or we have an issue in the code.


https://gerrit.asterisk.org/#/c/570/1/tests/test_stasis_endpoints.c
File tests/test_stasis_endpoints.c:

Line 186: 	/* Note: there's a few messages between the creation and the clear.
        : 	 * Wait for all of them... */
        : 	message_index = stasis_message_sink_wait_for(sink, message_index + 4,
        : 		cache_update, __func__, STASIS_SINK_DEFAULT_WAIT);
        : 	ast_test_validate(test, 0 <= message_index);
        : 	/* Now we should have a cache removal entry */
        : 	msg = sink->messages[message_index];
        : 	type = stasis_message_type(msg);
        : 	ast_test_validate(test, stasis_cache_update_type() == type);
        : 	update = stasis_message_data(msg);
        : 	ast_test_validate(test, ast_endpoint_snapshot_type() == update->type);
        : 	actual_snapshot = stasis_message_data(update->old_snapshot);
        : 	ast_test_validate(test, 0 == strcmp("TEST", actual_snapshot->tech));
        : 	ast_test_validate(test,
        : 		0 == strcmp(__func__, actual_snapshot->resource));
        : 	ast_test_validate(test, NULL == update->new_snapshot);
While the number of messages may now be different, I'm confused why we would not have similar, if not tweaked, code after this patch.

What messages are being delivered with this patch? (Note: if you're not sure, let me know and I'll investigate a bit, since I've had to do this before)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie47784adfb973ab0063e59fc18f390d7dd26d17b
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: David M. Lee <dlee at digium.com>
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes