pnoltes commented on code in PR #710: URL: https://github.com/apache/celix/pull/710#discussion_r1441600817
########## libs/utils/error_injector/celix_string_hash_map/include/celix_string_hash_map_ei.h: ########## @@ -17,8 +17,8 @@ * under the License. */ -#ifndef CELIX_CELIX_LONG_HASH_MAP_EI_H -#define CELIX_CELIX_LONG_HASH_MAP_EI_H +#ifndef CELIX_CELIX_STRING_HASH_MAP_EI_H Review Comment: nice catch ########## bundles/remote_services/rsa_spi/include/remote_constants.h: ########## @@ -37,13 +37,9 @@ static const char * const OSGI_RSA_SERVICE_EXPORTED_CONFIGS = "service.exported. static const char * const OSGI_RSA_SERVICE_LOCATION = "service.location"; /** - * Remote service admin property identifying the network interfaces that announce service. - * The property value is network interfaces name, it Can be a comma separated list of CIDR notation,eg:"eth0,en0". - * Default value is "", and the loopback interface will be used. Its exported service will be discovered - * only by other local clients on the same machine. - * If want to bound service to all network interfaces, we can set the property value to "all". + * It identify which types of remote service configurations are supported by a distribution provider. + * @ref https://docs.osgi.org/specification/osgi.cmpn/7.0.0/service.remoteservices.html#i1708968 */ -static const char * const CELIX_RSA_NETWORK_INTERFACES = "org.apache.celix.rsa.network.interfaces"; - +#define OSGI_RSA_REMOTE_CONFIGS_SUPPORTED "remote.configs.supported" Review Comment: nitpick: I prefer not to use OSGI_ prefixes. Apache Celix is inspired by OSGi, but there is not a native OSGi specification so IMO we should prefix with CELIX_ ########## bundles/remote_services/discovery_zeroconf/src/discovery_zeroconf_activator.c: ########## @@ -18,43 +18,116 @@ */ #include "discovery_zeroconf_announcer.h" #include "discovery_zeroconf_watcher.h" +#include "endpoint_listener.h" +#include "remote_constants.h" +#include "remote_service_admin.h" #include "celix_log_helper.h" #include "celix_bundle_activator.h" -#include "celix_types.h" +#include "celix_dm_component.h" +#include "celix_dm_service_dependency.h" +#include "celix_constants.h" #include "celix_errno.h" +#include <stdio.h> +#include <assert.h> typedef struct discovery_zeroconf_activator { celix_log_helper_t *logHelper; discovery_zeroconf_announcer_t *announcer; discovery_zeroconf_watcher_t *watcher; + endpoint_listener_t endpointListener; }discovery_zeroconf_activator_t; celix_status_t discoveryZeroconfActivator_start(discovery_zeroconf_activator_t *act, celix_bundle_context_t *ctx) { celix_status_t status = CELIX_SUCCESS; - celix_autoptr(celix_log_helper_t) logger = celix_logHelper_create(ctx,"celix_rsa_zeroconf_discovery"); + const char *fwUuid = celix_bundleContext_getProperty(ctx, CELIX_FRAMEWORK_UUID, NULL); + if (fwUuid == NULL) { + return CELIX_BUNDLE_EXCEPTION; + } + celix_autoptr(celix_log_helper_t) logger = celix_logHelper_create(ctx, "celix_rsa_zeroconf_discovery"); if (logger == NULL) { return CELIX_ENOMEM; } - celix_autoptr(discovery_zeroconf_announcer_t) announcer = NULL; - status = discoveryZeroconfAnnouncer_create(ctx, logger, &announcer); + //init announcer + celix_autoptr(celix_dm_component_t) announcerCmp = celix_dmComponent_create(ctx, "DZC_ANNOUNCER_CMP"); + if (announcerCmp == NULL) { + return CELIX_ENOMEM; + } + status = discoveryZeroconfAnnouncer_create(ctx, logger, &act->announcer); if (status != CELIX_SUCCESS) { return status; } - celix_autoptr(discovery_zeroconf_watcher_t) watcher = NULL; - status = discoveryZeroconfWatcher_create(ctx, logger, &watcher); + celix_dmComponent_setImplementation(announcerCmp, act->announcer); + CELIX_DM_COMPONENT_SET_IMPLEMENTATION_DESTROY_FUNCTION(announcerCmp, discovery_zeroconf_announcer_t, discoveryZeroconfAnnouncer_destroy); + celix_properties_t *props = celix_properties_create(); + if (props == NULL) { + return CELIX_ENOMEM; + } + char scope[256] = {0}; + (void)snprintf(scope, sizeof(scope), "(&(%s=*)(%s=%s))", CELIX_FRAMEWORK_SERVICE_NAME, + OSGI_RSA_ENDPOINT_FRAMEWORK_UUID, fwUuid); + celix_properties_set(props, OSGI_ENDPOINT_LISTENER_SCOPE, scope); + celix_properties_set(props, "DISCOVERY", "true");//Only use to avoid the discovery calls to unnecessary endpoint listener service.Endpoint should be filtered by the scope. Review Comment: I am not sure if I get this. As also stated in the comment, I expected that the OSGI_ENDPOINT_LISTENER_SCOPE was enough. Is the "DISCOVERY" property added for additional performance optimization? OSGi EndpointListener doc: > Endpoint Listener services can express their scope with the service property [ENDPOINT_LISTENER_SCOPE](https://docs.osgi.org/javadoc/osgi.cmpn/8.1.0/org/osgi/service/remoteserviceadmin/EndpointListener.html#ENDPOINT_LISTENER_SCOPE). This service property is a list of filters. An Endpoint Description should only be given to a Endpoint Listener when there is at least one filter that matches the Endpoint Description properties. This filter model is quite flexible. For example, a discovery bundle is only interested in locally originating Endpoint Descriptions. The following filter ensure that it only sees local endpoints. (org.osgi.framework.uuid=72dc5fd9-5f8f-4f8f-9821-9ebb433a5b72) ########## bundles/remote_services/discovery_configured/CMakeLists.txt: ########## @@ -21,6 +21,7 @@ if (RSA_DISCOVERY_CONFIGURED) VERSION 0.9.0 SYMBOLIC_NAME "apache_celix_rsa_discovery" NAME "Apache Celix RSA Configured Discovery" + FILENAME celix_rsa_discovery_configured Review Comment: 👍 Nice that the rsa bundles now have a filename with a celix_ prefix -- 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. To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
