Merge pull request #324 from AllenInstitute/bugfix/fix-description-da…

…ta-source-loading Prevent duplicate loading of same description source
AllenInstitute · Nov 14, 2024 · 658d28c · 658d28c
2 parents f0066e6 + be80cb2
commit 658d28c
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 4 deletions.
diff --git a/packages/core/services/DatabaseService/DatabaseServiceNoop.ts b/packages/core/services/DatabaseService/DatabaseServiceNoop.ts
@@ -1,6 +1,10 @@
 import DatabaseService from ".";
 
 export default class DatabaseServiceNoop extends DatabaseService {
+    public deleteSourceMetadata(): Promise<void> {
+        return Promise.reject("DatabaseServiceNoop:deleteSourceMetadata");
+    }
+
     public execute(): Promise<void> {
         return Promise.reject("DatabaseServiceNoop:execute");
     }

diff --git a/packages/core/services/DatabaseService/index.ts b/packages/core/services/DatabaseService/index.ts
@@ -30,6 +30,7 @@ export default abstract class DatabaseService {
         ...Object.values(AnnotationType),
         DatabaseService.OPEN_FILE_LINK_TYPE,
     ]);
+    private sourceMetadataName?: string;
     private currentAggregateSource?: string;
     // Initialize with AICS FMS data source name to pretend it always exists
     protected readonly existingDataSources = new Set([AICS_FMS_DATA_SOURCE_NAME]);
@@ -140,18 +141,25 @@ export default abstract class DatabaseService {
     }
 
     public async prepareSourceMetadata(sourceMetadata: Source): Promise<void> {
-        if (sourceMetadata.type && sourceMetadata.uri) {
-            this.deleteDataSource(this.SOURCE_METADATA_TABLE);
-            this.dataSourceToAnnotationsMap.clear();
+        const isPreviousSource = sourceMetadata.name === this.sourceMetadataName;
+        if (isPreviousSource) {
+            return;
         }
 
+        await this.deleteSourceMetadata();
         await this.prepareDataSource(
             {
                 ...sourceMetadata,
                 name: this.SOURCE_METADATA_TABLE,
             },
             true
         );
+        this.sourceMetadataName = sourceMetadata.name;
+    }
+
+    public async deleteSourceMetadata(): Promise<void> {
+        await this.deleteDataSource(this.SOURCE_METADATA_TABLE);
+        this.dataSourceToAnnotationsMap.clear();
     }
 
     protected async deleteDataSource(dataSource: string): Promise<void> {
@@ -456,7 +464,12 @@ export default abstract class DatabaseService {
 
     public async fetchAnnotations(dataSourceNames: string[]): Promise<Annotation[]> {
         const aggregateDataSourceName = dataSourceNames.sort().join(", ");
-        if (!this.dataSourceToAnnotationsMap.has(aggregateDataSourceName)) {
+        const hasAnnotations = this.dataSourceToAnnotationsMap.has(aggregateDataSourceName);
+        const hasDescriptions = this.dataSourceToAnnotationsMap
+            .get(aggregateDataSourceName)
+            ?.some((annotation) => !!annotation.description);
+        const shouldHaveDescriptions = dataSourceNames.includes(this.SOURCE_METADATA_TABLE);
+        if (!hasAnnotations || (!hasDescriptions && shouldHaveDescriptions)) {
             const sql = new SQLBuilder()
                 .select("column_name, data_type")
                 .from('information_schema"."columns')

diff --git a/packages/core/state/selection/logics.ts b/packages/core/state/selection/logics.ts
@@ -555,6 +555,8 @@ const changeSourceMetadataLogic = createLogic({
         );
         if (selectedSourceMetadata) {
             await databaseService.prepareSourceMetadata(selectedSourceMetadata);
+        } else {
+            await databaseService.deleteSourceMetadata();
         }
 
         dispatch(metadata.actions.requestAnnotations());
@@ -577,6 +579,8 @@ const addQueryLogic = createLogic({
             await databaseService.prepareDataSources(newQuery.parts.sources);
             if (newQuery.parts.sourceMetadata) {
                 await databaseService.prepareSourceMetadata(newQuery.parts.sourceMetadata);
+            } else {
+                await databaseService.deleteSourceMetadata();
             }
             // Hide warning pop-up if present and remove datasource error from state
             dispatch(removeDataSourceReloadError());

diff --git a/packages/core/state/selection/test/logics.test.ts b/packages/core/state/selection/test/logics.test.ts
@@ -24,6 +24,7 @@ import {
     selectNearbyFile,
     SET_SORT_COLUMN,
     changeDataSources,
+    changeSourceMetadata,
 } from "../actions";
 import { initialState, interaction } from "../../";
 import Annotation from "../../../entity/Annotation";
@@ -40,6 +41,7 @@ import FileSort, { SortOrder } from "../../../entity/FileSort";
 import { DatasetService } from "../../../services";
 import { DataSource } from "../../../services/DataSourceService";
 import HttpFileService from "../../../services/FileService/HttpFileService";
+import DatabaseServiceNoop from "../../../services/DatabaseService/DatabaseServiceNoop";
 import FileDownloadServiceNoop from "../../../services/FileDownloadService/FileDownloadServiceNoop";
 
 describe("Selection logics", () => {
@@ -947,7 +949,17 @@ describe("Selection logics", () => {
         it("dispatches new hierarchy, filters, sort, source, & opened folders from given URL", async () => {
             // Arrange
             const annotations = annotationsJson.map((annotation) => new Annotation(annotation));
+            class MockDatabaseService extends DatabaseServiceNoop {
+                public deleteSourceMetadata(): Promise<void> {
+                    return Promise.resolve();
+                }
+            }
             const state = mergeState(initialState, {
+                interaction: {
+                    platformDependentServices: {
+                        databaseService: new MockDatabaseService(),
+                    },
+                },
                 metadata: {
                     annotations,
                     dataSources: mockDataSources,
@@ -998,6 +1010,7 @@ describe("Selection logics", () => {
                     payload: sortColumn,
                 })
             ).to.be.true;
+            expect(actions.includesMatch(changeSourceMetadata())).to.be.true;
             expect(actions.includesMatch(changeDataSources(mockDataSources))).to.be.true;
         });
     });