This is an automated email from the ASF dual-hosted git repository.

cziegeler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/felix-dev.git


The following commit(s) were added to refs/heads/master by this push:
     new 072a0dc86f FELIX-6607 consider service ranking for conflicting web 
console plugins (#210)
072a0dc86f is described below

commit 072a0dc86f5d2b931664e621e9215ae099190999
Author: Konrad Windszus <[email protected]>
AuthorDate: Mon Jun 5 07:08:56 2023 +0200

    FELIX-6607 consider service ranking for conflicting web console plugins 
(#210)
    
    * FELIX-6607 consider service ranking for conflicting web console plugins
    
    * Make internal plugin have service.ranking = 0 and service.id = 0
---
 .../webconsole/internal/servlet/OsgiManager.java   |   8 +-
 .../webconsole/internal/servlet/PluginHolder.java  | 182 ++++++++++-----------
 2 files changed, 95 insertions(+), 95 deletions(-)

diff --git 
a/webconsole/src/main/java/org/apache/felix/webconsole/internal/servlet/OsgiManager.java
 
b/webconsole/src/main/java/org/apache/felix/webconsole/internal/servlet/OsgiManager.java
index 0d22599a24..5f69be2115 100644
--- 
a/webconsole/src/main/java/org/apache/felix/webconsole/internal/servlet/OsgiManager.java
+++ 
b/webconsole/src/main/java/org/apache/felix/webconsole/internal/servlet/OsgiManager.java
@@ -274,14 +274,14 @@ public class OsgiManager extends GenericServlet
     public OsgiManager(BundleContext bundleContext)
     {
         this.bundleContext = bundleContext;
-        this.holder = new PluginHolder(bundleContext);
+        this.holder = new PluginHolder(this, bundleContext);
 
         // new plugins setup
         for (int i = 0; i < PLUGIN_MAP.length; i++)
         {
             final String pluginClassName = PLUGIN_MAP[i++];
             final String label = PLUGIN_MAP[i];
-            holder.addInternalPlugin(this, pluginClassName, label);
+            holder.addInternalPlugin(pluginClassName, label);
         }
 
         // setup the included plugins
@@ -1192,14 +1192,14 @@ public class OsgiManager extends GenericServlet
             {
                 if (active)
                 {
-                    holder.removeOsgiManagerPlugin(label);
+                    holder.removeInternalPlugin(label);
                 }
             }
             else
             {
                 if (!active)
                 {
-                    holder.addInternalPlugin(this, pluginClassName, label);
+                    holder.addInternalPlugin(pluginClassName, label);
                 }
             }
         }
diff --git 
a/webconsole/src/main/java/org/apache/felix/webconsole/internal/servlet/PluginHolder.java
 
b/webconsole/src/main/java/org/apache/felix/webconsole/internal/servlet/PluginHolder.java
index d95d459b09..8b22be5e80 100644
--- 
a/webconsole/src/main/java/org/apache/felix/webconsole/internal/servlet/PluginHolder.java
+++ 
b/webconsole/src/main/java/org/apache/felix/webconsole/internal/servlet/PluginHolder.java
@@ -53,11 +53,13 @@ import org.osgi.service.log.LogService;
 class PluginHolder implements ServiceListener
 {
 
+    private final OsgiManager osgiManager;
+
     // The Web Console's bundle context to access the plugin services
     private final BundleContext bundleContext;
 
-    // registered plugins (Map<String label, Plugin plugin>)
-    private final Map plugins;
+    // registered plugins
+    private final Map<String, Plugin> plugins;
 
     // The servlet context used to initialize plugin services
     private ServletContext servletContext;
@@ -66,10 +68,12 @@ class PluginHolder implements ServiceListener
     private String defaultPluginLabel;
 
 
-    PluginHolder( final BundleContext context )
+
+    PluginHolder( final OsgiManager osgiManager, final BundleContext context )
     {
+        this.osgiManager = osgiManager;
         this.bundleContext = context;
-        this.plugins = new HashMap();
+        this.plugins = new HashMap<>();
     }
 
 
@@ -95,7 +99,7 @@ class PluginHolder implements ServiceListener
 
         try
         {
-            ServiceReference[] refs = bundleContext.getServiceReferences( 
WebConsoleConstants.SERVICE_NAME, null );
+            ServiceReference<?>[] refs = bundleContext.getServiceReferences( 
WebConsoleConstants.SERVICE_NAME, null );
             if ( refs != null )
             {
                 for ( int i = 0; i < refs.length; i++ )
@@ -151,34 +155,21 @@ class PluginHolder implements ServiceListener
         this.defaultPluginLabel = defaultPluginLabel;
     }
 
-    void addInternalPlugin( final OsgiManager osgiManager, final String 
pluginClassName, final String label)
+    void addInternalPlugin( final String pluginClassName, final String label)
     {
         final Plugin plugin = new InternalPlugin(this, osgiManager, 
pluginClassName, label);
         addPlugin( label, plugin );
     }
 
-    /**
-     * Adds an internal Web Console plugin
-     * @param consolePlugin The internal Web Console plugin to add
-     */
-    void addOsgiManagerPlugin( final AbstractWebConsolePlugin consolePlugin )
-    {
-        final String label = consolePlugin.getLabel();
-        final Plugin plugin = new Plugin( this, consolePlugin, label );
-        addPlugin( label, plugin );
-    }
-
-
     /**
      * Remove the internal Web Console plugin registered under the given label
      * @param label The label of the Web Console internal plugin to remove
      */
-    void removeOsgiManagerPlugin( final String label )
+    void removeInternalPlugin( final String label )
     {
         removePlugin( label );
     }
 
-
     /**
      * Returns the plugin registered under the given label or <code>null</code>
      * if none is registered under that label. If the label is 
<code>null</code>
@@ -197,7 +188,7 @@ class PluginHolder implements ServiceListener
             final Plugin plugin;
             synchronized ( plugins )
             {
-                plugin = ( Plugin ) plugins.get( label );
+                plugin = plugins.get( label );
             }
 
             if ( plugin != null )
@@ -402,7 +393,7 @@ class PluginHolder implements ServiceListener
     }
 
 
-    private void serviceAdded( final ServiceReference serviceReference )
+    private void serviceAdded( final ServiceReference<?> serviceReference )
     {
         final String label = getProperty( serviceReference, 
WebConsoleConstants.PLUGIN_LABEL );
         if ( label != null )
@@ -412,7 +403,7 @@ class PluginHolder implements ServiceListener
     }
 
 
-    private void serviceRemoved( final ServiceReference serviceReference )
+    private void serviceRemoved( final ServiceReference<?> serviceReference )
     {
         final String label = getProperty( serviceReference, 
WebConsoleConstants.PLUGIN_LABEL );
         if ( label != null )
@@ -426,6 +417,20 @@ class PluginHolder implements ServiceListener
     {
         synchronized ( plugins )
         {
+            plugins.compute( label, (key, oldPlugin) -> {
+                if (oldPlugin != null) {
+                    if (plugin.compareTo(oldPlugin) > 0) {
+                        osgiManager.log(LogService.LOG_WARNING, "Overwriting 
existing plugin " + oldPlugin.doGetConsolePlugin().getClass().getName() 
+                                + " having label " + key + " with new plugin " 
+ plugin.doGetConsolePlugin().getClass().getName()
+                                + " due to higher ranking " );
+                    } else {
+                        osgiManager.log(LogService.LOG_WARNING, "Ignoring new 
plugin " + plugin.doGetConsolePlugin().getClass().getName()
+                                + " having existing label " + key + " due to 
lower ranking than old plugin " +  
oldPlugin.doGetConsolePlugin().getClass().getName() );
+                        return oldPlugin;
+                    }
+                }
+                return plugin;
+            });
             plugins.put( label, plugin );
         }
     }
@@ -436,7 +441,7 @@ class PluginHolder implements ServiceListener
         final Plugin oldPlugin;
         synchronized ( plugins )
         {
-            oldPlugin = ( Plugin ) plugins.remove( label );
+            oldPlugin = plugins.remove( label );
         }
 
         if ( oldPlugin != null )
@@ -450,12 +455,12 @@ class PluginHolder implements ServiceListener
     {
         synchronized ( plugins )
         {
-            return ( Plugin[] ) plugins.values().toArray( new 
Plugin[plugins.size()] );
+            return plugins.values().toArray( new Plugin[plugins.size()] );
         }
     }
 
 
-    static String getProperty( final ServiceReference service, final String 
propertyName )
+    static String getProperty( final ServiceReference<?> service, final String 
propertyName )
     {
         final Object property = service.getProperty( propertyName );
         if ( property instanceof String )
@@ -466,43 +471,26 @@ class PluginHolder implements ServiceListener
         return null;
     }
 
-    private static class Plugin implements ServletConfig
+    private abstract static class Plugin implements ServletConfig, 
Comparable<Plugin>
     {
         private final PluginHolder holder;
+        protected final ServiceReference<?> serviceReference; // used for 
comparing conflicting services
         private final String label;
         private String title;
-        private AbstractWebConsolePlugin consolePlugin;
 
-        protected Plugin( final PluginHolder holder, final String label )
+        protected Plugin( final PluginHolder holder, final ServiceReference<?> 
serviceReference, final String label )
         {
             this.holder = holder;
+            this.serviceReference = serviceReference;
             this.label = label;
         }
 
 
-        protected Plugin( final PluginHolder holder, final 
AbstractWebConsolePlugin plugin, final String label )
-        {
-            this( holder, label );
-
-            if ( plugin == null )
-            {
-                throw new NullPointerException( "plugin" );
-            }
-
-            this.consolePlugin = plugin;
-        }
-
         void init() throws ServletException {
-            if (consolePlugin != null) {
-                consolePlugin.init( this );
-            }
         }
 
         void destroy()
         {
-            if (consolePlugin != null) {
-                consolePlugin.destroy();
-            }
         }
 
         /**
@@ -512,21 +500,47 @@ class PluginHolder implements ServiceListener
          */
         final void dispose()
         {
-            if ( consolePlugin != null )
+        }
+
+        @Override
+        public int compareTo(Plugin other)
+        {
+            // serviceReference = null means internal (i.e. service.ranking=0 
and service.id=0)
+            // mostly a copy from 
org.apache.felix.framework.ServiceRegistrationImpl.ServiceReferenceImpl
+
+            Long id = serviceReference != null ? (Long) 
serviceReference.getProperty(Constants.SERVICE_ID) : 0;
+            Long otherId = other.serviceReference != null ? (Long) 
other.serviceReference.getProperty(Constants.SERVICE_ID) : 0;
+
+            if (id.equals(otherId))
             {
-                try
-                {
-                    consolePlugin.destroy();
-                }
-                catch ( Exception e )
-                {
-                    // TODO: handle
-                }
+                return 0; // same service
+            }
 
-                doUngetConsolePlugin( consolePlugin );
+            Object rankObj = serviceReference != null ? 
serviceReference.getProperty(Constants.SERVICE_RANKING) : null;
+            Object otherObj = other.serviceReference != null ? 
other.serviceReference.getProperty(Constants.SERVICE_RANKING) : null;
 
-                consolePlugin = null;
+            // If no rank, then spec says it defaults to zero.
+            rankObj = (rankObj == null) ? new Integer(0) : rankObj;
+            otherObj = (otherObj == null) ? new Integer(0) : otherObj;
+
+            // If rank is not Integer, then spec says it defaults to zero.
+            Integer rank = (rankObj instanceof Integer)
+                ? (Integer) rankObj : new Integer(0);
+            Integer otherRank = (otherObj instanceof Integer)
+                ? (Integer) otherObj : new Integer(0);
+
+            // Sort by rank in ascending order.
+            if (rank.compareTo(otherRank) < 0)
+            {
+                return -1; // lower rank
             }
+            else if (rank.compareTo(otherRank) > 0)
+            {
+                return 1; // higher rank
+            }
+
+            // If ranks are equal, then sort by service id in descending order.
+            return (id.compareTo(otherId) < 0) ? 1 : -1;
         }
 
 
@@ -589,24 +603,19 @@ class PluginHolder implements ServiceListener
 
         final AbstractWebConsolePlugin getConsolePlugin()
         {
-            if ( consolePlugin == null )
+            final AbstractWebConsolePlugin consolePlugin = 
doGetConsolePlugin();
+            if ( consolePlugin != null )
             {
-                final AbstractWebConsolePlugin consolePlugin = 
doGetConsolePlugin();
-                if ( consolePlugin != null )
+                try
                 {
-                    try
-                    {
-                        this.consolePlugin = consolePlugin;
-                        init();
-                    }
-                    catch ( ServletException se )
-                    {
-                        // TODO: log
-                        this.consolePlugin = null;
-                    }
-                } else {
-                    // TODO: log !!
+                    init();
                 }
+                catch ( ServletException se )
+                {
+                    // TODO: log
+                }
+            } else {
+                // TODO: log !!
             }
             return consolePlugin;
         }
@@ -615,10 +624,7 @@ class PluginHolder implements ServiceListener
             return true;
         }
 
-        protected AbstractWebConsolePlugin doGetConsolePlugin()
-        {
-            return consolePlugin;
-        }
+        protected abstract AbstractWebConsolePlugin doGetConsolePlugin();
 
 
         protected void doUngetConsolePlugin( AbstractWebConsolePlugin 
consolePlugin )
@@ -634,9 +640,9 @@ class PluginHolder implements ServiceListener
         }
 
 
-        public Enumeration getInitParameterNames()
+        public Enumeration<?> getInitParameterNames()
         {
-            return new Enumeration()
+            return new Enumeration<Object>()
             {
                 public boolean hasMoreElements()
                 {
@@ -668,18 +674,12 @@ class PluginHolder implements ServiceListener
 
     private static class ServletPlugin extends Plugin
     {
-        private final ServiceReference serviceReference;
 
-
-        ServletPlugin( final PluginHolder holder, final ServiceReference 
serviceReference, final String label )
+        ServletPlugin( final PluginHolder holder, final ServiceReference<?> 
serviceReference, final String label )
         {
-            super(holder, label);
-            this.serviceReference = serviceReference;
+            super(holder, serviceReference, label);
         }
 
-
-
-
         Bundle getBundle()
         {
             return serviceReference.getBundle();
@@ -753,10 +753,10 @@ class PluginHolder implements ServiceListener
         }
 
 
-        public Enumeration getInitParameterNames()
+        public Enumeration<?> getInitParameterNames()
         {
             final String[] keys = serviceReference.getPropertyKeys();
-            return new Enumeration()
+            return new Enumeration<Object>()
             {
                 int idx = 0;
 
@@ -790,7 +790,7 @@ class PluginHolder implements ServiceListener
 
         protected InternalPlugin(PluginHolder holder, OsgiManager osgiManager, 
String pluginClassName, String label)
         {
-            super(holder, label);
+            super(holder, null, label);
             this.osgiManager = osgiManager;
             this.pluginClassName = pluginClassName;
         }
@@ -815,7 +815,7 @@ class PluginHolder implements ServiceListener
 
                 try
                 {
-                    Class pluginClass = 
getClass().getClassLoader().loadClass(pluginClassName);
+                    Class<?> pluginClass = 
getClass().getClassLoader().loadClass(pluginClassName);
                     plugin = (AbstractWebConsolePlugin) 
pluginClass.newInstance();
 
                     if (plugin instanceof OsgiManagerPlugin)

Reply via email to