Hey Matt, yeah, I also thought about moving the activated field and it's initialization to the EventEmitter class. However, in the end I decided against this. Mainly because I thought that it may be clearer to the user that the sub-classes can be deactivated which is not so straight forward if the Deactivatable code is "hidden" in a super class. However, we could change this. As long as the deactivatable types are mentioned in the documentation, this should be fine.
And yes, I think it could make sense to move the EventEmitter class to a more "central" place. I really thing it may be useful in other scenarios. Do we have similar cases in other modules? Christian 2013/9/3 Matt Benson <[email protected]> > Yes, with "why not?" I was speaking more of the idea of moving Emitter to > deltaspike core if we wanted folk to extend it. All I'm saying at the > moment, with Emitter staying in its current location, is that if all > subclasses are Deactivatable, it would seem to make sense to pull the > concept up and centralize its implementation. That's fairly well par for > the course in OO design, so I assume my signal to noise ratio is too low > with my musings about users extending Emitter mixed in. > > > On Tue, Sep 3, 2013 at 7:45 AM, John D. Ament <[email protected] > >wrote: > > > Since emitter is in the impl package, I think our statement to users > > would be "don't program against it" > > but then we tell them to deactivate using impl classes :-) > > > > Either way, since deactivation is optional even if someone did program > > against it all that would happen is that you could deactivate it (in > > this case). > > > > On Tue, Sep 3, 2013 at 8:43 AM, Matt Benson <[email protected]> > wrote: > > > Hi John, > > > I see what you mean. I was thinking of the base Emitter class as > > something > > > extensible for users, which on the one hand we could say is so simple > why > > > bother, but on the other hand could say why not? ...but I digress. If > > > they're all deactivatable then let's consolidate that deactivation > logic, > > > simply. > > > > > > Matt > > > > > > On Sep 3, 2013 7:38 AM, "John D. Ament" <[email protected]> > wrote: > > >> > > >> Matt > > >> > > >> What do you mean? We have three emitters, all three are > deactivatable... > > >> > > >> On Tue, Sep 3, 2013 at 8:35 AM, Matt Benson <[email protected]> > > wrote: > > >> > I see... not all emitters are deactivatable. Still, what about an > > >> > abstract > > >> > deactivatable emitter class? > > >> > On Sep 3, 2013 7:32 AM, "Matt Benson" <[email protected]> wrote: > > >> > > > >> >> Hi Christian, > > >> >> This activation check seems pretty generic if it's only based on > the > > >> >> emitter's runtime class... what about moving up to Emitter and > doing > > >> >> the > > >> >> check in #fireEvent()? > > >> >> > > >> >> br, > > >> >> Matt > > >> >> On Sep 3, 2013 2:13 AM, <[email protected]> wrote: > > >> >> > > >> >>> Updated Branches: > > >> >>> refs/heads/master 28169465d -> 502fecd7a > > >> >>> > > >> >>> > > >> >>> DELTASPIKE-409 Servlet module events should be deactivatable > > >> >>> > > >> >>> > > >> >>> Project: http://git-wip-us.apache.org/repos/asf/deltaspike/repo > > >> >>> Commit: > > >> >>> http://git-wip-us.apache.org/repos/asf/deltaspike/commit/502fecd7 > > >> >>> Tree: > > http://git-wip-us.apache.org/repos/asf/deltaspike/tree/502fecd7 > > >> >>> Diff: > > http://git-wip-us.apache.org/repos/asf/deltaspike/diff/502fecd7 > > >> >>> > > >> >>> Branch: refs/heads/master > > >> >>> Commit: 502fecd7a66950665ee6dda1d13d1043f7d81a20 > > >> >>> Parents: 2816946 > > >> >>> Author: Christian Kaltepoth <[email protected]> > > >> >>> Authored: Tue Sep 3 09:12:29 2013 +0200 > > >> >>> Committer: Christian Kaltepoth <[email protected]> > > >> >>> Committed: Tue Sep 3 09:12:29 2013 +0200 > > >> >>> > > >> >>> > > ---------------------------------------------------------------------- > > >> >>> .../impl/event/EventBridgeContextListener.java | 21 > > +++++++++++++--- > > >> >>> .../servlet/impl/event/EventBridgeFilter.java | 25 > > >> >>> ++++++++++++++++---- > > >> >>> .../impl/event/EventBridgeSessionListener.java | 21 > > +++++++++++++--- > > >> >>> 3 files changed, 56 insertions(+), 11 deletions(-) > > >> >>> > > ---------------------------------------------------------------------- > > >> >>> > > >> >>> > > >> >>> > > >> >>> > > >> >>> > > > http://git-wip-us.apache.org/repos/asf/deltaspike/blob/502fecd7/deltaspike/modules/servlet/impl/src/main/java/org/apache/deltaspike/servlet/impl/event/EventBridgeContextListener.java > > >> >>> > > ---------------------------------------------------------------------- > > >> >>> diff --git > > >> >>> > > >> >>> > > > a/deltaspike/modules/servlet/impl/src/main/java/org/apache/deltaspike/servlet/impl/event/EventBridgeContextListener.java > > >> >>> > > >> >>> > > > b/deltaspike/modules/servlet/impl/src/main/java/org/apache/deltaspike/servlet/impl/event/EventBridgeContextListener.java > > >> >>> index 2f7cd7c..c80d92e 100644 > > >> >>> --- > > >> >>> > > >> >>> > > > a/deltaspike/modules/servlet/impl/src/main/java/org/apache/deltaspike/servlet/impl/event/EventBridgeContextListener.java > > >> >>> +++ > > >> >>> > > >> >>> > > > b/deltaspike/modules/servlet/impl/src/main/java/org/apache/deltaspike/servlet/impl/event/EventBridgeContextListener.java > > >> >>> @@ -21,25 +21,40 @@ package > > org.apache.deltaspike.servlet.impl.event; > > >> >>> import javax.servlet.ServletContextEvent; > > >> >>> import javax.servlet.ServletContextListener; > > >> >>> > > >> >>> +import org.apache.deltaspike.core.spi.activation.Deactivatable; > > >> >>> +import org.apache.deltaspike.core.util.ClassDeactivationUtils; > > >> >>> import > org.apache.deltaspike.servlet.api.literal.DestroyedLiteral; > > >> >>> import > > org.apache.deltaspike.servlet.api.literal.InitializedLiteral; > > >> >>> > > >> >>> /** > > >> >>> * This class listens for servlet context events and forwards > them > > to > > >> >>> the CDI event bus. > > >> >>> */ > > >> >>> -public class EventBridgeContextListener extends EventEmitter > > >> >>> implements > > >> >>> ServletContextListener > > >> >>> +public class EventBridgeContextListener extends EventEmitter > > >> >>> implements > > >> >>> ServletContextListener, Deactivatable > > >> >>> { > > >> >>> > > >> >>> + private final boolean activated; > > >> >>> + > > >> >>> + public EventBridgeContextListener() > > >> >>> + { > > >> >>> + this.activated = > > >> >>> ClassDeactivationUtils.isActivated(getClass()); > > >> >>> + } > > >> >>> + > > >> >>> @Override > > >> >>> public void contextInitialized(ServletContextEvent sce) > > >> >>> { > > >> >>> - fireEvent(sce.getServletContext(), > > >> >>> InitializedLiteral.INSTANCE); > > >> >>> + if (activated) > > >> >>> + { > > >> >>> + fireEvent(sce.getServletContext(), > > >> >>> InitializedLiteral.INSTANCE); > > >> >>> + } > > >> >>> } > > >> >>> > > >> >>> @Override > > >> >>> public void contextDestroyed(ServletContextEvent sce) > > >> >>> { > > >> >>> - fireEvent(sce.getServletContext(), > > >> >>> DestroyedLiteral.INSTANCE); > > >> >>> + if (activated) > > >> >>> + { > > >> >>> + fireEvent(sce.getServletContext(), > > >> >>> DestroyedLiteral.INSTANCE); > > >> >>> + } > > >> >>> } > > >> >>> > > >> >>> } > > >> >>> > > >> >>> > > >> >>> > > >> >>> > > > http://git-wip-us.apache.org/repos/asf/deltaspike/blob/502fecd7/deltaspike/modules/servlet/impl/src/main/java/org/apache/deltaspike/servlet/impl/event/EventBridgeFilter.java > > >> >>> > > ---------------------------------------------------------------------- > > >> >>> diff --git > > >> >>> > > >> >>> > > > a/deltaspike/modules/servlet/impl/src/main/java/org/apache/deltaspike/servlet/impl/event/EventBridgeFilter.java > > >> >>> > > >> >>> > > > b/deltaspike/modules/servlet/impl/src/main/java/org/apache/deltaspike/servlet/impl/event/EventBridgeFilter.java > > >> >>> index 529d4c7..ce1f279 100644 > > >> >>> --- > > >> >>> > > >> >>> > > > a/deltaspike/modules/servlet/impl/src/main/java/org/apache/deltaspike/servlet/impl/event/EventBridgeFilter.java > > >> >>> +++ > > >> >>> > > >> >>> > > > b/deltaspike/modules/servlet/impl/src/main/java/org/apache/deltaspike/servlet/impl/event/EventBridgeFilter.java > > >> >>> @@ -27,15 +27,24 @@ import javax.servlet.ServletException; > > >> >>> import javax.servlet.ServletRequest; > > >> >>> import javax.servlet.ServletResponse; > > >> >>> > > >> >>> +import org.apache.deltaspike.core.spi.activation.Deactivatable; > > >> >>> +import org.apache.deltaspike.core.util.ClassDeactivationUtils; > > >> >>> import > org.apache.deltaspike.servlet.api.literal.DestroyedLiteral; > > >> >>> import > > org.apache.deltaspike.servlet.api.literal.InitializedLiteral; > > >> >>> > > >> >>> /** > > >> >>> * This filter sends events to the CDI event bus when requests > and > > >> >>> responses get created and destroyed. > > >> >>> */ > > >> >>> -public class EventBridgeFilter extends EventEmitter implements > > Filter > > >> >>> +public class EventBridgeFilter extends EventEmitter implements > > >> >>> Filter, > > >> >>> Deactivatable > > >> >>> { > > >> >>> > > >> >>> + private final boolean activated; > > >> >>> + > > >> >>> + public EventBridgeFilter() > > >> >>> + { > > >> >>> + this.activated = > > >> >>> ClassDeactivationUtils.isActivated(getClass()); > > >> >>> + } > > >> >>> + > > >> >>> @Override > > >> >>> public void init(FilterConfig config) throws ServletException > > >> >>> { > > >> >>> @@ -48,8 +57,11 @@ public class EventBridgeFilter extends > > EventEmitter > > >> >>> implements Filter > > >> >>> { > > >> >>> > > >> >>> // fire @Initialized events > > >> >>> - fireEvent(request, InitializedLiteral.INSTANCE); > > >> >>> - fireEvent(response, InitializedLiteral.INSTANCE); > > >> >>> + if (activated) > > >> >>> + { > > >> >>> + fireEvent(request, InitializedLiteral.INSTANCE); > > >> >>> + fireEvent(response, InitializedLiteral.INSTANCE); > > >> >>> + } > > >> >>> > > >> >>> try > > >> >>> { > > >> >>> @@ -58,8 +70,11 @@ public class EventBridgeFilter extends > > EventEmitter > > >> >>> implements Filter > > >> >>> finally > > >> >>> { > > >> >>> // fire @Destroyed events > > >> >>> - fireEvent(request, DestroyedLiteral.INSTANCE); > > >> >>> - fireEvent(response, DestroyedLiteral.INSTANCE); > > >> >>> + if (activated) > > >> >>> + { > > >> >>> + fireEvent(request, DestroyedLiteral.INSTANCE); > > >> >>> + fireEvent(response, DestroyedLiteral.INSTANCE); > > >> >>> + } > > >> >>> } > > >> >>> > > >> >>> } > > >> >>> > > >> >>> > > >> >>> > > >> >>> > > > http://git-wip-us.apache.org/repos/asf/deltaspike/blob/502fecd7/deltaspike/modules/servlet/impl/src/main/java/org/apache/deltaspike/servlet/impl/event/EventBridgeSessionListener.java > > >> >>> > > ---------------------------------------------------------------------- > > >> >>> diff --git > > >> >>> > > >> >>> > > > a/deltaspike/modules/servlet/impl/src/main/java/org/apache/deltaspike/servlet/impl/event/EventBridgeSessionListener.java > > >> >>> > > >> >>> > > > b/deltaspike/modules/servlet/impl/src/main/java/org/apache/deltaspike/servlet/impl/event/EventBridgeSessionListener.java > > >> >>> index c923ec5..73f14c3 100644 > > >> >>> --- > > >> >>> > > >> >>> > > > a/deltaspike/modules/servlet/impl/src/main/java/org/apache/deltaspike/servlet/impl/event/EventBridgeSessionListener.java > > >> >>> +++ > > >> >>> > > >> >>> > > > b/deltaspike/modules/servlet/impl/src/main/java/org/apache/deltaspike/servlet/impl/event/EventBridgeSessionListener.java > > >> >>> @@ -21,24 +21,39 @@ package > > org.apache.deltaspike.servlet.impl.event; > > >> >>> import javax.servlet.http.HttpSessionEvent; > > >> >>> import javax.servlet.http.HttpSessionListener; > > >> >>> > > >> >>> +import org.apache.deltaspike.core.spi.activation.Deactivatable; > > >> >>> +import org.apache.deltaspike.core.util.ClassDeactivationUtils; > > >> >>> import > org.apache.deltaspike.servlet.api.literal.DestroyedLiteral; > > >> >>> import > > org.apache.deltaspike.servlet.api.literal.InitializedLiteral; > > >> >>> > > >> >>> /** > > >> >>> * This class listens for HTTP session events and forwards them > to > > >> >>> the > > >> >>> CDI event bus. > > >> >>> */ > > >> >>> -public class EventBridgeSessionListener extends EventEmitter > > >> >>> implements > > >> >>> HttpSessionListener > > >> >>> +public class EventBridgeSessionListener extends EventEmitter > > >> >>> implements > > >> >>> HttpSessionListener, Deactivatable > > >> >>> { > > >> >>> > > >> >>> + private final boolean activated; > > >> >>> + > > >> >>> + public EventBridgeSessionListener() > > >> >>> + { > > >> >>> + this.activated = > > >> >>> ClassDeactivationUtils.isActivated(getClass()); > > >> >>> + } > > >> >>> + > > >> >>> @Override > > >> >>> public void sessionCreated(HttpSessionEvent se) > > >> >>> { > > >> >>> - fireEvent(se.getSession(), InitializedLiteral.INSTANCE); > > >> >>> + if (activated) > > >> >>> + { > > >> >>> + fireEvent(se.getSession(), > > InitializedLiteral.INSTANCE); > > >> >>> + } > > >> >>> } > > >> >>> > > >> >>> @Override > > >> >>> public void sessionDestroyed(HttpSessionEvent se) > > >> >>> { > > >> >>> - fireEvent(se.getSession(), DestroyedLiteral.INSTANCE); > > >> >>> + if (activated) > > >> >>> + { > > >> >>> + fireEvent(se.getSession(), > DestroyedLiteral.INSTANCE); > > >> >>> + } > > >> >>> } > > >> >>> } > > >> >>> > > >> >>> > > > -- Christian Kaltepoth Blog: http://blog.kaltepoth.de/ Twitter: http://twitter.com/chkal GitHub: https://github.com/chkal
