added check for the event path to decrease the amount of Session.getN…

…ode() calls in the DefaultFilter
fcrepo · Jun 29, 2013 · 59cd31b · 59cd31b
1 parent 2f6b89f
commit 59cd31b
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 34 deletions.
diff --git a/fcrepo-kernel/src/main/java/org/fcrepo/observer/DefaultFilter.java b/fcrepo-kernel/src/main/java/org/fcrepo/observer/DefaultFilter.java
@@ -16,14 +16,10 @@
 package org.fcrepo.observer;
 
 import static com.google.common.base.Throwables.propagate;
-import static org.fcrepo.utils.FedoraTypesUtils.isFedoraDatastream;
-import static org.fcrepo.utils.FedoraTypesUtils.isFedoraObject;
 
 import javax.annotation.PostConstruct;
 import javax.annotation.PreDestroy;
 import javax.inject.Inject;
-import javax.jcr.Node;
-import javax.jcr.PathNotFoundException;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
 import javax.jcr.observation.Event;
@@ -55,20 +51,33 @@ public class DefaultFilter implements EventFilter {
      */
     @Override
     public boolean apply(final Event event) {
-
         try {
-            final Node resource = session.getNode(event.getPath());
-            return isFedoraObject.apply(resource) ||
-                isFedoraDatastream.apply(resource);
-
-        } catch (final PathNotFoundException e) {
-            // not a node in the fedora workspace
-            return false;
+            /* check if start of the path points at a jcr system property */
+            return !(isJcrProperty(event) || isJcrSystemProperty(event));
         } catch (final RepositoryException e) {
             throw propagate(e);
         }
     }
 
+    /**
+     * Check if an {@link Event} is associated with a Jcr property
+     * @param event the {@link Event} to check
+     * @return true if the {@link Event} is a Jcr property
+     */
+    private boolean isJcrProperty(Event event) throws RepositoryException {
+        return event.getPath().substring(event.getPath().lastIndexOf('/'))
+                .startsWith("/jcr:");
+    }
+
+    /**
+     * Check if an {@link Event} is associated with a Jcr system property
+     * @param event the {@link Event} to check
+     * @return true if the {@link Event} is a Jcr system property
+     */
+    private boolean isJcrSystemProperty(Event event) throws RepositoryException {
+        return event.getPath().startsWith("/jcr:system");
+    }
+
     /**
      * @todo Add Documentation.
      */

diff --git a/fcrepo-kernel/src/test/java/org/fcrepo/observer/DefaultFilterTest.java b/fcrepo-kernel/src/test/java/org/fcrepo/observer/DefaultFilterTest.java
@@ -19,7 +19,6 @@
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.lang.reflect.Field;
@@ -123,39 +122,29 @@ public void shouldApplyToDatastream() throws Exception {
      */
     @SuppressWarnings("unchecked")
     @Test
-    public void shouldNotApplyToNonExistentNodes() throws Exception {
+    public void shouldNotApplyToJcrProperties() throws Exception {
 
-        final String testPath = "/foo/bar";
+        final String testPath = "/foo/bar/jcr:name";
         final Event mockEvent = mock(Event.class);
         when(mockEvent.getPath()).thenReturn(testPath);
         when(mockSession.getNode(testPath)).thenThrow(
                 PathNotFoundException.class);
         assertEquals(false, testObj.apply(mockEvent));
-        verify(mockSession).getNode(testPath);
     }
 
     /**
      * @todo Add Documentation.
      */
+    @SuppressWarnings("unchecked")
     @Test
-    public void shouldNotApplyToSystemNodes() throws Exception {
-        @SuppressWarnings("unchecked")
-        final Predicate<Node> mockFuncFalse = mock(Predicate.class);
-        final Predicate<Node> holdDS = FedoraTypesUtils.isFedoraDatastream;
-        final Predicate<Node> holdO = FedoraTypesUtils.isFedoraObject;
+    public void shouldNotApplyToJcrSystemNodes() throws Exception {
 
-        try {
-            FedoraTypesUtils.isFedoraDatastream = mockFuncFalse;
-            FedoraTypesUtils.isFedoraObject = mockFuncFalse;
-            final String testPath = "/foo/bar";
-            final Event mockEvent = mock(Event.class);
-            when(mockEvent.getPath()).thenReturn(testPath);
-            final Node mockNode = mock(Node.class);
-            when(mockSession.getNode(testPath)).thenReturn(mockNode);
-            assertEquals(false, testObj.apply(mockEvent));
-        } finally {
-            FedoraTypesUtils.isFedoraDatastream = holdDS;
-            FedoraTypesUtils.isFedoraObject = holdO;
-        }
+        final String testPath = "/jcr:system/foo/bar";
+        final Event mockEvent = mock(Event.class);
+        when(mockEvent.getPath()).thenReturn(testPath);
+        when(mockSession.getNode(testPath)).thenThrow(
+                PathNotFoundException.class);
+        assertEquals(false, testObj.apply(mockEvent));
     }
+
 }