Jerome Dochez wrote:
> indeed I can see there is a problem :(
> however, I think your fix might still not be complete...
>
> can you try mine :)
Dynamic reconfig tests like your patch too. :-)
Can we apply the attached patch before next integration also? It should
address hk2.config.Transactions NPE which happens at times.
Thanks,
Amy
> thanks, jerome
>
> Index: config/src/java/org/jvnet/hk2/config/ConfigSupport.java
> ===================================================================
> --- config/src/java/org/jvnet/hk2/config/ConfigSupport.java (revision
> 839)
> +++ config/src/java/org/jvnet/hk2/config/ConfigSupport.java (working
> copy)
> @@ -292,8 +292,8 @@
> // we don't really send the changed events for new
> comers.
> continue;
> }
> + ConfigBeanProxy proxy = null;
> if (event.getNewValue()==null) {
> - ConfigBeanProxy proxy = null;
> try {
> // getOldValue() can be null, we will notify a
> CHANGE event
> proxy =
> ConfigBeanProxy.class.cast(event.getOldValue());
> @@ -301,6 +301,7 @@
> // this is ok, the old value was probably a
> string or something like this...
> // we will notify the event.getSource() that it
> changed.
> }
> + // new value is null, but not old value, we removed
> something
> if (proxy!=null) {
> final NotProcessed nc =
> target.changed(Changed.TYPE.REMOVE, proxyType(proxy), proxy );
> if ( nc != null ) {
> @@ -308,13 +309,17 @@
> }
> continue;
> }
> - if (!changed.contains(eventSource)) {
> - proxy =
> ConfigBeanProxy.class.cast(event.getSource());
> - changed.add(eventSource);
> - final NotProcessed nc =
> target.changed(Changed.TYPE.CHANGE, proxyType(proxy), proxy);
> - if ( nc != null ) {
> - unprocessed.add( new
> UnprocessedChangeEvent(event, nc.getReason() ) );
> - }
> + }
> + // we fall back in this case, the newValue was nullm
> the old value was also null or was not a ConfigBean,
> + // we revert to just notify of the change.
> + // when the new value is not null, we also send the
> CHANGE on the source all the time, unless this was
> + // and added config bean.
> + if (!changed.contains(eventSource)) {
> + proxy =
> ConfigBeanProxy.class.cast(event.getSource());
> + changed.add(eventSource);
> + final NotProcessed nc =
> target.changed(Changed.TYPE.CHANGE, proxyType(proxy), proxy);
> + if ( nc != null ) {
> + unprocessed.add( new
> UnprocessedChangeEvent(event, nc.getReason() ) );
> }
> }
> } catch (Exception e) {
>
> Amy Roh wrote:
>
>> Attached the correct hk2 fix ;-)
>>
>> Amy Roh wrote:
>>
>>> Hi Jerome,
>>>
>>> I believe there's an oversight regarding hk2 revision 835 patch that
>>> was integreated for hk2 0.3.22.
>>>
>>> The attach diff fixes the oversight which ignored all CHANGE events
>>> when both event.getNewValue() and getOldValue() aren't NULL.
>>>
>>> Do we need another integration to include this fix?
>>>
>>> Thanks,
>>> Amy
>>>
>>> Amy Roh wrote:
>>>
>>>> I notice major regression on dynamic reconfig with the latest build.
>>>> Dynamic reconfig tests were passing last time web container related
>>>> changes were made on 9/10 and as late as of 9/12. I'm trying to
>>>> track down the cause. Pls. give me any headsup if you are aware of
>>>> any config changes that might have affected dynamic reconfig.
>>>>
>>>> Thanks,
>>>> Amy
>>>
>>>
>>> Subject:
>>> svn commit: r835 -
>>> trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java
>>> From:
>>> dochez_at_dev.java.net
>>> Date:
>>> Mon, 15 Sep 2008 05:21:19 +0000
>>> To:
>>> commits_at_hk2.dev.java.net
>>>
>>> To:
>>> commits_at_hk2.dev.java.net
>>>
>>>
>>> Author: dochez
>>> Date: 2008-09-15 05:21:17+0000
>>> New Revision: 835
>>>
>>> Modified:
>>> trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java
>>>
>>> Log:
>>> fixed IT 6001 : NPE when old value is null in sortAndDispatch
>>>
>>>
>>>
>>> Modified:
>>> trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java
>>> Url:
>>> https://hk2.dev.java.net/source/browse/hk2/trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java?view=diff&rev=835&p1=trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java&p2=trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java&r1=834&r2=835
>>>
>>> ==============================================================================
>>>
>>> ---
>>> trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java
>>> (original)
>>> +++
>>> trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java
>>> 2008-09-15 05:21:17+0000
>>> @@ -87,7 +87,7 @@
>>> public class ConfigSupport {
>>> /**
>>> - * Execute§ some logic on one config bean of type T protected by
>>> a transaction
>>> + * Execute� some logic on one config bean of type T protected
>>> by a transaction
>>> *
>>> * @param code code to execute
>>> * @param param config object participating in the transaction
>>> @@ -293,14 +293,23 @@
>>> continue;
>>> }
>>> if (event.getNewValue()==null) {
>>> - final ConfigBeanProxy proxy =
>>> ConfigBeanProxy.class.cast(event.getOldValue());
>>> - final NotProcessed nc =
>>> target.changed(Changed.TYPE.REMOVE, proxyType(proxy), proxy );
>>> - if ( nc != null ) {
>>> - unprocessed.add( new
>>> UnprocessedChangeEvent(event, nc.getReason() ) );
>>> + ConfigBeanProxy proxy = null;
>>> + try {
>>> + // getOldValue() can be null, we will notify
>>> a CHANGE event
>>> + proxy =
>>> ConfigBeanProxy.class.cast(event.getOldValue());
>>> + } catch (ClassCastException e) {
>>> + // this is ok, the old value was probably a
>>> string or something like this...
>>> + // we will notify the event.getSource() that
>>> it changed.
>>> + }
>>> + if (proxy!=null) {
>>> + final NotProcessed nc =
>>> target.changed(Changed.TYPE.REMOVE, proxyType(proxy), proxy );
>>> + if ( nc != null ) {
>>> + unprocessed.add( new
>>> UnprocessedChangeEvent(event, nc.getReason() ) );
>>> + }
>>> + continue;
>>> }
>>> - } else {
>>> if (!changed.contains(eventSource)) {
>>> - final ConfigBeanProxy proxy =
>>> ConfigBeanProxy.class.cast(event.getSource());
>>> + proxy =
>>> ConfigBeanProxy.class.cast(event.getSource());
>>> changed.add(eventSource);
>>> final NotProcessed nc =
>>> target.changed(Changed.TYPE.CHANGE, proxyType(proxy), proxy);
>>> if ( nc != null ) {
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: commits-unsubscribe_at_hk2.dev.java.net
>>> For additional commands, e-mail: commits-help_at_hk2.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
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>
Index: src/java/org/jvnet/hk2/config/Transactions.java
===================================================================
--- src/java/org/jvnet/hk2/config/Transactions.java (revision 837)
+++ src/java/org/jvnet/hk2/config/Transactions.java (working copy)
@@ -232,7 +232,7 @@
// dochez : should we notify the parent chain up to the root or stop at the first parent.
if (dom.parent()!=null && dom.parent().getListeners()!=null) {
for (ConfigListener parentListener : dom.parent().getListeners()) {
- if (!notifiedListeners.contains(parentListener)) {
+ if (parentListener!=null && !notifiedListeners.contains(parentListener)) {
try {
// create a new array each time to avoid any potential array changes?
UnprocessedChangeEvents unprocessed = parentListener.changed(mEvents.toArray(new PropertyChangeEvent[mEvents.size()]));