Re: [xwiki-devs] [xwiki-notifications] r21910 - in platform/core/trunk: xwiki-cache/xwiki-cache-tests/src/main/java/org/xwiki/cache/tests xwiki-component/xwiki-component-api/src/main/java/org/xwiki/component/manager xwiki-component/xwiki-component-default xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/embed xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/internal xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/manager xwiki-compo
13 Jul
2009
13 Jul
'09
8:44 p.m.
Hi Thomas,
On Jul 13, 2009, at 3:20 PM, tmortagne (SVN) wrote:
> Author: tmortagne
> Date: 2009-07-13 15:20:03 +0200 (Mon, 13 Jul 2009)
> New Revision: 21910
>
> Added:
> platform/core/trunk/xwiki-component/xwiki-component-
> default/.checkstyle
> platform/core/trunk/xwiki-component/xwiki-component-default/src/
> main/java/org/xwiki/component/manager/
> AbstractComponentDescriptorEvent.java
> platform/core/trunk/xwiki-component/xwiki-component-default/src/
> main/java/org/xwiki/component/manager/
> ComponentDescriptorRemovedEvent.java
> Modified:
> platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/main/java/
> org/xwiki/cache/tests/AbstractTestCache.java
> platform/core/trunk/xwiki-component/xwiki-component-api/src/main/
> java/org/xwiki/component/manager/ComponentEventManager.java
> platform/core/trunk/xwiki-component/xwiki-component-api/src/main/
> java/org/xwiki/component/manager/ComponentManager.java
> platform/core/trunk/xwiki-component/xwiki-component-default/src/
> main/java/org/xwiki/component/embed/EmbeddableComponentManager.java
> platform/core/trunk/xwiki-component/xwiki-component-default/src/
> main/java/org/xwiki/component/internal/DefaultComponentManager.java
> platform/core/trunk/xwiki-component/xwiki-component-default/src/
> main/java/org/xwiki/component/internal/
> StackingComponentEventManager.java
> 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/
> test/java/org/xwiki/component/embed/
> EmbeddableComponentManagerTest.java
> platform/core/trunk/xwiki-component/xwiki-component-default/src/
> test/java/org/xwiki/component/manager/
> ComponentDescriptorAddedEventTest.java
> platform/core/trunk/xwiki-core/src/test/java/com/xpn/xwiki/test/
> AbstractBridgedXWikiComponentTestCase.java
> platform/core/trunk/xwiki-plexus/src/main/java/org/xwiki/plexus/
> manager/PlexusComponentManager.java
> platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/test/
> java/org/xwiki/rendering/internal/parser/XWikiLinkParserTest.java
> platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-groovy/src/test/java/org/xwiki/rendering/macro/
> groovy/RenderingTests.java
> platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-html/src/test/java/org/xwiki/rendering/
> HTMLMacroTest.java
> platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-include/src/test/java/org/xwiki/rendering/internal/
> macro/IncludeMacroTest.java
> platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-rss/src/test/java/org/xwiki/rendering/
> RenderingTests.java
> platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-script/src/test/java/org/xwiki/rendering/macro/
> script/RenderingTests.java
> platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-velocity/src/test/java/org/xwiki/rendering/macro/
> velocity/RenderingTests.java
> platform/core/trunk/xwiki-shared-tests/src/main/java/org/xwiki/
> test/MockConfigurationSource.java
> platform/core/trunk/xwiki-shared-tests/src/main/java/org/xwiki/
> test/XWikiComponentInitializer.java
> Log:
> XWIKI-4077: Component Manager should generate event when the
> component is registered instead of initialized
> XWIKI-4081: Add way to unregister component
> XWIKI-4082: Add way to directly provide the component instance when
> register it
General comments:
- Would be great if you could separate code reformatting from code
changes. It makes it hard to read (I didn't read the commit fully as a
consequence).
- The code style has broken my styles in lots of places (see below for
some comments, I haven't commented every single place it broke
voluntary formatting).
I'd like to vote for not applying code style blindly in the future. It
breaks styles in lots of places and I hate it when I spend a good
amount of my time to align code properly and it breaks it for
producing suboptimal styling...
> Modified: platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/main/
> java/org/xwiki/cache/tests/AbstractTestCache.java
> ===================================================================
> --- platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/main/java/
> org/xwiki/cache/tests/AbstractTestCache.java 2009-07-13 11:47:52 UTC
> (rev 21909)
> +++ platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/main/java/
> org/xwiki/cache/tests/AbstractTestCache.java 2009-07-13 13:20:03 UTC
> (rev 21910)
> @@ -25,6 +25,7 @@
> import org.jmock.Mock;
> import org.xwiki.cache.CacheFactory;
> import org.xwiki.cache.CacheManager;
> +import org.xwiki.component.descriptor.DefaultComponentDescriptor;
> import org.xwiki.configuration.ConfigurationSource;
> import org.xwiki.test.AbstractXWikiComponentTestCase;
>
> @@ -81,8 +82,12 @@
> Mock mockConfigurationSource =
> mock(ConfigurationSource.class);
>
> mockConfigurationSource
> .stubs().method("getProperty").with(eq("cache.defaultCache"),
> ANYTHING).will(
> returnValue(this.roleHint));
> -
> getComponentManager().registerComponent(ConfigurationSource.class,
> "xwikiproperties",
> - mockConfigurationSource.proxy());
> +
> + DefaultComponentDescriptor<ConfigurationSource> descriptor =
> + new DefaultComponentDescriptor<ConfigurationSource>();
> + descriptor.setRole(ConfigurationSource.class);
> + descriptor.setRoleHint("xwikiproperties");
> + getComponentManager().registerComponent(descriptor,
> (ConfigurationSource) mockConfigurationSource.proxy());
Maybe we'll need to find an easier way. Like creating a component
descriptor automatically (without dependencies set). I see below that
it adds a lot of code.
[snip]
> }
>
> /**
> @@ -111,7 +116,7 @@
> */
> public CacheFactory getCacheFactory() throws Exception
> {
> - CacheManager cacheManager = (CacheManager)
> getComponentManager().lookup(CacheManager.class);
> + CacheManager cacheManager =
> getComponentManager().lookup(CacheManager.class);
>
> CacheFactory factory = cacheManager.getCacheFactory();
>
>
> Modified: platform/core/trunk/xwiki-component/xwiki-component-api/
> src/main/java/org/xwiki/component/manager/ComponentEventManager.java
> ===================================================================
> --- platform/core/trunk/xwiki-component/xwiki-component-api/src/main/
> java/org/xwiki/component/manager/ComponentEventManager.java
> 2009-07-13 11:47:52 UTC (rev 21909)
> +++ platform/core/trunk/xwiki-component/xwiki-component-api/src/main/
> java/org/xwiki/component/manager/ComponentEventManager.java
> 2009-07-13 13:20:03 UTC (rev 21910)
> @@ -26,18 +26,26 @@
> * Manages Component Events (when a component instance is created
> for example). It's recommended that implementations
> * use the Observation module to send the events. We're introducing
> this level of indirection in order to be able to
> * perform some processing before the events are fired. For example
> one implementation may want to stack the events
> - * before sending them.
> - *
> + * before sending them.
> + *
> * @version $Id$
> * @since 2.0M1
> */
> public interface ComponentEventManager
> {
> /**
> - * Notify all listeners that a component with the passed
> descriptor has been instantiated.
> + * Notify all listeners that a component with the passed
> descriptor has been registered.
> *
> - * @param <T> the component role type
> * @param descriptor the descriptor for the instantiated component
> + * @since 2.0M1
> */
> - <T> void notify(ComponentDescriptor<T> descriptor);
> + void notifyComponentRegistered(ComponentDescriptor< ? >
> descriptor);
> +
> + /**
> + * Notify all listeners that a component with the passed
> descriptor has been unregistered.
> + *
> + * @param descriptor the descriptor for the instantiated
> component
> + * @since 2.0M1
> + */
> + void notifyComponentUnregistered(ComponentDescriptor< ? >
> descriptor);
This doesn't look like it's been added in 2.0M1... ;)
[snip]
> Added: platform/core/trunk/xwiki-component/xwiki-component-
> default/.checkstyle
> ===================================================================
> --- platform/core/trunk/xwiki-component/xwiki-component-
> default/.checkstyle (rev 0)
> +++ platform/core/trunk/xwiki-component/xwiki-component-
> default/.checkstyle 2009-07-13 13:20:03 UTC (rev 21910)
> @@ -0,0 +1,10 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +
> +<fileset-config file-format-version="1.2.0" simple-config="true">
> + <fileset name="all" enabled="true" check-config-name="xwiki"
> local="false">
> + <file-match-pattern match-pattern="." include-pattern="true"/>
> + </fileset>
> + <filter name="FilesFromPackage" enabled="true">
> + <filter-data value="src/test/java"/>
> + </filter>
> +</fileset-config>
This seems to have been added by error.
[snip]
> /**
> * {@inheritDoc}
> + *
> * @see ComponentManager#getComponentDescriptorList(Class)
> */
> @SuppressWarnings("unchecked")
> public <T> List<ComponentDescriptor<T>>
> getComponentDescriptorList(Class<T> role)
> {
> - synchronized(this) {
> + synchronized (this) {
> List<ComponentDescriptor<T>> results = new
> ArrayList<ComponentDescriptor<T>>();
> for (Map.Entry<RoleHint< ? >, ComponentDescriptor< ? >>
> entry : this.descriptors.entrySet()) {
> - if
> (entry.getKey().getRole().getName().equals(role.getName())) {
> + if (entry.getKey().getRole() == role) {
I'm not sure about this change. I don't know why you've changed it.
I think what you've done may fail if the classes have been loaded from
a different class loader. Have you found a place in the Java spec
mentioning that equals for Class will always compare the class name
and not the class refefence?
note: I've seen you've changed this in lots of place so it's important
to be sure about what you're doing. I had done it this way to be on
the safe side and so that it'll always work.
[snip]
> - &&
> List.class.isAssignableFrom(dependency.getMappingType()))
> - {
> + if ((dependency.getMappingType() != null) &&
> List.class.isAssignableFrom(dependency.getMappingType())) {
> fieldValue = lookupList(dependency.getRole());
> } else if ((dependency.getMappingType() != null)
> - &&
> Map.class.isAssignableFrom(dependency.getMappingType()))
> - {
> + &&
> Map.class.isAssignableFrom(dependency.getMappingType())) {
> fieldValue = lookupMap(dependency.getRole());
> } else {
> fieldValue = lookup(dependency.getRole(),
> dependency.getRoleHint());
> }
> -
> +
> // Set the field by introspection
> if (fieldValue != null) {
> ReflectionUtils.setFieldValue(instance,
> dependency.getName(), fieldValue);
> @@ -313,16 +344,15 @@
> if
> (LogEnabled.class.isAssignableFrom(descriptor.getImplementation())) {
> ((LogEnabled) instance).enableLogging(new
> CommonsLoggingLogger(instance.getClass()));
> }
> -
> +
> // Composable
> // Only support Composable for classes implementing
> ComponentManager since for all other components
> // they should have ComponentManager injected.
> - if
> (ComponentManager
> .class.isAssignableFrom(descriptor.getImplementation())
> - &&
> Composable.class.isAssignableFrom(descriptor.getImplementation()))
> - {
> + if
> (ComponentManager
> .class.isAssignableFrom(descriptor.getImplementation())
> + &&
> Composable.class.isAssignableFrom(descriptor.getImplementation())) {
> ((Composable) instance).compose(this);
> }
note that these code style changes are not respecting what we had
decided, ie that for long lines the wrapped line will be formatted as:
if (.....
&& .....)
{
whatever
}
> -
> +
> // Initializable
> if
> (Initializable
> .class.isAssignableFrom(descriptor.getImplementation())) {
> ((Initializable) instance).initialize();
>
> Modified: platform/core/trunk/xwiki-component/xwiki-component-
> default/src/main/java/org/xwiki/component/internal/
> DefaultComponentManager.java
> ===================================================================
> --- platform/core/trunk/xwiki-component/xwiki-component-default/src/
> main/java/org/xwiki/component/internal/DefaultComponentManager.java
> 2009-07-13 11:47:52 UTC (rev 21909)
> +++ platform/core/trunk/xwiki-component/xwiki-component-default/src/
> main/java/org/xwiki/component/internal/DefaultComponentManager.java
> 2009-07-13 13:20:03 UTC (rev 21910)
> @@ -31,8 +31,8 @@
> import org.xwiki.component.manager.ComponentRepositoryException;
>
> /**
> - * Wraps the Component Manager in a component so that components
> requiring the component Manager can
> - * have it injected automatically.
> + * Wraps the Component Manager in a component so that components
> requiring the component Manager can have it injected
> + * automatically.
> *
> * @version $Id$
> * @since 2.0M1
> @@ -47,6 +47,7 @@
>
> /**
> * {@inheritDoc}
> + *
> * @see Composable#compose(ComponentManager)
> */
> public void compose(ComponentManager componentManager)
> @@ -56,15 +57,17 @@
>
> /**
> * {@inheritDoc}
> + *
> * @see ComponentManager#getComponentDescriptor(Class, String)
> */
> public <T> ComponentDescriptor<T>
> getComponentDescriptor(Class<T> role, String roleHint)
> {
> return this.componentManager.getComponentDescriptor(role,
> roleHint);
> }
> -
> +
> /**
> * {@inheritDoc}
> + *
> * @see ComponentManager#getComponentDescriptorList(Class)
> */
> public <T> List<ComponentDescriptor<T>>
> getComponentDescriptorList(Class<T> role)
> @@ -74,6 +77,7 @@
>
> /**
> * {@inheritDoc}
> + *
> * @see ComponentManager#lookup(Class, String)
> */
> public <T> T lookup(Class<T> role, String roleHint) throws
> ComponentLookupException
> @@ -83,6 +87,7 @@
>
> /**
> * {@inheritDoc}
> + *
> * @see ComponentManager#lookup(Class)
> */
> public <T> T lookup(Class<T> role) throws ComponentLookupException
> @@ -92,6 +97,7 @@
>
> /**
> * {@inheritDoc}
> + *
> * @see ComponentManager#lookupList(Class)
> */
> public <T> List<T> lookupList(Class<T> role) throws
> ComponentLookupException
> @@ -101,6 +107,7 @@
>
> /**
> * {@inheritDoc}
> + *
> * @see ComponentManager#lookupMap(Class)
> */
> public <T> Map<String, T> lookupMap(Class<T> role) throws
> ComponentLookupException
> @@ -110,6 +117,7 @@
>
> /**
> * {@inheritDoc}
> + *
> * @see ComponentManager#registerComponent(ComponentDescriptor)
> */
> public <T> void registerComponent(ComponentDescriptor<T>
> componentDescriptor) throws ComponentRepositoryException
> @@ -119,6 +127,29 @@
>
> /**
> * {@inheritDoc}
> + *
> + * @see
> org
> .xwiki
> .component
> .manager
> .ComponentManager
> #registerComponent(org.xwiki.component.descriptor.ComponentDescriptor,
> + * java.lang.Object)
> + */
Is this valid javadoc? I doubt it.
> + public <T> void registerComponent(ComponentDescriptor<T>
> componentDescriptor, T componentInstance)
> + throws ComponentRepositoryException
> + {
> +
> this.componentManager.registerComponent(componentDescriptor,
> componentInstance);
> + }
> +
> + /**
> + * {@inheritDoc}
> + *
> + * @see
> org
> .xwiki
> .component
> .manager.ComponentManager#unregisterComponent(java.lang.Class,
> java.lang.String)
> + */
> + public void unregisterComponent(Class< ? > role, String roleHint)
> + {
> + this.componentManager.unregisterComponent(role, roleHint);
> + }
> +
> + /**
> + * {@inheritDoc}
> + *
> * @see ComponentManager#release(Object)
> */
> public <T> void release(T component) throws
> ComponentLifecycleException
> @@ -128,6 +159,7 @@
>
> /**
> * {@inheritDoc}
> + *
> * @see ComponentManager#hasComponent(Class, String)
> */
> public <T> boolean hasComponent(Class<T> role, String roleHint)
> @@ -137,6 +169,7 @@
>
> /**
> * {@inheritDoc}
> + *
> * @see ComponentManager#hasComponent(Class)
> */
> public <T> boolean hasComponent(Class<T> role)
> @@ -146,6 +179,7 @@
>
> /**
> * {@inheritDoc}
> + *
> * @see
> ComponentManager#setComponentEventManager(ComponentEventManager)
> */
> public void setComponentEventManager(ComponentEventManager
> eventManager)
>
> Modified: platform/core/trunk/xwiki-component/xwiki-component-
> default/src/main/java/org/xwiki/component/internal/
> StackingComponentEventManager.java
> ===================================================================
> --- platform/core/trunk/xwiki-component/xwiki-component-default/src/
> main/java/org/xwiki/component/internal/
> StackingComponentEventManager.java 2009-07-13 11:47:52 UTC (rev 21909)
> +++ platform/core/trunk/xwiki-component/xwiki-component-default/src/
> main/java/org/xwiki/component/internal/
> StackingComponentEventManager.java 2009-07-13 13:20:03 UTC (rev 21910)
> @@ -23,13 +23,15 @@
>
> import org.xwiki.component.descriptor.ComponentDescriptor;
> import org.xwiki.component.manager.ComponentDescriptorAddedEvent;
> +import org.xwiki.component.manager.ComponentDescriptorRemovedEvent;
> import org.xwiki.component.manager.ComponentEventManager;
> import org.xwiki.observation.ObservationManager;
> +import org.xwiki.observation.event.Event;
>
> /**
> - * Allow stacking component events and flush them whenever the user
> of this class wants to. This is used for example
> - * at application initialization time when we don't want to send
> events before the Application Context has been
> - * initialized since components subscribing to these events may
> want to use the Application Context.
> + * Allow stacking component events and flush them whenever the user
> of this class wants to. This is used for example at
> + * application initialization time when we don't want to send
> events before the Application Context has been initialized
> + * since components subscribing to these events may want to use the
> Application Context.
> *
> * @version $Id$
> * @since 2.0M1
> @@ -37,48 +39,78 @@
> public class StackingComponentEventManager implements
> ComponentEventManager
> {
> private ObservationManager observationManager;
> -
> - private Stack<ComponentDescriptor< ? >> events = new
> Stack<ComponentDescriptor< ? >>();
> -
> +
> + private Stack<ComponentEventEntry> events = new
> Stack<ComponentEventEntry>();
> +
> private boolean shouldStack = true;
> -
> +
> /**
> * {@inheritDoc}
> - * @see ComponentEventManager#notify(ComponentDescriptor)
> + *
> + * @see
> org
> .xwiki
> .component
> .manager
> .ComponentEventManager
> #notifyComponentRegistered
> (org.xwiki.component.descriptor.ComponentDescriptor)
> */
> - public <T> void notify(ComponentDescriptor< T > descriptor)
> + public void notifyComponentRegistered(ComponentDescriptor< ? >
> descriptor)
> {
> - if (this.shouldStack) {
> - synchronized (this) {
> - this.events.push(descriptor);
> - }
> - } else {
> - notifyInternal(descriptor);
> - }
> + notifyComponentEvent(new
> ComponentDescriptorAddedEvent(descriptor.getRole(),
> descriptor.getRoleHint()),
> + descriptor);
> }
> -
> +
> + /**
> + * {@inheritDoc}
> + *
> + * @see
> org
> .xwiki
> .component
> .manager
> .ComponentEventManager
> #notifyComponentUnregistered
> (org.xwiki.component.descriptor.ComponentDescriptor)
> + */
> + public void notifyComponentUnregistered(ComponentDescriptor< ?
> > descriptor)
> + {
> + notifyComponentEvent(new
> ComponentDescriptorRemovedEvent(descriptor.getRole(),
> descriptor.getRoleHint()),
> + descriptor);
> + }
> +
> public synchronized void flushEvents()
> {
> - while(!this.events.isEmpty()) {
> - notifyInternal(events.remove(0));
> + for (ComponentEventEntry entry : this.events) {
> + sendEvent(entry.event, entry.descriptor);
> }
> }
> -
> +
> public void shouldStack(boolean shouldStack)
> {
> this.shouldStack = shouldStack;
> }
> -
> +
> public void setObservationManager(ObservationManager
> observationManager)
> {
> this.observationManager = observationManager;
> }
>
> - private void notifyInternal(ComponentDescriptor< ? > descriptor)
> + private void notifyComponentEvent(Event event,
> ComponentDescriptor< ? > descriptor)
> {
> + if (this.shouldStack) {
> + synchronized (this) {
> + this.events.push(new ComponentEventEntry(event,
> descriptor));
> + }
> + } else {
> + sendEvent(event, descriptor);
> + }
> + }
> +
> + private void sendEvent(Event event, ComponentDescriptor< ? >
> descriptor)
> + {
> if (this.observationManager != null) {
> - ComponentDescriptorAddedEvent event = new
> ComponentDescriptorAddedEvent(descriptor.getRole());
> this.observationManager.notify(event, this, descriptor);
> }
> }
> +
> + static class ComponentEventEntry
> + {
> + public Event event;
> +
> + public ComponentDescriptor< ? > descriptor;
> +
> + public ComponentEventEntry(Event event,
> ComponentDescriptor< ? > descriptor)
> + {
> + this.event = event;
> + this.descriptor = descriptor;
> + }
> + }
> }
>
>
> Property changes on: platform/core/trunk/xwiki-component/xwiki-
> component-default/src/main/java/org/xwiki/component/internal/
> StackingComponentEventManager.java
> ___________________________________________________________________
> Name: svn:keywords
> - Id
>
> Added: platform/core/trunk/xwiki-component/xwiki-component-default/
> src/main/java/org/xwiki/component/manager/
> AbstractComponentDescriptorEvent.java
> ===================================================================
> --- platform/core/trunk/xwiki-component/xwiki-component-default/src/
> main/java/org/xwiki/component/manager/
> AbstractComponentDescriptorEvent.java (rev 0)
> +++ platform/core/trunk/xwiki-component/xwiki-component-default/src/
> main/java/org/xwiki/component/manager/
> AbstractComponentDescriptorEvent.java 2009-07-13 13:20:03 UTC (rev
> 21910)
> @@ -0,0 +1,100 @@
> +/*
> + * See the NOTICE file distributed with this work for additional
> + * information regarding copyright ownership.
> + *
> + * This is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as
> + * published by the Free Software Foundation; either version 2.1 of
> + * the License, or (at your option) any later version.
> + *
> + * This software is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this software; if not, write to the Free
> + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
> + */
> +package org.xwiki.component.manager;
> +
> +import org.xwiki.observation.event.Event;
> +
> +/**
> + * Base class for events about components descriptors.
> + *
> + * @version $Id$
> + * @since 2.0M1
2.0M2?
[snip]
> + */
> +public abstract class AbstractComponentDescriptorEvent implements
> Event
> +{
> + private Class< ? > role;
> +
> + private String roleHint;
> +
> + /**
> + * Watches all roles (whenever a component is added it'll
> trigger this event).
> + */
> + public AbstractComponentDescriptorEvent()
> + {
> + this.role = null;
> + }
> +
> + /**
> + * @param role the component role to watch (all components
> matching this role will trigger this event)
> + */
> + public AbstractComponentDescriptorEvent(Class< ? > role)
> + {
> + this.role = role;
> + }
> +
> + /**
> + * @param role the component role/rolehint to watch
> + */
> + public AbstractComponentDescriptorEvent(Class< ? > role, String
> roleHint)
> + {
> + this.role = role;
> + this.roleHint = roleHint;
> + }
> +
> + /**
> + * @return the component's role being watched or null if all
> components registrations are watched
> + */
> + public Class< ? > getRole()
> + {
> + return this.role;
> + }
> +
> + /**
> + * @return the component's role hint being watched or null if
> all role's components registrations are watched
> + */
> + public String getRoleHint()
> + {
> + return this.roleHint;
> + }
> +
> + /**
> + * {@inheritDoc}
> + *
> + * @see
> org.xwiki.observation.event.Event#matches(java.lang.Object)
> + */
> + public boolean matches(Object otherEvent)
> + {
> + boolean result = false;
> +
> + if (otherEvent instanceof AbstractComponentDescriptorEvent) {
> + // If we're watching all roles return a match
> + if (getRole() == null) {
> + result = true;
> + } else {
> + AbstractComponentDescriptorEvent event =
> (AbstractComponentDescriptorEvent) otherEvent;
> + if (getRole() == event.getRole()) {
> + result = getRoleHint() == null ||
> getRoleHint().equals(event.getRoleHint());
> + }
> + }
> + }
> +
> + return result;
> + }
> +}
>
>
> Property changes on: platform/core/trunk/xwiki-component/xwiki-
> component-default/src/main/java/org/xwiki/component/manager/
> AbstractComponentDescriptorEvent.java
> ___________________________________________________________________
> Name: svn:mime-type
> + text/plain
> Name: svn:keywords
> + Id
> Name: svn:eol-style
> + native
>
> Modified: 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 2009-07-13 11:47:52 UTC (rev 21909)
> +++ platform/core/trunk/xwiki-component/xwiki-component-default/src/
> main/java/org/xwiki/component/manager/
> ComponentDescriptorAddedEvent.java 2009-07-13 13:20:03 UTC (rev 21910)
> @@ -27,54 +27,45 @@
> * @version $Id$
> * @since 2.0M1
> */
> -public class ComponentDescriptorAddedEvent implements Event
> +public class ComponentDescriptorAddedEvent extends
> AbstractComponentDescriptorEvent
> {
> - private Class< ? > role;
> -
> /**
> * Watches all roles (whenever a component is added it'll
> trigger this event).
> */
> public ComponentDescriptorAddedEvent()
> {
> - this.role = null;
> +
> }
> -
> +
> /**
> * @param role the component role to watch (all components
> matching this role will trigger this event)
> */
> public ComponentDescriptorAddedEvent(Class< ? > role)
> {
> - this.role = role;
> + super(role);
> }
> -
> +
> /**
> - * @return the component's role being watched or null if all
> component registrations are watched
> + * @param role the component role/rolehint to watch
> */
> - public Class< ? > getRole()
> + public ComponentDescriptorAddedEvent(Class< ? > role, String
> roleHint)
> {
> - return this.role;
> + super(role, roleHint);
> }
> -
> +
> /**
> * {@inheritDoc}
> + *
> * @see Event#matches(Object)
> */
> public boolean matches(Object otherEvent)
> {
> boolean result = false;
> -
> +
> if
> (ComponentDescriptorAddedEvent
> .class.isAssignableFrom(otherEvent.getClass())) {
> - // If we're watching all roles return a match
> - if (getRole() == null) {
> - result = true;
> - } else {
> - ComponentDescriptorAddedEvent event =
> (ComponentDescriptorAddedEvent) otherEvent;
> - if
> (getRole().getName().equals(event.getRole().getName())) {
> - result = true;
> - }
> - }
> + result = super.matches(otherEvent);
> }
> -
> - return result;
> +
> + return result;
> }
> }
>
> Added: platform/core/trunk/xwiki-component/xwiki-component-default/
> src/main/java/org/xwiki/component/manager/
> ComponentDescriptorRemovedEvent.java
> ===================================================================
> --- platform/core/trunk/xwiki-component/xwiki-component-default/src/
> main/java/org/xwiki/component/manager/
> ComponentDescriptorRemovedEvent.java (rev 0)
> +++ platform/core/trunk/xwiki-component/xwiki-component-default/src/
> main/java/org/xwiki/component/manager/
> ComponentDescriptorRemovedEvent.java 2009-07-13 13:20:03 UTC (rev
> 21910)
> @@ -0,0 +1,69 @@
> +/*
> + * See the NOTICE file distributed with this work for additional
> + * information regarding copyright ownership.
> + *
> + * This is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as
> + * published by the Free Software Foundation; either version 2.1 of
> + * the License, or (at your option) any later version.
> + *
> + * This software is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this software; if not, write to the Free
> + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
> + */
> +package org.xwiki.component.manager;
> +
> +/**
> + * Event sent to tell that a new Component Descriptor has been
> unregistered.
> + *
> + * @version $Id$
> + * @since 2.0M2
> + */
> +public class ComponentDescriptorRemovedEvent extends
> AbstractComponentDescriptorEvent
> +{
> + /**
> + * Watches all roles (whenever a component is added it'll
> trigger this event).
> + */
> + public ComponentDescriptorRemovedEvent()
> + {
> +
> + }
This needs a comment to explain why it's empty since empty blocks
trigger a checkstyle exception.
[snip]
> +
> + /**
> + * @param role the component role to watch (all components
> matching this role will trigger this event)
> + */
> + public ComponentDescriptorRemovedEvent(Class< ? > role)
> + {
> + super(role);
> + }
> +
> + /**
> + * @param role the component role/rolehint to watch
> + */
> + public ComponentDescriptorRemovedEvent(Class< ? > role, String
> roleHint)
> + {
> + super(role, roleHint);
> + }
> +
> + /**
> + * {@inheritDoc}
> + *
> + * @see
> org
> .xwiki
> .component
> .manager.AbstractComponentDescriptorEvent#matches(java.lang.Object)
> + */
> + public boolean matches(Object otherEvent)
> + {
> + boolean result = false;
> +
> + if
> (ComponentDescriptorRemovedEvent
> .class.isAssignableFrom(otherEvent.getClass())) {
> + result = super.matches(otherEvent);
> + }
> +
> + return result;
> + }
> +}
>
>
> Property changes on: platform/core/trunk/xwiki-component/xwiki-
> component-default/src/main/java/org/xwiki/component/manager/
> ComponentDescriptorRemovedEvent.java
> ___________________________________________________________________
> Name: svn:mime-type
> + text/plain
> Name: svn:keywords
> + Id
> Name: svn:eol-style
> + native
>
> Modified: platform/core/trunk/xwiki-component/xwiki-component-
> default/src/test/java/org/xwiki/component/embed/
> EmbeddableComponentManagerTest.java
> ===================================================================
> --- platform/core/trunk/xwiki-component/xwiki-component-default/src/
> test/java/org/xwiki/component/embed/
> EmbeddableComponentManagerTest.java 2009-07-13 11:47:52 UTC (rev
> 21909)
> +++ platform/core/trunk/xwiki-component/xwiki-component-default/src/
> test/java/org/xwiki/component/embed/
> EmbeddableComponentManagerTest.java 2009-07-13 13:20:03 UTC (rev
> 21910)
> @@ -26,6 +26,7 @@
> import org.junit.Test;
> import org.xwiki.component.descriptor.ComponentDescriptor;
> import org.xwiki.component.descriptor.DefaultComponentDescriptor;
> +import org.xwiki.component.manager.ComponentLookupException;
>
> /**
> * Unit tests for {@link EmbeddableComponentManager}.
> @@ -38,20 +39,20 @@
> public static interface Role
> {
> }
> -
> +
> public static class RoleImpl implements Role
> {
> }
> -
> +
> public static class OtherRoleImpl implements Role
> {
> }
> -
> +
> @Test
> public void testGetComponentDescriptorList() throws Exception
> {
> EmbeddableComponentManager ecm = new
> EmbeddableComponentManager();
> -
> +
> DefaultComponentDescriptor<Role> d1 = new
> DefaultComponentDescriptor<Role>();
> d1.setRole(Role.class);
> d1.setRoleHint("hint1");
> @@ -61,13 +62,13 @@
> d2.setRole(Role.class);
> d2.setRoleHint("hint2");
> ecm.registerComponent(d2);
> -
> +
> List<ComponentDescriptor<Role>> cds =
> ecm.getComponentDescriptorList(Role.class);
> Assert.assertEquals(2, cds.size());
> Assert.assertTrue(cds.contains(d1));
> Assert.assertTrue(cds.contains(d2));
> }
> -
> +
> @Test
> public void testRegisterComponentOverExistingOne() throws
> Exception
> {
> @@ -79,14 +80,50 @@
> ecm.registerComponent(d1);
>
> Object instance = ecm.lookup(Role.class);
> - Assert.assertEquals(RoleImpl.class.getName(),
> instance.getClass().getName());
> -
> + Assert.assertSame(RoleImpl.class, instance.getClass());
> +
> DefaultComponentDescriptor<Role> d2 = new
> DefaultComponentDescriptor<Role>();
> d2.setRole(Role.class);
> d2.setImplementation(OtherRoleImpl.class);
> ecm.registerComponent(d2);
>
> instance = ecm.lookup(Role.class);
> - Assert.assertEquals(OtherRoleImpl.class.getName(),
> instance.getClass().getName());
> + Assert.assertSame(OtherRoleImpl.class, instance.getClass());
> }
> +
> + @Test
> + public void testRegisterComponentInstance() throws Exception
> + {
> + EmbeddableComponentManager ecm = new
> EmbeddableComponentManager();
> +
> + DefaultComponentDescriptor<Role> d1 = new
> DefaultComponentDescriptor<Role>();
> + d1.setRole(Role.class);
> + d1.setImplementation(RoleImpl.class);
> + Role instance = new RoleImpl();
> + ecm.registerComponent(d1, instance);
> +
> + Assert.assertSame(instance, ecm.lookup(Role.class));
> + }
> +
> + @Test
> + public void testUnregisterComponent() throws Exception
> + {
> + EmbeddableComponentManager ecm = new
> EmbeddableComponentManager();
> +
> + DefaultComponentDescriptor<Role> d1 = new
> DefaultComponentDescriptor<Role>();
> + d1.setRole(Role.class);
> + d1.setImplementation(RoleImpl.class);
> + ecm.registerComponent(d1);
> +
> + Assert.assertSame(RoleImpl.class,
> ecm.lookup(Role.class).getClass());
> +
> + ecm.unregisterComponent(d1.getRole(), d1.getRoleHint());
> +
> + try {
> + ecm.lookup(d1.getRole());
> + Assert.fail("Should have thrown a
> ComponentLookupException");
> + } catch (ComponentLookupException e) {
> + // expected
> + }
> + }
> }
>
> Modified: platform/core/trunk/xwiki-component/xwiki-component-
> default/src/test/java/org/xwiki/component/manager/
> ComponentDescriptorAddedEventTest.java
> ===================================================================
> --- platform/core/trunk/xwiki-component/xwiki-component-default/src/
> test/java/org/xwiki/component/manager/
> ComponentDescriptorAddedEventTest.java 2009-07-13 11:47:52 UTC (rev
> 21909)
> +++ platform/core/trunk/xwiki-component/xwiki-component-default/src/
> test/java/org/xwiki/component/manager/
> ComponentDescriptorAddedEventTest.java 2009-07-13 13:20:03 UTC (rev
> 21910)
> @@ -38,34 +38,36 @@
> private static class Dummy
> {
> }
> -
> +
> @Test
> public void testMatchesAllComponentRoles()
> {
> ComponentDescriptorAddedEvent event = new
> ComponentDescriptorAddedEvent();
> // Note: We use any class for the test but it's supposed to
> be a component role class.
> - Assert.assertTrue(event.matches(new
> ComponentDescriptorAddedEvent(Dummy.class)));
> + Assert.assertTrue(event.matches(new
> ComponentDescriptorAddedEvent(Dummy.class, "rolehint")));
> }
>
> @Test
> public void testMatchesWhenDifferentEvent()
> {
> ComponentDescriptorAddedEvent event = new
> ComponentDescriptorAddedEvent();
> - Assert.assertFalse(event.matches(new Event() {
> + Assert.assertFalse(event.matches(new Event()
> + {
> public boolean matches(Object otherEvent)
> {
> return false;
> }
> }));
> }
> -
> +
> @Test
> public void testMatchesWhenSpecificRoleSpecified()
> {
> // Note: We use any class for the test but it's supposed to
> be a component role class.
> ComponentDescriptorAddedEvent event = new
> ComponentDescriptorAddedEvent(Dummy.class);
> - Assert.assertTrue(event.matches(new
> ComponentDescriptorAddedEvent(Dummy.class)));
> + Assert.assertTrue(event.matches(new
> ComponentDescriptorAddedEvent(Dummy.class, "rolehint")));
> // Use a different class so that it doesn't match
> - Assert.assertFalse(event.matches(new
> ComponentDescriptorAddedEvent(ComponentDescriptorAddedEvent.class)));
> + Assert.assertFalse(event.matches(new
> ComponentDescriptorAddedEvent(ComponentDescriptorAddedEvent.class,
> + "rolehint")));
> }
> }
>
> Modified: platform/core/trunk/xwiki-core/src/test/java/com/xpn/xwiki/
> test/AbstractBridgedXWikiComponentTestCase.java
> ===================================================================
> --- platform/core/trunk/xwiki-core/src/test/java/com/xpn/xwiki/test/
> AbstractBridgedXWikiComponentTestCase.java 2009-07-13 11:47:52 UTC
> (rev 21909)
> +++ platform/core/trunk/xwiki-core/src/test/java/com/xpn/xwiki/test/
> AbstractBridgedXWikiComponentTestCase.java 2009-07-13 13:20:03 UTC
> (rev 21910)
> @@ -20,6 +20,7 @@
> package com.xpn.xwiki.test;
>
> import org.jmock.Mock;
> +import org.xwiki.component.descriptor.DefaultComponentDescriptor;
> import org.xwiki.component.manager.ComponentManager;
> import org.xwiki.container.Container;
> import org.xwiki.context.Execution;
> @@ -67,7 +68,9 @@
>
> Mock mockCoreConfiguration = mock(CoreConfiguration.class);
>
> mockCoreConfiguration
> .stubs().method("getDefaultDocumentSyntax").will(returnValue("xwiki/
> 1.0"));
> -
> getComponentManager().registerComponent(CoreConfiguration.class,
> mockCoreConfiguration.proxy());
> + DefaultComponentDescriptor<CoreConfiguration> descriptor =
> new DefaultComponentDescriptor<CoreConfiguration>();
> + descriptor.setRole(CoreConfiguration.class);
> + getComponentManager().registerComponent(descriptor,
> (CoreConfiguration) mockCoreConfiguration.proxy());
> }
>
> @Override
>
> Modified: platform/core/trunk/xwiki-plexus/src/main/java/org/xwiki/
> plexus/manager/PlexusComponentManager.java
> ===================================================================
> --- platform/core/trunk/xwiki-plexus/src/main/java/org/xwiki/plexus/
> manager/PlexusComponentManager.java 2009-07-13 11:47:52 UTC (rev
> 21909)
> +++ platform/core/trunk/xwiki-plexus/src/main/java/org/xwiki/plexus/
> manager/PlexusComponentManager.java 2009-07-13 13:20:03 UTC (rev
> 21910)
> @@ -52,6 +52,7 @@
>
> /**
> * {@inheritDoc}
> + *
> * @see ComponentManager#hasComponent(Class, String)
> */
> public <T> boolean hasComponent(Class<T> role, String roleHint)
> @@ -61,6 +62,7 @@
>
> /**
> * {@inheritDoc}
> + *
> * @see ComponentManager#hasComponent(Class)
> */
> public <T> boolean hasComponent(Class<T> role)
> @@ -73,7 +75,8 @@
> *
> * @see org.xwiki.component.manager.ComponentManager#lookup(Class)
> */
> - public <T> T lookup(Class< T > role) throws
> ComponentLookupException
> + @SuppressWarnings("unchecked")
> + public <T> T lookup(Class<T> role) throws
> ComponentLookupException
> {
> T result;
> try {
> @@ -90,7 +93,8 @@
> *
> * @see
> org.xwiki.component.manager.ComponentManager#lookup(Class, String)
> */
> - public <T> T lookup(Class< T > role, String roleHint) throws
> ComponentLookupException
> + @SuppressWarnings("unchecked")
> + public <T> T lookup(Class<T> role, String roleHint) throws
> ComponentLookupException
> {
> T result;
> try {
> @@ -108,7 +112,8 @@
> *
> * @see
> org.xwiki.component.manager.ComponentManager#lookupMap(Class)
> */
> - public <T> Map<String, T> lookupMap(Class< T > role) throws
> ComponentLookupException
> + @SuppressWarnings("unchecked")
> + public <T> Map<String, T> lookupMap(Class<T> role) throws
> ComponentLookupException
> {
> Map<String, T> result;
> try {
> @@ -125,7 +130,8 @@
> *
> * @see
> org.xwiki.component.manager.ComponentManager#lookupList(Class)
> */
> - public <T> List< T > lookupList(Class< T > role) throws
> ComponentLookupException
> + @SuppressWarnings("unchecked")
> + public <T> List<T> lookupList(Class<T> role) throws
> ComponentLookupException
> {
> List<T> result;
> try {
> @@ -156,8 +162,7 @@
> *
> * @see
> org
> .xwiki
> .component
> .manager
> .ComponentManager
> #registerComponent(org.xwiki.component.descriptor.ComponentDescriptor)
> */
> - public <T> void registerComponent(ComponentDescriptor<T>
> componentDescriptor)
> - throws ComponentRepositoryException
> + public <T> void registerComponent(ComponentDescriptor<T>
> componentDescriptor) throws ComponentRepositoryException
> {
> org.codehaus.plexus.component.repository.ComponentDescriptor
> pcd =
> createPlexusComponentDescriptor(componentDescriptor);
> @@ -169,22 +174,37 @@
> }
> }
>
> + public <T> void registerComponent(ComponentDescriptor<T>
> componentDescriptor, T componentInstance)
> + throws ComponentRepositoryException
> + {
> + // TODO: find what to do here
> + throw new RuntimeException("Not implemented");
> + }
> +
> + public void unregisterComponent(Class< ? > role, String roleHint)
> + {
> + // TODO: find what to do here
> + throw new RuntimeException("Not implemented");
> + }
> +
> /**
> * {@inheritDoc}
> + *
> * @see ComponentManager#getComponentDescriptor(Class, String)
> * @since 2.0M1
> */
> - public <T> ComponentDescriptor<T> getComponentDescriptor(Class<
> T > role, String roleHint)
> + public <T> ComponentDescriptor<T>
> getComponentDescriptor(Class<T> role, String roleHint)
> {
> - return createXWikiComponentDescriptor(
> -
> this.plexusContainer.getComponentDescriptor(role.getName(),
> roleHint));
> + return
> createXWikiComponentDescriptor
> (this.plexusContainer.getComponentDescriptor(role.getName(),
> roleHint));
> }
> -
> +
> /**
> * {@inheritDoc}
> + *
> * @see ComponentManager#getComponentDescriptorList(Class)
> * @since 2.0M1
> */
> + @SuppressWarnings("unchecked")
> public <T> List<ComponentDescriptor<T>>
> getComponentDescriptorList(Class<T> role)
> {
>
> List<org.codehaus.plexus.component.repository.ComponentDescriptor>
> pcds =
> @@ -194,11 +214,13 @@
> for
> (org.codehaus.plexus.component.repository.ComponentDescriptor pcd :
> pcds) {
> results.add((ComponentDescriptor<T>)
> createXWikiComponentDescriptor(pcd));
> }
> +
> return results;
> }
>
> /**
> * {@inheritDoc}
> + *
> * @see
> ComponentManager#setComponentEventManager(ComponentEventManager)
> */
> public void setComponentEventManager(ComponentEventManager
> eventManager)
> @@ -206,23 +228,24 @@
> // Do nothing since there's no hook to get Plexus Events.
> }
>
> + @SuppressWarnings("unchecked")
> private <T> ComponentDescriptor<T> createXWikiComponentDescriptor(
> org.codehaus.plexus.component.repository.ComponentDescriptor
> pcd)
> {
> DefaultComponentDescriptor<T> descriptor = null;
> -
> +
> if (pcd != null) {
> descriptor = new DefaultComponentDescriptor<T>();
> descriptor.setImplementation((Class< ? extends T>)
> loadClass(pcd.getImplementation()));
> descriptor.setRoleHint(pcd.getRoleHint());
> descriptor.setRole((Class<T>) loadClass(pcd.getRole()));
> -
> +
> if ("per-lookup".equals(pcd.getInstantiationStrategy())) {
>
> descriptor
> .setInstantiationStrategy(ComponentInstantiationStrategy.PER_LOOKUP);
> } else {
>
> descriptor
> .setInstantiationStrategy(ComponentInstantiationStrategy.SINGLETON);
> }
> -
> +
> // Copy dependencies
> for (ComponentRequirement requirement :
> (List<ComponentRequirement>) pcd.getRequirements()) {
> DefaultComponentDependency dependency = new
> DefaultComponentDependency();
> @@ -230,18 +253,18 @@
> dependency.setRoleHint(requirement.getRoleHint());
>
> dependency
> .setMappingType(loadClass(requirement.getFieldMappingType()));
> dependency.setName(requirement.getFieldName());
> -
> +
> // TODO: Handle specific hints when we move to a
> more recent Plexus version.
> // See createPlexusComponentDescriptor
> descriptor.addComponentDependency(dependency);
> }
> }
> -
> - return descriptor;
> +
> + return descriptor;
> }
> -
> +
> private
> org.codehaus.plexus.component.repository.ComponentDescriptor
> createPlexusComponentDescriptor(
> - ComponentDescriptor<?> componentDescriptor)
> + ComponentDescriptor< ? > componentDescriptor)
> {
> org.codehaus.plexus.component.repository.ComponentDescriptor
> pcd =
> new
> org.codehaus.plexus.component.repository.ComponentDescriptor();
> @@ -249,7 +272,7 @@
> pcd.setRole(componentDescriptor.getRole().getName());
> pcd.setRoleHint(componentDescriptor.getRoleHint());
>
> pcd
> .setImplementation(componentDescriptor.getImplementation().getName());
> -
> +
> switch (componentDescriptor.getInstantiationStrategy()) {
> case PER_LOOKUP:
> pcd.setInstantiationStrategy("per-lookup");
> @@ -258,22 +281,19 @@
> pcd.setInstantiationStrategy("singleton");
> }
>
> - Collection<ComponentDependency<?>> componentDependencies =
> componentDescriptor.getComponentDependencies();
> - for (ComponentDependency<?> dependency :
> componentDependencies) {
> + Collection<ComponentDependency< ? >> componentDependencies
> = componentDescriptor.getComponentDependencies();
> + for (ComponentDependency< ? > dependency :
> componentDependencies) {
> ComponentRequirement requirement;
> -
> +
> // Handles several hints in case of lists (collections
> or maps)
> if
> (Collection.class.isAssignableFrom(dependency.getMappingType())
> - ||
> Map.class.isAssignableFrom(dependency.getMappingType()))
> - {
> + ||
> Map.class.isAssignableFrom(dependency.getMappingType())) {
> // TODO: Uncomment when we move to a more recent
> Plexus version which implements
> // ComponentRequirementList.
> /*
> - String[] hints = dependency.getHints();
> - if (hints != null && hints.length > 0) {
> -
> ((ComponentRequirementList
> )requirement).setRoleHints(Arrays.asList(hints));
> - }
> - */
> + * String[] hints = dependency.getHints(); if
> (hints != null && hints.length > 0) {
> + *
> ((ComponentRequirementList
> )requirement).setRoleHints(Arrays.asList(hints)); }
> + */
> requirement = new ComponentRequirement();
> } else {
> requirement = new ComponentRequirement();
> @@ -283,7 +303,7 @@
> requirement.setRoleHint(dependency.getRoleHint());
>
> requirement
> .setFieldMappingType(dependency.getMappingType().getName());
> requirement.setFieldName(dependency.getName());
> -
> +
> pcd.addRequirement(requirement);
> }
>
>
> Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-api/
> src/test/java/org/xwiki/rendering/internal/parser/
> XWikiLinkParserTest.java
> ===================================================================
> --- platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/test/
> java/org/xwiki/rendering/internal/parser/XWikiLinkParserTest.java
> 2009-07-13 11:47:52 UTC (rev 21909)
> +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/test/
> java/org/xwiki/rendering/internal/parser/XWikiLinkParserTest.java
> 2009-07-13 13:20:03 UTC (rev 21910)
> @@ -20,6 +20,8 @@
> package org.xwiki.rendering.internal.parser;
>
> import org.jmock.Mock;
> +import org.xwiki.component.descriptor.ComponentInstantiationStrategy;
> +import org.xwiki.component.descriptor.DefaultComponentDescriptor;
> import org.xwiki.rendering.listener.Link;
> import org.xwiki.rendering.listener.LinkType;
> import org.xwiki.rendering.parser.LinkParser;
> @@ -36,9 +38,15 @@
> {
> // Create a Mock WikiModel implementation so that the link
> parser works in wiki mode
> Mock mockWikiModel = mock(WikiModel.class);
> - getComponentManager().registerComponent(WikiModel.class,
> mockWikiModel.proxy());
> - LinkParser parser =
> getComponentManager().lookup(LinkParser.class, "xwiki/2.0");
>
> + DefaultComponentDescriptor<WikiModel> componentDescriptor =
> new DefaultComponentDescriptor<WikiModel>();
> + componentDescriptor.setRole(WikiModel.class);
> +
> componentDescriptor
> .setInstantiationStrategy(ComponentInstantiationStrategy.SINGLETON);
> + componentDescriptor.setImplementation(null);
> +
> +
> getComponentManager().registerComponent(componentDescriptor,
> (WikiModel) mockWikiModel.proxy());
> + LinkParser parser =
> getComponentManager().lookup(LinkParser.class, "xwiki/2.0");
> +
> Link link = parser.parse("");
> assertEquals("", link.getReference());
> assertEquals("Reference = []", link.toString());
> @@ -85,7 +93,7 @@
> assertEquals("mailto:john@smith.com", link.getReference());
> assertEquals(LinkType.URI, link.getType());
> assertEquals("Reference = [mailto:john@smith.com]",
> link.toString());
> -
> +
> // Verify image: URI is recognized
> link = parser.parse("image:some:content");
> assertEquals("image:some:content", link.getReference());
> @@ -97,13 +105,13 @@
> assertEquals("attach:some:content", link.getReference());
> assertEquals(LinkType.URI, link.getType());
> assertEquals("Reference = [attach:some:content]",
> link.toString());
> -
> +
> // Verify that unknown URIs are ignored
> // Note: We consider that myxwiki is the wiki name and http://xwiki.org
> is the page name
> link = parser.parse("mywiki:http://xwiki.org");
> assertEquals("mywiki:http://xwiki.org", link.getReference());
> assertEquals(LinkType.DOCUMENT, link.getType());
> assertEquals("Reference = [mywiki:http://xwiki.org]",
> link.toString());
> -
> +
> + }
> }
> -}
>
> Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/
> xwiki-rendering-macro-groovy/src/test/java/org/xwiki/rendering/macro/
> groovy/RenderingTests.java
> ===================================================================
> --- platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-groovy/src/test/java/org/xwiki/rendering/macro/
> groovy/RenderingTests.java 2009-07-13 11:47:52 UTC (rev 21909)
> +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-groovy/src/test/java/org/xwiki/rendering/macro/
> groovy/RenderingTests.java 2009-07-13 13:20:03 UTC (rev 21910)
> @@ -28,6 +28,7 @@
> import org.jmock.Expectations;
> import org.jmock.Mockery;
> import org.xwiki.bridge.DocumentAccessBridge;
> +import org.xwiki.component.descriptor.DefaultComponentDescriptor;
> import org.xwiki.component.embed.EmbeddableComponentManager;
> import org.xwiki.rendering.scaffolding.RenderingTestSuite;
> import org.xwiki.script.ScriptContextManager;
> @@ -61,18 +62,32 @@
>
> // Document Access Bridge Mock
> final DocumentAccessBridge mockDocumentAccessBridge =
> context.mock(DocumentAccessBridge.class);
> - context.checking(new Expectations() {{
> -
> allowing(mockDocumentAccessBridge).hasProgrammingRights();
> will(returnValue(true));
> - }});
> -
> componentManager.registerComponent(DocumentAccessBridge.class,
> mockDocumentAccessBridge);
> -
> + context.checking(new Expectations()
> + {
> + {
> +
> allowing(mockDocumentAccessBridge).hasProgrammingRights();
> + will(returnValue(true));
> + }
> + });
hmmm this is not following the recommended code style for
expectations. See jmock.org`
Can you please revert all these style changes?
side note: This why I never apply code style blindly. It'll always
break stuff you've done voluntarily.
> + DefaultComponentDescriptor<DocumentAccessBridge>
> descriptorDAB =
> + new DefaultComponentDescriptor<DocumentAccessBridge>();
> + descriptorDAB.setRole(DocumentAccessBridge.class);
> + componentManager.registerComponent(descriptorDAB,
> mockDocumentAccessBridge);
> +
> // Script Context Mock
> final ScriptContextManager mockScriptContextManager =
> context.mock(ScriptContextManager.class);
> final SimpleScriptContext scriptContext = new
> SimpleScriptContext();
> scriptContext.setAttribute("var", "value",
> ScriptContext.ENGINE_SCOPE);
> - context.checking(new Expectations() {{
> - allowing(mockScriptContextManager).getScriptContext();
> will(returnValue(scriptContext));
> - }});
> -
> componentManager.registerComponent(ScriptContextManager.class,
> mockScriptContextManager);
> + context.checking(new Expectations()
> + {
> + {
> +
> allowing(mockScriptContextManager).getScriptContext();
> + will(returnValue(scriptContext));
> + }
> + });
> + DefaultComponentDescriptor<ScriptContextManager>
> descriptorSCM =
> + new DefaultComponentDescriptor<ScriptContextManager>();
> + descriptorSCM.setRole(ScriptContextManager.class);
> + componentManager.registerComponent(descriptorSCM,
> mockScriptContextManager);
> }
> }
>
> Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/
> xwiki-rendering-macro-html/src/test/java/org/xwiki/rendering/
> HTMLMacroTest.java
> ===================================================================
> --- platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-html/src/test/java/org/xwiki/rendering/
> HTMLMacroTest.java 2009-07-13 11:47:52 UTC (rev 21909)
> +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-html/src/test/java/org/xwiki/rendering/
> HTMLMacroTest.java 2009-07-13 13:20:03 UTC (rev 21910)
> @@ -24,7 +24,7 @@
> import org.junit.Test;
> import org.xwiki.bridge.DocumentAccessBridge;
> import org.xwiki.bridge.DocumentNameSerializer;
> -import org.xwiki.component.embed.EmbeddableComponentManager;
> +import org.xwiki.component.descriptor.DefaultComponentDescriptor;
> import org.xwiki.rendering.internal.macro.html.HTMLMacro;
> import org.xwiki.rendering.macro.Macro;
> import org.xwiki.rendering.macro.MacroExecutionException;
> @@ -52,13 +52,19 @@
> {
> super.setUp();
>
> - DocumentAccessBridge dab =
> this.context.mock(DocumentAccessBridge.class);
> - ((EmbeddableComponentManager)
> getComponentManager()).registerComponent(DocumentAccessBridge.class,
> dab);
> -
> - DocumentNameSerializer dns =
> this.context.mock(DocumentNameSerializer.class);
> - ((EmbeddableComponentManager)
> getComponentManager
> ()).registerComponent(DocumentNameSerializer.class, dns);
> + DocumentAccessBridge mockDocumentAccessBridge =
> this.context.mock(DocumentAccessBridge.class);
> + DefaultComponentDescriptor<DocumentAccessBridge>
> descriptorDAB =
> + new DefaultComponentDescriptor<DocumentAccessBridge>();
> + descriptorDAB.setRole(DocumentAccessBridge.class);
> + getComponentManager().registerComponent(descriptorDAB,
> mockDocumentAccessBridge);
> +
> + DocumentNameSerializer mockDocumentNameSerializer =
> this.context.mock(DocumentNameSerializer.class);
> + DefaultComponentDescriptor<DocumentNameSerializer>
> descriptorDNS =
> + new DefaultComponentDescriptor<DocumentNameSerializer>();
> + descriptorDNS.setRole(DocumentNameSerializer.class);
> + getComponentManager().registerComponent(descriptorDNS,
> mockDocumentNameSerializer);
> }
> -
> +
> /**
> * Verify that inline HTML macros with non inline content
> generate an exception.
> */
>
> Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/
> xwiki-rendering-macro-include/src/test/java/org/xwiki/rendering/
> internal/macro/IncludeMacroTest.java
> ===================================================================
> --- platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-include/src/test/java/org/xwiki/rendering/internal/
> macro/IncludeMacroTest.java 2009-07-13 11:47:52 UTC (rev 21909)
> +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-include/src/test/java/org/xwiki/rendering/internal/
> macro/IncludeMacroTest.java 2009-07-13 13:20:03 UTC (rev 21910)
> @@ -24,6 +24,7 @@
>
> import org.jmock.Mock;
> import org.xwiki.bridge.DocumentAccessBridge;
> +import org.xwiki.component.descriptor.DefaultComponentDescriptor;
> import org.xwiki.rendering.block.Block;
> import org.xwiki.rendering.internal.macro.include.IncludeMacro;
> import
> org.xwiki.rendering.internal.transformation.MacroTransformation;
> @@ -47,23 +48,25 @@
> public class IncludeMacroTest extends AbstractRenderingTestCase
> {
> private Mock mockDocumentAccessBridge;
> -
> +
> @Override
> protected void registerComponents() throws Exception
> {
> this.mockDocumentAccessBridge =
> mock(DocumentAccessBridge.class);
> -
> getComponentManager().registerComponent(DocumentAccessBridge.class,
> this.mockDocumentAccessBridge.proxy());
> +
> + DefaultComponentDescriptor<DocumentAccessBridge> descriptor =
> + new DefaultComponentDescriptor<DocumentAccessBridge>();
> + descriptor.setRole(DocumentAccessBridge.class);
> + getComponentManager().registerComponent(descriptor,
> + (DocumentAccessBridge)
> this.mockDocumentAccessBridge.proxy());
> }
>
> public void testIncludeMacroWithNewContext() throws Exception
> {
> - String expected = "beginDocument\n"
> - + "beginMacroMarkerStandalone [velocity] [] [$myvar]\n"
> - + "beginParagraph\n"
> - + "onWord [hello]\n"
> - + "endParagraph\n"
> - + "endMacroMarkerStandalone [velocity] [] [$myvar]\n"
> - + "endDocument";
> + String expected =
> + "beginDocument\n" + "beginMacroMarkerStandalone
> [velocity] [] [$myvar]\n" + "beginParagraph\n"
> + + "onWord [hello]\n" + "endParagraph\n" +
> "endMacroMarkerStandalone [velocity] [] [$myvar]\n"
> + + "endDocument";
I much prefer my version. Can you please rollback? Again an example
why applying code style automatically sucks :)
>
> // Since it's not in the same context, we verify that a
> Velocity variable set in the including page is not
> // seen in the included page.
> @@ -88,21 +91,19 @@
>
> // Create a Macro transformation context with the Macro
> transformation object defined so that the include
> // macro can transform included page which is using a new
> context.
> - MacroTransformation macroTransformation =
> + MacroTransformation macroTransformation =
> (MacroTransformation)
> getComponentManager().lookup(Transformation.class, "macro");
> MacroTransformationContext context = new
> MacroTransformationContext();
> context.setMacroTransformation(macroTransformation);
> -
> +
> List<Block> blocks = macro.execute(parameters, null, context);
>
> assertBlocks(expected, blocks);
> }
> -
> +
> public void testIncludeMacroWithCurrentContext() throws Exception
> {
> - String expected = "beginDocument\n"
> - + "onMacroStandalone [someMacro] []\n"
> - + "endDocument";
> + String expected = "beginDocument\n" + "onMacroStandalone
> [someMacro] []\n" + "endDocument";
again
>
> IncludeMacro macro = (IncludeMacro)
> getComponentManager().lookup(Macro.class, "include");
>
> mockDocumentAccessBridge
> .expects(once()).method("isDocumentViewable").will(returnValue(true));
> @@ -119,7 +120,7 @@
>
> assertBlocks(expected, blocks);
> }
> -
> +
> public void testIncludeMacroWithNoDocumentSpecified() throws
> Exception
> {
> IncludeMacro macro = (IncludeMacro)
> getComponentManager().lookup(Macro.class, "include");
> @@ -129,8 +130,8 @@
> macro.execute(parameters, null, new
> MacroTransformationContext());
> fail("An exception should have been thrown");
> } catch (MacroExecutionException expected) {
> - assertEquals("You must specify a 'document' parameter
> pointing to the document to include.",
> - expected.getMessage());
> + assertEquals("You must specify a 'document' parameter
> pointing to the document to include.", expected
> + .getMessage());
again a brilliant example why it sucks... :(
My version is much nicer.
> }
> }
> }
>
> Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/
> xwiki-rendering-macro-rss/src/test/java/org/xwiki/rendering/
> RenderingTests.java
> ===================================================================
> --- platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-rss/src/test/java/org/xwiki/rendering/
> RenderingTests.java 2009-07-13 11:47:52 UTC (rev 21909)
> +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-rss/src/test/java/org/xwiki/rendering/
> RenderingTests.java 2009-07-13 13:20:03 UTC (rev 21910)
> @@ -22,11 +22,12 @@
> import junit.framework.Test;
> import junit.framework.TestCase;
>
> +import org.jmock.Expectations;
> +import org.jmock.Mockery;
> +import org.xwiki.bridge.SkinAccessBridge;
> +import org.xwiki.component.descriptor.DefaultComponentDescriptor;
> import org.xwiki.rendering.scaffolding.RenderingTestSuite;
> import org.xwiki.test.ComponentManagerTestSetup;
> -import org.xwiki.bridge.SkinAccessBridge;
> -import org.jmock.Mockery;
> -import org.jmock.Expectations;
>
> /**
> * All Rendering integration tests defined in text files using a
> special format.
> @@ -39,21 +40,27 @@
> public static Test suite() throws Exception
> {
> RenderingTestSuite suite = new RenderingTestSuite("Test RSS
> Macro");
> -
> +
> suite.addTestsFromResource("macrorss1", true);
> suite.addTestsFromResource("macrorss2", true);
>
> ComponentManagerTestSetup setup = new
> ComponentManagerTestSetup(suite);
>
> Mockery context = new Mockery();
> - final SkinAccessBridge mockSkinBridge =
> context.mock(SkinAccessBridge.class);
> -
> setup
> .getComponentManager().registerComponent(SkinAccessBridge.class,
> mockSkinBridge);
>
> - context.checking(new Expectations() {{
> -
> allowing(mockSkinBridge).getSkinFile(with(any(String.class)));
> - will(returnValue("/xwiki/skins/albatross/icons/black-
> rss.png"));
> - }});
> -
> + final SkinAccessBridge mockSkinAccessBridge =
> context.mock(SkinAccessBridge.class);
> + DefaultComponentDescriptor<SkinAccessBridge> descriptorSAB
> = new DefaultComponentDescriptor<SkinAccessBridge>();
> + descriptorSAB.setRole(SkinAccessBridge.class);
> +
> setup.getComponentManager().registerComponent(descriptorSAB,
> mockSkinAccessBridge);
> +
> + context.checking(new Expectations()
> + {
> + {
> +
> allowing(mockSkinAccessBridge).getSkinFile(with(any(String.class)));
> + will(returnValue("/xwiki/skins/albatross/icons/
> black-rss.png"));
> + }
> + });
> +
> return setup;
> }
> }
>
> Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/
> xwiki-rendering-macro-script/src/test/java/org/xwiki/rendering/macro/
> script/RenderingTests.java
> ===================================================================
> --- platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-script/src/test/java/org/xwiki/rendering/macro/
> script/RenderingTests.java 2009-07-13 11:47:52 UTC (rev 21909)
> +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-script/src/test/java/org/xwiki/rendering/macro/
> script/RenderingTests.java 2009-07-13 13:20:03 UTC (rev 21910)
> @@ -25,6 +25,7 @@
> import org.jmock.Expectations;
> import org.jmock.Mockery;
> import org.xwiki.bridge.DocumentAccessBridge;
> +import org.xwiki.component.descriptor.DefaultComponentDescriptor;
> import org.xwiki.component.embed.EmbeddableComponentManager;
> import org.xwiki.rendering.scaffolding.RenderingTestSuite;
> import org.xwiki.test.ComponentManagerTestSetup;
> @@ -45,22 +46,30 @@
> suite.addTestsFromResource("macroscript2", true);
> suite.addTestsFromResource("macroscript3", true);
> suite.addTestsFromResource("macroscript4", true);
> -
> +
> ComponentManagerTestSetup testSetup = new
> ComponentManagerTestSetup(suite);
> setUpMocks(testSetup.getComponentManager());
>
> return testSetup;
> }
> -
> +
> public static void setUpMocks(EmbeddableComponentManager
> componentManager)
> {
> Mockery context = new Mockery();
>
> // Document Access Bridge Mock setup
> final DocumentAccessBridge mockDocumentAccessBridge =
> context.mock(DocumentAccessBridge.class);
> - context.checking(new Expectations() {{
> -
> allowing(mockDocumentAccessBridge).hasProgrammingRights();
> will(returnValue(true));
> - }});
> -
> componentManager.registerComponent(DocumentAccessBridge.class,
> mockDocumentAccessBridge);
> + context.checking(new Expectations()
> + {
> + {
> +
> allowing(mockDocumentAccessBridge).hasProgrammingRights();
> + will(returnValue(true));
> + }
> + });
> +
> + DefaultComponentDescriptor<DocumentAccessBridge>
> descriptorDAB =
> + new DefaultComponentDescriptor<DocumentAccessBridge>();
> + descriptorDAB.setRole(DocumentAccessBridge.class);
> + componentManager.registerComponent(descriptorDAB,
> mockDocumentAccessBridge);
> }
> }
>
> Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/
> xwiki-rendering-macro-velocity/src/test/java/org/xwiki/rendering/
> macro/velocity/RenderingTests.java
> ===================================================================
> --- platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-velocity/src/test/java/org/xwiki/rendering/macro/
> velocity/RenderingTests.java 2009-07-13 11:47:52 UTC (rev 21909)
> +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-macros/xwiki-
> rendering-macro-velocity/src/test/java/org/xwiki/rendering/macro/
> velocity/RenderingTests.java 2009-07-13 13:20:03 UTC (rev 21910)
> @@ -24,6 +24,7 @@
>
> import org.jmock.Mockery;
> import org.xwiki.bridge.DocumentAccessBridge;
> +import org.xwiki.component.descriptor.DefaultComponentDescriptor;
> import org.xwiki.component.embed.EmbeddableComponentManager;
> import org.xwiki.rendering.scaffolding.RenderingTestSuite;
> import org.xwiki.test.ComponentManagerTestSetup;
> @@ -64,6 +65,9 @@
>
> // Document Access Bridge Mock
> final DocumentAccessBridge mockDocumentAccessBridge =
> context.mock(DocumentAccessBridge.class);
> -
> componentManager.registerComponent(DocumentAccessBridge.class,
> mockDocumentAccessBridge);
> + DefaultComponentDescriptor<DocumentAccessBridge>
> descriptorDAB =
> + new DefaultComponentDescriptor<DocumentAccessBridge>();
> + descriptorDAB.setRole(DocumentAccessBridge.class);
> + componentManager.registerComponent(descriptorDAB,
> mockDocumentAccessBridge);
> }
> }
>
> Modified: platform/core/trunk/xwiki-shared-tests/src/main/java/org/
> xwiki/test/MockConfigurationSource.java
> ===================================================================
> --- platform/core/trunk/xwiki-shared-tests/src/main/java/org/xwiki/
> test/MockConfigurationSource.java 2009-07-13 11:47:52 UTC (rev 21909)
> +++ platform/core/trunk/xwiki-shared-tests/src/main/java/org/xwiki/
> test/MockConfigurationSource.java 2009-07-13 13:20:03 UTC (rev 21910)
> @@ -23,8 +23,6 @@
> import java.util.List;
> import java.util.Properties;
>
> -import org.xwiki.component.descriptor.ComponentDescriptor;
> -import org.xwiki.component.descriptor.DefaultComponentDescriptor;
> import org.xwiki.configuration.ConfigurationSource;
>
> /**
>
> Modified: platform/core/trunk/xwiki-shared-tests/src/main/java/org/
> xwiki/test/XWikiComponentInitializer.java
> ===================================================================
> --- platform/core/trunk/xwiki-shared-tests/src/main/java/org/xwiki/
> test/XWikiComponentInitializer.java 2009-07-13 11:47:52 UTC (rev
> 21909)
> +++ platform/core/trunk/xwiki-shared-tests/src/main/java/org/xwiki/
> test/XWikiComponentInitializer.java 2009-07-13 13:20:03 UTC (rev
> 21910)
> @@ -24,6 +24,7 @@
> import java.net.MalformedURLException;
> import java.net.URL;
>
> +import org.xwiki.component.descriptor.DefaultComponentDescriptor;
> import org.xwiki.component.embed.EmbeddableComponentManager;
> import org.xwiki.configuration.ConfigurationSource;
> import org.xwiki.container.ApplicationContext;
> @@ -32,13 +33,18 @@
> import org.xwiki.context.ExecutionContext;
> import org.xwiki.context.ExecutionContextManager;
>
> +/**
> + * Initialize a component manager used in unit tests.
> + *
> + * @version $Id$
> + */
> public class XWikiComponentInitializer
> {
> private EmbeddableComponentManager componentManager;
>
> /**
> - * This method should be called before {@link
> #initializeExecution()} since some components will require the
> Container
> - * component to be set up (for example to access resource such
> as the XWikiconfiguration file).
> + * This method should be called before {@link
> #initializeExecution()} since some components will require the
> + * Container component to be set up (for example to access
> resource such as the XWikiconfiguration file).
> */
> public void initializeContainer() throws Exception
> {
> @@ -67,15 +73,24 @@
> }
> });
> }
> -
> +
> public void initializeConfigurationSource() throws Exception
> {
> // Register the mock configuration source for different
> roles so that tests always use the mock
> ConfigurationSource mockSource = new
> MockConfigurationSource();
> -
> getComponentManager().registerComponent(ConfigurationSource.class,
> mockSource);
> -
> getComponentManager().registerComponent(ConfigurationSource.class,
> "xwikiproperties", mockSource);
> +
> + DefaultComponentDescriptor<ConfigurationSource> descriptor;
> +
> + descriptor = new
> DefaultComponentDescriptor<ConfigurationSource>();
> + descriptor.setRole(ConfigurationSource.class);
> + getComponentManager().registerComponent(descriptor,
> mockSource);
> +
> + descriptor = new
> DefaultComponentDescriptor<ConfigurationSource>();
> + descriptor.setRole(ConfigurationSource.class);
> + descriptor.setRoleHint("xwikiproperties");
> + getComponentManager().registerComponent(descriptor,
> mockSource);
> }
> -
> +
> public void initializeExecution() throws Exception
> {
> // Initialize the Execution Context
> @@ -85,11 +100,11 @@
> ExecutionContext ec = new ExecutionContext();
>
> // Make sure we push this empty context in the Execution
> component before we call the initialization
> - // so that we don't get any NPE if some initializer code
> asks to get the Execution Context. This
> + // so that we don't get any NPE if some initializer code
> asks to get the Execution Context. This
> // happens for example with the Velocity Execution Context
> initializer which in turns calls the Velocity
> // Context initializers and some of them look inside the
> Execution Context.
> execution.setContext(ec);
> -
> +
> ecm.initialize(ec);
> }
Thanks
-Vincent
13 Jul
13 Jul
9:17 p.m.
New subject: [xwiki-devs] [xwiki-notifications] r21910 - in platform/core/trunk: xwiki-cache/xwiki-cache-tests/src/main/java/org/xwiki/cache/tests xwiki-component/xwiki-component-api/src/main/java/org/xwiki/component/manager xwiki-component/xwiki-component-default xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/embed xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/internal xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/manager xwiki-compo
Vincent Massol wrote:
Hi Thomas,
On Jul 13, 2009, at 3:20 PM, tmortagne (SVN) wrote:
Author: tmortagne Date: 2009-07-13 15:20:03 +0200 (Mon, 13 Jul 2009) New Revision: 21910
General comments: - Would be great if you could separate code reformatting from code changes. It makes it hard to read (I didn't read the commit fully as a consequence).
+10
- The code style has broken my styles in lots of places (see below for some comments, I haven't commented every single place it broke voluntary formatting).
I'd like to vote for not applying code style blindly in the future. It breaks styles in lots of places and I hate it when I spend a good amount of my time to align code properly and it breaks it for producing suboptimal styling...
There are two things to balance here: - forgetting to format the code manually - suboptimal formatting done automatically I for one prefer to have more codestyle-compliant code than less, and automatic application does this. The way I do commits is: - change the code - check what would the commit look like (using git diff) - if I detect something wrong in the codestyle, I selectively accept changes in the commit, using git add --interactive, which allows me to choose which files to commit, and even inside a file, which changes - final check on the prepared commit using git diff --cached This process takes a while, but it ensures high quality commits. -- Sergiu Dumitriu http://purl.org/net/sergiu/
9:25 p.m.
New subject: [xwiki-devs] [xwiki-notifications] r21910 - in platform/core/trunk: xwiki-cache/xwiki-cache-tests/src/main/java/org/xwiki/cache/tests xwiki-component/xwiki-component-api/src/main/java/org/xwiki/component/manager xwiki-component/xwiki-component-default xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/embed xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/internal xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/manager xwiki-compo
Sergiu Dumitriu wrote:
Vincent Massol wrote:
Hi Thomas,
On Jul 13, 2009, at 3:20 PM, tmortagne (SVN) wrote:
Author: tmortagne Date: 2009-07-13 15:20:03 +0200 (Mon, 13 Jul 2009) New Revision: 21910
General comments: - Would be great if you could separate code reformatting from code changes. It makes it hard to read (I didn't read the commit fully as a consequence).
+10
- The code style has broken my styles in lots of places (see below for some comments, I haven't commented every single place it broke voluntary formatting).
I'd like to vote for not applying code style blindly in the future. It breaks styles in lots of places and I hate it when I spend a good amount of my time to align code properly and it breaks it for producing suboptimal styling...
There are two things to balance here: - forgetting to format the code manually - suboptimal formatting done automatically
I for one prefer to have more codestyle-compliant code than less, and automatic application does this.
I agree with Sergiu here, maybe, if automatic is suboptimal, we need to revisit our codestyle files. It's quite difficult to only format pieces of a file (one's changes), and it often results in forgetting to do it. Happy coding, Anca
The way I do commits is:
- change the code - check what would the commit look like (using git diff) - if I detect something wrong in the codestyle, I selectively accept changes in the commit, using git add --interactive, which allows me to choose which files to commit, and even inside a file, which changes - final check on the prepared commit using git diff --cached
This process takes a while, but it ensures high quality commits.
10:34 p.m.
New subject: [xwiki-devs] [xwiki-notifications] r21910 - in platform/core/trunk: xwiki-cache/xwiki-cache-tests/src/main/java/org/xwiki/cache/tests xwiki-component/xwiki-component-api/src/main/java/org/xwiki/component/manager xwiki-component/xwiki-component-default xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/embed xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/internal xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/manager xwiki-compo
On Jul 13, 2009, at 11:25 PM, Anca Paula Luca wrote:
Sergiu Dumitriu wrote:
Vincent Massol wrote:
Hi Thomas,
On Jul 13, 2009, at 3:20 PM, tmortagne (SVN) wrote:
Author: tmortagne Date: 2009-07-13 15:20:03 +0200 (Mon, 13 Jul 2009) New Revision: 21910
General comments: - Would be great if you could separate code reformatting from code changes. It makes it hard to read (I didn't read the commit fully as a consequence).
+10
- The code style has broken my styles in lots of places (see below for some comments, I haven't commented every single place it broke voluntary formatting).
I'd like to vote for not applying code style blindly in the future. It breaks styles in lots of places and I hate it when I spend a good amount of my time to align code properly and it breaks it for producing suboptimal styling...
There are two things to balance here: - forgetting to format the code manually - suboptimal formatting done automatically
I for one prefer to have more codestyle-compliant code than less, and automatic application does this.
I agree with Sergiu here, maybe, if automatic is suboptimal, we need to revisit our codestyle files. It's quite difficult to only format pieces of a file (one's changes), and it often results in forgetting to do it.
codestyle files are always suboptimal since editors don't support all possible code styles. For ex the eclipse one is wrong in several instances (the IDEA one was better since IDEA supported more options in the past). In any case you cannot get perfect code styles with any IDE. (This is btw why I always format manually as I type). All I'm asking is that people stop blindly reformatting other people's code. This means that you're allowed to use automatic reformatting but you must check thereafter that no weirdies were introduced and manually fix introduced "errors". To see the kind of "errors" I'm talking about see my specific comments to thomas' commit. Thanks -Vincent
Happy coding, Anca
The way I do commits is:
- change the code - check what would the commit look like (using git diff) - if I detect something wrong in the codestyle, I selectively accept changes in the commit, using git add --interactive, which allows me to choose which files to commit, and even inside a file, which changes - final check on the prepared commit using git diff --cached
This process takes a while, but it ensures high quality commits.
14 Jul
14 Jul
6:02 a.m.
New subject: [xwiki-devs] [xwiki-notifications] r21910 - in platform/core/trunk: xwiki-cache/xwiki-cache-tests/src/main/java/org/xwiki/cache/tests xwiki-component/xwiki-component-api/src/main/java/org/xwiki/component/manager xwiki-component/xwiki-component-default xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/embed xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/internal xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/manager xwiki-compo
Vincent Massol wrote: > On Jul 13, 2009, at 11:25 PM, Anca Paula Luca wrote: > >> Sergiu Dumitriu wrote: >>> Vincent Massol wrote: >>>> Hi Thomas, >>>> >>>> On Jul 13, 2009, at 3:20 PM, tmortagne (SVN) wrote: >>>> >>>>> Author: tmortagne >>>>> Date: 2009-07-13 15:20:03 +0200 (Mon, 13 Jul 2009) >>>>> New Revision: 21910 >>>>> >>>> General comments: >>>> - Would be great if you could separate code reformatting from code >>>> changes. It makes it hard to read (I didn't read the commit fully >>>> as a >>>> consequence). >>> +10 >>> >>>> - The code style has broken my styles in lots of places (see below >>>> for >>>> some comments, I haven't commented every single place it broke >>>> voluntary formatting). >>>> >>>> I'd like to vote for not applying code style blindly in the >>>> future. It >>>> breaks styles in lots of places and I hate it when I spend a good >>>> amount of my time to align code properly and it breaks it for >>>> producing suboptimal styling... >>> There are two things to balance here: >>> - forgetting to format the code manually >>> - suboptimal formatting done automatically >>> >>> I for one prefer to have more codestyle-compliant code than less, and >>> automatic application does this. >> I agree with Sergiu here, maybe, if automatic is suboptimal, we need >> to revisit >> our codestyle files. It's quite difficult to only format pieces of a >> file (one's >> changes), and it often results in forgetting to do it. > > codestyle files are always suboptimal since editors don't support all > possible code styles. For ex the eclipse one is wrong in several > instances (the IDEA one was better since IDEA supported more options > in the past). > > In any case you cannot get perfect code styles with any IDE. (This is > btw why I always format manually as I type). Yes, but you can manually split a long line in multiple lines by using additional variables to store intermediate results and using a StringBuffer to concatenate strings. IMO if the code formatter fails to give the optimal result then you should adjust your code. Manual formatting can leave a lot of white spaces (especially at the end of lines) which the code formatter can easily fix. Thanks, Marius > > All I'm asking is that people stop blindly reformatting other people's > code. This means that you're allowed to use automatic reformatting but > you must check thereafter that no weirdies were introduced and > manually fix introduced "errors". > > To see the kind of "errors" I'm talking about see my specific comments > to thomas' commit. > > Thanks > -Vincent > >> Happy coding, >> Anca >> >>> The way I do commits is: >>> >>> - change the code >>> - check what would the commit look like (using git diff) >>> - if I detect something wrong in the codestyle, I selectively accept >>> changes in the commit, using git add --interactive, which allows me >>> to >>> choose which files to commit, and even inside a file, which changes >>> - final check on the prepared commit using git diff --cached >>> >>> This process takes a while, but it ensures high quality commits. > _______________________________________________ > devs mailing list > devs@xwiki.org > http://lists.xwiki.org/mailman/listinfo/devs
7:06 a.m.
New subject: [xwiki-devs] [xwiki-notifications] r21910 - in platform/core/trunk: xwiki-cache/xwiki-cache-tests/src/main/java/org/xwiki/cache/tests xwiki-component/xwiki-component-api/src/main/java/org/xwiki/component/manager xwiki-component/xwiki-component-default xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/embed xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/internal xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/manager xwiki-compo
On Jul 14, 2009, at 8:02 AM, Marius Dumitru Florea wrote: > > > Vincent Massol wrote: >> On Jul 13, 2009, at 11:25 PM, Anca Paula Luca wrote: >> >>> Sergiu Dumitriu wrote: >>>> Vincent Massol wrote: >>>>> Hi Thomas, >>>>> >>>>> On Jul 13, 2009, at 3:20 PM, tmortagne (SVN) wrote: >>>>> >>>>>> Author: tmortagne >>>>>> Date: 2009-07-13 15:20:03 +0200 (Mon, 13 Jul 2009) >>>>>> New Revision: 21910 >>>>>> >>>>> General comments: >>>>> - Would be great if you could separate code reformatting from code >>>>> changes. It makes it hard to read (I didn't read the commit fully >>>>> as a >>>>> consequence). >>>> +10 >>>> >>>>> - The code style has broken my styles in lots of places (see below >>>>> for >>>>> some comments, I haven't commented every single place it broke >>>>> voluntary formatting). >>>>> >>>>> I'd like to vote for not applying code style blindly in the >>>>> future. It >>>>> breaks styles in lots of places and I hate it when I spend a good >>>>> amount of my time to align code properly and it breaks it for >>>>> producing suboptimal styling... >>>> There are two things to balance here: >>>> - forgetting to format the code manually >>>> - suboptimal formatting done automatically >>>> >>>> I for one prefer to have more codestyle-compliant code than less, >>>> and >>>> automatic application does this. >>> I agree with Sergiu here, maybe, if automatic is suboptimal, we need >>> to revisit >>> our codestyle files. It's quite difficult to only format pieces of a >>> file (one's >>> changes), and it often results in forgetting to do it. >> >> codestyle files are always suboptimal since editors don't support all >> possible code styles. For ex the eclipse one is wrong in several >> instances (the IDEA one was better since IDEA supported more options >> in the past). >> >> In any case you cannot get perfect code styles with any IDE. (This is >> btw why I always format manually as I type). > > Yes, but you can manually split a long line in multiple lines by using > additional variables to store intermediate results and using a > StringBuffer to concatenate strings. IMO if the code formatter fails > to > give the optimal result then you should adjust your code. I don't agree with this in lots of case and especially in the case at hand for tests. BTW the compiler optimizes the concatenation of strings automatically and performance wise it's faster to do it with concatenation than with String Buffer. The case when it's better to use String buffer is when there are variables. > Manual formatting can leave a lot of white spaces (especially at the > end > of lines) which the code formatter can easily fix. This should be checked with checkstyle or some tool like this to ensure we don't have this. this is the only way to ensure we don't have spaces at end of lines. Formatting in IDE is only a best effort. Thanks -Vincent >> All I'm asking is that people stop blindly reformatting other >> people's >> code. This means that you're allowed to use automatic reformatting >> but >> you must check thereafter that no weirdies were introduced and >> manually fix introduced "errors". >> >> To see the kind of "errors" I'm talking about see my specific >> comments >> to thomas' commit. >> >> Thanks >> -Vincent >> >>> Happy coding, >>> Anca >>> >>>> The way I do commits is: >>>> >>>> - change the code >>>> - check what would the commit look like (using git diff) >>>> - if I detect something wrong in the codestyle, I selectively >>>> accept >>>> changes in the commit, using git add --interactive, which allows me >>>> to >>>> choose which files to commit, and even inside a file, which changes >>>> - final check on the prepared commit using git diff --cached >>>> >>>> This process takes a while, but it ensures high quality commits.
6:11 a.m.
New subject: [xwiki-devs] [xwiki-notifications] r21910 - in platform/core/trunk: xwiki-cache/xwiki-cache-tests/src/main/java/org/xwiki/cache/tests xwiki-component/xwiki-component-api/src/main/java/org/xwiki/component/manager xwiki-component/xwiki-component-default xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/embed xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/internal xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/manager xwiki-compo
Vincent Massol wrote:
Hi Thomas,
On Jul 13, 2009, at 3:20 PM, tmortagne (SVN) wrote:
Author: tmortagne Date: 2009-07-13 15:20:03 +0200 (Mon, 13 Jul 2009) New Revision: 21910
[snip]
public void testIncludeMacroWithNewContext() throws Exception { - String expected = "beginDocument\n" - + "beginMacroMarkerStandalone [velocity] [] [$myvar]\n" - + "beginParagraph\n" - + "onWord [hello]\n" - + "endParagraph\n" - + "endMacroMarkerStandalone [velocity] [] [$myvar]\n" - + "endDocument"; + String expected = + "beginDocument\n" + "beginMacroMarkerStandalone [velocity] [] [$myvar]\n" + "beginParagraph\n" + + "onWord [hello]\n" + "endParagraph\n" + "endMacroMarkerStandalone [velocity] [] [$myvar]\n" + + "endDocument";
I much prefer my version. Can you please rollback? Again an example why applying code style automatically sucks :)
Why don't you use a StringBuffer?
// Since it's not in the same context, we verify that a Velocity variable set in the including page is not // seen in the included page. @@ -88,21 +91,19 @@
// Create a Macro transformation context with the Macro transformation object defined so that the include // macro can transform included page which is using a new context. - MacroTransformation macroTransformation = + MacroTransformation macroTransformation = (MacroTransformation) getComponentManager().lookup(Transformation.class, "macro"); MacroTransformationContext context = new MacroTransformationContext(); context.setMacroTransformation(macroTransformation); - + List<Block> blocks = macro.execute(parameters, null, context);
assertBlocks(expected, blocks); } - + public void testIncludeMacroWithCurrentContext() throws Exception { - String expected = "beginDocument\n" - + "onMacroStandalone [someMacro] []\n" - + "endDocument"; + String expected = "beginDocument\n" + "onMacroStandalone [someMacro] []\n" + "endDocument";
again
again
IncludeMacro macro = (IncludeMacro) getComponentManager().lookup(Macro.class, "include");
mockDocumentAccessBridge .expects(once()).method("isDocumentViewable").will(returnValue(true)); @@ -119,7 +120,7 @@
assertBlocks(expected, blocks); } - + public void testIncludeMacroWithNoDocumentSpecified() throws Exception { IncludeMacro macro = (IncludeMacro) getComponentManager().lookup(Macro.class, "include"); @@ -129,8 +130,8 @@ macro.execute(parameters, null, new MacroTransformationContext()); fail("An exception should have been thrown"); } catch (MacroExecutionException expected) { - assertEquals("You must specify a 'document' parameter pointing to the document to include.", - expected.getMessage()); + assertEquals("You must specify a 'document' parameter pointing to the document to include.", expected + .getMessage());
again a brilliant example why it sucks... :(
Can you store the long string in a separate variable (maybe a StringBuffer if it's really long)? Thanks, Marius
7:13 a.m.
New subject: [xwiki-devs] [xwiki-notifications] r21910 - in platform/core/trunk: xwiki-cache/xwiki-cache-tests/src/main/java/org/xwiki/cache/tests xwiki-component/xwiki-component-api/src/main/java/org/xwiki/component/manager xwiki-component/xwiki-component-default xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/embed xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/internal xwiki-component/xwiki-component-default/src/main/java/org/xwiki/component/manager xwiki-compo
On Jul 14, 2009, at 8:11 AM, Marius Dumitru Florea wrote:
Vincent Massol wrote:
Hi Thomas,
On Jul 13, 2009, at 3:20 PM, tmortagne (SVN) wrote:
Author: tmortagne Date: 2009-07-13 15:20:03 +0200 (Mon, 13 Jul 2009) New Revision: 21910
[snip]
public void testIncludeMacroWithNewContext() throws Exception { - String expected = "beginDocument\n" - + "beginMacroMarkerStandalone [velocity] [] [$myvar]\n" - + "beginParagraph\n" - + "onWord [hello]\n" - + "endParagraph\n" - + "endMacroMarkerStandalone [velocity] [] [$myvar]\n" - + "endDocument"; + String expected = + "beginDocument\n" + "beginMacroMarkerStandalone [velocity] [] [$myvar]\n" + "beginParagraph\n" + + "onWord [hello]\n" + "endParagraph\n" + "endMacroMarkerStandalone [velocity] [] [$myvar]\n" + + "endDocument";
I much prefer my version. Can you please rollback? Again an example why applying code style automatically sucks :)
Why don't you use a StringBuffer?
It's more clear this way (and btw performance wise it's better - see my other mail in response to you). [snip]
IncludeMacro macro = (IncludeMacro) getComponentManager().lookup(Macro.class, "include");
mockDocumentAccessBridge .expects (once()).method("isDocumentViewable").will(returnValue(true)); @@ -119,7 +120,7 @@
assertBlocks(expected, blocks); } - + public void testIncludeMacroWithNoDocumentSpecified() throws Exception { IncludeMacro macro = (IncludeMacro) getComponentManager().lookup(Macro.class, "include"); @@ -129,8 +130,8 @@ macro.execute(parameters, null, new MacroTransformationContext()); fail("An exception should have been thrown"); } catch (MacroExecutionException expected) { - assertEquals("You must specify a 'document' parameter pointing to the document to include.", - expected.getMessage()); + assertEquals("You must specify a 'document' parameter pointing to the document to include.", expected + .getMessage());
again a brilliant example why it sucks... :(
Can you store the long string in a separate variable (maybe a StringBuffer if it's really long)?
Marius, I see where you want to go but this will never work. You can't modify code so that automatic formatting will always be perfect. If we want to automate formatting (like with a SVN hook to automatically apply code style - in a first commit for ex) then we'll have to accept suboptimal result. The balance of course is vs wrongly formatted code which is probably more the norm in our code in general. I'm just whinning because I'm very strict with my code style and I directly format it in a certain way that I find the best and the IDE formatting removes my carefully crafted styles... ;) If we could improve the IDE styles it could be ok. But I don't think Eclipse for ex support things like: - put braces on next line for if/for/etc only if the line is longer than 120 chars. We could also decide to remove this rule we had voted and (which I personally like - my preference has always been braces on next line everywhere for increased readability) - don't split on "." (as in the example above) - don't split for javadoc @see (to be verified this is invalid - I seem to recall it is) Thanks -Vincent
6254
Age (days ago)
6255
Last active (days ago)
7 comments
4 participants
participants (4)
-
Anca Paula Luca -
Marius Dumitru Florea -
Sergiu Dumitriu -
Vincent Massol