JBossDeveloper

we also need to add support for adding classes to the classpool with a byte[] and the posibility to get a byte[] representation of a MutableClassInfo instance.

adding classes to the classpool could be solved with a utilclass or via some sort of ClassPool interface, the way this is done in javassist:

ByteArrayClassPath cp = new ByteArrayClassPath(className, classfileBuffer);
pool.insertClassPath(cp);

public interface MutableClassInfo extends ClassInfo
{
 ...
 /**
 * Converts the class to a Class file.
 * After this method is called, no modifications to the class is allowed.
 *
 * @return
 */
 byte[] toByteCode();
}

Why do all these MutableX take MutableY as parameters?
e.g.

public interface MutableMethodInfo extends MethodInfo
{
 void setReturnType(MutableClassInfo returnType);

"stale.pedersen@jboss.org" wrote:

The usage of the Class parameter needs to be removed, internal methods using it will just use the CtClass object.

"stale.pedersen@jboss.org" wrote:

The question is the method getType(), can we just remove it altogether? -its been marked as deprecated.

"stale.pedersen@jboss.org" wrote:

Is there a reason why the JMI constructor is public? if not it should be set to package protected.

"stale.pedersen@jboss.org" wrote:

We need functionality to get the method signature from a method. This could be implemented in a utilclass, but i would rather see it added to the MethodInfo interface.

"stale.pedersen@jboss.org" wrote:

We might also need a method like ClassInfo.getClassLoader(), but i guess Kabir/Adrian know more about that.

"stale.pedersen@jboss.org" wrote:

Javassist have a lot of functionality to create new methods/constructors/etc that i dont think we need to support directly in MCI, but rather create some util classes to do that. Im thinking of classes/methods like: CtNewMethod.copy/abstractMethod/getter/setter/delegator/etc.

"alesj" wrote:
Why do all these MutableX take MutableY as parameters?
e.g.

public interface MutableMethodInfo extends MethodInfo
{
 void setReturnType(MutableClassInfo returnType);

Why would you force a MutableY as a return type?

"alesj" wrote:

"stale.pedersen@jboss.org" wrote:

The usage of the Class parameter needs to be removed, internal methods using it will just use the CtClass object.

How does this effect the (weak type) cache?

"stale.pedersen@jboss.org" wrote:

The question is the method getType(), can we just remove it altogether? -its been marked as deprecated.

You can, but then you have to fix all MC code. :-)
Unfortunately we have quite a few places where we use it.
MDR, Kernel, ...

"alesj" wrote:

"stale.pedersen@jboss.org" wrote:

We need functionality to get the method signature from a method. This could be implemented in a utilclass, but i would rather see it added to the MethodInfo interface.

Signature API is part of MDR sub-project.

"stale.pedersen@jboss.org" wrote:

mostly because then we are sure that we will get a CtClass instance within the parameter. there is also a String parameter if you would prefer to use that. it makes little sense to have a ClassInfo param since then we would just do the same work internally as with the String param.

"stale.pedersen@jboss.org" wrote:

its another signature, this is something similar to CtBehaviour.getSignature(). imo it belongs in MMI.

"stale.pedersen@jboss.org" wrote:

is it ok if i start commit to jboss-reflect? i can exclude the new files in the pom if needed.

"alesj" wrote:
I still don't see why it has to be a MutableCI.

"alesj" wrote:

Why isn't the one in MDR good enough?
Afaik AOP already uses MDR.

btw: there is a Javassist kind of Signature:
- http://anonsvn.jboss.org/repos/jbossas/projects/jboss-mdr/trunk/src/main/java/org/jboss/metadata/spi/signature/javassist/
I already use it in annotation scanning.

"kabir.khan@jboss.com" wrote:

"alesj" wrote:

How does this effect the (weak type) cache?

That is an implementation detail that needs to be reworked internally. Having Class as the key for this won't work since we don't want the class to be loaded. Internally the weak type cache stores class names in a map of cl, so this methods can work

TypeInfoFactory.getTypeInfo(String name, ClassLoader cl)

The use of WeakClassCache by Javassist will need revisiting so that it does not try to load the class every time we try to look something up since that is not needed by the underlying implementation.

"kabir.khan@jboss.com" wrote:

Create a wiki page with the work in progress?

regarding the changes to WeakClassCache ive only created a simple compare method to make sure that the cached object actually is correct.
take a look at the commit above for the implementation details.

- suggestions on better a solution to the WeakClassCache problem is wanted if we still want to make JavassistTypeInfo mutable compliant. if we end up creating another implementation we can do something different.

- yes, i know we need to change the caching of the getDeclaredMethods/Constructors/Fields for the mutable implementation, but im waiting with that untill we decide on what to do with JTI.

- or we can just get the classrepresentation from the ctclass like:

public Class<? extends Object> getType()
{
 if(clazz == null)
 clazz = ctClass.toClass(); //catch some exception
 return clazz;
}

im just wondering a bit about the usage of SignatureKey in JavassistMethodInfo. it doesnt seem to add any vaule that i can see. first of all its only used in the method JavassistMethodInfo.getName() where the name can be fetched from the CtMethod object instead.
with a mutable methodinfo it lack even more meaning since the method parameters and name dont need to be defined when creating the method.

i can understand the usage of SignatureKey in JavassistTypeInfo as it is used as a key for the maps that hold methods, fields, and constructors.
- even though a better solution would probably to use the signature of the method/field/constructor as the map key instead of SignatureKey (atleast it would be more efficient).

please explain the reason of having the SignatureKey in JavassistMethodInfo, and if there isnt a good reason ill remove it.

"stale.pedersen@jboss.org" wrote:

- even though a better solution would probably to use the signature of the method/field/constructor as the map key instead of SignatureKey (atleast it would be more efficient).

"stale.pedersen@jboss.org" wrote:

please explain the reason of having the SignatureKey in JavassistMethodInfo, and if there isnt a good reason ill remove it.

"alesj" wrote:

What does javassist return as CtBehavior::getSignature?
How does it look like?
I guess you get the same string even for diff instances of the same method/field/constructor?

"stale.pedersen@jboss.org" wrote:

"alesj" wrote:

What does javassist return as CtBehavior::getSignature?
How does it look like?
I guess you get the same string even for diff instances of the same method/field/constructor?

yes, using the method signature will create duplicate keys if methods got the same parameter signature. but using CtBehaviour.getLongName() (which add the class and methodname to the signature) we will always be sure that we have a unique key. eg for a method like: Pojo.foo(String), CtBehaviour.getLongName() would return Pojo.foo(java.lang.String).

"adrian@jboss.org" wrote:

There was a previous discussion about adding class name to the signature.
We ended up doing a halfway solution with the DeclaredMethodSignature
since we didn't have time to test changing the existing signatures.

im writing a few tests for the mutable implementation and there are part of the existing api that i feel are a bit cumberstone. example:

MutableClassInfo mci = new JavassistTypeInfoFactoryImpl().getMutable("Pojo", null);
MutableMethodInfo bar = mci.getDeclaredMethod("bar", new TypeInfo[] {(TypeInfo) new JavassistTypeInfoFactoryImpl().get("java.lang.String", Thread.currentThread().getContextClassLoader()) });

public MethodInfo getDeclaredMethod(String methodName, String[] methodArgs);