Skip to content
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

Use JUnit Server using Tomcat as test server #4454

Closed
erikgb opened this issue Feb 24, 2023 · 4 comments
Closed

Use JUnit Server using Tomcat as test server #4454

erikgb opened this issue Feb 24, 2023 · 4 comments
Labels
📶 enhancement issue is a new feature or improvement M1 Fixed in milestone 1
Milestone

Comments

@erikgb
Copy link
Contributor

erikgb commented Feb 24, 2023

Problem description

For the last couple of weeks, I've been trying to find a way to prepare for the big javax->jakarta migration, and Java/Jakarta EE upgrades in general (ref. #3559). After pulling out WireMock in #4439, the next challenge is the embedded Solr server. Even the latest release of Solr builds on top of Jetty 9.x which makes it really hard for us to move forward. Ref. #4252

To me, it seems like the only way forward would be to eliminate our (direct) Jetty dependencies, remove the dependency management of Jetty artifacts and allow our dependencies (Solr/ Lucene/Elastic) to pull in whatever version they require of Jetty. This could become tricky in aggregated runtimes (servers), but I think that could be manageable.

As an experiment, I ran the following dependency analyze command mvn dependency:tree -Dincludes=org.eclipse.jetty* -DoutputFile=<path>/jetty-deps.txt -DappendOutput=true, and I have attached the current result for the develop branch: jetty-deps.txt

Preferred solution

As a start, I suggest replacing all our Jetty-based test servers with JUnit Server using Tomcat. The goal is to also close #3507 with a PR closing this issue. I made a POC in #4437 - using Jetty, but the changes would be similar using Tomcat. WDYT? I need to get some directions from the maintainers before moving forward on this.

Are you interested in contributing a solution yourself?

Yes

Alternatives you've considered

No response

Anything else?

No response

@erikgb erikgb added the 📶 enhancement issue is a new feature or improvement label Feb 24, 2023
@abrokenjester
Copy link
Contributor

I'm all in favor of cleaning this up. I'm not very familiar with JUnit Server, but am not particularly married to the idea of using Jetty either. So if you've got appetite to clean this up using JUnit Server, I'm all for it!

@abrokenjester abrokenjester added this to the 4.3.0 milestone Feb 25, 2023
@erikgb
Copy link
Contributor Author

erikgb commented Feb 25, 2023

I'm all in favor of cleaning this up. I'm not very familiar with JUnit Server, but am not particularly married to the idea of using Jetty either. So if you've got appetite to clean this up using JUnit Server, I'm all for it!

Thanks for the feedback! I have no experience with JUnit Server, but it seems to provide a thin wrapper layer around different versions of Jetty and Tomcat. And seem to support custom test servers as well. So instead of maintaining the code for adapting to a particular servlet engine, that needs to be maintained during upgrades, it seems appealing to just switch a couple of dependency coordinates.

I'll start on a PR soon - then we'll see how this goes. Using a test server that is not 100% tied to upstream runtime dependencies (that we don't control) seems very appealing to me!

@JervenBolleman
Copy link
Contributor

Small heads up regarding tomcat as a SPARQL protocol hosting server engine.

Tomcat is very strict about RFC iri's, while browsers use the WhatWG definition of IRI's. Most people would not care about the difference between them, but SPARQL queries in IRI's tend to fall straight in the gap between those definitions, as they are about the brackets that are common in queries.

an extract from my tomcat configurations to deal with this.

 <Connector port="${http.port}" protocol="HTTP/1.1"
               connectionTimeout="20000"
               redirectPort="8443" 
               URIEncoding="UTF-8"
               relaxedQueryChars="&gt;&lt;[\]^`{|}"/>

note the extra relaxedQueryChars that are passed into the tomcat connector setup.

@erikgb
Copy link
Contributor Author

erikgb commented May 27, 2023

On second thoughts I don't think we should do this.

@erikgb erikgb closed this as completed May 27, 2023
@hmottestad hmottestad added the M1 Fixed in milestone 1 label Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📶 enhancement issue is a new feature or improvement M1 Fixed in milestone 1
Projects
None yet
Development

No branches or pull requests

4 participants