Skip to content

chore: removed non reactive json migration #36413

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

Merged
nidhi-nair merged 3 commits into from
Sep 20, 2024
Merged

chore: removed non reactive json migration #36413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a360f9
added CE changes for reactive migration
sondermanish Sep 19, 2024
52034ec
Merge remote-tracking branch 'origin/release' into chore/reactive-jso…
sondermanish Sep 19, 2024
612bb7b
added suggestions by code rabbit
sondermanish Sep 20, 2024
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
25 changes: 25 additions & 0 deletions ...rc/main/java/com/appsmith/server/applications/imports/ApplicationImportServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.appsmith.server.imports.internal.artifactbased.ArtifactBasedImportServiceCE;
import com.appsmith.server.layouts.UpdateLayoutService;
import com.appsmith.server.migrations.ApplicationVersion;
import com.appsmith.server.migrations.JsonSchemaMigration;
import com.appsmith.server.newactions.base.NewActionService;
import com.appsmith.server.services.ApplicationPageService;
import com.appsmith.server.solutions.ActionPermission;
Expand All @@ -53,6 +54,7 @@

import static com.appsmith.server.helpers.ImportExportUtils.setPropertiesToExistingApplication;
import static com.appsmith.server.helpers.ImportExportUtils.setPublishedApplicationProperties;
import static org.springframework.util.StringUtils.hasText;

