From 072d35b08fb09cfe48e266d7a764a59836ef3c8d Mon Sep 17 00:00:00 2001 From: Manish Kumar <107841575+sondermanish@users.noreply.github.com> Date: Mon, 26 Jun 2023 17:56:43 +0530 Subject: [PATCH] fix: returning empty datasourceStucture instead of empty on invalid (#24802) ## Description > TL;DR now returning empty datasourceStructure object instead of Mono.empty, also added the analytics call back. The issue has started in sentry because a snippet has recently been added to expect some response from server, if the response is empty it throws error Fixes #23675 #### Type of change - Bug fix (non-breaking change which fixes an issue) #### How Has This Been Tested? - [x] Manual - [x] Jest - [ ] Cypress ## Checklist: #### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../ce/DatasourceStructureSolutionCEImpl.java | 4 ++- .../DatasourceStructureSolutionTest.java | 28 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/DatasourceStructureSolutionCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/DatasourceStructureSolutionCEImpl.java index 9ca09a3ff3..e680a0b1d4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/DatasourceStructureSolutionCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/DatasourceStructureSolutionCEImpl.java @@ -80,7 +80,9 @@ public class DatasourceStructureSolutionCEImpl implements DatasourceStructureSol boolean ignoreCache) { if (Boolean.FALSE.equals(datasourceStorage.getIsValid())) { - return Mono.empty(); + return analyticsService.sendObjectEvent(AnalyticsEvents.DS_SCHEMA_FETCH_EVENT_FAILED, + datasourceStorage, getAnalyticsPropertiesForTestEventStatus(datasourceStorage, false)) + .then(Mono.just(new DatasourceStructure())); } Mono configurationStructureMono = datasourceStructureService diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/DatasourceStructureSolutionTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/DatasourceStructureSolutionTest.java index 74100a27e3..482e71b7eb 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/DatasourceStructureSolutionTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/DatasourceStructureSolutionTest.java @@ -4,6 +4,7 @@ package com.appsmith.server.solutions; import com.appsmith.external.models.DBAuth; import com.appsmith.external.models.Datasource; import com.appsmith.external.models.DatasourceConfiguration; +import com.appsmith.external.models.DatasourceStorage; import com.appsmith.external.models.DatasourceStorageDTO; import com.appsmith.external.models.DatasourceStorageStructure; import com.appsmith.external.models.DatasourceStructure; @@ -38,6 +39,7 @@ import org.springframework.util.StringUtils; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import java.util.HashSet; import java.util.List; import static com.appsmith.external.models.DatasourceStructure.TableType.TABLE; @@ -403,4 +405,30 @@ public class DatasourceStructureSolutionTest { }); } + + @Test + @WithUserDetails(value = "api_user") + public void verifyEmptyDatasourceStructureObjectIfDatasourceIsInvalid() { + DatasourceStorage datasourceStorage = new DatasourceStorage(); + datasourceStorage.setDatasourceId(datasourceId); + datasourceStorage.setEnvironmentId(defaultEnvironmentId); + datasourceStorage.setInvalids(new HashSet<>()); + datasourceStorage.getInvalids().add("random invalid"); + + doReturn(Mono.just(datasourceStorage)) + .when(datasourceStorageService).findByDatasourceAndEnvironmentId(any(), any()); + + Mono datasourceStructureMono = + datasourceStructureSolution.getStructure(datasourceId, Boolean.FALSE, defaultEnvironmentId); + + StepVerifier + .create(datasourceStructureMono) + .assertNext(datasourceStructure -> { + assertThat(datasourceStructure.getTables()).isNull(); + assertThat(datasourceStructure.getError()).isNull(); + }) + .verifyComplete(); + + } + }