build: upgrade appsmith to jdk 25 and spring boot 3.5.11#41644
build: upgrade appsmith to jdk 25 and spring boot 3.5.11#41644sondermanish wants to merge 2 commits intoreleasefrom
Conversation
sondermanish
requested review from
a team,
NilanshBansal,
abhvsn and
sharat87
as code owners
WalkthroughRefactors boolean property/accessor names (e.g., Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/server/appsmith-plugins/mssqlPlugin/pom.xml (1)
19-23:⚠️ Potential issue | 🟠 MajorUpgrade mssql-jdbc to 13.4.0.jre11 for Java 25 compatibility.
Version 11.2.1.jre17 supports only up to Java 18 and is incompatible with Java 25. Update to 13.4.0.jre11, which supports Java 25 and is Microsoft's recommended driver for Java 11 and higher runtimes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/appsmith-plugins/mssqlPlugin/pom.xml` around lines 19 - 23, Update the Maven dependency for com.microsoft.sqlserver:mssql-jdbc in the pom.xml by changing the <version> value from 11.2.1.jre17 to 13.4.0.jre11 so the mssql-jdbc artifactId is upgraded to a driver that supports Java 25; ensure the change is applied in the dependency block that declares groupId com.microsoft.sqlserver and artifactId mssql-jdbc and run a build to verify compatibility.app/server/build.sh (1)
5-12:⚠️ Potential issue | 🟠 MajorEnforce Java 25 consistently in the preflight check.
Line 5 declares Java 25 as minimum, but Lines 10-11 still allow Java 17. This weakens the fast-fail check and can defer failure to Maven compile time.
Suggested fix
min_java_major_version=25 @@ -if [[ "$maven_version_output" != *"Java version: 17."* ]] && [[ "$maven_version_output" != *"Java version: 25."* ]]; then - echo $'\n'"Maven is not using Java 17 or 25. Please install a supported Java version." >&2 +if [[ ! "$maven_version_output" =~ Java\ version:\ ${min_java_major_version}([.,[:space:]]|$) ]]; then + echo $'\n'"Maven is not using Java ${min_java_major_version}. Please install a supported Java version." >&2 exit 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/build.sh` around lines 5 - 12, The preflight check declares min_java_major_version=25 but the conditional still allows Java 17; update the check around maven_version_output (the conditional that currently tests for "Java version: 17." and "Java version: 25.") to enforce only Java 25 (or compare against min_java_major_version), i.e., remove the "Java version: 17." branch and ensure the script fails unless maven_version_output contains "Java version: 25." so the fast-fail matches the min_java_major_version variable.
🧹 Nitpick comments (6)
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/InstanceConfigHelperCEImpl.java (1)
220-220: Fix typo in log message.Minor typo: "conenncted" should be "connected".
📝 Proposed fix
- log.info("Fetched and set conenncted mongo db version as: {}", commonConfig.getMongoDBVersion()); + log.info("Fetched and set connected mongo db version as: {}", commonConfig.getMongoDBVersion());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/InstanceConfigHelperCEImpl.java` at line 220, The log message in InstanceConfigHelperCEImpl contains a typo: update the log.info call that currently reads "Fetched and set conenncted mongo db version as: {}" to use the correct spelling "connected" (i.e., "Fetched and set connected mongo db version as: {}") so the message is accurate when logging commonConfig.getMongoDBVersion().app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/JsonSchemaMigration.java (2)
159-172: Consider placing this comment insidecase 9:for clarity.Positioning a large comment block between switch cases is unconventional and may create ambiguity about which case it documents. Since it explains why version 9 is empty, placing it inside
case 9:would improve readability.📝 Suggested placement
case 8: MigrationHelperMethods.migrateThemeSettingsForAnvil(applicationJson); applicationJson.setServerSchemaVersion(9); - // In Server version 9, there was a bug where the Embedded REST API datasource URL - // was not being persisted correctly. Once the bug was fixed, - // any previously uncommitted changes started appearing as uncommitted modifications - // in the apps. To automatically commit these changes - // (which were now appearing as uncommitted), a migration process was needed. - // This migration fetches the datasource URL from the database - // and serializes it in Git if the URL exists. - // If the URL is missing, it copies the empty datasource configuration - // if the configuration is present in the database. - // Otherwise, it leaves the configuration unchanged. - // Due to an update in the migration logic after version 10 was shipped, - // the entire migration process was moved to version 11. - // This adjustment ensures that the same operation can be - // performed again for the changes introduced in version 10. case 9: + // In Server version 9, there was a bug where the Embedded REST API datasource URL + // was not being persisted correctly. Once the bug was fixed, + // any previously uncommitted changes started appearing as uncommitted modifications + // in the apps. To automatically commit these changes + // (which were now appearing as uncommitted), a migration process was needed. + // This migration fetches the datasource URL from the database + // and serializes it in Git if the URL exists. + // If the URL is missing, it copies the empty datasource configuration + // if the configuration is present in the database. + // Otherwise, it leaves the configuration unchanged. + // Due to an update in the migration logic after version 10 was shipped, + // the entire migration process was moved to version 11. + // This adjustment ensures that the same operation can be + // performed again for the changes introduced in version 10. applicationJson.setServerSchemaVersion(10);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/JsonSchemaMigration.java` around lines 159 - 172, Move the large explanatory comment block into the switch branch for version 9 to avoid ambiguity: locate the switch in JsonSchemaMigration and place the comment inside the `case 9:` block (near the start of that case) so it clearly documents why `case 9` is empty; preserve the existing text and formatting but relocate it from between switch cases into the `case 9:` body.
169-172: Clarify the migration logic distribution.The comment states "the entire migration process was moved to version 11," but case 10 (lines 176-180) contains REST API datasource migration logic. Consider clarifying whether the migration is split across cases 10 and 11, or if they address different aspects of the same issue.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/JsonSchemaMigration.java` around lines 169 - 172, The comment in JsonSchemaMigration is ambiguous about where migration logic resides; update the comment near the switch handling migration cases to clearly state that case 10 contains the REST API datasource migration logic while case 11 contains the moved/expanded "entire migration process" (or that case 11 completes/re-applies changes introduced in version 10), so readers know the migration is split across case 10 (REST API datasource changes) and case 11 (the relocated/complete process) rather than implying all logic was moved entirely to case 11.app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ResendEmailVerificationDTO.java (2)
13-13: Consider moving annotation to a separate line.The
@NotEmptyannotation was consolidated onto the same line as the field declaration. While syntactically correct, Java conventions typically place annotations on separate lines for better readability.📝 Conventional formatting
- `@NotEmpty` String token; + `@NotEmpty` + String token;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ResendEmailVerificationDTO.java` at line 13, Move the `@NotEmpty` annotation to its own line above the field declaration in the ResendEmailVerificationDTO class so the annotation is not on the same line as the field (i.e., place `@NotEmpty` on a separate line immediately before the declaration of the token field) to follow conventional Java formatting and improve readability.
11-11: Add validation to thebaseUrlfield.The
baseUrlfield lacks validation constraints. For a resend email verification flow, the base URL is likely required and should be validated similarly to thetokenfield.🛡️ Add validation constraint
- String baseUrl; + `@NotEmpty` + String baseUrl;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ResendEmailVerificationDTO.java` at line 11, The baseUrl field in ResendEmailVerificationDTO lacks validation; add the same required constraint used on the token field (e.g., annotate the baseUrl property with `@NotBlank` or equivalent javax.validation constraint) and optionally add a URL-format validator (e.g., Hibernate Validator's `@URL`) to ensure it's present and well-formed; update the ResendEmailVerificationDTO class (field baseUrl) accordingly so validation runs during the resend email verification flow.app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/BodyReceiver.java (1)
25-25: Import may become unused if switching toCollections.emptyMap().If you adopt the
Collections.emptyMap()suggestion above, this import can be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/BodyReceiver.java` at line 25, The import java.util.HashMap is likely unused if you switch to Collections.emptyMap(); update the BodyReceiver class to remove the HashMap import and ensure you import or reference java.util.Collections instead (or no import if using fully-qualified Collections.emptyMap()), so that unused-import warnings are eliminated and the code compiles cleanly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/BodyReceiver.java`:
- Around line 163-166: The getAttributes() implementation in BodyReceiver
returns an immutable/empty map which violates Spring Framework 6's
ClientHttpRequest.getAttributes() contract; update BodyReceiver.getAttributes()
to return a mutable Map instance instead (either by returning a new HashMap<>
each call or by adding a private final Map<String,Object> attributes = new
HashMap<>() field and returning that cached map) so callers can mutate
attributes as expected.
In
`@app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/utils/WhereConditionUtils.java`:
- Around line 94-96: The commented NOT_EQ and NOT_IN cases in
WhereConditionUtils are outdated and incorrectly implemented; restore them
inside the method that builds query conditions (the switch handling operators)
but change the calls to use fieldPath (not path), remove the erroneous
Mono.just(...) wrapping so the branch returns a Query like the others, and
implement NOT_IN to call query.whereNotIn(fieldPath, parseList((String) value))
while NOT_EQ should call query.whereNotEqualTo(fieldPath, value).
In
`@app/server/appsmith-plugins/saasPlugin/src/main/java/com/external/helpers/BodyReceiver.java`:
- Around line 133-136: The getAttributes() implementation in BodyReceiver
currently returns a new HashMap on every call which violates Spring's
HttpRequest#getAttributes() contract; change BodyReceiver to hold a persistent
mutable Map instance (e.g., a private final Map<String,Object> attributes field
initialized once per BodyReceiver instance) and have getAttributes() return that
field so repeated calls within the same request return the same map; ensure the
field is not static so it remains per-request and mutable for other components
to share.
In `@app/server/appsmith-server/pom.xml`:
- Around line 471-477: The pom currently restricts annotation processors via
annotationProcessorPaths but doesn't include the JMH processor; since you have
jmh-core and jmh-generator-annprocess declared, either add the
jmh-generator-annprocess artifact to the annotationProcessorPaths block (so the
JMH annotation processor runs) or remove the unused jmh-core and
jmh-generator-annprocess dependencies entirely if there are no `@Benchmark`
annotations in the module; update the pom to perform one of these two actions
and ensure the identifiers referenced are jmh-generator-annprocess, jmh-core,
annotationProcessorPaths and any `@Benchmark` usages when deciding which option to
apply.
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/ReleaseNotesUtilsCEImpl.java`:
- Line 54: The expression in ReleaseNotesUtilsCEImpl that currently uses
commonConfig.getIsCloudHosting() directly can NPE if null; update the boolean
condition in the release-notes query construction to perform a null-safe check
(e.g., use Boolean.TRUE.equals(commonConfig.getIsCloudHosting()) or
org.apache.commons.lang3.BooleanUtils.isTrue(commonConfig.getIsCloudHosting()))
while keeping the existing StringUtils.isEmpty(segmentConfig.getCeKey()) check,
so the combined clause is null-safe and preserves the original logic.
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitCloudServicesUtils.java`:
- Line 36: The null-unsafe check if (commonConfig.getIsCloudHosting()) can NPE
due to Boolean auto-unboxing; replace it with a null-safe Boolean comparison
such as using Boolean.TRUE.equals(commonConfig.getIsCloudHosting()) in
GitCloudServicesUtils (where getIsCloudHosting() is invoked) so the condition is
safe when the config value is null.
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.java`:
- Around line 228-262: The added INFO-level diagnostic logs in
OrganizationServiceCEImpl (the flatMap block that logs organization details,
iterates organization.getPolicies() and logs each policy including
policy.getPermissionGroups(), and logs after
repository.setUserPermissionsInObject(organization)) must be removed or
downgraded to DEBUG/TRACE before merging: either delete the verbose log
statements (including the forEach over organization.getPolicies()) or change
their logger level to DEBUG/TRACE and avoid printing sensitive fields like
permissionGroups; keep only minimal, non-sensitive logs around
repository.setUserPermissionsInObject(organization) if necessary.
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/AuthenticationServiceCEImpl.java`:
- Around line 251-255: The HttpClient created in AuthenticationServiceCEImpl is
not reassigned after calling httpClient.secure(...), so the unsecured instance
is still used when oAuth2.getUseSelfSignedCert() is true; modify the code to
capture the returned secured client (e.g., replace the final HttpClient
httpClient = HttpClient.create() with a mutable variable or assign the result to
a new variable) and set that variable to the result of
httpClient.secure(SSLHelper.sslCheckForHttpClient(...)) so the secured
HttpClient is actually used downstream.
In
`@app/server/appsmith-server/src/test/java/com/appsmith/server/git/autocommit/helpers/AutoCommitEligibilityHelperTest.java`:
- Around line 373-375: The test discards the configured ModifiedResources by
creating a new instance instead of using the one with isAllModified(true);
replace the call to applicationJson.setModifiedResources(new
ModifiedResources()) so it uses the configured variable (modifiedResources) set
earlier; locate the ModifiedResources variable and the
applicationJson.setModifiedResources(...) invocation in
AutoCommitEligibilityHelperTest and pass the configured modifiedResources object
so the isAllModified flag is preserved.
In `@app/server/lombok.config`:
- Line 2: Lombok is configured with lombok.getter.noIsPrefix = true which makes
boolean fields generate getX() accessors instead of isX(), so ensure every
boolean field in DTO classes has an explicit `@JsonProperty` mapping to the
expected "is..." JSON name; search DTOs for boolean fields (e.g., isAnonymous,
isSuperUser, isSystemTheme) and add
`@JsonProperty`("isAnonymous")/@JsonProperty("isSuperUser")/@JsonProperty("isSystemTheme")
(on the field or the generated getter) to preserve the original serialized names
and avoid deserialization mismatches.
In `@app/server/pom.xml`:
- Line 32: Update the Mockito dependency to a Mockito 5.x release that supports
Java 25 (minimum 5.20.0) to avoid Byte Buddy/classfile-version incompatibility;
locate any Maven dependency entries with groupId/artifactId like
org.mockito:mockito-core (and org.mockito:mockito-inline or other mockito
artifacts) and bump their <version> to 5.20.0 or later, and also update any
direct Byte Buddy dependency if present to a compatible version; run a quick mvn
-DskipTests=false test to verify the build under Java 25.
In `@app/server/system.properties`:
- Line 1: The IDE run configuration still points to Java 17; update the
ALTERNATIVE_JRE_PATH entry in the run configuration (ServerApplication.run.xml)
to reference the Java 25 JDK (e.g., change any "temurin-17" value to the
appropriate "temurin-25" or your local Java 25 install path) so it matches
java.runtime.version=25 and the pom.xml; save the .run XML so local IDE runs use
Java 25.
---
Outside diff comments:
In `@app/server/appsmith-plugins/mssqlPlugin/pom.xml`:
- Around line 19-23: Update the Maven dependency for
com.microsoft.sqlserver:mssql-jdbc in the pom.xml by changing the <version>
value from 11.2.1.jre17 to 13.4.0.jre11 so the mssql-jdbc artifactId is upgraded
to a driver that supports Java 25; ensure the change is applied in the
dependency block that declares groupId com.microsoft.sqlserver and artifactId
mssql-jdbc and run a build to verify compatibility.
In `@app/server/build.sh`:
- Around line 5-12: The preflight check declares min_java_major_version=25 but
the conditional still allows Java 17; update the check around
maven_version_output (the conditional that currently tests for "Java version:
17." and "Java version: 25.") to enforce only Java 25 (or compare against
min_java_major_version), i.e., remove the "Java version: 17." branch and ensure
the script fails unless maven_version_output contains "Java version: 25." so the
fast-fail matches the min_java_major_version variable.
---
Nitpick comments:
In
`@app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/BodyReceiver.java`:
- Line 25: The import java.util.HashMap is likely unused if you switch to
Collections.emptyMap(); update the BodyReceiver class to remove the HashMap
import and ensure you import or reference java.util.Collections instead (or no
import if using fully-qualified Collections.emptyMap()), so that unused-import
warnings are eliminated and the code compiles cleanly.
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ResendEmailVerificationDTO.java`:
- Line 13: Move the `@NotEmpty` annotation to its own line above the field
declaration in the ResendEmailVerificationDTO class so the annotation is not on
the same line as the field (i.e., place `@NotEmpty` on a separate line immediately
before the declaration of the token field) to follow conventional Java
formatting and improve readability.
- Line 11: The baseUrl field in ResendEmailVerificationDTO lacks validation; add
the same required constraint used on the token field (e.g., annotate the baseUrl
property with `@NotBlank` or equivalent javax.validation constraint) and
optionally add a URL-format validator (e.g., Hibernate Validator's `@URL`) to
ensure it's present and well-formed; update the ResendEmailVerificationDTO class
(field baseUrl) accordingly so validation runs during the resend email
verification flow.
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/InstanceConfigHelperCEImpl.java`:
- Line 220: The log message in InstanceConfigHelperCEImpl contains a typo:
update the log.info call that currently reads "Fetched and set conenncted mongo
db version as: {}" to use the correct spelling "connected" (i.e., "Fetched and
set connected mongo db version as: {}") so the message is accurate when logging
commonConfig.getMongoDBVersion().
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/JsonSchemaMigration.java`:
- Around line 159-172: Move the large explanatory comment block into the switch
branch for version 9 to avoid ambiguity: locate the switch in
JsonSchemaMigration and place the comment inside the `case 9:` block (near the
start of that case) so it clearly documents why `case 9` is empty; preserve the
existing text and formatting but relocate it from between switch cases into the
`case 9:` body.
- Around line 169-172: The comment in JsonSchemaMigration is ambiguous about
where migration logic resides; update the comment near the switch handling
migration cases to clearly state that case 10 contains the REST API datasource
migration logic while case 11 contains the moved/expanded "entire migration
process" (or that case 11 completes/re-applies changes introduced in version
10), so readers know the migration is split across case 10 (REST API datasource
changes) and case 11 (the relocated/complete process) rather than implying all
logic was moved entirely to case 11.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 22ac4318-128c-49e7-822d-d84f51b0ad24
📒 Files selected for processing (134)
Dockerfileapp/client/cypress/fixtures/user.jsonapp/client/src/ce/sagas/userSagas.tsxapp/client/src/ce/workers/Evaluation/__tests__/dataTreeUtils.test.tsapp/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.tsapp/client/src/constants/userConstants.tsapp/client/src/entities/DependencyMap/__tests__/index.test.tsapp/client/src/layoutSystems/common/dropTarget/unitTestUtils.tsapp/client/src/pages/UserAuth/Login.tsxapp/client/src/pages/setup/index.tsxapp/client/src/reducers/uiReducers/usersReducer.tsapp/client/src/workers/Evaluation/__tests__/evaluate.test.tsapp/client/src/workers/common/DataTreeEvaluator/mockData/mockUnEvalTree.tsapp/server/appsmith-git/pom.xmlapp/server/appsmith-interfaces/pom.xmlapp/server/appsmith-interfaces/src/main/java/com/appsmith/external/annotations/documenttype/DocumentType.javaapp/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/PluginConstants.javaapp/server/appsmith-interfaces/src/main/java/com/appsmith/external/dtos/GitBranchDTO.javaapp/server/appsmith-interfaces/src/main/java/com/appsmith/external/dtos/GitBranchListDTO.javaapp/server/appsmith-interfaces/src/main/java/com/appsmith/external/dtos/GitRefDTO.javaapp/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/connections/APIConnection.javaapp/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/connections/OAuth2AuthorizationCode.javaapp/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/connections/OAuth2ClientCredentials.javaapp/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/BodyReceiver.javaapp/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/DatasourceStructure.javaapp/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/UpdatableConnection.javaapp/server/appsmith-interfaces/src/test/java/com/appsmith/external/dtos/ModifiedResourcesTest.javaapp/server/appsmith-plugins/amazons3Plugin/pom.xmlapp/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/AmazonS3Plugin.javaapp/server/appsmith-plugins/anthropicPlugin/pom.xmlapp/server/appsmith-plugins/anthropicPlugin/src/main/java/com/external/plugins/utils/AnthropicMethodStrategy.javaapp/server/appsmith-plugins/appsmithAiPlugin/pom.xmlapp/server/appsmith-plugins/appsmithAiPlugin/src/main/java/com/external/plugins/services/TriggerServiceCEImpl.javaapp/server/appsmith-plugins/arangoDBPlugin/pom.xmlapp/server/appsmith-plugins/awsLambdaPlugin/pom.xmlapp/server/appsmith-plugins/awsLambdaPlugin/src/main/java/com/external/plugins/AwsLambdaPlugin.javaapp/server/appsmith-plugins/databricksPlugin/pom.xmlapp/server/appsmith-plugins/dynamoPlugin/pom.xmlapp/server/appsmith-plugins/elasticSearchPlugin/pom.xmlapp/server/appsmith-plugins/firestorePlugin/pom.xmlapp/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/utils/WhereConditionUtils.javaapp/server/appsmith-plugins/googleAiPlugin/pom.xmlapp/server/appsmith-plugins/googleAiPlugin/src/main/java/com/external/plugins/utils/GoogleAIMethodStrategy.javaapp/server/appsmith-plugins/googleSheetsPlugin/pom.xmlapp/server/appsmith-plugins/graphqlPlugin/pom.xmlapp/server/appsmith-plugins/jsPlugin/pom.xmlapp/server/appsmith-plugins/mongoPlugin/pom.xmlapp/server/appsmith-plugins/mssqlPlugin/pom.xmlapp/server/appsmith-plugins/mysqlPlugin/pom.xmlapp/server/appsmith-plugins/openAiPlugin/pom.xmlapp/server/appsmith-plugins/openAiPlugin/src/main/java/com/external/plugins/utils/OpenAIMethodStrategy.javaapp/server/appsmith-plugins/oraclePlugin/pom.xmlapp/server/appsmith-plugins/pom.xmlapp/server/appsmith-plugins/postgresPlugin/pom.xmlapp/server/appsmith-plugins/redisPlugin/pom.xmlapp/server/appsmith-plugins/redshiftPlugin/pom.xmlapp/server/appsmith-plugins/restApiPlugin/pom.xmlapp/server/appsmith-plugins/saasPlugin/pom.xmlapp/server/appsmith-plugins/saasPlugin/src/main/java/com/external/helpers/BodyReceiver.javaapp/server/appsmith-plugins/smtpPlugin/pom.xmlapp/server/appsmith-plugins/snowflakePlugin/pom.xmlapp/server/appsmith-server/pom.xmlapp/server/appsmith-server/src/main/java/com/appsmith/server/actioncollections/exportable/ActionCollectionExportableServiceCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/applications/exports/ApplicationExportServiceCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/artifacts/base/ArtifactServiceCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/artifacts/gitRoute/GitRouteArtifactCE.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SegmentConfig.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/configurations/mongo/SoftDeleteMongoQueryLookupStrategy.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/configurations/mongo/SoftDeleteMongoRepositoryFactory.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/configurations/mongo/SoftDeletePartTreeMongoQuery.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/domains/Plugin.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/domains/Workspace.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ApplicationTemplate.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ResendEmailVerificationDTO.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ResetUserPasswordDTO.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/dtos/TestEmailConfigRequestDTO.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/dtos/UpdateIsReadNotificationByIdDTO.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/dtos/UpdateMultiplePageLayoutDTO.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ce/UserUpdateCE_DTO.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/util/ObservabilityLogger.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/git/autocommit/AutoCommitSolutionCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/git/common/CommonGitServiceCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitCloudServicesUtils.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/helpers/RunBehaviourAnalyticsUtils.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/InstanceConfigHelperCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/ReleaseNotesUtilsCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/UserUtilsCE.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/migrations/JsonSchemaMigration.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/migrations/MigrationHelperMethods.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration007OptOutUnsupportedPluginsForAirGap.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration035RemoveMockDbEndPointInDatasourceInSelfHostedInstance.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/newactions/exportable/NewActionExportableServiceCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/newpages/exportable/NewPageExportableServiceCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/notifications/EmailSender.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/BaseAppsmithRepositoryCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCE.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AnalyticsServiceCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CacheableFeatureFlagHelperCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UsagePulseServiceCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserIdentifierServiceCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/AuthenticationServiceCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PluginScheduledTaskCE.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ScheduledTaskCEImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserSignupCEImpl.javaapp/server/appsmith-server/src/test/it/com/appsmith/server/git/GitBranchesIT.javaapp/server/appsmith-server/src/test/it/com/appsmith/server/git/GitBranchesWithCentralServiceIT.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/git/CommonGitServiceCETest.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/git/FSGitHandlerTest.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/git/GitExecutorTest.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/git/ServerSchemaMigrationEnforcerTest.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/git/autocommit/helpers/AutoCommitEligibilityHelperTest.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/ImportArtifactPermissionProviderTest.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/notifications/EmailSenderTest.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/services/UsagePulseServiceTest.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ApplicationServiceCETest.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ProductAlertServiceCEImplTest.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/UserIdentifierServiceCEImplTest.javaapp/server/build.shapp/server/lombok.configapp/server/pom.xmlapp/server/reactive-caching/pom.xmlapp/server/system.propertiescontributions/ServerCodeContributionsGuidelines/PluginCodeContributionsGuidelines.mdcontributions/ServerSetup.mddeploy/docker/base.dockerfile
| @Override | ||
| public Map<String, Object> getAttributes() { | ||
| return new HashMap<>(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Spring Framework 6 ClientHttpRequest getAttributes method contract mutable map
💡 Result:
In Spring Framework 6, the org.springframework.http.client.reactive.ClientHttpRequest interface (in spring-webflux module) includes a getAttributes method that returns a mutable Map<String, Object> of request attributes. This was added in Spring Framework 6.2 to support propagating attributes from WebClient requests to the underlying HTTP client libraries (e.g., Jetty, Reactor Netty, Apache HttpComponents) via an applyAttributes method in AbstractClientHttpRequest implementations. The map is mutable, allowing modifications before the request is committed, typically for passing contextual data through filter chains or to native requests. The synchronous org.springframework.http.client.ClientHttpRequest (for RestTemplate/RestClient) gained getAttributes in later versions (visible in 7.0+ docs), returning a mutable map as well. Official docs for 6.1.x may not show it due to version-specific builds, but third-party Javadocs (6.2.14) and GitHub PR #29958 confirm its presence and mutability in the reactive ClientHttpRequest for SF 6.x.
Citations:
- 1: https://javadoc.io/static/org.springframework/spring-web/6.2.14/org/springframework/http/client/reactive/ClientHttpRequest.html
- 2: https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/http/client/reactive/AbstractClientHttpRequest.html
- 3: Propagate WebClient attributes into underlying HTTP client request where possible spring-projects/spring-framework#29958
- 4: https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/http/client/AbstractClientHttpRequest.html
🏁 Script executed:
# Check the file to understand class hierarchy and current implementation
cat -n app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/BodyReceiver.java | head -170Repository: appsmithorg/appsmith
Length of output: 6701
🏁 Script executed:
# Search for usages of getAttributes() method from MinimalHttpOutputMessage or BodyReceiver
rg "getAttributes" --type java -B 2 -A 2Repository: appsmithorg/appsmith
Length of output: 7954
🏁 Script executed:
# Check what Spring Boot version is referenced in pom.xml or gradle files
fd "pom.xml" -o fd "build.gradle" | head -5 | xargs grep -l "spring.boot\|spring-boot" 2>/dev/null || echo "Searching for version..."
find . -name "pom.xml" -o -name "build.gradle" | head -1 | xargs cat 2>/dev/null | grep -i "spring.*boot\|version" | head -20Repository: appsmithorg/appsmith
Length of output: 729
The returned map must be mutable per Spring Framework 6 contract.
Collections.emptyMap() violates the Spring Framework 6 ClientHttpRequest.getAttributes() contract, which requires a mutable map for potential attribute propagation. Since this is a singleton, if optimization is desired, cache a single HashMap instance instead of allocating a new one on each invocation:
private final Map<String, Object> attributes = new HashMap<>();
`@Override`
public Map<String, Object> getAttributes() {
return attributes;
}Otherwise, the current implementation is acceptable, though it does create unnecessary allocations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/BodyReceiver.java`
around lines 163 - 166, The getAttributes() implementation in BodyReceiver
returns an immutable/empty map which violates Spring Framework 6's
ClientHttpRequest.getAttributes() contract; update BodyReceiver.getAttributes()
to return a mutable Map instance instead (either by returning a new HashMap<>
each call or by adding a private final Map<String,Object> attributes = new
HashMap<>() field and returning that cached map) so callers can mutate
attributes as expected.
| // TODO: NOT_EQ operator support is awaited in the next version of Firestore driver. | ||
| // case NOT_EQ: | ||
| // return Mono.just(query.whereNotEqualTo(path, value)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the Firestore driver version in Maven dependencies
fd -e xml pom.xml --exec grep -H "google-cloud-firestore" {} \;Repository: appsmithorg/appsmith
Length of output: 46
🏁 Script executed:
# Find the pom.xml file for the firestorePlugin module
fd pom.xml | grep -i firestoreRepository: appsmithorg/appsmith
Length of output: 115
🏁 Script executed:
# Also check the directory structure around the file under review
ls -la app/server/appsmith-plugins/firestorePlugin/ 2>/dev/null || echo "Direct path may vary, searching..."
fd -type d firestorePluginRepository: appsmithorg/appsmith
Length of output: 476
🏁 Script executed:
# Read the pom.xml to find Firestore driver version
cat -n app/server/appsmith-plugins/firestorePlugin/pom.xmlRepository: appsmithorg/appsmith
Length of output: 3477
🏁 Script executed:
# Also check the full context of WhereConditionUtils.java around the commented code
cat -n app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/utils/WhereConditionUtils.java | head -150Repository: appsmithorg/appsmith
Length of output: 6596
🌐 Web query:
firebase-admin 9.2.0 release notes NOT_EQ NOT_IN operators firestore
💡 Result:
The Firebase Admin Node.js SDK version 9.2.0 was released on September 15, 2020. Its release notes mention only additions for Firebase ML model management in Realtime Database and Cloud Messaging improvements. No changes related to Firestore NOT_EQ (!=) or NOT_IN operators are listed. Firestore added support for != (NOT_EQUAL) and not-in (NOT_IN) query operators in September 2020, as announced in the Firebase blog post on September 18, 2020. This was implemented in the @google-cloud/firestore package (used by firebase-admin) around version 4.x (commit on Sep 8, 2020), and client SDKs like JavaScript v7.21.0 / v8+. Official docs confirm Node.js server SDKs (including Admin SDK) support these operators via .where('field', '!=', value) and .where('field', 'not-in', [values]). An open feature request in firebase-admin-node repo (Issue #1131, Jan 2021) suggests explicit Admin SDK support may have lagged, but since firebase-admin re-exports @google-cloud/firestore, the feature was available by v9.2.0 via dependency upgrade. Example usage in Admin SDK: const admin = require('firebase-admin'); const db = admin.firestore; db.collection('cities').where('country', '!=', 'USA').get; db.collection('cities').where('country', 'not-in', ['USA', 'Japan']).get; Note: These operators do not match documents missing the field, require indexes for compound queries, and have inequality limitations.
Citations:
- 1: https://firebase.google.com/support/release-notes/admin/node
- 2: https://firebase-dot-devsite-v2-prod.appspot.com/support/release-notes/admin/node
- 3: https://firebase.blog/posts/2020/09/cloud-firestore-not-equal-queries/
- 4: https://cloud.google.com/firestore/docs/samples/firestore-query-filter-not-in
- 5: googleapis/nodejs-firestore@786e52f
- 6: [FR] != and not-in query operators firebase/firebase-admin-node#1131
- 7: not-in operator is not accepted in firestore collection query googleapis/nodejs-firestore#1345
🌐 Web query:
google-cloud-firestore whereNotEqualTo whereNotIn support version
💡 Result:
Google Cloud Firestore supports whereNotEqualTo (!=) and whereNotIn query operators in all client SDKs (Android/Java/Kotlin, iOS/Swift, Web/JavaScript, etc.) and server SDKs. These were added in September 2020 (Firestore release notes). They are treated as inequality filters, requiring the filtered field to be the first in any orderBy clause and have limitations like max 10 values for not-in, cannot combine not-in with != in Standard edition. Examples: - Android/Java/Kotlin: query.whereNotEqualTo("field", value); query.whereNotIn("field", list); - Web v9: where("field", "!=", value); where("field", "not-in", array); - Server Java: citiesRef.whereNotIn("country", Arrays.asList("USA", "Japan")); As of 2026-03-20, fully supported and documented in current docs. Does not match documents missing the field.
Citations:
- 1: https://firebase.google.com/docs/firestore/query-data/queries
- 2: https://docs.cloud.google.com/firestore/native/docs/query-data/queries
- 3: https://docs.cloud.google.com/firestore/docs/release-notes
- 4: https://firebase.blog/posts/2020/09/cloud-firestore-not-equal-queries/
- 5: https://cloud.google.com/firestore/docs/samples/firestore-query-filter-not-in
- 6: https://firebase.google.com/docs/reference/kotlin/com/google/firebase/firestore/Query
- 7: https://stackoverflow.com/questions/65471951/wherenotequalto-in-firestore-doesnt-work-but-whereequalto-does
Uncomment NOT_EQ and NOT_IN operators — but fix the code first.
The Firestore driver (firebase-admin 9.2.0) supports whereNotEqualTo() and whereNotIn() operators since September 2020, so the TODO comments are outdated. However, the commented code has defects:
- Line 96: Uses
pathinstead offieldPath, and wraps result inMono.just()(incorrect return type) - Line 123: Wraps result in
Mono.just()(all other operators returnQuerydirectly)
Correct implementation:
case NOT_EQ:
return query.whereNotEqualTo(fieldPath, value);
case NOT_IN:
return query.whereNotIn(fieldPath, parseList((String) value));
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/utils/WhereConditionUtils.java`
around lines 94 - 96, The commented NOT_EQ and NOT_IN cases in
WhereConditionUtils are outdated and incorrectly implemented; restore them
inside the method that builds query conditions (the switch handling operators)
but change the calls to use fieldPath (not path), remove the erroneous
Mono.just(...) wrapping so the branch returns a Query like the others, and
implement NOT_IN to call query.whereNotIn(fieldPath, parseList((String) value))
while NOT_EQ should call query.whereNotEqualTo(fieldPath, value).
| @Override | ||
| public Map<String, Object> getAttributes() { | ||
| return new HashMap<>(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Spring Framework used by Spring Boot 3.5.x, what is the expected contract of ClientHttpRequest#getAttributes() (or related request attributes API)? Should repeated calls return the same mutable map instance tied to that request object?
💡 Result:
For Spring Framework 6.2+ (used by Spring Boot 3.5.x), the expected contract of ClientHttpRequest#getAttributes—inherited from HttpRequest#getAttributes—is to return a mutable Map<String, Object> tied to that specific request instance. Repeated calls to getAttributes on the same ClientHttpRequest object return the same mutable map instance. This is evidenced by the official API documentation stating "Return a mutable map of request attributes for this request" and implementations such as AbstractClientHttpRequest and InterceptingClientHttpRequest, which propagate and use a consistent attributes map per request (e.g., request.getAttributes.forEach((key, value) -> delegate.getAttributes.put(key, value))). This design allows modifications to the attributes to be visible across interceptors and components processing the same request object, consistent with patterns in ServerWebExchange.getAttributes and other Spring HTTP request abstractions.
Citations:
- 1: https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/http/HttpRequest.html
- 2: https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/http/client/AbstractClientHttpRequest.html
- 3: https://github.com/spring-projects/spring-framework/blob/main/spring-web/src/main/java/org/springframework/http/client/InterceptingClientHttpRequest.java
- 4: https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-3.5-Release-Notes
- 5: https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/http/client/reactive/AbstractClientHttpRequest.html
Use a persistent attributes map instance per request.
The getAttributes() method returns a new HashMap on each call, but Spring Framework's contract for HttpRequest#getAttributes() requires repeated calls to return the same mutable map instance tied to the request. This breaks attribute visibility across components within the same request lifecycle.
Store attributes in a persistent instance field and return that instead.
Proposed fix
@@
- inserter.insert(
- MinimalHttpOutputMessage.INSTANCE, new SingleWriterContext(new WriteToConsumer<>(reference::set)));
+ inserter.insert(
+ new MinimalHttpOutputMessage(), new SingleWriterContext(new WriteToConsumer<>(reference::set)));
@@
- public static final MinimalHttpOutputMessage INSTANCE = new MinimalHttpOutputMessage();
+ private final Map<String, Object> attributes = new HashMap<>();
@@
`@Override`
public Map<String, Object> getAttributes() {
- return new HashMap<>();
+ return attributes;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/server/appsmith-plugins/saasPlugin/src/main/java/com/external/helpers/BodyReceiver.java`
around lines 133 - 136, The getAttributes() implementation in BodyReceiver
currently returns a new HashMap on every call which violates Spring's
HttpRequest#getAttributes() contract; change BodyReceiver to hold a persistent
mutable Map instance (e.g., a private final Map<String,Object> attributes field
initialized once per BodyReceiver instance) and have getAttributes() return that
field so repeated calls within the same request return the same map; ensure the
field is not static so it remains per-request and mutable for other components
to share.
| <annotationProcessorPaths> | ||
| <path> | ||
| <groupId>org.projectlombok</groupId> | ||
| <artifactId>lombok</artifactId> | ||
| <version>${lombok.version}</version> | ||
| </path> | ||
| </annotationProcessorPaths> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --type=java -C2 '@Benchmark|org\.openjdk\.jmh' app/server/appsmith-server -g '!**/target/**'Repository: appsmithorg/appsmith
Length of output: 46
🏁 Script executed:
rg -n "jmh-generator-annprocess" app/server/appsmith-server/pom.xmlRepository: appsmithorg/appsmith
Length of output: 129
🏁 Script executed:
cat app/server/appsmith-server/pom.xml | grep -A3 -B3 "jmh\|openjdk" | head -50Repository: appsmithorg/appsmith
Length of output: 857
The JMH processor path is missing, but the JMH dependency appears unused.
Once annotationProcessorPaths is set, Maven Compiler Plugin only discovers processors from those explicitly listed paths. The existing jmh-generator-annprocess dependency (line 392) won't run unless added here too.
However, no @Benchmark annotations were found in the module, suggesting JMH benchmarks have been removed. Either:
- Add the JMH generator to
annotationProcessorPathsif benchmarks still exist elsewhere, or - Remove the unused
jmh-coreandjmh-generator-annprocessdependencies.
Option 1: Add to processor path
<annotationProcessorPaths>
<path>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<version>${lombok.version}</version>
</path>
+ <path>
+ <groupId>org.openjdk.jmh</groupId>
+ <artifactId>jmh-generator-annprocess</artifactId>
+ <version>${jmh.version}</version>
+ </path>
</annotationProcessorPaths>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <annotationProcessorPaths> | |
| <path> | |
| <groupId>org.projectlombok</groupId> | |
| <artifactId>lombok</artifactId> | |
| <version>${lombok.version}</version> | |
| </path> | |
| </annotationProcessorPaths> | |
| <annotationProcessorPaths> | |
| <path> | |
| <groupId>org.projectlombok</groupId> | |
| <artifactId>lombok</artifactId> | |
| <version>${lombok.version}</version> | |
| </path> | |
| <path> | |
| <groupId>org.openjdk.jmh</groupId> | |
| <artifactId>jmh-generator-annprocess</artifactId> | |
| <version>${jmh.version}</version> | |
| </path> | |
| </annotationProcessorPaths> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/server/appsmith-server/pom.xml` around lines 471 - 477, The pom currently
restricts annotation processors via annotationProcessorPaths but doesn't include
the JMH processor; since you have jmh-core and jmh-generator-annprocess
declared, either add the jmh-generator-annprocess artifact to the
annotationProcessorPaths block (so the JMH annotation processor runs) or remove
the unused jmh-core and jmh-generator-annprocess dependencies entirely if there
are no `@Benchmark` annotations in the module; update the pom to perform one of
these two actions and ensure the identifiers referenced are
jmh-generator-annprocess, jmh-core, annotationProcessorPaths and any `@Benchmark`
usages when deciding which option to apply.
| // present | ||
| "&isSourceInstall=" | ||
| + (commonConfig.isCloudHosting() || StringUtils.isEmpty(segmentConfig.getCeKey())) | ||
| + (commonConfig.getIsCloudHosting() || StringUtils.isEmpty(segmentConfig.getCeKey())) |
There was a problem hiding this comment.
Use a null-safe cloud-hosting check in release-notes query construction.
Line 54 directly evaluates getIsCloudHosting() in a boolean expression, which can NPE if the value is null.
Proposed fix
- + (commonConfig.getIsCloudHosting() || StringUtils.isEmpty(segmentConfig.getCeKey()))
+ + (Boolean.TRUE.equals(commonConfig.getIsCloudHosting())
+ || StringUtils.isEmpty(segmentConfig.getCeKey()))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| + (commonConfig.getIsCloudHosting() || StringUtils.isEmpty(segmentConfig.getCeKey())) | |
| (Boolean.TRUE.equals(commonConfig.getIsCloudHosting()) | |
| || StringUtils.isEmpty(segmentConfig.getCeKey())) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/ReleaseNotesUtilsCEImpl.java`
at line 54, The expression in ReleaseNotesUtilsCEImpl that currently uses
commonConfig.getIsCloudHosting() directly can NPE if null; update the boolean
condition in the release-notes query construction to perform a null-safe check
(e.g., use Boolean.TRUE.equals(commonConfig.getIsCloudHosting()) or
org.apache.commons.lang3.BooleanUtils.isTrue(commonConfig.getIsCloudHosting()))
while keeping the existing StringUtils.isEmpty(segmentConfig.getCeKey()) check,
so the combined clause is null-safe and preserves the original logic.
| final HttpClient httpClient = HttpClient.create(); | ||
|
|
||
| if (oAuth2.isUseSelfSignedCert()) { | ||
| if (oAuth2.getUseSelfSignedCert()) { | ||
| httpClient.secure( | ||
| SSLHelper.sslCheckForHttpClient(datasourceStorage.getDatasourceConfiguration())); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
fd 'AuthenticationServiceCEImpl\.java$' -x sed -n '248,259p' {}
fd 'APIConnection\.java$' -x sed -n '12,20p' {}
rg -n --type java 'httpClient\s*=\s*httpClient\.secure|httpClient\.secure\('Repository: appsmithorg/appsmith
Length of output: 1497
Reassign the secured HttpClient.
Line 254 discards the result of HttpClient.secure(...); Reactor Netty returns a new HttpClient from secure(...), so the unsecured client is still used when useSelfSignedCert is enabled. The same pattern in APIConnection.java correctly reassigns the result.
Proposed fix
- final HttpClient httpClient = HttpClient.create();
+ HttpClient httpClient = HttpClient.create();
if (oAuth2.getUseSelfSignedCert()) {
- httpClient.secure(
+ httpClient = httpClient.secure(
SSLHelper.sslCheckForHttpClient(datasourceStorage.getDatasourceConfiguration()));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final HttpClient httpClient = HttpClient.create(); | |
| if (oAuth2.isUseSelfSignedCert()) { | |
| if (oAuth2.getUseSelfSignedCert()) { | |
| httpClient.secure( | |
| SSLHelper.sslCheckForHttpClient(datasourceStorage.getDatasourceConfiguration())); | |
| HttpClient httpClient = HttpClient.create(); | |
| if (oAuth2.getUseSelfSignedCert()) { | |
| httpClient = httpClient.secure( | |
| SSLHelper.sslCheckForHttpClient(datasourceStorage.getDatasourceConfiguration())); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/AuthenticationServiceCEImpl.java`
around lines 251 - 255, The HttpClient created in AuthenticationServiceCEImpl is
not reassigned after calling httpClient.secure(...), so the unsecured instance
is still used when oAuth2.getUseSelfSignedCert() is true; modify the code to
capture the returned secured client (e.g., replace the final HttpClient
httpClient = HttpClient.create() with a mutable variable or assign the result to
a new variable) and set that variable to the result of
httpClient.secure(SSLHelper.sslCheckForHttpClient(...)) so the secured
HttpClient is actually used downstream.
| ModifiedResources modifiedResources = new ModifiedResources(); | ||
| modifiedResources.setAllModified(true); | ||
| modifiedResources.setIsAllModified(true); | ||
| applicationJson.setModifiedResources(new ModifiedResources()); |
There was a problem hiding this comment.
Bug: Configured modifiedResources is discarded.
Line 374 sets isAllModified(true) on the local variable, but line 375 assigns a fresh, unconfigured ModifiedResources instance to applicationJson. The flag set on line 374 is lost.
🐛 Proposed fix
ModifiedResources modifiedResources = new ModifiedResources();
modifiedResources.setIsAllModified(true);
- applicationJson.setModifiedResources(new ModifiedResources());
+ applicationJson.setModifiedResources(modifiedResources);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ModifiedResources modifiedResources = new ModifiedResources(); | |
| modifiedResources.setAllModified(true); | |
| modifiedResources.setIsAllModified(true); | |
| applicationJson.setModifiedResources(new ModifiedResources()); | |
| ModifiedResources modifiedResources = new ModifiedResources(); | |
| modifiedResources.setIsAllModified(true); | |
| applicationJson.setModifiedResources(modifiedResources); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/server/appsmith-server/src/test/java/com/appsmith/server/git/autocommit/helpers/AutoCommitEligibilityHelperTest.java`
around lines 373 - 375, The test discards the configured ModifiedResources by
creating a new instance instead of using the one with isAllModified(true);
replace the call to applicationJson.setModifiedResources(new
ModifiedResources()) so it uses the configured variable (modifiedResources) set
earlier; locate the ModifiedResources variable and the
applicationJson.setModifiedResources(...) invocation in
AutoCommitEligibilityHelperTest and pass the configured modifiedResources object
so the isAllModified flag is preserved.
| @@ -0,0 +1,2 @@ | |||
| config.stopBubbling = true | |||
| lombok.getter.noIsPrefix = true | |||
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
lombok.getter.noIsPrefix true boolean getter behavior
💡 Result:
When lombok.getter.noIsPrefix is set to true in lombok.config, Lombok generates getters for boolean fields (both primitive boolean and Boolean) using the "get" prefix instead of the default "is" prefix. For example, for a field named "active": - Default (noIsPrefix=false): public boolean isActive - With noIsPrefix=true: public boolean getActive This applies project-wide via lombok.config. It also affects other Lombok features like @ToString that call getters, using "get" instead of "is". Note that primitive boolean defaults to "is" and Boolean to "get", but noIsPrefix=true forces "get" for both. There is no per-field @Getter(noIsPrefix=true) parameter; use @Accessors or manual methods for exceptions. This configuration remains supported as of Lombok v1.18.44 (March 2026).
Citations:
- 1: https://projectlombok.org/features/GetterSetter
- 2: https://www.educative.io/answers/what-is-the-noisprefix-configuration-in-lombok
- 3: https://projectlombok.org/api/lombok/ConfigurationKeys.html
- 4: https://projectlombok.org/changelog.html
🏁 Script executed:
# Search for boolean fields that might be affected
find . -type f -name "*.java" | head -20Repository: appsmithorg/appsmith
Length of output: 1861
🏁 Script executed:
# Look for any is...() method calls in the codebase
rg "\.is[A-Z]\w*\(\)" --type java -A 2 -B 2 | head -50Repository: appsmithorg/appsmith
Length of output: 6576
🏁 Script executed:
# Search for boolean field declarations with is prefix
rg "private\s+boolean\s+is[A-Z]" --type java -A 2 -B 2Repository: appsmithorg/appsmith
Length of output: 25044
🏁 Script executed:
# Check for Lombok annotations on boolean fields
rg "@Getter|@Data|@Value" --type java -B 5 -A 15 | grep -A 15 -B 5 "boolean is"Repository: appsmithorg/appsmith
Length of output: 46035
🏁 Script executed:
cat -n app/server/lombok.configRepository: appsmithorg/appsmith
Length of output: 136
🏁 Script executed:
# Check if there are any recent changes to how boolean getters are being used
rg "@JsonProperty.*is[A-Z]" --type java -A 2 | head -30Repository: appsmithorg/appsmith
Length of output: 2643
Ensure all boolean DTOs have explicit @JsonProperty mappings for serialization compatibility.
lombok.getter.noIsPrefix = true forces Lombok to generate get...() accessors instead of is...() for all boolean fields. The codebase already contains @JsonProperty annotations (e.g., isAnonymous, isSuperUser, isSystemTheme) to map to the expected "is..." property names in serialization. Verify that all boolean DTOs include these mappings to prevent JSON deserialization mismatches, especially for API contracts and cross-service communication.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/server/lombok.config` at line 2, Lombok is configured with
lombok.getter.noIsPrefix = true which makes boolean fields generate getX()
accessors instead of isX(), so ensure every boolean field in DTO classes has an
explicit `@JsonProperty` mapping to the expected "is..." JSON name; search DTOs
for boolean fields (e.g., isAnonymous, isSuperUser, isSystemTheme) and add
`@JsonProperty`("isAnonymous")/@JsonProperty("isSuperUser")/@JsonProperty("isSystemTheme")
(on the field or the generated getter) to preserve the original serialized names
and avoid deserialization mismatches.
| <jackson-bom.version>2.17.0</jackson-bom.version> | ||
| <jackson.version>2.17.0</jackson.version> | ||
| <java.version>17</java.version> | ||
| <java.version>25</java.version> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Mockito 4.4.0 (including mockito-inline 4.4.0 and its Byte Buddy dependency) support running on JDK 25 / Java class file version 69? If not, which Mockito version first supports Java 25?
💡 Result:
No, Mockito 4.4.0 (including mockito-inline 4.4.0 and its Byte Buddy dependency) does not support running on JDK 25 / Java class file version 69. Byte Buddy in Mockito 4.4.0 only supports up to Java 24 (class file version 68). Users encounter errors like "Java 25 (69) is not supported by the current version of Byte Buddy which officially supports Java 24 (68)" when trying to use it on JDK 25. Mockito 5.20.0 is the first version confirmed to work reliably on Java 25 without workarounds, as users reported upgrading from older versions to 5.20.0 resolved the issue, and Byte Buddy was bumped to supporting versions (e.g., 1.17.6 to 1.17.7 in 5.20.0 release notes).
Citations:
- 1: Java 25 Compatiblity mockito/mockito#3754
- 2: Tests failures after adding mockito-inline on Java 21 mockito/mockito#3328
- 3: https://github.com/mockito/mockito/releases
- 4: https://central.sonatype.com/artifact/org.mockito/mockito-core/4.9.0
- 5: https://central.sonatype.com/artifact/org.mockito/mockito-core/5.4.0
- 6: 1.17.7 - Java 25 (69) is not supported by the current version of Byte Buddy which officially supports Java 24 (68) raphw/byte-buddy#1852
Mockito 4.4.0 is incompatible with Java 25. Upgrade to Mockito 5.20.0 or later.
Mockito 4.4.0 and its Byte Buddy dependency do not support Java 25 (class file version 69); Byte Buddy in that version only supports up to Java 24 (class file version 68). This will cause build failures. Mockito 5.20.0 is the minimum version that supports Java 25.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/server/pom.xml` at line 32, Update the Mockito dependency to a Mockito
5.x release that supports Java 25 (minimum 5.20.0) to avoid Byte
Buddy/classfile-version incompatibility; locate any Maven dependency entries
with groupId/artifactId like org.mockito:mockito-core (and
org.mockito:mockito-inline or other mockito artifacts) and bump their <version>
to 5.20.0 or later, and also update any direct Byte Buddy dependency if present
to a compatible version; run a quick mvn -DskipTests=false test to verify the
build under Java 25.
| @@ -1 +1 @@ | |||
| java.runtime.version=17 | |||
| java.runtime.version=25 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only consistency check for Java version declarations in server build surfaces.
rg -n --hidden -S "java.runtime.version|<java.version>|sourceCompatibility|targetCompatibility|JAVA_HOME|jdk|temurin|corretto|openjdk" app/serverRepository: appsmithorg/appsmith
Length of output: 1746
Update IDE run configuration to match Java 25 upgrade.
system.properties and pom.xml are correctly set to Java 25, but app/server/.run/ServerApplication.run.xml still specifies temurin-17. Update the ALTERNATIVE_JRE_PATH to Java 25 to prevent local development environment mismatch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/server/system.properties` at line 1, The IDE run configuration still
points to Java 17; update the ALTERNATIVE_JRE_PATH entry in the run
configuration (ServerApplication.run.xml) to reference the Java 25 JDK (e.g.,
change any "temurin-17" value to the appropriate "temurin-25" or your local Java
25 install path) so it matches java.runtime.version=25 and the pom.xml; save the
.run XML so local IDE runs use Java 25.
1 participant
Description
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
Fixes #https://linear.app/appsmith/issue/APP-14906/upgrade-java-from-17-to-25
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/23338340009
Commit: 57190d3
Cypress dashboard.
Tags:
@tag.AllSpec:
Fri, 20 Mar 2026 11:32:03 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Chores
Documentation
Breaking Changes