SOLR-17321: Bump minimum required Java version to 21 #2497

iamsanjay · 2024-06-04T16:15:01Z

https://issues.apache.org/jira/browse/SOLR-17321

We are upgrading the minimum Java version for Solr main branch to 21. However, at the same, It has been suggested to be not so aggressive with SolrJ (and thus solr-api, a dependency) Java version – setting it to 17.

Depreciated API

AccessController [Deprecated since 17 and marked for removal]
AccessControlContext [Deprecated since 17 and marked for removal]
java.net.URL ctors [Deprecated since 20]
SecurityManager [Deprecated since 17 and marked for removal]
Thread#getId() [Deprecated in Java 19]

To not make any extra code changes below annotation were used:

 @SuppressForbidden(reason = "<custom message>") //Still need to decide what the message should be for all. Thinking of adding "Deprecated in Java <version>"

@SuppressWarnings("removal")

However, at few places, for demonstration purpose, URL has been created via new suggested way instead of using the ctors:

url = new URL(urlStr);
url = URI.create(urlStr).toURL();

Or,

URL solrURL = new URI(solrUrl).toURL();

Also, Bump up the ecj version to 3.37 and ca.cutterslade.analyze to 1.9.2.

Checklist

Please review the following and check all that apply:

I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
I have created a Jira issue and added the issue ID to my pull request title.
I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
I have developed this patch against the main branch.
I have run ./gradlew check.
[] I have added tests for my changes.
I have added documentation for the Reference Guide

iamsanjay · 2024-06-04T16:22:23Z

For now two things pending

Set different Java level for SolrJ and Solr. Currently both are 21.
And, another one is to change the test-via-crave.yml file to run the build with Java 21.

Feel free to push changes to this branch.

dsmiley

RE URL: If you have IntelliJ Ultimate edition (not sure if Community), you could do "Structural Replace" to find & replace systematically everywhere.

FYI as an ASF committer, you can get that edition for free -- https://www.jetbrains.com/shop/eform/apache?product=ALL

dsmiley · 2024-06-04T18:47:21Z

gradle/validation/dependency-analyze.gradle

@@ -23,14 +23,14 @@ allprojects { prj ->
    prj.apply plugin: 'ca.cutterslade.analyze'

    analyzeClassesDependencies {
-      warnUsedUndeclared = false // means fail build if UsedUndeclared found
-      warnUnusedDeclared = false // means fail build if UnusedDeclared found
+      warnUsedUndeclared = true // means fail build if UsedUndeclared found


you updated booleans but not the explanatory comment. Why change this any way?

solr/core/src/java/org/apache/solr/cli/PostTool.java

dsmiley · 2024-06-04T18:56:31Z

solr/core/src/test/org/apache/solr/cli/PostToolTest.java

@@ -210,6 +211,7 @@ public void testAppendParam() {
  }

  @Test
+  @SuppressForbidden(reason = "java.net.URL ctors deprecated since Java 20")


can this be put at the class level once instead of most tests?

solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java

solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformerDistrib.java

dsmiley · 2024-06-04T19:29:59Z

@SuppressForbidden(reason = "") //Still need to decide what the message should be for all. Thinking of adding "Deprecated in Java "

Ideally those are addressed for real, otherwise they will continue to linger. Could be a separate JIRA/PR.

janhoy · 2024-06-04T21:35:26Z

build.gradle

-rootProject.ext.minJavaVersionDefault = JavaVersion.VERSION_11
-rootProject.ext.minJavaVersionSolrJ = JavaVersion.VERSION_11
+rootProject.ext.minJavaVersionDefault = JavaVersion.VERSION_21
+rootProject.ext.minJavaVersionSolrJ = JavaVersion.VERSION_21


Use Java 17 for SolrJ

epugh

Great to see movement. I would prefer to see the Java deprecations fixed versus more suppress annotations!!! Excited for this.

solr/core/src/java/org/apache/solr/cli/PostTool.java

epugh · 2024-06-05T00:15:29Z

solr/core/src/java/org/apache/solr/cli/PostTool.java

-                          + URLEncoder.encode(url.toString(), UTF_8)
-                          + "&literal.url="
-                          + URLEncoder.encode(url.toString(), UTF_8)));
+              new URI(


If you run tidy do some white space changes go away?

epugh · 2024-06-05T00:17:15Z

solr/core/src/java/org/apache/solr/cli/PostTool.java

@@ -815,7 +820,7 @@ public void postFile(File file, OutputStream output, String type) throws Malform
          urlStr =
              appendParam(urlStr, "literal.id=" + URLEncoder.encode(file.getAbsolutePath(), UTF_8));
        }
-        url = new URL(urlStr);
+        url = URI.create(urlStr).toURL();


