Skip to content
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

Merged
merged 7 commits into from Apr 16, 2015

Conversation

escowles
Copy link
Contributor

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()),
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor

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?

@ajs6f
Copy link
Contributor

ajs6f commented Apr 15, 2015

I have one minor qualm-- do we want an IT for this?

@escowles
Copy link
Contributor Author

That's a good idea -- it'll give me a place to document the spring bean setup to use this.

@ajs6f
Copy link
Contributor

ajs6f commented Apr 15, 2015

Right on!

* @since 2015-04-15
*/
@ContextConfiguration({"/spring-test/eventing-suppress.xml", "/spring-test/repo.xml"})
public class SuppressByMixinFilterIT extends AbstractIT {
Copy link
Contributor

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?

@ajs6f
Copy link
Contributor

ajs6f commented Apr 16, 2015

@escowles Did you want me to review and merge this?

@escowles
Copy link
Contributor Author

@ajs6f yes, please!

@@ -119,6 +119,12 @@
<version>${project.version}</version>
<scope>test</scope>
</dependency>
<dependency>
Copy link
Contributor

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?

@escowles
Copy link
Contributor Author

@ajs6f I've moved the awaitility version to the top-level pom.xml and updated the spring comments.

ajs6f added a commit that referenced this pull request Apr 16, 2015
Adding SuppressByMixinFilter to suppress events related to nodes with any of a given set of mixins

Resolves: https://jira.duraspace.org/browse/FCREPO-1456
@ajs6f ajs6f merged commit c04df98 into master Apr 16, 2015
@ajs6f ajs6f deleted the event-suppression branch April 16, 2015 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants