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.

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

chore: add postSaveHook #39306

Merged
merged 2 commits into from
Feb 18, 2025
Merged

chore: add postSaveHook #39306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b1155a3
add postSaveHook
nsarupr Feb 17, 2025
260ad93
add metadata instead of individual fields for extensibility
nsarupr Feb 18, 2025
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
3 changes: 3 additions & 0 deletions ...ver/appsmith-interfaces/src/main/java/com/appsmith/external/models/DatasourceStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ public class DatasourceStorage extends GitSyncedDomain {
@Transient
Boolean isMock;

@Transient
Map<String, Object> metadata;

public DatasourceStorage(
String datasourceId,
String environmentId,
Expand Down
7 changes: 7 additions & 0 deletions ...erver/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/PluginExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,13 @@ default Mono<DatasourceStorage> preDeleteHook(DatasourceStorage datasourceStorag
return Mono.just(datasourceStorage);
}

/**
* This function is being called as a hook after saving a datasource.
*/
default Mono<DatasourceStorage> postSaveHook(DatasourceStorage datasourceStorage) {
return Mono.just(datasourceStorage);
}

/**
* This function fetches the structure of the tables/collections in the datasource. It's used to make query creation
* easier for the user.
Expand Down
1 change: 1 addition & 0 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/FieldNameCE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ public class FieldNameCE {

public static final String REMOTE_PLUGINS = "remotePlugins";
public static final String INSTANCE_ID = "instanceId";
public static final String TENANT_ID = "tenantId";
public static final String IP_ADDRESS = "ipAddress";
public static final String VERSION = "version";
public static final String PUBLISHED = "published";
Expand Down
67 changes: 46 additions & 21 deletions ...th-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@
import com.appsmith.server.repositories.DatasourceRepository;
import com.appsmith.server.repositories.NewActionRepository;
import com.appsmith.server.services.AnalyticsService;
import com.appsmith.server.services.ConfigService;
import com.appsmith.server.services.DatasourceContextService;
import com.appsmith.server.services.FeatureFlagService;
import com.appsmith.server.services.SequenceService;
import com.appsmith.server.services.SessionUserService;
import com.appsmith.server.services.TenantService;
import com.appsmith.server.services.WorkspaceService;
import com.appsmith.server.solutions.DatasourcePermission;
import com.appsmith.server.solutions.EnvironmentPermission;
Expand Down Expand Up @@ -64,6 +66,8 @@
import static com.appsmith.external.constants.spans.DatasourceSpan.FETCH_ALL_DATASOURCES_WITH_STORAGES;
import static com.appsmith.external.constants.spans.DatasourceSpan.FETCH_ALL_PLUGINS_IN_WORKSPACE;
import static com.appsmith.external.helpers.AppsmithBeanUtils.copyNestedNonNullProperties;
import static com.appsmith.server.constants.ce.FieldNameCE.INSTANCE_ID;
import static com.appsmith.server.constants.ce.FieldNameCE.TENANT_ID;
import static com.appsmith.server.dtos.DBOpsType.SAVE;
import static com.appsmith.server.helpers.CollectionUtils.isNullOrEmpty;
import static com.appsmith.server.helpers.DatasourceAnalyticsUtils.getAnalyticsProperties;
Expand Down Expand Up @@ -93,6 +97,8 @@ public class DatasourceServiceCEImpl implements DatasourceServiceCE {
private final RateLimitService rateLimitService;
private final FeatureFlagService featureFlagService;
private final ObservationRegistry observationRegistry;
private final TenantService tenantService;
private final ConfigService configService;

// Defines blocking duration for test as well as connection created for query execution
// This will block the creation of datasource connection for 5 minutes, in case of more than 3 failed connection
Expand All @@ -119,7 +125,9 @@ public DatasourceServiceCEImpl(
EnvironmentPermission environmentPermission,
RateLimitService rateLimitService,
FeatureFlagService featureFlagService,
ObservationRegistry observationRegistry) {
ObservationRegistry observationRegistry,
TenantService tenantService,
ConfigService configService) {

this.workspaceService = workspaceService;
this.sessionUserService = sessionUserService;
Expand All @@ -138,6 +146,8 @@ public DatasourceServiceCEImpl(
this.rateLimitService = rateLimitService;
this.featureFlagService = featureFlagService;
this.observationRegistry = observationRegistry;
this.tenantService = tenantService;
this.configService = configService;
}

@Override
Expand Down Expand Up @@ -223,27 +233,28 @@ private Mono<Datasource> createEx(
}

return datasourceMono.flatMap(savedDatasource -> this.organiseDatasourceStorages(savedDatasource)
.flatMap(datasourceStorage -> {
// Make sure that we are creating entries only if the id is not already populated
if (hasText(datasourceStorage.getId())) {
return Mono.just(datasourceStorage);
}
.flatMap(datasourceStorageX -> setAdditionalMetadataInDatasourceStorage(datasourceStorageX)
.flatMap(datasourceStorage -> {
// Make sure that we are creating entries only if the id is not already populated
if (hasText(datasourceStorage.getId())) {
return Mono.just(datasourceStorage);
}

return datasourceStorageService
.create(datasourceStorage, isDryOps)
.map(datasourceStorage1 -> {
if (datasourceStorageDryRunQueries != null && isDryOps) {
List<DatasourceStorage> datasourceStorages =
datasourceStorageDryRunQueries.get(SAVE);
if (datasourceStorages == null) {
datasourceStorages = new ArrayList<>();
}
datasourceStorages.add(datasourceStorage1);
datasourceStorageDryRunQueries.put(SAVE, datasourceStorages);
}
return datasourceStorage1;
});
})
return datasourceStorageService
.create(datasourceStorage, isDryOps)
.map(datasourceStorage1 -> {
if (datasourceStorageDryRunQueries != null && isDryOps) {
List<DatasourceStorage> datasourceStorages =
datasourceStorageDryRunQueries.get(SAVE);
if (datasourceStorages == null) {
datasourceStorages = new ArrayList<>();
}
datasourceStorages.add(datasourceStorage1);
datasourceStorageDryRunQueries.put(SAVE, datasourceStorages);
}
return datasourceStorage1;
});
}))
.map(datasourceStorageService::createDatasourceStorageDTOFromDatasourceStorage)
.collectMap(DatasourceStorageDTO::getEnvironmentId)
.map(savedStorages -> {
Expand All @@ -252,6 +263,20 @@ private Mono<Datasource> createEx(
}));
}

private Mono<DatasourceStorage> setAdditionalMetadataInDatasourceStorage(DatasourceStorage datasourceStorage) {
Mono<String> tenantIdMono = tenantService.getDefaultTenantId();
Mono<String> instanceIdMono = configService.getInstanceId();

Map<String, Object> metadata = new HashMap<>();

return tenantIdMono.zipWith(instanceIdMono).map(tuple -> {
metadata.put(TENANT_ID, tuple.getT1());
metadata.put(INSTANCE_ID, tuple.getT2());
datasourceStorage.setMetadata(metadata);
return datasourceStorage;
});
}

// this requires an EE override multiple environments
protected Flux<DatasourceStorage> organiseDatasourceStorages(@NotNull Datasource savedDatasource) {
Map<String, DatasourceStorageDTO> storages = savedDatasource.getDatasourceStorages();
Expand Down
10 changes: 8 additions & 2 deletions ...mith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
import com.appsmith.server.repositories.DatasourceRepository;
import com.appsmith.server.repositories.NewActionRepository;
import com.appsmith.server.services.AnalyticsService;
import com.appsmith.server.services.ConfigService;
import com.appsmith.server.services.DatasourceContextService;
import com.appsmith.server.services.FeatureFlagService;
import com.appsmith.server.services.SequenceService;
import com.appsmith.server.services.SessionUserService;
import com.appsmith.server.services.TenantService;
import com.appsmith.server.services.WorkspaceService;
import com.appsmith.server.solutions.DatasourcePermission;
import com.appsmith.server.solutions.EnvironmentPermission;
Expand Down Expand Up @@ -41,7 +43,9 @@ public DatasourceServiceImpl(
EnvironmentPermission environmentPermission,
RateLimitService rateLimitService,
FeatureFlagService featureFlagService,
ObservationRegistry observationRegistry) {
ObservationRegistry observationRegistry,
TenantService tenantService,
ConfigService configService) {

super(
repository,
Expand All @@ -60,6 +64,8 @@ public DatasourceServiceImpl(
environmentPermission,
rateLimitService,
featureFlagService,
observationRegistry);
observationRegistry,
tenantService,
configService);
}
}
15 changes: 14 additions & 1 deletion ...main/java/com/appsmith/server/datasourcestorages/base/DatasourceStorageServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,16 @@ public Mono<DatasourceStorage> executePreSaveActions(DatasourceStorage datasourc
return pluginExecutorMono.flatMap(pluginExecutor -> pluginExecutor.preSaveHook(datasourceStorage));
}

public Mono<DatasourceStorage> executePostSaveActions(DatasourceStorage datasourceStorage) {
Mono<Plugin> pluginMono = pluginService.findById(datasourceStorage.getPluginId());
Mono<PluginExecutor> pluginExecutorMono = pluginExecutorHelper
.getPluginExecutor(pluginMono)
.switchIfEmpty(Mono.error(new AppsmithException(
AppsmithError.NO_RESOURCE_FOUND, FieldName.PLUGIN, datasourceStorage.getPluginId())));

return pluginExecutorMono.flatMap(pluginExecutor -> pluginExecutor.postSaveHook(datasourceStorage));
}

@Override
public Mono<DatasourceStorage> validateDatasourceStorage(DatasourceStorage datasourceStorage) {

Expand Down Expand Up @@ -242,7 +252,10 @@ private Mono<DatasourceStorage> validateAndSaveDatasourceStorageToRepository(
unsavedDatasourceStorage.updateForBulkWriteOperation();
return Mono.just(unsavedDatasourceStorage);
}
return repository.save(unsavedDatasourceStorage).thenReturn(unsavedDatasourceStorage);
return repository
.save(unsavedDatasourceStorage)
.then(this.executePostSaveActions(unsavedDatasourceStorage))
.thenReturn(unsavedDatasourceStorage);
Comment on lines +255 to +258
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider transaction boundary for save operation.

The save operation and post-save actions should be executed within a transaction to ensure atomicity. If the post-save action fails, the save should be rolled back.

Consider using Spring's @Transactional annotation or a transaction template to wrap both operations:

-return repository
-        .save(unsavedDatasourceStorage)
-        .then(this.executePostSaveActions(unsavedDatasourceStorage))
-        .thenReturn(unsavedDatasourceStorage);
+return transactionTemplate.execute(status -> {
+    return repository
+            .save(unsavedDatasourceStorage)
+            .then(this.executePostSaveActions(unsavedDatasourceStorage))
+            .thenReturn(unsavedDatasourceStorage);
+});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return repository
.save(unsavedDatasourceStorage)
.then(this.executePostSaveActions(unsavedDatasourceStorage))
.thenReturn(unsavedDatasourceStorage);
return transactionTemplate.execute(status -> {
return repository
.save(unsavedDatasourceStorage)
.then(this.executePostSaveActions(unsavedDatasourceStorage))
.thenReturn(unsavedDatasourceStorage);
});

});
}

Expand Down
Loading