Curious if using a URI would be better than constantly converting back to string? Maybe another pr...

iamsanjay · 2024-06-05T12:42:38Z

RE URL: If you have IntelliJ Ultimate edition (not sure if Community), you could do "Structural Replace" to find & replace systematically everywhere.

Like always, Thanks for the air support. "Structural Replace" is a Game Changer!

dsmiley

Would it be too much to ask to separate all this URL conversion stuff to a separate PR, ultimately being its own commit focused on exactly that? Needn't be another JIRA; needn't touch CHANGES.txt in that commit. If I had to do this, I'd start with the trick of taking a GitHub PR URL and adding the ".patch" (or maybe it's ".diff" for a different format) and then copy-paste into IntellIJ "Apply patch from Clipboard" and selectively exclude stuff unrelated at that point (or after).

dsmiley · 2024-06-05T12:44:36Z

solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java

@@ -102,8 +104,9 @@ public static String buildRequestUrl(
    return basePath + path;
  }

+  @SuppressForbidden(reason = "java.net.URL ctors deprecated since Java 20")


No longer need this suppress.

epugh · 2024-06-05T12:48:44Z

RE URL: If you have IntelliJ Ultimate edition (not sure if Community), you could do "Structural Replace" to find & replace systematically everywhere.

Like always, Thanks for the air support. "Structural Replace" is a Game Changer!

I need to learn that! Looks amazing!

iamsanjay · 2024-06-05T12:54:02Z

Would it be too much to ask to separate all this URL conversion stuff to a separate PR, ultimately being its own commit focused on exactly that? Needn't be another JIRA; needn't touch CHANGES.txt in that commit. If I had to do this, I'd start with the trick of taking a GitHub PR URL and adding the ".patch" (or maybe it's ".diff" for a different format) and then copy-paste into IntellIJ "Apply patch from Clipboard" and selectively exclude stuff unrelated at that point (or after).

So SuppressForbidden is okay for now, and later on (Just after this one!) deal with removing it in a different PR?

dsmiley · 2024-06-05T13:15:46Z

I suggest flipping the order. Prep the code for Java 21 compatibility first so that we don't go adding SupressForbidden to many classes to only then fix the issue later. i.e. visit these classes once not twice. You know exactly how to fix it and have even starting doing so; it's not a tricky matter. Again, IntelliJ will do much of the work.

…

On Wed, Jun 5, 2024 at 8:55 AM Sanjay Dutt ***@***.***> wrote: Would it be too much to ask to separate all this URL conversion stuff to a separate PR, ultimately being its own commit focused on exactly that? Needn't be another JIRA; needn't touch CHANGES.txt in that commit. If I had to do this, I'd start with the trick of taking a GitHub PR URL and adding the ".patch" (or maybe it's ".diff" for a different format) and then copy-paste into IntellIJ "Apply patch from Clipboard" and selectively exclude stuff unrelated at that point (or after). So SuppressForbidden is okay for now, and later on (Just after this one!) deal with removing it in a different PR? — Reply to this email directly, view it on GitHub <#2497 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC4DTZJTS2ICJN2TM6AJZLZF4C77AVCNFSM6AAAAABIY26TRCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBZG44TCMZZGA> . You are receiving this because your review was requested.Message ID: ***@***.***>

iamsanjay · 2024-06-05T15:19:52Z

Test case failing, on main branch as well!

./gradlew :solr:core:test --tests "org.apache.solr.search.function.TestDenseVectorFunctionQuery.floatFieldVectors_shouldReturnFloatSimilarity" -Ptests.jvms=96 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=8F0CED8E85B304BD -Ptests.timeoutSuite=600000! -Ptests.file.encoding=US-ASCII

dsmiley · 2024-06-05T16:26:48Z

References to JavaVersion (a Gradle class) like >= JavaVersion.VERSION_16 should be reviewed to see if we can make changes to remove conditions that will no longer vary.

Searching for "Java 17" or "JDK 17" or similar searches or other versions can reveal stuff where maybe a change is needed.

iamsanjay · 2024-06-10T03:42:53Z

Setting different Java version for main and SolrJ resulting in error. Currently, :solr:api:ecjLintTest task is failing and below is the error.