@RequiredArgsConstructor
@Slf4j
Expand All @@ -69,6 +71,7 @@ public class ApplicationImportServiceCEImpl
private final ApplicationPermission applicationPermission;
private final PagePermission pagePermission;
private final ActionPermission actionPermission;
private final JsonSchemaMigration jsonSchemaMigration;
private final ImportableService<Theme> themeImportableService;
private final ImportableService<NewPage> newPageImportableService;
private final ImportableService<CustomJSLib> customJSLibImportableService;
Expand Down Expand Up @@ -639,4 +642,26 @@ public Mono<Set<String>> getDatasourceIdSetConsumedInArtifact(String baseArtifac
public Flux<String> getBranchedArtifactIdsByBranchedArtifactId(String branchedArtifactId) {
return applicationService.findAllBranchedApplicationIdsByBranchedApplicationId(branchedArtifactId, null);
}

@Override
public Mono<ApplicationJson> migrateArtifactExchangeJson(
String branchedArtifactId, ArtifactExchangeJson artifactExchangeJson) {
ApplicationJson applicationJson = (ApplicationJson) artifactExchangeJson;

if (!hasText(branchedArtifactId)) {
return jsonSchemaMigration.migrateApplicationJsonToLatestSchema(applicationJson, null, null);
}

return applicationService.findById(branchedArtifactId).flatMap(application -> {
String baseArtifactId = application.getBaseId();
String branchName = null;

if (application.getGitArtifactMetadata() != null) {
branchName = application.getGitArtifactMetadata().getBranchName();
}

return jsonSchemaMigration.migrateApplicationJsonToLatestSchema(
applicationJson, baseArtifactId, branchName);
});
}
}
3 changes: 3 additions & 0 deletions .../src/main/java/com/appsmith/server/applications/imports/ApplicationImportServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.appsmith.server.imports.importable.ImportableService;
import com.appsmith.server.imports.internal.artifactbased.ArtifactBasedImportService;
import com.appsmith.server.layouts.UpdateLayoutService;
import com.appsmith.server.migrations.JsonSchemaMigration;
import com.appsmith.server.newactions.base.NewActionService;
import com.appsmith.server.services.ApplicationPageService;
import com.appsmith.server.solutions.ActionPermission;
Expand All @@ -35,6 +36,7 @@ public ApplicationImportServiceImpl(
ApplicationPermission applicationPermission,
PagePermission pagePermission,
ActionPermission actionPermission,
JsonSchemaMigration jsonSchemaMigration,
ImportableService<Theme> themeImportableService,
ImportableService<NewPage> newPageImportableService,
ImportableService<CustomJSLib> customJSLibImportableService,
Expand All @@ -50,6 +52,7 @@ public ApplicationImportServiceImpl(
applicationPermission,
pagePermission,
actionPermission,
jsonSchemaMigration,
themeImportableService,
newPageImportableService,
customJSLibImportableService,
Expand Down
149 changes: 82 additions & 67 deletions ...psmith-server/src/main/java/com/appsmith/server/imports/internal/ImportServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,23 +411,32 @@ private Mono<Artifact> importArtifactInWorkspace(
String artifactContextString = artifactSpecificConstantsMap.get(FieldName.ARTIFACT_CONTEXT);

// step 1: Schema Migration
ArtifactExchangeJson importedDoc = jsonSchemaMigration.migrateArtifactToLatestSchema(artifactExchangeJson);

// Step 2: Validation of artifact Json
// check for validation error and raise exception if error found
String errorField = validateArtifactExchangeJson(importedDoc);
if (!errorField.isEmpty()) {
log.error("Error in importing {}. Field {} is missing", artifactContextString, errorField);
if (errorField.equals(artifactContextString)) {
return Mono.error(
new AppsmithException(
Mono<? extends ArtifactExchangeJson> migratedArtifactJsonMono = artifactBasedImportService
.migrateArtifactExchangeJson(branchedArtifactId, artifactExchangeJson)
.flatMap(importedDoc -> {
// Step 2: Validation of artifact Json
// check for validation error and raise exception if error found
String errorField = validateArtifactExchangeJson(importedDoc);
if (!errorField.isEmpty()) {
log.error("Error in importing {}. Field {} is missing", artifactContextString, errorField);

if (errorField.equals(artifactContextString)) {
return Mono.error(
new AppsmithException(
AppsmithError.VALIDATION_FAILURE,
"Field '" + artifactContextString
+ "'. Sorry! It seems you've imported a page-level JSON instead of an application. Please use the import within the page."));
}

return Mono.error(new AppsmithException(
AppsmithError.VALIDATION_FAILURE,
"Field '" + artifactContextString
+ "' Sorry! Seems like you've imported a page-level json instead of an application. Please use the import within the page."));
}
return Mono.error(new AppsmithException(
AppsmithError.VALIDATION_FAILURE, "Field '" + errorField + "' is missing in the JSON."));
}
"Field '" + errorField + "' is missing in the JSON."));
}

artifactBasedImportService.syncClientAndSchemaVersion(importedDoc);
return Mono.just(importedDoc);
})
.cache();

ImportingMetaDTO importingMetaDTO = new ImportingMetaDTO(
workspaceId,
Expand All @@ -441,7 +450,6 @@ private Mono<Artifact> importArtifactInWorkspace(
permissionGroups);

MappedImportableResourcesDTO mappedImportableResourcesDTO = new MappedImportableResourcesDTO();
artifactBasedImportService.syncClientAndSchemaVersion(importedDoc);

Mono<Workspace> workspaceMono = workspaceService
.findById(workspaceId, permissionProvider.getRequiredPermissionOnTargetWorkspace())
Expand Down Expand Up @@ -469,56 +477,63 @@ private Mono<Artifact> importArtifactInWorkspace(
.switchIfEmpty(Mono.just(List.of()))
.doOnNext(importingMetaDTO::setBranchedArtifactIds);

// this would import customJsLibs for all type of artifacts
Mono<Void> artifactSpecificImportableEntities =
artifactBasedImportService.generateArtifactSpecificImportableEntities(
importedDoc, importingMetaDTO, mappedImportableResourcesDTO);

/*
Calling the workspaceMono first to avoid creating multiple mongo transactions.
If the first db call inside a transaction is a Flux, then there's a chance of creating multiple mongo
transactions which will lead to NoSuchTransaction exception.
*/
final Mono<? extends Artifact> importableArtifactMono = workspaceMono
.then(Mono.defer(() -> Mono.when(branchedArtifactIdsMono, artifactSpecificImportableEntities)))
.then(Mono.defer(() -> artifactBasedImportService.updateAndSaveArtifactInContext(
importedDoc.getArtifact(), importingMetaDTO, mappedImportableResourcesDTO, currUserMono)))
.cache();

final Mono<? extends Artifact> importMono = importableArtifactMono
.then(Mono.defer(() -> generateImportableEntities(
importingMetaDTO,
mappedImportableResourcesDTO,
workspaceMono,
importableArtifactMono,
importedDoc)))
.then(importableArtifactMono)
.flatMap(importableArtifact -> updateImportableEntities(
artifactBasedImportService, importableArtifact, mappedImportableResourcesDTO, importingMetaDTO))
.flatMap(importableArtifact -> updateImportableArtifact(artifactBasedImportService, importableArtifact))
.onErrorResume(throwable -> {
String errorMessage = ImportExportUtils.getErrorMessage(throwable);
log.error("Error importing {}. Error: {}", artifactContextString, errorMessage, throwable);
return Mono.error(
new AppsmithException(AppsmithError.GENERIC_JSON_IMPORT_ERROR, workspaceId, errorMessage));
})
// execute dry run for datasource
.flatMap(importableArtifact -> dryOperationRepository
.executeAllDbOps(mappedImportableResourcesDTO)
.thenReturn(importableArtifact))
.as(transactionalOperator::transactional);

final Mono<? extends Artifact> resultMono = importMono
.flatMap(importableArtifact -> sendImportedContextAnalyticsEvent(
artifactBasedImportService, importableArtifact, AnalyticsEvents.IMPORT))
.zipWith(currUserMono)
.flatMap(tuple -> {
Artifact importableArtifact = tuple.getT1();
User user = tuple.getT2();
stopwatch.stopTimer();
stopwatch.stopAndLogTimeInMillis();
return sendImportRelatedAnalyticsEvent(importedDoc, importableArtifact, stopwatch, user);
});
final Mono<? extends Artifact> resultMono = migratedArtifactJsonMono.flatMap(importedDoc -> {

// this would import customJsLibs for all type of artifacts
Mono<Void> artifactSpecificImportableEntities =
artifactBasedImportService.generateArtifactSpecificImportableEntities(
importedDoc, importingMetaDTO, mappedImportableResourcesDTO);

/*
Calling the workspaceMono first to avoid creating multiple mongo transactions.
If the first db call inside a transaction is a Flux, then there's a chance of creating multiple mongo
transactions which will lead to NoSuchTransaction exception.
*/
final Mono<? extends Artifact> importableArtifactMono = workspaceMono
.then(Mono.defer(() -> Mono.when(branchedArtifactIdsMono, artifactSpecificImportableEntities)))
.then(Mono.defer(() -> artifactBasedImportService.updateAndSaveArtifactInContext(
importedDoc.getArtifact(), importingMetaDTO, mappedImportableResourcesDTO, currUserMono)))
.cache();

final Mono<? extends Artifact> importMono = importableArtifactMono
.then(Mono.defer(() -> generateImportableEntities(
importingMetaDTO,
mappedImportableResourcesDTO,
workspaceMono,
importableArtifactMono,
importedDoc)))
.then(importableArtifactMono)
.flatMap(importableArtifact -> updateImportableEntities(
artifactBasedImportService,
importableArtifact,
mappedImportableResourcesDTO,
importingMetaDTO))
.flatMap(importableArtifact ->
updateImportableArtifact(artifactBasedImportService, importableArtifact))
.onErrorResume(throwable -> {
String errorMessage = ImportExportUtils.getErrorMessage(throwable);
log.error("Error importing {}. Error: {}", artifactContextString, errorMessage, throwable);
return Mono.error(new AppsmithException(
AppsmithError.GENERIC_JSON_IMPORT_ERROR, workspaceId, errorMessage));
})
// execute dry run for datasource
.flatMap(importableArtifact -> dryOperationRepository
.executeAllDbOps(mappedImportableResourcesDTO)
.thenReturn(importableArtifact))
.as(transactionalOperator::transactional);

return importMono
.flatMap(importableArtifact -> sendImportedContextAnalyticsEvent(
artifactBasedImportService, importableArtifact, AnalyticsEvents.IMPORT))
.zipWith(currUserMono)
.flatMap(tuple -> {
Artifact importableArtifact = tuple.getT1();
User user = tuple.getT2();
stopwatch.stopTimer();
stopwatch.stopAndLogTimeInMillis();
return sendImportRelatedAnalyticsEvent(importedDoc, importableArtifact, stopwatch, user);
});
});

// Import Context is currently a slow API because it needs to import and create context, pages, actions
// and action collection. This process may take time and the client may cancel the request. This leads to the
Expand Down
2 changes: 2 additions & 0 deletions ...java/com/appsmith/server/imports/internal/artifactbased/ArtifactBasedImportServiceCE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,6 @@ Mono<Void> generateArtifactSpecificImportableEntities(
Mono<Set<String>> getDatasourceIdSetConsumedInArtifact(String baseArtifactId);

Flux<String> getBranchedArtifactIdsByBranchedArtifactId(String branchedArtifactId);

Mono<V> migrateArtifactExchangeJson(String branchedArtifactId, ArtifactExchangeJson artifactExchangeJson);
}
Loading