Add SiteMapParser#walkSiteMap(URL,Consumer) #190
Conversation
|
@lucboruta this is really nice. I think a unit test to demonstrate a Consumer action would be ideal. |
|
Cool. Since we don't want unit tests to depend on remote content, I'll overload the first method with a few other methods working on byte arrays, content types, etc., as was done for |
|
Since the different This PR now adds two public convenience methods:
|
| } | ||
| } else { | ||
| final Collection<SiteMapURL> links = ((SiteMap) sitemap).getSiteMapUrls(); | ||
| for (final SiteMapURL url : links) { |
There was a problem hiding this comment.
Just curious: why not write the loop as links.stream().filter(Objects::nonNull).forEach(action);?
There was a problem hiding this comment.
No good reason, it just mirrors the branch for sitemap indices above. I can add a commit with your version if you want.
There was a problem hiding this comment.
No problem. It's a matter of taste whether to use "classic" loops or functional streams.
|
+1 lgtm
|
| */ | ||
| public void walkSiteMap(URL onlineSitemapUrl, Consumer<SiteMapURL> action) throws UnknownFormatException, IOException { | ||
| if (action == null) { | ||
| return; |
There was a problem hiding this comment.
A comment using an slf4j logger will be appreciated here
There was a problem hiding this comment.
Done.
I would usually throw a NullPointerException in this context, but I noticed that methods like SiteMapParser#parseSiteMap(URL) are null-safe, hence the early return statement, for the sake of consistency.
| */ | ||
| public void walkSiteMap(AbstractSiteMap sitemap, Consumer<SiteMapURL> action) throws UnknownFormatException, IOException { | ||
| if (sitemap == null || action == null) { | ||
| return; |
There was a problem hiding this comment.
A comment using an slf4j logger will be appreciated here
Add SiteMapParser#walkSiteMap(URL,Consumer)
|
Resolved conflicts, rebased, updated changelog and merged 8ca46ff. Thanks, @lucboruta! Thanks, for the reviews (@Chaiavi, @lewismc)! |
This PR adds a convenience method to fetch a sitemap from the specified URL, recursively traverse any enclosed sitemap index, and perform the specified action for each sitemap URL. It is strongly inspired by methods like
Iterable#forEachandStream#forEach.The code is based on SitemapParser.java by @Chaiavi, with extra checks to avoid NullPointerExceptions.
I added the method to
SiteMapParserrather thanAbstractSiteMapsince the parser is needed to recurse into sitemap indices.I'm open to suggestions regarding the method's name and null-friendliness.