Juha - seen the zip file yet?

[squirest]

Hey,

late last night I emailed you a link to a zip file.

I don't have access to my home email so I was wondering if you've seen the email and if you have any comments.

Trev

[juha]

I've got the zip but haven't had the time to look at the code (busy working)

-- Juha

[squirest]

Ok,
unless you've got any screaming objections I'm going to commit this. I've also got the modelinfo cache working.

I need to update some of the ModelMBean*Info classes because they're not creating default descriptors in constructors that require it (which causes tests to blow up).

Trev

[juha]

the attribute descriptor handling needs to go back to an interceptor. the modelmbean is the wrong place to do it (I already sent you an email about it, I implemented it in the modelmbean once already, and moved it into an interceptor for a reason).

The descriptor values in the spec is not a fixed set. If anyone wants to introduce a new BettyBoop descriptor that affects the attribute cache handling now they need to rewrite the whole modelmbean implementation. Instead of configuring just a new interceptor with the new functionality. The interceptors are there to bring new behaviour to a modelmbean and I do object you removing it... most descriptors manage behavioral properties and therefore most (if not all) modelmbean behavior that is controlled by descriptors should be implemented as an interceptor. The fact that the current descriptors get cloned left and right is not enough of a reason to remove them.

Please ask before you go an make design changes.

-- Juha

[squirest]

> Please ask before you go an make design changes.
>
> -- Juha

What exactly do you think my email and zip file were if not me "asking before making design changes"?

Now that I'm home and have seen your email I'll reply in there.

[juha]

Am trying to save your time... if you ask before you implement you have less code to change.

-- Juha

[squirest]

I took some time to digest the email and your other information.

> Am trying to save your time... if you ask before you
> implement you have less code to change.
>
> -- Juha

I understand what you are saying now. Unfortunately I saw some code which I thought was an implementation decision without realising it was an expression of "the way".

I don't agree with your position on implementing MMB functionality as interceptors but then again, I'm not a recognised expert on them.

Anyhow, perhaps you can "save my time" now. In CVS, the mx.interceptor package has an Invocation object which you have based your interceptors on and an MBeanInvocation which I have based things like the ReflectedMBeanDispatcher on.

Which invocation object do you want to use?

[davidjencks]

IMO it is ESSENTIAL to implement descriptor functionality as interceptors. For instance, I put a descriptor into the xmbean xml parsing that describes what the effect on the service lifecycle of changing an attribute is (leave mbean started, stop change and start, or make a new mbean). This is obviously optional functionality depending on for instance whether the mbean supports any service lifecycle operations. I can't see any reasonable way of implementing this behavior except as an interceptor.

[squirest]

David,

that's an interesting feature.

First off, I'd like to adjust your terminology a bit. You're not "implementing descriptor functionality" as interceptors are you? You're using the attribute descriptor to tell an interceptor (with its own functionality) how to behave right?

These interceptors are configurable aren't they? So in addition to setting descriptor fields your interceptor is going to respond to you have to install the interceptor in the chain right? I'm guessing that it'll be in an interceptors descriptor unless you guys have other magic in mind.

Now consider a notional "attribute value caching" interceptor as per the spec. On a setAttribute it will check to see if "setMethod" is defined. If so, according to the spec, it must delegate the call to that method. If not, it just sets the value in the descriptor.

Of course, being an interceptor it wouldn't delegate the call to the method, it would delegate the call to the next interceptor in the chain hoping the method does get called.

Have you thought what might happen if the "attribute value caching" interceptor was put in the chain before your "service lifecycle" interceptor? You might not notice the problem until somehow the "setMethod" field went missing from the descriptor.

These interceptors *are* configurable right?

Your example interceptor, while valid and definately a neat idea is by your own admission, "optional functionality".

The thing is, optional, configurable interceptors is the whole reason I pulled the attribute value caching interceptor into the XMBean. I was drawing a line saying "This is a ModelMbean. It is the representative of the ManagedResource. It is the invokable at the end of the interceptor chain which implements all non-optional behaviour. It can cache attributes and can delegate calls to operations. Operations map to methods in java objects".

Even with that line drawn and nothing intercepting between the cache and the methods, the interactions you can define in the descriptor can yield strange results, something discussed in another thread. You're a smart guy and even you got caught out by the difference between read/write and getMethod/setMethod.

Juha's complaint seems to be that if you had a new BettyBoop descriptor which affects attribute value caching you would need to rewrite the modelmbean. Sure, I can see a BettyBoop descriptor which has settings for how to map operations to stored procs in an oracle database because there is no java object being managed.

Yes, making that pluggable is a good feature. However, my opinion is that such a pluggable component is not an interceptor. You don't get to say where it goes in the chain because it is the invokable - the thing that defines the core of what a particular flavour of mangaged resource can be and is responsible for interacting directly with it.

So for the record, I *like* configurable interceptors. Just not for everything.

Trevor

[marc.fleury]

> So for the record, I *like* configurable
> interceptors. Just not for everything.

huh just skimming the conversation (it is in the hit box :) but I NEED BADLY the configurable stuff on the client interceptors...

how did you externalize it?

marcf

[davidjencks]

> David,
>
> that's an interesting feature.
>
> First off, I'd like to adjust your terminology a bit.
> You're not "implementing descriptor functionality"
> as interceptors are you? You're using the attribute
> descriptor to tell an interceptor (with its own
> functionality) how to behave right?
yes, that is more precise.
>
> These interceptors are configurable aren't they? So
> in addition to setting descriptor fields your
> interceptor is going to respond to you have to
> install the interceptor in the chain right? I'm
> guessing that it'll be in an interceptors descriptor
> unless you guys have other magic in mind.

