Fix JsoupDocumentReader dropping relative link URLs - #6803
Open
wantaekchoi wants to merge 1 commit into
Open
Conversation
`JsoupDocumentReader` parses with an empty base URI, so jsoup cannot resolve a relative `href` and `abs:href` yields an empty string. With `includeLinkUrls` enabled, every relative link lands in the `linkUrls` metadata as "". The reference documentation states that the base URI for resolving relative links is taken from URL resources, and the module README promises absolute URLs for every link in the document. Take the base URI from the resource, keeping the empty one for resources that do not resolve to a URL. Signed-off-by: wantaek <wantaekchoi@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
JsoupDocumentReaderparses its resource with an empty base URI:Link extraction then asks jsoup for the absolute form of each
href:abs:resolves against the document base URI. With an empty base there is nothing to resolve against, so a relative link yields an empty string rather than a URL. Reading a page that contains<a href="guide.html">withincludeLinkUrlsenabled produces this metadata:The relative link is not shortened or left relative. It is gone, and nothing reports it.
Two places state the opposite behavior. The reference documentation, under the HTML (JSoup) reader in
etl-pipeline.adoc:And the module README, which lists as a feature:
Commit 82b46d2 introduced the reader, that README line, and that documentation line together. The base URI has been empty since.
Changes
Take the base URI from the resource:
Resources that do not resolve to a URL,
ByteArrayResourceamong them, keep the empty base and behave exactly as before.TextReaderalready treats a resource this way, catchingIOExceptionfromgetURL()when the resource has no URL to give.Absolute links are unaffected, since jsoup returns those unchanged whatever the base is.
This is the only
Jsoup.parsecall in the repository, so no sibling reader carries the same omission.Testing
testWithRelativeLinkUrlsreads a classpath resource holding one relative and one absolute link, then asserts that no entry inlinkUrlsis empty and that the relative link resolved against the resource. Without the change it fails on the empty entry:The existing
ByteArrayResourcetests cover the no-URL path and still pass.passes with 12 tests, 1 skipped, the skip being the pre-existing test that needs an internet connection.