AMX always works in terms of the raw values for emitting notifications.
This change breaks that behavior. AMX has no way of getting the OLD
raw value.
Now the situation becomes: get an attribute or set one with RAW
Listen for changes and everything is translated.
This is ugly.
Lloyd
On Oct 9, 2009, at 10:29 AM, kedar wrote:
> 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
> <
> config.jar
> >---------------------------------------------------------------------
> 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