I don't know what you have in mind for how to specify the interceptor chain for a model mbean, I was assuming you were implementing something. I can't write the interceptor until a more working xmbean is committed. The one I last saw in cvs didn't implement enough functionality to even test the descriptor parser.
>
> Now consider a notional "attribute value caching"
> interceptor as per the spec. On a setAttribute it
> will check to see if "setMethod" is defined. If so,
> according to the spec, it must delegate the call to
> that method. If not, it just sets the value in the
> descriptor.
>
> Of course, being an interceptor it wouldn't delegate
> the call to the method, it would delegate the call to
> the next interceptor in the chain hoping the method
> does get called.
>
> Have you thought what might happen if the "attribute
> value caching" interceptor was put in the chain
> before your "service lifecycle" interceptor? You
> might not notice the problem until somehow the
> "setMethod" field went missing from the descriptor.

I wouldn't mind ... I can't see any reason to cycle the mbean service state if no method gets called on it.
>
> These interceptors *are* configurable right?

I presume so, but I haven't seen what you are writing yet.
>
> Your example interceptor, while valid and definately
> a neat idea is by your own admission, "optional
> functionality".
>
> The thing is, optional, configurable interceptors is
> the whole reason I pulled the attribute value caching
> interceptor into the XMBean. I was drawing a line
> saying "This is a ModelMbean. It is the
> representative of the ManagedResource. It is the
> invokable at the end of the interceptor chain which
> implements all non-optional behaviour. It can cache
> attributes and can delegate calls to operations.
> Operations map to methods in java objects".

But... I might need some functionality that happens after some required functionality and before other required functionality. If you implement even the required functionality as interceptors, I might be able to break things more easily, but I can do stuff in the order I want, not artificially set by the implementation.
>
> Even with that line drawn and nothing intercepting
> between the cache and the methods, the interactions
> you can define in the descriptor can yield strange
> results, something discussed in another thread.
> You're a smart guy and even you got caught out by
> the difference between read/write and
> getMethod/setMethod.

True, but seeing the interceptor would have cleared that one up quickly.
>
> Juha's complaint seems to be that if you had a new
> BettyBoop descriptor which affects attribute value
> caching you would need to rewrite the modelmbean.
> Sure, I can see a BettyBoop descriptor which has
> settings for how to map operations to stored procs
> in an oracle database because there is no java
> object being managed.
>
> Yes, making that pluggable is a good feature.
> However, my opinion is that such a pluggable
> component is not an interceptor. You don't
> get to say where it goes in the chain because it is
> the invokable - the thing that defines the
> core of what a particular flavour of mangaged
> resource can be and is responsible for interacting
> directly with it.
>
> So for the record, I *like* configurable
> interceptors. Just not for everything.

Well, I like them for everything. I think it is simpler and more powerful. Maybe I should be worried about ease of breakage, but I'm not. How many people change the interceptor stacks in jboss, even though they are easy to configure?

david
>
> Trevor

[juha]

> In CVS,the mx.interceptor package has an Invocation object
> which you have based your interceptors on and an MBeanInvocation which I have based things like the
> ReflectedMBeanDispatcher on.
>
> Which invocation object do you want to use?

I don't care either way, whichever works (including the modelmbean invocation). I sure don't want two different invocation objects floating around.

-- Juha

[juha]

> Have you thought what might happen if the "attribute value caching" interceptor was put in the chain
> before your "service lifecycle" interceptor? You might not notice the problem until somehow the
> "setMethod" field went missing from the descriptor.

This is no different to JBoss interceptors for example. You can't arbitrarily throw around the interceptors that implement the EJB spec defined functionality and expect things to work as usual. So yes there is a functional dependency between some interceptors. This is not a problem though.


> Your example interceptor, while valid and definately a neat idea is by your own admission, "optional
> functionality".

so is attribute caching, persistence, security and export policies. They're all optional functionality. SetMethod and getMethod fields are also marked as optional descriptor fields, which would indicate that even attribute method mapping is optional (though not implementing it doesn't make much sense to me, maybe for completely transient resources it is not needed).

All of this belongs to the interceptors.

> The thing is, optional, configurable interceptors is the whole reason I pulled the attribute value caching
> interceptor into the XMBean. I was drawing a line saying "This is a ModelMbean. It is the
> representative of the ManagedResource. It is the invokable at the end of the interceptor chain which
> implements all non-optional behaviour. It can cache attributes and can delegate calls to operations.
> Operations map to methods in java objects".

You're drawing the line in the wrong place.

> Yes, making that pluggable is a good feature. However, my opinion is that such a pluggable
> component is not an interceptor. You don't get to say where it goes in the chain because it is
> the invokable

see the above, it doesn't matter. Interceptors is a single approach to configure and extend the modelmbean behavior. You're suggesting two different approaches, and that to me has no increased value. It is yet another step to increased complexity though.


-- Juha

[juha]

> I think it is simpler and more powerful.

That it is.

-- Juha

[squirest]

> I don't care either way, whichever works (including
> the modelmbean invocation). I sure don't want two
> different invocation objects floating around.
>
> -- Juha

Fine. I will use mx.interceptor.Invocation because there is likely to be fewer issues with that than with converting your interceptors to handle what I considered to be an "invocation". You will be able to leverage your existing scheme for interceptors. However, I will have to add a String[] getSignature() to return the signature array.

The capability.AbstractInvocationDispatcher will extend interceptor.Interceptor in order to speak interceptor.Invocation objects.

I'll then move server.OLDMBeanInvoker back to be server.MBeanInvoker.

I'll then remove the cruft I put in which doesn't fit your architecture (interceptor.MBeanInvocation, interceptor.MBeanInterceptor, interceptor.MBeanInvoker, interceptor.MBeanTarget).

Trevor