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
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs