Guys,
I have made a code change (attached) and tested it and it works
just like what we want. I have sent it to Jerome for his review. With
this change config listeners will get a chance to see the config
beans they get from events as normal config beans. If someone in
runtime is till interested in the tokenized value i.e. "${PORT}" and
not "1234", then they can query the raw attribute on the bean by
getting to Dom inside.
e.g.
ConfigBeanProxy ls = ...; //(say instance of Network Listener interface)
Dom dom = Dom.unwrap(ls); //corresponding "Dom"
dom.rawAttribute("port"); //returns "${PORT}".
either of:
ls.getPort() or dom.attribute("port") will return "1234", which is what
the listener is really interested in.
I don't think the runtime will ever need unresolved values (i.e. "${PORT"},
they are almost always interested in resolved ones).
Jerome, please review and let me know if this is something we should do.
Regards,
Kedar
PS - If someone is interested in seeing this in action, please put the
attached config.jar in your modules folder and let me know if it works
for you.
hk2 Code-diff's:
Index: config/src/java/org/jvnet/hk2/config/DomDocument.java
===================================================================
--- config/src/java/org/jvnet/hk2/config/DomDocument.java (revision 1223)
+++ config/src/java/org/jvnet/hk2/config/DomDocument.java (working copy)
@@ -59,7 +59,7 @@
* configuration is parsed, so this allows circular references —
* {_at_link Translator} may refer to objects in the configuration file being
read.
*/
- private volatile Translator translator = Translator.NOOP;
+ private volatile Translator translator = Translator.SYS_PROP_TR;
protected final Map<Inhabitant<? extends ConfigInjector>,ConfigModel>
models = new HashMap<Inhabitant<? extends ConfigInjector>, ConfigModel>();
private final MultiMap<Class, List<ConfigModel>> implementorsOf = new
MultiMap<Class, List<ConfigModel>>();
Index: config/src/java/org/jvnet/hk2/config/Translator.java
===================================================================
--- config/src/java/org/jvnet/hk2/config/Translator.java (revision 1223)
+++ config/src/java/org/jvnet/hk2/config/Translator.java (working copy)
@@ -56,4 +56,26 @@
return str;
}
};
+ public static final Translator SYS_PROP_TR = new Translator() {
+ public String translate(String str) {
+ String start = "${";
+ String end = "}";
+ int starti = str.indexOf(start);
+ int endi = str.indexOf(end);
+ if (starti >= 0 && endi >= 3 ) {
+ String pn = str.substring(starti+2, endi);
+ System.out.println("prop name: " + pn);
+ String pv = System.getProperty(pn);
+ if (pv == null) {
+ System.out.println("not replaced: " + str);
+ return str;
+ }
+ //throw new RuntimeException("Define the property first");
+ String replaced = str.replace(start + pn + end, pv);
+ System.out.println("replaced: " + replaced);
+ return replaced;
+ }
+ return str; //do nothing otherwise
+ }
+ };
}
Lloyd Chambers wrote:
> Shouldn't we supply old and new values?
>
> Translating values introduces a new subtle issue: the real value could
> remain unchanged, but the token could be different eg changing from
> ${FOO} to ${BAR} with both FOO and BAR equal to the same value.
>
> Lloyd
>
> On Oct 8, 2009, at 9:15 PM, Jerome Dochez wrote:
>
>> kedar wrote:
>>>
>>>
>>> Jerome Dochez wrote:
>>>> This is something that would be pretty easy to change in the config
>>>> layer but I am a little scared about possible regressions by
>>>> changing this now. I am surprised we did not have that raised before !
>>>>
>>>> Kedar, I know everything can be potentially tokenized but
>>>> realistically how much do we have tokenized right now (i.e. who will
>>>> be potentially impacted by a change a behaviour).
>>>
>>> I agree. But the problem right now is that listeners do something like:
>>> listener.getPort() and expect it to be a number. So, all listeners
>>> should be changed to call the service to translate the raw values,
>>> right?
>>>
>>> Besides, there is a correctness issue here. When I have:
>>>
>>> <system-property name="PORT" value="1234"/>
>>> ....
>>> <network-listener port="${PORT}"/>,
>>>
>>> when we restart, listener.getPort() returns 1234 correctly, but in case
>>> of dynamic reconfig the same call returns "${PORT}". Is that ok?
>>>
>> I understand perfectly that it is the problem. I suppose we should
>> look at all ConfigListener implementations to ensure that they are not
>> already doing translation of raw values because if we fix it, we will
>> break those.
>>>>
>>>> I think the safest of course is to keep the current behaviour and
>>>> change it post release if we can. There is a service that be can be
>>>> used by config listeners to translate raw values.
>>>>
>>>> jerome
>>>>
>>>> Tim Quinn wrote:
>>>>> Kedar Mhaswade wrote:
>>>>>> Tim Quinn wrote:
>>>>>>> If the user changes a config value (I tried using the admin
>>>>>>> console) that contains a token (such as
>>>>>>> ${com.sun.aas.instanceRoot}) then the event delivered to the
>>>>>>> "change" method returns the untranslated old and new values,
>>>>>>> still containing the tokens.
>>>>>>>
>>>>>>> Maybe this has been discussed before and I've forgotten... Is
>>>>>>> there something the "change" method can do to translate the
>>>>>>> values? It seems that the injected configuration object has
>>>>>>> already been updated with the translated new value. Is that
>>>>>>> expected and reliable?
>>>>>>> Thanks.
>>>>>>
>>>>>> Tim,
>>>>>>
>>>>>> Incidentally, I had some conversation with Jerome on this just
>>>>>> yesterday
>>>>>> and I think it is an issue.
>>>>>>
>>>>>> IMO, The config layer should already resolve (expand) the variable
>>>>>> in the configuration object given to the changed method in
>>>>>> ConfigListener
>>>>>> so that the runtime code does not have to do anything there.
>>>>> That's how it is now...at least that's what I saw.
>>>>>>
>>>>>> Alternatively, the variable expansion is just doing a replacement
>>>>>> from
>>>>>> system properties. So in the changed method, if a particular
>>>>>> attribute
>>>>>> contains {xxx} you could replace it with System.property(xxx). I have
>>>>>> tried it and it seems to work.
>>>>> I know that the changed method accepts a
>>>>> java.beans.PropertyChangeEvent argument. If only it were an
>>>>> org.jvnet.hk2.config.PropertyChangeEvent we could have exposed
>>>>> getRaw[Old | New]Value methods, for example. Maybe there could be
>>>>> a utility method so this could be implemented once instead of in
>>>>> each ConfigListener?
>>>>> This would be especially helpful if, as Lloyd said, the translation
>>>>> should include the <system-property> settings - or are those
>>>>> already set so they are accessible via System.getProperty()?
>>>>>>
>>>>>> I am going to spend some time on making the injected configuration
>>>>>> object have the variable-expanded values though.
>>>>> But it seems to do that already. Am I missing something?
>>>>>
>>>>> - Tim
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
>>>> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
>>> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
>> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>>
>
> Lloyd Chambers
> lloyd.chambers_at_sun.com
> GlassFish Team
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>