[dna-issues] [JBoss JIRA] Updated: (DNA-456) Add JSR-170 Observation Optional Feature

[Randall Hauch (JIRA)]

     [ https://jira.jboss.org/jira/browse/DNA-456?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Randall Hauch updated DNA-456:
------------------------------

    Attachment: DNA-456-fix-concurrency-issues.patch


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.

> Add JSR-170 Observation Optional Feature
> ----------------------------------------
>
>                 Key: DNA-456
>                 URL: https://jira.jboss.org/jira/browse/DNA-456
>             Project: DNA
>          Issue Type: Feature Request
>          Components: JCR
>    Affects Versions: 0.5
>            Reporter: Brian Carothers
>            Assignee: Dan Florian
>             Fix For: 0.7
>
>         Attachments: DNA-456-dna-graph.patch, DNA-456-dna-jcr.patch, DNA-456-dna-repository.patch, DNA-456-fix-concurrency-issues.patch
>
>
> JSR-170 defines several optional features.  The observation feature would provide notification of node and property changes to registered listeners.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira