Beehive
  1. Beehive
  2. BEEHIVE-179

context.getControlPropertySet() should return null for non-existent annotation

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: V1Beta
    • Fix Version/s: V1Beta
    • Component/s: Controls
    • Labels:
      None
    • Environment:
      Windows

      Description

      I call

      ServiceControl.TypesJarName typesJarNameAnn =
      (ServiceControl.TypesJarName)context.
      getControlPropertySet(ServiceControl.TypesJarName.class);

      in the Impl of my control (ServiceControl). TypesJarName is a valid annotation which is defined in the public interface but not used in either the public interface nor in the JCX.

      I expected to receive null but instead I get a non-null object with defaulted values for its members.

      This is at odds with the javadoc for the API ControlBeanContext.getControlPropertySet() and in any case you need a way to be able to tell whether an annotation is present or not (especially if you have marker annotations).

        Activity

        Hide
        Kyle Marvin added a comment -

        This is wrapped up in the following commented out code, from the impl of org.apache.beehive.controls.runtime.bean.ControlBeanContext.getControlPropertySet():

        // TODO: Add a PropertySet meta-attribute that indicates the PropertySet is optional.
        // This implies: a) no getters/setters, and b) the PropertySet can appear to be
        // 'null' from the impl perspective if unset; otherwise, a PropertySet containing all
        // default values will be returned.
        //if (!map.containsPropertySet(propertySet))
        // return null;

        If a PropertySet/annotation is optional, then we can't really generate the client-visible getter for it. There is no way to express an unset value, unless it is the default. If the impl isn't using defaulted semantics, it seems wrong to tell the client that the default is the property value. I feel pretty strongly that client/impl views of property state must be identical.

        Similarly, if 'unset' is a valid value for the property, there would need to be a way to 'unset' a property, which again doesn't make sense using the standard JavaBeans accessor model.

        So the proposed resolution would be to add an 'optional' boolean attribute to the PropertySet annotation type, which has the semantics described in the comment above.

        Show
        Kyle Marvin added a comment - This is wrapped up in the following commented out code, from the impl of org.apache.beehive.controls.runtime.bean.ControlBeanContext.getControlPropertySet(): // TODO: Add a PropertySet meta-attribute that indicates the PropertySet is optional. // This implies: a) no getters/setters, and b) the PropertySet can appear to be // 'null' from the impl perspective if unset; otherwise, a PropertySet containing all // default values will be returned. //if (!map.containsPropertySet(propertySet)) // return null; If a PropertySet/annotation is optional, then we can't really generate the client-visible getter for it. There is no way to express an unset value, unless it is the default. If the impl isn't using defaulted semantics, it seems wrong to tell the client that the default is the property value. I feel pretty strongly that client/impl views of property state must be identical. Similarly, if 'unset' is a valid value for the property, there would need to be a way to 'unset' a property, which again doesn't make sense using the standard JavaBeans accessor model. So the proposed resolution would be to add an 'optional' boolean attribute to the PropertySet annotation type, which has the semantics described in the comment above.
        Hide
        Lawrence Jones added a comment -

        That sounds like a fine solution for the case where you need to have a PropertySet meta-annotation on the annotation definition.

        Can you let me know what would happen (ie can you spot the presence/absence of an annotation using the getControlPropertySet() API) in the following 2 cases?

        1. An annotation definition in the control public interface but without the PropertySet meta-annotation.

        2. An annotation definition which may (optionally) be used on a JCX or public interface, whose definition is somewhere else completely (and which does not have the PropertySet meta-annotation).

        My intention is to use the context APIs in all of these cases but maybe in some I'd be better off just using straight reflection?

        Thanks.

        Show
        Lawrence Jones added a comment - That sounds like a fine solution for the case where you need to have a PropertySet meta-annotation on the annotation definition. Can you let me know what would happen (ie can you spot the presence/absence of an annotation using the getControlPropertySet() API) in the following 2 cases? 1. An annotation definition in the control public interface but without the PropertySet meta-annotation. 2. An annotation definition which may (optionally) be used on a JCX or public interface, whose definition is somewhere else completely (and which does not have the PropertySet meta-annotation). My intention is to use the context APIs in all of these cases but maybe in some I'd be better off just using straight reflection? Thanks.
        Hide
        Kyle Marvin added a comment -

        I really don't understand your question.

        ControlBeanContext.getControlPropertySet() is used to query PropertySet values, so the only annotation type classes that can be passed to it are ones that are annotated with @PropertySet (whether locally within a control public interface, or defined externally and referenced from within one).

        Show
        Kyle Marvin added a comment - I really don't understand your question. ControlBeanContext.getControlPropertySet() is used to query PropertySet values, so the only annotation type classes that can be passed to it are ones that are annotated with @PropertySet (whether locally within a control public interface, or defined externally and referenced from within one).
        Hide
        Lawrence Jones added a comment -

        Am I right in thinking that the only way I can check whether an annotation that is not declared as a PropertySet is present or not is through reflection on the interface?

        And also that if I happened to pass the class name of such an annotation to getControlPropertySet() then I would always get null regardless of whether the annotation was present or not?

        If so then that's fine. I do not wish the user to be able to set the value of this annotation any way other than via placing the annotation on the JCX (I do not want setters to work at a later time etc.) - so I think I should remove the PropertySet meta-annotation and use reflection to get at the value at runtime?

        Nonetheless I think the original problem still remains - if I have an annotation that has been defined as a PropertySet, then I should be able to tell whether the annotation is present or not.

        I am happy with the solution you suggested for this earlier - the 'optional' member on PropertySet. (In particular if the PropertySet you wish to make available is a marker annotation - i.e. has no members - then you must use the optional member.)

        Show
        Lawrence Jones added a comment - Am I right in thinking that the only way I can check whether an annotation that is not declared as a PropertySet is present or not is through reflection on the interface? And also that if I happened to pass the class name of such an annotation to getControlPropertySet() then I would always get null regardless of whether the annotation was present or not? If so then that's fine. I do not wish the user to be able to set the value of this annotation any way other than via placing the annotation on the JCX (I do not want setters to work at a later time etc.) - so I think I should remove the PropertySet meta-annotation and use reflection to get at the value at runtime? Nonetheless I think the original problem still remains - if I have an annotation that has been defined as a PropertySet, then I should be able to tell whether the annotation is present or not. I am happy with the solution you suggested for this earlier - the 'optional' member on PropertySet. (In particular if the PropertySet you wish to make available is a marker annotation - i.e. has no members - then you must use the optional member.)
        Hide
        Kyle Marvin added a comment -

        Let me see if I can flip the question around:

        Why would you want to have an annotation that you'd use to decorate an instance or interface or method on a control and not have it be a property?

        By making it a property, you get lots of things for free: defaulting (if you want it), client accessors, auto-management by the runtime, an external config model, visibility in any IDE that knows how to do bean introspection, ...

        I guess I'm trying to tease out what makes you not want to use PropertySets, as an indicator that there is some missing feature.

        Show
        Kyle Marvin added a comment - Let me see if I can flip the question around: Why would you want to have an annotation that you'd use to decorate an instance or interface or method on a control and not have it be a property? By making it a property, you get lots of things for free: defaulting (if you want it), client accessors, auto-management by the runtime, an external config model, visibility in any IDE that knows how to do bean introspection, ... I guess I'm trying to tease out what makes you not want to use PropertySets, as an indicator that there is some missing feature.
        Hide
        Lawrence Jones added a comment -

        You may not have a choice. As we go forward there are going to be sets of standard annotations which a custom control author may wish to use but which are not defined with PropertySet on them.

        In this particular case the TypesJarName is something that is intrinsic to the development of the Service Control - if you want to change it you will probably at the same time need to change the source code of the JCX.

        I don't want anyone who is not the JCX developer to be capable of changing this. Specifically I don't want authors of the control client or administrators to be able to set this property by calling setters.

        Show
        Lawrence Jones added a comment - You may not have a choice. As we go forward there are going to be sets of standard annotations which a custom control author may wish to use but which are not defined with PropertySet on them. In this particular case the TypesJarName is something that is intrinsic to the development of the Service Control - if you want to change it you will probably at the same time need to change the source code of the JCX. I don't want anyone who is not the JCX developer to be capable of changing this. Specifically I don't want authors of the control client or administrators to be able to set this property by calling setters.
        Hide
        Kenneth Tam added a comment -

        I'm also reluctant to embrace the idea of encouraging non-propertyset annotations as part of the programming model for control extensions. I think we're better off adding configurability to propertysets to provide the kind of control you want. For example, there is currently support for read-only properties, which would give you the "no setters" feature you're looking for – just say @PropertySet( hasSetters=false ). The expectation is that there will be additional flexibility regarding whether a propertyset can be externally override, etc.

        Show
        Kenneth Tam added a comment - I'm also reluctant to embrace the idea of encouraging non-propertyset annotations as part of the programming model for control extensions. I think we're better off adding configurability to propertysets to provide the kind of control you want. For example, there is currently support for read-only properties, which would give you the "no setters" feature you're looking for – just say @PropertySet( hasSetters=false ). The expectation is that there will be additional flexibility regarding whether a propertyset can be externally override, etc.
        Hide
        Lawrence Jones added a comment -

        That sounds good and it looks like there's already an externalConfig member on PropertySet - so maybe I can set TypesJarName to be a PropertySet but to have externalConfig = false and hasSetters = false. That would address my concerns.

        I would still need Kyle's original suggestion of an additional 'optional' member on PropertySet such that getControlPropertySet() on that annotation would return null if the annotation was not present.

        Show
        Lawrence Jones added a comment - That sounds good and it looks like there's already an externalConfig member on PropertySet - so maybe I can set TypesJarName to be a PropertySet but to have externalConfig = false and hasSetters = false. That would address my concerns. I would still need Kyle's original suggestion of an additional 'optional' member on PropertySet such that getControlPropertySet() on that annotation would return null if the annotation was not present.
        Hide
        Kyle Marvin added a comment -

        The proposed soln (adding an optional() attribute to @PropertySet) has been implemented as SVN revision 154098.

        Show
        Kyle Marvin added a comment - The proposed soln (adding an optional() attribute to @PropertySet) has been implemented as SVN revision 154098.
        Hide
        James Song added a comment -

        Optional propertySet is tested by trunk\controls\test\src\controls\org\apache\beehive\controls\test\controls\property\PropertyControl.java and passes.

        Show
        James Song added a comment - Optional propertySet is tested by trunk\controls\test\src\controls\org\apache\beehive\controls\test\controls\property\PropertyControl.java and passes.

          People

          • Assignee:
            James Song
            Reporter:
            Lawrence Jones
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development