diff --git a/.github/workflows/precommit.yml b/.github/workflows/precommit.yml index b6ba15fe1c598..635a9ce003cf5 100644 --- a/.github/workflows/precommit.yml +++ b/.github/workflows/precommit.yml @@ -1,4 +1,4 @@ -name: Gradle Precommit +name: Gradle PreCommit on: [pull_request] jobs: @@ -21,8 +21,11 @@ jobs: java-version: ${{ matrix.java }} distribution: temurin cache: gradle - - name: Run Gradle (precommit) + - name: Run Gradle (pre-commit) continue-on-error: ${{ matrix.experimental }} shell: bash - run: | - ./gradlew javadoc precommit --parallel + run: ./gradlew javadoc precommit --parallel + - name: Run Gradle (sanity-check) + continue-on-error: ${{ matrix.experimental }} + shell: bash + run: ./gradlew rewriteDryRun -Dorg.gradle.jvmargs=-Xmx8G diff --git a/CHANGELOG.md b/CHANGELOG.md index cdea7c564ade0..4b9d8df7c94a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add build-tooling to run in FIPS environment ([#18921](https://github.com/opensearch-project/OpenSearch/pull/18921)) - Add SMILE/CBOR/YAML document format support to Bulk GRPC endpoint ([#19744](https://github.com/opensearch-project/OpenSearch/pull/19744)) - Implement GRPC ConstantScoreQuery, FuzzyQuery, MatchBoolPrefixQuery, MatchPhrasePrefix, PrefixQuery, MatchQuery ([#19854](https://github.com/opensearch-project/OpenSearch/pull/19854)) +- [rewrite] Add `GradleBestPractices` ([#19885](https://github.com/opensearch-project/OpenSearch/pull/19885)) ### Changed - Faster `terms` query creation for `keyword` field with index and docValues enabled ([#19350](https://github.com/opensearch-project/OpenSearch/pull/19350)) diff --git a/build.gradle b/build.gradle index a6dc4348bdd6b..defe86b34b645 100644 --- a/build.gradle +++ b/build.gradle @@ -59,6 +59,7 @@ plugins { id "org.gradle.test-retry" version "1.6.2" apply false id "test-report-aggregation" id 'jacoco-report-aggregation' + id 'org.openrewrite.rewrite' version '7.19.0' apply false } apply from: 'gradle/build-complete.gradle' @@ -70,6 +71,7 @@ apply from: 'gradle/local-distribution.gradle' apply from: 'gradle/run.gradle' apply from: 'gradle/missing-javadoc.gradle' apply from: 'gradle/code-coverage.gradle' +apply from: 'gradle/rewrite.gradle' // Apply FIPS configuration to all projects allprojects { diff --git a/distribution/archives/build.gradle b/distribution/archives/build.gradle index b6cb165cb6333..ff18c12959053 100644 --- a/distribution/archives/build.gradle +++ b/distribution/archives/build.gradle @@ -28,7 +28,7 @@ * under the License. */ -import org.opensearch.gradle.JavaPackageType +import org.opensearch.gradle.JavaPackageType apply plugin: 'opensearch.internal-distribution-archive-setup' @@ -193,7 +193,7 @@ distribution_archives { } } - + linuxPpc64leTar { archiveClassifier = 'linux-ppc64le' content { diff --git a/gradle/rewrite.gradle b/gradle/rewrite.gradle new file mode 100644 index 0000000000000..60a6f485e09bb --- /dev/null +++ b/gradle/rewrite.gradle @@ -0,0 +1,28 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +project.apply plugin: 'org.openrewrite.rewrite' + +rewrite { + activeRecipe('org.opensearch.openrewrite.SanityCheck') + exclusion('**ActionListener.java') + exclusion('**SearchAfterIT.java') + exclusion('**StarTreeMapper.java') + exclusion('**package-info.java') + setExportDatatables(true) + setFailOnDryRunResults(true) +} + +dependencies { + rewrite(platform('org.openrewrite.recipe:rewrite-recipe-bom:3.17.0')) + rewrite('org.openrewrite.recipe:rewrite-migrate-java:3.20.0') + rewrite('org.openrewrite.recipe:rewrite-java-security:3.19.2') + rewrite('org.openrewrite.recipe:rewrite-rewrite:0.14.1') + rewrite('org.openrewrite.recipe:rewrite-static-analysis:2.20.0') + rewrite('org.openrewrite.recipe:rewrite-third-party:0.30.0') +} diff --git a/gradle/wrapper/gradle-wrapper.jar b/gradle/wrapper/gradle-wrapper.jar index 1b33c55baabb5..f8e1ee3125fe0 100644 Binary files a/gradle/wrapper/gradle-wrapper.jar and b/gradle/wrapper/gradle-wrapper.jar differ diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 7e324494c4c00..a8eccb6a331e8 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,8 +1,8 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-9.2.0-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-9.2.0-bin.zip networkTimeout=10000 validateDistributionUrl=true zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists -distributionSha256Sum=16f2b95838c1ddcf7242b1c39e7bbbb43c842f1f1a1a0dc4959b6d4d68abcac3 +distributionSha256Sum=df67a32e86e3276d011735facb1535f64d0d88df84fa87521e90becc2d735444 diff --git a/gradlew b/gradlew index 23d15a9367071..cd69147b477bb 100755 --- a/gradlew +++ b/gradlew @@ -1,7 +1,7 @@ #!/bin/sh # -# Copyright © 2015-2021 the original authors. +# Copyright © 2015 the original authors. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -202,7 +202,7 @@ fi # Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script. -DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"' +DEFAULT_JVM_OPTS= # Collect all arguments for the java command: # * DEFAULT_JVM_OPTS, JAVA_OPTS, and optsEnvironmentVar are not allowed to contain shell fragments, diff --git a/gradlew.bat b/gradlew.bat old mode 100644 new mode 100755 index 5eed7ee845284..53c7bc8d9d39a --- a/gradlew.bat +++ b/gradlew.bat @@ -36,7 +36,7 @@ set APP_HOME=%DIRNAME% for %%i in ("%APP_HOME%") do set APP_HOME=%%~fi @rem Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script. -set DEFAULT_JVM_OPTS="-Xmx64m" "-Xms64m" +set DEFAULT_JVM_OPTS= @rem Find java.exe if defined JAVA_HOME goto findJavaFromJavaHome @@ -70,11 +70,10 @@ goto fail :execute @rem Setup the command line -set CLASSPATH= @rem Execute Gradle -"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" -jar "%APP_HOME%\gradle\wrapper\gradle-wrapper.jar" %* +"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -jar "%APP_HOME%\gradle\wrapper\gradle-wrapper.jar" %* :end @rem End local scope for the variables with windows NT shell diff --git a/modules/build.gradle b/modules/build.gradle index 0c69a43af0509..f089bcd982efe 100644 --- a/modules/build.gradle +++ b/modules/build.gradle @@ -32,7 +32,7 @@ configure(subprojects.findAll { it.parent.path == project.path }) { group = 'org.opensearch.plugin' // for modules which publish client jars apply plugin: 'opensearch.testclusters' apply plugin: 'opensearch.opensearchplugin' - + opensearchplugin { // for local OpenSearch plugins, the name of the plugin is the same as the directory name = project.name diff --git a/plugins/arrow-flight-rpc/src/main/java/org/apache/arrow/flight/OSFlightClient.java b/plugins/arrow-flight-rpc/src/main/java/org/apache/arrow/flight/OSFlightClient.java index 0efafd370c651..4e6faa5d4c3cc 100644 --- a/plugins/arrow-flight-rpc/src/main/java/org/apache/arrow/flight/OSFlightClient.java +++ b/plugins/arrow-flight-rpc/src/main/java/org/apache/arrow/flight/OSFlightClient.java @@ -47,7 +47,7 @@ public final static class Builder { private ExecutorService executorService; private Class channelType; private SslContext sslContext; - + private Builder() {} Builder(BufferAllocator allocator, Location location) { @@ -131,7 +131,7 @@ public FlightClient build() { builder.channelType( Class.forName("io.netty.channel.epoll.EpollDomainSocketChannel") .asSubclass(ServerChannel.class)); - final EventLoopGroup elg = + final EventLoopGroup elg = Class.forName("io.netty.channel.epoll.EpollEventLoopGroup") .asSubclass(EventLoopGroup.class) .getDeclaredConstructor() @@ -142,7 +142,7 @@ public FlightClient build() { builder.channelType( Class.forName("io.netty.channel.kqueue.KQueueDomainSocketChannel") .asSubclass(ServerChannel.class)); - final EventLoopGroup elg = + final EventLoopGroup elg = Class.forName("io.netty.channel.kqueue.KQueueEventLoopGroup") .asSubclass(EventLoopGroup.class) .getDeclaredConstructor() @@ -179,7 +179,7 @@ public FlightClient build() { builder.sslContext(sslContext); } else { final SslContextBuilder sslContextBuilder = GrpcSslContexts.forClient(); - + if (!this.verifyServer) { sslContextBuilder.trustManager(InsecureTrustManagerFactory.INSTANCE); } else if (this.trustedCertificates != null @@ -215,11 +215,11 @@ public FlightClient build() { if (channelType != null) { builder.channelType(channelType); } - + if (workerELG != null) { builder.eventLoopGroup(workerELG); } - + return new FlightClient(allocator, builder.build(), middleware); } @@ -232,7 +232,7 @@ public Builder channelType(Class channelType this.channelType = channelType; return this; } - + public Builder eventLoopGroup(EventLoopGroup workerELG) { this.workerELG = workerELG; return this; diff --git a/plugins/arrow-flight-rpc/src/main/java/org/apache/arrow/flight/OSFlightServer.java b/plugins/arrow-flight-rpc/src/main/java/org/apache/arrow/flight/OSFlightServer.java index 551c5a22754b9..002ed3e4fd1af 100644 --- a/plugins/arrow-flight-rpc/src/main/java/org/apache/arrow/flight/OSFlightServer.java +++ b/plugins/arrow-flight-rpc/src/main/java/org/apache/arrow/flight/OSFlightServer.java @@ -121,10 +121,10 @@ public FlightServer build() { this.middleware(FlightConstants.HEADER_KEY, new ServerHeaderMiddleware.Factory()); final NettyServerBuilder builder; - + // Use primary location for initial setup Location primaryLocation = location != null ? location : listenAddresses.get(0); - + switch (primaryLocation.getUri().getScheme()) { case LocationSchemes.GRPC_DOMAIN_SOCKET: { @@ -268,7 +268,7 @@ public FlightServer build() { builder.addListenAddress(listenAddress.toSocketAddress()); } } - + builder.intercept(new ServerInterceptorAdapter(interceptors)); try { @@ -472,7 +472,7 @@ public Builder location(Location location) { this.location = Preconditions.checkNotNull(location); return this; } - + public Builder addListenAddress(Location location) { this.listenAddresses.add(Preconditions.checkNotNull(location)); return this; diff --git a/qa/remote-clusters/build.gradle b/qa/remote-clusters/build.gradle index a68b970562fff..7ad4f0ce63640 100644 --- a/qa/remote-clusters/build.gradle +++ b/qa/remote-clusters/build.gradle @@ -54,8 +54,8 @@ opensearch_distributions { tasks.named("preProcessFixture").configure { dependsOn opensearch_distributions.docker // always run the task, otherwise the folders won't be created - outputs.upToDateWhen { - false + outputs.upToDateWhen { + false } doLast { // tests expect to have an empty repo diff --git a/qa/systemd-test/src/test/java/org/opensearch/systemdinteg/SystemdIntegTests.java b/qa/systemd-test/src/test/java/org/opensearch/systemdinteg/SystemdIntegTests.java index bc6c929bdf766..9ec9c80455b43 100644 --- a/qa/systemd-test/src/test/java/org/opensearch/systemdinteg/SystemdIntegTests.java +++ b/qa/systemd-test/src/test/java/org/opensearch/systemdinteg/SystemdIntegTests.java @@ -124,14 +124,14 @@ public void testReadWritePaths() throws IOException, InterruptedException { public void testMaxProcesses() throws IOException, InterruptedException { String limits = executeCommand("sudo su -c 'cat /proc/" + opensearchPid + "/limits'", "Failed to read process limits"); - assertTrue("Max processes limit should be 4096 or unlimited", + assertTrue("Max processes limit should be 4096 or unlimited", limits.contains("Max processes 4096 4096") || limits.contains("Max processes unlimited unlimited")); } public void testFileDescriptorLimit() throws IOException, InterruptedException { String limits = executeCommand("sudo su -c 'cat /proc/" + opensearchPid + "/limits'", "Failed to read process limits"); - assertTrue("File descriptor limit should be at least 65535", + assertTrue("File descriptor limit should be at least 65535", limits.contains("Max open files 65535 65535") || limits.contains("Max open files unlimited unlimited")); } diff --git a/rewrite.yml b/rewrite.yml new file mode 100644 index 0000000000000..5a673efd3702f --- /dev/null +++ b/rewrite.yml @@ -0,0 +1,69 @@ +--- +type: specs.openrewrite.org/v1beta/recipe +name: org.opensearch.openrewrite.SanityCheck +displayName: Apply all Java & Gradle best practices +description: Comprehensive code quality recipe combining modernization, security, and best practices. +tags: + - java + - gradle + - static-analysis + - cleanup +recipeList: + - org.openrewrite.gradle.EnableGradleBuildCache + - org.openrewrite.gradle.EnableGradleParallelExecution + - org.openrewrite.gradle.GradleBestPractices + - org.openrewrite.java.format.NormalizeLineBreaks + - org.openrewrite.java.format.RemoveTrailingWhitespace + - org.openrewrite.staticanalysis.RemoveUnusedPrivateMethods + # TBD + # - org.openrewrite.java.RemoveUnusedImports + # - org.openrewrite.java.format.NormalizeFormat + # - org.openrewrite.java.migrate.UpgradeToJava17 + # - org.openrewrite.java.migrate.lang.FindVirtualThreadOpportunities # don't want to use: https://github.com/diffplug/spotless/pull/2684#discussion_r2433831887 + # - org.openrewrite.java.migrate.lang.StringRulesRecipes + # - org.openrewrite.java.migrate.util.JavaLangAPIs + # - org.openrewrite.java.migrate.util.JavaUtilAPIs + # - org.openrewrite.java.migrate.util.MigrateInflaterDeflaterToClose + # - org.openrewrite.java.migrate.util.ReplaceStreamCollectWithToList + # - org.openrewrite.java.migrate.util.SequencedCollection + # - org.openrewrite.java.recipes.JavaRecipeBestPractices + # - org.openrewrite.java.recipes.RecipeTestingBestPractices + # - org.openrewrite.java.security.JavaSecurityBestPractices + # - org.openrewrite.staticanalysis.BufferedWriterCreationRecipes + # - org.openrewrite.staticanalysis.CommonStaticAnalysis + # - org.openrewrite.staticanalysis.EqualsAvoidsNull + # - org.openrewrite.staticanalysis.JavaApiBestPractices + # - org.openrewrite.staticanalysis.LowercasePackage + # - org.openrewrite.staticanalysis.MissingOverrideAnnotation + # - org.openrewrite.staticanalysis.ModifierOrder + # - org.openrewrite.staticanalysis.NoFinalizer + # - org.openrewrite.staticanalysis.NoToStringOnStringType + # - org.openrewrite.staticanalysis.NoValueOfOnStringType + # - org.openrewrite.staticanalysis.RemoveUnusedLocalVariables + # - org.openrewrite.staticanalysis.RemoveUnusedPrivateFields + # - org.openrewrite.staticanalysis.ReplaceApacheCommonsLang3ValidateNotNullWithObjectsRequireNonNull + # - org.openrewrite.staticanalysis.SimplifyTernaryRecipes + # - org.openrewrite.staticanalysis.URLEqualsHashCodeRecipes + # - org.openrewrite.staticanalysis.UnnecessaryCloseInTryWithResources + # - org.openrewrite.staticanalysis.UnnecessaryExplicitTypeArguments + # - org.openrewrite.staticanalysis.UnnecessaryParentheses + # - org.openrewrite.staticanalysis.UnnecessaryReturnAsLastStatement + # - tech.picnic.errorprone.refasterrules.BigDecimalRulesRecipes + # - tech.picnic.errorprone.refasterrules.CharSequenceRulesRecipes + # - tech.picnic.errorprone.refasterrules.ClassRulesRecipes + # - tech.picnic.errorprone.refasterrules.CollectionRulesRecipes + # - tech.picnic.errorprone.refasterrules.ComparatorRulesRecipes + # - tech.picnic.errorprone.refasterrules.EqualityRulesRecipes + # - tech.picnic.errorprone.refasterrules.FileRulesRecipes + # - tech.picnic.errorprone.refasterrules.MapRulesRecipes + # - tech.picnic.errorprone.refasterrules.MicrometerRulesRecipes + # - tech.picnic.errorprone.refasterrules.MockitoRulesRecipes + # - tech.picnic.errorprone.refasterrules.NullRulesRecipes + # - tech.picnic.errorprone.refasterrules.OptionalRulesRecipes + # - tech.picnic.errorprone.refasterrules.PatternRulesRecipes + # - tech.picnic.errorprone.refasterrules.PreconditionsRulesRecipes + # - tech.picnic.errorprone.refasterrules.PrimitiveRulesRecipes + # - tech.picnic.errorprone.refasterrules.StreamRulesRecipes + # - tech.picnic.errorprone.refasterrules.StringRulesRecipes + # - tech.picnic.errorprone.refasterrules.TimeRulesRecipes +--- diff --git a/test/fixtures/azure-fixture/build.gradle b/test/fixtures/azure-fixture/build.gradle index 904297a3b4c65..7b04f39f2be48 100644 --- a/test/fixtures/azure-fixture/build.gradle +++ b/test/fixtures/azure-fixture/build.gradle @@ -41,8 +41,8 @@ dependencies { preProcessFixture { dependsOn jar, configurations.runtimeClasspath // always run the task, otherwise the folders won't be created - outputs.upToDateWhen { - false + outputs.upToDateWhen { + false } doLast { file("${testFixturesDir}/shared").mkdirs() diff --git a/test/fixtures/gcs-fixture/build.gradle b/test/fixtures/gcs-fixture/build.gradle index 60f672e6bd00b..153bfeb21ee2d 100644 --- a/test/fixtures/gcs-fixture/build.gradle +++ b/test/fixtures/gcs-fixture/build.gradle @@ -41,8 +41,8 @@ group = 'fixture.gcs' preProcessFixture { dependsOn jar, configurations.runtimeClasspath // always run the task, otherwise the folders won't be created - outputs.upToDateWhen { - false + outputs.upToDateWhen { + false } doLast { file("${testFixturesDir}/shared").mkdirs() diff --git a/test/fixtures/s3-fixture/build.gradle b/test/fixtures/s3-fixture/build.gradle index 519e8514af4d4..9b0dcbb52f812 100644 --- a/test/fixtures/s3-fixture/build.gradle +++ b/test/fixtures/s3-fixture/build.gradle @@ -41,8 +41,8 @@ dependencies { preProcessFixture { dependsOn jar, configurations.runtimeClasspath // always run the task, otherwise the folders won't be created - outputs.upToDateWhen { - false + outputs.upToDateWhen { + false } doLast { file("${testFixturesDir}/shared").mkdirs()