For now I provided a org.xwiki.component.util.DefaultParameterizedType which is implemented like the one in the oracle JVM and which is what I seen other libraries that need custom ParameterizedType doing. Works pretty well so far with both orache JVM and OpenJDK. At worst if someone report an issue with another JVM I have a backup plan which is to change any ParameterizedType provided to EmbeddableComponentManager in our DefaultParameterizedType but I would prefer to avoid that if not absolutely necessary.
Also note that the issue is only when you lookup a component, you can't have this kind of issue with injection. We don't have that much ParameterizedType, only reference resolver/serializer plus the Provider so it's not a very commons task either. On Sun, Mar 11, 2012 at 9:05 PM, Andreas Jonsson <[email protected]> wrote: > Hi, > > We must consider how we define the equality of RoleHints. Different > implementations of the interface ParameterizedType do not need to > implement equals and hashValue consistently. For instance, this lookup > doesn't work at the moment: Actually they do need to implement equals the same way since that's part of the specs but there is no spec for hashCode yes. > > Utils.getComponent(new ParameterizedType() { > @Override > public Type [] getActualTypeArguments() { > Type [] args = { > String.class > }; > return args; > } > > @Override > public Type getOwnerType() { > return null; > } > > @Override > public Type getRawType() { > return DocumentReferenceResolver.class; > } > }); > You don't need to do that, DocumentReferenceResolver provide to statics Type to make easier to lookup it. Utils.getComponent(DocumentReferenceResolver.TYPE_STRING) > At the same time, there is the backwards compliancy issue. This might be > ok for RoleHint equality: > > /** > * Defines the equality of types. > * > * @param t1 operand for comparison. > * @param t2 operand for comparison. > * @return {\code true} if the roles should be considered equal. > */ > private boolean typeEquals(Type t1, Type t2) { > > if (t1 instanceof ParameterizedType && t2 instanceof > ParameterizedType) { > ParameterizedType pt1 = (ParameterizedType) t1; > ParameterizedType pt2 = (ParameterizedType) t2; > > Type [] args1 = pt1.getActualTypeArguments(); > Type [] args2 = pt2.getActualTypeArguments(); > > if (args1.length != args2.length) { > return false; > } > > for (int i = 0; i < args1.length; i++) { > if (!typeEquals(args1[i], args2[i])) { > return false; > } > } > } else if ((t1 instanceof Class && !(t2 instanceof Class)) > || (t2 instanceof Class && !(t1 instanceof Class))) { > /* > * For compliancy with legacy code. > * > * Looking up components using raw classes only > * matches components registered as raw classes, and a > * component registered as raw class can only be > * looked up using a raw class. > */ > return false; > } > > Class c1 = ReflectionUtils.getTypeClass(t1); > Class c2 = ReflectionUtils.getTypeClass(t2); > > return c1.equals(c2); > } > > @Override > @SuppressWarnings("unchecked") > public boolean equals(Object obj) > { > if (this == obj) { > return true; > } > > if ((obj == null) || (obj.getClass() != this.getClass())) { > return false; > } > > RoleHint<T> rolehint = (RoleHint<T>) obj; > > return typeEquals(getRoleType(), rolehint.getRoleType()) > && (getHint() == rolehint.getHint() || (getHint() != null && > getHint().equals(rolehint.getHint()))); > } > > @Override > public int hashCode() > { > int hash = 8; > > hash = 31 * hash + (null == getRoleType() ? 0 : > ReflectionUtils.getTypeClass(getRoleType()).hashCode()); > hash = 31 * hash + (null == getHint() ? 0 : getHint().hashCode()); > > return hash; > } > > /Andreas > > > 2012-03-06 14:51, Thomas Mortagne skrev: >> It's committed. >> >> Would be nice to have comments on the new APIs (suggest better naming, >> etc...). It's a very important API and we really need to have >> something we are sure of before 4.0 final. >> >> On Mon, Mar 5, 2012 at 6:53 PM, Thomas Mortagne >> <[email protected]> wrote: >>> Hi guys, >>> >>> I'm almost done but there is one little detail left: for easier >>> retro-compatibility I would like to introduce a new @Role annotation >>> and deprecate the @ComponentRole annotation. The idea is that Role >>> will "officially" take into account generic parameters while >>> ComponentRole does not. That way no existing component role behavior >>> will be broken or require crappy retro-compatibility like registering >>> automatically both generic and non generic roles (what I started to do >>> at first). >>> >>> On Thu, Mar 1, 2012 at 6:20 PM, Thomas Mortagne >>> <[email protected]> wrote: >>>> Hi devs, >>>> >>>> I recently modified component manager internals and changed the role >>>> used as key in the map from Class to Type so that it properly make a >>>> difference between Provider<Toto> and Provider<Titi> without the need >>>> to generate a special hint. >>>> >>>> For the one not aware of what Type is lets say it's what Class is >>>> extending. Among other things Type is also extended by >>>> ParameterizedType which is interesting here because it basically >>>> contain a Class with it's generic parameters (it also indicate if the >>>> type in included in another one but we don't care in our case ;)). >>>> >>>> To summarize: >>>> * Provider is a Class >>>> * Provider<String> is ParameterizedType >>>> and both are Type >>>> >>>> That said I would like to change the whole ComponentManager and >>>> related descriptors APIs to use Type as role instead of Class. >>>> >>>> = What it bring >>>> >>>> What it means is that it makes possible to cleanly represent a >>>> Provider in which the generic type is meaningful right in the >>>> ComponentDescriptor (right now the generic type is extract from the >>>> implementation class when the provider is registered which is pretty >>>> crappy) and simplify a lot the current implementation. >>>> >>>> But it also mean we could use this new "feature" for other components. >>>> For example right now we have things like: >>>> >>>> @Component >>>> public class DefaultStringDocumentReferenceResolver implements >>>> DocumentReferenceResolver<String> >>>> >>>> and >>>> >>>> @Component >>>> @Named("default/reference") >>>> public class DefaultReferenceDocumentReferenceResolver implements >>>> DocumentReferenceResolver<EntityReference> >>>> >>>> and injected by >>>> >>>> @Inject >>>> DocumentReferenceResolver<String> resolver; >>>> >>>> and >>>> >>>> @Inject >>>> @Named("default/reference") >>>> DocumentReferenceResolver<EntityReference> resolver; >>>> >>>> Having Type used as role means that we could have: >>>> >>>> @Inject >>>> DocumentReferenceResolver<String> resolver; >>>> >>>> and >>>> >>>> @Inject >>>> DocumentReferenceResolver<EntityReference> resolver; >>>> >>>> = What it breaks >>>> >>>> So what's the catch ? >>>> >>>> Basically it would break any code that currently programmatically >>>> register or unregister Provider since It would remove the current hack >>>> to discover the Provider parameter (assuming that the >>>> ComponentDescriptor now provide the complete role). But that's OK IMO >>>> given that Provider support is very recent and I doubt it's a common >>>> use case. >>>> >>>> We would obviously keep and deprecate all the existing APIs based on >>>> Class which are pretty easy to implement since Class extends Type. >>>> Even if removing them would not cause any issue when you build it's >>>> not binary compatible since it's not the same method signature. >>>> >>>> So WDYT ? >>>> >>>> Here is my +1 and since we are starting a new major version it sounds >>>> a good occasion so introduce such change. >>>> >>>> -- >>>> Thomas Mortagne >>> >>> >>> >>> -- >>> Thomas Mortagne >> >> >> > > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs -- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
