abhinav-phi opened a new pull request, #2112: URL: https://github.com/apache/stormcrawler/pull/2112
Fixes #2108 ## The problem `ParseResult.get(url)` is documented as a convenience accessor, but on a miss it creates a `ParseData` and **stores it in the map** before returning it. `put(url, key, value)` and `set(url, metadata)` go through the same path. Both parser bolts (`JSoupParserBolt` and the Tika `ParserBolt`) iterate the whole map and emit one document tuple per entry, so an entry created by a lookup is emitted like a parsed document. The auto-created entry also has a `null` content field, while the `(text, metadata)` constructor sets an empty byte array, so the two paths do not agree on what an empty document looks like. Nothing in the tree hits this today: every non-test caller passes the URL of the page being parsed, so the map only ever gains entries for documents that exist. The problem is the API. A parse filter written against the Javadoc may reasonably call `get()` to check whether an earlier filter produced a sub-document; if the URL it probes comes from page content, the probe creates an entry that the bolt then emits as a document. Sub-documents are not filtered by URLFilters (only outlinks are), so that entry reaches the indexer as a document URL with no content. ## What this PR changes ### 1. `ParseResult` now separates lookup from creation * **`getIfPresent(url)`** — new read-only accessor: returns the `ParseData` stored for the URL or `null`, and never modifies the `ParseResult`. This is what a parse filter should use to probe whether a sub-document already exists. * **`getOrCreate(url)`** — the creating accessor used by `put()`, `set()` and the parser bolts for the parent URL; modifications made to the returned instance are stored in the `ParseResult`. * **`get(url)`** is kept as a `@Deprecated` delegate to `getOrCreate()`, so parse filters outside this repository keep compiling and behaving identically. Its Javadoc now states explicitly that it creates an entry and points to the two methods above. We deliberately did **not** change the behaviour of `get()` itself: silently turning "creates an entry" into "returns `null`" would either throw an NPE or quietly lose metadata for existing external filters that build sub-documents via `parse.get(url).getMetadata()`. Deprecating it makes the ambiguity visible at the call site without breaking anyone. All in-tree callers of `parse.get(url)` — the parse filters in `core` (`CommaSeparatedToMultivaluedMetadata`, `CollectionTagger`, `DomainParseFilter`, `LDJsonParseFilter`, `LinkParseFilter`, `MD5SignatureParseFilter`, `MimeTypeNormalization`, `XPathFilter`), the `jsoup` filters, `LanguageID` (langid), `JsRenderingDetector` (playwright) and both parser bolts — have been migrated to `getOrCreate(url)`. This is a mechanical, behaviour-identical change; the deprecated method remains only for external compatibility. ### 2. Backstop in the parser bolts `JSoupParserBolt` and the Tika `ParserBolt` now skip entries which carry **no content, no text and no metadata** when they emit, so an empty entry can no longer reach the indexer as a content-less document tuple. Sub-documents created by filters with actual content, text or metadata are still emitted exactly as before. ### 3. Consistent `ParseData` constructors The no-arg and metadata-only `ParseData` constructors now set `content` to an empty byte array, like the `(text, metadata)` constructor, so no code path can emit a `null` content field. ### 4. Documentation The custom parse filter example in `docs/src/main/asciidoc/extending.adoc` now uses `getOrCreate(url)`. ## Tests * **`ParseResultTest`** (new, core): verifies that `getIfPresent()` returns `null` for an unknown URL without mutating the result and returns the stored instance otherwise; that `getOrCreate()`, `put()` and `set()` create entries and `getOrCreate()` returns the same instance on repeated calls; that the deprecated `get()` keeps its historic behaviour; and that all `ParseData` constructors produce a non-`null`, empty content array. * **`SubDocumentsFilterTest#testEmptySubDocumentsAreNotEmitted`** (new, core): a parse filter creates an entry for a URL that was never parsed; the `JSoupParserBolt` emits only the parent document. * **`ParserBoltTest#testEmptySubDocumentsAreNotEmitted`** (new, tika): same backstop check for the Tika `ParserBolt`. * **`SubDocumentsParseFilter`** (test fixture): the sub-documents it creates now carry a metadata entry — since empty entries are no longer emitted by the bolts, the fixture creates realistic sub-documents and still asserts that they are emitted. ## Verification * `mvn -pl core test` — 425 tests, 0 failures. * `mvn -pl external/tika,external/langid,external/playwright -am test` — all modules build; `JsRenderingDetectorTest` (12 tests) passes. * Code formatted with google-java-format (AOSP) via `git-code-format-maven-plugin`; Apache RAT checks applied to the new files. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
