diff --git a/fcrepo-kernel/src/main/java/org/fcrepo/kernel/observer/DefaultFilter.java b/fcrepo-kernel/src/main/java/org/fcrepo/kernel/observer/DefaultFilter.java index 742cb10114..e0b8849ea1 100644 --- a/fcrepo-kernel/src/main/java/org/fcrepo/kernel/observer/DefaultFilter.java +++ b/fcrepo-kernel/src/main/java/org/fcrepo/kernel/observer/DefaultFilter.java @@ -16,13 +16,15 @@ package org.fcrepo.kernel.observer; import static com.google.common.base.Throwables.propagate; -import static org.fcrepo.kernel.utils.FedoraTypesUtils.isFedoraDatastream; -import static org.fcrepo.kernel.utils.FedoraTypesUtils.isFedoraObject; +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.fcrepo.kernel.utils.FedoraTypesUtils.isFedoraObjectOrDatastream; -import javax.annotation.PostConstruct; -import javax.annotation.PreDestroy; import javax.inject.Inject; -import javax.jcr.Item; import javax.jcr.Node; import javax.jcr.PathNotFoundException; import javax.jcr.RepositoryException; @@ -46,10 +48,6 @@ public class DefaultFilter implements EventFilter { @Inject private Repository repository; - // it's safe to keep the session around, because this code does not mutate - // the state of the repository - private Session session; - /** * Filter observer events to only include events on a FedoraObject or * Datastream, or properties of an FedoraObject or Datastream. @@ -59,34 +57,43 @@ public class DefaultFilter implements EventFilter { */ @Override public boolean apply(final Event event) { + Session session = null; try { - final Item item = session.getItem(event.getPath()); - final Node n = item.isNode() ? (Node)item : item.getParent(); - return isFedoraObject.apply(n) || isFedoraDatastream.apply(n); + String nPath = event.getPath(); + int nType = event.getType(); + switch(nType) { + case NODE_ADDED: + break; + case NODE_REMOVED: + return true; + case PROPERTY_ADDED: + nPath = nPath.substring(0, nPath.lastIndexOf('/')); + break; + case PROPERTY_REMOVED: + nPath = nPath.substring(0, nPath.lastIndexOf('/')); + break; + case PROPERTY_CHANGED: + nPath = nPath.substring(0, nPath.lastIndexOf('/')); + break; + case NODE_MOVED: + break; + default: + return false; + } + + session = repository.login(); + final Node n = session.getNode(nPath); + return isFedoraObjectOrDatastream.apply(n); } catch (final PathNotFoundException e) { // not a node in the fedora workspace return false; } catch (final RepositoryException e) { throw propagate(e); + } finally { + if (session != null) { + session.logout(); + } } } - /** - * Initialize a long-running read-only JCR session - * to use for filtering events - * @throws RepositoryException - */ - @PostConstruct - public void acquireSession() throws RepositoryException { - session = repository.login(); - } - - /** - * Log-out of the read-only JCR session before destroying - * the filter. - */ - @PreDestroy - public void releaseSession() { - session.logout(); - } } diff --git a/fcrepo-kernel/src/main/java/org/fcrepo/kernel/observer/SimpleObserver.java b/fcrepo-kernel/src/main/java/org/fcrepo/kernel/observer/SimpleObserver.java index f54e4a6bb8..2145ad1f8b 100644 --- a/fcrepo-kernel/src/main/java/org/fcrepo/kernel/observer/SimpleObserver.java +++ b/fcrepo-kernel/src/main/java/org/fcrepo/kernel/observer/SimpleObserver.java @@ -31,10 +31,8 @@ import java.util.Set; import javax.annotation.PostConstruct; +import javax.annotation.PreDestroy; import javax.inject.Inject; -import javax.jcr.Item; -import javax.jcr.Node; -import javax.jcr.PathNotFoundException; import javax.jcr.RepositoryException; import javax.jcr.Session; import javax.jcr.observation.Event; @@ -79,6 +77,7 @@ public class SimpleObserver implements EventListener { @Inject private EventFilter eventFilter; + // THIS SESSION SHOULD NOT BE USED TO LOOK UP NODES private Session session; /** @@ -93,6 +92,16 @@ public void buildListener() throws RepositoryException { session.save(); } + /** + * logout of the session + * @throws RepositoryException + */ + @PreDestroy + public void stopListening() throws RepositoryException { + session.getWorkspace().getObservationManager().removeEventListener(this); + session.logout(); + } + /** * Filter JCR events and transform them into our own FedoraEvents. * @@ -101,30 +110,42 @@ public void buildListener() throws RepositoryException { @Override public void onEvent(final javax.jcr.observation.EventIterator events) { // keep track of nodes that trigger events to prevent duplicates - final Set posted = new HashSet(); + // size to minimize resizing. + final Set posted = new HashSet((int)events.getSize() * 2 / 3); // post non-duplicate events approved by the filter for (final Event e : filter(new EventIterator(events), eventFilter)) { try { - final Item item = session.getItem(e.getPath()); - Node n = null; - if ( item.isNode() ) { - n = (Node)item; - } else { - n = item.getParent(); + String nPath = e.getPath(); + int nType = e.getType(); + // is jump table faster than two bitwise comparisons? + switch(nType) { + case NODE_ADDED: + break; + case NODE_REMOVED: + break; + case PROPERTY_ADDED: + nPath = nPath.substring(0, nPath.lastIndexOf('/')); + break; + case PROPERTY_REMOVED: + nPath = nPath.substring(0, nPath.lastIndexOf('/')); + break; + case PROPERTY_CHANGED: + nPath = nPath.substring(0, nPath.lastIndexOf('/')); + break; + case NODE_MOVED: + break; + default: + nPath = null; } - if ( n != null && !posted.contains(n) ) { + if ( nPath != null && !posted.contains(nPath) ) { EVENT_COUNTER.inc(); - LOGGER.debug("Putting event: " + e.toString() - + " on the bus."); + LOGGER.debug("Putting event: {} ({}) on the bus", nPath, nType); eventBus.post(new FedoraEvent(e)); - posted.add(n); + posted.add(nPath); } else { - LOGGER.debug("Skipping: " + e); + LOGGER.debug("Skipping event: {} ({}) on the bus", nPath, nType); } - } catch (final PathNotFoundException ex) { - // we can ignore these - LOGGER.trace("Not a node in the Fedora workspace: " + e); } catch ( RepositoryException ex ) { throw propagate(ex); } diff --git a/fcrepo-kernel/src/main/java/org/fcrepo/kernel/services/functions/AllTypesPredicate.java b/fcrepo-kernel/src/main/java/org/fcrepo/kernel/services/functions/AllTypesPredicate.java new file mode 100644 index 0000000000..94e5f8622c --- /dev/null +++ b/fcrepo-kernel/src/main/java/org/fcrepo/kernel/services/functions/AllTypesPredicate.java @@ -0,0 +1,41 @@ +/** + * Copyright 2013 DuraSpace, Inc. + * + * Licensed 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.fcrepo.kernel.services.functions; + +/** + * Predicate to match nodes with all of the given mixin types + * @author armintor@gmail.com + * + */ +public class AllTypesPredicate extends BooleanTypesPredicate { + + private final int test; + + /** + * True if all the types specified match. + * @param types + */ + public AllTypesPredicate(String...types) { + super(types); + this.test = types.length; + } + + @Override + protected boolean test(final int matched) { + return matched == test; + } + +} diff --git a/fcrepo-kernel/src/main/java/org/fcrepo/kernel/services/functions/AnyTypesPredicate.java b/fcrepo-kernel/src/main/java/org/fcrepo/kernel/services/functions/AnyTypesPredicate.java new file mode 100644 index 0000000000..86a43fb77b --- /dev/null +++ b/fcrepo-kernel/src/main/java/org/fcrepo/kernel/services/functions/AnyTypesPredicate.java @@ -0,0 +1,38 @@ +/** + * Copyright 2013 DuraSpace, Inc. + * + * Licensed 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.fcrepo.kernel.services.functions; + + +/** + * Predicate to match nodes with any of the given mixin types + * @author armintor@gmail.com + * + */ +public class AnyTypesPredicate extends BooleanTypesPredicate { + /** + * True if any of the types specified match. + * @param types + */ + public AnyTypesPredicate(String...types) { + super(types); + } + + @Override + protected boolean test(final int matched) { + return matched > 0; + } + +} diff --git a/fcrepo-kernel/src/main/java/org/fcrepo/kernel/services/functions/BooleanTypesPredicate.java b/fcrepo-kernel/src/main/java/org/fcrepo/kernel/services/functions/BooleanTypesPredicate.java new file mode 100644 index 0000000000..7e3c3a7d48 --- /dev/null +++ b/fcrepo-kernel/src/main/java/org/fcrepo/kernel/services/functions/BooleanTypesPredicate.java @@ -0,0 +1,68 @@ +/** + * Copyright 2013 DuraSpace, Inc. + * + * Licensed 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.fcrepo.kernel.services.functions; + +import static com.google.common.base.Throwables.propagate; + +import java.util.Arrays; +import java.util.Collection; + +import javax.jcr.Node; +import javax.jcr.RepositoryException; +import javax.jcr.nodetype.NodeType; + +import com.google.common.base.Predicate; + +/** + * Base class for matching sets of node types + * @author armintor@gmail.com + * + */ +public abstract class BooleanTypesPredicate implements Predicate { + + protected final Collection nodeTypes; + + /** + * Base constructor for function peforming boolean ops on matched node types. + * @param types + */ + public BooleanTypesPredicate(String... types) { + nodeTypes = Arrays.asList(types); + } + + @Override + public boolean apply(Node input) { + if (input == null) { + throw new IllegalArgumentException( + "null node passed to" + getClass().getName() + ); + } + int matched = 0; + try { + for (NodeType nodeType: input.getMixinNodeTypes()) { + if (nodeTypes.contains(nodeType.getName())) { + matched++; + } + } + } catch (RepositoryException e) { + propagate(e); + } + return test(matched); + } + + protected abstract boolean test(int matched); + +} diff --git a/fcrepo-kernel/src/main/java/org/fcrepo/kernel/utils/FedoraTypesUtils.java b/fcrepo-kernel/src/main/java/org/fcrepo/kernel/utils/FedoraTypesUtils.java index baf607e6d6..0e21e551e8 100644 --- a/fcrepo-kernel/src/main/java/org/fcrepo/kernel/utils/FedoraTypesUtils.java +++ b/fcrepo-kernel/src/main/java/org/fcrepo/kernel/utils/FedoraTypesUtils.java @@ -54,6 +54,7 @@ import javax.jcr.version.VersionHistory; import org.fcrepo.jcr.FedoraJcrTypes; +import org.fcrepo.kernel.services.functions.AnyTypesPredicate; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; import org.joda.time.format.DateTimeFormatter; @@ -142,37 +143,22 @@ public boolean apply(final Node node) { /** * Predicate for determining whether this {@link Node} is a Fedora object. */ - public static Predicate isFedoraObject = new Predicate() { - - @Override - public boolean apply(final Node node) { - checkArgument(node != null, "null cannot be a Fedora object!"); - try { - return map(node.getMixinNodeTypes(), nodetype2name).contains( - FEDORA_OBJECT); - } catch (final RepositoryException e) { - throw propagate(e); - } - } - }; + public static Predicate isFedoraObject = + new AnyTypesPredicate(FEDORA_OBJECT); /** * Predicate for determining whether this {@link Node} is a Fedora * datastream. */ - public static Predicate isFedoraDatastream = new Predicate() { + public static Predicate isFedoraDatastream = + new AnyTypesPredicate(FEDORA_DATASTREAM); - @Override - public boolean apply(final Node node) { - checkArgument(node != null, "null cannot be a Fedora datastream!"); - try { - return map(node.getMixinNodeTypes(), nodetype2name).contains( - FEDORA_DATASTREAM); - } catch (final RepositoryException e) { - throw propagate(e); - } - } - }; + /** + * Predicate for objects, datastreams, whatever! + */ + + public static Predicate isFedoraObjectOrDatastream = + new AnyTypesPredicate(FEDORA_OBJECT, FEDORA_DATASTREAM); /** * Translates a {@link NodeType} to its {@link String} name. diff --git a/fcrepo-kernel/src/test/java/org/fcrepo/kernel/observer/DefaultFilterTest.java b/fcrepo-kernel/src/test/java/org/fcrepo/kernel/observer/DefaultFilterTest.java index 992d6836f7..3204342776 100644 --- a/fcrepo-kernel/src/test/java/org/fcrepo/kernel/observer/DefaultFilterTest.java +++ b/fcrepo-kernel/src/test/java/org/fcrepo/kernel/observer/DefaultFilterTest.java @@ -18,9 +18,11 @@ import static com.google.common.base.Predicates.alwaysFalse; import static com.google.common.base.Predicates.alwaysTrue; -import static org.fcrepo.kernel.utils.FedoraTypesUtils.isFedoraDatastream; -import static org.fcrepo.kernel.utils.FedoraTypesUtils.isFedoraObject; +import static org.fcrepo.kernel.utils.FedoraTypesUtils.isFedoraObjectOrDatastream; +import static javax.jcr.observation.Event.NODE_ADDED; +import static javax.jcr.observation.Event.PROPERTY_ADDED; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; @@ -71,78 +73,56 @@ public class DefaultFilterTest { public void setUp() throws Exception { initMocks(this); when(mockEvent.getPath()).thenReturn(testPath); + when(mockEvent.getType()).thenReturn(NODE_ADDED); when(mockNode.isNode()).thenReturn(true); testObj = new DefaultFilter(); final Field repoF = DefaultFilter.class.getDeclaredField("repository"); repoF.setAccessible(true); when(mockRepo.login()).thenReturn(mockSession); repoF.set(testObj, mockRepo); - testObj.acquireSession(); } @After public void tearDown() { - testObj.releaseSession(); } @Test - public void shouldApplyToObject() throws Exception { + public void shouldApplyToObjectOrDatastream() throws Exception { when(mockSession.getItem(testPath)).thenReturn(mockNode); - final Predicate holdDS = isFedoraDatastream; - final Predicate holdO = isFedoraObject; + final Predicate holdO = isFedoraObjectOrDatastream; try { - isFedoraDatastream = alwaysFalse(); - isFedoraObject = alwaysTrue(); + isFedoraObjectOrDatastream = alwaysTrue(); assertTrue(testObj.apply(mockEvent)); } finally { - isFedoraDatastream = holdDS; - isFedoraObject = holdO; - } - } - - @Test - public void shouldApplyToDatastream() throws Exception { - when(mockSession.getItem(testPath)).thenReturn(mockNode); - final Predicate holdDS = isFedoraDatastream; - final Predicate holdO = isFedoraObject; - try { - isFedoraDatastream = alwaysTrue(); - isFedoraObject = alwaysFalse(); - assertTrue(testObj.apply(mockEvent)); - } finally { - isFedoraDatastream = holdDS; - isFedoraObject = holdO; + isFedoraObjectOrDatastream = holdO; } } @Test public void shouldNotApplyToNonExistentNodes() throws Exception { - when(mockSession.getItem(testPath)).thenThrow( + when(mockSession.getNode(testPath)).thenThrow( new PathNotFoundException("Expected.")); assertEquals(false, testObj.apply(mockEvent)); } @Test public void shouldNotApplyToNonFedoraNodes() throws Exception { - when(mockSession.getItem(testPath)).thenReturn(mockNode); - final Predicate holdDS = isFedoraDatastream; - final Predicate holdO = isFedoraObject; + when(mockSession.getNode(testPath)).thenReturn(mockNode); + final Predicate holdO = isFedoraObjectOrDatastream; try { - isFedoraDatastream = alwaysFalse(); - isFedoraObject = alwaysFalse(); - assertEquals(false, testObj.apply(mockEvent)); + isFedoraObjectOrDatastream = alwaysFalse(); + assertFalse(testObj.apply(mockEvent)); } finally { - isFedoraDatastream = holdDS; - isFedoraObject = holdO; + isFedoraObjectOrDatastream = holdO; } } @Test(expected = RuntimeException.class) public void testBadItem() throws RepositoryException { - when(mockSession.getItem(testPath)).thenReturn(mockProperty); - when(mockProperty.isNode()).thenReturn(false); - when(mockProperty.getParent()).thenThrow( + when(mockEvent.getType()).thenReturn(PROPERTY_ADDED); + when(mockSession.getNode("/foo")).thenThrow( new RepositoryException("Expected.")); + when(mockProperty.isNode()).thenReturn(false); testObj.apply(mockEvent); } @@ -151,15 +131,12 @@ public void testProperty() throws RepositoryException { when(mockProperty.isNode()).thenReturn(false); when(mockProperty.getParent()).thenReturn(mockNode); when(mockSession.getItem(testPath)).thenReturn(mockProperty); - final Predicate holdDS = isFedoraDatastream; - final Predicate holdO = isFedoraObject; + final Predicate holdO = isFedoraObjectOrDatastream; try { - isFedoraDatastream = alwaysFalse(); - isFedoraObject = alwaysTrue(); - assertEquals(true, testObj.apply(mockEvent)); + isFedoraObjectOrDatastream = alwaysTrue(); + assertTrue(testObj.apply(mockEvent)); } finally { - isFedoraDatastream = holdDS; - isFedoraObject = holdO; + isFedoraObjectOrDatastream = holdO; } } } diff --git a/fcrepo-kernel/src/test/java/org/fcrepo/kernel/observer/SimpleObserverTest.java b/fcrepo-kernel/src/test/java/org/fcrepo/kernel/observer/SimpleObserverTest.java index 4d02451acb..c281ae5681 100644 --- a/fcrepo-kernel/src/test/java/org/fcrepo/kernel/observer/SimpleObserverTest.java +++ b/fcrepo-kernel/src/test/java/org/fcrepo/kernel/observer/SimpleObserverTest.java @@ -19,6 +19,7 @@ import static com.google.common.collect.Iterables.filter; import static java.util.Arrays.asList; import static org.fcrepo.kernel.observer.SimpleObserver.EVENT_TYPES; +import static javax.jcr.observation.Event.NODE_ADDED; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; @@ -100,9 +101,10 @@ public void testOnEvent() throws Exception { setField("session", testObj, mockSession); final Event mockEvent = mock(Event.class); when(mockEvent.getPath()).thenReturn("/foo/bar"); + when(mockEvent.getType()).thenReturn(NODE_ADDED); final Node mockNode = mock(Node.class); when(mockNode.isNode()).thenReturn(true); - when(mockSession.getItem(any(String.class))).thenReturn(mockNode); + when(mockSession.getNode(any(String.class))).thenReturn(mockNode); final EventIterator mockEvents = mock(EventIterator.class); final List iterable = asList(new Event[] {mockEvent}); mockStatic(Iterables.class);