Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update client version in JsonSchemaVersionsFallback (#38390) #38391

Merged
merged 5 commits into from
Jan 3, 2025
Merged

fix: update client version in JsonSchemaVersionsFallback (#38390) #38391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b3aaaea
fix: update client version in JsonSchemaVersionsFallback (#38390)
rahulbarwal Dec 27, 2024
0d745ea
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
rahulbarwal Dec 30, 2024
d24930d
fix: update clientSchemaVersion to prevent backward compatibility issues
rahulbarwal Dec 30, 2024
019a8da
Updates clientSchemaVersion to latest
rahulbarwal Jan 2, 2025
51523c9
update versions in json file
rahulbarwal Jan 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...smith-server/src/main/java/com/appsmith/server/migrations/JsonSchemaVersionsFallback.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
@Component
public class JsonSchemaVersionsFallback {
private static final Integer serverVersion = 11;
public static final Integer clientVersion = 1;
public static final Integer clientVersion = 2;

public Integer getServerVersion() {
return serverVersion;
Expand Down
3 changes: 2 additions & 1 deletion ...ppsmith-server/src/test/java/com/appsmith/server/imports/internal/ImportServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ public void setup() {
testApplication.setLastDeployedAt(Instant.now());
testApplication.setModifiedBy("some-user");
testApplication.setGitApplicationMetadata(new GitArtifactMetadata());
testApplication.setClientSchemaVersion(jsonSchemaVersions.getClientVersion());

Application.ThemeSetting themeSettings = getThemeSetting();
testApplication.setUnpublishedApplicationDetail(new ApplicationDetail());
Expand Down Expand Up @@ -2731,7 +2732,7 @@ public void applySchemaMigration_jsonFileWithFirstVersion_migratedToLatestVersio
ApplicationJson latestApplicationJson = tuple.getT2();

assertThat(v1ApplicationJson.getServerSchemaVersion()).isEqualTo(1);
assertThat(v1ApplicationJson.getClientSchemaVersion()).isEqualTo(1);
assertThat(v1ApplicationJson.getClientSchemaVersion()).isEqualTo(2);

assertThat(latestApplicationJson.getServerSchemaVersion())
.isEqualTo(jsonSchemaVersions.getServerVersion());
Expand Down
2 changes: 1 addition & 1 deletion ...server/appsmith-server/src/test/resources/com/appsmith/server/migrations/application.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"artifactJsonType": "APPLICATION",
"clientSchemaVersion": 1.0,
"clientSchemaVersion": 2.0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Test resources need to be updated to match new client schema version

The clientSchemaVersion update to 2.0 in migrations/application.json is correct and matches JsonSchemaVersionsFallback.clientVersion = 2, but other test resources are still using version 1.0/1 which needs to be updated:

  • test_assets/ImportExportServiceTest/*.json files need clientSchemaVersion: 2
  • git/*.json files need clientSchemaVersion: 2
  • imports/internal/faulty-dsl.json needs clientSchemaVersion: 2

These test files are used in import/export and migration tests that verify version compatibility. Having inconsistent versions across test resources could lead to false test failures.

🔗 Analysis chain

LGTM! Version update aligns with PR objectives.

The clientSchemaVersion update from 1.0 to 2.0 correctly implements the intended breaking change for HTML column type support.

Let's verify consistency across other test resources:

Inconsistent schema versions found in test resources - update needed

All test resources except migrations/application.json are using clientSchemaVersion 1.0/1, which conflicts with the schema version upgrade to 2.0. These test files need to be updated to maintain consistency:

  • test_assets/ImportExportServiceTest/*.json files (7 files)
  • git/*.json files (4 files)
  • imports/internal/faulty-dsl.json

Let's verify if these files are used in migration/import tests that specifically need the older version:

Let's check the test files to understand if they specifically test version compatibility:

Let's try a different approach to find version-related test cases:

Let's check the JsonSchemaVersions class to understand the version management:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Find and check clientSchemaVersion in other test resources
rg -l "clientSchemaVersion" app/server/appsmith-server/src/test/resources/ | while read -r file; do
  echo "=== $file ==="
  # Show the line containing clientSchemaVersion and its context
  rg -C 2 "clientSchemaVersion" "$file"
done

Length of output: 6498

Script:

#!/bin/bash
# Search for test files that use these json resources
rg -l "ImportExportServiceTest|faulty-dsl.json|ce-automation-test.json|autocommit.json" app/server/appsmith-server/src/test/java/

Length of output: 1148

Script:

#!/bin/bash
# Check test files for version-related tests
echo "=== Checking ServerSchemaMigrationEnforcerTest ==="
rg -C 5 "clientSchemaVersion|schemaVersion" app/server/appsmith-server/src/test/java/com/appsmith/server/git/ServerSchemaMigrationEnforcerTest.java

echo -e "\n=== Checking ImportServiceTests ==="
rg -C 5 "clientSchemaVersion|schemaVersion" app/server/appsmith-server/src/test/java/com/appsmith/server/imports/internal/ImportServiceTests.java

echo -e "\n=== Checking ImportApplicationTransactionServiceTest ==="
rg -C 5 "clientSchemaVersion|schemaVersion" app/server/appsmith-server/src/test/java/com/appsmith/server/transactions/ImportApplicationTransactionServiceTest.java

Length of output: 789

Script:

#!/bin/bash
# Search for JsonSchemaVersions or version-related imports/constants
echo "=== Checking for JsonSchemaVersions usage ==="
rg -C 5 "JsonSchemaVersions" app/server/appsmith-server/src/test/java/

echo -e "\n=== Checking for version-related test methods ==="
rg -C 5 "test.*[Vv]ersion|[Vv]ersion.*test" app/server/appsmith-server/src/test/java/

echo -e "\n=== Checking for version assertions ==="
rg -C 5 "assert.*[Vv]ersion|[Vv]ersion.*assert" app/server/appsmith-server/src/test/java/

Length of output: 69826

Script:

#!/bin/bash
# Check JsonSchemaVersions implementation
echo "=== Checking JsonSchemaVersions class ==="
rg -C 5 "class JsonSchemaVersions" app/server/appsmith-server/src/main/java/

echo -e "\n=== Checking JsonSchemaVersionsFallback class ==="
rg -C 5 "class JsonSchemaVersionsFallback" app/server/appsmith-server/src/main/java/

Length of output: 4895

"serverSchemaVersion": 7.0,
"exportedApplication": {
"name": "json-schema-migration",
Expand Down
2 changes: 1 addition & 1 deletion .../appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/file-with-v1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"serverSchemaVersion": 1,
"clientSchemaVersion": 1
"clientSchemaVersion": 2
}
Loading