New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding SuppressByMixinFilter to suppress events related to nodes with any of a given set of mixins #769
Conversation
… any of a given set of mixins
return (org.modeshape.jcr.api.observation.Event) event; | ||
final org.modeshape.jcr.api.observation.Event modeEvent = | ||
(org.modeshape.jcr.api.observation.Event) event; | ||
return ImmutableSet.copyOf(transform(ImmutableList.copyOf(modeEvent.getMixinNodeTypes()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why a copy to a Set
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what the code was doing before -- but we could just remove that extra step and return the List.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see no advantage to the extra copy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, perhaps just Arrays.asList()
instead of a copy to an ImmutableList
?
I have one minor qualm-- do we want an IT for this? |
That's a good idea -- it'll give me a place to document the spring bean setup to use this. |
Right on! |
* @since 2015-04-15 | ||
*/ | ||
@ContextConfiguration({"/spring-test/eventing-suppress.xml", "/spring-test/repo.xml"}) | ||
public class SuppressByMixinFilterIT extends AbstractIT { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we can use awaitility here as we do for other event-driven tests in fcrepo-jms?
@escowles Did you want me to review and merge this? |
@ajs6f yes, please! |
@@ -119,6 +119,12 @@ | |||
<version>${project.version}</version> | |||
<scope>test</scope> | |||
</dependency> | |||
<dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fcrepo-jms
also uses awaitility, so maybe we should move the versioning info up into fcrepo4
's pom?
…n config comments
@ajs6f I've moved the awaitility version to the top-level pom.xml and updated the spring comments. |
Adding SuppressByMixinFilter to suppress events related to nodes with any of a given set of mixins Resolves: https://jira.duraspace.org/browse/FCREPO-1456
Fixes https://jira.duraspace.org/browse/FCREPO-1456