You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ju...@apache.org on 2014/01/28 22:14:06 UTC
svn commit: r1562233 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation:
ChangeProcessor.java EventGenerator.java EventQueue.java QueueingHandler.java
Author: jukka
Date: Tue Jan 28 21:14:06 2014
New Revision: 1562233
URL: http://svn.apache.org/r1562233
Log:
OAK-1133: Observation listener PLUS
Extract JCR-specific bits away from EventGenerator
Added:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/EventQueue.java (with props)
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/QueueingHandler.java (with props)
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/EventGenerator.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java?rev=1562233&r1=1562232&r2=1562233&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java Tue Jan 28 21:14:06 2014
@@ -157,7 +157,7 @@ public class ChangeProcessor implements
String basePath = provider.getPath();
EventFilter userFilter = provider.getFilter(previousRoot, root);
EventFilter acFilter = new ACFilter(previousRoot, root, permissionProvider, basePath);
- EventGenerator events = new EventGenerator(
+ EventQueue events = new EventQueue(
namePathMapper, info, previousRoot, root, basePath,
Filters.all(userFilter, acFilter));
if (events.hasNext() && runningMonitor.enterIf(running)) {
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/EventGenerator.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/EventGenerator.java?rev=1562233&r1=1562232&r2=1562233&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/EventGenerator.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/EventGenerator.java Tue Jan 28 21:14:06 2014
@@ -18,15 +18,8 @@
*/
package org.apache.jackrabbit.oak.plugins.observation;
-import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.Lists.newArrayList;
import static com.google.common.collect.Lists.newLinkedList;
-import static javax.jcr.observation.Event.NODE_ADDED;
-import static javax.jcr.observation.Event.NODE_MOVED;
-import static javax.jcr.observation.Event.NODE_REMOVED;
-import static javax.jcr.observation.Event.PROPERTY_ADDED;
-import static javax.jcr.observation.Event.PROPERTY_CHANGED;
-import static javax.jcr.observation.Event.PROPERTY_REMOVED;
import static org.apache.jackrabbit.oak.api.Type.NAMES;
import static org.apache.jackrabbit.oak.api.Type.STRING;
import static org.apache.jackrabbit.oak.core.AbstractTree.OAK_CHILD_ORDER;
@@ -35,63 +28,47 @@ import static org.apache.jackrabbit.oak.
import java.util.LinkedList;
import java.util.List;
-import java.util.Map;
-import java.util.NoSuchElementException;
import javax.annotation.Nonnull;
-import javax.jcr.observation.Event;
-import javax.jcr.observation.EventIterator;
import org.apache.jackrabbit.oak.api.PropertyState;
-import org.apache.jackrabbit.oak.commons.PathUtils;
-import org.apache.jackrabbit.oak.core.ImmutableTree;
-import org.apache.jackrabbit.oak.namepath.NamePathMapper;
-import org.apache.jackrabbit.oak.plugins.observation.filter.EventFilter;
-import org.apache.jackrabbit.oak.plugins.observation.filter.Filters;
-import org.apache.jackrabbit.oak.plugins.observation.filter.VisibleFilter;
import org.apache.jackrabbit.oak.plugins.observation.handler.ChangeHandler;
-import org.apache.jackrabbit.oak.plugins.observation.handler.FilteredHandler;
-import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.apache.jackrabbit.oak.spi.state.NodeStateDiff;
-import com.google.common.collect.ImmutableMap;
-
/**
* Generator of a traversable view of events.
*/
-public class EventGenerator implements EventIterator {
-
- private final EventContext context;
-
- private final LinkedList<Event> events = newLinkedList();
- private final LinkedList<Runnable> generators = newLinkedList();
+public class EventGenerator {
- private long position = 0;
+ private final LinkedList<Runnable> continuations = newLinkedList();
/**
* Create a new instance of a {@code EventGenerator} reporting events to the
* passed {@code listener} after filtering with the passed {@code filter}.
- *
- * @param filter filter for filtering changes
*/
public EventGenerator(
- @Nonnull NamePathMapper namePathMapper, CommitInfo info,
@Nonnull NodeState before, @Nonnull NodeState after,
- @Nonnull String basePath, @Nonnull EventFilter filter) {
- this.context = new EventContext(namePathMapper, info);
+ @Nonnull ChangeHandler handler) {
+ continuations.add(new DiffContinuation(handler, before, after));
+ }
- filter = Filters.all(new VisibleFilter(), checkNotNull(filter));
+ public boolean isComplete() {
+ return continuations.isEmpty();
+ }
- new EventDiff(before, after, basePath, filter).run();
+ public void generate() {
+ if (!continuations.isEmpty()) {
+ continuations.removeFirst().run();
+ }
}
- private class EventDiff implements NodeStateDiff, Runnable {
+ private class DiffContinuation implements NodeStateDiff, Runnable {
/**
* The diff handler of the parent node, or {@code null} for the root.
*/
- private final EventDiff parent;
+ private final DiffContinuation parent;
/**
* The name of this node, or the empty string for the root.
@@ -113,27 +90,16 @@ public class EventGenerator implements E
*/
private final ChangeHandler handler;
- EventDiff(NodeState before, NodeState after, String path,
- EventFilter filter) {
- String name = null;
- ChangeHandler handler = new Handler(
- new ImmutableTree(before), new ImmutableTree(after));
- for (String element : PathUtils.elements(path)) {
- name = element;
- before = before.getChildNode(name);
- after = after.getChildNode(name);
- handler = handler.getChildHandler(name, before, after);
- }
-
+ DiffContinuation(ChangeHandler handler, NodeState before, NodeState after) {
this.parent = null;
- this.name = name;
+ this.name = null;
this.before = before;
this.after = after;
- this.handler = new FilteredHandler(filter, handler);
+ this.handler = handler;
}
- private EventDiff(
- EventDiff parent, ChangeHandler handler,
+ private DiffContinuation(
+ DiffContinuation parent, ChangeHandler handler,
String name, NodeState before, NodeState after) {
this.parent = parent;
this.name = name;
@@ -148,9 +114,11 @@ public class EventGenerator implements E
public void run() {
if (parent != null) {
if (before == MISSING_NODE) {
- parent.handleAddedNode(name, after); // postponed handling of added nodes
+ // postponed handling of added nodes
+ parent.handleAddedNode(name, after);
} else if (after == MISSING_NODE) {
- parent.handleDeletedNode(name, before); // postponed handling of removed nodes
+ // postponed handling of removed nodes
+ parent.handleDeletedNode(name, before);
}
}
@@ -213,7 +181,7 @@ public class EventGenerator implements E
String name, NodeState before, NodeState after) {
ChangeHandler h = handler.getChildHandler(name, before, after);
if (h != null) {
- generators.add(new EventDiff(this, h, name, before, after));
+ continuations.add(new DiffContinuation(this, h, name, before, after));
return true;
} else {
return false;
@@ -260,136 +228,4 @@ public class EventGenerator implements E
}
- private class Handler implements ChangeHandler {
-
- private final ImmutableTree before;
- private final ImmutableTree after;
-
- Handler(ImmutableTree before, ImmutableTree after) {
- this.before = before;
- this.after = after;
- }
-
- @Override
- public ChangeHandler getChildHandler(
- String name, NodeState before, NodeState after) {
- return new Handler(
- new ImmutableTree(this.before, name, before),
- new ImmutableTree(this.after, name, after));
- }
-
- @Override
- public void propertyAdded(PropertyState after) {
- events.add(new EventImpl(
- context, PROPERTY_ADDED, this.after, after.getName()));
- }
-
- @Override
- public void propertyChanged(PropertyState before, PropertyState after) {
- events.add(new EventImpl(
- context, PROPERTY_CHANGED, this.after, after.getName()));
- }
-
- @Override
- public void propertyDeleted(PropertyState before) {
- events.add(new EventImpl(
- context, PROPERTY_REMOVED, this.before, before.getName()));
- }
-
- @Override
- public void nodeAdded(String name, NodeState after) {
- ImmutableTree tree = new ImmutableTree(this.after, name, after);
- events.add(new EventImpl(context, NODE_ADDED, tree));
- }
-
- @Override
- public void nodeDeleted(String name, NodeState before) {
- ImmutableTree tree = new ImmutableTree(this.before, name, before);
- events.add(new EventImpl(context, NODE_REMOVED, tree));
- }
-
- @Override
- public void nodeMoved(String sourcePath, String name, NodeState moved) {
- ImmutableTree tree = new ImmutableTree(this.after, name, moved);
- Map<String, String> info = ImmutableMap.of(
- "srcAbsPath", context.getJcrPath(sourcePath),
- "destAbsPath", context.getJcrPath(tree.getPath()));
- events.add(new EventImpl(context, NODE_MOVED, tree, info));
- }
-
- @Override
- public void nodeReordered(
- String destName, String name, NodeState reordered) {
- Map<String, String> info = ImmutableMap.of(
- "srcChildRelPath", context.getJcrName(name),
- "destChildRelPath", context.getJcrName(destName));
- ImmutableTree tree = new ImmutableTree(after, name, reordered);
- events.add(new EventImpl(context, NODE_MOVED, tree, info));
- }
-
- }
-
- //-----------------------------------------------------< EventIterator >--
-
- @Override
- public long getSize() {
- if (generators.isEmpty()) {
- return position + events.size();
- } else {
- return -1;
- }
- }
-
- @Override
- public long getPosition() {
- return position;
- }
-
- @Override
- public boolean hasNext() {
- while (events.isEmpty()) {
- if (generators.isEmpty()) {
- return false;
- } else {
- generators.removeFirst().run();
- }
- }
- return true;
- }
-
- @Override
- public void skip(long skipNum) {
- while (skipNum > events.size()) {
- position += events.size();
- skipNum -= events.size();
- events.clear();
- // the remove below throws NoSuchElementException if there
- // are no more generators, which is correct as then we can't
- // skip over enough events
- generators.removeFirst().run();
- }
- position += skipNum;
- events.subList(0, (int) skipNum).clear();
- }
-
- @Override
- public Event nextEvent() {
- if (hasNext()) {
- position++;
- return events.removeFirst();
- } else {
- throw new NoSuchElementException();
- }
- }
-
- @Override
- public Event next() {
- return nextEvent();
- }
-
- @Override
- public void remove() {
- throw new UnsupportedOperationException();
- }
-
}
Added: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/EventQueue.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/EventQueue.java?rev=1562233&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/EventQueue.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/EventQueue.java Tue Jan 28 21:14:06 2014
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.observation;
+
+import static com.google.common.collect.Lists.newLinkedList;
+
+import java.util.LinkedList;
+import java.util.NoSuchElementException;
+
+import javax.annotation.Nonnull;
+import javax.jcr.observation.Event;
+import javax.jcr.observation.EventIterator;
+
+import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.core.ImmutableTree;
+import org.apache.jackrabbit.oak.namepath.NamePathMapper;
+import org.apache.jackrabbit.oak.plugins.observation.filter.EventFilter;
+import org.apache.jackrabbit.oak.plugins.observation.filter.Filters;
+import org.apache.jackrabbit.oak.plugins.observation.filter.VisibleFilter;
+import org.apache.jackrabbit.oak.plugins.observation.handler.ChangeHandler;
+import org.apache.jackrabbit.oak.plugins.observation.handler.FilteredHandler;
+import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
+import org.apache.jackrabbit.oak.spi.state.NodeState;
+
+/**
+ * Queue of JCR Events generated from a given content change
+ */
+class EventQueue implements EventIterator {
+
+ private final EventGenerator generator;
+
+ private final LinkedList<Event> queue = newLinkedList();
+
+ private long position = 0;
+
+ public EventQueue(
+ @Nonnull NamePathMapper namePathMapper, CommitInfo info,
+ @Nonnull NodeState before, @Nonnull NodeState after,
+ @Nonnull String basePath, @Nonnull EventFilter filter) {
+ ChangeHandler handler = new QueueingHandler(
+ queue, new EventContext(namePathMapper, info),
+ new ImmutableTree(before), new ImmutableTree(after));
+ for (String name : PathUtils.elements(basePath)) {
+ before = before.getChildNode(name);
+ after = after.getChildNode(name);
+ handler = handler.getChildHandler(name, before, after);
+ }
+ handler = new FilteredHandler(
+ Filters.all(new VisibleFilter(), filter),
+ handler);
+
+ this.generator = new EventGenerator(before, after, handler);
+ }
+
+ //-----------------------------------------------------< EventIterator >--
+
+ @Override
+ public long getSize() {
+ if (generator.isComplete()) {
+ // no more new events will be generated, so count just those
+ // that have already been iterated and those left in the queue
+ return position + queue.size();
+ } else {
+ // the generator is not yet done, so there's no way
+ // to know how many events may still be generated
+ return -1;
+ }
+ }
+
+ @Override
+ public long getPosition() {
+ return position;
+ }
+
+ @Override
+ public boolean hasNext() {
+ while (queue.isEmpty()) {
+ if (generator.isComplete()) {
+ return false;
+ } else {
+ generator.generate();
+ }
+ }
+ return true;
+ }
+
+ @Override
+ public void skip(long skipNum) {
+ // generate events until all events to skip have been queued
+ while (skipNum > queue.size()) {
+ // drop all currently queued events as we're skipping them all
+ position += queue.size();
+ skipNum -= queue.size();
+ queue.clear();
+
+ // generate more events if possible, otherwise fail
+ if (!generator.isComplete()) {
+ generator.generate();
+ } else {
+ throw new NoSuchElementException("Not enough events to skip");
+ }
+ }
+
+ // the remaining events to skip are guaranteed to all be in the
+ // queue, so we can just drop those events and advance the position
+ queue.subList(0, (int) skipNum).clear();
+ position += skipNum;
+ }
+
+ @Override
+ public Event nextEvent() {
+ if (hasNext()) {
+ position++;
+ return queue.removeFirst();
+ } else {
+ throw new NoSuchElementException();
+ }
+ }
+
+ @Override
+ public Event next() {
+ return nextEvent();
+ }
+
+ @Override
+ public void remove() {
+ throw new UnsupportedOperationException();
+ }
+
+}
Propchange: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/EventQueue.java
------------------------------------------------------------------------------
svn:eol-style = native
Added: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/QueueingHandler.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/QueueingHandler.java?rev=1562233&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/QueueingHandler.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/QueueingHandler.java Tue Jan 28 21:14:06 2014
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.observation;
+
+import static javax.jcr.observation.Event.NODE_ADDED;
+import static javax.jcr.observation.Event.NODE_MOVED;
+import static javax.jcr.observation.Event.NODE_REMOVED;
+import static javax.jcr.observation.Event.PROPERTY_ADDED;
+import static javax.jcr.observation.Event.PROPERTY_CHANGED;
+import static javax.jcr.observation.Event.PROPERTY_REMOVED;
+
+import java.util.LinkedList;
+import java.util.Map;
+
+import javax.jcr.observation.Event;
+
+import org.apache.jackrabbit.oak.api.PropertyState;
+import org.apache.jackrabbit.oak.core.ImmutableTree;
+import org.apache.jackrabbit.oak.plugins.observation.handler.ChangeHandler;
+import org.apache.jackrabbit.oak.spi.state.NodeState;
+
+import com.google.common.collect.ImmutableMap;
+
+/**
+ * Change handler that generates JCR Event instances and places them
+ * in an event queue.
+ */
+class QueueingHandler implements ChangeHandler {
+
+ private final LinkedList<Event> queue;
+
+ private final EventContext context;
+
+ private final ImmutableTree before;
+
+ private final ImmutableTree after;
+
+ QueueingHandler(
+ LinkedList<Event> queue, EventContext context,
+ ImmutableTree before, ImmutableTree after) {
+ this.queue = queue;
+ this.context = context;
+ this.before = before;
+ this.after = after;
+ }
+
+ @Override
+ public ChangeHandler getChildHandler(
+ String name, NodeState before, NodeState after) {
+ return new QueueingHandler(
+ queue, context,
+ new ImmutableTree(this.before, name, before),
+ new ImmutableTree(this.after, name, after));
+ }
+
+ @Override
+ public void propertyAdded(PropertyState after) {
+ queue.add(new EventImpl(
+ context, PROPERTY_ADDED, this.after, after.getName()));
+ }
+
+ @Override
+ public void propertyChanged(PropertyState before, PropertyState after) {
+ queue.add(new EventImpl(
+ context, PROPERTY_CHANGED, this.after, after.getName()));
+ }
+
+ @Override
+ public void propertyDeleted(PropertyState before) {
+ queue.add(new EventImpl(
+ context, PROPERTY_REMOVED, this.before, before.getName()));
+ }
+
+ @Override
+ public void nodeAdded(String name, NodeState after) {
+ ImmutableTree tree = new ImmutableTree(this.after, name, after);
+ queue.add(new EventImpl(context, NODE_ADDED, tree));
+ }
+
+ @Override
+ public void nodeDeleted(String name, NodeState before) {
+ ImmutableTree tree = new ImmutableTree(this.before, name, before);
+ queue.add(new EventImpl(context, NODE_REMOVED, tree));
+ }
+
+ @Override
+ public void nodeMoved(String sourcePath, String name, NodeState moved) {
+ ImmutableTree tree = new ImmutableTree(this.after, name, moved);
+ Map<String, String> info = ImmutableMap.of(
+ "srcAbsPath", context.getJcrPath(sourcePath),
+ "destAbsPath", context.getJcrPath(tree.getPath()));
+ queue.add(new EventImpl(context, NODE_MOVED, tree, info));
+ }
+
+ @Override
+ public void nodeReordered(
+ String destName, String name, NodeState reordered) {
+ Map<String, String> info = ImmutableMap.of(
+ "srcChildRelPath", context.getJcrName(name),
+ "destChildRelPath", context.getJcrName(destName));
+ ImmutableTree tree = new ImmutableTree(after, name, reordered);
+ queue.add(new EventImpl(context, NODE_MOVED, tree, info));
+ }
+
+}
Propchange: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/QueueingHandler.java
------------------------------------------------------------------------------
svn:eol-style = native
Re: svn commit: r1562233 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation:
ChangeProcessor.java EventGenerator.java EventQueue.java QueueingHandler.java
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Wed, Jan 29, 2014 at 9:44 AM, Michael Dürig <md...@apache.org> wrote:
> On 29.1.14 3:35 , Jukka Zitting wrote:
>> Those aren't side effects, they're the explicitly what the classes are
>> wired up to do. There's no other effects of that wiring.
>
> I didn't doubt that this works as designed. But still the design relies on
> side effects of the parameterless method generate(). If there weren't any
> side effect generate() could as well be substituted by a constant.
Ah, I see your point. You're right, the EventGenerator is not
functional in design. That's what we had also earlier with
EventIterable.createChildGenerator(), now the statefulness of the code
is just exposed a bit more explicitly. We can't make this code
entirely functional as long as there are no native continuations in
Java.
>> To make this more explicit, we could replace the direct
>> LinkedList accesses in QueueingHandler with something like
>> EventQueue.addEvent(), which would encapsulate the queue entirely
>> within EventQueue.
>
> Yes, let's do that.
Done in r1562464.
BR,
Jukka Zitting
Re: svn commit: r1562233 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation:
ChangeProcessor.java EventGenerator.java EventQueue.java QueueingHandler.java
Posted by Michael Dürig <md...@apache.org>.
On 29.1.14 3:35 , Jukka Zitting wrote:
> Hi,
>
> On Wed, Jan 29, 2014 at 4:44 AM, Michael Dürig <md...@apache.org> wrote:
>> I don't like how this relies on side effects: generate() puts the events
>> into a queue owned by QueueingHandler and referenced by this class
>> (EventQueue).
>
> Those aren't side effects, they're the explicitly what the classes are
> wired up to do. There's no other effects of that wiring.
I didn't doubt that this works as designed. But still the design relies
on side effects of the parameterless method generate(). If there weren't
any side effect generate() could as well be substituted by a constant.
To make this more explicit, we could replace the direct
> LinkedList accesses in QueueingHandler with something like
> EventQueue.addEvent(), which would encapsulate the queue entirely
> within EventQueue.
Yes, let's do that.
Michael
>
> BR,
>
> Jukka Zitting
>
Re: svn commit: r1562233 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation:
ChangeProcessor.java EventGenerator.java EventQueue.java QueueingHandler.java
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Wed, Jan 29, 2014 at 4:44 AM, Michael Dürig <md...@apache.org> wrote:
> I don't like how this relies on side effects: generate() puts the events
> into a queue owned by QueueingHandler and referenced by this class
> (EventQueue).
Those aren't side effects, they're the explicitly what the classes are
wired up to do. There's no other effects of that wiring.
The responsibility of the EventGenerator is to incrementally run
through the content diff, making callbacks to the given handler.
The responsibility of the QueueingHandler is to turn those callbacks
to JCR events and place them in the given queue.
The responsibility of the EventQueue is to manage the queue and make
it accessible to higher level clients.
> Look how concise this was initially:
>
> concat(listener.iterator(), concat(childEvents.iterator()));
>
> just flatmap that shit ;-)
Right, but doing so caused OAK-1318 and ended up requiring a custom
iterator wrapper that worked much like EventQueue now does.
> Also EventQueue is not really a queue but just an iterator implementing that
> flatmap by means of the generator and the queueing handler. Could we at
> least somehow remove that explicit reference to the queue from EventQueue
> and access it through the queueing handler?
That's something I wanted to avoid, see the list of responsibilities
above. To make this more explicit, we could replace the direct
LinkedList accesses in QueueingHandler with something like
EventQueue.addEvent(), which would encapsulate the queue entirely
within EventQueue.
BR,
Jukka Zitting
Re: svn commit: r1562233 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation:
ChangeProcessor.java EventGenerator.java EventQueue.java QueueingHandler.java
Posted by Michael Dürig <md...@apache.org>.
Hi,
On 28.1.14 10:14 , jukka@apache.org wrote:
> Author: jukka
> Date: Tue Jan 28 21:14:06 2014
> New Revision: 1562233
>
> URL: http://svn.apache.org/r1562233
> Log:
> OAK-1133: Observation listener PLUS
>
> + public boolean hasNext() {
> + while (queue.isEmpty()) {
> + if (generator.isComplete()) {
> + return false;
> + } else {
> + generator.generate();
> + }
> + }
> + return true;
> + }
I don't like how this relies on side effects: generate() puts the events
into a queue owned by QueueingHandler and referenced by this class
(EventQueue).
Look how concise this was initially:
concat(listener.iterator(), concat(childEvents.iterator()));
just flatmap that shit ;-)
Now the same logic is sprinkled over three classes, which even live in
different modules.
Also EventQueue is not really a queue but just an iterator implementing
that flatmap by means of the generator and the queueing handler. Could
we at least somehow remove that explicit reference to the queue from
EventQueue and access it through the queueing handler?
Michael