Skip to content

chore: refactor theme import flow to support dry ops for pg #34733

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
AnaghHegde merged 11 commits into from
Jul 24, 2024
Merged

chore: refactor theme import flow to support dry ops for pg #34733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a8b1181
refactor theme import flow to support dry ops
AnaghHegde Jul 4, 2024
b8727db
add application id while saving to db
AnaghHegde Jul 5, 2024
ac95b19
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
AnaghHegde Jul 9, 2024
0a5932e
fix test failures
AnaghHegde Jul 9, 2024
9ff0ed8
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
AnaghHegde Jul 9, 2024
8c1e065
chore
AnaghHegde Jul 9, 2024
e5e6cb3
review comments
AnaghHegde Jul 9, 2024
334ea3d
Merge branch 'release' into pg-transaction-theme-import-flow
AnaghHegde Jul 10, 2024
b48b662
use bulk ops for save and update
AnaghHegde Jul 10, 2024
6913799
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
AnaghHegde Jul 23, 2024
0b83e89
review comments
AnaghHegde Jul 23, 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
6 changes: 6 additions & 0 deletions ...ith-server/src/main/java/com/appsmith/server/dtos/ce/MappedImportableResourcesCE_DTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

import com.appsmith.external.models.Datasource;
import com.appsmith.external.models.DatasourceStorage;
import com.appsmith.server.domains.Application;
import com.appsmith.server.domains.Context;
import com.appsmith.server.domains.CustomJSLib;
import com.appsmith.server.domains.Theme;
import com.appsmith.server.dtos.CustomJSLibContextDTO;
import com.appsmith.server.dtos.DBOpsType;
import com.appsmith.server.dtos.ImportActionCollectionResultDTO;
Expand Down Expand Up @@ -71,4 +73,8 @@ public class MappedImportableResourcesCE_DTO {
datasourceDryRunQueries.put(dbOpsType, Collections.synchronizedList(new ArrayList<>()));
}
}

Map<String, List<Theme>> themeDryRunQueries = new HashMap<>();

Map<String, List<Application>> applicationDryRunQueries = new HashMap<>();
Copy link
Contributor

@abhvsn abhvsn Jul 9, 2024

Choose a reason for hiding this comment

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

Are we expecting the value as List<Application> in any of the flows?

Copy link
Contributor Author

@AnaghHegde AnaghHegde Jul 9, 2024

Choose a reason for hiding this comment

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

@abhvsn At this movement its just only one update, but once i move to refactoring the entities like page, application we will have multiple updates of application object.

Copy link
Contributor

@abhvsn abhvsn Jul 10, 2024

Choose a reason for hiding this comment

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

Yeah that's where this comment coming from. Considering this will be the same object instead of creating multiple entries can we just keep updating the existing application object. That way # of db ops will be reduced and also we will not face any issue around data loss because of overwriting.

Copy link
Contributor Author

@AnaghHegde AnaghHegde Jul 10, 2024

Choose a reason for hiding this comment

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

That is a good point but i am worried that due to parallel executions we might miss some updates. Hence i decided to keep this as list. And when we use bulk update we will not face much issues.

Copy link
Contributor

@abhvsn abhvsn Jul 11, 2024

Choose a reason for hiding this comment

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

@AnaghHegde after todays update are we aligned on keeping this as a single application instead of list of application?

Copy link
Contributor Author

@AnaghHegde AnaghHegde Jul 12, 2024

Choose a reason for hiding this comment

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

@abhvsn Yes correct, this will be updated but i will do this in page import PR. The current flow has only instance of application in the list.

}
66 changes: 63 additions & 3 deletions ...ppsmith-server/src/main/java/com/appsmith/server/repositories/DryOperationRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@

