pnoltes commented on a change in pull request #264:
URL: https://github.com/apache/celix/pull/264#discussion_r447536602



##########
File path: bundles/pubsub/pubsub_admin_tcp/src/pubsub_tcp_admin.c
##########
@@ -610,15 +610,13 @@ celix_status_t pubsub_tcpAdmin_addDiscoveredEndpoint(void 
*handle, const celix_p
 
     if (pubsub_tcpAdmin_endpointIsPublisher(endpoint)) {
         celixThreadMutex_lock(&psa->topicReceivers.mutex);
-        const char *scope = celix_properties_get(endpoint, 
PUBSUB_ENDPOINT_TOPIC_SCOPE, NULL);
-        const char *topic = celix_properties_get(endpoint, 
PUBSUB_ENDPOINT_TOPIC_NAME, NULL);
-        char *key = pubsubEndpoint_createScopeTopicKey(scope, topic);
-
-        pubsub_tcp_topic_receiver_t *receiver = 
hashMap_get(psa->topicReceivers.map, key);
-        if (receiver != NULL) {
-            pubsub_tcpAdmin_connectEndpointToReceiver(psa, receiver, endpoint);
+        hash_map_iterator_t iter = 
hashMapIterator_construct(psa->topicReceivers.map);

Review comment:
       No, this is not possible. 
   
   If you have an subscriber with a topic value, but no scope. Note that this 
is now valid, because scope is not mandatory anymore. 
   This subscriber must be connected to all topic senders with a "default" 
scope. Also not that all endpoint have a scope value, if none is given this 
will be "default". 
   The key of the subscribers TopicReceiver will only be based on the topic 
value, no the scope. So you cannot retrieve the topic receiver from the map 
using a key created by the endpoints topic and scope values. 
   
   Also note that internally a subscriber and publisher with no scope value and 
with a "default" scope value need to be handled differently to ensure that 
filtering works for the no scope value or "default" scope value. 
   
   So looping through the topic receiver is IMO the cleanest path. Worst case 
scenario this is more overhead, but on average I think is the other way around 
(no need to alloc, initialize and free a key).  
   
   Also this is more future proof if we decide to allow wildchars (scope=* 
and/or topic=*), a feature already requested a few times. In the PR setup it 
only depends on the mathEndpointWithTopicAndScope call. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to