Skip to content

Commit

Permalink
chore: Migration for missing datasource configuration on default rest…
Browse files Browse the repository at this point in the history
… datasources for git connected app. (#36203)

## Description



Fixes #`Issue Number`  
_or_  
Fixes `Issue URL`
> [!WARNING]  
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

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

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/10789950948>
> Commit: 69729ce
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10789950948&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Git, @tag.ImportExport`
> Spec:
> <hr>Tue, 10 Sep 2024 10:07:42 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**
- Enhanced application JSON migration process to support additional
contextual parameters, improving accuracy and relevance.
- Introduced a new helper class to facilitate datasource configuration
during migrations.

- **Bug Fixes**
- Improved handling of incompatible schemas during migration, ensuring
robust error management.

- **Tests**
- Adjusted test cases to accommodate changes in method signatures for
migration processes, ensuring continued functionality.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
sondermanish committed Sep 10, 2024
1 parent 70a7164 commit da5d37e
Show file tree
Hide file tree
Showing 8 changed files with 269 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,8 @@ public Mono<ArtifactExchangeJson> reconstructArtifactExchangeJsonFromFilesInRepo
getApplicationResource(applicationReference.getMetadata(), ApplicationJson.class);
ApplicationJson applicationJson = getApplicationJsonFromGitReference(applicationReference);
copyNestedNonNullProperties(metadata, applicationJson);
return jsonSchemaMigration.migrateApplicationJsonToLatestSchema(applicationJson);
return jsonSchemaMigration.migrateApplicationJsonToLatestSchema(
applicationJson, baseArtifactId, branchName);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,27 @@
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.helpers.CollectionUtils;
import com.appsmith.server.migrations.utils.JsonSchemaMigrationHelper;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Component;
import reactor.core.publisher.Mono;

import java.util.Map;

@Slf4j
@Component
@RequiredArgsConstructor
public class JsonSchemaMigration {

private final JsonSchemaVersions jsonSchemaVersions;
private final JsonSchemaMigrationHelper jsonSchemaMigrationHelper;

private boolean isCompatible(ApplicationJson applicationJson) {
return (applicationJson.getClientSchemaVersion() <= jsonSchemaVersions.getClientVersion())
&& (applicationJson.getServerSchemaVersion() <= jsonSchemaVersions.getServerVersion());
}

/**
* This is a temporary check which is being placed for the compatibility of server versions in scenarios
* where user is moving a json from an instance which has
* release_autocommit_feature_enabled true to an instance which has the flag as false. In that case the server
* version number of json would be 8 and in new instance it would be not compatible.
* @param applicationJson
* @return
*/
private boolean isAutocommitVersionBump(ApplicationJson applicationJson) {
return jsonSchemaVersions.getServerVersion() == 7 && applicationJson.getServerSchemaVersion() == 8;
}

private void setSchemaVersions(ApplicationJson applicationJson) {
applicationJson.setServerSchemaVersion(getCorrectSchemaVersion(applicationJson.getServerSchemaVersion()));
applicationJson.setClientSchemaVersion(getCorrectSchemaVersion(applicationJson.getClientSchemaVersion()));
Expand All @@ -53,24 +45,53 @@ public Mono<? extends ArtifactExchangeJson> migrateArtifactExchangeJsonToLatestS
ArtifactExchangeJson artifactExchangeJson) {

if (ArtifactType.APPLICATION.equals(artifactExchangeJson.getArtifactJsonType())) {
return migrateApplicationJsonToLatestSchema((ApplicationJson) artifactExchangeJson);
return migrateApplicationJsonToLatestSchema((ApplicationJson) artifactExchangeJson, null, null);
}

return Mono.fromCallable(() -> artifactExchangeJson);
}

public Mono<ApplicationJson> migrateApplicationJsonToLatestSchema(ApplicationJson applicationJson) {
public Mono<ApplicationJson> migrateApplicationJsonToLatestSchema(
ApplicationJson applicationJson, String baseApplicationId, String branchName) {
return Mono.fromCallable(() -> {
setSchemaVersions(applicationJson);
if (isCompatible(applicationJson)) {
return migrateServerSchema(applicationJson);
}

if (isAutocommitVersionBump(applicationJson)) {
return migrateServerSchema(applicationJson);
return applicationJson;
})
.flatMap(appJson -> {
if (!isCompatible(appJson)) {
return Mono.empty();
}

return null;
// Taking a tech debt over here for import of file application.
// All migration above version 9 is reactive
// 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;
});
}

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

applicationJson.setServerSchemaVersion(jsonSchemaVersions.getServerVersion());
return applicationJson;
});
})
.switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.INCOMPATIBLE_IMPORTED_JSON)));
}
Expand All @@ -81,7 +102,7 @@ public Mono<ApplicationJson> migrateApplicationJsonToLatestSchema(ApplicationJso
* @param artifactExchangeJson : the json to be imported
* @return transformed artifact exchange json
*/
@Deprecated
@Deprecated(forRemoval = true, since = "Use migrateArtifactJsonToLatestSchema")
public ArtifactExchangeJson migrateArtifactToLatestSchema(ArtifactExchangeJson artifactExchangeJson) {

if (!ArtifactType.APPLICATION.equals(artifactExchangeJson.getArtifactJsonType())) {
Expand All @@ -91,11 +112,11 @@ public ArtifactExchangeJson migrateArtifactToLatestSchema(ArtifactExchangeJson a
ApplicationJson applicationJson = (ApplicationJson) artifactExchangeJson;
setSchemaVersions(applicationJson);
if (!isCompatible(applicationJson)) {
if (!isAutocommitVersionBump(applicationJson)) {
throw new AppsmithException(AppsmithError.INCOMPATIBLE_IMPORTED_JSON);
}
throw new AppsmithException(AppsmithError.INCOMPATIBLE_IMPORTED_JSON);
}
return migrateServerSchema(applicationJson);

applicationJson = migrateServerSchema(applicationJson);
return nonReactiveServerMigrationForImport(applicationJson);
}

/**
Expand Down Expand Up @@ -145,11 +166,37 @@ private ApplicationJson migrateServerSchema(ApplicationJson applicationJson) {
MigrationHelperMethods.ensureXmlParserPresenceInCustomJsLibList(applicationJson);
applicationJson.setServerSchemaVersion(7);
case 7:
applicationJson.setServerSchemaVersion(8);
case 8:
MigrationHelperMethods.migrateThemeSettingsForAnvil(applicationJson);
applicationJson.setServerSchemaVersion(9);

// This is not supposed to have anymore additions to the schema.
default:
// Unable to detect the serverSchema

}

return applicationJson;
}

/**
* This method is an alternative to reactive way of adding migrations to application json.
* this is getting used by flows which haven't implemented the reactive way yet.
* @param applicationJson : application json for which migration has to be done.
* @return return application json after migration
*/
private ApplicationJson nonReactiveServerMigrationForImport(ApplicationJson applicationJson) {
if (jsonSchemaVersions.getServerVersion().equals(applicationJson.getServerSchemaVersion())) {
return applicationJson;
}

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

if (applicationJson.getServerSchemaVersion().equals(jsonSchemaVersions.getServerVersion())) {
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 = 9;
private static final Integer serverVersion = 10;
public static final Integer clientVersion = 1;

public Integer getServerVersion() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package com.appsmith.server.migrations;

import com.appsmith.external.constants.PluginConstants;
import com.appsmith.external.helpers.MustacheHelper;
import com.appsmith.external.models.ActionDTO;
import com.appsmith.external.models.BaseDomain;
import com.appsmith.external.models.Datasource;
import com.appsmith.external.models.DatasourceConfiguration;
import com.appsmith.external.models.InvisibleActionFields;
import com.appsmith.external.models.Property;
import com.appsmith.server.constants.ApplicationConstants;
Expand Down Expand Up @@ -1231,4 +1234,106 @@ public static void setThemeSettings(Application.ThemeSetting themeSetting) {
themeSetting.setSizing(1);
}
}

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

// 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());

// condition to check if the action has missing url or has no config at all
boolean isDatasourceConfigurationOrUrlMissing = actionDatasource.getDatasourceConfiguration() == null
|| !org.springframework.util.StringUtils.hasText(
actionDatasource.getDatasourceConfiguration().getUrl());

return isActionDefaultRestDatasource && isDatasourceConfigurationOrUrlMissing;
}

/**
* Adds datasource configuration and relevant url to the embedded datasource actions.
* @param applicationJson: ApplicationJson for which the migration has to be performed
* @param defaultDatasourceActionMap: gitSyncId to actions with default rest datasource map
*/
public static void migrateApplicationJsonToVersionTen(
ApplicationJson applicationJson, Map<String, NewAction> defaultDatasourceActionMap) {
List<NewAction> actionList = applicationJson.getActionList();
if (CollectionUtils.isNullOrEmpty(actionList)) {
return;
}

for (NewAction action : actionList) {
if (action.getUnpublishedAction() == null
|| action.getUnpublishedAction().getDatasource() == null) {
continue;
}

Datasource actionDatasource = action.getUnpublishedAction().getDatasource();
if (conditionForDefaultRestDatasourceMigration(action)) {
// Idea is to add datasourceConfiguration to existing DEFAULT_REST_DATASOURCE apis,
// for which the datasource configuration is missing
// the url would be set to empty string as right url is not present over here.
setDatasourceConfigDetailsInDefaultRestDatasourceForActions(action, defaultDatasourceActionMap);
}
}
}

/**
* Finds if the applicationJson has any default rest datasource which has a null datasource configuration
* or an unset url.
* @param applicationJson : Application Json for which requirement is to be checked.
* @return true if the application has a rest api which doesn't have a valid datasource configuration.
*/
public static Boolean doesRestApiRequireMigration(ApplicationJson applicationJson) {
List<NewAction> actionList = applicationJson.getActionList();
if (CollectionUtils.isNullOrEmpty(actionList)) {
return Boolean.FALSE;
}

for (NewAction action : actionList) {
if (action.getUnpublishedAction() == null
|| action.getUnpublishedAction().getDatasource() == null) {
continue;
}

Datasource actionDatasource = action.getUnpublishedAction().getDatasource();
if (conditionForDefaultRestDatasourceMigration(action)) {
return Boolean.TRUE;
}
}

return Boolean.FALSE;
}

/**
* Adds the relevant url in the default rest datasource for the given action from an action in the db
* otherwise sets the url to empty
* it's established that action doesn't have the datasource.
* @param action : default rest datasource actions which doesn't have valid datasource configuration.
* @param defaultDatasourceActionMap : gitSyncId to actions with default rest datasource map
*/
public static void setDatasourceConfigDetailsInDefaultRestDatasourceForActions(
NewAction action, Map<String, NewAction> defaultDatasourceActionMap) {

ActionDTO actionDTO = action.getUnpublishedAction();
Datasource actionDatasource = actionDTO.getDatasource();
DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration();

if (defaultDatasourceActionMap.containsKey(action.getGitSyncId())) {
NewAction actionFromMap = defaultDatasourceActionMap.get(action.getGitSyncId());
DatasourceConfiguration datasourceConfigurationFromDBAction =
actionFromMap.getUnpublishedAction().getDatasource().getDatasourceConfiguration();

if (datasourceConfigurationFromDBAction != null) {
datasourceConfiguration.setUrl(datasourceConfigurationFromDBAction.getUrl());
}
}

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

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

import com.appsmith.external.constants.PluginConstants;
import com.appsmith.server.applications.base.ApplicationService;
import com.appsmith.server.domains.Application;
import com.appsmith.server.domains.NewAction;
import com.appsmith.server.dtos.ApplicationJson;
import com.appsmith.server.migrations.MigrationHelperMethods;
import com.appsmith.server.newactions.base.NewActionService;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Component;
import org.springframework.util.StringUtils;
import reactor.core.publisher.Mono;

import java.util.Map;
import java.util.Optional;

@Component
@Slf4j
@RequiredArgsConstructor
public class JsonSchemaMigrationHelper {

private final ApplicationService applicationService;
private final NewActionService newActionService;

public Mono<ApplicationJson> addDatasourceConfigurationToDefaultRestApiActions(
String baseApplicationId, String branchName, ApplicationJson applicationJson) {

Mono<ApplicationJson> contingencyMigrationJson = Mono.defer(() -> Mono.fromCallable(() -> {
MigrationHelperMethods.migrateApplicationJsonToVersionTen(applicationJson, Map.of());
return applicationJson;
}));

if (!StringUtils.hasText(baseApplicationId) || !StringUtils.hasText(branchName)) {
return contingencyMigrationJson;
}

Mono<Application> applicationMono = applicationService
.findByBranchNameAndBaseApplicationId(branchName, baseApplicationId, null)
.cache();

return applicationMono
.flatMap(branchedApplication -> {
return newActionService
.findAllByApplicationIdAndViewMode(
branchedApplication.getId(), Boolean.FALSE, Optional.empty(), Optional.empty())
.filter(action -> {
if (action.getUnpublishedAction() == null
|| action.getUnpublishedAction().getDatasource() == null) {
return false;
}

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

return !reverseFlag;
})
.collectMap(NewAction::getGitSyncId);
})
.map(newActionMap -> {
MigrationHelperMethods.migrateApplicationJsonToVersionTen(applicationJson, newActionMap);
return applicationJson;
})
.switchIfEmpty(contingencyMigrationJson)
.onErrorResume(error -> {
log.error("Error occurred while migrating actions of application json. {}", error.getMessage());
return contingencyMigrationJson;
});
}
}
Loading

0 comments on commit da5d37e

Please sign in to comment.