Jerome,
Looks good.
Adding PropertiesDesc.systemProperties() works fine.
Lloyd
..............................................
Lloyd Chambers
lloyd.chambers_at_sun.com
GlassFish team, LSARC member
On Oct 21, 2008, at 10:18 AM, Jerome Dochez wrote:
> Lloyd Chambers wrote:
>> Jerome,
>>
>> Yes, agreed if we maintain a separation of <property> and <system-
>> property>.
>>
>> If we merge them, then each PropertyDesc would systemProperty().
>>
>> Maybe I misunderstood your intent...
> so my intent was something like that
>
> @configured
> public interface JavaConfig extends... {
>
>
> @PropertiesDesc {
> systemProperties = false, // should be default value in fact
> {
> @PropertyDesc(name="recycle-objects", defaultValue="true",
> dataType=Boolean.class),
> @PropertyDesc(name="reader-threads", defaultValue="0",
> dataType=NonNegativeInteger.class)
> }
> @element("property")
> List<Property> properties;
>
> @PropertiesDesc {
> systemProperties = true, // should be default value in fact
> {
> @PropertyDesc(name="-Xmx", defaultValue="512m",
> dataType=String.class),
> @PropertyDesc(name="-Djava.security.SecurityManager",
> defaultValue="false", dataType=Boolean.class)
> }
> @element("system-property")
> List<Property> systemProperties;
>
> }
>
> I am not sure if we can add a boolean to @PropertiesDesc being a
> annotation aggregator, but if that's possible, I think it would fit
> nicely.
>>
>> Lloyd
>>
>> On Oct 21, 2008, at 10:01 AM, Jerome Dochez wrote:
>>
>>> Lloyd Chambers wrote:
>>>> Jerome,
>>>>
>>>> 1. On the annotation, yes we agreed on this yesterday when we
>>>> talked. I'll add systemProperty(), and remove required():
>>> I was thinking to have systemProperty() on the PropertiesDesc
>>> annotation, since you could group all the system properties
>>> together. Do we need such a fine grain level of control ?
>>>
>>> jerome
>>>>
>>>> @Retention(RUNTIME)
>>>> @Target({TYPE})
>>>> @Documented
>>>> public @interface PropertyDesc {
>>>> /** name of the property */
>>>> String name();
>>>>
>>>> /** default value of the property */
>>>> String defaultValue() default "\u0000";
>>>>
>>>> /** freeform description */
>>>> String description() default "\u0000";
>>>>
>>>> /** the DataType class, can be Class<? extends {_at_link
>>>> DataType}> or String.class, Integer.class, etc */
>>>> 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();
>>>>
>>>> /** returns true if this is a <system-property> */
>>>> boolean systemProperty();
>>>> }
>>>>
>>>>
>>>> Lloyd
>>>>
>>>> On Oct 20, 2008, at 9:29 PM, Jerome Dochez wrote:
>>>>
>>>>> Furthermore, I see no reasons to have SystemProperty,
>>>>> SystemProperties are just properties with a special meaning to
>>>>> the parent object.
>>>>>
>>>>> I think we should consider removing SystemProperty and have an
>>>>> attribute on PropertiesDesc annotation that specify whether this
>>>>> PropertyBag is system or not
>>>>>
>>>>> what do you think ?
>>>>>
>>>>> jerome
>>>>>
>>>>> 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
>>>>>>
>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> 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
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>