You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@aries.apache.org by "amichair (via GitHub)" <gi...@apache.org> on 2026/04/30 06:30:47 UTC

[PR] EventAdmin improvements [aries-rsa]

amichair opened a new pull request, #95:
URL: https://github.com/apache/aries-rsa/pull/95

   several fixes and improvements related to EventAdmin event publishing.


-- 
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-unsubscribe@aries.apache.org

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


Re: [PR] EventAdmin improvements [aries-rsa]

Posted by "amichair (via GitHub)" <gi...@apache.org>.
amichair commented on code in PR #95:
URL: https://github.com/apache/aries-rsa/pull/95#discussion_r3170610497


##########
rsa/src/main/java/org/apache/aries/rsa/core/event/EventAdminSender.java:
##########
@@ -18,85 +18,148 @@
  */
 package org.apache.aries.rsa.core.event;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.security.cert.X509Certificate;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceReference;
+import org.osgi.framework.Version;
 import org.osgi.service.event.Event;
 import org.osgi.service.event.EventAdmin;
 import org.osgi.service.remoteserviceadmin.EndpointDescription;
+import org.osgi.service.remoteserviceadmin.ExportReference;
 import org.osgi.service.remoteserviceadmin.ImportReference;
 import org.osgi.service.remoteserviceadmin.RemoteServiceAdminEvent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class EventAdminSender {
-    private HashMap<Integer, String> typeToTopic;
+
+    private static final Logger LOG = LoggerFactory.getLogger(EventAdminSender.class);
+
     private BundleContext context;
 
     public EventAdminSender(BundleContext context) {
         this.context = context;
-        typeToTopic = new HashMap<>();
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_ERROR, "EXPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_REGISTRATION, "EXPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UNREGISTRATION, "EXPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UPDATE, "EXPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_WARNING, "EXPORT_WARNING");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_ERROR, "IMPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_REGISTRATION, "IMPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UNREGISTRATION, "IMPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UPDATE, "IMPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_WARNING, "IMPORT_WARNING");
     }
 
