[ https://jira.jboss.org/jira/browse/JGRP-1068?page=com.atlassian.jira.plug... ] Richard Achmatowicz commented on JGRP-1068: ------------------------------------------- I'm just about to go through and update the @Property processing for some of the InetAddress based and List<InetAddress> based variables we use, so that they are all treated during @Property processing. I found this example in MPING: public void init() throws Exception { super.init(); String str=Util.getProperty(new String[]{Global.MPING_MCAST_ADDR}, null, "mcast_addr", false, null); if(str != null) mcast_addr=InetAddress.getByName(str); str=Util.getProperty(new String[]{Global.MPING_MCAST_PORT}, null, "mcast_port", false, null); if(str != null) mcast_port=Integer.parseInt(str); str=Util.getProperty(new String[]{Global.MPING_IP_TTL}, null, "ip_ttl", false, null); if(str != null) ip_ttl=Integer.parseInt(str); if(mcast_addr == null) { mcast_addr=InetAddress.getByName("230.5.6.7"); } } This started me thinking about how we process properties in JGroups, especially now with @Property. Does the following concur with your understanding of how properties should be processed? 1. Usera can specify defaults for properties in one of two ways. The usual way is to provide a default for the value in a stack configuration file. Over and above that, they may provide a default on the command line using a named system property. The system property should override the default in the stack configuration file through system property substitution within the stack config file. (NOTE: we could also systematically check system properties during property conversion, with no need to use variable substitution in config files - but forget about this when reading point 2.) 2. Once the stack config file has been processed, each @Property in a protocol gets a value assigned through PropertyConverter processing. At this stage, the users intentions for the variable should have been determined. No further system variable substitution for @Property values should be required. 3. After property processing, @Property variables may have valid values(e.g. user assigned), invalid values(e.g. user assigned IPv6 address, but no scope) or no value (null). We don't really have any systematic way of checking for the case of no (default) value at present. One way to do this is via sanity checks, after property processing and before init() is called on each protocol. This is the intention for the InetAddress related stuff - to call inetAddressSanityCheck() to assign a consistent IP version and check IPv6 scopes before init() starts. The good thing about this is that we have access to all @Property values in the stack and can assign defaults accordingly. This is needed for determining the correct IP version to use. On the other hand, there are a lot of variables in a lot of protocols which may require programmer assigned defaults, and assigning them in this centralized location through table lookup doesn't scale well. It would be more convenient to assign a programmer assigned default value through the @Property mechanism itself. But because @Proeprty processing uses converters to assign values to variables, the assignment of programmer assigned default values would need to be incorporated into the ProeprtyConverter mechanism. For example, in the case of jgroups.udp.mcast_addr, we used to specify a default string value "224.x.y.z" which would get converted in init() to an InetAddress if mcast_addr == null, just like in the code above. But now this happens too late. Is it worth adding an attribute to @Property to specify a programmer supplied default value for the property which would be passed through the converter if no user specified value was present? At the moment, during property processing, we check if a user assigned value is present: if yes, we call the converter; if no, we don't call the converter. This means that some fields, which take complex data structures which need special conversion from strings, like initial_hosts, have null for a default and have to be picked up after property processing has completed. We could incorporate programmer assigned defaults as follows: (i) introduce a new field in @Property called default (ii) during property processing, we check if a user assigned value is present: if yes, we call the converter; if no, we call the converter using the default value, if there is one; if there is no default value, we do nothing. In this way, all properties have either a user assigned or programmer assigned default value. The default values are referenced by Global.jgroups.protocol.variable name which picks up the default from a table during processing. With this in place, we eliminate checking for default values in the sanify checks phase, and just have to concentrate on the sanity check itself. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://jira.jboss.org/jira/browse/JGRP-1068?page=com.atlassian.jira.plug... ] Richard Achmatowicz commented on JGRP-1068: ------------------------------------------- I implemented the default processing we have been discussing tonight. It's pretty simple. I moved sanityCheck out from createProtcols and added a new method: private Protocol setupProtocolStack(String configuration, ProtocolStack st) throws Exception { Vector<ProtocolConfiguration> protocol_configs=parseConfigurations(configuration); Vector<Protocol> protocols=createProtocols(protocol_configs, st); if(protocols == null) return null; sanityCheck(protocols); processDefaultValues(protocol_configs, protocols) ; return connectProtocols(protocols); } For each protocol in turn, processDefaultValues simply creates a map of all properties which have no user specified value, and once the map is created, goes through each entry and if a default value is present, computes the value and assigns it to the field. I didn't include the cases for defaultValueIPv4 nor defaultValueIPv6 as they depend on code for the IP version checking. To incorporate them once the IP version checking stuff is written is really simple, something like: // get the default value for the field if (InetAddressInfo.isInetAddressRelated(field)) { if (assumeIPv4) { default = property.defaultIPv4() ; else default = property.defaultIPv6() ; } else { default = property.default() ; } I haven't yet committed the stuff but will tomorrow morning. The testsuite runs OK at present with the code in. This code is self contained, in the sense that it doesn't depend on any data gathered in the basic @Proeprty processing phasse. It is the case that the three processing phases * basic @Proeprty processing * IP version checking * default processing do need similar types of information (e.g. which fields/methods are InetAddress related) and in theory it would be efficient to gather that info on the first pass and share it on the secodna nd third passes, but it leads to less maintainable code. I'm going to write the three indepene3nct of each other at first. It can be optimized later if desired. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://jira.jboss.org/jira/browse/JGRP-1068?page=com.atlassian.jira.plug... ] Richard Achmatowicz commented on JGRP-1068: ------------------------------------------- Comiited the first phase od defaultValue() processing. Updated testcase ProtocolConfiguration to include test for processing of defaults. Once IP version checking is also in place, will modify for defaultValueIPv4&6. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://jira.jboss.org/jira/browse/JGRP-1068?page=com.atlassian.jira.plug... ] Richard Achmatowicz closed JGRP-1068. ------------------------------------- Resolution: Done Added test cases for defaults processing into ProtocolConfiguratorTest. Committed changes. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira