[
https://issues.apache.org/jira/browse/TIKA-4766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18094374#comment-18094374
]
ASF GitHub Bot commented on TIKA-4766:
--------------------------------------
krickert commented on code in PR #2921:
URL: https://github.com/apache/tika/pull/2921#discussion_r3538981657
##########
tika-e2e-tests/tika-grpc/src/test/java/org/apache/tika/pipes/filesystem/HandlerTypeTest.java:
##########
@@ -313,7 +313,7 @@ void testParseContextJson() throws Exception {
Assertions.assertEquals("PARSE_SUCCESS", htmlReply.getStatus(),
"Parse should succeed with HTML handler type");
- String htmlContent =
htmlReply.getFieldsMap().get("X-TIKA:content");
+ String htmlContent = htmlReply.getDocument().getMarkdown();
Review Comment:
Good catch, the e2e suite really was only asserting that 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 the `grpc.allowComponentManagement` /
`allowPerRequestConfig` opt-ins after TIKA-4764 landed on main, otherwise every
test dies with `PERMISSION_DENIED`. Updated the three config JSONs.
Note that 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.
##########
tika-grpc-mapper/src/main/java/org/apache/tika/grpc/mapper/DocumentBuilder.java:
##########
@@ -0,0 +1,149 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tika.grpc.mapper;
+
+import java.time.Instant;
+import java.util.List;
+
+import org.apache.tika.grpc.mapper.content.MarkdownBlockTreeBuilder;
+import org.apache.tika.grpc.mapper.transform.DocumentTransformers;
+import org.apache.tika.grpc.mapper.transform.FormatCategoryDetector;
+import org.apache.tika.grpc.v1.Document;
+import org.apache.tika.grpc.v1.ParseStatus;
+import org.apache.tika.grpc.v1.SourceOrigin;
+import org.apache.tika.metadata.Metadata;
+import org.apache.tika.metadata.TikaCoreProperties;
+
+/**
+ * Builds a {@link Document} from Tika's parse output: the pipes default
content handler
+ * renders the extracted content as markdown, so this class parses that
markdown into
+ * the structured {@code Block} tree once (format-agnostic) and delegates
metadata
+ * mapping to {@link DocumentTransformers} (format-specific, one transformer
per
+ * concern). Embedded documents recurse into {@link
Document#getEmbeddedList()}.
+ */
+public final class DocumentBuilder {
+
+ private static final DocumentTransformers TRANSFORMERS =
DocumentTransformers.defaults();
+
+ private DocumentBuilder() {
+ }
+
+ /** Maps Tika parse output (primary metadata plus optional embedded
metadata list) to {@link Document}. */
+ public static Document build(Metadata primary, List<Metadata> allMetadata,
String markdownBody,
+ String docId, String pipesStatus, long
parseTimeMs) {
+ if (primary == null) {
Review Comment:
You're right, it was unreachable. Wired the server to pass `null` when the
metadata list is truly empty (`metadataList.isEmpty() ? null :
metadataList.get(0)`), so the branch is now the real path for empty output
instead of building a Document from a blank `Metadata`.
While making it reachable I also fixed a semantic bug in that branch:
`EMPTY_OUTPUT` is a success status in pipes, so it no longer unconditionally
attaches the "No metadata returned from parse" error. That message is only
added when the mapped status is actually non-success. Covered by a new unit
test (`emptyOutputWithNoMetadataIsSuccessNotFailure`) plus an updated
failure-path test using a real `FETCH_EXCEPTION` status.
> Replace tika-grpc fields map with a typed Document parse contract
> -----------------------------------------------------------------
>
> Key: TIKA-4766
> URL: https://issues.apache.org/jira/browse/TIKA-4766
> Project: Tika
> Issue Type: New Feature
> Components: tika-pipes
> Affects Versions: 4.0.0
> Reporter: Kristian Rickert
> Priority: Major
> Labels: grpc, pipes, protobuf
>
> Replace the flat {{FetchAndParseReply.fields}} map ({{map<string,string>}})
> with a typed parse result.
> Approach (reshaped from the original {{ParseResponse}} design after review in
> [PR #2916|https://github.com/apache/tika/pull/2916]): a single small
> {{Document}} proto (~200 lines) rather than per-format metadata messages.
> * *Content*: a structured markdown block tree -- headings, paragraphs, lists,
> tables, code blocks, inline runs (CommonMark + GFM) -- plus the rendered
> markdown string {{ToMarkdownContentHandler}} already produces (TIKA-4730).
> * *Typed common metadata*: title, authors, keywords, languages,
> created/modified as {{Timestamp}}s, page/word/character counts, dimensions,
> rights.
> * *Lossless tagged tail*: every remaining metadata key,
> multivalue-preserving, typed only where Tika's own {{Property}} declares a
> type, string otherwise -- never guessed.
> * *Embedded documents* recurse as fully typed child {{Document}}s.
> * Format specifics live in per-parser {{DocumentTransformer}} code
> ({{tika-grpc-mapper}}), never in the wire contract, so metadata churn never
> forces a client rebuild.
> Modules: {{tika-grpc-api}} (proto + generated messages + bundled
> {{FileDescriptorSet}}), {{tika-grpc-mapper}}, {{tika-grpc}} integration.
> Breaking change for clients reading {{fields}} (field number reserved).
> PR: [https://github.com/apache/tika/pull/2921] (supersedes
> [#2916|https://github.com/apache/tika/pull/2916])
> Follow-ups will be tracked in separate issues: pluggable external parsers
> (opaque {{Any}} extension results from registered gRPC services), and a
> Markdown input parser.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
