ModelMBeanInfo values returned by MMB instances

[squirest]

Hey,

The descriptor for MMBOpInfo allows you to set a targetObject. Should this be stripped out of the MMBInfo that's returned by getMBeanInfo()?

I can't find a reference in the spec and I haven't checked the RI (because my gut reaction is to strip it so if the RI leaves it in I would consider it a bug).

Opinions?

Trev

[juha]

Hmm, that's interesting. Neither targetObject or targetType is in the spec, only in the javadoc.

It looks like a workaround to support multiple targets in model mbean. The descriptor is not the right place to store the target objects reference though (descriptors travel to the client, serialization nightmare right there).

Hmm, ok...

I'm against stripping of descriptors based on principle. Everything else is returned to the client as-is, and since the targetObject & type is not part of the spec defined descriptor fields, I think its ok to assume they're optional. In other words, let's not use them to store object references at least (EJB Handles, IOR's might be ok, as we can store a reasonable string representation, I think). For ObjectReference type we need to use a string representation as well, something that maps into the actual targets in the ModelMBean implementation.

Ok, thinking thinking..

Yes, targetObject and type is needed in MMBOp for multitargets, don't strip them. The mapping of object refs to names must be done in setManagedResource() method when multitarget MMB is being setup. In the descriptor we will store the name, not the actual object ref.

-- Juha

[squirest]

> Hmm, that's interesting. Neither targetObject or
> targetType is in the spec, only in the javadoc.

Bzzzzt. Bottom of page 84 and bottom of page 86.

The only way they are "optional" is if we go for the clause "valid for the implementation" on page 86.

Trev

[adrian.brock]

Chapter 4: ModelMBeanOperationInfo Implementation
Bottom of page 81

This descriptor must have a name field which matches the name given by the
getName method of the corresponding metadata object. It must have a
descriptorType with the value “operation” and a role of “operation”,
“getter”, or “setter”. It may also contain the following defined fields:
displayName, class, targetObject, targetType, lastReturnedValue,
currencyTimeLimit, lastReturnedTimeStamp, visibility, and
presentationString. See “Operation Descriptor Fields” on page 97 for a detailed
description of each of these fields.

My italics, so it is optional.

Regards,
Adrian

[adrian.brock]

I didn't get as far as page 84, 86 :-)

I agree with Juha, don't accept object references.
I think you should throw a RuntimeOperationsException if
somebody tries it.

Regards.
Adrian

[juha]

>
> Bzzzzt. Bottom of page 84 and bottom of page 86.

ahh, I have completely missed that. I've taken the predefined descriptor fields on page 94 as a definitive list (its lacking description of both fields for OperationInfo).

> The only way they are "optional" is if we go for the
> clause "valid for the implementation" on page 86.

I think they are optional. They're really only needed for implementations that support multiple targets (the mandatory fields that must be supported are on page 94).Similarly MMBean implementations that do not support persistence may ignore persistencePolicy fields.

-- Juha

[squirest]

Okay,

I'm not going to argue for inclusion of *anything* that's optional. God knows I can rant on about "issues" that I have with MMB.

So targetXXX is not supported right?

Just for interests sake, Juha, when you spoke about "ObjectReference" were you thinking it was an ObjectName or were you thinking that it was a reference to a java object (my interpretation)?

Trev

[juha]

> So targetXXX is not supported right?

well, I think we're going to need it eventually (or some other field we can define) once we get into model mbeans with multiple target objects (however how multitargets are configured is still something that needs to be solved, as the ModelMBean API doesn't really directly support it, we need to hack together something to use with the setManagedResource() call).

However, I think you can ignore the targetXXX fields for now, no need to strip them, etc.

> Just for interests sake, Juha, when you spoke about
> "ObjectReference" were you thinking it was an
> ObjectName or were you thinking that it was a
> reference to a java object (my interpretation)?

Reference to a java object. Something with an ObjectName should probably be of type "MBean" and not "ObjectReference".

For multitargets we're going to need some mechanism to bring a string to object mapping to modelmbean so we can avoid the managed resource themselves being passed back on forth inside the descriptor. Something in style of

MultiTarget mt = new MultiTarget();
mt.add("target1", new MyTarget(), "ObjectReference");
mt.add("target2", new FooBar(), "ObjectReference");

