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.
--
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]
