You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@etch.apache.org by sc...@apache.org on 2009/04/03 23:04:32 UTC
svn commit: r761793 - in /incubator/etch/branches/config/services/config/src:
main/java/org/apache/etch/services/config/YamlConfig.java
test/java/org/apache/etch/services/config/TestYamlConfig.java
Author: sccomer
Date: Fri Apr 3 21:04:31 2009
New Revision: 761793
URL: http://svn.apache.org/viewvc?rev=761793&view=rev
Log:
fix a problem with array and map change notifications.
Modified:
incubator/etch/branches/config/services/config/src/main/java/org/apache/etch/services/config/YamlConfig.java
incubator/etch/branches/config/services/config/src/test/java/org/apache/etch/services/config/TestYamlConfig.java
Modified: incubator/etch/branches/config/services/config/src/main/java/org/apache/etch/services/config/YamlConfig.java
URL: http://svn.apache.org/viewvc/incubator/etch/branches/config/services/config/src/main/java/org/apache/etch/services/config/YamlConfig.java?rev=761793&r1=761792&r2=761793&view=diff
==============================================================================
--- incubator/etch/branches/config/services/config/src/main/java/org/apache/etch/services/config/YamlConfig.java (original)
+++ incubator/etch/branches/config/services/config/src/main/java/org/apache/etch/services/config/YamlConfig.java Fri Apr 3 21:04:31 2009
@@ -664,38 +664,38 @@
private void updateConfig() throws ConfigurationException
{
- if (file.lastModified() != lastModified)
+ if (file.lastModified() == lastModified)
+ return;
+
+ long t0 = file.lastModified();
+ Object o = loadConfig0( file );
+ long t1 = file.lastModified();
+
+ if (t1 != t0)
+ return;
+
+ Set<Object> changeIds = new HashSet<Object>();
+
+ synchronized (subs)
{
- long t0 = file.lastModified();
- Object o = loadConfig0( file );
- long t1 = file.lastModified();
+ Map<Integer, Conf> newConfigs = new HashMap<Integer, Conf>( configs );
+ Set<Integer> newSubs = Collections.synchronizedSet(
+ new HashSet<Integer>( subs ) );
+ Set<Integer> changeSet = new HashSet<Integer>();
- if (t1 == t0)
- {
- Set<Object> changeIds = new HashSet<Object>();
-
- synchronized (subs)
- {
- Map<Integer, Conf> newConfigs = new HashMap<Integer, Conf>( configs );
- Set<Integer> newSubs = Collections.synchronizedSet(
- new HashSet<Integer>( subs ) );
- Set<Integer> changeSet = new HashSet<Integer>();
-
- updateObject( newConfigs, newSubs, changeSet, 0,
- null, null, o );
-
- // compute those ids in changeSet or their ancestors that
- // are subscribed to.
- computeChangeIds( changeIds, newConfigs, newSubs, changeSet );
-
- lastModified = t0;
- configs = newConfigs;
- subs = newSubs;
- }
-
- fireConfigValuesChanged( changeIds.toArray() );
- }
+ updateObject( newConfigs, newSubs, changeSet, 0,
+ null, null, o );
+
+ // compute those ids in changeSet or their ancestors that
+ // are subscribed to.
+ computeChangeIds( changeIds, newConfigs, newSubs, changeSet );
+
+ lastModified = t0;
+ configs = newConfigs;
+ subs = newSubs;
}
+
+ fireConfigValuesChanged( changeIds.toArray() );
}
private void computeChangeIds( Set<Object> changeIds,
@@ -732,7 +732,7 @@
if (valueIsScalar( value ))
{
int k = (int) idgen.next();
- newConfigs.put( k, new Conf( parent, nameOrIndex, value ) );
+ newConfigs.put( k, new Conf( k, parent, nameOrIndex, value ) );
return k;
}
@@ -819,16 +819,16 @@
if (c.isMap())
{
destroyMap( newConfigs, newSubs, iid, c );
- newConfigs.put( iid, new Conf( parent, nameOrIndex, value ) );
- changeSet.add( iid );
+ newConfigs.put( iid, new Conf( iid, parent, nameOrIndex, value ) );
+ add( newConfigs, changeSet, iid );
return;
}
if (c.isList())
{
destroyList( newConfigs, newSubs, iid, c );
- newConfigs.put( iid, new Conf( parent, nameOrIndex, value ) );
- changeSet.add( iid );
+ newConfigs.put( iid, new Conf( iid, parent, nameOrIndex, value ) );
+ add( newConfigs, changeSet, iid );
return;
}
@@ -837,8 +837,8 @@
if (value.equals( c.value ))
return;
- newConfigs.put( iid, new Conf( parent, nameOrIndex, value ) );
- changeSet.add( iid );
+ newConfigs.put( iid, new Conf( iid, parent, nameOrIndex, value ) );
+ add( newConfigs, changeSet, iid );
return;
}
}
@@ -847,11 +847,20 @@
value.getClass() + " from " + c.value.getClass() );
}
+ private void add( Map<Integer, Conf> newConfigs, Set<Integer> changeSet, int iid )
+ {
+ System.out.println( "changeSet.add( "+iid+" )" );
+ System.out.println( " conf: "+newConfigs.get( iid ) );
+// new Throwable().printStackTrace( System.out );
+
+ changeSet.add( iid );
+ }
+
private int importList( Map<Integer, Conf> newConfigs, Integer parent, Object nameOrIndex, List<?> value )
{
List<Integer> v = new ArrayList<Integer>( value.size() );
int k = (int) idgen.next();
- newConfigs.put( k, new Conf( parent, nameOrIndex, v ) );
+ newConfigs.put( k, new Conf( k, parent, nameOrIndex, Collections.unmodifiableList( v ) ) );
int i = 0;
for (Object o: value )
@@ -865,8 +874,8 @@
List<?> value )
{
List<Integer> v = new ArrayList<Integer>( value.size() );
- newConfigs.put( iid, new Conf( parent, nameOrIndex, v ) );
- changeSet.add( iid );
+ newConfigs.put( iid, new Conf( iid, parent, nameOrIndex, Collections.unmodifiableList( v ) ) );
+ add( newConfigs, changeSet, iid );
int i = 0;
for (Object o: value )
@@ -877,29 +886,39 @@
Set<Integer> changeSet, int iid, Integer parent, Object nameOrIndex,
List<?> value, Conf c )
{
- int n = Math.max( value.size(), c.size() );
+ List<Integer> newList = new ArrayList<Integer>( c.list() );
+ boolean changed = false;
+
+ int n = Math.max( value.size(), newList.size() );
for (int i = 0; i < n; i++)
{
- if (i < value.size() && i < c.size())
+ if (i < value.size() && i < newList.size())
{
- updateObject( newConfigs, newSubs, changeSet, c.list().get( i ), iid, i, value.get( i ) );
+ updateObject( newConfigs, newSubs, changeSet, newList.get( i ), iid, i, value.get( i ) );
}
- else if (i < c.size()) // ran out of new values
+ else if (i < newList.size()) // ran out of new values (newList too long)
{
- destroy( newConfigs, newSubs, c.list().get( i ) );
- c.list().set( i, null );
+ destroy( newConfigs, newSubs, newList.get( i ) );
+ newList.set( i, null );
+ changed = true;
// this entry will eventually be removed...
}
- else // i < value.size() // extending c
+ else // i < value.size() // extending newList
{
- c.list().add( importObject( newConfigs, iid, i, value.get( i ) ) );
+ newList.add( importObject( newConfigs, iid, i, value.get( i ) ) );
+ changed = true;
}
}
- // trim c if it is too long.
+ // trim newList if it is too long.
- while (c.list().size() > value.size())
- c.list().remove( value.size() );
+ while (newList.size() > value.size())
+ newList.remove( newList.size() - 1 );
+
+ newConfigs.put( iid, new Conf( iid, parent, nameOrIndex, Collections.unmodifiableList( newList ) ) );
+
+ if (changed)
+ add( newConfigs, changeSet, iid );
}
private void destroyList( Map<Integer, Conf> newConfigs, Set<Integer> newSubs, int iid, Conf c )
@@ -914,7 +933,7 @@
{
Map<String, Integer> v = new HashMap<String, Integer>( value.size() * 4 / 3 + 1 );
int k = (int) idgen.next();
- newConfigs.put( k, new Conf( parent, nameOrIndex, Collections.unmodifiableMap( v ) ) );
+ newConfigs.put( k, new Conf( k, parent, nameOrIndex, Collections.unmodifiableMap( v ) ) );
for (Map.Entry<?, ?> me: value.entrySet() )
{
@@ -930,8 +949,8 @@
Map<?, ?> value )
{
Map<String, Integer> v = new HashMap<String, Integer>( value.size() * 4 / 3 + 1 );
- newConfigs.put( iid, new Conf( parent, nameOrIndex, Collections.unmodifiableMap( v ) ) );
- changeSet.add( iid );
+ newConfigs.put( iid, new Conf( iid, parent, nameOrIndex, Collections.unmodifiableMap( v ) ) );
+ add( newConfigs, changeSet, iid );
for (Map.Entry<?, ?> me: value.entrySet() )
{
@@ -945,6 +964,7 @@
Map<?, ?> value, Conf c )
{
Map<String, Integer> newMap = new HashMap<String, Integer>( c.map() );
+ boolean changed = false;
// Look for names in the current map which are not in the new.
@@ -957,6 +977,7 @@
// this name will have to be deleted.
newMap.remove( name );
destroy( newConfigs, newSubs, siid );
+ changed = true;
}
}
@@ -971,6 +992,7 @@
// import the object
int k = importObject( newConfigs, iid, name, v );
newMap.put( name, k );
+ changed = true;
}
else
{
@@ -980,8 +1002,10 @@
}
}
- newConfigs.put( iid, new Conf( parent, nameOrIndex, Collections.unmodifiableMap( newMap ) ) );
- changeSet.add( iid );
+ newConfigs.put( iid, new Conf( iid, parent, nameOrIndex, Collections.unmodifiableMap( newMap ) ) );
+
+ if (changed)
+ add( newConfigs, changeSet, iid );
}
private void destroy( Map<Integer, Conf> newConfigs, Set<Integer> newSubs, int iid )
@@ -1101,7 +1125,7 @@
private class Conf
{
- public Conf( Integer parent, Object nameOrIndex, Object value )
+ public Conf( int id, Integer parent, Object nameOrIndex, Object value )
{
if (parent == null && nameOrIndex != null)
throw new IllegalArgumentException(
@@ -1114,10 +1138,13 @@
if (value == null)
throw new IllegalArgumentException( "value == null" );
+ this.id = id;
this.parent = parent;
this.nameOrIndex = nameOrIndex;
this.value = value;
}
+
+ private final int id;
public boolean isScalar()
{
@@ -1181,7 +1208,7 @@
@Override
public String toString()
{
- return String.format( "Conf parent %s nameOrIndex %s, value %s", parent, nameOrIndex, value );
+ return String.format( "Conf %d parent %s nameOrIndex %s, value %s", id, parent, nameOrIndex, value );
}
///CLOVER:ON
Modified: incubator/etch/branches/config/services/config/src/test/java/org/apache/etch/services/config/TestYamlConfig.java
URL: http://svn.apache.org/viewvc/incubator/etch/branches/config/services/config/src/test/java/org/apache/etch/services/config/TestYamlConfig.java?rev=761793&r1=761792&r2=761793&view=diff
==============================================================================
--- incubator/etch/branches/config/services/config/src/test/java/org/apache/etch/services/config/TestYamlConfig.java (original)
+++ incubator/etch/branches/config/services/config/src/test/java/org/apache/etch/services/config/TestYamlConfig.java Fri Apr 3 21:04:31 2009
@@ -21,7 +21,6 @@
package org.apache.etch.services.config;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
@@ -1090,14 +1089,11 @@
Object r = c.getRoot();
Assert.assertNotNull( r );
- Assert.assertFalse( client.changed.contains( r ) );
- Assert.assertEquals( 0, client.changed.size() );
+ Assert.assertEquals( set(), client.changed );
c.subscribe( r );
- Thread.sleep( 1000 );
-
- Assert.assertTrue( client.changed.contains( r ) );
- Assert.assertEquals( 1, client.changed.size() );
+ Thread.sleep( 100 );
+ Assert.assertEquals( set( r ), client.changed );
}
/** @throws Exception */
@@ -1112,14 +1108,11 @@
Object x = c.getConfigPath( r, "users" );
Assert.assertNotNull( x );
- Assert.assertFalse( client.changed.contains( x ) );
- Assert.assertEquals( 0, client.changed.size() );
+ Assert.assertEquals( set(), client.changed );
c.subscribe( x );
- Thread.sleep( 1000 );
-
- Assert.assertTrue( client.changed.contains( x ) );
- Assert.assertEquals( 1, client.changed.size() );
+ Thread.sleep( 100 );
+ Assert.assertEquals( set( x ), client.changed );
}
/** @throws Exception */
@@ -1137,17 +1130,15 @@
Object y = c.getConfigPath( r, "primes" );
Assert.assertNotNull( y );
- Assert.assertFalse( client.changed.contains( x ) );
- Assert.assertFalse( client.changed.contains( y ) );
- Assert.assertEquals( 0, client.changed.size() );
+ Assert.assertEquals( set(), client.changed );
c.subscribe( x );
- c.subscribe( y );
- Thread.sleep( 1000 );
+ Thread.sleep( 100 );
+ Assert.assertEquals( set( x ), client.changed );
- Assert.assertTrue( client.changed.contains( x ) );
- Assert.assertTrue( client.changed.contains( y ) );
- Assert.assertEquals( 2, client.changed.size() );
+ c.subscribe( y );
+ Thread.sleep( 100 );
+ Assert.assertEquals( set( y ), client.changed );
}
/** @throws Exception */
@@ -1616,6 +1607,38 @@
Assert.assertEquals( list( "fish", map( "a", 5, "b", 6, "c", 7 ), list( 2, 3, 4 ), "bear" ), c.getListPath( root, "list", null ) );
}
+ /** @throws Exception */
+ @Test
+ public void subscribeUpdate0() throws Exception
+ {
+ setupUpdate( UPDATE+0, "." );
+ Assert.assertEquals( set(), client.changed );
+ }
+
+ /** @throws Exception */
+ @Test
+ public void subscribeUpdate1a() throws Exception
+ {
+ YamlConfig c = setupUpdate( UPDATE+1, "." );
+ Assert.assertEquals( set( c.getConfigPath( c.getRoot(), "." ) ), client.changed );
+ }
+
+ /** @throws Exception */
+ @Test
+ public void subscribeUpdate1b() throws Exception
+ {
+ YamlConfig c = setupUpdate( UPDATE+1, "foo" );
+ Assert.assertEquals( set( c.getConfigPath( c.getRoot(), "foo" ) ), client.changed );
+ }
+
+ /** @throws Exception */
+ @Test
+ public void subscribeUpdate1c() throws Exception
+ {
+ setupUpdate( UPDATE+1, "bar" );
+ Assert.assertEquals( set(), client.changed );
+ }
+
private void testLoadConfig( String name, boolean expected )
throws Exception
{
@@ -1642,14 +1665,14 @@
Assert.assertEquals( size, c.size( node ) );
}
- private YamlConfig setupUpdate( String updateConfig ) throws Exception
+ private YamlConfig setupUpdate( String updateConfig, String path ) throws Exception
{
YamlConfig c = new YamlConfig( client, UPDATE+0 );
client.setServer( c );
Object root = c.getRoot();
c.setInterval( 50 );
- c.subscribe( root );
+ c.subscribePath( root, path );
c.setConfig( updateConfig );
Thread.sleep( 1000 );
c.unsubscribeAll();
@@ -1657,6 +1680,11 @@
return c;
}
+ private YamlConfig setupUpdate( String updateConfig ) throws Exception
+ {
+ return setupUpdate( updateConfig, "." );
+ }
+
private List<?> list( Object ... objs )
{
List<Object> list = new ArrayList<Object>( objs.length );
@@ -1674,19 +1702,29 @@
return map;
}
+ private Set<?> set( Object ... objs )
+ {
+ Set<Object> set = new HashSet<Object>();
+ for (Object o: objs)
+ set.add( o );
+ return set;
+ }
+
private static class MyConfigurationClient implements ConfigurationClient
{
public void configValuesChanged( Object[] updated )
{
+ Set<Object> changed = new HashSet<Object>();
for (Object id: updated)
{
System.out.println( "id changed: "+id );
System.out.println( "path changed: "+server.getPath( id ) );
changed.add( id );
}
+ this.changed = changed;
}
- private final Set<Object> changed = Collections.synchronizedSet( new HashSet<Object>() );
+ private Set<Object> changed = new HashSet<Object>();
public void setServer( ConfigurationServer server )
{