On Jun 1, 2009, at 4:12 PM, Sergiu Dumitriu wrote:

vmassol (SVN) wrote:

...

Author: vmassol Date: 2009-06-01 10:49:54 +0200 (Mon, 01 Jun 2009) New Revision: 20681

Log: XWIKI-3913: Add support for Component Manager events

* TODO: Refactor localization module to use the refactored Observation module * TODO: Refactor Rendering API module by removing notion of Source Macro Manager (rollback as before) and instead use Component Manager events to register Macros dynamically.

Added: platform/core/trunk/xwiki-component/xwiki-component-default/ src/main/java/org/xwiki/component/manager/ ComponentDescriptorAddedEvent.java =================================================================== --- platform/core/trunk/xwiki-component/xwiki-component-default/src/ main/java/org/xwiki/component/manager/ ComponentDescriptorAddedEvent.java (rev 0) +++ platform/core/trunk/xwiki-component/xwiki-component-default/src/ main/java/org/xwiki/component/manager/ ComponentDescriptorAddedEvent.java 2009-06-01 08:49:54 UTC (rev 20681) + public boolean matches(Object otherEvent) + { + boolean result = false; + + if (ComponentDescriptorAddedEvent .class.isAssignableFrom(otherEvent.getClass())) { + ComponentDescriptorAddedEvent event = (ComponentDescriptorAddedEvent) otherEvent;

I'd like to support a wildcard here, like a listener for all component registrations (if getRole == null, return true).

Also, why is getRole.getName compared to simply getRole?

Fixed, thanks.

...

+ if (getRole().getName().equals(event.getRole())) { + result = true; + } + }

Modified: platform/core/trunk/xwiki-containers/xwiki-container- servlet/src/main/java/org/xwiki/container/servlet/ XWikiPlexusServletContextListener.java =================================================================== // This is a temporary bridge to allow non XWiki components to lookup XWiki components. - // We're putting the XWiki Component Manager instance in the Servlet Context so that it's - // available in the XWikiAction class which in turn puts it into the XWikiContext instance. - // Class that need to lookup then just need to get it from the XWikiContext instance. - // This is of course not necessary for XWiki components since they just need to implement - // the Composable interface to get access to the Component Manager or better they simply - // need to define the Components they require as field members and configure the Plexus - // deployment descriptors (components.xml) so that they are automatically injected. servletContextEvent.getServletContext().setAttribute(

org.xwiki.component.manager.ComponentManager.class.getName(), this.componentManager); }

Why did you remove this comment?

Because It wasn't valid any more: - we're not using XWikiAction - there's no components.xml - the compnent manager is now injected. I thought it wasn't necessary to explain all this here and that saying it's a temporary bridge that allows non components to lookup components is enough.

...

Modified: platform/core/trunk/xwiki-core/src/test/java/com/xpn/ xwiki/XWikiTest.java =================================================================== --- platform/core/trunk/xwiki-core/src/test/java/com/xpn/xwiki/ XWikiTest.java 2009-06-01 07:45:41 UTC (rev 20680) +++ platform/core/trunk/xwiki-core/src/test/java/com/xpn/xwiki/ XWikiTest.java 2009-06-01 08:49:54 UTC (rev 20681) @@ -20,6 +20,7 @@ package com.xpn.xwiki;

A bit of duplication here:

Actually I think that a bit of duplication isn't bad in tests if it makes the test simpler to read. That's what I thought there.

...

- TestSaveEventListener listener = new TestSaveEventListener(); + Mock mockListener = mock(EventListener.class); + mockListener .stubs().method("getName").will(returnValue("testlistener")); + mockListener .expects (once()).method("getEvents").will(returnValue(Arrays.asList(new DocumentSaveEvent("xwikitest:Some.Document")))); + ObservationManager om = (ObservationManager) getComponentManager().lookup(ObservationManager.class); - om.addListener(new DocumentSaveEvent("xwikitest:Some.Document"), listener); + om.addListener((EventListener) mockListener.proxy()); Modified: platform/core/trunk/xwiki-observation/src/main/java/org/ xwiki/observation/internal/DefaultObservationManager.java ===================================================================

...

+ for (Event listenerEvent : regListener.events) {

Why do you check the class also? match() should take care of this, and it prohibits event inheritance.

Fixed, thanks. Thanks for the review! -Vincent