From 160ee4428b49a7fbea922b5f217e7423c554377d Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 14 Aug 2019 12:06:32 +1000 Subject: [PATCH] Issue #2578 EventListener Resolve differences between eventListeners added as beans and beans added as EventListeners. The behaviour should now be the same regardless of how they listener is added and all listeners are now beans. Signed-off-by: Greg Wilkins --- .../src/main/resources/etc/jetty-setuid.xml | 2 +- .../src/main/resources/etc/jetty-started.xml | 2 +- .../javax-annotation-api/src/config/jetty.xml | 2 +- .../src/main/jetty/jetty.xml | 2 +- .../src/config/jetty.xml | 2 +- .../web/src/config/jetty.xml | 2 +- .../src/base/etc/test-jetty.xml | 2 +- .../jetty-simple-webapp/src/config/jetty.xml | 2 +- .../jetty-simple-webapp/src/config/jetty.xml | 2 +- .../jetty-run-mojo-jsp/src/config/jetty.xml | 2 +- .../webapp-war/src/config/jetty.xml | 2 +- .../jetty-simple-webapp/src/config/jetty.xml | 2 +- .../jetty-simple-webapp/src/config/jetty.xml | 2 +- .../jetty-simple-webapp/src/config/jetty.xml | 2 +- .../beer-server/src/config/jetty.xml | 2 +- .../src/main/resources/jetty-maven.xml | 2 +- .../jetty-http-boot-context-as-service.xml | 2 +- .../etc/jetty-http-boot-webapp-as-service.xml | 2 +- .../etc/jetty-http-boot-with-annotations.xml | 2 +- .../etc/jetty-http-boot-with-bundle.xml | 2 +- .../jetty-http-boot-with-javax-websocket.xml | 2 +- .../config/etc/jetty-http-boot-with-jsp.xml | 2 +- .../etc/jetty-http-boot-with-websocket.xml | 2 +- .../src/test/config/etc/jetty-http.xml | 2 +- .../src/test/config/etc/jetty-https.xml | 2 +- .../jetty/server/handler/ContextHandler.java | 3 +- .../util/component/ContainerLifeCycle.java | 67 ++++++++++++------- .../component/ContainerLifeCycleTest.java | 10 +-- .../jetty/webapp/WebAppContextTest.java | 2 +- 29 files changed, 74 insertions(+), 58 deletions(-) diff --git a/jetty-home/src/main/resources/etc/jetty-setuid.xml b/jetty-home/src/main/resources/etc/jetty-setuid.xml index 2fd25150b6cd..3f2bff2f06b6 100644 --- a/jetty-home/src/main/resources/etc/jetty-setuid.xml +++ b/jetty-home/src/main/resources/etc/jetty-setuid.xml @@ -6,7 +6,7 @@ - + diff --git a/jetty-home/src/main/resources/etc/jetty-started.xml b/jetty-home/src/main/resources/etc/jetty-started.xml index b8d6007fbcda..11a2e1517954 100644 --- a/jetty-home/src/main/resources/etc/jetty-started.xml +++ b/jetty-home/src/main/resources/etc/jetty-started.xml @@ -5,7 +5,7 @@ - + diff --git a/jetty-maven-plugin/src/it/javax-annotation-api/src/config/jetty.xml b/jetty-maven-plugin/src/it/javax-annotation-api/src/config/jetty.xml index ced70a66d09d..c3eddc27cd9c 100644 --- a/jetty-maven-plugin/src/it/javax-annotation-api/src/config/jetty.xml +++ b/jetty-maven-plugin/src/it/javax-annotation-api/src/config/jetty.xml @@ -24,7 +24,7 @@ - + diff --git a/jetty-maven-plugin/src/it/jetty-cdi-run-forked/src/main/jetty/jetty.xml b/jetty-maven-plugin/src/it/jetty-cdi-run-forked/src/main/jetty/jetty.xml index 5389324070a4..e93ed47392b2 100644 --- a/jetty-maven-plugin/src/it/jetty-cdi-run-forked/src/main/jetty/jetty.xml +++ b/jetty-maven-plugin/src/it/jetty-cdi-run-forked/src/main/jetty/jetty.xml @@ -24,7 +24,7 @@ - + diff --git a/jetty-maven-plugin/src/it/jetty-deploy-war-mojo-it/src/config/jetty.xml b/jetty-maven-plugin/src/it/jetty-deploy-war-mojo-it/src/config/jetty.xml index ced70a66d09d..c3eddc27cd9c 100644 --- a/jetty-maven-plugin/src/it/jetty-deploy-war-mojo-it/src/config/jetty.xml +++ b/jetty-maven-plugin/src/it/jetty-deploy-war-mojo-it/src/config/jetty.xml @@ -24,7 +24,7 @@ - + diff --git a/jetty-maven-plugin/src/it/jetty-maven-plugin-provided-module-dep/web/src/config/jetty.xml b/jetty-maven-plugin/src/it/jetty-maven-plugin-provided-module-dep/web/src/config/jetty.xml index b2f1ae174319..f4ea80360030 100644 --- a/jetty-maven-plugin/src/it/jetty-maven-plugin-provided-module-dep/web/src/config/jetty.xml +++ b/jetty-maven-plugin/src/it/jetty-maven-plugin-provided-module-dep/web/src/config/jetty.xml @@ -30,7 +30,7 @@ - + diff --git a/jetty-maven-plugin/src/it/jetty-run-distro-mojo-it/jetty-simple-webapp/src/base/etc/test-jetty.xml b/jetty-maven-plugin/src/it/jetty-run-distro-mojo-it/jetty-simple-webapp/src/base/etc/test-jetty.xml index 08d848ec4c65..c3df301c562d 100644 --- a/jetty-maven-plugin/src/it/jetty-run-distro-mojo-it/jetty-simple-webapp/src/base/etc/test-jetty.xml +++ b/jetty-maven-plugin/src/it/jetty-run-distro-mojo-it/jetty-simple-webapp/src/base/etc/test-jetty.xml @@ -3,7 +3,7 @@ - + diff --git a/jetty-maven-plugin/src/it/jetty-run-forked-mojo-it/jetty-simple-webapp/src/config/jetty.xml b/jetty-maven-plugin/src/it/jetty-run-forked-mojo-it/jetty-simple-webapp/src/config/jetty.xml index ced70a66d09d..c3eddc27cd9c 100644 --- a/jetty-maven-plugin/src/it/jetty-run-forked-mojo-it/jetty-simple-webapp/src/config/jetty.xml +++ b/jetty-maven-plugin/src/it/jetty-run-forked-mojo-it/jetty-simple-webapp/src/config/jetty.xml @@ -24,7 +24,7 @@ - + diff --git a/jetty-maven-plugin/src/it/jetty-run-mojo-it/jetty-simple-webapp/src/config/jetty.xml b/jetty-maven-plugin/src/it/jetty-run-mojo-it/jetty-simple-webapp/src/config/jetty.xml index ced70a66d09d..c3eddc27cd9c 100644 --- a/jetty-maven-plugin/src/it/jetty-run-mojo-it/jetty-simple-webapp/src/config/jetty.xml +++ b/jetty-maven-plugin/src/it/jetty-run-mojo-it/jetty-simple-webapp/src/config/jetty.xml @@ -24,7 +24,7 @@ - + diff --git a/jetty-maven-plugin/src/it/jetty-run-mojo-jsp/src/config/jetty.xml b/jetty-maven-plugin/src/it/jetty-run-mojo-jsp/src/config/jetty.xml index ced70a66d09d..c3eddc27cd9c 100644 --- a/jetty-maven-plugin/src/it/jetty-run-mojo-jsp/src/config/jetty.xml +++ b/jetty-maven-plugin/src/it/jetty-run-mojo-jsp/src/config/jetty.xml @@ -24,7 +24,7 @@ - + diff --git a/jetty-maven-plugin/src/it/jetty-run-mojo-multi-module-single-war-it/webapp-war/src/config/jetty.xml b/jetty-maven-plugin/src/it/jetty-run-mojo-multi-module-single-war-it/webapp-war/src/config/jetty.xml index ced70a66d09d..c3eddc27cd9c 100644 --- a/jetty-maven-plugin/src/it/jetty-run-mojo-multi-module-single-war-it/webapp-war/src/config/jetty.xml +++ b/jetty-maven-plugin/src/it/jetty-run-mojo-multi-module-single-war-it/webapp-war/src/config/jetty.xml @@ -24,7 +24,7 @@ - + diff --git a/jetty-maven-plugin/src/it/jetty-run-war-exploded-mojo-it/jetty-simple-webapp/src/config/jetty.xml b/jetty-maven-plugin/src/it/jetty-run-war-exploded-mojo-it/jetty-simple-webapp/src/config/jetty.xml index ced70a66d09d..c3eddc27cd9c 100644 --- a/jetty-maven-plugin/src/it/jetty-run-war-exploded-mojo-it/jetty-simple-webapp/src/config/jetty.xml +++ b/jetty-maven-plugin/src/it/jetty-run-war-exploded-mojo-it/jetty-simple-webapp/src/config/jetty.xml @@ -24,7 +24,7 @@ - + diff --git a/jetty-maven-plugin/src/it/jetty-run-war-mojo-it/jetty-simple-webapp/src/config/jetty.xml b/jetty-maven-plugin/src/it/jetty-run-war-mojo-it/jetty-simple-webapp/src/config/jetty.xml index ced70a66d09d..c3eddc27cd9c 100644 --- a/jetty-maven-plugin/src/it/jetty-run-war-mojo-it/jetty-simple-webapp/src/config/jetty.xml +++ b/jetty-maven-plugin/src/it/jetty-run-war-mojo-it/jetty-simple-webapp/src/config/jetty.xml @@ -24,7 +24,7 @@ - + diff --git a/jetty-maven-plugin/src/it/jetty-start-mojo-it/jetty-simple-webapp/src/config/jetty.xml b/jetty-maven-plugin/src/it/jetty-start-mojo-it/jetty-simple-webapp/src/config/jetty.xml index ced70a66d09d..c3eddc27cd9c 100644 --- a/jetty-maven-plugin/src/it/jetty-start-mojo-it/jetty-simple-webapp/src/config/jetty.xml +++ b/jetty-maven-plugin/src/it/jetty-start-mojo-it/jetty-simple-webapp/src/config/jetty.xml @@ -24,7 +24,7 @@ - + diff --git a/jetty-maven-plugin/src/it/run-mojo-gwt-it/beer-server/src/config/jetty.xml b/jetty-maven-plugin/src/it/run-mojo-gwt-it/beer-server/src/config/jetty.xml index ced70a66d09d..c3eddc27cd9c 100644 --- a/jetty-maven-plugin/src/it/run-mojo-gwt-it/beer-server/src/config/jetty.xml +++ b/jetty-maven-plugin/src/it/run-mojo-gwt-it/beer-server/src/config/jetty.xml @@ -24,7 +24,7 @@ - + diff --git a/jetty-maven-plugin/src/main/resources/jetty-maven.xml b/jetty-maven-plugin/src/main/resources/jetty-maven.xml index 8ee79fbec55f..47d10a84d4f5 100644 --- a/jetty-maven-plugin/src/main/resources/jetty-maven.xml +++ b/jetty-maven-plugin/src/main/resources/jetty-maven.xml @@ -2,7 +2,7 @@ - + diff --git a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-context-as-service.xml b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-context-as-service.xml index cd907934b86e..89ce0ee13842 100644 --- a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-context-as-service.xml +++ b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-context-as-service.xml @@ -31,7 +31,7 @@ - + boot.context.service.port diff --git a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-webapp-as-service.xml b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-webapp-as-service.xml index df4f9a29512d..460d8a9d64f7 100644 --- a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-webapp-as-service.xml +++ b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-webapp-as-service.xml @@ -31,7 +31,7 @@ - + boot.webapp.service.port diff --git a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-annotations.xml b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-annotations.xml index fafa0ecdd183..d2d92135b9dd 100644 --- a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-annotations.xml +++ b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-annotations.xml @@ -31,7 +31,7 @@ - + boot.annotations.port diff --git a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-bundle.xml b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-bundle.xml index 80335e9184a2..6cb5d15395a7 100644 --- a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-bundle.xml +++ b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-bundle.xml @@ -31,7 +31,7 @@ - + boot.bundle.port diff --git a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-javax-websocket.xml b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-javax-websocket.xml index 856a577d3290..aecb4fc9dd5c 100644 --- a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-javax-websocket.xml +++ b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-javax-websocket.xml @@ -31,7 +31,7 @@ - + boot.javax.websocket.port diff --git a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-jsp.xml b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-jsp.xml index aeda97879e83..b1134bea4a66 100644 --- a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-jsp.xml +++ b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-jsp.xml @@ -31,7 +31,7 @@ - + boot.jsp.port diff --git a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-websocket.xml b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-websocket.xml index 3c45c45a368c..a52a35179bdc 100644 --- a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-websocket.xml +++ b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http-boot-with-websocket.xml @@ -31,7 +31,7 @@ - + boot.websocket.port diff --git a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http.xml b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http.xml index 6384118ee3bc..5c7000a8bb89 100644 --- a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http.xml +++ b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http.xml @@ -31,7 +31,7 @@ - + foo.foo diff --git a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-https.xml b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-https.xml index 7b408b6dc4d2..520db2dffef0 100644 --- a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-https.xml +++ b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-https.xml @@ -16,7 +16,7 @@ - + boot.https.port diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index cb8d9f45c901..d5b419b50172 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -260,7 +260,6 @@ public void dump(Appendable out, String indent) throws IOException { dumpObjects(out, indent, new ClassLoaderDump(getClassLoader()), - new DumpableCollection("eventListeners " + this, _eventListeners), new DumpableCollection("handler attributes " + this, ((AttributesMap)getAttributes()).getAttributeEntrySet()), new DumpableCollection("context attributes " + this, ((Context)getServletContext()).getAttributeEntrySet()), new DumpableCollection("initparams " + this, getInitParams().entrySet())); @@ -688,6 +687,8 @@ public void addEventListener(EventListener listener) */ public void removeEventListener(EventListener listener) { + super.removeEventListener(listener); + _eventListeners.remove(listener); if (listener instanceof ContextScopeListener) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java index 407ad3dd266c..d706c4ceac8b 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java @@ -65,6 +65,10 @@ * If adding a bean that is shared between multiple {@link ContainerLifeCycle} instances, then it should be started * before being added, so it is unmanaged, or the API must be used to explicitly set it as unmanaged. *

