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
>