Metadata rewrite

[kabirkhan]

I take it that the new org.jboss.metadata stuff in the container module
http://www.jboss.com/index.html?module=bb&op=viewtopic&p=3952697#3952697
replaces the org.jboss.repository implementation for the AOP metadata?

[adrian.brock]

It will when I've integrated it (probably this weekend?)

This is my task, you've already provided "the path"
on where the metadata needs to be, I'm just changing the implementation.

[kabirkhan]

I don't think anything has been done on this yet? A few of my AOP-MC tasks depends on this. If you want I can take a look, but a few pointers would be appreciated :-)

[adrian.brock]

It is the org.jboss.metadata in the container project.

The three major parts that need doing are:
1) Creating the composite metadata tree from the key scopes
2) Mechansims to deploy metadata at different scopes
3) Adding constructor, method, parameter annotations to the api.

I've outlined (1) elsewhere, but it is not trivial because of the
"feedback" used to construct the tree, e.g. the deployment determines
the cluster.

(2) is really an issue for the deployers

(3) needs to define a "joinpoint notion" so you can for instance
say I want to override the annotations/metadata for this method/constructor
I didn't look at how you did this with the org.jboss.repository stuff

I stopped doing this before to work on the deployers but now I've
got to the stage where I'm going to get back to it with the deployment
metadata population/overriding.

[kabirkhan]

I had a look a while ago, and how the metadata tree hangs together isn't totally clear to me yet, but I will take a look unless you have made a start. I'll try to duplicate/replace the functionality that we already have for the old metadata as a start.

I think that the bean-level annotations should go in at CommonLevels.INSTANCE? My main question at the moment is what goes in above? Is this the stuff from the deployers, i.e. 2) above? What happens if the tree above is empty?

For 3) IIRC all that it was possible to annotate was at bean level or for properties, so for properties the name of the getter/setter is used at present.

[kabirkhan]

I have rewritten the AOPMetaDataContext and friends to use the new metadata, and so had to tweak the API. All tests pass, but I will look a bit more at how I am defining the tree since I am not sure if things like overriding will actually work how I set it up...

A few things:
I left the org.jboss.repository.spi.MetaDataContext interface as the parent for the AOPMetaDataContext, it should probably be moved to org.jboss.metadata., but there already is a context.MetaDataContext there which I am not sure what does.

I also created an intermediate interface org.jboss.kernel.spi.metadata.MutableMetaDataContext for the addition of the annotations from the metadata, since stuff like AnnotationMetaData is unknown to the kernel module.

I create the bean metadata at instance level, but am ignoring any parent scopes for now, so that needs to be handled.

For the joinpoint data, I now create a subscope of the instance scope at joinpoint level indexed by method hash

[adrian.brock]

Ok, I'll take a look at what you did when I have time.

You probably did it wrong, but that's ok. ;-)

Like I said on a different thread (in the POJO forum), the real solution is to
provide two different views of the metadata.

1) The particular level that is modifiable, e.g. the AOP instance annotations
e.g. (scope key) instance=SomeInstanceId
2) The tree structure that is used at runtime and is read only
instance=SomeInstanceId,class=com.acme.Blah,deployment=whatever.jar,etc.
this is made up on the particular levels defined in (1)

The repository needs to contain both views.

[adrian.brock]

I've committed the code for this refactoring

AOP-CHANGES

It required a change to the AOP advisor to map MetaDataContext -> MetaData
I left the old class still in the repository in case somebody does a partial
integration (don't want NoClassDefFoundError).

I hope I didn't break anything, the aop-mc-int tests weren't in a good shape
(e.g. all the weave tests were broken).

KNOWN TEST ISSUE (Metadata stack)

We'll actually I know I broke one thing, but that would be easily fixed if
we decide it isn't a useful feature.
Basically when I callout to a joinpoint I push the metadata onto the
org.jboss.metadata.spi.MetaDataStack
this is how I pass the MetaData to aop,
but it also means the constructor or lifecycle methods can peek
the metadata.

However, this seems to confuse AOP since the lazily loaded aspects
also seem to pick up this metadata.
For now I'm "masking" this stack in the AOPConstructorJoinpoint
but that means some of the new tests are failing because the constructor
expects to be able to peek the stack.

Maybe this isn't such a good idea and it should be a feature provided
optionally by an interceptor at runtime?

ANNOTATION SUPPORT

The new MetaData has support for all types of annotations
(field, constructor, method, method parameters - in fact whatever
you can think of as a "Signature"), but the MC xml still doesn't support
overrriding all these.

DEFINING SCOPES

The major piece of work that is left is to replace the
BasicKernelMetaDataRepository.initMetaData()
with something that knows how to properly construct scoped metadata contexts.
This is going to require some kind of "plugin' mechanism away from the
MC since this is likely to get very JBossAS specific, e.g.
clustering or transaction local scopes.

The only scopes currently supported are hardwired:
instance, class and JVM

REPOSITORY.JBOSS.COM

Finally, I put this in the repository as "snapshot-metadata" because
I don't want the JBossAS build picking this up until Kabir has had a chance
to verify it.

[kabirkhan]

I've committed the code for this refactoring
I hope I didn't break anything, the aop-mc-int tests weren't in a good shape
(e.g. all the weave tests were broken).

Yes following your change :-) http://jira.jboss.com/jira/browse/JBAOP-287 I've got these fixed for the JDK 5 tests, but will need to get this working with JDK 1.4 as well.

