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.

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

chore: Migration for missing datasource configuration on default rest datasources for git connected app. #36203

Merged
merged 3 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make this an additional systematic path similar to the json file migrations? This is a legitimate case of a new data point that needs to be added to the repository, and, although is expected fewer times than file migrations, should be accounted for as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So what you mean is that let's add a reactive migration set separately itself?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes please

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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this a separate method?

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
Loading