[dna-commits] DNA SVN: r1399 - in trunk: dna-graph/src/main/java/org/jboss/dna/graph/observe and 5 other directories.

dna-commits at lists.jboss.org dna-commits at lists.jboss.org
Fri Dec 4 01:56:16 EST 2009


Author: rhauch
Date: 2009-12-04 01:56:15 -0500 (Fri, 04 Dec 2009)
New Revision: 1399

Removed:
   trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/ObservedId.java
Modified:
   trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/federation/JoinRequestProcessor.java
   trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/ChangeObserver.java
   trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/Changes.java
   trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/NetChangeObserver.java
   trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/ObservationBus.java
   trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/Observer.java
   trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/SetPropertyRequest.java
   trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrObservationManager.java
   trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrRepository.java
   trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java
   trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrObservationManagerTest.java
   trunk/dna-repository/src/main/java/org/jboss/dna/repository/RepositoryService.java
   trunk/dna-repository/src/main/java/org/jboss/dna/repository/sequencer/SequencingService.java
Log:
DNA-456 I've attached a patch that fixes all of the problems, so now all of the Observation tests should be passing (except for the still missing deep events upon delete and clone).

The major problem was in how the JcrRepository.RepositoryObservationManager.notify(...) method was behaving.  This Observer is actually listening to the RepositoryLibrary's ObservationBus instance, which is the observer for all of the RepositorySource objects, and the 'notify(...)' method is being called within the same thread as the RepositorySource is processing the requests (and firing the Changes).  The whole purpose of the JcrRepository.RepositoryObservationManager is to quickly enqueue the Changes for processing in another thread, to get off of the thread used by the connector (so the request processing can be completed).  It turns out that the 'notify(...)' method was passing the list of session observers into the Runnable.  BUT THIS LIST CHANGES WHENEVER A NEW JCR LISTENER IS ADDED OR REMOVED. As a result, the changes may get sent to listeners that were registered AFTER the save().  This is the whole reason why the timestamp checking (originally) and ObservedId (more!
  recently) were needed.

The fix was pretty simple.  Rather than pass the list of session observers to the Runnable, the RepositoryObservationManager should grab a snapshot of the list.  This is trivial to do, since the list is a CopyOnWriteArrayList: simply get an Iterator<Observer> from the list and pass THAT to the Runnable.  Thus, the snapshot of the observer list is made within the same thread where the connector is running, which is within the context of the save().  Any listener added after the save completes will not be in the snapshot used to fire off the changes.

In other words, instead of this:

            final List<Observer> observersList = observers;
            Runnable sender = new Runnable() {
                public void run() {
                    for (Observer observer: observersList ) {
                        observer.notify(changes);
                    }
                }
            };
            this.observerService.execute(sender);

the method should be this:

            final Iterator<Observer> observerIterator = observers.iterator();
            Runnable sender = new Runnable() {
                public void run() {
                    while (observerIterator.hasNext()) {
                        Observer observer = observerIterator.next();
                        observer.notify(changes);
                    }
                }
            };
            this.observerService.execute(sender);

Another improvement was added to the 'notify(...)' method.  Since the JcrRepository only cares about one source (usually the federated source), it can safely ignore all Changes objects that are from other sources.  Doing this saves work being added to the worker queue.

After this change was made, there was no more need for the ObservedId, so that (and all uses of it) was removed.

One minor problem was that SetPropertyRequest.setNewProperty(boolean) was always setting the 'is new' property to true, rather than to the supplied value.  This was always resulting in a PROPERTY_ADDED event rather than a PROPERTY_CHANGED when the property is not new.

Another issue was that the Federated Connector was not always firing the events in the correct order.  I don't think this mattered in the results, but this was fixed.

Finally, the tests in the sequencer examples were failing because the SequencingService was processing the NetChange.getModifiedProperties().  Obviously this missed any ADDED property.  Therefore, a 'getAddOrModifiedProperties()' method was added to the NetChange class, and the SequencingService changed to use this method.

These changes were committed, and the recently ignored tests were added back in.

Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/federation/JoinRequestProcessor.java
===================================================================
--- trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/federation/JoinRequestProcessor.java	2009-12-03 23:30:42 UTC (rev 1398)
+++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/federation/JoinRequestProcessor.java	2009-12-04 06:56:15 UTC (rev 1399)
@@ -49,6 +49,7 @@
 import org.jboss.dna.graph.property.ValueComparators;
 import org.jboss.dna.graph.request.AccessQueryRequest;
 import org.jboss.dna.graph.request.CacheableRequest;