import com.appsmith.external.models.Datasource;
import com.appsmith.external.models.DatasourceStorage;
import com.appsmith.server.acl.AclPermission;
import com.appsmith.server.domains.Application;
import com.appsmith.server.domains.CustomJSLib;
import com.appsmith.server.domains.Theme;
import com.appsmith.server.dtos.DBOpsType;
import com.appsmith.server.dtos.MappedImportableResourcesDTO;
import jakarta.annotation.PostConstruct;
import lombok.RequiredArgsConstructor;
Expand All @@ -25,13 +29,18 @@ public class DryOperationRepository {

private final CustomJSLibRepository customJSLibRepository;

private final ThemeRepository themeRepository;

private final ApplicationRepository applicationRepository;

private Map<Class<?>, AppsmithRepository<?>> repoByEntityClass;

@PostConstruct
public void init() {
final Map<Class<?>, AppsmithRepository<?>> map = new HashMap<>();
map.put(Datasource.class, datasourceRepository);
map.put(DatasourceStorage.class, datasourceStorageRepository);
map.put(Theme.class, themeRepository);
map.put(CustomJSLib.class, customJSLibRepository);
repoByEntityClass = Collections.unmodifiableMap(map);
}
Expand All @@ -52,10 +61,28 @@ private Flux<CustomJSLib> saveCustomJSLibToDb(List<CustomJSLib> customJSLibs) {
return customJSLibRepository.saveAll(customJSLibs);
}

private Flux<Theme> saveThemeToDb(List<Theme> theme) {
return themeRepository.saveAll(theme);
}

private Mono<Boolean> archiveTheme(List<String> themeIds) {
return themeRepository.archiveAllById(themeIds);
}

private Mono<List<Theme>> updateTheme(List<Theme> themes) {
return themeRepository.bulkUpdate(themes).thenReturn(themes);
}

private Mono<Application> updateApplication(Application application) {
String id = application.getId();
application.setId(null);
return applicationRepository.updateById(id, application, AclPermission.MANAGE_APPLICATIONS);
}

public Mono<Void> executeAllDbOps(MappedImportableResourcesDTO mappedImportableResourcesDTO) {

Flux<List<Datasource>> datasourceFLux = Flux.fromIterable(mappedImportableResourcesDTO
.getDatasourceStorageDryRunQueries()
.getDatasourceDryRunQueries()
.keySet())
.flatMap(key -> {
List<Datasource> datasourceList = mappedImportableResourcesDTO
Expand All @@ -74,15 +101,48 @@ public Mono<Void> executeAllDbOps(MappedImportableResourcesDTO mappedImportableR
return saveDatasourceStorageToDb(datasourceStorageList).collectList();
});

Flux<List<CustomJSLib>> customJSLibFLux = Flux.fromIterable(
Flux<List<CustomJSLib>> customJSLibFlux = Flux.fromIterable(
mappedImportableResourcesDTO.getCustomJSLibsDryOps().keySet())
.flatMap(key -> {
List<CustomJSLib> customJSLibList =
mappedImportableResourcesDTO.getCustomJSLibsDryOps().get(key);
return saveCustomJSLibToDb(customJSLibList).collectList();
});

return Flux.merge(datasourceFLux, datasourceStorageFLux, customJSLibFLux)
Flux<List<Theme>> themeFlux = Flux.fromIterable(
mappedImportableResourcesDTO.getThemeDryRunQueries().keySet())
.flatMap(key -> {
List<Theme> themeList =
mappedImportableResourcesDTO.getThemeDryRunQueries().get(key);
if (key.equals(DBOpsType.SAVE.name())) {
return saveThemeToDb(themeList).collectList();
} else if (key.equals(DBOpsType.DELETE.name())) {
return archiveTheme(themeList.stream().map(Theme::getId).toList())
.then(Mono.just(themeList));
} else {
return updateTheme(themeList);
}
});

Flux<List<Application>> applicationFlux = Flux.fromIterable(mappedImportableResourcesDTO
.getApplicationDryRunQueries()
.keySet())
.flatMap(key -> {
List<Application> applicationList = mappedImportableResourcesDTO
.getApplicationDryRunQueries()
.get(key);
if (key.equals(DBOpsType.SAVE.name())) {
return Flux.fromIterable(applicationList)
.flatMap(this::updateApplication)
.collectList();
} else {
return Flux.fromIterable(applicationList)
.flatMap(this::updateApplication)
.collectList();
}
});

return Flux.merge(datasourceFLux, datasourceStorageFLux, customJSLibFlux, themeFlux, applicationFlux)
.then();
}
}
2 changes: 2 additions & 0 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/themes/base/ThemeServiceCE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,7 @@ public interface ThemeServiceCE extends CrudService<Theme, String> {

Mono<Theme> getOrSaveTheme(Theme theme, Application destApplication);

Mono<Theme> getOrSaveTheme(Theme theme, Application destApplication, boolean isDryOps);

Mono<Application> archiveApplicationThemes(Application application);
}
9 changes: 9 additions & 0 deletions ...ver/appsmith-server/src/main/java/com/appsmith/server/themes/base/ThemeServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,11 @@ public Mono<Theme> updateName(String id, Theme themeDto) {

@Override
public Mono<Theme> getOrSaveTheme(Theme theme, Application destApplication) {
return getOrSaveTheme(theme, destApplication, false);
}

@Override
public Mono<Theme> getOrSaveTheme(Theme theme, Application destApplication, boolean isDryOps) {
if (theme == null) { // this application was exported without theme, assign the legacy theme to it
return repository.getSystemThemeByName(Theme.LEGACY_THEME_NAME, READ_THEMES); // return the default theme
} else if (theme.isSystemTheme()) {
Expand All @@ -452,6 +457,10 @@ public Mono<Theme> getOrSaveTheme(Theme theme, Application destApplication) {
newTheme.setName(theme.getName());
newTheme.setDisplayName(theme.getDisplayName());
newTheme.setSystemTheme(false);
if (isDryOps) {
newTheme.updateForBulkWriteOperation();
return Mono.just(newTheme);
}
return repository.save(newTheme);
}
}
Expand Down
92 changes: 75 additions & 17 deletions ...ver/src/main/java/com/appsmith/server/themes/importable/ThemeImportableServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.appsmith.server.domains.Theme;
import com.appsmith.server.domains.Workspace;
import com.appsmith.server.dtos.ArtifactExchangeJson;
import com.appsmith.server.dtos.DBOpsType;
import com.appsmith.server.dtos.ImportingMetaDTO;
import com.appsmith.server.dtos.MappedImportableResourcesDTO;
import com.appsmith.server.imports.importable.ImportableServiceCE;
Expand All @@ -16,7 +17,9 @@
import org.springframework.util.StringUtils;
import reactor.core.publisher.Mono;

import static com.appsmith.server.acl.AclPermission.MANAGE_THEMES;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

public class ThemeImportableServiceCEImpl implements ImportableServiceCE<Theme> {

Expand Down Expand Up @@ -71,12 +74,14 @@ public Mono<Void> importEntities(
Mono<Theme> editModeTheme = updateExistingAppThemeFromJSON(
importableArtifact,
importableArtifact.getUnpublishedThemeId(),
artifactExchangeJson.getUnpublishedTheme());
artifactExchangeJson.getUnpublishedTheme(),
mappedImportableResourcesDTO);

Mono<Theme> publishedModeTheme = updateExistingAppThemeFromJSON(
importableArtifact,
importableArtifact.getPublishedThemeId(),
artifactExchangeJson.getPublishedTheme());
artifactExchangeJson.getPublishedTheme(),
mappedImportableResourcesDTO);

return Mono.zip(editModeTheme, publishedModeTheme)
.flatMap(importedThemesTuple -> {
Expand All @@ -86,41 +91,74 @@ public Mono<Void> importEntities(

importableArtifact.setUnpublishedThemeId(editModeThemeId);
importableArtifact.setPublishedThemeId(publishedModeThemeId);
// this will update the theme id in DB
return applicationService.setAppTheme(
importableArtifact.getId(),
editModeThemeId,
publishedModeThemeId,
applicationPermission.getEditPermission());
// this will update the theme in the application and will be updated to db in the dry ops
// execution

Application application = new Application();
application.setPublishedModeThemeId(publishedModeThemeId);
application.setUnpublishedThemeId(editModeThemeId);
application.setId(importableArtifact.getId());

addDryOpsForApplication(
mappedImportableResourcesDTO.getApplicationDryRunQueries(), application);
return Mono.just(importableArtifact);
})
.then();
});
}

private Mono<Theme> updateExistingAppThemeFromJSON(
Artifact destinationArtifact, String existingThemeId, Theme themeFromJson) {
Artifact destinationArtifact,
String existingThemeId,
Theme themeFromJson,
MappedImportableResourcesDTO mappedImportableResourcesDTO) {
if (!StringUtils.hasLength(existingThemeId)) {
return themeService.getOrSaveTheme(themeFromJson, (Application) destinationArtifact);
return themeService
.getOrSaveTheme(themeFromJson, (Application) destinationArtifact, true)
.map(createdTheme -> {
addDryOpsForEntity(
DBOpsType.SAVE, mappedImportableResourcesDTO.getThemeDryRunQueries(), createdTheme);
return createdTheme;
});
}

return repository
.findById(existingThemeId)
.defaultIfEmpty(new Theme()) // fallback when application theme is deleted
.flatMap(existingTheme -> {
if (!StringUtils.hasLength(existingTheme.getId()) || existingTheme.isSystemTheme()) {
return themeService.getOrSaveTheme(themeFromJson, (Application) destinationArtifact);
return themeService
.getOrSaveTheme(themeFromJson, (Application) destinationArtifact, true)
.map(createdTheme -> {
addDryOpsForEntity(
DBOpsType.SAVE,
mappedImportableResourcesDTO.getThemeDryRunQueries(),
createdTheme);
return createdTheme;
});
} else {
if (themeFromJson.isSystemTheme()) {
return themeService
.getOrSaveTheme(themeFromJson, (Application) destinationArtifact)
.getOrSaveTheme(themeFromJson, (Application) destinationArtifact, true)
.flatMap(importedTheme -> {
// need to delete the old existingTheme
return repository
.archiveById(existingThemeId)
.thenReturn(importedTheme);
addDryOpsForEntity(
DBOpsType.SAVE,
mappedImportableResourcesDTO.getThemeDryRunQueries(),
importedTheme);
addDryOpsForEntity(
DBOpsType.DELETE,
mappedImportableResourcesDTO.getThemeDryRunQueries(),
existingTheme);
return Mono.just(importedTheme);
});
} else {
return repository.updateById(existingThemeId, themeFromJson, MANAGE_THEMES);
themeFromJson.setId(existingThemeId);
addDryOpsForEntity(
DBOpsType.UPDATE,
mappedImportableResourcesDTO.getThemeDryRunQueries(),
themeFromJson);
return Mono.just(themeFromJson);
}
}
});
Expand All @@ -141,4 +179,24 @@ public Mono<Void> importEntities(
importableArtifactMono,
artifactExchangeJson);
}

private void addDryOpsForEntity(DBOpsType queryType, Map<String, List<Theme>> dryRunOpsMap, Theme createdTheme) {
if (dryRunOpsMap.containsKey(queryType.name())) {
dryRunOpsMap.get(queryType.name()).add(createdTheme);
} else {
List<Theme> themes = new ArrayList<>();
themes.add(createdTheme);
dryRunOpsMap.put(queryType.name(), themes);
}
}

private void addDryOpsForApplication(Map<String, List<Application>> dryRunOpsMap, Application application) {
if (dryRunOpsMap.containsKey(DBOpsType.UPDATE.name())) {
dryRunOpsMap.get(DBOpsType.UPDATE.name()).add(application);
} else {
List<Application> applicationList = new ArrayList<>();
applicationList.add(application);
dryRunOpsMap.put(DBOpsType.UPDATE.name(), applicationList);
}
}
}
Loading