+ * All {@link EventListener}s added via {@link #addEventListener(EventListener)} are also added as beans and all beans + * added via an {@link #addBean(Object)} method that are also {@link EventListener}s are added as listeners via a + * call to {@link #addEventListener(EventListener)}. + *

* This class also provides utility methods to dump deep structures of objects. * In the dump, the following symbols are used to indicate the type of contained object: * 

@@ -347,18 +351,17 @@ public boolean addBean(Object o, Managed managed)
 
         Bean newBean = new Bean(o);
 
-        // if the bean is a Listener
-        if (o instanceof EventListener)
-            addEventListener((EventListener)o);
-
         // Add the bean
         _beans.add(newBean);
 
-        // Tell existing listeners about the new bean
+        // Tell any existing listeners about the new bean
         for (Container.Listener l : _listeners)
-        {
             l.beanAdded(this, o);
-        }
+
+        // if the bean is a Listener.  Note we check the _eventListenerBeans here
+        // to avoid calling extended version of addEventListener before detecting it is already added.
+        if (o instanceof EventListener && !_eventListenerBeans.contains(o))
+            addEventListener((EventListener)o);
 
         try
         {
@@ -457,12 +460,20 @@ public void addManaged(LifeCycle lifecycle)
     @Override
     public void addEventListener(EventListener listener)
     {
+        // Has it already been added as a listener?
         if (_eventListenerBeans.contains(listener))
+            // yes - nothing to do
             return;
 
         super.addEventListener(listener);
         _eventListenerBeans.add(listener);
 
+        // If it is not yet a bean,
+        if (!contains(listener))
+            // add it as a bean first, we wont be called back because we've already added it to the _eventListenerBeans list
+            // but we've not added it to the _listeners list, so it won't be told about itself!
+            addBean(listener);
+
         if (listener instanceof Container.Listener)
         {
             Container.Listener cl = (Container.Listener)listener;
@@ -485,6 +496,27 @@ public void addEventListener(EventListener listener)
         }
     }
 
+    @Override
+    public void removeEventListener(EventListener listener)
+    {
+        if (_eventListenerBeans.remove(listener))
+        {
+            super.removeEventListener(listener);
+            removeBean(listener);
+            if (_listeners.remove(listener))
+            {
+                // remove existing beans
+                for (Bean b : _beans)
+                {
+                    ((Container.Listener)listener).beanRemoved(this, b._bean);
+
+                    if (listener instanceof InheritedListener && b.isManaged() && b._bean instanceof Container)
+                        ((Container)b._bean).removeBean(listener);
+                }
+            }
+        }
+    }
+
     /**
      * Manages a bean already contained by this aggregate, so that it is started/stopped/destroyed with this
      * aggregate.
@@ -651,7 +683,8 @@ private boolean remove(Bean bean)
                 l.beanRemoved(this, bean._bean);
             }
 
-            if (bean._bean instanceof EventListener)
+            // Remove event listeners, checking list here to avoid calling extended removeEventListener if already removed.
+            if (bean._bean instanceof EventListener && _eventListenerBeans.contains(bean._bean))
                 removeEventListener((EventListener)bean._bean);
 
             // stop managed beans
@@ -675,24 +708,6 @@ private boolean remove(Bean bean)
         return false;
     }
 
-    @Override
-    public void removeEventListener(EventListener listener)
-    {
-        _eventListenerBeans.remove(listener);
-        super.removeEventListener(listener);
-        if (_listeners.remove(listener))
-        {
-            // remove existing beans
-            for (Bean b : _beans)
-            {
-                ((Container.Listener)listener).beanRemoved(this, b._bean);
-
-                if (listener instanceof InheritedListener && b.isManaged() && b._bean instanceof Container)
-                    ((Container)b._bean).removeBean(listener);
-            }
-        }
-    }
-
     @Override
     public void setStopTimeout(long stopTimeout)
     {
diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/component/ContainerLifeCycleTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/component/ContainerLifeCycleTest.java
index b90272680c74..bb94c41d6d1a 100644
--- a/jetty-util/src/test/java/org/eclipse/jetty/util/component/ContainerLifeCycleTest.java
+++ b/jetty-util/src/test/java/org/eclipse/jetty/util/component/ContainerLifeCycleTest.java
@@ -447,20 +447,20 @@ String toString()
 
         c0.addBean(inherited);
 
-        assertEquals("inherited", handled.poll());
+        assertEquals("listener", handled.poll());
         assertEquals("added", operation.poll());
         assertEquals(c0, parent.poll());
-        assertEquals(c00, child.poll());
+        assertEquals(inherited, child.poll());
 
         assertEquals("inherited", handled.poll());
         assertEquals("added", operation.poll());
         assertEquals(c0, parent.poll());
-        assertEquals(listener, child.poll());
+        assertEquals(c00, child.poll());
 
-        assertEquals("listener", handled.poll());
+        assertEquals("inherited", handled.poll());
         assertEquals("added", operation.poll());
         assertEquals(c0, parent.poll());
-        assertEquals(inherited, child.poll());
+        assertEquals(listener, child.poll());
 
         assertEquals("inherited", handled.poll());
         assertEquals("added", operation.poll());
diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java
index 20e90ef451d5..7bccdbbfc0d7 100644
--- a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java
+++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java
@@ -90,7 +90,7 @@ public void testSessionListeners()
         server.setHandler(wac);
         wac.addEventListener(new MySessionListener());
 
-        Collection listeners = wac.getSessionHandler().getBeans(org.eclipse.jetty.webapp.WebAppContextTest.MySessionListener.class);
+        Collection listeners = wac.getSessionHandler().getBeans(MySessionListener.class);
         assertNotNull(listeners);
         assertEquals(1, listeners.size());
     }