Better exception handling in observation filters

fcrepo · Apr 17, 2015 · 51f952a · 51f952a
1 parent fdde34d
commit 51f952a
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 41 deletions.
diff --git a/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/observer/DefaultFilter.java b/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/observer/DefaultFilter.java
@@ -16,7 +16,6 @@
 package org.fcrepo.kernel.impl.observer;
 
 import static com.google.common.collect.Sets.newHashSet;
-import static java.util.Arrays.asList;
 import static java.util.Arrays.stream;
 import static java.util.Collections.disjoint;
 import static java.util.stream.Collectors.toList;
@@ -28,9 +27,6 @@
 
 import java.util.Collection;
 import java.util.HashSet;
-import java.util.List;
-import java.util.function.Predicate;
-
 import javax.jcr.PathNotFoundException;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
@@ -44,9 +40,8 @@
 
 /**
  * {@link EventFilter} that passes only events emitted from nodes with a Fedora
- * JCR type, or properties attached to them, except in the case of a node
- * removal. In that case, since we cannot test the node for its types, we assume
- * that any non-JCR namespaced node is fair game.
+ * JCR mixin type, or properties attached to them, except in the case of a node
+ * removal.
  *
  * @author ajs6f
  * @author barmintor
@@ -58,38 +53,27 @@
  */
 public class DefaultFilter implements EventFilter {
 
-    private static final List<String> TYPES = asList(FEDORA_RESOURCE, FEDORA_BINARY,
-            FEDORA_NON_RDF_SOURCE_DESCRIPTION, FEDORA_CONTAINER);
-
-    private static final HashSet<String> fedoraMixins =
+    private static final HashSet<String> FEDORA_MIXINS =
             newHashSet(FEDORA_BINARY, FEDORA_CONTAINER, FEDORA_NON_RDF_SOURCE_DESCRIPTION, FEDORA_RESOURCE);
 
     private static final Logger LOGGER = getLogger(DefaultFilter.class);
 
     @Override
-    public Predicate<Event> getFilter(final Session session) {
+    public DefaultFilter getFilter(final Session session) {
         return new DefaultFilter();
     }
 
     @Override
     public boolean test(final Event event) {
-        try {
-            return !disjoint(getMixinTypes(event), fedoraMixins);
-        } catch (final PathNotFoundException e) {
-            LOGGER.trace("Dropping event from outside our assigned workspace:\n", e);
-            return false;
-        } catch (final RepositoryException e) {
-            throw new RepositoryRuntimeException(e);
-        }
+        return !disjoint(getMixinTypes(event), FEDORA_MIXINS);
     }
 
-    protected static Collection<String> getMixinTypes(final Event event)
-            throws PathNotFoundException, RepositoryException {
+    protected static Collection<String> getMixinTypes(final Event event) {
         try {
             final org.modeshape.jcr.api.observation.Event modeEvent = (org.modeshape.jcr.api.observation.Event) event;
             return stream(modeEvent.getMixinNodeTypes()).map(NodeType::toString).collect(toList());
-    } catch (final ClassCastException e) {
-            throw new ClassCastException(event + " is not a Modeshape Event");
+        } catch (final RepositoryException e) {
+            throw new RepositoryRuntimeException(e);
         }
     }
 }
diff --git a/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/observer/SuppressByMixinFilter.java b/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/observer/SuppressByMixinFilter.java
@@ -15,17 +15,17 @@
  */
 package org.fcrepo.kernel.impl.observer;
 
+import static java.util.Collections.disjoint;
 import static org.slf4j.LoggerFactory.getLogger;
 
 import javax.jcr.PathNotFoundException;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
 import javax.jcr.observation.Event;
 
-import com.google.common.base.Predicate;
-
 import org.fcrepo.kernel.exception.RepositoryRuntimeException;
 import org.fcrepo.kernel.observer.EventFilter;
+
 import org.slf4j.Logger;
 
 import java.util.Collections;
@@ -36,38 +36,32 @@
  * emitted by nodes with a provided set of mixins.
  *
  * @author escowles
+ * @author ajs6f
  * @since 2015-04-15
  */
-public class SuppressByMixinFilter extends DefaultFilter implements EventFilter {
+public class SuppressByMixinFilter extends DefaultFilter {
 
     private static final Logger LOGGER = getLogger(SuppressByMixinFilter.class);
-    private Set<String> suppressedMixins;
+    private final Set<String> suppressedMixins;
 
     /**
      * Default constructor.
      */
     public SuppressByMixinFilter(final Set<String> suppressedMixins) {
         this.suppressedMixins = suppressedMixins;
-        for (String mixin : suppressedMixins) {
+        for (final String mixin : suppressedMixins) {
             LOGGER.info("Suppressing events for nodes with mixin: {}", mixin);
         }
     }
 
     @Override
-    public Predicate<Event> getFilter(final Session session) {
+    public SuppressByMixinFilter getFilter(final Session session) {
         return this;
     }
 
     @Override
-    public boolean apply(final Event event) {
-        try {
-            return super.apply(event) && Collections.disjoint(getMixinTypes(event), suppressedMixins);
-        } catch (final PathNotFoundException e) {
-            LOGGER.trace("Dropping event from outside our assigned workspace:\n", e);
-            return false;
-        } catch (final RepositoryException e) {
-            throw new RepositoryRuntimeException(e);
-        }
+    public boolean test(final Event event) {
+        return super.test(event) && disjoint(getMixinTypes(event), suppressedMixins);
     }
 
 }
diff --git a/...-kernel-impl/src/test/java/org/fcrepo/kernel/impl/observer/SuppressByMixinFilterTest.java b/...-kernel-impl/src/test/java/org/fcrepo/kernel/impl/observer/SuppressByMixinFilterTest.java
@@ -64,12 +64,12 @@ public void setUp() {
     @Test
     public void shouldSuppressMixin() throws Exception {
         when(mockEvent.getMixinNodeTypes()).thenReturn(new NodeType[] {fedoraContainer, internalEvent});
-        assertFalse(testObj.getFilter(mockSession).apply(mockEvent));
+        assertFalse(testObj.getFilter(mockSession).test(mockEvent));
     }
 
     @Test
     public void shouldAllowOthers() throws Exception {
         when(mockEvent.getMixinNodeTypes()).thenReturn(new NodeType[] {fedoraContainer});
-        assertTrue(testObj.getFilter(mockSession).apply(mockEvent));
+        assertTrue(testObj.getFilter(mockSession).test(mockEvent));
     }
 }