> Task :solr:api:ecjLintTest
----------
1. ERROR in /Users/sanjaydutt/Documents/Solr/solr-main/solr/solr/api/src/test/org/apache/solr/util/TestSolrVersion.java (at line 19)
        import org.apache.solr.SolrTestCase;
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The import org.apache.solr.SolrTestCase cannot be resolved
----------
2. ERROR in /Users/sanjaydutt/Documents/Solr/solr-main/solr/solr/api/src/test/org/apache/solr/util/TestSolrVersion.java (at line 23)
        public class TestSolrVersion extends SolrTestCase {
                                             ^^^^^^^^^^^^
SolrTestCase cannot be resolved to a type

The api test cases not able to access the class, SolrTestClass, from the main branch. Further, I logged the java version that has been used to configure SolrJ via adding println statements.

ecj-lint.gradle

allprojects {
  plugins.withType(JavaPlugin) {
    // Create a [sourceSetName]EcjLint task for each source set
    // with a non-empty java.srcDirs. These tasks are then
    // attached to project's "ecjLint" task.

    println "The project name is " + project + " and the sourceCompatibility is " + project.sourceCompatibility

    def lintTasks = sourceSets.collect { sourceSet ->
      def srcDirs = sourceSet.java.srcDirs.findAll { dir -> dir.exists() }

      tasks.create(sourceSet.getTaskName("ecjLint", null), JavaExec, {task ->
....
...

Output

> Configure project :solr:server
The project name is project ':solr:server' and the sourceCompatibility is 21

> Configure project :solr:core
The project name is project ':solr:core' and the sourceCompatibility is 21

> Configure project :solr:api
The project name is project ':solr:api' and the sourceCompatibility is 17

> Configure project :solr:solrj
The project name is project ':solr:solrj' and the sourceCompatibility is 17

The version has changed for SolrJ and for api as well. However, as @dsmiley pointed out before that test classes for both of these project should point to 21. Still looking for a way How we can achieve that.

iamsanjay requested review from epugh, dsmiley, janhoy and risdenk June 4, 2024 16:15

github-actions bot added test-framework jetty-server client:solrj module:jwt-auth tool:build tests cat:search cat:cloud cat:index cat:cli cat:schema cat:security cat:packagemanager module:langid module:scripting labels Jun 4, 2024

dsmiley reviewed Jun 4, 2024

View reviewed changes

janhoy reviewed Jun 4, 2024

View reviewed changes

epugh reviewed Jun 5, 2024

View reviewed changes

dsmiley reviewed Jun 5, 2024

View reviewed changes

This was referenced Jun 6, 2024

force the use jdk21 through Crave #2498

Closed

SOLR-17321: Remove Deprecated URL API in Preparation for Java 21 #2501

Merged

iamsanjay closed this Aug 23, 2024

iamsanjay force-pushed the Solr_Java21 branch from 7152c64 to ca24876 Compare August 23, 2024 16:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SOLR-17321: Bump minimum required Java version to 21 #2497

SOLR-17321: Bump minimum required Java version to 21 #2497

iamsanjay commented Jun 4, 2024 •

edited

Loading

iamsanjay commented Jun 4, 2024

dsmiley left a comment

dsmiley Jun 4, 2024

dsmiley Jun 4, 2024

dsmiley commented Jun 4, 2024

janhoy Jun 4, 2024

epugh left a comment

epugh Jun 5, 2024

epugh Jun 5, 2024

iamsanjay commented Jun 5, 2024 •

edited

Loading

dsmiley left a comment

dsmiley Jun 5, 2024

epugh commented Jun 5, 2024

iamsanjay commented Jun 5, 2024

dsmiley commented Jun 5, 2024 via email

iamsanjay commented Jun 5, 2024

dsmiley commented Jun 5, 2024

iamsanjay commented Jun 10, 2024

SOLR-17321: Bump minimum required Java version to 21 #2497

SOLR-17321: Bump minimum required Java version to 21 #2497

Conversation

iamsanjay commented Jun 4, 2024 • edited Loading

Checklist

iamsanjay commented Jun 4, 2024

dsmiley left a comment

Choose a reason for hiding this comment

dsmiley Jun 4, 2024

Choose a reason for hiding this comment

dsmiley Jun 4, 2024

Choose a reason for hiding this comment

dsmiley commented Jun 4, 2024

janhoy Jun 4, 2024

Choose a reason for hiding this comment

epugh left a comment

Choose a reason for hiding this comment

epugh Jun 5, 2024

Choose a reason for hiding this comment

epugh Jun 5, 2024

Choose a reason for hiding this comment

iamsanjay commented Jun 5, 2024 • edited Loading

dsmiley left a comment

Choose a reason for hiding this comment

dsmiley Jun 5, 2024

Choose a reason for hiding this comment

epugh commented Jun 5, 2024

iamsanjay commented Jun 5, 2024

dsmiley commented Jun 5, 2024 via email

iamsanjay commented Jun 5, 2024

dsmiley commented Jun 5, 2024

iamsanjay commented Jun 10, 2024

iamsanjay commented Jun 4, 2024 •

edited

Loading

iamsanjay commented Jun 5, 2024 •

edited

Loading