-
Notifications
You must be signed in to change notification settings - Fork 684
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Jetty12 + EE10 #2876
base: main
Are you sure you want to change the base?
Jetty12 + EE10 #2876
Changes from all commits
421c095
0b80a80
ed63623
aecec4b
1629241
52d5416
a047f5e
d98b23d
7025003
221c3f5
c813d4a
873bf7d
e4e4540
fdaf430
871b6c4
780c0a2
dbe7c4f
4df91b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,8 @@ | |
import static org.apache.solr.servlet.SolrDispatchFilter.Action.REMOTEQUERY; | ||
|
||
import io.opentelemetry.api.trace.Span; | ||
import jakarta.servlet.http.HttpServletRequest; | ||
import jakarta.servlet.http.HttpServletResponse; | ||
import java.io.IOException; | ||
import java.lang.invoke.MethodHandles; | ||
import java.util.ArrayList; | ||
|
@@ -35,8 +37,6 @@ | |
import java.util.Locale; | ||
import java.util.Map; | ||
import java.util.Set; | ||
import javax.servlet.http.HttpServletRequest; | ||
import javax.servlet.http.HttpServletResponse; | ||
import net.jcip.annotations.ThreadSafe; | ||
import org.apache.solr.client.solrj.SolrRequest; | ||
import org.apache.solr.common.SolrException; | ||
|
@@ -436,7 +436,12 @@ protected void executeCoreRequest(SolrQueryResponse rsp) { | |
// SolrCore counter | ||
core.close(); | ||
core = null; | ||
response.getHeaderNames().stream().forEach(name -> response.setHeader(name, null)); | ||
// Skip specific headers | ||
// workaround for response.setHeader(name, null) | ||
response.getHeaderNames().stream() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks good for now IMO. Longer term I'd love to see this code go away in favor of having the v2 API stuff get handled by its own servlet, but we're pretty far from that being workable for now. It is a little hacky in the interim though 😦 |
||
.filter(name -> !name.equalsIgnoreCase("Content-Length")) | ||
.forEach(name -> response.setHeader(name, "")); | ||
response.setContentLength(-1); | ||
invokeJerseyRequest( | ||
cores, null, cores.getJerseyApplicationHandler(), cores.getRequestHandlers(), rsp); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -100,9 +100,14 @@ public class ShowFileRequestHandler extends RequestHandlerBase implements Permis | |
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); | ||
|
||
static { | ||
KNOWN_MIME_TYPES = new HashSet<>(MimeTypes.getKnownMimeTypes()); | ||
KNOWN_MIME_TYPES = new HashSet<>(); | ||
for (MimeTypes.Type type : MimeTypes.Type.values()) { | ||
KNOWN_MIME_TYPES.add(type.toString()); | ||
} | ||
KNOWN_MIME_TYPES.add("text/xml"); | ||
KNOWN_MIME_TYPES.add("text/javascript"); | ||
KNOWN_MIME_TYPES.add("text/csv"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did csv/xhtml get added? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Starting from Jetty 11 these two mime types are not included in |
||
KNOWN_MIME_TYPES.add("application/xhtml+xml"); | ||
} | ||
|
||
public ShowFileRequestHandler() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -90,7 +90,11 @@ public <T extends SolrJerseyResponse> T handlePotentiallyAsynchronousTask( | |
} | ||
|
||
MDCLoggingContext.setCoreName(coreName); | ||
TraceUtils.setDbInstance(req, coreName); | ||
try { | ||
TraceUtils.setDbInstance(req, coreName); | ||
} catch (Exception e) { | ||
// TODO: Find better way to fix NPE | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we use "nocommit" which will also fail precommit until we get it fixed, serving as a reminder. Any way, this try-catch definitely shouldn't be here if it should be anywhere. We should fix what's null instead. |
||
} | ||
if (taskId == null) { | ||
return supplier.get(); | ||
} else { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would go with jersey version 3.1.9 if possible, so that we can remove this entry completely (see line above).