Finally, I put this in the repository as "snapshot-metadata" because
I don't want the JBossAS build picking this up until Kabir has had a chance
to verify it.

Is this snapshot taken from trunk? I need to create one for JDK 1.4 as well to get the aop tests passing

[adrian.brock]

Is this snapshot taken from trunk? I need to create one for JDK 1.4 as well to get the aop tests passing

Yes it is just trunk, I just wanted a different name for the snapshot in
repository.jboss.com to avoid the appserver picking it up (yet...).

[kabirkhan]

I've had a quick look and some of the aop-mc-int tests are failing. AFAICT we always have a MetaData passed in to AOP now, whereas before the presence of a MetaDataContext determined if we should create an instance proxy or not.

I have experimented with the following in AOPConstructorJoinPoint:

 ...
 MetaData metaData = MetaDataStack.peek();
 MetaDataStack.mask();
 try
 {

 MetaData md = getNonEmptyMetaData(metaData);
 ContainerCache cache = ContainerCache.initialise(manager, clazz, md);
 ...


 private MetaData getNonEmptyMetaData(MetaData metaData)
 {
 if (metaData == null)
 {
 return null;
 }

 if (metaData.getAnnotations() != MetaData.NO_ANNOTATIONS)
 {
 return metaData;
 }

 return null;
 }

However, I have so far found 2 issues with this.

1) In the case of property annotations, the call to getAnnotations() returns NO_ANNOTATIONS. Where/how do I get hold of the property annotations? MetaData.getComponentMetaData() currently needs the signature as an argument. I could of course iterate over all the methods, and obtain the component metadata's for each of them, but am not sure if this is "clean" enough.

-----
2) In the case of real annotations, e.g.
@Blah
class Test{}

@Blah is now part of the MetaData, and so I am wrongly creating an instance proxy.

-------
In other words I need a good way to determine the annotations, and to discover what level they come from. This in turn raises the question, what is the cutoff level for added metadata to cause an instance proxy to be created?

[adrian.brock]

I'm not sure I understand the second part, but I understand the first part.

The answer to the final question is that the cutoff for creating an instance proxy
is whether it has instance annotations (or metadata - objects).
Obviouslly if there are server annotations (like default transaction timeout)
they will be shared by all instances.

The object you are getting is the MetaData (read only and merged)
which is the object that you should use in AOP.

However, this does not tell you if there are instance annotations.
What you really need to do is the following (pseudo code):

// Get the instance retrieval for this context
KernelMetaDataRepository repository = kernel.getMetaDataRepository();
MetaDataRetrieval retrieval = repository.getMetaDataRetrieval(context);

if (retrieval.isEmpty() == false)
 // Need an instance proxy

NOTE: obviously the "isEmpty()" doesn't exist
and it would need to check for annotations, object metadata and
component metadata as well.

But, you don't have a reference to the kernel or the context in the code
where you need to do this. I'm not sure what is the best way to solve this?

I don't like the idea of reintroducing the MetaData stuff into the BeanInfo
even the getDependencies(MetaData) is ugly.

That is unless it was implemented properly with the "InstanceClassInfo"
really holding the instance annotations, but that would probably require
a lot of rework in AOP to use the ClassInfo instead of Class or CtClass?

[adrian.brock]

On the final point (InstanceClassInfo), although this would make the BeanInfo
opaque, you could just ask the InstanceClassInfo if it has instance annotations,
it is not a good solution anymore because the MetaData has been expanded
to include abritary objects (not just annotations) which don't fit well
with the ClassInfo notion.

[kabirkhan]

However, this does not tell you if there are instance annotations.
What you really need to do is the following (pseudo code):

// Get the instance retrieval for this context
KernelMetaDataRepository repository = kernel.getMetaDataRepository();
MetaDataRetrieval retrieval = repository.getMetaDataRetrieval(context);

if (retrieval.isEmpty() == false)
 // Need an instance proxy

I'm not really gettting this... I have found a way (not necessarily the best and final, but it works for my tests) to get hold of the context so I can get hold of the kernel. But for a bean annotated with "real" annotations as

@Test
public class AnnotatedChild extends Base
{
}

 Annotation[] ai = retrieval.retrieveAnnotations().getValue();
 Annotation[] ai2 = retrieval.retrieveLocalAnnotations().getValue();
 Annotation[] anns = metaData.getAnnotations();

The ai, ai2 and anns all contain the same annotations. I

[kabirkhan]

In fact

metaData.getAnnotations()

ends up in its retrieval.getAnnotations()