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

Applied code changes to fresh clone. #596

Closed
wants to merge 2 commits into from

Conversation

Projects
None yet
2 participants
@nikhiltri
Copy link
Contributor

commented Oct 28, 2014

No description provided.

createURI(getRDFNamespaceForJcrNamespace(JCR_NAMESPACE) +
"mixinTypes");

public static final Function<RDFNode, String> stringConverter = new Function<RDFNode, String>() {

This comment has been minimized.

Copy link
@awoods

awoods Oct 29, 2014

Member

This function should be private.

*/
public static Iterator<String> getAllValuesForPredicate(final Model rdf,
final Node subject, final Node predicate) {
final NodeIterator statements =

This comment has been minimized.

Copy link
@awoods

awoods Oct 29, 2014

Member

As a point of readability/clarity, this NodeIterator is actually an iterator of "objects" not "statements".
It may be more clear to name the variable accordingly.

* @param rdf
* @param subject
* @param predicate
* @return first value for the given predicate or null if not found

This comment has been minimized.

Copy link
@awoods

awoods Oct 29, 2014

Member

Documentation should read:
@return all values ...

@@ -109,6 +119,27 @@
private static final Logger LOGGER =
getLogger(StreamingBaseHtmlProvider.class);

protected final Predicate<NodeType> acceptWhenTemplateExists = new Predicate<NodeType>() {

This comment has been minimized.

Copy link
@awoods

awoods Oct 29, 2014

Member

Change from protected to private.

}
};

protected final Predicate<String> acceptWhenTemplateMapContainsKey = new Predicate<String>() {

This comment has been minimized.

Copy link
@awoods

awoods Oct 29, 2014

Member

Change from protected to private.

primaryNodeType.getName();

// Create a list of the primary type and all its parents
final List<NodeType> nodeTypesList = new ArrayList<NodeType>();

This comment has been minimized.

Copy link
@awoods

awoods Oct 29, 2014

Member

Change from:
final List<NodeType> nodeTypesList = new ArrayList<NodeType>();
to:
final List<NodeType> nodeTypesList = new ArrayList<>();


@SuppressWarnings({"unchecked", "rawtypes"})
@Test
public void testWriteToWithParentTemplate() throws WebApplicationException,

This comment has been minimized.

Copy link
@awoods

awoods Oct 29, 2014

Member

Ideally, this test would exercise the block in StreamingBaseHtmlProvider.getTemplate():

           final String mixin = Iterables.find(copy, acceptWhenTemplateMapContainsKey, null);
           if (mixin != null) {
               LOGGER.debug("Matched mixin type: {}", mixin);
              template = templatesMap.get(mixin);
           }

However, the test as written results in mixin being null and the if block above being skipped.

This comment has been minimized.

Copy link
@nikhiltri

nikhiltri Oct 29, 2014

Author Contributor

I changed the predicate of testData2 to mixinTypesPredicate (line 108 above) so the mixin is found here.

final ImmutableList<String> copy = copyOf(mixinTypes);
final String mixin = Iterables.find(copy, acceptWhenTemplateMapContainsKey, null);
if (mixin != null) {
LOGGER.debug("Matched mixin type: {}", mixin);

This comment has been minimized.

Copy link
@awoods

awoods Oct 29, 2014

Member

When I run the test with a debugger (or simply look for the debug statement here "Matched mixin type"), this block is not being executed... which is one of the main reasons for this PR.

@awoods

This comment has been minimized.

Copy link
Member

commented Nov 4, 2014

Resolved with: e3da11f

@awoods awoods closed this Nov 4, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.