Re: Adding capability in AbsolutePathReferenceVisitor to resolve relative to base tag url (issue1726041)

[jo...@gmail.com]

http://codereview.appspot.com/1726041/diff/6001/7002
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java
(right):

http://codereview.appspot.com/1726041/diff/6001/7002#newcode42
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java:42:
public enum TagsToMakeAbsolute {
nit: since this is already a subclass of AbsolutePathReferenceVisitor
you can probably just call this Tags

http://codereview.appspot.com/1726041/diff/6001/7002#newcode60
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java:60:
TagsToMakeAbsolute(Map<String, String> resourceTags) {
make private

http://codereview.appspot.com/1726041/diff/6001/7002#newcode62
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java:62:
}
nit: missing space here

http://codereview.appspot.com/1726041/diff/6001/7002#newcode69
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java:69:
Map<String, String> tagsToMakeAbsolute;
should be private final

http://codereview.appspot.com/1726041/diff/6001/7002#newcode88
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java:88:
Uri baseUri = getUriToResolveUrisRelativeTo(gadget, node);
just call this getBaseUri, or perhaps getBaseResolutionUri

http://codereview.appspot.com/1726041/diff/6001/7002#newcode171
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java:171:
protected String getBaseHref(Document doc) {
this can probably be private -- when would you see a need to customize?

http://codereview.appspot.com/1726041/diff/6001/7002#newcode177
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java:177:
Attr attr = (Attr) list.item(0).getAttributes().getNamedItem("href");
getAttributes() can return null - either:
A. check this to avoid NPE, or
B. cast list.item(0) to an Element (getElementsByTagName's NodeList is
populated exclusively with them by contract) and use
getAttribute("href") (note that this API returns "" rather than null if
the attr doesn't exist)

http://codereview.appspot.com/1726041/show