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
Refactor fixity-related methods #338
Conversation
This feature was only useful for chained cache loaders on a single host (e.g. not across clusters, and not for other types of binary stores).
serialization The content location may be expensive to calculate (some cluster configurations), is often unnecessary for day-to-day operations, etc
…f of the whole CacheEntry
stored - getFixity() should return a collection of fixity results that provide one or more fixity results for all copies of the binary data - simplify the infinispan distributed executation calls for ISPN-backed binary stores
final NodeType primaryNodeType = node.getPrimaryNodeType(); | ||
nodeTypesB.add(primaryNodeType); | ||
|
||
if (primaryNodeType != null && primaryNodeType.getSupertypes() != null) { |
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.
Is primaryNodeType ever null?
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.
In practice, no. In our tests, we devote hundreds of lines to mocking out all those chained calls. I don't think it does any harm, and would clean our tests up considerably.
Check out any unit test that calls the NodeRdfContext, for example, e.g. https://github.com/futures/fcrepo4/blob/refactor-fixity/fcrepo-kernel/src/test/java/org/fcrepo/kernel/rdf/impl/HierarchyRdfContextTest.java#L204
I was going to have ReferencesRdfContext extend NodeRdfContext, but all the mocking I needed to do that was just too much trouble.
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.
Fair enough. Have you had a chance to investigate if jcr-mock could help in this situation?
https://github.com/tacitknowledge/jcr-mock
I wonder if LowLevelCacheEntry.java can now be removed in this refactoring? |
@@ -216,8 +196,7 @@ public RdfStream getFixityResultsModel(final IdentifierTranslator subjects, | |||
* @throws RepositoryException | |||
*/ | |||
@Override | |||
public Collection<FixityResult> runFixityAndFixProblems( | |||
final Datastream datastream) throws RepositoryException { | |||
public Collection<FixityResult> runFixityAndFixProblems(final Datastream datastream) throws RepositoryException { |
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.
This method should probably be renamed to reflect the fact that we are no longer "fixing problems".
Resolved with: ae8d5b0...c719b68 Addresses: https://www.pivotaltracker.com/story/show/70699932 |
The commits below can (and should) be squashed before merging. They've been separated out for understanding all the moving parts, but are probably unable to stand alone.
Most of this work is in preparation for Modeshape 4 / Infinispan 6, which significantly change some of the APIs we've been using (for the better). This is an attempt to refactor the existing code in a way that will work well enough with both what we have now and prepare us for Modeshape 4.
One controversial change might be 962458b - removing self-healing from fixity failures. The way we're currently doing it is not very good, and has no chance of working going forward.
https://www.pivotaltracker.com/story/show/70699932