You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "janhoy (via GitHub)" <gi...@apache.org> on 2025/09/19 15:14:02 UTC
[PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
janhoy opened a new pull request, #3670:
URL: https://github.com/apache/solr/pull/3670
https://issues.apache.org/jira/browse/SOLR-7632
This work builds on the one in #3361 but instead of making a new module, we add it as a capability to the existing extraction handler through specifying `extraction.backend=tikaserver`.
This first required refactoring extraction handler to detach it from the Tika-v1 API. There is a new interface `ExtractionBackend` that takes generic `ExtractionRequest` object in and returns an `ExtractionResult` bean, and a new `LocalTikaExtractionBackend` implementation that encapsulates all Tikav1 api handling. This implementation can be deprecated, and in Solr 10, the `tikaserver` one can be made default.
All existing tests pass. New tests are added using TestContainers to spin up Tika.
Note: Most of the coding was done by JetBrains Junie, so reviewers may want to ensure nothing fancy has slipped into the code.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3315387172
Any luck with security manager?? I had many difficulties
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2373235913
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/DummyExtractionBackend.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.InputStream;
+
+/** Dummy backend that emits predictable test data without actually parsing input content. */
Review Comment:
```suggestion
/** Simple backend that emits predictable dummy data without actually parsing input content. */
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2373729413
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/DummyExtractionBackend.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.InputStream;
+
+/** Dummy backend that emits predictable test data without actually parsing input content. */
Review Comment:
Or if "Test Containers" turns into not being viable...
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3319380473
Yea, that’s annoying. Perhaps we could disable JSM for this test or for tests in the entire module?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3313050350
Exciting!
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2408996844
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java:
##########
@@ -0,0 +1,220 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.InputStream;
+import java.time.Duration;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.tika.sax.BodyContentHandler;
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.InputStreamRequestContent;
+import org.eclipse.jetty.client.InputStreamResponseListener;
+import org.eclipse.jetty.client.Request;
+import org.eclipse.jetty.client.Response;
+import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler;
+import org.xml.sax.helpers.DefaultHandler;
+
+/** Extraction backend using the Tika Server. It uses a shared Jetty HttpClient. */
+public class TikaServerExtractionBackend implements ExtractionBackend {
+ private static volatile HttpClient SHARED_CLIENT;
+ private static volatile ExecutorService SHARED_EXECUTOR;
+ private static final Object INIT_LOCK = new Object();
+ private static volatile boolean INITIALIZED = false;
+ private static volatile boolean SHUTDOWN = false;
+ private final String baseUrl; // e.g., http://localhost:9998
+ private final Duration timeout = Duration.ofMinutes(3);
Review Comment:
Yes! If you enable OCR scanning, and have multiple pages, it could be quite a while!
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2408998748
##########
solr/modules/extraction/gradle.lockfile:
##########
@@ -99,7 +102,8 @@ javax.inject:javax.inject:1=annotationProcessor,errorprone,testAnnotationProcess
javax.measure:unit-api:1.0=compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
joda-time:joda-time:2.14.0=compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
junit:junit:4.13.2=jarValidation,testCompileClasspath,testRuntimeClasspath
-net.java.dev.jna:jna:5.12.1=compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
+net.java.dev.jna:jna:5.12.1=compileClasspath,runtimeClasspath,runtimeLibs
+net.java.dev.jna:jna:5.13.0=jarValidation,testCompileClasspath,testRuntimeClasspath
Review Comment:
no idea, but seems odd..
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2413075579
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractingDocumentLoader.java:
##########
@@ -75,40 +55,33 @@ public class ExtractingDocumentLoader extends ContentStreamLoader {
/** Extract Only supported format. Default */
public static final String XML_FORMAT = "xml";
- /** XHTML XPath parser. */
- private static final XPathParser PARSER = new XPathParser("xhtml", XHTMLContentHandler.XHTML);
-
final SolrCore core;
final SolrParams params;
final UpdateRequestProcessor processor;
final boolean ignoreTikaException;
- protected AutoDetectParser autoDetectParser;
+ final boolean backCompat;
private final AddUpdateCommand templateAdd;
- protected TikaConfig config;
- protected ParseContextConfig parseContextConfig;
protected SolrContentHandlerFactory factory;
+ protected ExtractionBackend backend;
public ExtractingDocumentLoader(
SolrQueryRequest req,
UpdateRequestProcessor processor,
- TikaConfig config,
- ParseContextConfig parseContextConfig,
- SolrContentHandlerFactory factory) {
+ SolrContentHandlerFactory factory,
+ ExtractionBackend backend) {
this.params = req.getParams();
this.core = req.getCore();
- this.config = config;
- this.parseContextConfig = parseContextConfig;
this.processor = processor;
+ this.backCompat = params.getBool(ExtractingParams.BACK_COMPATIBILITY, true);
Review Comment:
For some reason, this does not work as expected. It will pick up the back-compat parameter on the update request, but if the same is configured in the handler definition in solrconfig.xml, it is not part of `req.getParams()`. Also, we do not have access to `initParams` namedlist here, only in the handler.
This means that currently back-compat is on by default (I believe default should be off), and can only be modified per request.
I tried to instead read this parameter in `load()`, but even tehre, the SolrQueryRequest object does not contain config from initParams in solrconfig. I could do a hack and pass initParams in the ExtractingDocumentLoader constructor and write code to fall back to reading that, but I was sure that Solr would handle merging params from init and request automatically. Any insight?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2425391376
##########
gradle/libs.versions.toml:
##########
@@ -140,6 +140,8 @@ jersey-containers = "2.39.1"
# @keep for version alignment
jetbrains-annotations = "26.0.2"
# @keep for version alignment
+jna = "5.13.0"
Review Comment:
@malliaridis It worked to pin the jna version here, thanks
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2428703518
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractingRequestHandler.java:
##########
@@ -81,25 +64,116 @@ public void inform(SolrCore core) {
parseContextConfig =
new ParseContextConfig(core.getResourceLoader(), parseContextConfigLoc);
}
+
+ // Always create local backend
+ this.localBackend = new LocalTikaExtractionBackend(core, tikaConfigLoc, parseContextConfig);
+
+ // Optionally create Tika Server backend if URL configured
+ String tikaServerUrl = (String) initArgs.get(ExtractingParams.TIKASERVER_URL);
+ if (tikaServerUrl != null) {
+ int timeoutSecs = 0;
+ Object initTimeout = initArgs.get(ExtractingParams.TIKASERVER_TIMEOUT_SECS);
+ if (initTimeout != null) {
+ try {
+ timeoutSecs = Integer.parseInt(String.valueOf(initTimeout));
+ } catch (NumberFormatException nfe) {
+ throw new SolrException(
+ ErrorCode.SERVER_ERROR,
+ "Invalid value for '"
+ + ExtractingParams.TIKASERVER_TIMEOUT_SECS
+ + "': "
+ + initTimeout,
+ nfe);
+ }
+ }
+ Object maxCharsObj = initArgs.get(ExtractingParams.TIKASERVER_MAX_CHARS);
+ long maxCharsLimit = TikaServerExtractionBackend.DEFAULT_MAXCHARS_LIMIT;
+ if (maxCharsObj != null) {
+ try {
+ maxCharsLimit = Long.parseLong(String.valueOf(maxCharsObj));
+ } catch (NumberFormatException nfe) {
+ throw new SolrException(
+ ErrorCode.SERVER_ERROR,
+ "Invalid value for '"
+ + ExtractingParams.TIKASERVER_MAX_CHARS
+ + "': "
+ + maxCharsLimit);
+ }
+ }
+ this.tikaServerBackend =
+ new TikaServerExtractionBackend(tikaServerUrl, timeoutSecs, initArgs, maxCharsLimit);
+ }
+
+ // Choose default backend name
+ String backendName = (String) initArgs.get(ExtractingParams.EXTRACTION_BACKEND);
+ this.defaultBackendName =
+ (backendName == null || backendName.trim().isEmpty())
+ ? LocalTikaExtractionBackend.NAME
+ : backendName;
+
+ // If the default is tikaserver but no URL configured, fall back to local
+ if (TikaServerExtractionBackend.NAME.equals(this.defaultBackendName)
+ && this.tikaServerBackend == null) {
+ this.defaultBackendName = LocalTikaExtractionBackend.NAME;
Review Comment:
Do not silently fall back to `local` if `tikaserver` is explicitly configured and `tikaserver.url` is missing. Instead, throw an exception.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3381066537
@epugh Do you see value in splitting this PR into first an introduction of backend concept and then the tikaserver backend, or shall we bring this PR (which is now all green) to completion and backport in one go? If folks find it hard / intimidating to review due to its size, it may make sense to split. But if it is manageable, it is faster to stay in this PR for everything.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2404180680
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractionRequest.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/** Immutable request info needed by extraction backends. */
+public class ExtractionRequest {
+ public final String streamType; // explicit MIME type (optional)
+ public final String resourceName; // filename hint
+ public final String contentType; // HTTP content-type header
+ public final String charset; // derived charset if available
+ public final String streamName;
+ public final String streamSourceInfo;
+ public final Long streamSize;
+ public final String resourcePassword; // optional password for encrypted docs
+ public final java.util.LinkedHashMap<java.util.regex.Pattern, String>
+ passwordsMap; // optional passwords map
+ public final String extractFormat;
+ public final boolean recursive;
+ public final Map<String, String> tikaRequestHeaders = new HashMap<>();
Review Comment:
The `recursive` and `tikaRequestHeaders` properties are only used by TikaServer backend. Local backend has always used recursive parsing by default. The `tikaRequestHeaders` are tightly coupled to the TikaServer request and makes no sense for local. It is currently only used to pass a header for a unit test.
We could generify these so they make more sense for any backend, but it is also somewhat inflexible as there may be tons of request headers you could pass to TikaServer. But then, there is no way for end user to pass any header, it is all in code, so headers could probably instead be deduced from some `features` map...
Alternatively we rename these to include `tikaServer` in their names and document them to be for that backend only?
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/XmlSanitizingReader.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.IOException;
+import java.io.Reader;
+
+/**
+ * Minimal reader that drops only null numeric character entities from the stream.
+ *
+ * <p>Recognizes decimal and hexadecimal numeric entities that resolve to code point 0 (e.g. "�",
+ * "�", "�", "�") and removes them. If a null entity is unterminated (no ';'), it is
+ * still removed when a non-entity character terminates the sequence. Everything else is passed
+ * through unchanged.
+ */
+final class XmlSanitizingReader extends Reader {
Review Comment:
This reader feels like overkill to avoid the `�x` chars that tika-server returns in some cases. Could do a simple string replace but then I'd need to consume the entire stream first. I looked for commons libs that could achieve the same and if SAX parser could be configured to be lenient, but no avail so far.
Appreciate tips on how to get rid of the weight of this class.
##########
solr/modules/extraction/gradle.lockfile:
##########
@@ -99,7 +102,8 @@ javax.inject:javax.inject:1=annotationProcessor,errorprone,testAnnotationProcess
javax.measure:unit-api:1.0=compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
joda-time:joda-time:2.14.0=compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
junit:junit:4.13.2=jarValidation,testCompileClasspath,testRuntimeClasspath
-net.java.dev.jna:jna:5.12.1=compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
+net.java.dev.jna:jna:5.12.1=compileClasspath,runtimeClasspath,runtimeLibs
+net.java.dev.jna:jna:5.13.0=jarValidation,testCompileClasspath,testRuntimeClasspath
Review Comment:
Is it expected to have two difference versions of JNA here?
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java:
##########
@@ -0,0 +1,220 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.InputStream;
+import java.time.Duration;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.tika.sax.BodyContentHandler;
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.InputStreamRequestContent;
+import org.eclipse.jetty.client.InputStreamResponseListener;
+import org.eclipse.jetty.client.Request;
+import org.eclipse.jetty.client.Response;
+import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler;
+import org.xml.sax.helpers.DefaultHandler;
+
+/** Extraction backend using the Tika Server. It uses a shared Jetty HttpClient. */
+public class TikaServerExtractionBackend implements ExtractionBackend {
+ private static volatile HttpClient SHARED_CLIENT;
+ private static volatile ExecutorService SHARED_EXECUTOR;
+ private static final Object INIT_LOCK = new Object();
+ private static volatile boolean INITIALIZED = false;
+ private static volatile boolean SHUTDOWN = false;
+ private final String baseUrl; // e.g., http://localhost:9998
+ private final Duration timeout = Duration.ofMinutes(3);
Review Comment:
Make timeout configurable?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2410800305
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractingDocumentLoader.java:
##########
@@ -131,169 +105,192 @@ public void load(
ContentStream stream,
UpdateRequestProcessor processor)
throws Exception {
- Parser parser = null;
String streamType = req.getParams().get(ExtractingParams.STREAM_TYPE, null);
- if (streamType != null) {
- // Cache? Parsers are lightweight to construct and thread-safe, so I'm told
- MediaType mt = MediaType.parse(streamType.trim().toLowerCase(Locale.ROOT));
- parser = new DefaultParser(config.getMediaTypeRegistry()).getParsers().get(mt);
- } else {
- parser = autoDetectParser;
- }
- if (parser != null) {
- Metadata metadata = new Metadata();
-
- // If you specify the resource name (the filename, roughly) with this parameter,
- // then Tika can make use of it in guessing the appropriate MIME type:
- String resourceName = req.getParams().get(ExtractingParams.RESOURCE_NAME, null);
- if (resourceName != null) {
- metadata.add(TikaMetadataKeys.RESOURCE_NAME_KEY, resourceName);
- }
- // Provide stream's content type as hint for auto detection
- if (stream.getContentType() != null) {
- metadata.add(HttpHeaders.CONTENT_TYPE, stream.getContentType());
- }
+ String resourceName = req.getParams().get(ExtractingParams.RESOURCE_NAME, null);
- try (InputStream inputStream = stream.getStream()) {
- metadata.add(ExtractingMetadataConstants.STREAM_NAME, stream.getName());
- metadata.add(ExtractingMetadataConstants.STREAM_SOURCE_INFO, stream.getSourceInfo());
- metadata.add(ExtractingMetadataConstants.STREAM_SIZE, String.valueOf(stream.getSize()));
- metadata.add(ExtractingMetadataConstants.STREAM_CONTENT_TYPE, stream.getContentType());
- // HtmlParser and TXTParser regard Metadata.CONTENT_ENCODING in metadata
- String charset = ContentStreamBase.getCharsetFromContentType(stream.getContentType());
- if (charset != null) {
- metadata.add(HttpHeaders.CONTENT_ENCODING, charset);
+ try (InputStream inputStream = stream.getStream()) {
+ String charset = ContentStreamBase.getCharsetFromContentType(stream.getContentType());
+
+ String xpathExpr = params.get(ExtractingParams.XPATH_EXPRESSION);
+ boolean extractOnly = params.getBool(ExtractingParams.EXTRACT_ONLY, false);
+ boolean recursive = params.getBool(ExtractingParams.RECURSIVE, false);
+ String extractFormat =
+ params.get(ExtractingParams.EXTRACT_FORMAT, extractOnly ? XML_FORMAT : TEXT_FORMAT);
+
+ // Parse optional passwords file into a map
+ LinkedHashMap<Pattern, String> pwMap = null;
+ String passwordsFile = params.get("passwordsFile");
+ if (passwordsFile != null) {
+ try (java.io.InputStream is = core.getResourceLoader().openResource(passwordsFile)) {
+ pwMap = RegexRulesPasswordProvider.parseRulesFile(is);
}
+ }
- String xpathExpr = params.get(ExtractingParams.XPATH_EXPRESSION);
- boolean extractOnly = params.getBool(ExtractingParams.EXTRACT_ONLY, false);
- SolrContentHandler handler =
- factory.createSolrContentHandler(metadata, params, req.getSchema());
- ContentHandler parsingHandler = handler;
-
- StringWriter writer = null;
- BaseMarkupSerializer serializer = null;
- if (extractOnly == true) {
- String extractFormat = params.get(ExtractingParams.EXTRACT_FORMAT, "xml");
- writer = new StringWriter();
- if (extractFormat.equals(TEXT_FORMAT)) {
- serializer = new TextSerializer();
- serializer.setOutputCharStream(writer);
- serializer.setOutputFormat(new OutputFormat("Text", "UTF-8", true));
- } else {
- serializer = new XMLSerializer(writer, new OutputFormat("XML", "UTF-8", true));
- }
- if (xpathExpr != null) {
- Matcher matcher = PARSER.parse(xpathExpr);
- serializer
- .startDocument(); // The MatchingContentHandler does not invoke startDocument. See
- // https://lists.apache.org/thread.html/5ec63e104e564a2363e45f74d5aced6520b7d32b4b625762ef56cb86%401226775505%40%3Cdev.tika.apache.org%3E
- parsingHandler = new MatchingContentHandler(serializer, matcher);
- } else {
- parsingHandler = serializer;
- }
- } else if (xpathExpr != null) {
- Matcher matcher = PARSER.parse(xpathExpr);
- parsingHandler = new MatchingContentHandler(handler, matcher);
- } // else leave it as is
+ ExtractionRequest extractionRequest =
+ new ExtractionRequest(
+ streamType,
+ resourceName,
+ stream.getContentType(),
+ charset,
+ stream.getName(),
+ stream.getSourceInfo(),
+ stream.getSize(),
+ params.get(ExtractingParams.RESOURCE_PASSWORD, null),
+ pwMap,
+ extractFormat,
+ recursive,
+ Collections.emptyMap());
+
+ boolean captureAttr = params.getBool(ExtractingParams.CAPTURE_ATTRIBUTES, false);
+ String[] captureElems = params.getParams(ExtractingParams.CAPTURE_ELEMENTS);
+ boolean needsSaxParsing =
+ extractOnly
+ || xpathExpr != null
+ || captureAttr
+ || (captureElems != null && captureElems.length > 0)
+ || (params.get(ExtractingParams.RESOURCE_PASSWORD) != null)
+ || (passwordsFile != null);
+ if (extractOnly) {
try {
- // potentially use a wrapper handler for parsing, but we still need the SolrContentHandler
- // for getting the document.
- ParseContext context = parseContextConfig.create();
-
- context.set(Parser.class, parser);
- context.set(HtmlMapper.class, MostlyPassthroughHtmlMapper.INSTANCE);
-
- // Password handling
- RegexRulesPasswordProvider epp = new RegexRulesPasswordProvider();
- String pwMapFile = params.get(ExtractingParams.PASSWORD_MAP_FILE);
- if (pwMapFile != null && pwMapFile.length() > 0) {
- InputStream is = req.getCore().getResourceLoader().openResource(pwMapFile);
- if (is != null) {
- log.debug("Password file supplied: {}", pwMapFile);
- epp.parse(is);
+ ExtractionMetadata md = backend.buildMetadataFromRequest(extractionRequest);
+ String content;
+ if (ExtractingDocumentLoader.TEXT_FORMAT.equals(extractionRequest.extractFormat)
+ || xpathExpr != null) {
+ content =
+ extractWithHandler(
+ inputStream, xpathExpr, extractionRequest, md, new ToTextContentHandler());
+ } else { // XML format
+ content =
+ extractWithHandler(
+ inputStream, xpathExpr, extractionRequest, md, new ToXMLContentHandler());
+ if (!content.startsWith("<?xml")) {
+ content = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" + content;
}
}
- context.set(PasswordProvider.class, epp);
- String resourcePassword = params.get(ExtractingParams.RESOURCE_PASSWORD);
- if (resourcePassword != null) {
- epp.setExplicitPassword(resourcePassword);
- log.debug("Literal password supplied for file {}", resourceName);
+
+ appendBackCompatTikaMetadata(md);
+
+ rsp.add(stream.getName(), content);
+ NamedList<String[]> metadataNL = new NamedList<>();
+ for (String name : md.keySet()) {
+ metadataNL.add(name, md.get(name).toArray(new String[0]));
+ }
+ rsp.add(stream.getName() + "_metadata", metadataNL);
+ } catch (Exception e) {
+ if (ignoreTikaException) {
+ if (log.isWarnEnabled())
+ log.warn("skip extracting text due to {}.", e.getLocalizedMessage(), e);
+ return;
}
- parser.parse(inputStream, parsingHandler, metadata, context);
- } catch (TikaException e) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+ }
+ return;
+ }
+
+ if (needsSaxParsing) {
+ ExtractionMetadata metadata = backend.buildMetadataFromRequest(extractionRequest);
+ SolrContentHandler handler =
+ factory.createSolrContentHandler(metadata, params, req.getSchema());
+ try {
+ backend.extractWithSaxHandler(inputStream, extractionRequest, metadata, handler);
+ } catch (Exception e) {
if (ignoreTikaException) {
if (log.isWarnEnabled()) {
- log.warn(
- "skip extracting text due to {}. metadata={}",
- e.getLocalizedMessage(),
- metadata,
- e);
+ log.warn("skip extracting text due to {}.", e.getLocalizedMessage(), e);
+ return;
}
- } else {
- throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
}
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
}
- if (extractOnly == false) {
- addDoc(handler);
- } else {
- // serializer is not null, so we need to call endDoc on it if using xpath
- if (xpathExpr != null) {
- serializer.endDocument();
- }
- rsp.add(stream.getName(), writer.toString());
- writer.close();
- String[] names = metadata.names();
- NamedList<String[]> metadataNL = new NamedList<>();
- for (int i = 0; i < names.length; i++) {
- String[] vals = metadata.getValues(names[i]);
- metadataNL.add(names[i], vals);
- }
- rsp.add(stream.getName() + "_metadata", metadataNL);
+ appendBackCompatTikaMetadata(handler.metadata);
+
+ addDoc(handler);
+ return;
+ }
+
+ ExtractionResult result;
+ try {
+ result = backend.extract(inputStream, extractionRequest);
+ } catch (Exception e) {
+ if (ignoreTikaException) {
+ if (log.isWarnEnabled())
+ log.warn("skip extracting text due to {}.", e.getLocalizedMessage(), e);
+ return;
}
- } catch (SAXException e) {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
}
- } else {
- throw new SolrException(
- SolrException.ErrorCode.BAD_REQUEST,
- "Stream type of "
- + streamType
- + " didn't match any known parsers. Please supply the "
- + ExtractingParams.STREAM_TYPE
- + " parameter.");
+
+ ExtractionMetadata metadata = result.getMetadata();
+
+ appendBackCompatTikaMetadata(metadata);
+
+ SolrContentHandler handler =
+ factory.createSolrContentHandler(metadata, params, req.getSchema());
+ handler.appendToContent(result.getContent());
+ addDoc(handler);
}
}
- public static class MostlyPassthroughHtmlMapper implements HtmlMapper {
- public static final HtmlMapper INSTANCE = new MostlyPassthroughHtmlMapper();
-
- /**
- * Keep all elements and their content.
- *
- * <p>Apparently <SCRIPT> and <STYLE> elements are blocked elsewhere
- */
- @Override
- public boolean isDiscardElement(String name) {
- return false;
+ /*
+ * Extracts content from the given input stream using an optional XPath expression
+ * and a SAX content handler. The extraction process may filter content based on
+ * the XPath expression, if provided.
+ */
+ private String extractWithHandler(
+ InputStream inputStream,
+ String xpathExpr,
+ ExtractionRequest extractionRequest,
+ ExtractionMetadata md,
+ DefaultHandler ch)
+ throws Exception {
+ if (xpathExpr != null) {
+ org.apache.tika.sax.xpath.XPathParser xparser =
+ new org.apache.tika.sax.xpath.XPathParser(
+ "xhtml", org.apache.tika.sax.XHTMLContentHandler.XHTML);
+ org.apache.tika.sax.xpath.Matcher matcher = xparser.parse(xpathExpr);
+ ch = new org.apache.tika.sax.xpath.MatchingContentHandler(ch, matcher);
}
+ backend.extractWithSaxHandler(inputStream, extractionRequest, md, ch);
+ return ch.toString();
+ }
+
+ private final Map<String, String> fieldMappings = new LinkedHashMap<>();
- /** Lowercases the attribute name */
- @Override
- public String mapSafeAttribute(String elementName, String attributeName) {
- return attributeName.toLowerCase(Locale.ENGLISH);
+ // TODO: Improve backward compatibility by adding more mappings
Review Comment:
Guess there may be other mappings one could add for back-compat. But this was added mostly to get tests passing in an easy way without modifying the test itself. Do we believe that a back-compat mode is useful?
NB: Currently the `extraction.metadataBackCompat` param is not parsed properly if configured in solrconfig.xml and not on the update request.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3395458014
Getting close! I won't be able to test till Tuesday...
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy merged PR #3670:
URL: https://github.com/apache/solr/pull/3670
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3336231367
Now `testLiteralsOverride` also passes. That leaves
* `testXPath` - known error in pulling xpath - difference in XML format between local Tika and server?
* `testCapture` - needs debugging why capturing tags won't work
* `testPdfWithImages` - nested docs need `/rmeta` endpoint, not yet tested
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3336451788
Love the way you fixed it. Does this mean in practice that folks might see different resutls depending on which backend they use and the specfici document? On the other hand, that also seems totally okay in the sense that they are different backends...
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2373266673
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractionBackend.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.InputStream;
+import org.apache.tika.metadata.HttpHeaders;
+import org.apache.tika.metadata.TikaMetadataKeys;
+
+/** Strategy interface for content extraction backends. */
+public interface ExtractionBackend {
+ /**
+ * Extract plain text and metadata from the inputStream. Implementations should not close the
+ * inputStream. This API is backend-neutral and does not expose SAX or XML-specific types.
+ */
+ ExtractionResult extract(InputStream inputStream, ExtractionRequest request) throws Exception;
+
+ /**
+ * Perform extractOnly operation. If extractFormat equals ExtractingDocumentLoader.TEXT_FORMAT,
+ * return plain text. If XML, return XML body as string. Implementations may support optional
+ * xpathExpr; if unsupported and xpathExpr is not null, they should throw
+ * UnsupportedOperationException.
+ */
+ ExtractionResult extractOnly(InputStream inputStream, ExtractionRequest request, String xpathExpr)
+ throws Exception;
+
+ /**
+ * Parse the content and stream SAX events into the provided SolrContentHandler, while also
+ * filling outMetadata with extracted metadata.
+ */
+ void parseToSolrContentHandler(
+ InputStream inputStream,
+ ExtractionRequest request,
+ SolrContentHandler handler,
+ ExtractionMetadata outMetadata)
+ throws Exception;
+
+ /** Build ExtractionMetadata from the request context */
+ default ExtractionMetadata buildMetadataFromRequest(ExtractionRequest request) {
+ ExtractionMetadata md = new ExtractionMetadata();
+ if (request.resourceName != null)
Review Comment:
I wish we used `{}` around our indented if statements... Just me!
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2373373404
##########
solr/modules/extraction/src/test-files/extraction/solr/collection1/conf/solrconfig.xml:
##########
@@ -152,6 +152,8 @@
<requestHandler name="/update/extract" class="org.apache.solr.handler.extraction.ExtractingRequestHandler">
<str name="parseContext.config">parseContext.xml</str>
+ <str name="extraction.backend">${solr.test.extraction.backend:local}</str>
+ <str name="tikaserver.url">${solr.test.tikaserver.url:}</str>
Review Comment:
This solrconfig file is part of the test suite, so the sysprop substitution is specific to tests. Users would configure their solrconfig.xml as they wish.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2430169567
##########
solr/modules/extraction/src/test/org/apache/solr/handler/extraction/TikaServerExtractionBackendTest.java:
##########
@@ -0,0 +1,226 @@
+/*
+ * 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.solr.handler.extraction;
+
+import com.carrotsearch.randomizedtesting.ThreadFilter;
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
+import java.io.ByteArrayInputStream;
+import java.nio.file.Files;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.tests.util.QuickPatchThreadsFilter;
+import org.apache.solr.SolrIgnoredThreadsFilter;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
+import org.apache.tika.sax.ToXMLContentHandler;
Review Comment:
Do not depend on tika API here
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/RegexRulesPasswordProvider.java:
##########
Review Comment:
Do not depend on tika API here
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractingDocumentLoader.java:
##########
@@ -33,38 +34,16 @@
import org.apache.solr.response.SolrQueryResponse;
import org.apache.solr.update.AddUpdateCommand;
import org.apache.solr.update.processor.UpdateRequestProcessor;
-import org.apache.tika.config.TikaConfig;
-import org.apache.tika.exception.TikaException;
-import org.apache.tika.metadata.HttpHeaders;
-import org.apache.tika.metadata.Metadata;
-import org.apache.tika.metadata.TikaMetadataKeys;
-import org.apache.tika.mime.MediaType;
-import org.apache.tika.parser.AutoDetectParser;
-import org.apache.tika.parser.DefaultParser;
-import org.apache.tika.parser.ParseContext;
-import org.apache.tika.parser.Parser;
-import org.apache.tika.parser.PasswordProvider;
-import org.apache.tika.parser.html.HtmlMapper;
-import org.apache.tika.sax.XHTMLContentHandler;
-import org.apache.tika.sax.xpath.Matcher;
-import org.apache.tika.sax.xpath.MatchingContentHandler;
-import org.apache.tika.sax.xpath.XPathParser;
-import org.apache.xml.serialize.BaseMarkupSerializer;
-import org.apache.xml.serialize.OutputFormat;
-import org.apache.xml.serialize.TextSerializer;
-import org.apache.xml.serialize.XMLSerializer;
+import org.apache.tika.sax.ToTextContentHandler;
+import org.apache.tika.sax.ToXMLContentHandler;
Review Comment:
Do not import tika classes here
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java:
##########
@@ -0,0 +1,450 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.ConnectException;
+import java.net.SocketTimeoutException;
+import java.nio.channels.ClosedChannelException;
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.util.RefCounted;
+import org.apache.tika.sax.BodyContentHandler;
Review Comment:
Do not import tika classes here
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2428647227
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java:
##########
@@ -0,0 +1,403 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.ConnectException;
+import java.net.SocketTimeoutException;
+import java.nio.channels.ClosedChannelException;
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.tika.sax.BodyContentHandler;
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.InputStreamRequestContent;
+import org.eclipse.jetty.client.InputStreamResponseListener;
+import org.eclipse.jetty.client.Request;
+import org.eclipse.jetty.client.Response;
+import org.eclipse.jetty.io.EofException;
+import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler;
+import org.xml.sax.helpers.DefaultHandler;
+
+/** Extraction backend using the Tika Server. It uses a shared Jetty HttpClient. */
+public class TikaServerExtractionBackend implements ExtractionBackend {
+ public static final long DEFAULT_MAXCHARS_LIMIT = 100 * 1024 * 1024;
+ private static volatile HttpClient SHARED_CLIENT;
+ private static volatile ExecutorService SHARED_EXECUTOR;
+ private static final Object INIT_LOCK = new Object();
+ private static volatile boolean INITIALIZED = false;
+ private static volatile boolean SHUTDOWN = false;
+ private final String baseUrl;
+ private static final int DEFAULT_TIMEOUT_SECONDS = 3 * 60;
+ private final Duration defaultTimeout;
+ private final TikaServerParser tikaServerResponseParser = new TikaServerParser();
+ private boolean tikaMetadataCompatibility;
+ private HashMap<String, Object> initArgsMap = new HashMap<>();
+ private final long maxCharsLimit;
+
+ public TikaServerExtractionBackend(String baseUrl) {
+ this(baseUrl, DEFAULT_TIMEOUT_SECONDS, null, DEFAULT_MAXCHARS_LIMIT);
+ }
+
+ public TikaServerExtractionBackend(
+ String baseUrl, int timeoutSeconds, NamedList<?> initArgs, long maxCharsLimit) {
+ this.maxCharsLimit = maxCharsLimit;
+ if (initArgs != null) {
+ initArgs.toMap(this.initArgsMap);
+ }
+ Object metaCompatObh = this.initArgsMap.get(ExtractingParams.TIKASERVER_METADATA_COMPATIBILITY);
+ if (metaCompatObh != null) {
+ this.tikaMetadataCompatibility = Boolean.parseBoolean(metaCompatObh.toString());
+ }
+ if (timeoutSeconds <= 0) {
+ timeoutSeconds = DEFAULT_TIMEOUT_SECONDS;
+ }
+ if (baseUrl.endsWith("/")) {
+ this.baseUrl = baseUrl.substring(0, baseUrl.length() - 1);
+ } else {
+ this.baseUrl = baseUrl;
+ }
+ this.defaultTimeout =
+ Duration.ofSeconds(timeoutSeconds > 0 ? timeoutSeconds : DEFAULT_TIMEOUT_SECONDS);
+ }
+
+ public static final String NAME = "tikaserver";
+
+ @Override
+ public String name() {
+ return NAME;
+ }
+
+ @Override
+ public ExtractionResult extract(InputStream inputStream, ExtractionRequest request)
+ throws Exception {
+ try (InputStream tikaResponse = callTikaServer(inputStream, request)) {
+ ExtractionMetadata md = buildMetadataFromRequest(request);
+ BodyContentHandler bodyContentHandler = new BodyContentHandler(-1);
+ if (request.tikaServerRecursive) {
+ tikaServerResponseParser.parseRmetaJson(tikaResponse, bodyContentHandler, md);
+ } else {
+ tikaServerResponseParser.parseXml(tikaResponse, bodyContentHandler, md);
+ }
+ if (tikaMetadataCompatibility) {
+ appendBackCompatTikaMetadata(md);
+ }
+ return new ExtractionResult(bodyContentHandler.toString(), md);
+ }
+ }
+
+ @Override
+ public void extractWithSaxHandler(
+ InputStream inputStream,
+ ExtractionRequest request,
+ ExtractionMetadata md,
+ DefaultHandler saxContentHandler)
+ throws Exception {
+ try (InputStream tikaResponse = callTikaServer(inputStream, request)) {
+ if (request.tikaServerRecursive) {
+ tikaServerResponseParser.parseRmetaJson(tikaResponse, saxContentHandler, md);
+ } else {
+ tikaServerResponseParser.parseXml(tikaResponse, saxContentHandler, md);
+ }
+ if (tikaMetadataCompatibility) {
+ appendBackCompatTikaMetadata(md);
+ }
+ }
+ }
+
+ /**
+ * Call the Tika Server to extract text and metadata. Depending on <code>request.recursive</code>,
+ * will either return XML (false) or JSON array (true). <b>The recursive mode consumes more memory
+ * both on the TikaServer side and on the Solr side</b>
+ *
+ * @return InputStream of the response body, either XML or JSON depending on <code>
+ * request.tikaserverRecursive</code>
+ */
+ InputStream callTikaServer(InputStream inputStream, ExtractionRequest request) throws Exception {
+ String url = baseUrl + (request.tikaServerRecursive ? "/rmeta" : "/tika");
+
+ ensureClientInitialized();
+ HttpClient client = SHARED_CLIENT;
+
+ Request req = client.newRequest(url).method("PUT");
+ Duration effectiveTimeout =
+ (request.tikaServerTimeoutSeconds != null && request.tikaServerTimeoutSeconds > 0)
+ ? Duration.ofSeconds(request.tikaServerTimeoutSeconds)
+ : defaultTimeout;
+ req.timeout(effectiveTimeout.toMillis(), TimeUnit.MILLISECONDS);
+
+ // Headers
+ String accept = (request.tikaServerRecursive ? "application/json" : "text/xml");
+ req.headers(h -> h.add("Accept", accept));
+ String contentType = (request.streamType != null) ? request.streamType : request.contentType;
+ if (contentType != null) {
+ req.headers(h -> h.add("Content-Type", contentType));
+ }
+ if (!request.tikaServerRequestHeaders.isEmpty()) {
+ req.headers(
+ h ->
+ request.tikaServerRequestHeaders.forEach(
+ (k, v) -> {
+ if (k != null && v != null) h.add(k, v);
+ }));
+ }
+
+ ExtractionMetadata md = buildMetadataFromRequest(request);
+ if (request.resourcePassword != null || request.passwordsMap != null) {
+ RegexRulesPasswordProvider passwordProvider = new RegexRulesPasswordProvider();
+ if (request.resourcePassword != null) {
+ passwordProvider.setExplicitPassword(request.resourcePassword);
+ }
+ if (request.passwordsMap != null) {
+ passwordProvider.setPasswordMap(request.passwordsMap);
+ }
+ String pwd = passwordProvider.getPassword(md);
+ if (pwd != null) {
+ req.headers(h -> h.add("Password", pwd)); // Tika Server expects this header if provided
+ }
+ }
+ if (request.resourceName != null) {
+ req.headers(
+ h ->
+ h.add(
+ "Content-Disposition", "attachment; filename=\"" + request.resourceName + "\""));
+ }
+
+ if (contentType != null) {
+ req.body(new InputStreamRequestContent(contentType, inputStream));
+ } else {
+ req.body(new InputStreamRequestContent(inputStream));
+ }
+
+ InputStreamResponseListener listener = new InputStreamResponseListener();
+ req.send(listener);
+
+ final Response response;
+ try {
+ response = listener.get(effectiveTimeout.toMillis(), TimeUnit.MILLISECONDS);
+ } catch (TimeoutException te) {
+ throw new SolrException(
+ SolrException.ErrorCode.GATEWAY_TIMEOUT,
+ "Timeout after "
+ + effectiveTimeout.toMillis()
+ + " ms while waiting for response from TikaServer "
+ + url,
+ te);
+ } catch (InterruptedException ie) {
+ Thread.currentThread().interrupt();
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Interrupted while waiting for response from TikaServer " + url,
+ ie);
+ } catch (ExecutionException ee) {
+ Throwable cause = ee.getCause();
+ if (cause instanceof ConnectException
+ || cause instanceof SocketTimeoutException
+ || cause instanceof EofException
+ || cause instanceof ClosedChannelException) {
+ throw new SolrException(
+ SolrException.ErrorCode.SERVICE_UNAVAILABLE,
+ "Error communicating with TikaServer "
+ + url
+ + ": "
+ + cause.getClass().getSimpleName()
+ + ": "
+ + cause.getMessage(),
+ cause);
+ }
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Unexpected error while calling TikaServer " + url,
+ ee);
+ }
+
+ int code = response.getStatus();
+ if (code < 200 || code >= 300) {
+ SolrException.ErrorCode errorCode = SolrException.ErrorCode.getErrorCode(code);
+ String reason = response.getReason();
+ String msg =
+ "TikaServer "
+ + url
+ + " returned status "
+ + code
+ + (reason != null ? " (" + reason + ")" : "");
+ throw new SolrException(errorCode, msg);
+ }
+
+ InputStream responseStream = listener.getInputStream();
+ // Bound the amount of data we read from Tika Server to avoid excessive memory/CPU usage
+ return new LimitingInputStream(responseStream, maxCharsLimit);
+ }
+
+ private static class LimitingInputStream extends InputStream {
+ private final InputStream in;
+ private final long max;
+ private long count;
+
+ LimitingInputStream(InputStream in, long max) {
+ this.in = in;
+ this.max = max;
+ this.count = 0L;
+ }
+
+ private void checkLimit(long toAdd) {
+ if (max <= 0) return; // non-positive means unlimited
+ long newCount = count + toAdd;
+ if (newCount > max) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "TikaServer response exceeded the configured maximum size of " + max + " bytes");
+ }
+ count = newCount;
+ }
+
+ @Override
+ public int read() throws IOException {
+ int b = in.read();
+ if (b != -1) {
+ checkLimit(1);
+ }
+ return b;
+ }
+
+ @Override
+ public int read(byte[] b, int off, int len) throws IOException {
+ int n = in.read(b, off, len);
+ if (n > 0) {
+ checkLimit(n);
+ }
+ return n;
+ }
+
+ @Override
+ public long skip(long n) throws IOException {
+ long skipped = in.skip(n);
+ if (skipped > 0) {
+ checkLimit(skipped);
+ }
+ return skipped;
+ }
+
+ @Override
+ public void close() throws IOException {
+ in.close();
+ }
+
+ @Override
+ public int available() throws IOException {
+ return in.available();
+ }
+ }
+
+ private static void ensureClientInitialized() {
+ if (INITIALIZED) return;
+ synchronized (INIT_LOCK) {
+ if (INITIALIZED) return;
+ ThreadFactory tf = new SolrNamedThreadFactory("TikaServerHttpClient");
+ ExecutorService exec = ExecutorUtil.newMDCAwareCachedThreadPool(tf);
+ HttpClient client = new HttpClient();
+ client.setExecutor(exec);
+ client.setScheduler(new ScheduledExecutorScheduler("TikaServerHttpClient-scheduler", true));
+ try {
+ client.start();
+ } catch (Exception e) {
+ try {
+ exec.shutdownNow();
+ } catch (Throwable ignore) {
+ }
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR, "Failed to start shared Jetty HttpClient", e);
+ }
+ SHARED_EXECUTOR = exec;
+ SHARED_CLIENT = client;
+ INITIALIZED = true;
+ SHUTDOWN = false;
+ }
+ }
+
+ private final Map<String, String> fieldMappings = new LinkedHashMap<>();
+
+ // TODO: Improve backward compatibility by adding more mappings
+ {
+ fieldMappings.put("dc:title", "title");
+ fieldMappings.put("dc:creator", "author");
+ fieldMappings.put("dc:description", "description");
+ fieldMappings.put("dc:subject", "subject");
+ fieldMappings.put("dc:language", "language");
+ fieldMappings.put("dc:publisher", "publisher");
+ fieldMappings.put("dcterms:created", "created");
+ fieldMappings.put("dcterms:modified", "modified");
+ fieldMappings.put("meta:author", "Author");
+ fieldMappings.put("meta:creation-date", "Creation-Date");
+ fieldMappings.put("meta:save-date", "Last-Save-Date");
+ fieldMappings.put("meta:keyword", "Keywords");
+ fieldMappings.put("pdf:docinfo:keywords", "Keywords");
+ }
+
+ /*
+ * Appends back-compatible metadata into the given {@code ExtractionMetadata} instance by mapping
+ * source fields to target fields, provided that backward compatibility is enabled. If a source
+ * field exists and the target field is not yet populated, the values from the source field will
+ * be added to the target field.
+ */
+ private void appendBackCompatTikaMetadata(ExtractionMetadata md) {
+ for (Map.Entry<String, String> mapping : fieldMappings.entrySet()) {
+ String sourceField = mapping.getKey();
+ String targetField = mapping.getValue();
+ if (md.getFirst(sourceField) != null && md.getFirst(targetField) == null) {
+ md.add(targetField, md.get(sourceField));
+ }
+ }
+ }
+
+ @Override
+ public void close() {
+ if (SHUTDOWN) return;
+ synchronized (INIT_LOCK) {
+ if (SHUTDOWN) return;
Review Comment:
If several collections use ExtractionHandler, then we cannot close the static shared client blindly, only on last close
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "malliaridis (via GitHub)" <gi...@apache.org>.
malliaridis commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2422105312
##########
solr/modules/extraction/gradle.lockfile:
##########
@@ -99,7 +102,8 @@ javax.inject:javax.inject:1=annotationProcessor,errorprone,testAnnotationProcess
javax.measure:unit-api:1.0=compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
joda-time:joda-time:2.14.0=compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
junit:junit:4.13.2=jarValidation,testCompileClasspath,testRuntimeClasspath
-net.java.dev.jna:jna:5.12.1=compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
+net.java.dev.jna:jna:5.12.1=compileClasspath,runtimeClasspath,runtimeLibs
+net.java.dev.jna:jna:5.13.0=jarValidation,testCompileClasspath,testRuntimeClasspath
Review Comment:
This is a sign that we are not aligning the versions and we let the transitive dependencies be out of sync. The easiest way to resolve this is to add the library with the latest supported version, or the highest version from the lock files (here 5.13.0) to the `libs.version.toml` (remember to include the `# keep` comment to prevent tidy from removing it). It will then align the versions across the project. The addition would be something like
```toml
[versions]
# ...
jna = "5.13.0"
# ...
[libraries]
# ...
jna-jna = { module = "net.java.dev.jna:jna", version.ref = "jna" }
# ...
```
If that does not solve the issue, then we likely do not depend on our `platform` module, either transitive through `solr:api` or directly (through dependency on `platform(project(":platform"))`). To fix this case, add in addition to the above also the `[configuration] platform(project(":platform"))` to the module's gradle file, where configuration is usually one of the configuration listed in the lockfile.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3382022130
> I'm okay with this as one pr.
I feel the pressure of the Solr 10 and 9.10 trains soon leaving the station. Would be perfect to have this go in both those releases so we can deprecate local mode in 9.10 and then remove local mode in 10.0 and get a slimmer distro without all the JARs.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3319271554
Testcontainers and docker don't love the SecurityManager. I had claude try to run the tests and add additional permissions to `solr-tests.policy`, and after an hour or so, I had a lot more, but no love:
```
// Needed for testcontainers
permission java.io.FilePermission "/Users/epugh/.testcontainers.properties", "read";
permission java.io.FilePermission "/Users/epugh/.docker-java.properties", "read";
permission java.io.FilePermission "/Users/epugh/.docker/-", "read";
permission java.io.FilePermission "/usr/local/opt/go@1.22/bin/docker-machine", "read";
permission java.io.FilePermission "/usr/local/opt/go@1.20/bin/docker-machine", "read";
permission java.io.FilePermission "/Users/epugh/.asdf/installs/nodejs/20.18.3/bin/docker-machine", "read";
permission java.io.FilePermission "/Users/epugh/.asdf/shims/docker-machine", "read";
permission java.io.FilePermission "/Users/epugh/.nvm/versions/node/v14.21.2/bin/docker-machine", "read";
permission java.io.FilePermission "/Users/epugh/.rbenv/shims/docker-machine", "read";
permission java.io.FilePermission "/usr/local/bin/docker-machine", "read";
permission java.io.FilePermission "/usr/local/sbin/docker-machine", "read";
permission java.io.FilePermission "/System/Cryptexes/App/usr/bin/docker-machine", "read";
permission java.io.FilePermission "/usr/bin/docker-machine", "read";
permission java.io.FilePermission "/bin/docker-machine", "read";
permission java.io.FilePermission "/usr/sbin/docker-machine", "read";
permission java.io.FilePermission "/sbin/docker-machine", "read";
permission java.io.FilePermission "/usr/local/MacGPG2/bin/docker-machine", "read";
permission java.io.FilePermission "/Library/Apple/usr/bin/docker-machine", "read";
permission java.io.FilePermission "/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin/docker-machine", "read";
permission java.io.FilePermission "/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin/docker-machine", "read";
permission java.io.FilePermission "/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin/docker-machine", "read";
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3328554888
I have 90% of a working `.bats` test that downloads and fires up TikaServer. Should we add that so our scripts tests will validate the code? Thoughts?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3336397547
The `textXpath` test tries to capture `<a>` tags directly under `<body>`. But it also captures the `<div><a>` tag. I checked the XML I get from local Tika, and it is different from XML we get from Tika Server 3. From TikaServer all the `<div>` tags are stripped, so that the `<a>` element appears to be just below `<body>`. I believe it is because the default HTML parser is now JSoup, which has some other rules. See https://issues.apache.org/jira/browse/TIKA-2562
Thus, this test document can be rewritten to use something else than div, and the test will work.
I believe the same is the issue with `testCapture` test, as it relies on capturing `<div>`.
That gives us a solution for the remaining three failing tests đŸ¥³
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2428968067
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java:
##########
@@ -0,0 +1,434 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.ConnectException;
+import java.net.SocketTimeoutException;
+import java.nio.channels.ClosedChannelException;
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.tika.sax.BodyContentHandler;
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.InputStreamRequestContent;
+import org.eclipse.jetty.client.InputStreamResponseListener;
+import org.eclipse.jetty.client.Request;
+import org.eclipse.jetty.client.Response;
+import org.eclipse.jetty.io.EofException;
+import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler;
+import org.xml.sax.helpers.DefaultHandler;
+
+/** Extraction backend using the Tika Server. It uses a shared Jetty HttpClient. */
+public class TikaServerExtractionBackend implements ExtractionBackend {
+ /**
+ * Default maximum response size (100MB) to prevent excessive memory usage from large documents
+ */
+ public static final long DEFAULT_MAXCHARS_LIMIT = 100 * 1024 * 1024;
+
+ private static volatile HttpClient SHARED_CLIENT;
+ private static volatile ExecutorService SHARED_EXECUTOR;
+ private static final Object INIT_LOCK = new Object();
+ private static volatile boolean INITIALIZED = false;
+ private static volatile int REFERENCE_COUNT = 0;
+ private final String baseUrl;
+ private static final int DEFAULT_TIMEOUT_SECONDS = 3 * 60;
+ private final Duration defaultTimeout;
+ private final TikaServerParser tikaServerResponseParser = new TikaServerParser();
+ private boolean tikaMetadataCompatibility;
+ private HashMap<String, Object> initArgsMap = new HashMap<>();
+ private final long maxCharsLimit;
+
+ public TikaServerExtractionBackend(String baseUrl) {
+ this(baseUrl, DEFAULT_TIMEOUT_SECONDS, null, DEFAULT_MAXCHARS_LIMIT);
+ }
+
+ public TikaServerExtractionBackend(
+ String baseUrl, int timeoutSeconds, NamedList<?> initArgs, long maxCharsLimit) {
+ // Validate baseUrl
+ if (baseUrl == null || baseUrl.trim().isEmpty()) {
+ throw new IllegalArgumentException("baseUrl cannot be null or empty");
+ }
+ // Validate URL format and scheme
+ try {
+ java.net.URI uri = new java.net.URI(baseUrl);
+ String scheme = uri.getScheme();
+ if (scheme == null
+ || (!scheme.equalsIgnoreCase("http") && !scheme.equalsIgnoreCase("https"))) {
+ throw new IllegalArgumentException(
+ "baseUrl must use http or https scheme, got: " + baseUrl);
+ }
+ uri.toURL(); // Additional validation that it's a valid URL
+ } catch (java.net.URISyntaxException | java.net.MalformedURLException e) {
+ throw new IllegalArgumentException("Invalid baseUrl: " + baseUrl, e);
+ }
+
+ this.maxCharsLimit = maxCharsLimit;
+ if (initArgs != null) {
+ initArgs.toMap(this.initArgsMap);
+ }
+ Object metaCompatObh = this.initArgsMap.get(ExtractingParams.TIKASERVER_METADATA_COMPATIBILITY);
+ if (metaCompatObh != null) {
+ this.tikaMetadataCompatibility = Boolean.parseBoolean(metaCompatObh.toString());
+ }
+ if (timeoutSeconds <= 0) {
+ timeoutSeconds = DEFAULT_TIMEOUT_SECONDS;
+ }
+ if (baseUrl.endsWith("/")) {
+ this.baseUrl = baseUrl.substring(0, baseUrl.length() - 1);
+ } else {
+ this.baseUrl = baseUrl;
+ }
+ this.defaultTimeout =
+ Duration.ofSeconds(timeoutSeconds > 0 ? timeoutSeconds : DEFAULT_TIMEOUT_SECONDS);
+
+ // Increment reference count when a backend instance is created
+ synchronized (INIT_LOCK) {
+ REFERENCE_COUNT++;
+ }
+ }
+
+ public static final String NAME = "tikaserver";
+
+ @Override
+ public String name() {
+ return NAME;
+ }
+
+ @Override
+ public ExtractionResult extract(InputStream inputStream, ExtractionRequest request)
+ throws Exception {
+ try (InputStream tikaResponse = callTikaServer(inputStream, request)) {
+ ExtractionMetadata md = buildMetadataFromRequest(request);
+ BodyContentHandler bodyContentHandler = new BodyContentHandler(-1);
+ if (request.tikaServerRecursive) {
+ tikaServerResponseParser.parseRmetaJson(tikaResponse, bodyContentHandler, md);
+ } else {
+ tikaServerResponseParser.parseXml(tikaResponse, bodyContentHandler, md);
+ }
+ if (tikaMetadataCompatibility) {
+ appendBackCompatTikaMetadata(md);
+ }
+ return new ExtractionResult(bodyContentHandler.toString(), md);
+ }
+ }
+
+ @Override
+ public void extractWithSaxHandler(
+ InputStream inputStream,
+ ExtractionRequest request,
+ ExtractionMetadata md,
+ DefaultHandler saxContentHandler)
+ throws Exception {
+ try (InputStream tikaResponse = callTikaServer(inputStream, request)) {
+ if (request.tikaServerRecursive) {
+ tikaServerResponseParser.parseRmetaJson(tikaResponse, saxContentHandler, md);
+ } else {
+ tikaServerResponseParser.parseXml(tikaResponse, saxContentHandler, md);
+ }
+ if (tikaMetadataCompatibility) {
+ appendBackCompatTikaMetadata(md);
+ }
+ }
+ }
+
+ /**
+ * Call the Tika Server to extract text and metadata. Depending on <code>request.recursive</code>,
+ * will either return XML (false) or JSON array (true). <b>The recursive mode consumes more memory
+ * both on the TikaServer side and on the Solr side</b>
+ *
+ * @return InputStream of the response body, either XML or JSON depending on <code>
+ * request.tikaserverRecursive</code>
+ */
+ InputStream callTikaServer(InputStream inputStream, ExtractionRequest request) throws Exception {
+ String url = baseUrl + (request.tikaServerRecursive ? "/rmeta" : "/tika");
+
+ ensureClientInitialized();
+ HttpClient client = SHARED_CLIENT;
+
+ Request req = client.newRequest(url).method("PUT");
+ Duration effectiveTimeout =
+ (request.tikaServerTimeoutSeconds != null && request.tikaServerTimeoutSeconds > 0)
+ ? Duration.ofSeconds(request.tikaServerTimeoutSeconds)
+ : defaultTimeout;
+ req.timeout(effectiveTimeout.toMillis(), TimeUnit.MILLISECONDS);
+
+ // Headers
+ String accept = (request.tikaServerRecursive ? "application/json" : "text/xml");
+ req.headers(h -> h.add("Accept", accept));
+ String contentType = (request.streamType != null) ? request.streamType : request.contentType;
+ if (contentType != null) {
+ req.headers(h -> h.add("Content-Type", contentType));
+ }
+ if (!request.tikaServerRequestHeaders.isEmpty()) {
+ req.headers(
+ h ->
+ request.tikaServerRequestHeaders.forEach(
+ (k, v) -> {
+ if (k != null && v != null) h.add(k, v);
+ }));
+ }
+
+ ExtractionMetadata md = buildMetadataFromRequest(request);
+ if (request.resourcePassword != null || request.passwordsMap != null) {
+ RegexRulesPasswordProvider passwordProvider = new RegexRulesPasswordProvider();
+ if (request.resourcePassword != null) {
+ passwordProvider.setExplicitPassword(request.resourcePassword);
+ }
+ if (request.passwordsMap != null) {
+ passwordProvider.setPasswordMap(request.passwordsMap);
+ }
+ String pwd = passwordProvider.getPassword(md);
+ if (pwd != null) {
+ req.headers(h -> h.add("Password", pwd)); // Tika Server expects this header if provided
+ }
+ }
+ if (request.resourceName != null) {
+ req.headers(
+ h ->
+ h.add(
+ "Content-Disposition", "attachment; filename=\"" + request.resourceName + "\""));
+ }
+
+ if (contentType != null) {
+ req.body(new InputStreamRequestContent(contentType, inputStream));
+ } else {
+ req.body(new InputStreamRequestContent(inputStream));
+ }
+
+ InputStreamResponseListener listener = new InputStreamResponseListener();
+ req.send(listener);
+
+ final Response response;
+ try {
+ response = listener.get(effectiveTimeout.toMillis(), TimeUnit.MILLISECONDS);
+ } catch (TimeoutException te) {
+ throw new SolrException(
+ SolrException.ErrorCode.GATEWAY_TIMEOUT,
+ "Timeout after "
+ + effectiveTimeout.toMillis()
+ + " ms while waiting for response from TikaServer "
+ + url,
+ te);
+ } catch (InterruptedException ie) {
+ Thread.currentThread().interrupt();
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Interrupted while waiting for response from TikaServer " + url,
+ ie);
+ } catch (ExecutionException ee) {
+ Throwable cause = ee.getCause();
+ if (cause instanceof ConnectException
+ || cause instanceof SocketTimeoutException
+ || cause instanceof EofException
+ || cause instanceof ClosedChannelException) {
+ throw new SolrException(
+ SolrException.ErrorCode.SERVICE_UNAVAILABLE,
+ "Error communicating with TikaServer "
+ + url
+ + ": "
+ + cause.getClass().getSimpleName()
+ + ": "
+ + cause.getMessage(),
+ cause);
+ }
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Unexpected error while calling TikaServer " + url,
+ ee);
+ }
+
+ int code = response.getStatus();
+ if (code < 200 || code >= 300) {
+ SolrException.ErrorCode errorCode = SolrException.ErrorCode.getErrorCode(code);
+ String reason = response.getReason();
+ String msg =
+ "TikaServer "
+ + url
+ + " returned status "
+ + code
+ + (reason != null ? " (" + reason + ")" : "");
+ throw new SolrException(errorCode, msg);
+ }
+
+ InputStream responseStream = listener.getInputStream();
+ // Bound the amount of data we read from Tika Server to avoid excessive memory/CPU usage
+ return new LimitingInputStream(responseStream, maxCharsLimit);
+ }
+
+ private static class LimitingInputStream extends InputStream {
+ private final InputStream in;
+ private final long max;
+ private long count;
+
+ LimitingInputStream(InputStream in, long max) {
+ this.in = in;
+ this.max = max;
+ this.count = 0L;
+ }
+
+ private void checkLimit(long toAdd) {
+ if (max <= 0) return; // non-positive means unlimited
+ long newCount = count + toAdd;
+ if (newCount > max) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "TikaServer response exceeded the configured maximum size of " + max + " bytes");
+ }
+ count = newCount;
+ }
+
+ @Override
+ public int read() throws IOException {
+ int b = in.read();
+ if (b != -1) {
+ checkLimit(1);
+ }
+ return b;
+ }
+
+ @Override
+ public int read(byte[] b, int off, int len) throws IOException {
+ int n = in.read(b, off, len);
+ if (n > 0) {
+ checkLimit(n);
+ }
+ return n;
+ }
+
+ @Override
+ public long skip(long n) throws IOException {
+ long skipped = in.skip(n);
+ if (skipped > 0) {
+ checkLimit(skipped);
+ }
+ return skipped;
+ }
+
+ @Override
+ public void close() throws IOException {
+ in.close();
+ }
+
+ @Override
+ public int available() throws IOException {
+ return in.available();
+ }
+ }
+
+ private static void ensureClientInitialized() {
+ if (INITIALIZED && SHARED_CLIENT != null) return;
+ synchronized (INIT_LOCK) {
+ if (INITIALIZED && SHARED_CLIENT != null) return;
+ ThreadFactory tf = new SolrNamedThreadFactory("TikaServerHttpClient");
+ ExecutorService exec = ExecutorUtil.newMDCAwareCachedThreadPool(tf);
+ HttpClient client = new HttpClient();
+ client.setExecutor(exec);
+ client.setScheduler(new ScheduledExecutorScheduler("TikaServerHttpClient-scheduler", true));
+ try {
+ client.start();
+ } catch (Exception e) {
+ try {
+ exec.shutdownNow();
+ } catch (Throwable ignore) {
+ }
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR, "Failed to start shared Jetty HttpClient", e);
+ }
+ SHARED_EXECUTOR = exec;
+ SHARED_CLIENT = client;
+ INITIALIZED = true;
+ }
+ }
+
+ private final Map<String, String> fieldMappings = new LinkedHashMap<>();
+
+ // TODO: Improve backward compatibility by adding more mappings
+ {
+ fieldMappings.put("dc:title", "title");
+ fieldMappings.put("dc:creator", "author");
+ fieldMappings.put("dc:description", "description");
+ fieldMappings.put("dc:subject", "subject");
+ fieldMappings.put("dc:language", "language");
+ fieldMappings.put("dc:publisher", "publisher");
+ fieldMappings.put("dcterms:created", "created");
+ fieldMappings.put("dcterms:modified", "modified");
+ fieldMappings.put("meta:author", "Author");
+ fieldMappings.put("meta:creation-date", "Creation-Date");
+ fieldMappings.put("meta:save-date", "Last-Save-Date");
+ fieldMappings.put("meta:keyword", "Keywords");
+ fieldMappings.put("pdf:docinfo:keywords", "Keywords");
+ }
+
+ /*
+ * Appends back-compatible metadata into the given {@code ExtractionMetadata} instance by mapping
+ * source fields to target fields, provided that backward compatibility is enabled. If a source
+ * field exists and the target field is not yet populated, the values from the source field will
+ * be added to the target field.
+ */
+ private void appendBackCompatTikaMetadata(ExtractionMetadata md) {
+ for (Map.Entry<String, String> mapping : fieldMappings.entrySet()) {
+ String sourceField = mapping.getKey();
+ String targetField = mapping.getValue();
+ if (md.getFirst(sourceField) != null && md.getFirst(targetField) == null) {
+ md.add(targetField, md.get(sourceField));
+ }
+ }
+ }
+
+ @Override
+ public void close() {
+ synchronized (INIT_LOCK) {
+ // Decrement reference count
+ REFERENCE_COUNT--;
Review Comment:
Use Solr's own `RefCounted` class for reference counting
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3401693598
> Any luck with security manager?? I had many difficulties
Skipping JSM for extraction tests was the easy way out, and given that JSM is going away as soon as you upgrade to JRE 24+, I'm not too worried about this shortcut.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2428716937
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractingRequestHandler.java:
##########
@@ -81,25 +64,116 @@ public void inform(SolrCore core) {
parseContextConfig =
new ParseContextConfig(core.getResourceLoader(), parseContextConfigLoc);
}
+
+ // Always create local backend
+ this.localBackend = new LocalTikaExtractionBackend(core, tikaConfigLoc, parseContextConfig);
+
+ // Optionally create Tika Server backend if URL configured
+ String tikaServerUrl = (String) initArgs.get(ExtractingParams.TIKASERVER_URL);
+ if (tikaServerUrl != null) {
+ int timeoutSecs = 0;
+ Object initTimeout = initArgs.get(ExtractingParams.TIKASERVER_TIMEOUT_SECS);
+ if (initTimeout != null) {
+ try {
+ timeoutSecs = Integer.parseInt(String.valueOf(initTimeout));
+ } catch (NumberFormatException nfe) {
+ throw new SolrException(
+ ErrorCode.SERVER_ERROR,
+ "Invalid value for '"
+ + ExtractingParams.TIKASERVER_TIMEOUT_SECS
+ + "': "
+ + initTimeout,
+ nfe);
+ }
+ }
+ Object maxCharsObj = initArgs.get(ExtractingParams.TIKASERVER_MAX_CHARS);
+ long maxCharsLimit = TikaServerExtractionBackend.DEFAULT_MAXCHARS_LIMIT;
+ if (maxCharsObj != null) {
+ try {
+ maxCharsLimit = Long.parseLong(String.valueOf(maxCharsObj));
+ } catch (NumberFormatException nfe) {
+ throw new SolrException(
+ ErrorCode.SERVER_ERROR,
+ "Invalid value for '"
+ + ExtractingParams.TIKASERVER_MAX_CHARS
+ + "': "
+ + maxCharsLimit);
+ }
+ }
+ this.tikaServerBackend =
+ new TikaServerExtractionBackend(tikaServerUrl, timeoutSecs, initArgs, maxCharsLimit);
+ }
+
+ // Choose default backend name
+ String backendName = (String) initArgs.get(ExtractingParams.EXTRACTION_BACKEND);
+ this.defaultBackendName =
Review Comment:
Validate that backendName is either null or one of the two allowed values
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3376533653
Feels like you are getting close! I will try and test this branch out today/tomorrow.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2373264168
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractingRequestHandler.java:
##########
@@ -77,20 +63,37 @@ public void inform(SolrCore core) {
parseContextConfig =
new ParseContextConfig(core.getResourceLoader(), parseContextConfigLoc);
}
+
+ // Initialize backend factory once; backends are created lazily on demand
+ String tikaServerUrl = (String) initArgs.get(TIKASERVER_URL);
+ backendFactory =
+ new ExtractionBackendFactory(core, tikaConfigLoc, parseContextConfig, tikaServerUrl);
+
+ // Choose default backend name (do not instantiate yet)
+ String backendName = (String) initArgs.get(ExtractingParams.EXTRACTION_BACKEND);
+ defaultBackendName =
Review Comment:
instead of `.ID` should it be `.NAME`? Or maybe call it `defaultBackend` and git it an ID value?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2373593269
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractionBackend.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.InputStream;
+import org.apache.tika.metadata.HttpHeaders;
+import org.apache.tika.metadata.TikaMetadataKeys;
+
+/** Strategy interface for content extraction backends. */
+public interface ExtractionBackend {
+ /**
+ * Extract plain text and metadata from the inputStream. Implementations should not close the
+ * inputStream. This API is backend-neutral and does not expose SAX or XML-specific types.
+ */
+ ExtractionResult extract(InputStream inputStream, ExtractionRequest request) throws Exception;
+
+ /**
+ * Perform extractOnly operation. If extractFormat equals ExtractingDocumentLoader.TEXT_FORMAT,
+ * return plain text. If XML, return XML body as string. Implementations may support optional
+ * xpathExpr; if unsupported and xpathExpr is not null, they should throw
+ * UnsupportedOperationException.
+ */
+ ExtractionResult extractOnly(InputStream inputStream, ExtractionRequest request, String xpathExpr)
+ throws Exception;
+
+ /**
+ * Parse the content and stream SAX events into the provided SolrContentHandler, while also
+ * filling outMetadata with extracted metadata.
+ */
+ void parseToSolrContentHandler(
+ InputStream inputStream,
+ ExtractionRequest request,
+ SolrContentHandler handler,
+ ExtractionMetadata outMetadata)
+ throws Exception;
+
+ /** Build ExtractionMetadata from the request context */
+ default ExtractionMetadata buildMetadataFromRequest(ExtractionRequest request) {
+ ExtractionMetadata md = new ExtractionMetadata();
+ if (request.resourceName != null)
Review Comment:
I'll instead replace these with a new convenience method `md.addIfNotNull()`..
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2373583885
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractingRequestHandler.java:
##########
@@ -77,20 +63,37 @@ public void inform(SolrCore core) {
parseContextConfig =
new ParseContextConfig(core.getResourceLoader(), parseContextConfigLoc);
}
+
+ // Initialize backend factory once; backends are created lazily on demand
+ String tikaServerUrl = (String) initArgs.get(TIKASERVER_URL);
+ backendFactory =
+ new ExtractionBackendFactory(core, tikaConfigLoc, parseContextConfig, tikaServerUrl);
+
+ // Choose default backend name (do not instantiate yet)
+ String backendName = (String) initArgs.get(ExtractingParams.EXTRACTION_BACKEND);
+ defaultBackendName =
Review Comment:
Agree, better use `NAME` consistently.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3328551433
I had a thought late last night @janhoy... So while I am super excited about the pluggable idea, I wonder if we have lost the core goal? The core goal is _to offer a way of reading rich documents for indexing in Solr without the maintenance burden on Solr and to be more in line with future of Tika_. If that is the core goal, I wonder if we should just target TikaServer in Solr 10, and not worry about any backcompat, beyond documenting it etc. We should just embrace the new ways of Tika working. If that makes sense, maybe the fact that some capabilities in Solr don't work at this point, like passwords or xpath, is okay if it's a Solr 10 only thing?
Does saying this is a Solr 10 only thing make it easier to have the tests pass by tweaking them and our implementation to leverage how Tika 3 and TikaServer works?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2428785645
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractionRequest.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/** Immutable request info needed by extraction backends. */
+public class ExtractionRequest {
+ public final String streamType;
+ public final String resourceName;
+ public final String contentType;
+ public final String charset;
+ public final String streamName;
+ public final String streamSourceInfo;
+ public final Long streamSize;
+ public final String resourcePassword;
+ public final java.util.LinkedHashMap<java.util.regex.Pattern, String> passwordsMap;
+ public final String extractFormat;
+
+ // Below variables are only used by TikaServerExtractionBackend
+ public final boolean tikaServerRecursive;
+ public final Integer tikaServerTimeoutSeconds; // optional per-request override
+ public final Map<String, String> tikaServerRequestHeaders = new HashMap<>();
+
+ /**
+ * Constructs an ExtractionRequest object containing metadata and configurations for extraction
+ * backends.
+ *
+ * @param streamType the explicit MIME type of the document (optional)
+ * @param resourceName the name of the resource, typically a filename hint
+ * @param contentType the HTTP content-type header value
+ * @param charset the derived character set of the stream if available
+ * @param streamName the name of the content stream
+ * @param streamSourceInfo additional information about the stream source
+ * @param streamSize the size of the stream in bytes
+ * @param resourcePassword an optional password used for encrypted documents
+ * @param passwordsMap an optional map of regex patterns to passwords for encrypted content
+ * @param extractFormat the desired format for extraction output
+ * @param tikaServerRecursive a flag indicating whether extraction should be recursive. TikaServer
+ * only
+ * @param tikaServerRequestHeaders optional headers to be included in requests to the extraction
+ * service. TikaServer only
+ */
+ public ExtractionRequest(
+ String streamType,
+ String resourceName,
Review Comment:
Consider builder pattern
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2426644943
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerParser.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Map;
+import javax.xml.parsers.SAXParser;
+import javax.xml.parsers.SAXParserFactory;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.Utils;
+import org.xml.sax.ContentHandler;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.helpers.DefaultHandler;
+
+public class TikaServerParser {
+ private final SAXParser saxParser;
+
+ public TikaServerParser() {
+ SAXParserFactory factory = SAXParserFactory.newInstance();
+ factory.setNamespaceAware(true);
+ try {
+ factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+ factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+ factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+ } catch (Throwable ignore) {
+ // Some parsers may not support all features; ignore
+ }
+ try {
+ saxParser = factory.newSAXParser();
+ } catch (Exception e) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+ }
+ }
+
+ /**
+ * Parses response in XML format from Tika Server /tika endpoint. The result is that the metadata
+ * object is populated and the content handler is called with extracted text.
+ */
+ public void parseXml(InputStream inputStream, ContentHandler handler, ExtractionMetadata metadata)
+ throws IOException, SAXException {
+ DefaultHandler xmlHandler = new TikaXmlResponseSaxContentHandler(handler, metadata);
+ try (Reader reader =
+ new XmlSanitizingReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8))) {
+ saxParser.parse(new InputSource(reader), xmlHandler);
+ }
+ }
+
+ /**
+ * Parses response in JSON format from Tika Server /rmeta endpoint. The result is that the
+ * metadata object is populated, and the content handler is called with extracted text.
+ *
+ * @param jsonStream - JSON stream to parse
+ * @param handler - SAX content handler to call with extracted text
+ * @param md - metadata object to populate
+ */
+ @SuppressWarnings({"rawtypes", "PatternVariableCanBeUsed"})
+ void parseRmetaJson(InputStream jsonStream, DefaultHandler handler, ExtractionMetadata md)
+ throws IOException, SAXException {
+ Object parsed = Utils.fromJSON(jsonStream);
Review Comment:
I implemented a generic config `tikaserver.maxChars` that will limit the max characters returned from TikaServer. Defaults to 100Mb. While this does not directly translate to how much memory would be used in Solr for this document, one can assume that the body text is the majority of most extractions and would consume appx this much heap. So this is a kind of circuit breaker.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2412978867
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java:
##########
@@ -0,0 +1,279 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.InputStream;
+import java.net.ConnectException;
+import java.net.SocketTimeoutException;
+import java.nio.channels.ClosedChannelException;
+import java.time.Duration;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.tika.sax.BodyContentHandler;
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.InputStreamRequestContent;
+import org.eclipse.jetty.client.InputStreamResponseListener;
+import org.eclipse.jetty.client.Request;
+import org.eclipse.jetty.client.Response;
+import org.eclipse.jetty.io.EofException;
+import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler;
+import org.xml.sax.helpers.DefaultHandler;
+
+/** Extraction backend using the Tika Server. It uses a shared Jetty HttpClient. */
+public class TikaServerExtractionBackend implements ExtractionBackend {
+ private static volatile HttpClient SHARED_CLIENT;
+ private static volatile ExecutorService SHARED_EXECUTOR;
+ private static final Object INIT_LOCK = new Object();
+ private static volatile boolean INITIALIZED = false;
+ private static volatile boolean SHUTDOWN = false;
+ private final String baseUrl;
+ private static final int DEFAULT_TIMEOUT_SECONDS = 3 * 60;
+ private final Duration defaultTimeout;
+ private final TikaServerParser tikaServerResponseParser = new TikaServerParser();
+
+ public TikaServerExtractionBackend(String baseUrl) {
+ this(baseUrl, DEFAULT_TIMEOUT_SECONDS);
+ }
+
+ public TikaServerExtractionBackend(String baseUrl, int timeoutSeconds) {
+ if (baseUrl.endsWith("/")) {
+ this.baseUrl = baseUrl.substring(0, baseUrl.length() - 1);
+ } else {
+ this.baseUrl = baseUrl;
+ }
+ this.defaultTimeout =
+ Duration.ofSeconds(timeoutSeconds > 0 ? timeoutSeconds : DEFAULT_TIMEOUT_SECONDS);
+ }
+
+ public static final String NAME = "tikaserver";
+
+ @Override
+ public String name() {
+ return NAME;
+ }
+
+ @Override
+ public ExtractionResult extract(InputStream inputStream, ExtractionRequest request)
+ throws Exception {
+ try (InputStream tikaResponse = callTikaServer(inputStream, request)) {
+ ExtractionMetadata md = buildMetadataFromRequest(request);
+ BodyContentHandler bodyContentHandler = new BodyContentHandler(-1);
+ if (request.tikaServerRecursive) {
+ tikaServerResponseParser.parseRmetaJson(tikaResponse, bodyContentHandler, md);
+ } else {
+ tikaServerResponseParser.parseXml(tikaResponse, bodyContentHandler, md);
+ }
+ return new ExtractionResult(bodyContentHandler.toString(), md);
+ }
+ }
+
+ @Override
+ public void extractWithSaxHandler(
+ InputStream inputStream,
+ ExtractionRequest request,
+ ExtractionMetadata md,
+ DefaultHandler saxContentHandler)
+ throws Exception {
+ try (InputStream tikaResponse = callTikaServer(inputStream, request)) {
+ if (request.tikaServerRecursive) {
+ tikaServerResponseParser.parseRmetaJson(tikaResponse, saxContentHandler, md);
+ } else {
+ tikaServerResponseParser.parseXml(tikaResponse, saxContentHandler, md);
+ }
+ }
+ }
+
+ /**
+ * Call the Tika Server to extract text and metadata. Depending on <code>request.recursive</code>,
+ * will either return XML (false) or JSON array (true). <b>The recursive mode consumes more memory
+ * both on the TikaServer side and on the Solr side</b>
+ *
+ * @return InputStream of the response body, either XML or JSON depending on <code>
+ * request.tikaserverRecursive</code>
+ */
+ InputStream callTikaServer(InputStream inputStream, ExtractionRequest request) throws Exception {
+ String url = baseUrl + (request.tikaServerRecursive ? "/rmeta" : "/tika");
+
+ ensureClientInitialized();
+ HttpClient client = SHARED_CLIENT;
+
+ Request req = client.newRequest(url).method("PUT");
+ Duration effectiveTimeout =
+ (request.tikaServerTimeoutSeconds != null && request.tikaServerTimeoutSeconds > 0)
+ ? Duration.ofSeconds(request.tikaServerTimeoutSeconds)
+ : defaultTimeout;
+ req.timeout(effectiveTimeout.toMillis(), TimeUnit.MILLISECONDS);
+
+ // Headers
+ String accept = (request.tikaServerRecursive ? "application/json" : "text/xml");
+ req.headers(h -> h.add("Accept", accept));
+ String contentType = (request.streamType != null) ? request.streamType : request.contentType;
+ if (contentType != null) {
+ req.headers(h -> h.add("Content-Type", contentType));
+ }
+ if (!request.tikaServerRequestHeaders.isEmpty()) {
+ req.headers(
+ h ->
+ request.tikaServerRequestHeaders.forEach(
+ (k, v) -> {
+ if (k != null && v != null) h.add(k, v);
+ }));
+ }
+
+ ExtractionMetadata md = buildMetadataFromRequest(request);
+ if (request.resourcePassword != null || request.passwordsMap != null) {
+ RegexRulesPasswordProvider passwordProvider = new RegexRulesPasswordProvider();
+ if (request.resourcePassword != null) {
+ passwordProvider.setExplicitPassword(request.resourcePassword);
+ }
+ if (request.passwordsMap != null) {
+ passwordProvider.setPasswordMap(request.passwordsMap);
+ }
+ String pwd = passwordProvider.getPassword(md);
+ if (pwd != null) {
+ req.headers(h -> h.add("Password", pwd)); // Tika Server expects this header if provided
+ }
+ }
+ if (request.resourceName != null) {
+ req.headers(
+ h ->
+ h.add(
+ "Content-Disposition", "attachment; filename=\"" + request.resourceName + "\""));
+ }
+
+ if (contentType != null) {
+ req.body(new InputStreamRequestContent(contentType, inputStream));
+ } else {
+ req.body(new InputStreamRequestContent(inputStream));
+ }
+
+ InputStreamResponseListener listener = new InputStreamResponseListener();
+ req.send(listener);
+
+ final Response response;
+ try {
+ response = listener.get(effectiveTimeout.toMillis(), TimeUnit.MILLISECONDS);
+ } catch (TimeoutException te) {
+ throw new SolrException(
+ SolrException.ErrorCode.GATEWAY_TIMEOUT,
+ "Timeout after "
+ + effectiveTimeout.toMillis()
+ + " ms while waiting for response from TikaServer "
+ + url,
+ te);
+ } catch (InterruptedException ie) {
+ Thread.currentThread().interrupt();
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Interrupted while waiting for response from TikaServer " + url,
+ ie);
+ } catch (ExecutionException ee) {
+ Throwable cause = ee.getCause();
+ if (cause instanceof ConnectException
+ || cause instanceof SocketTimeoutException
+ || cause instanceof EofException
+ || cause instanceof ClosedChannelException) {
+ throw new SolrException(
+ SolrException.ErrorCode.SERVICE_UNAVAILABLE,
+ "Error communicating with TikaServer "
Review Comment:
I added a new response code to our `SolrException` class: 504 Gateway Timeout, see line 178.
I could have used 503, but in this case, it is not really Solr itself that is unavailable (due to timeout), but the call to TikaServer that timed out. Using a different return code, indexing clients can more easily distringuish whether solr itself may have died or if it's the tika server that failed to respond within the timeout.
Appreciate a review of error handling in this class. If the call to TikaServer failed with an exception in line 190, I'm not entirely sure that HTTP 503 is the best choice, perhaps 504 here as well to signal it is a problem with the connection to the TikaServer backend?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2412992252
##########
solr/solr-ref-guide/modules/indexing-guide/pages/indexing-with-tika.adoc:
##########
@@ -391,6 +463,33 @@ This is only required if you have customized your Tika implementation.
+
Example: `tika.config=/path/to/tika.config`
+`tikaserver.timeoutSeconds`::
++
+[%autowidth,frame=none]
+|===
+|Optional |Default: `180` seconds
+|===
++
+Sets the HTTP timeout when communicating with Tika Server, in seconds. Can be set per request as a parameter or as a default in the request handler configuration. If set on the request it overrides the handler default for that call only.
++
+Examples:
++
+- Per request: `tikaserver.timeoutSeconds=60`
+- In `solrconfig.xml`: `<int name="tikaserver.timeoutSeconds">60</int>`
+
+
+`tikaserver.url`::
++
+[%autowidth,frame=none]
+|===
+|Optional |Default: none
+|===
++
+Specifies the URL of the Tika server to use when the `extraction.backend` parameter is set to `tikaserver`.
+This parameter is required when using the `tikaserver` backend. This parameter can only be configured in the configuration, not per request.
++
Review Comment:
Instead of building support for HTTPs and custom certs from the beginning, let's document the limitation instead:
```suggestion
+
If your TikaServer is using HTTPS, it needs to use a verifiable SSL certificate. If using self-signed or custom Certificate Authority, you will need to add those to Solr's Truststore. The `tikaserver` backend currently does not have support for configuring custom certificates for Tika alone.
+
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2411299023
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractingParams.java:
##########
@@ -136,4 +136,19 @@ public interface ExtractingParams {
* .*=<defaultmypassword> at the end
*/
public static final String PASSWORD_MAP_FILE = "passwordsFile";
+
+ /** Backend selection parameter and */
+ public static final String EXTRACTION_BACKEND = "extraction.backend";
+
+ /** Fix metadata to match Tika 1.x */
+ public static final String BACK_COMPATIBILITY = "extraction.metadataBackCompat";
+
+ /**
+ * Enable recursive parsing of embedded documents when using TikaServer. This is experimental,
+ * uses /rmeta endpoint, uses more RAM and is disabled by default.
+ */
+ public static final String RECURSIVE = "extraction.tikaserver.recursive";
Review Comment:
Rename as `tikaserver.recursive`
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "iamsanjay (via GitHub)" <gi...@apache.org>.
iamsanjay commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3319389324
I had the similar experience as I was upgrading kafka. And then I stopped.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2381746602
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractingRequestHandler.java:
##########
@@ -28,26 +26,24 @@
import org.apache.solr.security.PermissionNameProvider;
import org.apache.solr.update.processor.UpdateRequestProcessor;
import org.apache.solr.util.plugin.SolrCoreAware;
-import org.apache.tika.config.TikaConfig;
/**
* Handler for rich documents like PDF or Word or any other file format that Tika handles that need
* the text to be extracted first from the document.
- *
- * @deprecated Will be replaced with something similar that calls out to a separate Tika Server
- * process running in its own JVM.
*/
-@Deprecated(since = "9.10.0")
Review Comment:
@epugh I undeprecated this and the Loader, and instead deprecated the Local backend. This part needs to be backported before 9.10 release. Also perhaps wording in major-changes...
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3337472814
Last commit adds recursive parsing as an option `&recursive=true` for backend `tikaserver`. I moved the failing image test to the Local test class, since there is a difference in how PDFs with images are parsed in 1.x and 3.x. In 1.x embedded images would leave traces in the extracted text by default. Not anymore. But I added a test to the tikaserver backend test class that extracts the same pdf, with recursive enabled and also a special HTTP header to TikaServer `X-Tika-PDFextractInlineImages=true` which indeed extracts the images an adds their names to the text. This recursive business is highly experimental, it's a different endpoint `/rmeta`, returning JSON which needs to be buffered entirely in memory both on TikaServer side and on Solr side. Also all the embedded documents are returned concatenated together in the content, and I believe that the metadata from the main object is the only retained?
All tests are now green. However, there is still a thread leak in the tikaserver test. I think there are some HttpClient stuff not released.
Other TODO:
- [ ] Lots of code from GenAI, which needs review and rewrite / simplification.
- [ ] There may be debug print and TODOs left here and there
- [ ] The back-compat metadata map is AI generated and by no means complete or even correct đŸ¤£
- [ ] Lack of JavaDoc everywhere
- [ ] Perhaps some code is Java21, making backport a challenge
- [ ] Not much attention has been given to exception handling, retrying, timeout values etc
- [ ] Should probably use Jetty HTTP client.
- [ ] Throughput with many update requests? Right now the request thread will be blocking on Tika response and parsing..
- [ ] If TikaServer supports HTTPS we'd probably need to handle self-signed SSL either through truststore, or custom config like we did for JWT-auth.
- [ ] If TIkaServer supports Auth, that must be thought of
- [ ] Complete refGuide docs
- [ ] Split the huge PR into stages, i.e. first only support for pluggable backend, then add the tikaserver backend
That concludes the "POC", proving that it is doable to do a drop-in replacement for users.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3337590686
We now have a separate github workflow testing extraction code, with TestContainers. It is only for the sake of this PR, not intendend for merge :)
The thread leaks definitely looks related to ordinary Solr objects.
```
> Task :solr:modules:extraction:test
ExtractingRequestHandlerTikaServerTest > classMethod FAILED
java.lang.AssertionError: ObjectTracker found 11 object(s) that were not released!!! [MockDirectoryWrapper, Http2SolrClient, Http2SolrClient, Http2SolrClient, MDCAwareThreadPoolExecutor, SolrIndexSearcher, MockDirectoryWrapper, SolrCore, Http2SolrClient, LBHttp2SolrClient, MockDirectoryWrapper]
org.apache.lucene.tests.store.MockDirectoryWrapper:org.apache.solr.common.util.ObjectReleaseTracker$ObjectTrackerException: org.apache.lucene.tests.store.MockDirectoryWrapper
at org.apache.solr.common.util.ObjectReleaseTracker.track(ObjectReleaseTracker.java:54)
...
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3337616731
@epugh and others - I'll be on holiday for a week from today. Feel free to commit anything you like directly to this branch without asking, if you want to play around or move things closer to perfection. Normal review comments are of course welcome too, but commits eats comments for breakfast :)
Any phased merge can be done later, as the interface boundaries are fairly clean, hopefully.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3403339131
> To make an even cleaner separation, let's not import any `org.apache.tika` classes in the base classes that will survive the removal of `local` backend.
Very astute comment! I am excited about adding `tika-pipes` as a replacement for `local` in the future...
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2425412615
##########
solr/solr-ref-guide/modules/indexing-guide/pages/indexing-with-tika.adoc:
##########
@@ -355,6 +409,26 @@ See the section <<Indexing Encrypted Documents>> for more information about usin
+
Example: `passwordsFile=/path/to/passwords.txt`
+`tikaserver.metadata.compatibility`::
++
+[%autowidth,frame=none]
Review Comment:
I lean towards *NOT* documenting this feature (`tikaserver.metadata.compatibility`) in the first release, as it still feels unfinished and hacky. If we don't document it, and mark java parts as experimental, it will be easier to remove it later and avoid feature creep. I believe it could be useful, but if so, it should be more configurable and more clearly documented.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2425475396
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerParser.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Map;
+import javax.xml.parsers.SAXParser;
+import javax.xml.parsers.SAXParserFactory;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.Utils;
+import org.xml.sax.ContentHandler;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.helpers.DefaultHandler;
+
+public class TikaServerParser {
+ private final SAXParser saxParser;
+
+ public TikaServerParser() {
+ SAXParserFactory factory = SAXParserFactory.newInstance();
+ factory.setNamespaceAware(true);
+ try {
+ factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+ factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+ factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+ } catch (Throwable ignore) {
+ // Some parsers may not support all features; ignore
+ }
+ try {
+ saxParser = factory.newSAXParser();
+ } catch (Exception e) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+ }
+ }
+
+ /**
+ * Parses response in XML format from Tika Server /tika endpoint. The result is that the metadata
+ * object is populated and the content handler is called with extracted text.
+ */
+ public void parseXml(InputStream inputStream, ContentHandler handler, ExtractionMetadata metadata)
+ throws IOException, SAXException {
+ DefaultHandler xmlHandler = new TikaXmlResponseSaxContentHandler(handler, metadata);
+ try (Reader reader =
+ new XmlSanitizingReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8))) {
+ saxParser.parse(new InputSource(reader), xmlHandler);
+ }
+ }
+
+ /**
+ * Parses response in JSON format from Tika Server /rmeta endpoint. The result is that the
+ * metadata object is populated, and the content handler is called with extracted text.
+ *
+ * @param jsonStream - JSON stream to parse
+ * @param handler - SAX content handler to call with extracted text
+ * @param md - metadata object to populate
+ */
+ @SuppressWarnings({"rawtypes", "PatternVariableCanBeUsed"})
+ void parseRmetaJson(InputStream jsonStream, DefaultHandler handler, ExtractionMetadata md)
+ throws IOException, SAXException {
+ Object parsed = Utils.fromJSON(jsonStream);
+ if (!(parsed instanceof List)) {
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR, "Unexpected /rmeta response, expected JSON array");
+ }
+ List list = (List) parsed;
+ for (Object o : list) {
+ if (!(o instanceof Map)) continue;
+ Map map = (Map) o;
+ // Copy metadata
+ for (Object k : map.keySet()) {
+ String key = String.valueOf(k);
+ Object val = map.get(k);
+ if ("X-TIKA:content".equalsIgnoreCase(key)) {
+ // handled below
+ continue;
+ }
+ if (val instanceof List) {
+ for (Object v : (List) val) {
+ if (v != null) md.add(key, String.valueOf(v));
+ }
+ } else if (val != null) {
+ md.add(key, String.valueOf(val));
+ }
+ }
+ Object content = map.get("X-TIKA:content");
+ if (content != null) {
+ String xhtml = String.valueOf(content);
+ if (!xhtml.isEmpty() && handler != null) {
+ InputStream inputStream =
+ new ByteArrayInputStream(xhtml.getBytes(StandardCharsets.UTF_8));
+ try (Reader reader =
+ new XmlSanitizingReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8))) {
+ saxParser.parse(new InputSource(reader), handler);
+ }
+ }
+ }
+ }
+ }
+
+ /** Custom SAX handler that will extract meta tags from the tika xml and delegate */
+ static class TikaXmlResponseSaxContentHandler extends DefaultHandler {
+ private final ContentHandler delegate;
+ private final ExtractionMetadata metadata;
+ private boolean inHead = false;
+
+ public TikaXmlResponseSaxContentHandler(ContentHandler delegate, ExtractionMetadata metadata) {
+ this.delegate = delegate;
+ this.metadata = metadata;
+ }
+
+ @Override
+ public void startDocument() throws SAXException {
+ if (delegate != null) delegate.startDocument();
+ }
+
+ @Override
+ public void endDocument() throws SAXException {
+ if (delegate != null) delegate.endDocument();
+ }
+
+ @Override
+ public void startElement(
+ String uri, String localName, String qName, org.xml.sax.Attributes attributes)
+ throws SAXException {
+ String ln = localName != null && !localName.isEmpty() ? localName : qName;
+ if ("head".equalsIgnoreCase(ln)) {
+ inHead = true;
+ } else if (inHead && "meta".equalsIgnoreCase(ln) && attributes != null) {
+ String name = attributes.getValue("name");
+ String content = attributes.getValue("content");
+ if (name != null && content != null) {
+ metadata.add(name, content);
+ }
+ }
+ if (delegate != null) delegate.startElement(uri, localName, qName, attributes);
+ }
+
+ @Override
+ public void endElement(String uri, String localName, String qName) throws SAXException {
+ String ln = localName != null && !localName.isEmpty() ? localName : qName;
+ if ("head".equalsIgnoreCase(ln)) {
+ inHead = false;
+ }
+ if (delegate != null) delegate.endElement(uri, localName, qName);
+ }
+
+ @Override
+ public void characters(char[] ch, int start, int length) throws SAXException {
+ if (delegate != null) delegate.characters(ch, start, length);
Review Comment:
For normal Tika XML response, we can implement a "circuit breaker" here, by counting chars added and throwing an exception if we pass the limit (e.g. 1Gb)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2373267263
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractionBackend.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.InputStream;
+import org.apache.tika.metadata.HttpHeaders;
+import org.apache.tika.metadata.TikaMetadataKeys;
+
+/** Strategy interface for content extraction backends. */
+public interface ExtractionBackend {
+ /**
+ * Extract plain text and metadata from the inputStream. Implementations should not close the
+ * inputStream. This API is backend-neutral and does not expose SAX or XML-specific types.
+ */
+ ExtractionResult extract(InputStream inputStream, ExtractionRequest request) throws Exception;
+
+ /**
+ * Perform extractOnly operation. If extractFormat equals ExtractingDocumentLoader.TEXT_FORMAT,
+ * return plain text. If XML, return XML body as string. Implementations may support optional
+ * xpathExpr; if unsupported and xpathExpr is not null, they should throw
+ * UnsupportedOperationException.
+ */
+ ExtractionResult extractOnly(InputStream inputStream, ExtractionRequest request, String xpathExpr)
+ throws Exception;
+
+ /**
+ * Parse the content and stream SAX events into the provided SolrContentHandler, while also
+ * filling outMetadata with extracted metadata.
+ */
+ void parseToSolrContentHandler(
+ InputStream inputStream,
+ ExtractionRequest request,
+ SolrContentHandler handler,
+ ExtractionMetadata outMetadata)
+ throws Exception;
+
+ /** Build ExtractionMetadata from the request context */
+ default ExtractionMetadata buildMetadataFromRequest(ExtractionRequest request) {
+ ExtractionMetadata md = new ExtractionMetadata();
+ if (request.resourceName != null)
+ md.add(TikaMetadataKeys.RESOURCE_NAME_KEY, request.resourceName);
+ if (request.contentType != null) md.add(HttpHeaders.CONTENT_TYPE, request.contentType);
+ if (request.streamName != null)
+ md.add(ExtractingMetadataConstants.STREAM_NAME, request.streamName);
+ if (request.streamSourceInfo != null)
+ md.add(ExtractingMetadataConstants.STREAM_SOURCE_INFO, request.streamSourceInfo);
+ if (request.streamSize != null)
+ md.add(ExtractingMetadataConstants.STREAM_SIZE, String.valueOf(request.streamSize));
+ if (request.contentType != null)
+ md.add(ExtractingMetadataConstants.STREAM_CONTENT_TYPE, request.contentType);
+ if (request.charset != null) md.add(HttpHeaders.CONTENT_ENCODING, request.charset);
+ return md;
+ }
+
+ /** A short name for debugging/config, e.g., "local" or "dummy". */
+ String name();
Review Comment:
decide if it's name or id?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3325976701
I vote for moving in the direction of Tika 3 and how it works and maybe update the tests? If this is Solr 10, can't we add the changes to being "breaking changes"? Also, a thought... could we have a set of tests that validate how Tika 1 worked, and are specific to Tika 1, and antoher that handle what Tika 3 does. Then we don't have to make everything from 1 work in 3... ANd maybe there are things in 3 that we would want to test? AFter all, in Solr 10, don't we eliminate tika 1 approach anyway?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2373232009
##########
solr/modules/extraction/src/test-files/extraction/solr/collection1/conf/solrconfig.xml:
##########
@@ -152,6 +152,8 @@
<requestHandler name="/update/extract" class="org.apache.solr.handler.extraction.ExtractingRequestHandler">
<str name="parseContext.config">parseContext.xml</str>
+ <str name="extraction.backend">${solr.test.extraction.backend:local}</str>
+ <str name="tikaserver.url">${solr.test.tikaserver.url:}</str>
Review Comment:
What do you think about "solr.tikaserver.url" instead? I could imagine that on start up you might pass into solr `bin/solr start -Dsolr.tikaserver.url=http://tools.mycompany.com/tika" ? Do we need it to be specific to `solr.test`?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3334645293
So, pushed a commit with some nice changes:
* Refactor some logic back to ExtractingDocumentLoader, simplify ExtractionBackend interface to two methods
* Add `backCompatibility=true` config option to enable duplicating some metadata like Tika 1.x did, e.g. both `dc:title` and `title`
* Fix true SAX streaming parser for Tika-Server XML response. We now have or own `TikaXmlResponseSaxContentHandler` which takes care of pulling metadata from the response, while delegating other SAX parsing to whatever `ContentHandler` is passed to the parse method. This lets us re-use existing code to extract plain-text, xml, or capturing, xpath style tags
Not all tests pass, but two more are green.
<img width="321" height="384" alt="Skjermbilde 2025-09-25 kl 17 02 08" src="https://github.com/user-attachments/assets/74f683f7-9c98-4106-9fa8-2c712529897e" />
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3401417445
Did a series of final improvements, based on review feedback from Claude Code, added a new test, hardened input parameter validation etc.
Planning to merge this to main Wednesday morning CET, hopefully incorporating feedback from @epugh taking it for a spin.
If any of you have a review pending, let me know so I can incorporate that as well.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2428662450
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java:
##########
@@ -0,0 +1,403 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.ConnectException;
+import java.net.SocketTimeoutException;
+import java.nio.channels.ClosedChannelException;
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.tika.sax.BodyContentHandler;
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.InputStreamRequestContent;
+import org.eclipse.jetty.client.InputStreamResponseListener;
+import org.eclipse.jetty.client.Request;
+import org.eclipse.jetty.client.Response;
+import org.eclipse.jetty.io.EofException;
+import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler;
+import org.xml.sax.helpers.DefaultHandler;
+
+/** Extraction backend using the Tika Server. It uses a shared Jetty HttpClient. */
+public class TikaServerExtractionBackend implements ExtractionBackend {
+ public static final long DEFAULT_MAXCHARS_LIMIT = 100 * 1024 * 1024;
+ private static volatile HttpClient SHARED_CLIENT;
+ private static volatile ExecutorService SHARED_EXECUTOR;
+ private static final Object INIT_LOCK = new Object();
+ private static volatile boolean INITIALIZED = false;
+ private static volatile boolean SHUTDOWN = false;
+ private final String baseUrl;
+ private static final int DEFAULT_TIMEOUT_SECONDS = 3 * 60;
+ private final Duration defaultTimeout;
+ private final TikaServerParser tikaServerResponseParser = new TikaServerParser();
+ private boolean tikaMetadataCompatibility;
+ private HashMap<String, Object> initArgsMap = new HashMap<>();
+ private final long maxCharsLimit;
+
+ public TikaServerExtractionBackend(String baseUrl) {
+ this(baseUrl, DEFAULT_TIMEOUT_SECONDS, null, DEFAULT_MAXCHARS_LIMIT);
+ }
+
+ public TikaServerExtractionBackend(
+ String baseUrl, int timeoutSeconds, NamedList<?> initArgs, long maxCharsLimit) {
+ this.maxCharsLimit = maxCharsLimit;
Review Comment:
Should validate baseUrl
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2426770023
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractingRequestHandler.java:
##########
@@ -81,25 +64,117 @@ public void inform(SolrCore core) {
parseContextConfig =
new ParseContextConfig(core.getResourceLoader(), parseContextConfigLoc);
}
+
+ // Always create local backend
+ this.localBackend = new LocalTikaExtractionBackend(core, tikaConfigLoc, parseContextConfig);
+
+ // Optionally create Tika Server backend if URL configured
+ String tikaServerUrl = (String) initArgs.get(ExtractingParams.TIKASERVER_URL);
+ if (tikaServerUrl != null) {
+ int timeoutSecs = 0;
+ Object initTimeout = initArgs.get(ExtractingParams.TIKASERVER_TIMEOUT_SECS);
+ if (initTimeout != null) {
+ try {
+ timeoutSecs = Integer.parseInt(String.valueOf(initTimeout));
+ } catch (NumberFormatException nfe) {
+ throw new SolrException(
+ ErrorCode.SERVER_ERROR,
+ "Invalid value for '"
+ + ExtractingParams.TIKASERVER_TIMEOUT_SECS
+ + "': "
+ + initTimeout,
+ nfe);
+ }
+ }
+ Object maxCharsObj = initArgs.get(ExtractingParams.TIKASERVER_MAX_CHARS);
+ long maxCharsLimit = 1024 * 1024 * 1024;
+ ;
Review Comment:
Remove the extra semicolon on line 91. This is a formatting issue that makes the code look inconsistent.
```suggestion
```
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java:
##########
@@ -0,0 +1,402 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.ConnectException;
+import java.net.SocketTimeoutException;
+import java.nio.channels.ClosedChannelException;
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.tika.sax.BodyContentHandler;
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.InputStreamRequestContent;
+import org.eclipse.jetty.client.InputStreamResponseListener;
+import org.eclipse.jetty.client.Request;
+import org.eclipse.jetty.client.Response;
+import org.eclipse.jetty.io.EofException;
+import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler;
+import org.xml.sax.helpers.DefaultHandler;
+
+/** Extraction backend using the Tika Server. It uses a shared Jetty HttpClient. */
+public class TikaServerExtractionBackend implements ExtractionBackend {
+ private static volatile HttpClient SHARED_CLIENT;
+ private static volatile ExecutorService SHARED_EXECUTOR;
+ private static final Object INIT_LOCK = new Object();
+ private static volatile boolean INITIALIZED = false;
+ private static volatile boolean SHUTDOWN = false;
+ private final String baseUrl;
+ private static final int DEFAULT_TIMEOUT_SECONDS = 3 * 60;
+ private final Duration defaultTimeout;
+ private final TikaServerParser tikaServerResponseParser = new TikaServerParser();
+ private boolean tikaMetadataCompatibility;
+ private HashMap<String, Object> initArgsMap = new HashMap<>();
+ private final long maxCharsLimit;
+
+ public TikaServerExtractionBackend(String baseUrl) {
+ this(baseUrl, DEFAULT_TIMEOUT_SECONDS, null, 100 * 1024 * 1024);
+ }
+
+ public TikaServerExtractionBackend(
+ String baseUrl, int timeoutSeconds, NamedList<?> initArgs, long maxCharsLimit) {
+ this.maxCharsLimit = maxCharsLimit;
+ if (initArgs != null) {
+ initArgs.toMap(this.initArgsMap);
+ }
+ Object metaCompatObh = this.initArgsMap.get(ExtractingParams.TIKASERVER_METADATA_COMPATIBILITY);
+ if (metaCompatObh != null) {
+ this.tikaMetadataCompatibility = Boolean.parseBoolean(metaCompatObh.toString());
+ }
+ if (timeoutSeconds <= 0) {
+ timeoutSeconds = DEFAULT_TIMEOUT_SECONDS;
+ }
+ if (baseUrl.endsWith("/")) {
+ this.baseUrl = baseUrl.substring(0, baseUrl.length() - 1);
+ } else {
+ this.baseUrl = baseUrl;
+ }
+ this.defaultTimeout =
+ Duration.ofSeconds(timeoutSeconds > 0 ? timeoutSeconds : DEFAULT_TIMEOUT_SECONDS);
+ }
+
+ public static final String NAME = "tikaserver";
+
+ @Override
+ public String name() {
+ return NAME;
+ }
+
+ @Override
+ public ExtractionResult extract(InputStream inputStream, ExtractionRequest request)
+ throws Exception {
+ try (InputStream tikaResponse = callTikaServer(inputStream, request)) {
+ ExtractionMetadata md = buildMetadataFromRequest(request);
+ BodyContentHandler bodyContentHandler = new BodyContentHandler(-1);
+ if (request.tikaServerRecursive) {
+ tikaServerResponseParser.parseRmetaJson(tikaResponse, bodyContentHandler, md);
+ } else {
+ tikaServerResponseParser.parseXml(tikaResponse, bodyContentHandler, md);
+ }
+ if (tikaMetadataCompatibility) {
+ appendBackCompatTikaMetadata(md);
+ }
+ return new ExtractionResult(bodyContentHandler.toString(), md);
+ }
+ }
+
+ @Override
+ public void extractWithSaxHandler(
+ InputStream inputStream,
+ ExtractionRequest request,
+ ExtractionMetadata md,
+ DefaultHandler saxContentHandler)
+ throws Exception {
+ try (InputStream tikaResponse = callTikaServer(inputStream, request)) {
+ if (request.tikaServerRecursive) {
+ tikaServerResponseParser.parseRmetaJson(tikaResponse, saxContentHandler, md);
+ } else {
+ tikaServerResponseParser.parseXml(tikaResponse, saxContentHandler, md);
+ }
+ if (tikaMetadataCompatibility) {
+ appendBackCompatTikaMetadata(md);
+ }
+ }
+ }
+
+ /**
+ * Call the Tika Server to extract text and metadata. Depending on <code>request.recursive</code>,
+ * will either return XML (false) or JSON array (true). <b>The recursive mode consumes more memory
+ * both on the TikaServer side and on the Solr side</b>
+ *
+ * @return InputStream of the response body, either XML or JSON depending on <code>
+ * request.tikaserverRecursive</code>
+ */
+ InputStream callTikaServer(InputStream inputStream, ExtractionRequest request) throws Exception {
+ String url = baseUrl + (request.tikaServerRecursive ? "/rmeta" : "/tika");
+
+ ensureClientInitialized();
+ HttpClient client = SHARED_CLIENT;
+
+ Request req = client.newRequest(url).method("PUT");
+ Duration effectiveTimeout =
+ (request.tikaServerTimeoutSeconds != null && request.tikaServerTimeoutSeconds > 0)
+ ? Duration.ofSeconds(request.tikaServerTimeoutSeconds)
+ : defaultTimeout;
+ req.timeout(effectiveTimeout.toMillis(), TimeUnit.MILLISECONDS);
+
+ // Headers
+ String accept = (request.tikaServerRecursive ? "application/json" : "text/xml");
+ req.headers(h -> h.add("Accept", accept));
+ String contentType = (request.streamType != null) ? request.streamType : request.contentType;
+ if (contentType != null) {
+ req.headers(h -> h.add("Content-Type", contentType));
+ }
+ if (!request.tikaServerRequestHeaders.isEmpty()) {
+ req.headers(
+ h ->
+ request.tikaServerRequestHeaders.forEach(
+ (k, v) -> {
+ if (k != null && v != null) h.add(k, v);
+ }));
+ }
+
+ ExtractionMetadata md = buildMetadataFromRequest(request);
+ if (request.resourcePassword != null || request.passwordsMap != null) {
+ RegexRulesPasswordProvider passwordProvider = new RegexRulesPasswordProvider();
+ if (request.resourcePassword != null) {
+ passwordProvider.setExplicitPassword(request.resourcePassword);
+ }
+ if (request.passwordsMap != null) {
+ passwordProvider.setPasswordMap(request.passwordsMap);
+ }
+ String pwd = passwordProvider.getPassword(md);
+ if (pwd != null) {
+ req.headers(h -> h.add("Password", pwd)); // Tika Server expects this header if provided
+ }
+ }
+ if (request.resourceName != null) {
+ req.headers(
+ h ->
+ h.add(
+ "Content-Disposition", "attachment; filename=\"" + request.resourceName + "\""));
+ }
+
+ if (contentType != null) {
+ req.body(new InputStreamRequestContent(contentType, inputStream));
+ } else {
+ req.body(new InputStreamRequestContent(inputStream));
+ }
+
+ InputStreamResponseListener listener = new InputStreamResponseListener();
+ req.send(listener);
+
+ final Response response;
+ try {
+ response = listener.get(effectiveTimeout.toMillis(), TimeUnit.MILLISECONDS);
+ } catch (TimeoutException te) {
+ throw new SolrException(
+ SolrException.ErrorCode.GATEWAY_TIMEOUT,
+ "Timeout after "
+ + effectiveTimeout.toMillis()
+ + " ms while waiting for response from TikaServer "
+ + url,
+ te);
+ } catch (InterruptedException ie) {
+ Thread.currentThread().interrupt();
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Interrupted while waiting for response from TikaServer " + url,
+ ie);
+ } catch (ExecutionException ee) {
+ Throwable cause = ee.getCause();
+ if (cause instanceof ConnectException
+ || cause instanceof SocketTimeoutException
+ || cause instanceof EofException
+ || cause instanceof ClosedChannelException) {
+ throw new SolrException(
+ SolrException.ErrorCode.SERVICE_UNAVAILABLE,
+ "Error communicating with TikaServer "
+ + url
+ + ": "
+ + cause.getClass().getSimpleName()
+ + ": "
+ + cause.getMessage(),
+ cause);
+ }
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Unexpected error while calling TikaServer " + url,
+ ee);
+ }
+
+ int code = response.getStatus();
+ if (code < 200 || code >= 300) {
+ SolrException.ErrorCode errorCode = SolrException.ErrorCode.getErrorCode(code);
+ String reason = response.getReason();
+ String msg =
+ "TikaServer "
+ + url
+ + " returned status "
+ + code
+ + (reason != null ? " (" + reason + ")" : "");
+ throw new SolrException(errorCode, msg);
+ }
+
+ InputStream responseStream = listener.getInputStream();
+ // Bound the amount of data we read from Tika Server to avoid excessive memory/CPU usage
+ return new LimitingInputStream(responseStream, maxCharsLimit);
+ }
+
+ private static class LimitingInputStream extends InputStream {
+ private final InputStream in;
+ private final long max;
+ private long count;
+
+ LimitingInputStream(InputStream in, long max) {
+ this.in = in;
+ this.max = max;
+ this.count = 0L;
+ }
+
+ private void checkLimit(long toAdd) {
+ if (max <= 0) return; // non-positive means unlimited
+ long newCount = count + toAdd;
+ if (newCount > max) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "TikaServer response exceeded the configured maximum size of " + max + " bytes");
+ }
+ count = newCount;
+ }
+
+ @Override
+ public int read() throws IOException {
+ int b = in.read();
+ if (b != -1) {
+ checkLimit(1);
+ }
+ return b;
+ }
+
+ @Override
+ public int read(byte[] b, int off, int len) throws IOException {
+ int n = in.read(b, off, len);
+ if (n > 0) {
+ checkLimit(n);
+ }
+ return n;
+ }
+
+ @Override
+ public long skip(long n) throws IOException {
+ long skipped = in.skip(n);
+ if (skipped > 0) {
+ checkLimit(skipped);
+ }
+ return skipped;
+ }
+
+ @Override
+ public void close() throws IOException {
+ in.close();
+ }
+
+ @Override
+ public int available() throws IOException {
+ return in.available();
+ }
+ }
+
+ private static void ensureClientInitialized() {
+ if (INITIALIZED) return;
+ synchronized (INIT_LOCK) {
+ if (INITIALIZED) return;
+ ThreadFactory tf = new SolrNamedThreadFactory("TikaServerHttpClient");
+ ExecutorService exec = ExecutorUtil.newMDCAwareCachedThreadPool(tf);
+ HttpClient client = new HttpClient();
+ client.setExecutor(exec);
+ client.setScheduler(new ScheduledExecutorScheduler("TikaServerHttpClient-scheduler", true));
+ try {
+ client.start();
+ } catch (Exception e) {
+ try {
+ exec.shutdownNow();
+ } catch (Throwable ignore) {
+ }
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR, "Failed to start shared Jetty HttpClient", e);
+ }
+ SHARED_EXECUTOR = exec;
+ SHARED_CLIENT = client;
+ INITIALIZED = true;
+ SHUTDOWN = false;
+ }
+ }
+
+ private final Map<String, String> fieldMappings = new LinkedHashMap<>();
+
+ // TODO: Improve backward compatibility by adding more mappings
Review Comment:
This TODO comment indicates incomplete functionality. Consider either implementing the additional mappings or documenting the current limitations more clearly in the code comments.
```suggestion
/*
* Backward compatibility mappings for Tika metadata fields.
* Only the following mappings are currently supported:
* dc:title -> title
* dc:creator -> author
* dc:description -> description
* dc:subject -> subject
* dc:language -> language
* dc:publisher -> publisher
* dcterms:created -> created
* dcterms:modified -> modified
* meta:author -> Author
* meta:creation-date -> Creation-Date
* meta:save-date -> Last-Save-Date
* meta:keyword -> Keywords
* pdf:docinfo:keywords -> Keywords
* If additional mappings are required for backward compatibility,
* they must be added here.
*/
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3396142911
> Getting close! I won't be able to test till Tuesday...
Please do test!
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2425441971
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerParser.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Map;
+import javax.xml.parsers.SAXParser;
+import javax.xml.parsers.SAXParserFactory;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.Utils;
+import org.xml.sax.ContentHandler;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.helpers.DefaultHandler;
+
+public class TikaServerParser {
+ private final SAXParser saxParser;
+
+ public TikaServerParser() {
+ SAXParserFactory factory = SAXParserFactory.newInstance();
+ factory.setNamespaceAware(true);
+ try {
+ factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+ factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+ factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+ } catch (Throwable ignore) {
+ // Some parsers may not support all features; ignore
+ }
+ try {
+ saxParser = factory.newSAXParser();
+ } catch (Exception e) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+ }
+ }
+
+ /**
+ * Parses response in XML format from Tika Server /tika endpoint. The result is that the metadata
+ * object is populated and the content handler is called with extracted text.
+ */
+ public void parseXml(InputStream inputStream, ContentHandler handler, ExtractionMetadata metadata)
+ throws IOException, SAXException {
+ DefaultHandler xmlHandler = new TikaXmlResponseSaxContentHandler(handler, metadata);
+ try (Reader reader =
+ new XmlSanitizingReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8))) {
+ saxParser.parse(new InputSource(reader), xmlHandler);
+ }
+ }
+
+ /**
+ * Parses response in JSON format from Tika Server /rmeta endpoint. The result is that the
+ * metadata object is populated, and the content handler is called with extracted text.
+ *
+ * @param jsonStream - JSON stream to parse
+ * @param handler - SAX content handler to call with extracted text
+ * @param md - metadata object to populate
+ */
+ @SuppressWarnings({"rawtypes", "PatternVariableCanBeUsed"})
+ void parseRmetaJson(InputStream jsonStream, DefaultHandler handler, ExtractionMetadata md)
+ throws IOException, SAXException {
+ Object parsed = Utils.fromJSON(jsonStream);
Review Comment:
In case user uses `tikaserver.recursive` option, this code path will be triggered, as the stream is a huge JSON object. Do we want to provide som OOM prevention guardrails here? What if a document misbehaves and produces several Gb of content? Then we'd risk OOM in Solr process. We also grab the content part of the JSON into String `xhtml` further down, doubling heap usage.
I imagine some way of aborting the parsing after at a configurable number of bytes when reading stream from Tika. But that would probably have to be implemented in `Utils.fromJSON`? Thoughts?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3314351420
Status:
* Parses docs using TikaServer
* Can switch between `xml` (html) and `text` format of the content field
* Randomized the choice of backend for the main test class
* ExtractOnly not fully implemented for tikaserver, some tests fail
TBD:
* The whole xpath / SAX parsing of XML response is missing
* We use JDK HTTP client, could perhaps use Jetty client. See other POC for example, including making timeouts configurable
* Must make sure that `tikaserver.url` is only configurable on requesthandler config in solrconfig, not as a request parameter (security)
* RefGuide docs, especially how to start TikaServer etc
* Remove the DummyExtractionBackend
Anyone, please feel free to hack away on this if it looks exciting, committing directly to the PR branch.
Question: Would it bring value to isolate the refactoring in one PR and then another one to add the tikaserver impl?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2426756867
##########
.github/workflows/gradle-extraction-check.yml:
##########
@@ -0,0 +1,28 @@
+# This test covers TikaServer tests for SolrCell, since crave does not support docker yet.
+name: Extraction module tests with Docker
+
+on:
+ pull_request:
+ branches:
+ - '*'
+ paths:
+ - 'solr/modules/extraction/**'
Review Comment:
I kept this github action test for extractino module since Crave does not support docker TestContainers tests (yet). But limiting it to run only for changes in the module.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2426645729
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerParser.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Map;
+import javax.xml.parsers.SAXParser;
+import javax.xml.parsers.SAXParserFactory;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.Utils;
+import org.xml.sax.ContentHandler;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.helpers.DefaultHandler;
+
+public class TikaServerParser {
+ private final SAXParser saxParser;
+
+ public TikaServerParser() {
+ SAXParserFactory factory = SAXParserFactory.newInstance();
+ factory.setNamespaceAware(true);
+ try {
+ factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+ factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+ factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+ } catch (Throwable ignore) {
+ // Some parsers may not support all features; ignore
+ }
+ try {
+ saxParser = factory.newSAXParser();
+ } catch (Exception e) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+ }
+ }
+
+ /**
+ * Parses response in XML format from Tika Server /tika endpoint. The result is that the metadata
+ * object is populated and the content handler is called with extracted text.
+ */
+ public void parseXml(InputStream inputStream, ContentHandler handler, ExtractionMetadata metadata)
+ throws IOException, SAXException {
+ DefaultHandler xmlHandler = new TikaXmlResponseSaxContentHandler(handler, metadata);
+ try (Reader reader =
+ new XmlSanitizingReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8))) {
+ saxParser.parse(new InputSource(reader), xmlHandler);
+ }
+ }
+
+ /**
+ * Parses response in JSON format from Tika Server /rmeta endpoint. The result is that the
+ * metadata object is populated, and the content handler is called with extracted text.
+ *
+ * @param jsonStream - JSON stream to parse
+ * @param handler - SAX content handler to call with extracted text
+ * @param md - metadata object to populate
+ */
+ @SuppressWarnings({"rawtypes", "PatternVariableCanBeUsed"})
+ void parseRmetaJson(InputStream jsonStream, DefaultHandler handler, ExtractionMetadata md)
+ throws IOException, SAXException {
+ Object parsed = Utils.fromJSON(jsonStream);
+ if (!(parsed instanceof List)) {
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR, "Unexpected /rmeta response, expected JSON array");
+ }
+ List list = (List) parsed;
+ for (Object o : list) {
+ if (!(o instanceof Map)) continue;
+ Map map = (Map) o;
+ // Copy metadata
+ for (Object k : map.keySet()) {
+ String key = String.valueOf(k);
+ Object val = map.get(k);
+ if ("X-TIKA:content".equalsIgnoreCase(key)) {
+ // handled below
+ continue;
+ }
+ if (val instanceof List) {
+ for (Object v : (List) val) {
+ if (v != null) md.add(key, String.valueOf(v));
+ }
+ } else if (val != null) {
+ md.add(key, String.valueOf(val));
+ }
+ }
+ Object content = map.get("X-TIKA:content");
+ if (content != null) {
+ String xhtml = String.valueOf(content);
+ if (!xhtml.isEmpty() && handler != null) {
+ InputStream inputStream =
+ new ByteArrayInputStream(xhtml.getBytes(StandardCharsets.UTF_8));
+ try (Reader reader =
+ new XmlSanitizingReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8))) {
+ saxParser.parse(new InputSource(reader), handler);
+ }
+ }
+ }
+ }
+ }
+
+ /** Custom SAX handler that will extract meta tags from the tika xml and delegate */
+ static class TikaXmlResponseSaxContentHandler extends DefaultHandler {
+ private final ContentHandler delegate;
+ private final ExtractionMetadata metadata;
+ private boolean inHead = false;
+
+ public TikaXmlResponseSaxContentHandler(ContentHandler delegate, ExtractionMetadata metadata) {
+ this.delegate = delegate;
+ this.metadata = metadata;
+ }
+
+ @Override
+ public void startDocument() throws SAXException {
+ if (delegate != null) delegate.startDocument();
+ }
+
+ @Override
+ public void endDocument() throws SAXException {
+ if (delegate != null) delegate.endDocument();
+ }
+
+ @Override
+ public void startElement(
+ String uri, String localName, String qName, org.xml.sax.Attributes attributes)
+ throws SAXException {
+ String ln = localName != null && !localName.isEmpty() ? localName : qName;
+ if ("head".equalsIgnoreCase(ln)) {
+ inHead = true;
+ } else if (inHead && "meta".equalsIgnoreCase(ln) && attributes != null) {
+ String name = attributes.getValue("name");
+ String content = attributes.getValue("content");
+ if (name != null && content != null) {
+ metadata.add(name, content);
+ }
+ }
+ if (delegate != null) delegate.startElement(uri, localName, qName, attributes);
+ }
+
+ @Override
+ public void endElement(String uri, String localName, String qName) throws SAXException {
+ String ln = localName != null && !localName.isEmpty() ? localName : qName;
+ if ("head".equalsIgnoreCase(ln)) {
+ inHead = false;
+ }
+ if (delegate != null) delegate.endElement(uri, localName, qName);
+ }
+
+ @Override
+ public void characters(char[] ch, int start, int length) throws SAXException {
+ if (delegate != null) delegate.characters(ch, start, length);
Review Comment:
Did it using a stream wrapper instead.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2412923002
##########
solr/solrj/src/java/org/apache/solr/common/SolrException.java:
##########
@@ -45,6 +45,7 @@ public enum ErrorCode {
TOO_MANY_REQUESTS(429),
SERVER_ERROR(500),
SERVICE_UNAVAILABLE(503),
Review Comment:
Add JavaDoc comment for the new GATEWAY_TIMEOUT error code to explain when it's used (e.g., timeouts when communicating with external services like TikaServer).
```suggestion
SERVICE_UNAVAILABLE(503),
/**
* Indicates that Solr, acting as a gateway or proxy, did not receive a timely response from an upstream server.
* Typically used for timeouts when communicating with external services such as TikaServer.
*/
```
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractingParams.java:
##########
@@ -136,4 +136,19 @@ public interface ExtractingParams {
* .*=<defaultmypassword> at the end
*/
public static final String PASSWORD_MAP_FILE = "passwordsFile";
+
+ /** Backend selection parameter and */
Review Comment:
Incomplete JavaDoc comment - missing description after 'and'.
```suggestion
/**
* Backend selection parameter. Specifies which extraction backend to use for processing documents,
* such as Tika or other supported backends.
*/
```
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractingDocumentLoader.java:
##########
@@ -75,40 +55,34 @@ public class ExtractingDocumentLoader extends ContentStreamLoader {
/** Extract Only supported format. Default */
public static final String XML_FORMAT = "xml";
- /** XHTML XPath parser. */
- private static final XPathParser PARSER = new XPathParser("xhtml", XHTMLContentHandler.XHTML);
-
final SolrCore core;
final SolrParams params;
final UpdateRequestProcessor processor;
final boolean ignoreTikaException;
- protected AutoDetectParser autoDetectParser;
+ final boolean backCompat;
private final AddUpdateCommand templateAdd;
- protected TikaConfig config;
- protected ParseContextConfig parseContextConfig;
protected SolrContentHandlerFactory factory;
+ protected ExtractionBackend backend;
public ExtractingDocumentLoader(
SolrQueryRequest req,
UpdateRequestProcessor processor,
- TikaConfig config,
- ParseContextConfig parseContextConfig,
- SolrContentHandlerFactory factory) {
+ SolrContentHandlerFactory factory,
+ ExtractionBackend backend) {
this.params = req.getParams();
this.core = req.getCore();
- this.config = config;
- this.parseContextConfig = parseContextConfig;
this.processor = processor;
+ this.backCompat = params.getBool(ExtractingParams.BACK_COMPATIBILITY, true);
templateAdd = new AddUpdateCommand(req);
templateAdd.overwrite = params.getBool(UpdateParams.OVERWRITE, true);
templateAdd.commitWithin = params.getInt(UpdateParams.COMMIT_WITHIN, -1);
+ templateAdd.overwrite = params.getBool(UpdateParams.OVERWRITE, true);
Review Comment:
Duplicate assignment of `templateAdd.overwrite` - this field is set twice with the same value on lines 80 and 82.
```suggestion
```
##########
solr/solr-ref-guide/modules/indexing-guide/pages/indexing-with-tika.adoc:
##########
@@ -391,6 +463,33 @@ This is only required if you have customized your Tika implementation.
+
Example: `tika.config=/path/to/tika.config`
+`tikaserver.timeoutSeconds`::
++
+[%autowidth,frame=none]
+|===
+|Optional |Default: `180` seconds
+|===
++
+Sets the HTTP timeout when communicating with Tika Server, in seconds. Can be set per request as a parameter or as a default in the request handler configuration. If set on the request it overrides the handler default for that call only.
++
+Examples:
++
+- Per request: `tikaserver.timeoutSeconds=60`
+- In `solrconfig.xml`: `<int name="tikaserver.timeoutSeconds">60</int>`
+
+
+`tikaserver.url`::
++
+[%autowidth,frame=none]
+|===
+|Optional |Default: none
+|===
++
+Specifies the URL of the Tika server to use when the `extraction.backend` parameter is set to `tikaserver`.
+This parameter is required when using the `tikaserver` backend. Defaults to `http://localhost:9998` if not specified. This parameter can only be configured in the configuration, not per request.
Review Comment:
Contradictory statements: first says the parameter is 'required', then says it 'Defaults to `http://localhost:9998` if not specified'. Clarify whether it's required or has a default.
```suggestion
This parameter is optional when using the `tikaserver` backend. If not specified, it defaults to `http://localhost:9998`. This parameter can only be configured in the configuration, not per request.
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2373729032
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/DummyExtractionBackend.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.InputStream;
+
+/** Dummy backend that emits predictable test data without actually parsing input content. */
Review Comment:
Sounds good.. If we practice and find it futzy to get tika set up, then we can always add it back ;-).
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2373267857
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractionBackendFactory.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.solr.core.SolrCore;
+
+/**
+ * Factory for ExtractionBackend instances. Lazily constructs backends by short name (e.g., "local",
+ * "dummy") and caches them for reuse.
+ */
+public class ExtractionBackendFactory {
+ private final SolrCore core;
+ private final String tikaConfigLoc;
+ private final ParseContextConfig parseContextConfig;
+ private final String tikaServerUrl;
+ private final Map<String, ExtractionBackend> cache = new ConcurrentHashMap<>();
+
+ public ExtractionBackendFactory(
+ SolrCore core,
+ String tikaConfigLoc,
+ ParseContextConfig parseContextConfig,
+ String tikaServerUrl) {
+ this.core = core;
+ this.tikaConfigLoc = tikaConfigLoc;
+ this.parseContextConfig = parseContextConfig;
+ this.tikaServerUrl = tikaServerUrl;
+ }
+
+ /** Returns a backend instance for the given name, creating it if necessary. */
+ public ExtractionBackend getBackend(String name) {
+ String key = normalize(name);
+ return cache.computeIfAbsent(
Review Comment:
cool!
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3325360384
@epugh So I pushed a baby step more code, providing
* Sub classed the test, so all the `assertQ` tests run both for `local` and `tikaserver` backends
* Made some more tikaserver tests pass
<img width="348" height="459" alt="Skjermbilde 2025-09-23 kl 19 29 10" src="https://github.com/user-attachments/assets/2f71100b-b1a4-4807-acbc-422aad69b11d" />
Failing tests typically fail due to lack of support for the SAX style capture feature of Tika API. Others fail due to difference in metadata between Tika1 and TikaServer3, e.g. where Tika1 would output metadata like `title` and `author`, this is now normalized and would be `dc:title` and `dc:author`. We can either provide a (feature-toggled?) mapper in TikaServer that tries to fill in those old metadata keys, and make tests pass that way. Or we could modify tests to look for the "modern" normalized equivalent, which is output by both Tika1 and TikaServer. I think perhaps we can start with the latter.
Unfortunately it looks like Crave cannot run TestContainers tests (no docker available?)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3336287600
I love seeing the updates as you make progress. Commits are fun to read too! I am really impressed that we are actually able to use the existing tests to measure progress, it's a reminder on the value of the tests in helping us understand "what features of Tika do we use? The ones in the tests!".
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3336173646
> Are you seeing any issues in how TikaServer works that maybe are better fixed there? Some great progress!
I think not really. Only "quirk" I saw was that if you ingest a plain txt document, you get back an XML with a tile like `<title>�</title>`. It is parsed by `TextAndCSVParser`. When injecting that XML into SAX parser, it bails out on invalid character, so I inserted an XML sanitizer.
Other than that, I think TIka Server has what we need. It accepts password as HTTP header. And it accepts some PDF parser config also through headers. But more advanced parser config shuold be done on the TikaServer side, and good thing is that the user will have 100% control over their TikaServer and can configure it as they wish, much more than you could with SolrrCell.
We should probably start using `/rmeta` endpoint (recursive meta) parser, since our current SolrCell parser is recursive. But that may mean some more advanced XML parsing? Have not really checked.
I'm fairly optimistic on getting the remaining tests passing.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2422089844
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/XmlSanitizingReader.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.IOException;
+import java.io.Reader;
+
+/**
+ * Minimal reader that drops only null numeric character entities from the stream.
+ *
+ * <p>Recognizes decimal and hexadecimal numeric entities that resolve to code point 0 (e.g. "�",
+ * "�", "�", "�") and removes them. If a null entity is unterminated (no ';'), it is
+ * still removed when a non-entity character terminates the sequence. Everything else is passed
+ * through unchanged.
+ */
+final class XmlSanitizingReader extends Reader {
Review Comment:
Ok, replaced the 180 line class with a 118 line one which is much impler and easier to maintain, yet still efficient.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2424992757
##########
solr/solrj/src/java/org/apache/solr/common/SolrException.java:
##########
@@ -45,6 +45,7 @@ public enum ErrorCode {
TOO_MANY_REQUESTS(429),
SERVER_ERROR(500),
SERVICE_UNAVAILABLE(503),
Review Comment:
The new GATEWAY_TIMEOUT error code should be documented with a brief comment explaining when it's used, similar to other error codes in this enum.
```suggestion
SERVICE_UNAVAILABLE(503),
/** Used when a gateway or proxy times out waiting for a response from an upstream server (HTTP 504). */
```
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java:
##########
@@ -0,0 +1,336 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.InputStream;
+import java.net.ConnectException;
+import java.net.SocketTimeoutException;
+import java.nio.channels.ClosedChannelException;
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.tika.sax.BodyContentHandler;
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.InputStreamRequestContent;
+import org.eclipse.jetty.client.InputStreamResponseListener;
+import org.eclipse.jetty.client.Request;
+import org.eclipse.jetty.client.Response;
+import org.eclipse.jetty.io.EofException;
+import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler;
+import org.xml.sax.helpers.DefaultHandler;
+
+/** Extraction backend using the Tika Server. It uses a shared Jetty HttpClient. */
+public class TikaServerExtractionBackend implements ExtractionBackend {
+ private static volatile HttpClient SHARED_CLIENT;
+ private static volatile ExecutorService SHARED_EXECUTOR;
+ private static final Object INIT_LOCK = new Object();
+ private static volatile boolean INITIALIZED = false;
+ private static volatile boolean SHUTDOWN = false;
+ private final String baseUrl;
+ private static final int DEFAULT_TIMEOUT_SECONDS = 3 * 60;
+ private final Duration defaultTimeout;
+ private final TikaServerParser tikaServerResponseParser = new TikaServerParser();
+ private boolean tikaMetadataCompatibility;
+ private HashMap<String, Object> initArgsMap = new HashMap<>();
+
+ public TikaServerExtractionBackend(String baseUrl) {
+ this(baseUrl, DEFAULT_TIMEOUT_SECONDS, null);
+ }
+
+ public TikaServerExtractionBackend(String baseUrl, int timeoutSeconds, NamedList<?> initArgs) {
+ if (initArgs != null) {
+ initArgs.toMap(this.initArgsMap);
+ }
+ Object metaCompatObh = this.initArgsMap.get(ExtractingParams.TIKASERVER_METADATA_COMPATIBILITY);
+ if (metaCompatObh != null) {
+ this.tikaMetadataCompatibility = Boolean.parseBoolean(metaCompatObh.toString());
+ }
+ if (timeoutSeconds <= 0) {
+ timeoutSeconds = DEFAULT_TIMEOUT_SECONDS;
+ }
+ if (baseUrl.endsWith("/")) {
+ this.baseUrl = baseUrl.substring(0, baseUrl.length() - 1);
+ } else {
+ this.baseUrl = baseUrl;
+ }
+ this.defaultTimeout =
+ Duration.ofSeconds(timeoutSeconds > 0 ? timeoutSeconds : DEFAULT_TIMEOUT_SECONDS);
+ }
+
+ public static final String NAME = "tikaserver";
+
+ @Override
+ public String name() {
+ return NAME;
+ }
+
+ @Override
+ public ExtractionResult extract(InputStream inputStream, ExtractionRequest request)
+ throws Exception {
+ try (InputStream tikaResponse = callTikaServer(inputStream, request)) {
+ ExtractionMetadata md = buildMetadataFromRequest(request);
+ BodyContentHandler bodyContentHandler = new BodyContentHandler(-1);
+ if (request.tikaServerRecursive) {
+ tikaServerResponseParser.parseRmetaJson(tikaResponse, bodyContentHandler, md);
+ } else {
+ tikaServerResponseParser.parseXml(tikaResponse, bodyContentHandler, md);
+ }
+ if (tikaMetadataCompatibility) {
+ appendBackCompatTikaMetadata(md);
+ }
+ return new ExtractionResult(bodyContentHandler.toString(), md);
+ }
+ }
+
+ @Override
+ public void extractWithSaxHandler(
+ InputStream inputStream,
+ ExtractionRequest request,
+ ExtractionMetadata md,
+ DefaultHandler saxContentHandler)
+ throws Exception {
+ try (InputStream tikaResponse = callTikaServer(inputStream, request)) {
+ if (request.tikaServerRecursive) {
+ tikaServerResponseParser.parseRmetaJson(tikaResponse, saxContentHandler, md);
+ } else {
+ tikaServerResponseParser.parseXml(tikaResponse, saxContentHandler, md);
+ }
+ if (tikaMetadataCompatibility) {
+ appendBackCompatTikaMetadata(md);
+ }
+ }
+ }
+
+ /**
+ * Call the Tika Server to extract text and metadata. Depending on <code>request.recursive</code>,
+ * will either return XML (false) or JSON array (true). <b>The recursive mode consumes more memory
+ * both on the TikaServer side and on the Solr side</b>
+ *
+ * @return InputStream of the response body, either XML or JSON depending on <code>
+ * request.tikaserverRecursive</code>
+ */
+ InputStream callTikaServer(InputStream inputStream, ExtractionRequest request) throws Exception {
+ String url = baseUrl + (request.tikaServerRecursive ? "/rmeta" : "/tika");
+
+ ensureClientInitialized();
+ HttpClient client = SHARED_CLIENT;
+
+ Request req = client.newRequest(url).method("PUT");
+ Duration effectiveTimeout =
+ (request.tikaServerTimeoutSeconds != null && request.tikaServerTimeoutSeconds > 0)
+ ? Duration.ofSeconds(request.tikaServerTimeoutSeconds)
+ : defaultTimeout;
+ req.timeout(effectiveTimeout.toMillis(), TimeUnit.MILLISECONDS);
+
+ // Headers
+ String accept = (request.tikaServerRecursive ? "application/json" : "text/xml");
+ req.headers(h -> h.add("Accept", accept));
+ String contentType = (request.streamType != null) ? request.streamType : request.contentType;
+ if (contentType != null) {
+ req.headers(h -> h.add("Content-Type", contentType));
+ }
+ if (!request.tikaServerRequestHeaders.isEmpty()) {
+ req.headers(
+ h ->
+ request.tikaServerRequestHeaders.forEach(
+ (k, v) -> {
+ if (k != null && v != null) h.add(k, v);
+ }));
+ }
+
+ ExtractionMetadata md = buildMetadataFromRequest(request);
+ if (request.resourcePassword != null || request.passwordsMap != null) {
+ RegexRulesPasswordProvider passwordProvider = new RegexRulesPasswordProvider();
+ if (request.resourcePassword != null) {
+ passwordProvider.setExplicitPassword(request.resourcePassword);
+ }
+ if (request.passwordsMap != null) {
+ passwordProvider.setPasswordMap(request.passwordsMap);
+ }
+ String pwd = passwordProvider.getPassword(md);
+ if (pwd != null) {
+ req.headers(h -> h.add("Password", pwd)); // Tika Server expects this header if provided
+ }
+ }
+ if (request.resourceName != null) {
+ req.headers(
+ h ->
+ h.add(
+ "Content-Disposition", "attachment; filename=\"" + request.resourceName + "\""));
+ }
+
+ if (contentType != null) {
+ req.body(new InputStreamRequestContent(contentType, inputStream));
+ } else {
+ req.body(new InputStreamRequestContent(inputStream));
+ }
+
+ InputStreamResponseListener listener = new InputStreamResponseListener();
+ req.send(listener);
+
+ final Response response;
+ try {
+ response = listener.get(effectiveTimeout.toMillis(), TimeUnit.MILLISECONDS);
+ } catch (TimeoutException te) {
+ throw new SolrException(
+ SolrException.ErrorCode.GATEWAY_TIMEOUT,
+ "Timeout after "
+ + effectiveTimeout.toMillis()
+ + " ms while waiting for response from TikaServer "
+ + url,
+ te);
+ } catch (InterruptedException ie) {
+ Thread.currentThread().interrupt();
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Interrupted while waiting for response from TikaServer " + url,
+ ie);
+ } catch (ExecutionException ee) {
+ Throwable cause = ee.getCause();
+ if (cause instanceof ConnectException
+ || cause instanceof SocketTimeoutException
+ || cause instanceof EofException
+ || cause instanceof ClosedChannelException) {
+ throw new SolrException(
+ SolrException.ErrorCode.SERVICE_UNAVAILABLE,
+ "Error communicating with TikaServer "
+ + url
+ + ": "
+ + cause.getClass().getSimpleName()
+ + ": "
+ + cause.getMessage(),
+ cause);
+ }
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Unexpected error while calling TikaServer " + url,
+ ee);
+ }
+
+ int code = response.getStatus();
+ if (code < 200 || code >= 300) {
+ SolrException.ErrorCode errorCode = SolrException.ErrorCode.getErrorCode(code);
+ String reason = response.getReason();
+ String msg =
+ "TikaServer "
+ + url
+ + " returned status "
+ + code
+ + (reason != null ? " (" + reason + ")" : "");
+ throw new SolrException(errorCode, msg);
+ }
+
+ return listener.getInputStream();
+ }
+
+ private static void ensureClientInitialized() {
+ if (INITIALIZED) return;
+ synchronized (INIT_LOCK) {
+ if (INITIALIZED) return;
+ ThreadFactory tf = new SolrNamedThreadFactory("TikaServerHttpClient");
+ ExecutorService exec = ExecutorUtil.newMDCAwareCachedThreadPool(tf);
+ HttpClient client = new HttpClient();
+ client.setExecutor(exec);
+ client.setScheduler(new ScheduledExecutorScheduler("TikaServerHttpClient-scheduler", true));
+ try {
+ client.start();
+ } catch (Exception e) {
+ try {
+ exec.shutdownNow();
+ } catch (Throwable ignore) {
+ }
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR, "Failed to start shared Jetty HttpClient", e);
+ }
+ SHARED_EXECUTOR = exec;
+ SHARED_CLIENT = client;
+ INITIALIZED = true;
+ SHUTDOWN = false;
+ }
+ }
+
+ private final Map<String, String> fieldMappings = new LinkedHashMap<>();
+
+ // TODO: Improve backward compatibility by adding more mappings
+ {
+ fieldMappings.put("dc:title", "title");
+ fieldMappings.put("dc:creator", "author");
+ fieldMappings.put("dc:description", "description");
+ fieldMappings.put("dc:subject", "subject");
+ fieldMappings.put("dc:language", "language");
+ fieldMappings.put("dc:publisher", "publisher");
+ fieldMappings.put("dcterms:created", "created");
+ fieldMappings.put("dcterms:modified", "modified");
+ fieldMappings.put("meta:author", "Author");
+ fieldMappings.put("meta:creation-date", "Creation-Date");
+ fieldMappings.put("meta:save-date", "Last-Save-Date");
+ fieldMappings.put("meta:keyword", "Keywords");
+ fieldMappings.put("pdf:docinfo:keywords", "Keywords");
+ }
Review Comment:
[nitpick] The field mappings are hardcoded in an instance initializer block. Consider moving this to a static final Map or making it configurable to improve maintainability and allow customization.
##########
solr/modules/extraction/src/test-files/extraction/simple.html:
##########
@@ -10,7 +10,7 @@
Here is some text
</p>
<p>distinct<br/>words</p>
-<div>Here is some text in a div</div>
+<h1>Here is some text in a h1</h1>
Review Comment:
Corrected article usage from 'a h1' to 'an h1' since 'h1' starts with a vowel sound.
```suggestion
<h1>Here is some text in an h1</h1>
```
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/XmlSanitizingReader.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.FilterReader;
+import java.io.IOException;
+import java.io.Reader;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Filters out null character entities (�, �, etc.) from XML content.
+ *
+ * <p>Removes numeric character entities that resolve to code point 0, such as <code>�</code> or
+ * <code>�</code>. Everything else is passed through unchanged.
+ */
+final class XmlSanitizingReader extends FilterReader {
+ private static final Pattern NULL_ENTITY_PATTERN =
+ Pattern.compile("&#(0+|x0+);", Pattern.CASE_INSENSITIVE);
Review Comment:
The regex pattern may not handle all null entity variations correctly. The pattern `&#(0+|x0+);` would match `�+;` but not `�` or `�`. Consider using `&#(0+|x0*);` to properly match zero or more zeros after 'x'.
```suggestion
Pattern.compile("&#(0+|x0*);", Pattern.CASE_INSENSITIVE);
```
##########
solr/modules/extraction/src/test-files/extraction/example.html:
##########
@@ -6,8 +6,8 @@
<p>
Here is some text
</p>
-<div>Here is some text in a div</div>
-<div>This has a <a href="http://www.apache.org">link</a>.</div>
+<h1>a h1 tag</h1>
Review Comment:
Corrected article usage from 'a h1' to 'an h1' since 'h1' starts with a vowel sound.
```suggestion
<h1>an h1 tag</h1>
```
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java:
##########
@@ -0,0 +1,336 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.InputStream;
+import java.net.ConnectException;
+import java.net.SocketTimeoutException;
+import java.nio.channels.ClosedChannelException;
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.tika.sax.BodyContentHandler;
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.InputStreamRequestContent;
+import org.eclipse.jetty.client.InputStreamResponseListener;
+import org.eclipse.jetty.client.Request;
+import org.eclipse.jetty.client.Response;
+import org.eclipse.jetty.io.EofException;
+import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler;
+import org.xml.sax.helpers.DefaultHandler;
+
+/** Extraction backend using the Tika Server. It uses a shared Jetty HttpClient. */
+public class TikaServerExtractionBackend implements ExtractionBackend {
+ private static volatile HttpClient SHARED_CLIENT;
+ private static volatile ExecutorService SHARED_EXECUTOR;
+ private static final Object INIT_LOCK = new Object();
+ private static volatile boolean INITIALIZED = false;
+ private static volatile boolean SHUTDOWN = false;
+ private final String baseUrl;
+ private static final int DEFAULT_TIMEOUT_SECONDS = 3 * 60;
+ private final Duration defaultTimeout;
+ private final TikaServerParser tikaServerResponseParser = new TikaServerParser();
+ private boolean tikaMetadataCompatibility;
+ private HashMap<String, Object> initArgsMap = new HashMap<>();
+
+ public TikaServerExtractionBackend(String baseUrl) {
+ this(baseUrl, DEFAULT_TIMEOUT_SECONDS, null);
+ }
+
+ public TikaServerExtractionBackend(String baseUrl, int timeoutSeconds, NamedList<?> initArgs) {
+ if (initArgs != null) {
+ initArgs.toMap(this.initArgsMap);
+ }
+ Object metaCompatObh = this.initArgsMap.get(ExtractingParams.TIKASERVER_METADATA_COMPATIBILITY);
+ if (metaCompatObh != null) {
+ this.tikaMetadataCompatibility = Boolean.parseBoolean(metaCompatObh.toString());
+ }
+ if (timeoutSeconds <= 0) {
+ timeoutSeconds = DEFAULT_TIMEOUT_SECONDS;
+ }
+ if (baseUrl.endsWith("/")) {
+ this.baseUrl = baseUrl.substring(0, baseUrl.length() - 1);
+ } else {
+ this.baseUrl = baseUrl;
+ }
+ this.defaultTimeout =
+ Duration.ofSeconds(timeoutSeconds > 0 ? timeoutSeconds : DEFAULT_TIMEOUT_SECONDS);
+ }
+
+ public static final String NAME = "tikaserver";
+
+ @Override
+ public String name() {
+ return NAME;
+ }
+
+ @Override
+ public ExtractionResult extract(InputStream inputStream, ExtractionRequest request)
+ throws Exception {
+ try (InputStream tikaResponse = callTikaServer(inputStream, request)) {
+ ExtractionMetadata md = buildMetadataFromRequest(request);
+ BodyContentHandler bodyContentHandler = new BodyContentHandler(-1);
+ if (request.tikaServerRecursive) {
+ tikaServerResponseParser.parseRmetaJson(tikaResponse, bodyContentHandler, md);
+ } else {
+ tikaServerResponseParser.parseXml(tikaResponse, bodyContentHandler, md);
+ }
+ if (tikaMetadataCompatibility) {
+ appendBackCompatTikaMetadata(md);
+ }
+ return new ExtractionResult(bodyContentHandler.toString(), md);
+ }
+ }
+
+ @Override
+ public void extractWithSaxHandler(
+ InputStream inputStream,
+ ExtractionRequest request,
+ ExtractionMetadata md,
+ DefaultHandler saxContentHandler)
+ throws Exception {
+ try (InputStream tikaResponse = callTikaServer(inputStream, request)) {
+ if (request.tikaServerRecursive) {
+ tikaServerResponseParser.parseRmetaJson(tikaResponse, saxContentHandler, md);
+ } else {
+ tikaServerResponseParser.parseXml(tikaResponse, saxContentHandler, md);
+ }
+ if (tikaMetadataCompatibility) {
+ appendBackCompatTikaMetadata(md);
+ }
+ }
+ }
+
+ /**
+ * Call the Tika Server to extract text and metadata. Depending on <code>request.recursive</code>,
+ * will either return XML (false) or JSON array (true). <b>The recursive mode consumes more memory
+ * both on the TikaServer side and on the Solr side</b>
+ *
+ * @return InputStream of the response body, either XML or JSON depending on <code>
+ * request.tikaserverRecursive</code>
+ */
+ InputStream callTikaServer(InputStream inputStream, ExtractionRequest request) throws Exception {
+ String url = baseUrl + (request.tikaServerRecursive ? "/rmeta" : "/tika");
+
+ ensureClientInitialized();
+ HttpClient client = SHARED_CLIENT;
+
+ Request req = client.newRequest(url).method("PUT");
+ Duration effectiveTimeout =
+ (request.tikaServerTimeoutSeconds != null && request.tikaServerTimeoutSeconds > 0)
+ ? Duration.ofSeconds(request.tikaServerTimeoutSeconds)
+ : defaultTimeout;
+ req.timeout(effectiveTimeout.toMillis(), TimeUnit.MILLISECONDS);
+
+ // Headers
+ String accept = (request.tikaServerRecursive ? "application/json" : "text/xml");
+ req.headers(h -> h.add("Accept", accept));
+ String contentType = (request.streamType != null) ? request.streamType : request.contentType;
+ if (contentType != null) {
+ req.headers(h -> h.add("Content-Type", contentType));
+ }
+ if (!request.tikaServerRequestHeaders.isEmpty()) {
+ req.headers(
+ h ->
+ request.tikaServerRequestHeaders.forEach(
+ (k, v) -> {
+ if (k != null && v != null) h.add(k, v);
+ }));
+ }
+
+ ExtractionMetadata md = buildMetadataFromRequest(request);
+ if (request.resourcePassword != null || request.passwordsMap != null) {
+ RegexRulesPasswordProvider passwordProvider = new RegexRulesPasswordProvider();
+ if (request.resourcePassword != null) {
+ passwordProvider.setExplicitPassword(request.resourcePassword);
+ }
+ if (request.passwordsMap != null) {
+ passwordProvider.setPasswordMap(request.passwordsMap);
+ }
+ String pwd = passwordProvider.getPassword(md);
+ if (pwd != null) {
+ req.headers(h -> h.add("Password", pwd)); // Tika Server expects this header if provided
+ }
+ }
+ if (request.resourceName != null) {
+ req.headers(
+ h ->
+ h.add(
+ "Content-Disposition", "attachment; filename=\"" + request.resourceName + "\""));
+ }
+
+ if (contentType != null) {
+ req.body(new InputStreamRequestContent(contentType, inputStream));
+ } else {
+ req.body(new InputStreamRequestContent(inputStream));
+ }
+
+ InputStreamResponseListener listener = new InputStreamResponseListener();
+ req.send(listener);
+
+ final Response response;
+ try {
+ response = listener.get(effectiveTimeout.toMillis(), TimeUnit.MILLISECONDS);
+ } catch (TimeoutException te) {
+ throw new SolrException(
+ SolrException.ErrorCode.GATEWAY_TIMEOUT,
+ "Timeout after "
+ + effectiveTimeout.toMillis()
+ + " ms while waiting for response from TikaServer "
+ + url,
+ te);
+ } catch (InterruptedException ie) {
+ Thread.currentThread().interrupt();
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Interrupted while waiting for response from TikaServer " + url,
+ ie);
+ } catch (ExecutionException ee) {
+ Throwable cause = ee.getCause();
+ if (cause instanceof ConnectException
+ || cause instanceof SocketTimeoutException
+ || cause instanceof EofException
+ || cause instanceof ClosedChannelException) {
+ throw new SolrException(
+ SolrException.ErrorCode.SERVICE_UNAVAILABLE,
+ "Error communicating with TikaServer "
+ + url
+ + ": "
+ + cause.getClass().getSimpleName()
+ + ": "
+ + cause.getMessage(),
+ cause);
+ }
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Unexpected error while calling TikaServer " + url,
+ ee);
+ }
+
+ int code = response.getStatus();
+ if (code < 200 || code >= 300) {
+ SolrException.ErrorCode errorCode = SolrException.ErrorCode.getErrorCode(code);
+ String reason = response.getReason();
+ String msg =
+ "TikaServer "
+ + url
+ + " returned status "
+ + code
+ + (reason != null ? " (" + reason + ")" : "");
+ throw new SolrException(errorCode, msg);
+ }
+
+ return listener.getInputStream();
+ }
+
+ private static void ensureClientInitialized() {
+ if (INITIALIZED) return;
+ synchronized (INIT_LOCK) {
+ if (INITIALIZED) return;
+ ThreadFactory tf = new SolrNamedThreadFactory("TikaServerHttpClient");
+ ExecutorService exec = ExecutorUtil.newMDCAwareCachedThreadPool(tf);
+ HttpClient client = new HttpClient();
+ client.setExecutor(exec);
+ client.setScheduler(new ScheduledExecutorScheduler("TikaServerHttpClient-scheduler", true));
+ try {
+ client.start();
+ } catch (Exception e) {
+ try {
+ exec.shutdownNow();
+ } catch (Throwable ignore) {
+ }
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR, "Failed to start shared Jetty HttpClient", e);
+ }
+ SHARED_EXECUTOR = exec;
+ SHARED_CLIENT = client;
+ INITIALIZED = true;
+ SHUTDOWN = false;
+ }
+ }
+
+ private final Map<String, String> fieldMappings = new LinkedHashMap<>();
+
+ // TODO: Improve backward compatibility by adding more mappings
+ {
+ fieldMappings.put("dc:title", "title");
+ fieldMappings.put("dc:creator", "author");
+ fieldMappings.put("dc:description", "description");
+ fieldMappings.put("dc:subject", "subject");
+ fieldMappings.put("dc:language", "language");
+ fieldMappings.put("dc:publisher", "publisher");
+ fieldMappings.put("dcterms:created", "created");
+ fieldMappings.put("dcterms:modified", "modified");
+ fieldMappings.put("meta:author", "Author");
+ fieldMappings.put("meta:creation-date", "Creation-Date");
+ fieldMappings.put("meta:save-date", "Last-Save-Date");
+ fieldMappings.put("meta:keyword", "Keywords");
+ fieldMappings.put("pdf:docinfo:keywords", "Keywords");
+ }
+
+ /*
+ * Appends back-compatible metadata into the given {@code ExtractionMetadata} instance by mapping
+ * source fields to target fields, provided that backward compatibility is enabled. If a source
+ * field exists and the target field is not yet populated, the values from the source field will
+ * be added to the target field.
+ */
+ private void appendBackCompatTikaMetadata(ExtractionMetadata md) {
+ for (Map.Entry<String, String> mapping : fieldMappings.entrySet()) {
+ String sourceField = mapping.getKey();
+ String targetField = mapping.getValue();
+ if (md.getFirst(sourceField) != null && md.getFirst(targetField) == null) {
+ md.add(targetField, md.get(sourceField));
Review Comment:
This line copies a List<String> directly instead of individual values. It should iterate through the source values: `for (String value : md.get(sourceField)) { md.add(targetField, value); }`
```suggestion
for (String value : md.get(sourceField)) {
md.add(targetField, value);
}
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2424991874
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractingDocumentLoader.java:
##########
@@ -75,40 +55,33 @@ public class ExtractingDocumentLoader extends ContentStreamLoader {
/** Extract Only supported format. Default */
public static final String XML_FORMAT = "xml";
- /** XHTML XPath parser. */
- private static final XPathParser PARSER = new XPathParser("xhtml", XHTMLContentHandler.XHTML);
-
final SolrCore core;
final SolrParams params;
final UpdateRequestProcessor processor;
final boolean ignoreTikaException;
- protected AutoDetectParser autoDetectParser;
+ final boolean backCompat;
private final AddUpdateCommand templateAdd;
- protected TikaConfig config;
- protected ParseContextConfig parseContextConfig;
protected SolrContentHandlerFactory factory;
+ protected ExtractionBackend backend;
public ExtractingDocumentLoader(
SolrQueryRequest req,
UpdateRequestProcessor processor,
- TikaConfig config,
- ParseContextConfig parseContextConfig,
- SolrContentHandlerFactory factory) {
+ SolrContentHandlerFactory factory,
+ ExtractionBackend backend) {
this.params = req.getParams();
this.core = req.getCore();
- this.config = config;
- this.parseContextConfig = parseContextConfig;
this.processor = processor;
+ this.backCompat = params.getBool(ExtractingParams.BACK_COMPATIBILITY, true);
Review Comment:
I ended up sending initMap from handler to TikaExtractionBackend, and then parse the param there, and document that this option is only settable in config, not request.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2382675218
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractingRequestHandler.java:
##########
@@ -28,26 +26,24 @@
import org.apache.solr.security.PermissionNameProvider;
import org.apache.solr.update.processor.UpdateRequestProcessor;
import org.apache.solr.util.plugin.SolrCoreAware;
-import org.apache.tika.config.TikaConfig;
/**
* Handler for rich documents like PDF or Word or any other file format that Tika handles that need
* the text to be extracted first from the document.
- *
- * @deprecated Will be replaced with something similar that calls out to a separate Tika Server
- * process running in its own JVM.
*/
-@Deprecated(since = "9.10.0")
Review Comment:
totally!
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3319521394
When I first saw `DummyExtractionBackend` my first thought was that it should be in the test class hierarchy. However, would there be value in keeping it? If you wanted to test your set up in Solr (and not worry about the Tika side), could it be useful for that? "I send a doc and I get something back"....
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2373576131
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/DummyExtractionBackend.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.InputStream;
+
+/** Dummy backend that emits predictable test data without actually parsing input content. */
Review Comment:
I was going to remove the dummy stuff. I hear you about the need for a test mode. However, the tika server is just one `docker run` command away, so not sure it is a pressing need?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2373723621
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractionBackend.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.io.InputStream;
+import org.apache.tika.metadata.HttpHeaders;
+import org.apache.tika.metadata.TikaMetadataKeys;
+
+/** Strategy interface for content extraction backends. */
+public interface ExtractionBackend {
+ /**
+ * Extract plain text and metadata from the inputStream. Implementations should not close the
+ * inputStream. This API is backend-neutral and does not expose SAX or XML-specific types.
+ */
+ ExtractionResult extract(InputStream inputStream, ExtractionRequest request) throws Exception;
+
+ /**
+ * Perform extractOnly operation. If extractFormat equals ExtractingDocumentLoader.TEXT_FORMAT,
+ * return plain text. If XML, return XML body as string. Implementations may support optional
+ * xpathExpr; if unsupported and xpathExpr is not null, they should throw
+ * UnsupportedOperationException.
+ */
+ ExtractionResult extractOnly(InputStream inputStream, ExtractionRequest request, String xpathExpr)
+ throws Exception;
+
+ /**
+ * Parse the content and stream SAX events into the provided SolrContentHandler, while also
+ * filling outMetadata with extracted metadata.
+ */
+ void parseToSolrContentHandler(
+ InputStream inputStream,
+ ExtractionRequest request,
+ SolrContentHandler handler,
+ ExtractionMetadata outMetadata)
+ throws Exception;
+
+ /** Build ExtractionMetadata from the request context */
+ default ExtractionMetadata buildMetadataFromRequest(ExtractionRequest request) {
+ ExtractionMetadata md = new ExtractionMetadata();
+ if (request.resourceName != null)
Review Comment:
Nice!
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3328856372
Sounds like a plan! I didnt' know about the need for the xml streaming parsing... Having *both* in 9x is a much nicer migration then just a hard swap in 10. Plus, if someone wanted to keep the old 9x local processing version in 10, they could of course create their own backend and reference it!
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3328791839
My thought was to land tikaserver in Solr 9.x as opt-in while deprecating local. The server variant need not respond with exactly same metadata, and some of the tests which test specifically 1.x functionaly can be moved to that test class. But for simple use cases that 90% of users need, like extracting text and normal metadata from PDF, Word etc we get feature parity. Then we remove the local Tika parser in 10.0 and make server the default. I.e. users will have a transition path even in 9.x.
I started with the JSON output from Tika Server, but since it does not support streaming but a full copy in memory, I'm moving to the `/tika` endpoint with XML response, where the TikaServer streams XHTML as parsing happens, without buffering all in memory first. Same on SolrCell side, I'm successfully parsing the XHTML with SAX, parsing all the `<meta>` tags. Next is to feed the sax stream into `SolrContentHandler` which will handle the capturing stuff. This shuold both give a small mem footprint and unlock more of the SolrCell features.
While it is true that Tika 1.x and Tika 3.x has many breaking changes, that is mainly for the Java API. The XML parse result which is a `content` string and a `metadata` map stays the same, so no conceptual difference there. The metadata keys are a bit different/normalized, but we don't need to bridge that. We can simply document that when using `tikaserver` they should look for `dc:title` istead of `title`, and SolrCell allows you to map those to whatever schema field you like already.
The big question is of course whether we manage to get a stable tika server impl which is production ready before 10.0, and whether the refactoring leaves the old `local` impl as stable as it has been, the memory footprint may have increased etc.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2430792372
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/fromtika/BodyContentHandler.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.solr.handler.extraction.fromtika;
+
+import java.io.OutputStream;
+import java.io.Writer;
+import org.apache.tika.sax.WriteOutContentHandler;
+import org.apache.tika.sax.XHTMLContentHandler;
+import org.apache.tika.sax.xpath.Matcher;
+import org.apache.tika.sax.xpath.MatchingContentHandler;
+import org.apache.tika.sax.xpath.XPathParser;
Review Comment:
Here are more imports depending on Tika. These classes can also be copied into our project, but it starts to get deep, with a total of 18 java source files if we want to bring it all in.
<img width="247" height="485" alt="Skjermbilde 2025-10-15 kl 01 59 33" src="https://github.com/user-attachments/assets/f72270d5-4ccc-4de3-a4c1-5f62e0cd0619" />
Seems a bit much for [ONE usage in TikaServerExtractionBackend](https://github.com/apache/solr/blob/23b2359b1e9e3449239ac76c82b2037916852a1d/solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java#L129), so I'll leave it in there and defer to later to find / write a replacement for the `BodyContentHandler`.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3670:
URL: https://github.com/apache/solr/pull/3670#discussion_r2408998078
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractionRequest.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.solr.handler.extraction;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/** Immutable request info needed by extraction backends. */
+public class ExtractionRequest {
+ public final String streamType; // explicit MIME type (optional)
+ public final String resourceName; // filename hint
+ public final String contentType; // HTTP content-type header
+ public final String charset; // derived charset if available
+ public final String streamName;
+ public final String streamSourceInfo;
+ public final Long streamSize;
+ public final String resourcePassword; // optional password for encrypted docs
+ public final java.util.LinkedHashMap<java.util.regex.Pattern, String>
+ passwordsMap; // optional passwords map
+ public final String extractFormat;
+ public final boolean recursive;
+ public final Map<String, String> tikaRequestHeaders = new HashMap<>();
Review Comment:
I'm okay with documenting them with `tikaServer`. Different backends will want different things.
One thought, should we follow the same rules as for system properties. So dot lower case names?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3336421681
Yey! Rewrote tests to capture/xpath `<h1>` tags instead of `<div>` tags:
<img width="321" height="390" alt="Skjermbilde 2025-09-26 kl 02 57 56" src="https://github.com/user-attachments/assets/d14b7750-adc4-4edb-b9df-a6d8565a83b2" />
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3334841234
Are you seeing any issues in how TikaServer works that maybe are better fixed there? Some great progress!
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3381764042
> @epugh Do you see value in splitting this PR into first an introduction of backend concept and then the tikaserver backend, or shall we bring this PR (which is now all green) to completion and backport in one go? If folks find it hard / intimidating to review due to its size, it may make sense to split. But if it is manageable, it is faster to stay in this PR for everything.
I'm okay with this as one pr. I base this on: 1) I know your development style. So I have a high degree of confidence going in. 2) The "blast radius" of getting this not perfect is small. We aren't changing a core aspect of Solr. 3) Solr TEN IS COMING BABY! 4) Honestly, who is going to review this besides you, me and maybe @tballison :-(.
Would you be interested in demoing this at the next community meetup to get some more eyes on it? Or barring that, would you like me to?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-7632 TikaServer as pluggable backend to existing extraction handler [solr]
Posted by "chatman (via GitHub)" <gi...@apache.org>.
chatman commented on PR #3670:
URL: https://github.com/apache/solr/pull/3670#issuecomment-3393041760
> Would be perfect to have this go in both those releases so we can deprecate local mode in 9.10 and then remove local mode in 10.0 and get a slimmer distro without all the JARs.
+1, thank you for your work on this.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org