From 4c61d6b77476f53c775fc01d53f557f4ecee55ed Mon Sep 17 00:00:00 2001 From: Nilesh Sarupriya Date: Tue, 11 Mar 2025 14:46:14 +0530 Subject: [PATCH] chore: move metadata calculation to datasource storage (#39657) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description > [!TIP] > _Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team)._ > > _Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR._ 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.Sanity" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: b8355265f5d5054f8e6ecb5a2c61bcb9b789fbc2 > Cypress dashboard. > Tags: `@tag.Sanity` > Spec: >
Tue, 11 Mar 2025 07:40:11 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **Refactor** - Streamlined and optimized the data source configuration process by simplifying how associated metadata is handled. - Improved the underlying service interactions to enhance system performance and maintainability. Co-authored-by: Nilesh Sarupriya <20905988+nsarupr@users.noreply.github.com> --- .../base/DatasourceServiceCEImpl.java | 68 ++++++------------- .../base/DatasourceServiceImpl.java | 10 +-- ...asourceStorageServiceCECompatibleImpl.java | 15 +++- .../base/DatasourceStorageServiceCEImpl.java | 33 ++++++++- .../base/DatasourceStorageServiceImpl.java | 15 +++- 5 files changed, 79 insertions(+), 62 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java index 7506ec80d9..4bcc60418f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java @@ -28,10 +28,8 @@ import com.appsmith.server.ratelimiting.RateLimitService; 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.OrganizationService; import com.appsmith.server.services.SequenceService; import com.appsmith.server.services.SessionUserService; import com.appsmith.server.services.WorkspaceService; @@ -66,8 +64,6 @@ import java.util.UUID; 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; @@ -97,9 +93,6 @@ public class DatasourceServiceCEImpl implements DatasourceServiceCE { private final RateLimitService rateLimitService; private final FeatureFlagService featureFlagService; private final ObservationRegistry observationRegistry; - private final OrganizationService organizationService; - 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 // attempts @@ -125,9 +118,7 @@ public class DatasourceServiceCEImpl implements DatasourceServiceCE { EnvironmentPermission environmentPermission, RateLimitService rateLimitService, FeatureFlagService featureFlagService, - ObservationRegistry observationRegistry, - OrganizationService organizationService, - ConfigService configService) { + ObservationRegistry observationRegistry) { this.workspaceService = workspaceService; this.sessionUserService = sessionUserService; @@ -146,8 +137,6 @@ public class DatasourceServiceCEImpl implements DatasourceServiceCE { this.rateLimitService = rateLimitService; this.featureFlagService = featureFlagService; this.observationRegistry = observationRegistry; - this.organizationService = organizationService; - this.configService = configService; } @Override @@ -235,28 +224,27 @@ public class DatasourceServiceCEImpl implements DatasourceServiceCE { } return datasourceMono.flatMap(savedDatasource -> this.organiseDatasourceStorages(savedDatasource) - .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); - } + .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 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 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 -> { @@ -265,20 +253,6 @@ public class DatasourceServiceCEImpl implements DatasourceServiceCE { })); } - private Mono setAdditionalMetadataInDatasourceStorage(DatasourceStorage datasourceStorage) { - Mono organizationIdMono = organizationService.getCurrentUserOrganizationId(); - Mono instanceIdMono = configService.getInstanceId(); - - Map metadata = new HashMap<>(); - - return organizationIdMono.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 organiseDatasourceStorages(@NotNull Datasource savedDatasource) { Map storages = savedDatasource.getDatasourceStorages(); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceImpl.java index adb06b2f3b..1fc7bf66b4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceImpl.java @@ -8,10 +8,8 @@ import com.appsmith.server.ratelimiting.RateLimitService; 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.OrganizationService; import com.appsmith.server.services.SequenceService; import com.appsmith.server.services.SessionUserService; import com.appsmith.server.services.WorkspaceService; @@ -43,9 +41,7 @@ public class DatasourceServiceImpl extends DatasourceServiceCEImpl implements Da EnvironmentPermission environmentPermission, RateLimitService rateLimitService, FeatureFlagService featureFlagService, - ObservationRegistry observationRegistry, - OrganizationService organizationService, - ConfigService configService) { + ObservationRegistry observationRegistry) { super( repository, @@ -64,8 +60,6 @@ public class DatasourceServiceImpl extends DatasourceServiceCEImpl implements Da environmentPermission, rateLimitService, featureFlagService, - observationRegistry, - organizationService, - configService); + observationRegistry); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/datasourcestorages/base/DatasourceStorageServiceCECompatibleImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/datasourcestorages/base/DatasourceStorageServiceCECompatibleImpl.java index 6bc68d185f..6a3e252691 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/datasourcestorages/base/DatasourceStorageServiceCECompatibleImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/datasourcestorages/base/DatasourceStorageServiceCECompatibleImpl.java @@ -4,6 +4,8 @@ import com.appsmith.server.helpers.PluginExecutorHelper; import com.appsmith.server.plugins.base.PluginService; import com.appsmith.server.repositories.DatasourceStorageRepository; import com.appsmith.server.services.AnalyticsService; +import com.appsmith.server.services.ConfigService; +import com.appsmith.server.services.OrganizationService; import com.appsmith.server.solutions.DatasourcePermission; import org.springframework.stereotype.Service; @@ -16,7 +18,16 @@ public class DatasourceStorageServiceCECompatibleImpl extends DatasourceStorageS DatasourcePermission datasourcePermission, PluginService pluginService, PluginExecutorHelper pluginExecutorHelper, - AnalyticsService analyticsService) { - super(repository, datasourcePermission, pluginService, pluginExecutorHelper, analyticsService); + AnalyticsService analyticsService, + ConfigService configService, + OrganizationService organizationService) { + super( + repository, + datasourcePermission, + pluginService, + pluginExecutorHelper, + analyticsService, + configService, + organizationService); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/datasourcestorages/base/DatasourceStorageServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/datasourcestorages/base/DatasourceStorageServiceCEImpl.java index 5b2a16b0f8..3029f86178 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/datasourcestorages/base/DatasourceStorageServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/datasourcestorages/base/DatasourceStorageServiceCEImpl.java @@ -17,6 +17,8 @@ import com.appsmith.server.helpers.PluginExecutorHelper; import com.appsmith.server.plugins.base.PluginService; import com.appsmith.server.repositories.DatasourceStorageRepository; import com.appsmith.server.services.AnalyticsService; +import com.appsmith.server.services.ConfigService; +import com.appsmith.server.services.OrganizationService; import com.appsmith.server.solutions.DatasourcePermission; import lombok.extern.slf4j.Slf4j; import org.springframework.util.CollectionUtils; @@ -30,6 +32,8 @@ import java.util.Map; import java.util.Set; import static com.appsmith.external.helpers.AppsmithBeanUtils.copyNestedNonNullProperties; +import static com.appsmith.server.constants.FieldName.INSTANCE_ID; +import static com.appsmith.server.constants.FieldName.ORGANIZATION_ID; import static java.lang.Boolean.FALSE; import static java.lang.Boolean.TRUE; @@ -41,18 +45,24 @@ public class DatasourceStorageServiceCEImpl implements DatasourceStorageServiceC private final PluginService pluginService; private final PluginExecutorHelper pluginExecutorHelper; private final AnalyticsService analyticsService; + private final ConfigService configService; + private final OrganizationService organizationService; public DatasourceStorageServiceCEImpl( DatasourceStorageRepository repository, DatasourcePermission datasourcePermission, PluginService pluginService, PluginExecutorHelper pluginExecutorHelper, - AnalyticsService analyticsService) { + AnalyticsService analyticsService, + ConfigService configService, + OrganizationService organizationService) { this.repository = repository; this.datasourcePermission = datasourcePermission; this.pluginService = pluginService; this.pluginExecutorHelper = pluginExecutorHelper; this.analyticsService = analyticsService; + this.configService = configService; + this.organizationService = organizationService; } @Override @@ -254,8 +264,10 @@ public class DatasourceStorageServiceCEImpl implements DatasourceStorageServiceC } return repository .save(unsavedDatasourceStorage) - .then(this.executePostSaveActions(unsavedDatasourceStorage)) - .thenReturn(unsavedDatasourceStorage); + .flatMap(savedDatasourceStorage -> setAdditionalMetadataInDatasourceStorage( + savedDatasourceStorage) + .flatMap(this::executePostSaveActions) + .thenReturn(savedDatasourceStorage)); }); } @@ -265,6 +277,21 @@ public class DatasourceStorageServiceCEImpl implements DatasourceStorageServiceC return Mono.just(datasourceStorage); } + private Mono setAdditionalMetadataInDatasourceStorage(DatasourceStorage datasourceStorage) { + Mono organizationIdMono = organizationService.getCurrentUserOrganizationId(); + Mono instanceIdMono = configService.getInstanceId(); + + Map metadata = new HashMap<>(); + + return organizationIdMono.zipWith(instanceIdMono).map(tuple -> { + // Change this to ORGANIZATION_ID once we have the organizationId field in the datasource storage + metadata.put(ORGANIZATION_ID, tuple.getT1()); + metadata.put(INSTANCE_ID, tuple.getT2()); + datasourceStorage.setMetadata(metadata); + return datasourceStorage; + }); + } + private DatasourceStorage sanitizeDatasourceStorage(DatasourceStorage datasourceStorage) { if (datasourceStorage.getDatasourceConfiguration() != null && !CollectionUtils.isEmpty( diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/datasourcestorages/base/DatasourceStorageServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/datasourcestorages/base/DatasourceStorageServiceImpl.java index 6d1c40e6e9..d10f3ead4b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/datasourcestorages/base/DatasourceStorageServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/datasourcestorages/base/DatasourceStorageServiceImpl.java @@ -4,6 +4,8 @@ import com.appsmith.server.helpers.PluginExecutorHelper; import com.appsmith.server.plugins.base.PluginService; import com.appsmith.server.repositories.DatasourceStorageRepository; import com.appsmith.server.services.AnalyticsService; +import com.appsmith.server.services.ConfigService; +import com.appsmith.server.services.OrganizationService; import com.appsmith.server.solutions.DatasourcePermission; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Service; @@ -17,7 +19,16 @@ public class DatasourceStorageServiceImpl extends DatasourceStorageServiceCEComp DatasourcePermission datasourcePermission, PluginService pluginService, PluginExecutorHelper pluginExecutorHelper, - AnalyticsService analyticsService) { - super(repository, datasourcePermission, pluginService, pluginExecutorHelper, analyticsService); + AnalyticsService analyticsService, + ConfigService configService, + OrganizationService organizationService) { + super( + repository, + datasourcePermission, + pluginService, + pluginExecutorHelper, + analyticsService, + configService, + organizationService); } }