Skip to content

Commit

Permalink
chore: autocommit migration for annotation and embedded datasource ch…
Browse files Browse the repository at this point in the history
…anges. (#36261)

## Description
- Added autocommit migration to avoid uncommited changes on some of the
applications.


Fixes #`Issue Number`  
_or_  
Fixes `Issue URL`

## Automation

/ok-to-test tags="@tag.Git"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!IMPORTANT]
> 🟣 🟣 🟣 Your tests are running.
> Tests running at:
<https://github.com/appsmithorg/appsmith/actions/runs/10878546679>
> Commit: 0013cde
> Workflow: `PR Automation test suite`
> Tags: `@tag.Git`
> Spec: ``
> <hr>Mon, 16 Sep 2024 06:20:40 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Updated the server version from 10 to 11, enhancing compatibility and
functionality related to JSON schema handling.
- Improved logic for default REST datasource migrations, making it more
robust and accessible.
- Added support for SSH connection configurations in datasource
management.

- **Bug Fixes**
- Added null checks to prevent potential errors during datasource
migrations, enhancing overall reliability.

- **Refactor**
- Streamlined filtering logic for actions, improving code readability
and maintainability.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
sondermanish authored Sep 16, 2024
1 parent 3728299 commit 71261b1
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ public class DatasourceConfiguration implements AppsmithDomain {
@JsonView({Views.Public.class, FromRequest.class})
AuthenticationDTO authentication;

@JsonView({Views.Public.class, FromRequest.class})
SSHConnection sshProxy;

@JsonView({Views.Public.class, FromRequest.class})
Boolean sshProxyEnabled;

@JsonView({Views.Public.class, FromRequest.class, Git.class})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,28 +67,38 @@ public Mono<ApplicationJson> migrateApplicationJsonToLatestSchema(
// TODO: make import flow migration reactive
return Mono.just(migrateServerSchema(appJson))
.flatMap(migratedApplicationJson -> {
if (migratedApplicationJson.getServerSchemaVersion() == 9
&& Boolean.TRUE.equals(MigrationHelperMethods.doesRestApiRequireMigration(
migratedApplicationJson))) {
return jsonSchemaMigrationHelper
.addDatasourceConfigurationToDefaultRestApiActions(
baseApplicationId, branchName, migratedApplicationJson)
.map(applicationJsonWithMigration10 -> {
applicationJsonWithMigration10.setServerSchemaVersion(10);
return applicationJsonWithMigration10;
});
// 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.
if (migratedApplicationJson.getServerSchemaVersion() == 9) {
migratedApplicationJson.setServerSchemaVersion(10);
}

if (migratedApplicationJson.getServerSchemaVersion() == 10) {
if (Boolean.TRUE.equals(MigrationHelperMethods.doesRestApiRequireMigration(
migratedApplicationJson))) {
return jsonSchemaMigrationHelper
.addDatasourceConfigurationToDefaultRestApiActions(
baseApplicationId, branchName, migratedApplicationJson);
}

migratedApplicationJson.setServerSchemaVersion(11);
}

migratedApplicationJson.setServerSchemaVersion(10);
return Mono.just(migratedApplicationJson);
})
.map(migratedAppJson -> {
if (applicationJson
.getServerSchemaVersion()
.equals(jsonSchemaVersions.getServerVersion())) {
return applicationJson;
}

applicationJson.setServerSchemaVersion(jsonSchemaVersions.getServerVersion());
return applicationJson;
});
Expand Down Expand Up @@ -193,16 +203,14 @@ private ApplicationJson nonReactiveServerMigrationForImport(ApplicationJson appl

switch (applicationJson.getServerSchemaVersion()) {
case 9:
applicationJson.setServerSchemaVersion(10);
case 10:
// this if for cases where we have empty datasource configs
MigrationHelperMethods.migrateApplicationJsonToVersionTen(applicationJson, Map.of());
applicationJson.setServerSchemaVersion(10);
applicationJson.setServerSchemaVersion(11);
default:
}

if (applicationJson.getServerSchemaVersion().equals(jsonSchemaVersions.getServerVersion())) {
return applicationJson;
}

applicationJson.setServerSchemaVersion(jsonSchemaVersions.getServerVersion());
return applicationJson;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

@Component
public class JsonSchemaVersionsFallback {
private static final Integer serverVersion = 10;
private static final Integer serverVersion = 11;
public static final Integer clientVersion = 1;

public Integer getServerVersion() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import static com.appsmith.external.constants.PluginConstants.PackageName.GRAPHQL_PLUGIN;
import static com.appsmith.external.constants.PluginConstants.PackageName.REST_API_PLUGIN;
import static com.appsmith.server.constants.ResourceModes.EDIT;
import static com.appsmith.server.constants.ResourceModes.VIEW;
import static org.springframework.data.mongodb.core.query.Criteria.where;
Expand Down Expand Up @@ -1235,13 +1237,34 @@ public static void setThemeSettings(Application.ThemeSetting themeSetting) {
}
}

private static boolean conditionForDefaultRestDatasourceMigration(NewAction action) {
public static boolean conditionForDefaultRestDatasource(NewAction action) {
if (action.getUnpublishedAction() == null
|| action.getUnpublishedAction().getDatasource() == null) {
return false;
}

Datasource actionDatasource = action.getUnpublishedAction().getDatasource();

// probable check for the default rest datasource action is.
// it has no datasource id and action's plugin id is either rest-api or graphql plugin.
boolean probableCheckForDefaultRestDatasource = !org.springframework.util.StringUtils.hasText(
actionDatasource.getId())
&& (REST_API_PLUGIN.equals(action.getPluginId()) || GRAPHQL_PLUGIN.equals(action.getPluginId()));

// condition to check if the action is default rest datasource.
// it has no datasource id and name is equal to DEFAULT_REST_DATASOURCE
boolean isActionDefaultRestDatasource = !org.springframework.util.StringUtils.hasText(actionDatasource.getId())
&& PluginConstants.DEFAULT_REST_DATASOURCE.equals(actionDatasource.getName());
boolean certainCheckForDefaultRestDatasource =
!org.springframework.util.StringUtils.hasText(actionDatasource.getId())
&& PluginConstants.DEFAULT_REST_DATASOURCE.equals(actionDatasource.getName());

// Two separate types of checks over here, it's either the obvious certain way to identify or
// the likely chance that the datasource is present.
return certainCheckForDefaultRestDatasource || probableCheckForDefaultRestDatasource;
}

private static boolean conditionForDefaultRestDatasourceMigration(NewAction action) {
boolean isActionDefaultRestDatasource = conditionForDefaultRestDatasource(action);
Datasource actionDatasource = action.getUnpublishedAction().getDatasource();

// condition to check if the action has missing url or has no config at all
boolean isDatasourceConfigurationOrUrlMissing = actionDatasource.getDatasourceConfiguration() == null
Expand Down Expand Up @@ -1322,18 +1345,25 @@ public static void setDatasourceConfigDetailsInDefaultRestDatasourceForActions(

if (defaultDatasourceActionMap.containsKey(action.getGitSyncId())) {
NewAction actionFromMap = defaultDatasourceActionMap.get(action.getGitSyncId());
// NPE check to avoid migration failures
if (actionFromMap.getUnpublishedAction() == null
|| actionFromMap.getUnpublishedAction().getDatasource() == null
|| actionFromMap.getUnpublishedAction().getDatasource().getDatasourceConfiguration() == null) {
return;
}

// set the datasource config in the json action only if the datasource config from db is not null,
// else it'll start to show as uncommited changes.
DatasourceConfiguration datasourceConfigurationFromDBAction =
actionFromMap.getUnpublishedAction().getDatasource().getDatasourceConfiguration();

if (datasourceConfigurationFromDBAction != null) {
datasourceConfiguration.setUrl(datasourceConfigurationFromDBAction.getUrl());
}
}
// At this point it's established that datasource config of db action is not null.
datasourceConfiguration.setUrl(datasourceConfigurationFromDBAction.getUrl());
actionDatasource.setDatasourceConfiguration(datasourceConfiguration);

if (!org.springframework.util.StringUtils.hasText(datasourceConfiguration.getUrl())) {
} else {
datasourceConfiguration.setUrl("");
actionDatasource.setDatasourceConfiguration(datasourceConfiguration);
}

actionDatasource.setDatasourceConfiguration(datasourceConfiguration);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.appsmith.server.migrations.utils;

import com.appsmith.external.constants.PluginConstants;
import com.appsmith.external.models.Datasource;
import com.appsmith.external.models.PluginType;
import com.appsmith.server.applications.base.ApplicationService;
import com.appsmith.server.domains.Application;
import com.appsmith.server.domains.NewAction;
Expand Down Expand Up @@ -51,14 +53,27 @@ public Mono<ApplicationJson> addDatasourceConfigurationToDefaultRestApiActions(
return false;
}

boolean reverseFlag = StringUtils.hasText(action.getUnpublishedAction()
.getDatasource()
.getId())
|| !PluginConstants.DEFAULT_REST_DATASOURCE.equals(action.getUnpublishedAction()
.getDatasource()
.getName());
Datasource actionDatasource =
action.getUnpublishedAction().getDatasource();

return !reverseFlag;
// lenient probable check for the default rest datasource action is.
// As we don't have any harm in the allowing API actions present in db.
// it has no datasource id and action's plugin type is API
boolean probableCheckForDefaultRestDatasource =
!org.springframework.util.StringUtils.hasText(actionDatasource.getId())
&& PluginType.API.equals(action.getPluginType());

// condition to check if the action is default rest datasource.
// it has no datasource id and name is equal to DEFAULT_REST_DATASOURCE
boolean certainCheckForDefaultRestDatasource =
!org.springframework.util.StringUtils.hasText(actionDatasource.getId())
&& PluginConstants.DEFAULT_REST_DATASOURCE.equals(
actionDatasource.getName());

// Two separate types of checks over here, it's either the obvious certain way to
// identify or
// the likely chance that the datasource is present.
return certainCheckForDefaultRestDatasource || probableCheckForDefaultRestDatasource;
})
.collectMap(NewAction::getGitSyncId);
})
Expand Down

0 comments on commit 71261b1

Please sign in to comment.