-    public void send(RemoteServiceAdminEvent rsaEvent) {
-        final Event event = toEvent(rsaEvent);
-        ServiceReference<EventAdmin> sref = this.context.getServiceReference(EventAdmin.class);
+    private void notifyEventAdmins(String type, Event event) {
+        // according to the EventAdmin specs, we publish to the service with the
+        // highest ranking (which is the one returned by getServiceReference)
+        ServiceReference<EventAdmin> sref = context.getServiceReference(EventAdmin.class);

Review Comment:
   so that would be a DYNAMIC OPTIONAL GREEDY volatile reference with a null check and method-local reference? I don't know which is simpler to read/explain, so I left it the way it was :-)



-- 
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-unsubscribe@aries.apache.org

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


Re: [PR] EventAdmin improvements [aries-rsa]

Posted by "alien11689 (via GitHub)" <gi...@apache.org>.
alien11689 commented on code in PR #95:
URL: https://github.com/apache/aries-rsa/pull/95#discussion_r3170713264


##########
rsa/src/main/java/org/apache/aries/rsa/core/event/EventAdminSender.java:
##########
@@ -18,85 +18,148 @@
  */
 package org.apache.aries.rsa.core.event;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.security.cert.X509Certificate;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceReference;
+import org.osgi.framework.Version;
 import org.osgi.service.event.Event;
 import org.osgi.service.event.EventAdmin;
 import org.osgi.service.remoteserviceadmin.EndpointDescription;
+import org.osgi.service.remoteserviceadmin.ExportReference;
 import org.osgi.service.remoteserviceadmin.ImportReference;
 import org.osgi.service.remoteserviceadmin.RemoteServiceAdminEvent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class EventAdminSender {
-    private HashMap<Integer, String> typeToTopic;
+
+    private static final Logger LOG = LoggerFactory.getLogger(EventAdminSender.class);
+
     private BundleContext context;
 
     public EventAdminSender(BundleContext context) {
         this.context = context;
-        typeToTopic = new HashMap<>();
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_ERROR, "EXPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_REGISTRATION, "EXPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UNREGISTRATION, "EXPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UPDATE, "EXPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_WARNING, "EXPORT_WARNING");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_ERROR, "IMPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_REGISTRATION, "IMPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UNREGISTRATION, "IMPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UPDATE, "IMPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_WARNING, "IMPORT_WARNING");
     }
 
-    public void send(RemoteServiceAdminEvent rsaEvent) {
-        final Event event = toEvent(rsaEvent);
-        ServiceReference<EventAdmin> sref = this.context.getServiceReference(EventAdmin.class);
+    private void notifyEventAdmins(String type, Event event) {
+        // according to the EventAdmin specs, we publish to the service with the
+        // highest ranking (which is the one returned by getServiceReference)
+        ServiceReference<EventAdmin> sref = context.getServiceReference(EventAdmin.class);
         if (sref != null) {
-            final EventAdmin eventAdmin = this.context.getService(sref);
-            AccessController.doPrivileged(new PrivilegedAction<Void>() {
-                public Void run() {
-                    eventAdmin.postEvent(event);
-                    return null;
+            LOG.debug("Publishing event {} to EventAdmin", type);
+            final EventAdmin eventAdmin = context.getService(sref);
+            if (eventAdmin != null) {
+                try {
+                    AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
+                        eventAdmin.postEvent(event);
+                        return null;
+                    });
+                } finally {
+                    context.ungetService(sref);
                 }
-            });
+            }
             this.context.ungetService(sref);
         }
     }
 
-    private Event toEvent(RemoteServiceAdminEvent rsaEvent) {
-        String topic = getTopic(rsaEvent);
+    public void send(RemoteServiceAdminEvent rsae) {
+        String type = getConstName(RemoteServiceAdminEvent.class, null, rsae.getType(), "UNKNOWN_EVENT");
+        String topic = "org/osgi/service/remoteserviceadmin/" + type;
+        Map<String, Object> props = createProps(rsae);
+        Event event = new Event(topic, props);
+        notifyEventAdmins(type, event);
+    }
+
+    private static <K, V> void putIfNotNull(Map<K, V> map, K key, V val) {
+        if (val != null) {
+            map.put(key, val);
+        }
+    }
+
+    /**
+     * Returns the name of the first constant field (public static final) in the given class

Review Comment:
   Oh, why people are not using enums... it's like java 1.4...
   I would prefer not to rely on reflection here as an abstraction
   the usage is limites only to a single class and assume the field names starting with `IMPORT_` and `EXPORT_` - let's link in the javadoc to this class and make it explicit only for it https://docs.osgi.org/javadoc/osgi.cmpn/8.1.0/org/osgi/service/remoteserviceadmin/RemoteServiceAdminEvent.html 
   
   I don't think the pattern should be repeated (but I may be wrong...)
   
   other thing - can we use or get rid off prefix parameter? it's not used right now 
   



-- 
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-unsubscribe@aries.apache.org

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


Re: [PR] EventAdmin improvements [aries-rsa]

Posted by "alien11689 (via GitHub)" <gi...@apache.org>.
alien11689 commented on code in PR #95:
URL: https://github.com/apache/aries-rsa/pull/95#discussion_r3170800050


##########
rsa/src/main/java/org/apache/aries/rsa/core/event/EventAdminSender.java:
##########
@@ -18,85 +18,148 @@
  */
 package org.apache.aries.rsa.core.event;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.security.cert.X509Certificate;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceReference;
+import org.osgi.framework.Version;
 import org.osgi.service.event.Event;
 import org.osgi.service.event.EventAdmin;
 import org.osgi.service.remoteserviceadmin.EndpointDescription;
+import org.osgi.service.remoteserviceadmin.ExportReference;
 import org.osgi.service.remoteserviceadmin.ImportReference;
 import org.osgi.service.remoteserviceadmin.RemoteServiceAdminEvent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class EventAdminSender {
-    private HashMap<Integer, String> typeToTopic;
+
+    private static final Logger LOG = LoggerFactory.getLogger(EventAdminSender.class);
+
     private BundleContext context;
 
     public EventAdminSender(BundleContext context) {
         this.context = context;
-        typeToTopic = new HashMap<>();
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_ERROR, "EXPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_REGISTRATION, "EXPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UNREGISTRATION, "EXPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UPDATE, "EXPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_WARNING, "EXPORT_WARNING");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_ERROR, "IMPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_REGISTRATION, "IMPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UNREGISTRATION, "IMPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UPDATE, "IMPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_WARNING, "IMPORT_WARNING");
     }
 
-    public void send(RemoteServiceAdminEvent rsaEvent) {
-        final Event event = toEvent(rsaEvent);
-        ServiceReference<EventAdmin> sref = this.context.getServiceReference(EventAdmin.class);
+    private void notifyEventAdmins(String type, Event event) {
+        // according to the EventAdmin specs, we publish to the service with the
+        // highest ranking (which is the one returned by getServiceReference)
+        ServiceReference<EventAdmin> sref = context.getServiceReference(EventAdmin.class);
         if (sref != null) {
-            final EventAdmin eventAdmin = this.context.getService(sref);
-            AccessController.doPrivileged(new PrivilegedAction<Void>() {
-                public Void run() {
-                    eventAdmin.postEvent(event);
-                    return null;
+            LOG.debug("Publishing event {} to EventAdmin", type);
+            final EventAdmin eventAdmin = context.getService(sref);
+            if (eventAdmin != null) {
+                try {
+                    AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
+                        eventAdmin.postEvent(event);
+                        return null;
+                    });
+                } finally {
+                    context.ungetService(sref);
                 }
-            });
+            }
             this.context.ungetService(sref);
         }
     }
 
-    private Event toEvent(RemoteServiceAdminEvent rsaEvent) {
-        String topic = getTopic(rsaEvent);
+    public void send(RemoteServiceAdminEvent rsae) {
+        String type = getConstName(RemoteServiceAdminEvent.class, null, rsae.getType(), "UNKNOWN_EVENT");
+        String topic = "org/osgi/service/remoteserviceadmin/" + type;
+        Map<String, Object> props = createProps(rsae);
+        Event event = new Event(topic, props);
+        notifyEventAdmins(type, event);
+    }
+
+    private static <K, V> void putIfNotNull(Map<K, V> map, K key, V val) {
+        if (val != null) {
+            map.put(key, val);
+        }
+    }
+
+    /**
+     * Returns the name of the first constant field (public static final) in the given class

Review Comment:
   You have deleted a lot of code in the PR so this is the improvment :) 
   I am looking at the PRs holistically. Now I understand why you made a lookup via reflection - I would only like to limit the impact of the method because it's very limited
   it's your call if you want to drop a commit making a lookup



-- 
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-unsubscribe@aries.apache.org

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


Re: [PR] EventAdmin improvements [aries-rsa]

Posted by "amichair (via GitHub)" <gi...@apache.org>.
amichair commented on code in PR #95:
URL: https://github.com/apache/aries-rsa/pull/95#discussion_r3170443692


##########
rsa/src/main/java/org/apache/aries/rsa/core/event/EventAdminSender.java:
##########
@@ -18,85 +18,148 @@
  */
 package org.apache.aries.rsa.core.event;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.security.cert.X509Certificate;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceReference;
+import org.osgi.framework.Version;
 import org.osgi.service.event.Event;
 import org.osgi.service.event.EventAdmin;
 import org.osgi.service.remoteserviceadmin.EndpointDescription;
+import org.osgi.service.remoteserviceadmin.ExportReference;
 import org.osgi.service.remoteserviceadmin.ImportReference;
 import org.osgi.service.remoteserviceadmin.RemoteServiceAdminEvent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class EventAdminSender {
-    private HashMap<Integer, String> typeToTopic;
+
+    private static final Logger LOG = LoggerFactory.getLogger(EventAdminSender.class);
+
     private BundleContext context;
 
     public EventAdminSender(BundleContext context) {
         this.context = context;
-        typeToTopic = new HashMap<>();
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_ERROR, "EXPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_REGISTRATION, "EXPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UNREGISTRATION, "EXPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UPDATE, "EXPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_WARNING, "EXPORT_WARNING");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_ERROR, "IMPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_REGISTRATION, "IMPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UNREGISTRATION, "IMPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UPDATE, "IMPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_WARNING, "IMPORT_WARNING");
     }
 
-    public void send(RemoteServiceAdminEvent rsaEvent) {
-        final Event event = toEvent(rsaEvent);
-        ServiceReference<EventAdmin> sref = this.context.getServiceReference(EventAdmin.class);
+    private void notifyEventAdmins(String type, Event event) {
+        // according to the EventAdmin specs, we publish to the service with the
+        // highest ranking (which is the one returned by getServiceReference)
+        ServiceReference<EventAdmin> sref = context.getServiceReference(EventAdmin.class);
         if (sref != null) {
-            final EventAdmin eventAdmin = this.context.getService(sref);
-            AccessController.doPrivileged(new PrivilegedAction<Void>() {
-                public Void run() {
-                    eventAdmin.postEvent(event);
-                    return null;
+            LOG.debug("Publishing event {} to EventAdmin", type);
+            final EventAdmin eventAdmin = context.getService(sref);
+            if (eventAdmin != null) {
+                try {
+                    AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
+                        eventAdmin.postEvent(event);
+                        return null;
+                    });
+                } finally {
+                    context.ungetService(sref);

Review Comment:
   oops it got messed up when resolving conflicts when rewriting git history to organize the commits into a PR... originally there was just the outer one, and I moved it into a finally block, but the original sneaked back in. Good thing you caught it!



-- 
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-unsubscribe@aries.apache.org

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


Re: [PR] EventAdmin improvements [aries-rsa]

Posted by "amichair (via GitHub)" <gi...@apache.org>.
amichair merged PR #95:
URL: https://github.com/apache/aries-rsa/pull/95


-- 
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-unsubscribe@aries.apache.org

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


Re: [PR] EventAdmin improvements [aries-rsa]

Posted by "amichair (via GitHub)" <gi...@apache.org>.
amichair commented on code in PR #95:
URL: https://github.com/apache/aries-rsa/pull/95#discussion_r3170443692


##########
rsa/src/main/java/org/apache/aries/rsa/core/event/EventAdminSender.java:
##########
@@ -18,85 +18,148 @@
  */
 package org.apache.aries.rsa.core.event;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.security.cert.X509Certificate;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceReference;
+import org.osgi.framework.Version;
 import org.osgi.service.event.Event;
 import org.osgi.service.event.EventAdmin;
 import org.osgi.service.remoteserviceadmin.EndpointDescription;
+import org.osgi.service.remoteserviceadmin.ExportReference;
 import org.osgi.service.remoteserviceadmin.ImportReference;
 import org.osgi.service.remoteserviceadmin.RemoteServiceAdminEvent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class EventAdminSender {
-    private HashMap<Integer, String> typeToTopic;
+
+    private static final Logger LOG = LoggerFactory.getLogger(EventAdminSender.class);
+
     private BundleContext context;
 
     public EventAdminSender(BundleContext context) {
         this.context = context;
-        typeToTopic = new HashMap<>();
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_ERROR, "EXPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_REGISTRATION, "EXPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UNREGISTRATION, "EXPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UPDATE, "EXPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_WARNING, "EXPORT_WARNING");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_ERROR, "IMPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_REGISTRATION, "IMPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UNREGISTRATION, "IMPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UPDATE, "IMPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_WARNING, "IMPORT_WARNING");
     }
 
-    public void send(RemoteServiceAdminEvent rsaEvent) {
-        final Event event = toEvent(rsaEvent);
-        ServiceReference<EventAdmin> sref = this.context.getServiceReference(EventAdmin.class);
+    private void notifyEventAdmins(String type, Event event) {
+        // according to the EventAdmin specs, we publish to the service with the
+        // highest ranking (which is the one returned by getServiceReference)
+        ServiceReference<EventAdmin> sref = context.getServiceReference(EventAdmin.class);
         if (sref != null) {
-            final EventAdmin eventAdmin = this.context.getService(sref);
-            AccessController.doPrivileged(new PrivilegedAction<Void>() {
-                public Void run() {
-                    eventAdmin.postEvent(event);
-                    return null;
+            LOG.debug("Publishing event {} to EventAdmin", type);
+            final EventAdmin eventAdmin = context.getService(sref);
+            if (eventAdmin != null) {
+                try {
+                    AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
+                        eventAdmin.postEvent(event);
+                        return null;
+                    });
+                } finally {
+                    context.ungetService(sref);

Review Comment:
   oops it got messed up when resolving conflicts when rewriting git history to organize the commits into a PR... originally there was just the outer one, and I moved it a finally block, but the original sneaked back in. Good thing you caught it!



-- 
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-unsubscribe@aries.apache.org

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


Re: [PR] EventAdmin improvements [aries-rsa]

Posted by "alien11689 (via GitHub)" <gi...@apache.org>.
alien11689 commented on code in PR #95:
URL: https://github.com/apache/aries-rsa/pull/95#discussion_r3170079066


##########
rsa/src/main/java/org/apache/aries/rsa/core/event/EventAdminSender.java:
##########
@@ -18,85 +18,148 @@
  */
 package org.apache.aries.rsa.core.event;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.security.cert.X509Certificate;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceReference;
+import org.osgi.framework.Version;
 import org.osgi.service.event.Event;
 import org.osgi.service.event.EventAdmin;
 import org.osgi.service.remoteserviceadmin.EndpointDescription;
+import org.osgi.service.remoteserviceadmin.ExportReference;
 import org.osgi.service.remoteserviceadmin.ImportReference;
 import org.osgi.service.remoteserviceadmin.RemoteServiceAdminEvent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class EventAdminSender {
-    private HashMap<Integer, String> typeToTopic;
+
+    private static final Logger LOG = LoggerFactory.getLogger(EventAdminSender.class);
+
     private BundleContext context;
 
     public EventAdminSender(BundleContext context) {
         this.context = context;
-        typeToTopic = new HashMap<>();
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_ERROR, "EXPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_REGISTRATION, "EXPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UNREGISTRATION, "EXPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UPDATE, "EXPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_WARNING, "EXPORT_WARNING");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_ERROR, "IMPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_REGISTRATION, "IMPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UNREGISTRATION, "IMPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UPDATE, "IMPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_WARNING, "IMPORT_WARNING");
     }
 
-    public void send(RemoteServiceAdminEvent rsaEvent) {
-        final Event event = toEvent(rsaEvent);
-        ServiceReference<EventAdmin> sref = this.context.getServiceReference(EventAdmin.class);
+    private void notifyEventAdmins(String type, Event event) {
+        // according to the EventAdmin specs, we publish to the service with the
+        // highest ranking (which is the one returned by getServiceReference)
+        ServiceReference<EventAdmin> sref = context.getServiceReference(EventAdmin.class);
         if (sref != null) {
-            final EventAdmin eventAdmin = this.context.getService(sref);
-            AccessController.doPrivileged(new PrivilegedAction<Void>() {
-                public Void run() {
-                    eventAdmin.postEvent(event);
-                    return null;
+            LOG.debug("Publishing event {} to EventAdmin", type);
+            final EventAdmin eventAdmin = context.getService(sref);
+            if (eventAdmin != null) {
+                try {
+                    AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
+                        eventAdmin.postEvent(event);
+                        return null;
+                    });
+                } finally {
+                    context.ungetService(sref);
                 }
-            });
+            }
             this.context.ungetService(sref);
         }
     }
 
-    private Event toEvent(RemoteServiceAdminEvent rsaEvent) {
-        String topic = getTopic(rsaEvent);
+    public void send(RemoteServiceAdminEvent rsae) {
+        String type = getConstName(RemoteServiceAdminEvent.class, null, rsae.getType(), "UNKNOWN_EVENT");
+        String topic = "org/osgi/service/remoteserviceadmin/" + type;
+        Map<String, Object> props = createProps(rsae);
+        Event event = new Event(topic, props);
+        notifyEventAdmins(type, event);
+    }
+
+    private static <K, V> void putIfNotNull(Map<K, V> map, K key, V val) {
+        if (val != null) {
+            map.put(key, val);
+        }
+    }
+
+    /**
+     * Returns the name of the first constant field (public static final) in the given class

Review Comment:
   is it a very strange coupling that the first final static field needs to be the event type name? maybe we can focus on the specific field name? ordring seems to be fragile



##########
rsa/src/main/java/org/apache/aries/rsa/core/event/EventAdminSender.java:
##########
@@ -18,85 +18,148 @@
  */
 package org.apache.aries.rsa.core.event;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.security.cert.X509Certificate;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceReference;
+import org.osgi.framework.Version;
 import org.osgi.service.event.Event;
 import org.osgi.service.event.EventAdmin;
 import org.osgi.service.remoteserviceadmin.EndpointDescription;
+import org.osgi.service.remoteserviceadmin.ExportReference;
 import org.osgi.service.remoteserviceadmin.ImportReference;
 import org.osgi.service.remoteserviceadmin.RemoteServiceAdminEvent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class EventAdminSender {
-    private HashMap<Integer, String> typeToTopic;
+
+    private static final Logger LOG = LoggerFactory.getLogger(EventAdminSender.class);
+
     private BundleContext context;
 
     public EventAdminSender(BundleContext context) {
         this.context = context;
-        typeToTopic = new HashMap<>();
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_ERROR, "EXPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_REGISTRATION, "EXPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UNREGISTRATION, "EXPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UPDATE, "EXPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_WARNING, "EXPORT_WARNING");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_ERROR, "IMPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_REGISTRATION, "IMPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UNREGISTRATION, "IMPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UPDATE, "IMPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_WARNING, "IMPORT_WARNING");
     }
 
-    public void send(RemoteServiceAdminEvent rsaEvent) {
-        final Event event = toEvent(rsaEvent);
-        ServiceReference<EventAdmin> sref = this.context.getServiceReference(EventAdmin.class);
+    private void notifyEventAdmins(String type, Event event) {
+        // according to the EventAdmin specs, we publish to the service with the
+        // highest ranking (which is the one returned by getServiceReference)
+        ServiceReference<EventAdmin> sref = context.getServiceReference(EventAdmin.class);
         if (sref != null) {
-            final EventAdmin eventAdmin = this.context.getService(sref);
-            AccessController.doPrivileged(new PrivilegedAction<Void>() {
-                public Void run() {
-                    eventAdmin.postEvent(event);
-                    return null;
+            LOG.debug("Publishing event {} to EventAdmin", type);
+            final EventAdmin eventAdmin = context.getService(sref);
+            if (eventAdmin != null) {
+                try {
+                    AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
+                        eventAdmin.postEvent(event);
+                        return null;
+                    });
+                } finally {
+                    context.ungetService(sref);

Review Comment:
   are we ungetting service twice?



##########
rsa/src/main/java/org/apache/aries/rsa/core/event/EventAdminSender.java:
##########
@@ -18,85 +18,148 @@
  */
 package org.apache.aries.rsa.core.event;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.security.cert.X509Certificate;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceReference;
+import org.osgi.framework.Version;
 import org.osgi.service.event.Event;
 import org.osgi.service.event.EventAdmin;
 import org.osgi.service.remoteserviceadmin.EndpointDescription;
+import org.osgi.service.remoteserviceadmin.ExportReference;
 import org.osgi.service.remoteserviceadmin.ImportReference;
 import org.osgi.service.remoteserviceadmin.RemoteServiceAdminEvent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class EventAdminSender {
-    private HashMap<Integer, String> typeToTopic;
+
+    private static final Logger LOG = LoggerFactory.getLogger(EventAdminSender.class);
+
     private BundleContext context;
 
     public EventAdminSender(BundleContext context) {
         this.context = context;
-        typeToTopic = new HashMap<>();
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_ERROR, "EXPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_REGISTRATION, "EXPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UNREGISTRATION, "EXPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UPDATE, "EXPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_WARNING, "EXPORT_WARNING");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_ERROR, "IMPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_REGISTRATION, "IMPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UNREGISTRATION, "IMPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UPDATE, "IMPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_WARNING, "IMPORT_WARNING");
     }
 
-    public void send(RemoteServiceAdminEvent rsaEvent) {
-        final Event event = toEvent(rsaEvent);
-        ServiceReference<EventAdmin> sref = this.context.getServiceReference(EventAdmin.class);
+    private void notifyEventAdmins(String type, Event event) {
+        // according to the EventAdmin specs, we publish to the service with the
+        // highest ranking (which is the one returned by getServiceReference)
+        ServiceReference<EventAdmin> sref = context.getServiceReference(EventAdmin.class);

Review Comment:
   can we use DS instead of getting the servuce for each event?



-- 
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-unsubscribe@aries.apache.org

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


Re: [PR] EventAdmin improvements [aries-rsa]

Posted by "amichair (via GitHub)" <gi...@apache.org>.
amichair commented on code in PR #95:
URL: https://github.com/apache/aries-rsa/pull/95#discussion_r3170755171


##########
rsa/src/main/java/org/apache/aries/rsa/core/event/EventAdminSender.java:
##########
@@ -18,85 +18,148 @@
  */
 package org.apache.aries.rsa.core.event;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.security.cert.X509Certificate;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceReference;
+import org.osgi.framework.Version;
 import org.osgi.service.event.Event;
 import org.osgi.service.event.EventAdmin;
 import org.osgi.service.remoteserviceadmin.EndpointDescription;
+import org.osgi.service.remoteserviceadmin.ExportReference;
 import org.osgi.service.remoteserviceadmin.ImportReference;
 import org.osgi.service.remoteserviceadmin.RemoteServiceAdminEvent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class EventAdminSender {
-    private HashMap<Integer, String> typeToTopic;
+
+    private static final Logger LOG = LoggerFactory.getLogger(EventAdminSender.class);
+
     private BundleContext context;
 
     public EventAdminSender(BundleContext context) {
         this.context = context;
-        typeToTopic = new HashMap<>();
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_ERROR, "EXPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_REGISTRATION, "EXPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UNREGISTRATION, "EXPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UPDATE, "EXPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_WARNING, "EXPORT_WARNING");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_ERROR, "IMPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_REGISTRATION, "IMPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UNREGISTRATION, "IMPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UPDATE, "IMPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_WARNING, "IMPORT_WARNING");
     }
 
-    public void send(RemoteServiceAdminEvent rsaEvent) {
-        final Event event = toEvent(rsaEvent);
-        ServiceReference<EventAdmin> sref = this.context.getServiceReference(EventAdmin.class);
+    private void notifyEventAdmins(String type, Event event) {
+        // according to the EventAdmin specs, we publish to the service with the
+        // highest ranking (which is the one returned by getServiceReference)
+        ServiceReference<EventAdmin> sref = context.getServiceReference(EventAdmin.class);
         if (sref != null) {
-            final EventAdmin eventAdmin = this.context.getService(sref);
-            AccessController.doPrivileged(new PrivilegedAction<Void>() {
-                public Void run() {
-                    eventAdmin.postEvent(event);
-                    return null;
+            LOG.debug("Publishing event {} to EventAdmin", type);
+            final EventAdmin eventAdmin = context.getService(sref);
+            if (eventAdmin != null) {
+                try {
+                    AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
+                        eventAdmin.postEvent(event);
+                        return null;
+                    });
+                } finally {
+                    context.ungetService(sref);
                 }
-            });
+            }
             this.context.ungetService(sref);
         }
     }
 
-    private Event toEvent(RemoteServiceAdminEvent rsaEvent) {
-        String topic = getTopic(rsaEvent);
+    public void send(RemoteServiceAdminEvent rsae) {
+        String type = getConstName(RemoteServiceAdminEvent.class, null, rsae.getType(), "UNKNOWN_EVENT");
+        String topic = "org/osgi/service/remoteserviceadmin/" + type;
+        Map<String, Object> props = createProps(rsae);
+        Event event = new Event(topic, props);
+        notifyEventAdmins(type, event);
+    }
+
+    private static <K, V> void putIfNotNull(Map<K, V> map, K key, V val) {
+        if (val != null) {
+            map.put(key, val);
+        }
+    }
+
+    /**
+     * Returns the name of the first constant field (public static final) in the given class

Review Comment:
   until the last line I thought you were saying to just delete this commit and stay with the map and strings... but then the last line makes it seem like u want to leave the utility method? Now I'm confused ;-)
   And following the discussion, I'm gonna change my mind and say it's not worth the trouble, maybe just delete the commit and leave it the way it was. ok?



-- 
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-unsubscribe@aries.apache.org

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


Re: [PR] EventAdmin improvements [aries-rsa]

Posted by "amichair (via GitHub)" <gi...@apache.org>.
amichair commented on code in PR #95:
URL: https://github.com/apache/aries-rsa/pull/95#discussion_r3171144143


##########
rsa/src/main/java/org/apache/aries/rsa/core/event/EventAdminSender.java:
##########
@@ -18,85 +18,148 @@
  */
 package org.apache.aries.rsa.core.event;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.security.cert.X509Certificate;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceReference;
+import org.osgi.framework.Version;
 import org.osgi.service.event.Event;
 import org.osgi.service.event.EventAdmin;
 import org.osgi.service.remoteserviceadmin.EndpointDescription;
+import org.osgi.service.remoteserviceadmin.ExportReference;
 import org.osgi.service.remoteserviceadmin.ImportReference;
 import org.osgi.service.remoteserviceadmin.RemoteServiceAdminEvent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class EventAdminSender {
-    private HashMap<Integer, String> typeToTopic;
+
+    private static final Logger LOG = LoggerFactory.getLogger(EventAdminSender.class);
+
     private BundleContext context;
 
     public EventAdminSender(BundleContext context) {
         this.context = context;
-        typeToTopic = new HashMap<>();
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_ERROR, "EXPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_REGISTRATION, "EXPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UNREGISTRATION, "EXPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UPDATE, "EXPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_WARNING, "EXPORT_WARNING");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_ERROR, "IMPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_REGISTRATION, "IMPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UNREGISTRATION, "IMPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UPDATE, "IMPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_WARNING, "IMPORT_WARNING");
     }
 
-    public void send(RemoteServiceAdminEvent rsaEvent) {
-        final Event event = toEvent(rsaEvent);
-        ServiceReference<EventAdmin> sref = this.context.getServiceReference(EventAdmin.class);
+    private void notifyEventAdmins(String type, Event event) {
+        // according to the EventAdmin specs, we publish to the service with the
+        // highest ranking (which is the one returned by getServiceReference)
+        ServiceReference<EventAdmin> sref = context.getServiceReference(EventAdmin.class);
         if (sref != null) {
-            final EventAdmin eventAdmin = this.context.getService(sref);
-            AccessController.doPrivileged(new PrivilegedAction<Void>() {
-                public Void run() {
-                    eventAdmin.postEvent(event);
-                    return null;
+            LOG.debug("Publishing event {} to EventAdmin", type);
+            final EventAdmin eventAdmin = context.getService(sref);
+            if (eventAdmin != null) {
+                try {
+                    AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
+                        eventAdmin.postEvent(event);
+                        return null;
+                    });
+                } finally {
+                    context.ungetService(sref);
                 }
-            });
+            }
             this.context.ungetService(sref);
         }
     }
 
-    private Event toEvent(RemoteServiceAdminEvent rsaEvent) {
-        String topic = getTopic(rsaEvent);
+    public void send(RemoteServiceAdminEvent rsae) {
+        String type = getConstName(RemoteServiceAdminEvent.class, null, rsae.getType(), "UNKNOWN_EVENT");
+        String topic = "org/osgi/service/remoteserviceadmin/" + type;
+        Map<String, Object> props = createProps(rsae);
+        Event event = new Event(topic, props);
+        notifyEventAdmins(type, event);
+    }
+
+    private static <K, V> void putIfNotNull(Map<K, V> map, K key, V val) {
+        if (val != null) {
+            map.put(key, val);
+        }
+    }
+
+    /**
+     * Returns the name of the first constant field (public static final) in the given class

Review Comment:
   ok now I went a different route with a switch statement, tiny change but a bit more contained and efficient.



-- 
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-unsubscribe@aries.apache.org

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


Re: [PR] EventAdmin improvements [aries-rsa]

Posted by "amichair (via GitHub)" <gi...@apache.org>.
amichair commented on code in PR #95:
URL: https://github.com/apache/aries-rsa/pull/95#discussion_r3170480902


##########
rsa/src/main/java/org/apache/aries/rsa/core/event/EventAdminSender.java:
##########
@@ -18,85 +18,148 @@
  */
 package org.apache.aries.rsa.core.event;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.security.cert.X509Certificate;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceReference;
+import org.osgi.framework.Version;
 import org.osgi.service.event.Event;
 import org.osgi.service.event.EventAdmin;
 import org.osgi.service.remoteserviceadmin.EndpointDescription;
+import org.osgi.service.remoteserviceadmin.ExportReference;
 import org.osgi.service.remoteserviceadmin.ImportReference;
 import org.osgi.service.remoteserviceadmin.RemoteServiceAdminEvent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class EventAdminSender {
-    private HashMap<Integer, String> typeToTopic;
+
+    private static final Logger LOG = LoggerFactory.getLogger(EventAdminSender.class);
+
     private BundleContext context;
 
     public EventAdminSender(BundleContext context) {
         this.context = context;
-        typeToTopic = new HashMap<>();
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_ERROR, "EXPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_REGISTRATION, "EXPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UNREGISTRATION, "EXPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_UPDATE, "EXPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.EXPORT_WARNING, "EXPORT_WARNING");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_ERROR, "IMPORT_ERROR");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_REGISTRATION, "IMPORT_REGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UNREGISTRATION, "IMPORT_UNREGISTRATION");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_UPDATE, "IMPORT_UPDATE");
-        typeToTopic.put(RemoteServiceAdminEvent.IMPORT_WARNING, "IMPORT_WARNING");
     }
 
-    public void send(RemoteServiceAdminEvent rsaEvent) {
-        final Event event = toEvent(rsaEvent);
-        ServiceReference<EventAdmin> sref = this.context.getServiceReference(EventAdmin.class);
+    private void notifyEventAdmins(String type, Event event) {
+        // according to the EventAdmin specs, we publish to the service with the
+        // highest ranking (which is the one returned by getServiceReference)
+        ServiceReference<EventAdmin> sref = context.getServiceReference(EventAdmin.class);
         if (sref != null) {
-            final EventAdmin eventAdmin = this.context.getService(sref);
-            AccessController.doPrivileged(new PrivilegedAction<Void>() {
-                public Void run() {
-                    eventAdmin.postEvent(event);
-                    return null;
+            LOG.debug("Publishing event {} to EventAdmin", type);
+            final EventAdmin eventAdmin = context.getService(sref);
+            if (eventAdmin != null) {
+                try {
+                    AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
+                        eventAdmin.postEvent(event);
+                        return null;
+                    });
+                } finally {
+                    context.ungetService(sref);
                 }
-            });
+            }
             this.context.ungetService(sref);
         }
     }
 
-    private Event toEvent(RemoteServiceAdminEvent rsaEvent) {
-        String topic = getTopic(rsaEvent);
+    public void send(RemoteServiceAdminEvent rsae) {
+        String type = getConstName(RemoteServiceAdminEvent.class, null, rsae.getType(), "UNKNOWN_EVENT");
+        String topic = "org/osgi/service/remoteserviceadmin/" + type;
+        Map<String, Object> props = createProps(rsae);
+        Event event = new Event(topic, props);
+        notifyEventAdmins(type, event);
+    }
+
+    private static <K, V> void putIfNotNull(Map<K, V> map, K key, V val) {
+        if (val != null) {
+            map.put(key, val);
+        }
+    }
+
+    /**
+     * Returns the name of the first constant field (public static final) in the given class

Review Comment:
   all constants in RemoteServiceAdminEvent are event types, so there is no current issue in practice. The prefix param is there in case we want to narrow it down to a subset of constant names (EXPORT_ and IMPORT_ in this case), but that's not bullet proof either. If you don't like this change I can delete this commit and go back to the hard-coded map, but it's a trade-off, because then the code breaks with certainty if any constants are added anyway. I think the reflection is a tad safer and nicer than a hard coded list in this specific case, but it's not a clear cut decision. Whatever you say.



-- 
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-unsubscribe@aries.apache.org

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