[xwiki-devs] [PROPOSAL] Change component role from Class to Type
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
+1 Best regards, Andreas 2012-03-01 18:20, Thomas Mortagne skrev:
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.
On Thu, Mar 1, 2012 at 18:20, 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.
+1, Sounds good and it is effectively the right time to do so. (by the way, a very clear explanation of the issue, great mail, pleasant to read)
-- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
On Mar 1, 2012, at 6:20 PM, Thomas Mortagne 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.
+1 too So we could summarize all this in JIRA by creating an issue that says "Add support for generics in Component injection". Thanks -Vincent
+1 Thanks, Marius On Thu, Mar 1, 2012 at 7: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 _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
+1 Thanks, Eduard On Fri, Mar 2, 2012 at 1:19 PM, Marius Dumitru Florea < [email protected]> wrote:
+1
Thanks, Marius
On Thu, Mar 1, 2012 at 7: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 _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
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
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
-- Thomas Mortagne
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: 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; } }); 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
Maybe we should also think a little bit more if 'Type' really is the right interface to use. Both Guice and CDI defines 'TypeLiteral' classes. Maybe we should do the same? Best Regards, /Andreas 2012-03-11 21:05, Andreas Jonsson skrev:
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:
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; } });
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
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
Hi Thomas/All, All looks good except CM.lookupComponent() which Ï don't like a lot for the following reasons: * It's a bit long. It's longer than before and it would be nicer if it were as short as before or even shorter ;) * It's not symmetrical with other lookups like lookupList and lookupMap, which should theoretically be lookupComponentList() and lookupComponentMap() which are even longer Thus I'd prefer that the new API for lookup be named differently. I propose to use "resolve": CM.resolve(Type) CM.resolveList(Type) CM.resolveMap(Type) Alternative: CM.getInstance(Type) CM.getInstanceList(Type) CM.getInstanceMap(Type) or even the short: CM.get(Type) CM.getList(Type) CM.getMap(Type) For information Guice uses getInstance(), Picocontainer uses getComponentInstance(). WDYT? My preference goes to getInsta nce(): * users are tempted by default to start typing "get" to get an instance and then they'll see getInstance in autocompletion and choose it. * it's the same as Guice Thanks -Vincent On Mar 6, 2012, at 2:51 PM, Thomas Mortagne wrote:
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
-- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
On Wed, Apr 4, 2012 at 20:00, Vincent Massol <[email protected]> wrote:
Hi Thomas/All,
All looks good except CM.lookupComponent() which Ï don't like a lot for the following reasons: * It's a bit long. It's longer than before and it would be nicer if it were as short as before or even shorter ;) * It's not symmetrical with other lookups like lookupList and lookupMap, which should theoretically be lookupComponentList() and lookupComponentMap() which are even longer
Thus I'd prefer that the new API for lookup be named differently.
I propose to use "resolve":
CM.resolve(Type) CM.resolveList(Type) CM.resolveMap(Type)
-0
Alternative:
CM.getInstance(Type) CM.getInstanceList(Type) CM.getInstanceMap(Type)
+1
or even the short:
CM.get(Type) CM.getList(Type) CM.getMap(Type)
+0
For information Guice uses getInstance(), Picocontainer uses getComponentInstance().
WDYT?
My preference goes to getInsta nce(): * users are tempted by default to start typing "get" to get an instance and then they'll see getInstance in autocompletion and choose it. * it's the same as Guice
Thanks -Vincent
On Mar 6, 2012, at 2:51 PM, Thomas Mortagne wrote:
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
-- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
On 04/04/2012 02:00 PM, Vincent Massol wrote:
Hi Thomas/All,
All looks good except CM.lookupComponent() which Ï don't like a lot for the following reasons: * It's a bit long. It's longer than before and it would be nicer if it were as short as before or even shorter ;) * It's not symmetrical with other lookups like lookupList and lookupMap, which should theoretically be lookupComponentList() and lookupComponentMap() which are even longer
Thus I'd prefer that the new API for lookup be named differently.
I propose to use "resolve":
CM.resolve(Type) CM.resolveList(Type) CM.resolveMap(Type)
Alternative:
CM.getInstance(Type) CM.getInstanceList(Type) CM.getInstanceMap(Type)
or even the short:
CM.get(Type) CM.getList(Type) CM.getMap(Type)
For information Guice uses getInstance(), Picocontainer uses getComponentInstance().
WDYT?
My preference goes to getInstance(): * users are tempted by default to start typing "get" to get an instance and then they'll see getInstance in autocompletion and choose it. * it's the same as Guice
Same, +1 for getInstance. -- Sergiu Dumitriu http://purl.org/net/sergiu/
On Wed, Apr 4, 2012 at 8:00 PM, Vincent Massol <[email protected]> wrote:
Hi Thomas/All,
All looks good except CM.lookupComponent() which Ï don't like a lot for the following reasons: * It's a bit long. It's longer than before and it would be nicer if it were as short as before or even shorter ;) * It's not symmetrical with other lookups like lookupList and lookupMap, which should theoretically be lookupComponentList() and lookupComponentMap() which are even longer
Thus I'd prefer that the new API for lookup be named differently.
I propose to use "resolve":
CM.resolve(Type) CM.resolveList(Type) CM.resolveMap(Type)
Alternative:
CM.getInstance(Type) CM.getInstanceList(Type) CM.getInstanceMap(Type)
or even the short:
CM.get(Type) CM.getList(Type) CM.getMap(Type)
For information Guice uses getInstance(), Picocontainer uses getComponentInstance().
WDYT?
"get" sounds a bit lightweight to be for something supposed to also initialize what you ask but if Guice use that I guess it means it's OK. I'm OK with any of theses suggestions so I will apply the voted one.
My preference goes to getInsta nce(): * users are tempted by default to start typing "get" to get an instance and then they'll see getInstance in autocompletion and choose it. * it's the same as Guice
"getInstance" is longer than "lookup" ;)
Thanks -Vincent
On Mar 6, 2012, at 2:51 PM, Thomas Mortagne wrote:
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
-- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
On Apr 5, 2012, at 8:35 AM, Thomas Mortagne wrote:
On Wed, Apr 4, 2012 at 8:00 PM, Vincent Massol <[email protected]> wrote:
Hi Thomas/All,
All looks good except CM.lookupComponent() which Ï don't like a lot for the following reasons: * It's a bit long. It's longer than before and it would be nicer if it were as short as before or even shorter ;) * It's not symmetrical with other lookups like lookupList and lookupMap, which should theoretically be lookupComponentList() and lookupComponentMap() which are even longer
Thus I'd prefer that the new API for lookup be named differently.
I propose to use "resolve":
CM.resolve(Type) CM.resolveList(Type) CM.resolveMap(Type)
Alternative:
CM.getInstance(Type) CM.getInstanceList(Type) CM.getInstanceMap(Type)
or even the short:
CM.get(Type) CM.getList(Type) CM.getMap(Type)
For information Guice uses getInstance(), Picocontainer uses getComponentInstance().
WDYT?
"get" sounds a bit lightweight to be for something supposed to also initialize what you ask but if Guice use that I guess it means it's OK. I'm OK with any of theses suggestions so I will apply the voted one.
My preference goes to getInsta nce(): * users are tempted by default to start typing "get" to get an instance and then they'll see getInstance in autocompletion and choose it. * it's the same as Guice
"getInstance" is longer than "lookup" ;)
Yes…. But I couldn't find a short one that would get everyone's agreement ;) It's still shorter than lookupComponent. There's also getComponent, getComponentList and getComponentMap which are not that bad but getInstance() seems to win everyone's opinnion so far and I think it's good that it's the same API than Guice. Thanks -Vincent
Thanks -Vincent
On Mar 6, 2012, at 2:51 PM, Thomas Mortagne wrote:
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
-- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________ 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
On 03/01/2012 12:20 PM, Thomas Mortagne 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.
+1. -- Sergiu Dumitriu http://purl.org/net/sergiu/
+1 On Mon, Mar 5, 2012 at 19:28, Sergiu Dumitriu <[email protected]> wrote:
On 03/01/2012 12:20 PM, Thomas Mortagne 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 DefaultStringDocumentReference**Resolver implements DocumentReferenceResolver<**String>
and
@Component @Named("default/reference") public class DefaultReferenceDocumentRefere**nceResolver 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.
+1.
-- Sergiu Dumitriu http://purl.org/net/sergiu/
______________________________**_________________ devs mailing list [email protected] http://lists.xwiki.org/**mailman/listinfo/devs<http://lists.xwiki.org/mailman/listinfo/devs>
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
participants (7)
-
Andreas Jonsson -
Denis Gervalle -
Eduard Moraru -
Marius Dumitru Florea -
Sergiu Dumitriu -
Thomas Mortagne -
Vincent Massol