Kedar,
Yes, it makes sense *if* we want DuckTyped methods, rather than
repeating 4 times. If not, no need.
Now as far as annotating on the List declation, the only regrettable
thing is that interfaces extending PropertyBag have to redundantly
declare the getProperty() method already declared in PropertyBag.
This is one reason I did the annotation on the class.
@Element("property")
List<Property> getProperty();
Since there can be only one List of properties, perhaps it's not so
bad to annotate the class so as to not have to redeclared this method?
Lloyd
On Oct 20, 2008, at 5:34 PM, Kedar Mhaswade wrote:
> Property is more ubiquitous than SystemProperty.
>
> AFAIK, SystemProperty exists at Domain, Cluster, Config and Server
> alone. Do you think it makes any good having a separate Bag of
> SystemProperties for these 4?
>
> Thanks,
> Kedar
>
> Lloyd Chambers wrote:
>> Jerome et al,
>> One question that arises: since we have Property and PropertyBag,
>> should there by SystemProperty (exists) and SystemPropertyBag
>> (doesn't yet exist)?
>> Lloyd
>> On Oct 20, 2008, at 10:07 AM, Lloyd Chambers wrote:
>>> Thanks Jerome. Comments inline.
>>>
>>> On Oct 17, 2008, at 9:58 PM, Jerome Dochez wrote:
>>>
>>>> I like it, some remarks :
>>>>
>>>> 1. why is the annotation placed at the class level, shouldn't the
>>>> field List<Property> be annotated instead
>>> Good idea. I can't think of any reason not to do this, except
>>> that it could annotate the wrong method I suppose.
>>>
>>>>
>>>> 2. doing field annotation would remove the need to
>>>> SystemPropertiesDesc since you would just annotate List<Property>
>>>> systemProperties
>>> Yes, this is true. We would need to then make an explicit
>>> assumption about the name of the method in order to find and
>>> distinguish the annotations.
>>>
>>>>
>>>> 3. for description(), I would encourage for some i18n where
>>>> instead of an english description, we could reference a
>>>> LocalString.properties element. This would allow us to generate
>>>> localized @Configured documentation.
>>> It's essentially javadoc-equivalent I was thinking, which is all
>>> English at present. But if we think it's something we would
>>> expose via the GUI (for example), then it could be I18N. Perhaps
>>> it can be both, description() and descriptionKey().
>>>
>>>>
>>>> 4. is this a pure documentation play or should we push runtime
>>>> enforcement ?
>>> I think at the very least that it would be very helpful to
>>> customers to see warning messages in the log file if unknown
>>> properties were added, for example a mis-spelled one. This is
>>> much better than something mysteriously not working. The warning
>>> could emit a list of all legal properties, which could be very
>>> helpful.
>>>
>>> And perhaps an enforce() flag could be added, so that a module
>>> implementor could choose to enforce rejection of unknown properties.
>>>
>>>>
>>>> 5. I would question the "required()" boolean, I think AFAIK,
>>>> there are no required properties and if there are, maybe we
>>>> should move them to real attribute or element.
>>> OK, I wasn't sure on this. I think there might be one or two
>>> cases that require a property, I forget which ones.
>>>
>>>
>>>>
>>>>
>>>> Lloyd Chambers wrote:
>>>>> Team,
>>>>>
>>>>> I just finished going through "Sun GlassFish Enterprise Server
>>>>> v3 Prelude Administration Reference". It took a lot of "eyeball
>>>>> work" going through every page, and each and every interface--
>>>>> this is error prone and hard to maintain.
>>>>>
>>>>> Many properties and system properties are documented in the
>>>>> reference, yet none of them are documented in the @Configured
>>>>> interfaces--at all. This stands in stark contrast to attributes
>>>>> and elements, which are naturally javadoc'd.
>>>>>
>>>>> Several other issues come to mind for properties (eg <property>
>>>>> and <system-property>):
>>>>> - no formal mechanism to document the legal properties (except
>>>>> free-form javadoc comments, none there as yet);
>>>>> - impossible to validate property names or values;
>>>>> - cannot warn the user if an unknown or mis-spelled property is
>>>>> being used (eg "readerThreads" instead of "reader-threads");
>>>>> - documentation for public API is not possible; there is nothing
>>>>> to generate it from;
>>>>> - no way for dependent interfaces (eg AMX) to validate or
>>>>> provide metadata.
>>>>>
>>>>> --- PROPOSAL ---
>>>>>
>>>>> 1) Add annotations to describe the legal properties and system
>>>>> properties;
>>>>> 2) Add these annotations on all the @Configured interfaces which
>>>>> have properties;
>>>>> 3) Encourage developers to maintain the annotations.
>>>>>
>>>>> Below is an example of how this would work for @Configured
>>>>> HttpListener. One alternative would be to define a "sidecar"
>>>>> class that has "public static final String" variables for each
>>>>> property, and annotate each of those individually.
>>>>>
>>>>> Note that because there cannot be more than one annotation of a
>>>>> given type, a grouping annotation is used to allow annotations
>>>>> for every property. See annotation classes further below.
>>>>>
>>>>> @PropertiesDesc({
>>>>> @PropertyDesc(name="recycle-objects", defaultValue="true",
>>>>> dataType=Boolean.class),
>>>>> @PropertyDesc(name="reader-threads", defaultValue="0",
>>>>> dataType=NonNegativeInteger.class),
>>>>> @PropertyDesc(name="acceptor-queue-length",
>>>>> defaultValue="4096", dataType=NonNegativeInteger.class),
>>>>> @PropertyDesc(name="reader-queue-length", defaultValue="4096",
>>>>> dataType=NonNegativeInteger.class),
>>>>> @PropertyDesc(name="use-nio-direct-bytebuffer",
>>>>> defaultValue="false", dataType=Boolean.class),
>>>>> @PropertyDesc(name="authPassthroughEnabled",
>>>>> defaultValue="false", dataType=Boolean.class),
>>>>> @PropertyDesc(name="proxyHandler",
>>>>> defaultValue="com.sun.enterprise.web.ProxyHandlerImpl"),
>>>>> @PropertyDesc(name="proxiedProtocol", values={"ws/tcp", "http",
>>>>> "https", "tls"}),
>>>>> @PropertyDesc(name="bufferSize", defaultValue="4096",
>>>>> dataType=NonNegativeInteger.class),
>>>>> @PropertyDesc(name="connectionTimeout", defaultValue="30",
>>>>> dataType=NonNegativeInteger.class),
>>>>> @PropertyDesc(name="maxKeepAliveRequests", defaultValue="250",
>>>>> dataType=NonNegativeInteger.class),
>>>>> @PropertyDesc(name="traceEnabled", defaultValue="true",
>>>>> dataType=Boolean.class),
>>>>> @PropertyDesc(name="cometSupport", defaultValue="false",
>>>>> dataType=Boolean.class),
>>>>> @PropertyDesc(name="jkEnabled", defaultValue="false",
>>>>> dataType=Boolean.class),
>>>>> @PropertyDesc(name="compression", defaultValue="off",
>>>>> values={"off","on","force"}),
>>>>> @PropertyDesc(name="compressableMimeType", defaultValue="text/
>>>>> html, text/xml, text/plain"),
>>>>> @PropertyDesc(name="noCompressionUserAgents", defaultValue=""),
>>>>> @PropertyDesc(name="compressionSize",
>>>>> dataType=NonNegativeInteger.class),
>>>>> @PropertyDesc(name="minCompressionSize",
>>>>> dataType=NonNegativeInteger.class),
>>>>> @PropertyDesc(name="crlFile"),
>>>>> @PropertyDesc(name="trustAlgorithm", values="PKIX"),
>>>>> @PropertyDesc(name="trustMaxCertLength", defaultValue="5",
>>>>> dataType=Integer.class),
>>>>> @PropertyDesc(name="disableUploadTimeout", defaultValue="true",
>>>>> dataType=Boolean.class),
>>>>> @PropertyDesc(name="connectionUploadTimeout", defaultValue="5",
>>>>> dataType=NonNegativeInteger.class),
>>>>> @PropertyDesc(name="uriEncoding", defaultValue="UTF-8",
>>>>> values={"UTF-8"})
>>>>> })
>>>>>
>>>>> @AMXConfigInfo
>>>>> ( amxInterfaceName
>>>>> ="com.sun.appserv.management.config.HTTPListenerConfig")
>>>>> @Configured
>>>>> public interface HttpListener extends ConfigBeanProxy,
>>>>> Injectable, PropertyBag {
>>>>>
>>>>> ...
>>>>> }
>>>>>
>>>>> ---
>>>>>
>>>>> /**
>>>>> * Describes properties or system properties that might exist as
>>>>> sub-elements.
>>>>> */
>>>>> @Retention(RUNTIME)
>>>>> @Target({TYPE})
>>>>> public @interface PropertyDesc {
>>>>> /** name of the property */
>>>>> String name();
>>>>>
>>>>> /** default value of the property */
>>>>> String defaultValue() default "\u0000";
>>>>>
>>>>> /** freeform description */
>>>>> String description() default "\u0000";
>>>>>
>>>>> /** Indicates that this property is required (rare) */
>>>>> boolean required() default false;
>>>>>
>>>>> /** the DataType class, can be Class<? extends {_at_link
>>>>> DataType}> or String.class */
>>>>> Class dataType() default String.class;
>>>>>
>>>>> /** Possible values, might not be a complete list and/or there
>>>>> could be other alternatives
>>>>> such as specific numbers, variables, etc.
>>>>> */
>>>>> String[] values();
>>>>> }
>>>>>
>>>>>
>>>>> /**
>>>>> * Annotation that holds an array of {_at_link PropertyDesc} for
>>>>> system properties eg {_at_link SystemProperty}.
>>>>> * Needed because it's not otherwise possible to have more than
>>>>> one annotation of the same type.
>>>>> */
>>>>> @Retention(RUNTIME)
>>>>> @Target({TYPE,METHOD})
>>>>> public @interface SystemPropertiesDesc {
>>>>> /** name of the property */
>>>>> PropertyDesc[] value();
>>>>> }
>>>>>
>>>>> /**
>>>>> * Annotation that holds an array of {_at_link PropertyDesc} for
>>>>> properties eg {_at_link Property}.
>>>>> * Needed because it's not otherwise possible to have more than
>>>>> one annotation of the same type.
>>>>> */
>>>>> @Retention(RUNTIME)
>>>>> @Target({TYPE,METHOD})
>>>>> public @interface PropertiesDesc {
>>>>> /** name of the property */
>>>>> PropertyDesc[] value();
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> ..............................................
>>>>> Lloyd Chambers
>>>>> lloyd.chambers_at_sun.com
>>>>> GlassFish team, LSARC member
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>>>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>