Descriptor descOp1 = new DescriptorSupport();
Descriptor descOp2 = new DescriptorSupport();
descOp1.setFieldValue("targetObject", "target1");
descOp2.setFieldValue("targetObject", "target2");

mmBean.setManagedResource(mt, "MultiTarget");

and in the MMB implementation store the map of name to target object and then in invoke() implementation delegate the operation call to the correct method in the correct target object. No actual java references in the descriptors.

-- Juha

> Trev

[squirest]

> MultiTarget mt = new MultiTarget();
> mt.add("target1", new MyTarget(),
> , "ObjectReference");
> mt.add("target2", new FooBar(),
> , "ObjectReference");

This is good - it's how it should be in the spec.

Rather than introduce a new class (which would need to be in org.jboss) we could actually re-use the Descriptor class.

Descriptor d = new DescriptorSupport();
d.addTargetStuff();
MMB.setManagedResource(d, "targetDescriptor");

It's a minor issue but it might encourage the the spec authors to include it as part of the standard. Or am I just dreaming?

Trev

[squirest]

Ok, along similar lines, i.e. modifying MMBInfo.

In an MMBAttributeInfo descriptor you can specify a getMethod and setMethod so that in addition to the value caching the managed resource gets an invoke on the relevant method.

Each method must be defined as an operation (not sure if the method *has* to be marked as "gettter" or "setter").

This opens us to a problem already seen in the RI. Namely that attribute-specific operations are left exposed as plain operations (bypassing attribute cache). I beleive the spirit of the spec is that attributeOps are *not* invokable as plain operations - as we've implemented for stdmbeans.

For consistency this means that operations specified as getters/setters by the creator of the MMB instance and that have been bound to Attributes should not appear in the OperationInfo[] member of the MBeanInfo returned by the MMB.getMBeanInfo() operation.

Make sense?

If it does, it means that the MMBInfo used to create the MMB will be different from the MBeanInfo you can get via the MBeanServer.

Thoughts?
Trev

[squirest]

Sorry, I keep replying to my own posts. Further to my assertion that it's ok to modify MMBInfo (I still agree about your proposal for handling multiple targets though).

I think there is a precedent for returning MMBInfo different from what was passed into the MMB at creation time.

With a persistent MMB I believe it's legal to supply MMBInfo with nothing more than a top-level Descriptor. Then all of the operation/constructor/attribute guff is expected to be retrieved from the persistence manager.

I'm pretty sure that's the spirit of one of the examples that comes with the RI.

Trev

[juha]

> Rather than introduce a new class (which would need
> to be in org.jboss) we could actually re-use the
> Descriptor class.
>
> Descriptor d = new DescriptorSupport();
> d.addTargetStuff();
> MMB.setManagedResource(d, "targetDescriptor");
>
> It's a minor issue but it might encourage the the
> spec authors to include it as part of the standard.
> Or am I just dreaming?

I think you're dreaming ;)

But you are correct. We should, and will, reuse the descriptor class in this case.

-- Juha

[juha]

> Each method must be defined as an operation (not sure
> if the method *has* to be marked as "gettter" or
> "setter").

That seems to have been the intention at some point. However, as far as I can tell, they forgot to describe the semantics of these two fields in the spec...

I would set this role for setter and getter operations though. I can't see any harm in doing so.


> This opens us to a problem already seen in the RI.
> Namely that attribute-specific operations are left
> exposed as plain operations (bypassing attribute
> cache). I beleive the spirit of the spec is that
> attributeOps are *not* invokable as plain operations
> - as we've implemented for stdmbeans.

On the other hand, exposing the operations allows you to specifically force-read and -write the values, always knowing you reach the resource object itself and bypass the cache...

So the question is, should we hide the setter and getter operations already from the MBeanInfo or should that be left up to the tools (such as a HTTP adaptor) to figure out?


> For consistency this means that operations specified
> as getters/setters by the creator of the MMB instance
> and that have been bound to Attributes should not
> appear in the OperationInfo[] member of the MBeanInfo
> returned by the MMB.getMBeanInfo() operation.
>
> Make sense?

yeah I can see where you're coming from...

hmm thinking thinking...

I'm sort of undecided... hmm, yeah go ahead, get rid of operations with getter or setter role. If something comes up, its easy to change.

-- Juha