TIKA-4766: typed Document parse contract for tika-grpc

[krickert]

Summary

Follow-up to #2916, reshaped per the review there. Instead of mirroring Tika's open metadata taxonomy in protobuf (~5k lines of proto, per-format messages), this PR types the thing that is actually stable: the parsed document. One small contract — document.proto is 208 lines — and format specifics live in per-parser mapping code, never in the wire.

FetchAndParseReply.fields (map<string,string>, field 2, now reserved) is replaced by FetchAndParseReply.document.

How this answers the #2916 review

Concern from #2916	Where it landed
"11k lines to nail down maybe 80% of an open set"	208-line contract; whole PR is ~3.9k lines, most of it mapper code + tests
Clients rebuild when metadata definitions change	A metadata key is now data (extra tail), not schema — add/rename/retype a Tika Property and no client regenerates anything
Lossless catch-all as source of truth	Document.extra carries every Tika key, multivalue-preserving; a test asserts nothing is dropped
Special handling only for DC + core props	DocumentMetadata types only the bounded cross-format fields (title, authors, dates as Timestamp, counts, dimensions, rights)
Break it into individually reviewable tasks	This PR is the contract only; see "Deliberately not in this PR"

The shape

1. Content tree: markdown (the same render ToMarkdownContentHandler already produces since TIKA-4730) plus blocks — that markdown parsed once, format-agnostically, into a structured tree of headings/paragraphs/lists/tables/code blocks/inline runs (CommonMark + GFM, a spec that does not churn). This is what a downstream NLP/RAG/embeddings consumer actually wants: typed tables and sections, not a string to re-parse.
2. Typed common metadata: DocumentMetadata, grouped by concern, not by source format. Dates are Timestamps, counts are ints — not strings that 12 language clients each re-parse.
3. Tagged tail: extra — every remaining key, typed only where Tika's own Property declares a type (integer/real/boolean/date), string otherwise, never guessed.
4. embedded recurses: a PDF with an embedded image is a parent Document with a fully typed child — no forcing two formats into one bucket (this was the oneof problem from TIKA-4766: Typed parse response grpc #2916).
5. Adding a format = adding a DocumentTransformer (see tika-grpc-mapper/docs/EXTENSIONS.md); PdfDocumentTransformer is 65 lines and the wire contract does not move.

Deliberately not in this PR (follow-ups, each its own PR)

• Pluggable external parsers: registering a third-party gRPC service whose output rides along on the Document as a google.protobuf.Any — so wildly different result shapes (e.g. a document-layout model's tree) never require Tika to model them. Built and tested on a branch; kept out to keep this reviewable.
• A Markdown parser for .md input files (separate JIRA).
• Richer typed fields, if and only if real cross-format demand appears — they'd be additive optional fields, compatible both directions.

Open decisions where reviewer preference wins

1. Tail shape: repeated MetadataField with a typed value oneof (as implemented) vs the map<string, StringList> suggested in TIKA-4766: Typed parse response grpc #2916. The typed-where-declared tail preserves types without guessing; the map is maximally churn-proof. Swapping is a one-message change — happy to go either way.
2. markdown + blocks both: today both ship (string render + structured tree). If payload size matters, a per-request flag choosing one is easy.
3. Hard removal vs staged: fields is hard-removed (4.0, nothing consumes it yet); can switch to deprecate-then-remove if preferred.

Client migration

Before	After
fields["X-TIKA:content"]	document.markdown (or walk document.blocks)
fields["Content-Type"]	document.content_type
Ad hoc title/author/date strings	document.metadata.title / .authors / .created (Timestamp)
Any other key	document.extra (typed by declared Property type, string otherwise)

Test plan

• ./mvnw -pl tika-grpc-api,tika-grpc-mapper,tika-grpc test — green (transformer tests against real parse fixtures per format, block-tree tests, DocumentBuilder envelope/status/embedded tests, server tests reading FetchAndParseReply.document)
• tika-grpc-api jar bundles META-INF/org.apache.tika.grpc.v1.descriptors (verified: contains document.proto)
• e2e tika-grpc-e2e-test compiles against the new API
• CI

Downstream context: this contract is what the OpenNLP gRPC work (OPENNLP-1833) will consume as input — Tika parse → typed document → NLP/embeddings without re-parsing strings.

[krickert]

Follow-up work is broken out into TIKA-4771 (pluggable external parsers) and TIKA-4772 (document event streaming) rather than growing this PR.

[krickert]

@nddipiazza @tballison what do you think of this design instead? far less fields - ability to roll your own protobuf model in the future. Best of both worlds. Document structure is very markdown-friendly. I'll make the output of this be able to be the input of the grpc OpenNLP grpc server.

[nddipiazza]

Review: e2e test coverage for the new typed Document contract

Reviewed the design overall — the shift from mirroring Tika's metadata taxonomy in protobuf to a small, stable document.proto (208 lines) plus per-parser mapper code is a solid answer to the #2916 feedback. A few things worth addressing before merge:

Testing gap: e2e coverage does not exercise the new contract

The only change in tika-e2e-tests/tika-grpc is a 2-line fix in HandlerTypeTest.java:

- String htmlContent = htmlReply.getFieldsMap().get("X-TIKA:content");
+ String htmlContent = htmlReply.getDocument().getMarkdown();

(same for the TEXT handler case). This is a compile-fix to keep the pre-existing assertion working against the new API — it only proves document.markdown is non-empty over a real gRPC round-trip for two handler types.

It does not exercise, end-to-end, through the live server:

• DocumentMetadata typed fields (title/authors/dates/counts) — no e2e assertion anywhere
• extra tagged-tail typing (int/bool/date/string) — no e2e assertion
• blocks structured content tree (headings/tables/lists/code) — untouched by any e2e test
• embedded recursion for container formats (e.g. an Office doc with an embedded image) — no e2e coverage at all
• format_category routing hint — untouched

The other e2e tests (FileSystemFetcherTest, IgniteConfigStoreTest, ExternalTestBase) only ever inspect getFetchKey()/getStatus() on FetchAndParseReply — none of them look at getDocument(), so the bulk-corpus/streaming/ignite e2e paths give zero signal on the new typed contract.

The real proof of correctness for the mapping logic lives entirely in tika-grpc-mapper's unit tests (DocumentBuilderTest, MarkdownBlockTreeBuilderTest, one test class per transformer), which feed fixture-derived Metadata/markdown into DocumentBuilder in-process. That's good for the mapping logic itself, but it bypasses the actual gRPC wire serialization, the live server (TikaGrpcServerImpl), the pipes client, and fetcher plumbing entirely.

Ask: add (or extend HandlerTypeTest) at least one e2e case per format that fetches a real file through the live gRPC server and asserts on document.metadata (a couple of typed fields), document.extra (at least one tagged key), and one document.embedded case (e.g. an Office/PDF file with an embedded image) — not just that markdown is non-empty. Right now this new, larger surface area has no live-server coverage at all.

Other findings from code inspection

1. Likely-dead error path: DocumentBuilder.build()'s primary == null branch (returns FAILED with "No metadata returned from parse") looks unreachable in production — the only caller, TikaGrpcServerImpl, always passes a non-null Metadata (tikaMetadata = new Metadata() as fallback when emitData()/metadata list is empty). Worth double-checking this is intentional, since a real fetch failure with no emit data currently produces an "empty-but-success-shaped" Document rather than hitting this explicit error branch.
2. Duplication: 7 of 8 format transformers (Generic, Pdf, Html, Image, Office, Rtf, Epub) repeat an identical block mapping TITLE/DESCRIPTION/CREATOR/SUBJECT/LANGUAGE/CREATED/MODIFIED. Consider factoring this into a shared helper in TransformSupport called by each transformer, keeping only the format-specific additions per-transformer.
3. Minor: parseTimeMs in TikaGrpcServerImpl measures fetch+parse combined (timer starts before pipesClient.process()), though the field's doc/intent reads as parse-only time.

Nice work on the overall shape of the contract — the main blocker from my read is the e2e coverage gap above.

[nddipiazza]

Overall this is a solid design and a much better-scoped answer to the #2916 feedback: a small stable 208-line proto, format logic pushed into per-parser Java transformers, and a lossless typed+tagged tail. Requesting changes mainly on test coverage for the new contract, plus a couple of correctness/maintainability issues found while reading the mapper code (see inline comments).

[krickert]

@nddipiazza great feedback..

Review: e2e test coverage for the new typed Document contract

Reviewed the design overall — the shift from mirroring Tika's metadata taxonomy in protobuf to a small, stable document.proto (208 lines) plus per-parser mapper code is a solid answer to the #2916 feedback. A few things worth addressing before merge:

Testing gap: e2e coverage does not exercise the new contract

The only change in tika-e2e-tests/tika-grpc is a 2-line fix in HandlerTypeTest.java:

- String htmlContent = htmlReply.getFieldsMap().get("X-TIKA:content");
+ String htmlContent = htmlReply.getDocument().getMarkdown();

(same for the TEXT handler case). This is a compile-fix to keep the pre-existing assertion working against the new API — it only proves document.markdown is non-empty over a real gRPC round-trip for two handler types.

It does not exercise, end-to-end, through the live server:

• DocumentMetadata typed fields (title/authors/dates/counts) — no e2e assertion anywhere
• extra tagged-tail typing (int/bool/date/string) — no e2e assertion
• blocks structured content tree (headings/tables/lists/code) — untouched by any e2e test
• embedded recursion for container formats (e.g. an Office doc with an embedded image) — no e2e coverage at all
• format_category routing hint — untouched

The other e2e tests (FileSystemFetcherTest, IgniteConfigStoreTest, ExternalTestBase) only ever inspect getFetchKey()/getStatus() on FetchAndParseReply — none of them look at getDocument(), so the bulk-corpus/streaming/ignite e2e paths give zero signal on the new typed contract.

The real proof of correctness for the mapping logic lives entirely in tika-grpc-mapper's unit tests (DocumentBuilderTest, MarkdownBlockTreeBuilderTest, one test class per transformer), which feed fixture-derived Metadata/markdown into DocumentBuilder in-process. That's good for the mapping logic itself, but it bypasses the actual gRPC wire serialization, the live server (TikaGrpcServerImpl), the pipes client, and fetcher plumbing entirely.

Ask: add (or extend HandlerTypeTest) at least one e2e case per format that fetches a real file through the live gRPC server and asserts on document.metadata (a couple of typed fields), document.extra (at least one tagged key), and one document.embedded case (e.g. an Office/PDF file with an embedded image) — not just that markdown is non-empty. Right now this new, larger surface area has no live-server coverage at all.

Other findings from code inspection

1. Likely-dead error path: DocumentBuilder.build()'s primary == null branch (returns FAILED with "No metadata returned from parse") looks unreachable in production — the only caller, TikaGrpcServerImpl, always passes a non-null Metadata (tikaMetadata = new Metadata() as fallback when emitData()/metadata list is empty). Worth double-checking this is intentional, since a real fetch failure with no emit data currently produces an "empty-but-success-shaped" Document rather than hitting this explicit error branch.
2. Duplication: 7 of 8 format transformers (Generic, Pdf, Html, Image, Office, Rtf, Epub) repeat an identical block mapping TITLE/DESCRIPTION/CREATOR/SUBJECT/LANGUAGE/CREATED/MODIFIED. Consider factoring this into a shared helper in TransformSupport called by each transformer, keeping only the format-specific additions per-transformer.
3. Minor: parseTimeMs in TikaGrpcServerImpl measures fetch+parse combined (timer starts before pipesClient.process()), though the field's doc/intent reads as parse-only time.

Nice work on the overall shape of the contract — the main blocker from my read is the e2e coverage gap above.

My bad; the e2e suite really was only asserting "markdown is non-empty". Added three live-server cases to HandlerTypeTest:

• PDF (testPDF.pdf): asserts typed document.metadata (title, page count), a boolean-typed document.extra key (pdf:encrypted), document.blocks, and format_category.
• HTML (sample.html): asserts typed title, FORMAT_WEB, and a string-typed extra key.
• Embedded (test_recursive_embedded.docx): asserts FORMAT_OFFICE, typed created/modified dates, and that document.embedded recurses with per-child metadata.

Two things fell out of writing these:

1. The extra-tail typing was actually broken over the live server - MetadataTagger relies on Property.get(key), but Tika's property registry is populated lazily by class init, so in the server JVM pdf:encrypted came back as a string, not a boolean. The unit tests never caught it because they happen to load the vocabulary classes first. Fixed by force-loading the tika-core metadata vocabularies in a static initializer in MetadataTagger, and the new e2e test now pins the behavior.
2. The e2e configs needed grpc.allowComponentManagement / allowPerRequestConfig opt-ins after TIKA-4764 landed on main, otherwise every test dies with PERMISSION_DENIED - updated the three config JSONs.

Note the CI e2e job doesn't currently execute these (the -pl tika-e2e-tests -am verify invocation only builds the aggregator POM and no plugins are provisioned) - that's a pre-existing infra gap, but the suite passes locally against a live server.

[krickert]

Thanks for the review @nddipiazza, all four points addressed:

• e2e tests now exercise the typed contract over a live server (PDF/HTML/embedded DOCX), which flushed out and fixed a real bug: lazy Property registry init made typed extra keys degrade to strings in the server JVM.
• The primary == null branch is now reachable (server passes null on empty output) and correctly treats EMPTY_OUTPUT as success.
• Common field mapping deduped into TransformSupport.mapCommonFields.
• Renamed parse_time_ms to fetch_parse_time_ms to match what's actually measured.

Also merged latest main (the lombok removal and split-packages changes). Full build plus mapper, grpc, and e2e suites pass locally.