+import org.jboss.dna.graph.request.ChangeRequest;
 import org.jboss.dna.graph.request.CloneBranchRequest;
 import org.jboss.dna.graph.request.CloneWorkspaceRequest;
 import org.jboss.dna.graph.request.CopyBranchRequest;
@@ -102,7 +103,8 @@
         // this.repository = repository;
         this.propertyFactory = context.getPropertyFactory();
         this.pathFactory = context.getValueFactories().getPathFactory();
-        this.mirrorProcessor = new JoinMirrorRequestProcessor(repository.getSourceName(), context, observer, now,
+        // The mirror processor should never send anything to an observer, since all requests go to this processor's observer
+        this.mirrorProcessor = new JoinMirrorRequestProcessor(repository.getSourceName(), context, null, now,
                                                               repository.getDefaultCachePolicy());
     }
 
@@ -184,6 +186,10 @@
             }
             mirrorProcessor.setFederatedRequest(forked);
             mirrorProcessor.process(original);
+            // If this is a change request, record it on this processor so it goes to the observer ...
+            if (original instanceof ChangeRequest && !original.hasError() && !original.isCancelled()) {
+                recordChange((ChangeRequest)original);
+            }
         } else {
             this.federatedRequest = forked;
             process(original);

Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/ChangeObserver.java
===================================================================
--- trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/ChangeObserver.java	2009-12-03 23:30:42 UTC (rev 1398)
+++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/ChangeObserver.java	2009-12-04 06:56:15 UTC (rev 1399)
@@ -41,20 +41,9 @@
 public abstract class ChangeObserver implements Observer {
 
     private final CopyOnWriteArraySet<ChangeSourceReference> sources = new CopyOnWriteArraySet<ChangeSourceReference>();
-    private final ObservedId id;
 
     protected ChangeObserver() {
-        this.id = new ObservedId();
     }
-    
-    /**
-     * {@inheritDoc}
-     *
-     * @see org.jboss.dna.graph.observe.Observer#getId()
-     */
-    public final ObservedId getId() {
-        return this.id;
-    }
 
     /**
      * Records that this listener has successfully registered by the supplied {@link Observable}.

Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/Changes.java
===================================================================
--- trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/Changes.java	2009-12-03 23:30:42 UTC (rev 1398)
+++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/Changes.java	2009-12-04 06:56:15 UTC (rev 1399)
@@ -40,7 +40,6 @@
 
     private static final long serialVersionUID = 1L;
 
-    protected final ObservedId id;
     protected final String processId;
     protected final String contextId;
     protected final String userName;
@@ -56,7 +55,6 @@
                     List<ChangeRequest> requests ) {
         assert requests != null;
         assert !requests.isEmpty();
-        this.id = new ObservedId();
         this.userName = userName;
         this.sourceName = sourceName;
         this.timestamp = timestamp;
@@ -71,7 +69,6 @@
     }
 
     protected Changes( Changes changes ) {
-        this.id = new ObservedId();
         this.userName = changes.userName;
         this.sourceName = changes.sourceName;
         this.timestamp = changes.timestamp;
@@ -85,13 +82,6 @@
         assert this.processId != null;
         assert this.contextId != null;
     }
-    
-    /**
-     * @return the unique ID of these changes
-     */
-    public ObservedId getId() {
-        return this.id;
-    }
 
     /**
      * Get the user that made these changes.

Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/NetChangeObserver.java
===================================================================
--- trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/NetChangeObserver.java	2009-12-03 23:30:42 UTC (rev 1398)
+++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/NetChangeObserver.java	2009-12-04 06:56:15 UTC (rev 1399)
@@ -78,7 +78,7 @@
 
     protected NetChangeObserver() {
     }
-    
+
     /**
      * @param workspace the workspace of the location (never <code>null</code>)
      * @param location the location whose details are being deleted (never <code>null</code>)
@@ -163,7 +163,7 @@
                 }
             } else if (change instanceof SetPropertyRequest) {
                 SetPropertyRequest set = (SetPropertyRequest)change;
-                
+
                 if (set.isNewProperty()) {
                     details.addProperty(set.property());
                 } else {
@@ -197,7 +197,8 @@
             } else if (change instanceof CopyBranchRequest) {
                 details.addEventType(ChangeType.NODE_ADDED);
             } else if (change instanceof MoveBranchRequest) {
-                // the old location is a removed node event and if it is the same location as the original location it is a reorder
+                // the old location is a removed node event and if it is the same location as the original location it is a
+                // reorder
                 Location original = ((MoveBranchRequest)change).getActualLocationBefore();
                 NetChangeDetails originalDetails = findDetailsByLocation(workspace, original, detailsByLocationByWorkspace);
                 originalDetails.addEventType(ChangeType.NODE_REMOVED);
@@ -206,13 +207,13 @@
                 details.addEventType(ChangeType.NODE_ADDED);
             } else if (change instanceof CloneBranchRequest) {
                 CloneBranchRequest cloneRequest = (CloneBranchRequest)change;
-                
+
                 // create event details for any nodes that were removed
                 for (Location removed : cloneRequest.getRemovedNodes()) {
                     NetChangeDetails removedDetails = findDetailsByLocation(workspace, removed, detailsByLocationByWorkspace);
                     removedDetails.addEventType(ChangeType.NODE_REMOVED);
                 }
-                
+
                 // create event details for new node
                 details.addEventType(ChangeType.NODE_ADDED);
             } else if (change instanceof RenameNodeRequest) {
@@ -226,7 +227,7 @@
             } else if (change instanceof UpdateValuesRequest) {
                 // TODO need to know if this is a new property
                 UpdateValuesRequest updateValuesRequest = (UpdateValuesRequest)change;
-                
+
                 if (!updateValuesRequest.addedValues().isEmpty() || !updateValuesRequest.removedValues().isEmpty()) {
                     details.addEventType(ChangeType.PROPERTY_CHANGED);
                     // TODO need to set property like details.changeProperty(property);
@@ -319,6 +320,7 @@
         private final EnumSet<ChangeType> eventTypes;
         private final Set<Property> addedProperties;
         private final Set<Property> modifiedProperties;
+        private final Set<Property> addedOrModifiedProperties;
         private final Set<Name> removedProperties;
         private final int hc;
 
@@ -334,12 +336,29 @@
             this.location = location;
             this.hc = HashCode.compute(this.workspaceName, this.location);
             this.eventTypes = eventTypes;
-            if (addedProperties == null) addedProperties = Collections.emptySet();
-            if (modifiedProperties == null) modifiedProperties = Collections.emptySet();
+            Set<Property> addedOrModified = null;
+            if (addedProperties == null) {
+                addedProperties = Collections.emptySet();
+                addedOrModified = modifiedProperties; // may be null
+            } else {
+                addedOrModified = addedProperties;
+            }
+            if (modifiedProperties == null) {
+                if (addedOrModified == null) addedOrModified = Collections.emptySet();
+                modifiedProperties = Collections.emptySet();
+            } else {
+                if (addedOrModified == null) {
+                    addedOrModified = modifiedProperties;
+                } else {
+                    addedOrModified = new HashSet<Property>(modifiedProperties);
+                    addedOrModified.addAll(addedProperties);
+                }
+            }
             if (removedProperties == null) removedProperties = Collections.emptySet();
             this.addedProperties = Collections.unmodifiableSet(addedProperties);
             this.modifiedProperties = Collections.unmodifiableSet(modifiedProperties);
             this.removedProperties = Collections.unmodifiableSet(removedProperties);
+            this.addedOrModifiedProperties = Collections.unmodifiableSet(addedOrModified);
         }
 
         /**
@@ -378,6 +397,15 @@
         }
 
         /**
+         * Get the combination of {@link #getAddedProperties() added} and {@link #getModifiedProperties() modified} properties.
+         * 
+         * @return the immutable set of properties that were added or modified; never null but possibly empty
+         */
+        public Set<Property> getAddedOrModifiedProperties() {
+            return this.addedOrModifiedProperties;
+        }
+
+        /**
          * @return removedProperties
          */
         public Set<Name> getRemovedProperties() {
@@ -530,7 +558,7 @@
                 if (property.getName().equals(propertyName)) {
                     handled = true;
                     this.addedProperties.remove(property);
-                    
+
                     // get rid of event type if no longer applicable
                     if (this.addedProperties.isEmpty()) {
                         this.eventTypes.remove(ChangeType.PROPERTY_ADDED);
@@ -545,7 +573,7 @@
                 for (Property property : this.modifiedProperties) {
                     if (property.getName().equals(propertyName)) {
                         this.modifiedProperties.remove(property);
-                        
+
                         // get rid of event type if no longer applicable
                         if (this.modifiedProperties.isEmpty()) {
                             this.eventTypes.remove(ChangeType.PROPERTY_CHANGED);

Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/ObservationBus.java
===================================================================
--- trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/ObservationBus.java	2009-12-03 23:30:42 UTC (rev 1398)
+++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/ObservationBus.java	2009-12-04 06:56:15 UTC (rev 1399)
@@ -32,24 +32,12 @@
 @ThreadSafe
 public class ObservationBus implements Observable, Observer {
     private final ChangeObservers observers = new ChangeObservers();
-    
-    private final ObservedId id;
 
     public ObservationBus() {
-        this.id = new ObservedId();
     }
 
     /**
      * {@inheritDoc}
-     *
-     * @see org.jboss.dna.graph.observe.Observer#getId()
-     */
-    public ObservedId getId() {
-        return this.id;
-    }
-
-    /**
-     * {@inheritDoc}
      * 
      * @see org.jboss.dna.graph.observe.Observable#register(org.jboss.dna.graph.observe.Observer)
      */

Deleted: trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/ObservedId.java
===================================================================
--- trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/ObservedId.java	2009-12-03 23:30:42 UTC (rev 1398)
+++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/ObservedId.java	2009-12-04 06:56:15 UTC (rev 1399)
@@ -1,61 +0,0 @@
-/*
- * JBoss DNA (http://www.jboss.org/dna)
- * See the COPYRIGHT.txt file distributed with this work for information
- * regarding copyright ownership.  Some portions may be licensed
- * to Red Hat, Inc. under one or more contributor license agreements.
- * See the AUTHORS.txt file in the distribution for a full listing of 
- * individual contributors.
- *
- * JBoss DNA is free software. Unless otherwise indicated, all code in JBoss DNA
- * is licensed to you under the terms of the GNU Lesser General Public License as
- * published by the Free Software Foundation; either version 2.1 of
- * the License, or (at your option) any later version.
- * 
- * JBoss DNA is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this software; if not, write to the Free
- * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
- * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
- */
-package org.jboss.dna.graph.observe;
-
-import java.io.Serializable;
-import java.util.concurrent.atomic.AtomicLong;
-import net.jcip.annotations.Immutable;
-
-/**
- * A unique identifier for an event or observer that can be compared with IDs created before or after this ID.
- */
- at Immutable
-public final class ObservedId implements Serializable {
-
-    private static final long serialVersionUID = 1L;
-
-    private static final AtomicLong idSequencer = new AtomicLong(0);
-    
-    private static long getNextId() {
-        return idSequencer.getAndIncrement();
-    }
-    
-    private final long id;
-    
-    /**
-     * Constructs a unique ID.
-     */
-    public ObservedId() {
-        this.id = getNextId();
-    }
-    
-    /**
-     * @param otherId the ID being compared to
-     * @return <code>true</code> if this ID sequentially comes before the other ID
-     */
-    public boolean isBefore(ObservedId otherId) {
-        return (this.id < otherId.id);
-    }
-
-}

Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/Observer.java
===================================================================
--- trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/Observer.java	2009-12-03 23:30:42 UTC (rev 1398)
+++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/observe/Observer.java	2009-12-04 06:56:15 UTC (rev 1399)
@@ -29,14 +29,6 @@
 public interface Observer {
 
     /**
-     * The ID that uniquely identifies this observer. This ID can be used to determine if {@link Changes changes} came before or
-     * after this observer was created.
-     * 
-     * @return the unique observer identifier (never <code>null</code>)
-     */
-    ObservedId getId();
-
-    /**
      * Method that is called for each {@link Changes set of changes} from the {@link Observable} instance(s) with which this
      * observer is registered.
      * 

Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/SetPropertyRequest.java
===================================================================
--- trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/SetPropertyRequest.java	2009-12-03 23:30:42 UTC (rev 1398)
+++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/SetPropertyRequest.java	2009-12-04 06:56:15 UTC (rev 1399)
@@ -140,7 +140,7 @@
      * @throws IllegalStateException if the request is frozen
      */
     public void setNewProperty( boolean created ) {
-        this.actualCreation = true;
+        this.actualCreation = created;
     }
 
     /**

Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrObservationManager.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrObservationManager.java	2009-12-03 23:30:42 UTC (rev 1398)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrObservationManager.java	2009-12-04 06:56:15 UTC (rev 1399)
@@ -539,7 +539,6 @@
                 // don't accept unless IDs are different
                 return !getSessionId().equals(changes.getContextId());
             }
-
             return true;
         }
 
@@ -661,12 +660,6 @@
          */
         @Override
         public void notify( Changes changes ) {
-
-            // don't process if changes occurred before this listener was registered
-            if (changes.getId().isBefore(getId())) {
-                return;
-            }
-
             // check source first
             if (!acceptBasedOnEventSource(changes)) {
                 return;

Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrRepository.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrRepository.java	2009-12-03 23:30:42 UTC (rev 1398)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrRepository.java	2009-12-04 06:56:15 UTC (rev 1399)
@@ -33,6 +33,7 @@
 import java.util.Collections;
 import java.util.EnumMap;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -73,7 +74,6 @@
 import org.jboss.dna.graph.connector.inmemory.InMemoryRepositorySource;
 import org.jboss.dna.graph.observe.Changes;
 import org.jboss.dna.graph.observe.Observable;
-import org.jboss.dna.graph.observe.ObservedId;
 import org.jboss.dna.graph.observe.Observer;
 import org.jboss.dna.graph.property.Name;
 import org.jboss.dna.graph.property.NamespaceRegistry;
@@ -280,7 +280,7 @@
      * @param repositoryObservable the repository library observable associated with this repository (never <code>null</code>)
      * @param descriptors the {@link #getDescriptorKeys() descriptors} for this repository; may be <code>null</code>.
      * @param options the optional {@link Option settings} for this repository; may be null
-     * @throws IllegalArgumentException If <code>executionContext</code>, <code>connectionFactory</code>, 
+     * @throws IllegalArgumentException If <code>executionContext</code>, <code>connectionFactory</code>,
      *         <code>repositorySourceName</code>, or <code>repositoryObservable</code> is <code>null</code>.
      */
     public JcrRepository( ExecutionContext executionContext,
@@ -557,7 +557,20 @@
         return sourceName;
     }
 
+    String getSystemSourceName() {
+        return systemSourceName;
+    }
+
     /**
+     * Get the name of the source that we want to observe.
+     * 
+     * @return the name of the source that should be observed; never null
+     */
+    String getObservableSourceName() {
+        return WORKSPACES_SHARE_SYSTEM_BRANCH ? federatedSource.getName() : sourceName;
+    }
+
+    /**
      * @return executionContext
      */
     ExecutionContext getExecutionContext() {
@@ -796,16 +809,6 @@
         return lockManager;
     }
 
-    /**
-     * Returns the name of this repository
-     * 
-     * @return the name of this repository
-     * @see #sourceName
-     */
-    String getName() {
-        return this.sourceName;
-    }
-
     protected class FederatedRepositoryContext implements RepositoryContext {
         private final RepositoryConnectionFactory connectionFactory;
 
@@ -941,22 +944,11 @@
     }
 
     protected class RepositoryObservationManager implements Observable, Observer {
-        
+
         private final ExecutorService observerService = Executors.newSingleThreadExecutor();
         private final CopyOnWriteArrayList<Observer> observers = new CopyOnWriteArrayList<Observer>();
-        private final ObservedId id;
-        
-        public RepositoryObservationManager() {
-            this.id = new ObservedId();
-        }
 
-        /**
-         * {@inheritDoc}
-         *
-         * @see org.jboss.dna.graph.observe.Observer#getId()
-         */
-        public ObservedId getId() {
-            return this.id;
+        protected RepositoryObservationManager() {
         }
 
         /**
@@ -965,22 +957,35 @@
          * @see org.jboss.dna.graph.observe.Observer#notify(org.jboss.dna.graph.observe.Changes)
          */
         public void notify( final Changes changes ) {
-            final List<Observer> listeners = observers;
-            
-            Runnable command = new Runnable() {
+            // We only care about events that come from the federated source ...
+            if (!changes.getSourceName().equals(getObservableSourceName())) return;
+
+            // We're still in the thread where the connector published its changes,
+            // so we need to create a runnable that will send these changes to all
+            // of the observers <i>at this moment</i>. Because 'observers' is
+            // a CopyOnWriteArrayList, we can't old onto the list (because the list's content
+            // might change). Instead, hold onto the Iterator over the listeners,
+            // and that will be a snapshot of the listeners <i>at this moment</i>
+            if (observers.isEmpty()) return;
+            final Iterator<Observer> observerIterator = observers.iterator();
+
+            Runnable sender = new Runnable() {
                 public void run() {
-                    for (Observer observer : listeners) {
+                    while (observerIterator.hasNext()) {
+                        Observer observer = observerIterator.next();
+                        assert observer != null;
                         observer.notify(changes);
                     }
-                }                
+                }
             };
 
-            this.observerService.execute(command);
+            // Now let the executor service run this in another thread ...
+            this.observerService.execute(sender);
         }
 
         /**
          * {@inheritDoc}
-         *
+         * 
          * @see org.jboss.dna.graph.observe.Observable#register(org.jboss.dna.graph.observe.Observer)
          */
         public boolean register( Observer observer ) {
@@ -990,7 +995,7 @@
 
         /**
          * {@inheritDoc}
-         *
+         * 
          * @see org.jboss.dna.graph.observe.Observable#unregister(org.jboss.dna.graph.observe.Observer)
          */
         public boolean unregister( Observer observer ) {
@@ -998,5 +1003,4 @@
             return this.observers.remove(observer);
         }
     }
-
 }

Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java	2009-12-03 23:30:42 UTC (rev 1398)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java	2009-12-04 06:56:15 UTC (rev 1399)
@@ -335,8 +335,8 @@
                            String workspaceName ) {
         SecurityContext context = getExecutionContext().getSecurityContext();
 
-        return context.hasRole(roleName) || context.hasRole(roleName + "." + this.repository.getName())
-               || context.hasRole(roleName + "." + this.repository.getName() + "." + workspaceName);
+        return context.hasRole(roleName) || context.hasRole(roleName + "." + this.repository.getRepositorySourceName())
+               || context.hasRole(roleName + "." + this.repository.getRepositorySourceName() + "." + workspaceName);
     }
 
     /**

Modified: trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrObservationManagerTest.java
===================================================================
--- trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrObservationManagerTest.java	2009-12-03 23:30:42 UTC (rev 1398)
+++ trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrObservationManagerTest.java	2009-12-04 06:56:15 UTC (rev 1399)
@@ -168,8 +168,10 @@
         final String SOURCE = "store";
 
         this.config = new JcrConfiguration();
-        this.config.repositorySource("store").usingClass(InMemoryRepositorySource.class).setRetryLimit(100).setProperty("defaultWorkspaceName",
-                                                                                                                        WORKSPACE);
+        this.config.repositorySource("store")
+                   .usingClass(InMemoryRepositorySource.class)
+                   .setRetryLimit(100)
+                   .setProperty("defaultWorkspaceName", WORKSPACE);
         this.config.repository(REPOSITORY).setSource(SOURCE).setOption(Option.JAAS_LOGIN_CONFIG_NAME, "dna-jcr");
         this.config.save();
 
@@ -187,13 +189,15 @@
     }
 
     void checkResults( TestListener listener ) {
-        if ( listener.getActualEventCount() != listener.getExpectedEventCount() ) {
+        if (listener.getActualEventCount() != listener.getExpectedEventCount()) {
             // Wrong number ...
             StringBuilder sb = new StringBuilder(" Actual events were: ");
-            for ( Event event : listener.getEvents() ) {
+            for (Event event : listener.getEvents()) {
                 sb.append('\n').append(event);
             }
-            assertThat("Received incorrect number of events."+ sb.toString(), listener.getActualEventCount(), is(listener.getExpectedEventCount()));
+            assertThat("Received incorrect number of events." + sb.toString(),
+                       listener.getActualEventCount(),
+                       is(listener.getExpectedEventCount()));
             assertThat(listener.getErrorMessage(), listener.getErrorMessage(), is(nullValue()));
         }
     }
@@ -293,10 +297,12 @@
     public void shouldReceiveNodeAddedEventWhenRegisteredToReceiveAllEvents() throws Exception {
         System.out.println("shouldReceiveNodeAddedEventWhenRegisteredToReceiveAllEvents");
         // register listener (add + 3 property events)
+        Node root = getRoot();
+        save();
         TestListener listener = addListener(4, ALL_EVENTS, null, false, null, null, false);
 
         // add node
-        Node addedNode = getRoot().addNode("node1", UNSTRUCTURED);
+        Node addedNode = root.addNode("node1", UNSTRUCTURED);
         save();
 
         // event handling
@@ -591,9 +597,8 @@
 
         // make sure same listener isn't added again
         getObservationManager().addEventListener(listener, ALL_EVENTS, null, false, null, null, false);
-        assertThat("The same listener should not be added more than once.",
-                   getObservationManager().getRegisteredEventListeners().getSize(),
-                   is(2L));
+        assertThat("The same listener should not be added more than once.", getObservationManager().getRegisteredEventListeners()
+                                                                                                   .getSize(), is(2L));
     }
 
     /**
@@ -1174,8 +1179,8 @@
 
         // tests
         checkResults(listener);
-        assertTrue("Path for jrc:primaryType property was not found.",
-                   containsPath(listener, node.getProperty("jcr:primaryType").getPath()));
+        assertTrue("Path for jrc:primaryType property was not found.", containsPath(listener, node.getProperty("jcr:primaryType")
+                                                                                                  .getPath()));
     }
 
     // ===========================================================================================================================

Modified: trunk/dna-repository/src/main/java/org/jboss/dna/repository/RepositoryService.java
===================================================================
--- trunk/dna-repository/src/main/java/org/jboss/dna/repository/RepositoryService.java	2009-12-03 23:30:42 UTC (rev 1398)
+++ trunk/dna-repository/src/main/java/org/jboss/dna/repository/RepositoryService.java	2009-12-04 06:56:15 UTC (rev 1399)
@@ -43,7 +43,6 @@
 import org.jboss.dna.graph.connector.RepositorySource;
 import org.jboss.dna.graph.observe.Changes;
 import org.jboss.dna.graph.observe.NetChangeObserver;
-import org.jboss.dna.graph.observe.ObservedId;
 import org.jboss.dna.graph.observe.Observer;
 import org.jboss.dna.graph.property.Name;
 import org.jboss.dna.graph.property.Path;
@@ -113,7 +112,6 @@
         }
     }
 
-    private final ObservedId id;
     private final ExecutionContext context;
     private final RepositoryLibrary sources;
     private final String configurationSourceName;
@@ -150,7 +148,6 @@
         if (problems == null) problems = new SimpleProblems();
         Path sourcesPath = pathFactory.create(pathToConfigurationRoot, DnaLexicon.SOURCES);
 
-        this.id = new ObservedId();
         this.sources = new RepositoryLibrary(configurationSource, configurationWorkspaceName, sourcesPath, context);
         this.sources.addSource(configurationSource);
         this.pathToConfigurationRoot = pathToConfigurationRoot;
@@ -163,16 +160,7 @@
 
     /**
      * {@inheritDoc}
-     *
-     * @see org.jboss.dna.graph.observe.Observer#getId()
      */
-    public ObservedId getId() {
-        return this.id;
-    }
-
-    /**
-     * {@inheritDoc}
-     */
     public final ServiceAdministrator getAdministrator() {
         return this.administrator;
     }

Modified: trunk/dna-repository/src/main/java/org/jboss/dna/repository/sequencer/SequencingService.java
===================================================================
--- trunk/dna-repository/src/main/java/org/jboss/dna/repository/sequencer/SequencingService.java	2009-12-03 23:30:42 UTC (rev 1398)
+++ trunk/dna-repository/src/main/java/org/jboss/dna/repository/sequencer/SequencingService.java	2009-12-04 06:56:15 UTC (rev 1399)
@@ -417,7 +417,7 @@
                     for (Sequencer sequencer : allSequencers) {
                         final SequencerConfig config = sequencer.getConfiguration();
                         for (SequencerPathExpression pathExpression : config.getPathExpressions()) {
-                            for (Property property : change.getModifiedProperties()) {
+                            for (Property property : change.getAddedOrModifiedProperties()) {
                                 Name propertyName = property.getName();
                                 String propertyNameStr = context.getValueFactories().getStringFactory().create(propertyName);
                                 String path = nodePathStr + "/@" + propertyNameStr;



More information about the dna-commits mailing list