From 561e07a6890cd05c257d0049226a1ca6321c42a2 Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Fri, 14 Apr 2023 15:59:21 +0000 Subject: [PATCH 1/3] Sort database items by dateAdded Before selecting an existing database, let's sort them by date. We're opting to exclude databases with errors during the sorting process. --- .../ql-vscode/src/skeleton-query-wizard.ts | 42 ++++--- .../skeleton-query-wizard.test.ts | 118 +++++++++--------- 2 files changed, 88 insertions(+), 72 deletions(-) diff --git a/extensions/ql-vscode/src/skeleton-query-wizard.ts b/extensions/ql-vscode/src/skeleton-query-wizard.ts index baa3178f8cf..4ae9c973bc0 100644 --- a/extensions/ql-vscode/src/skeleton-query-wizard.ts +++ b/extensions/ql-vscode/src/skeleton-query-wizard.ts @@ -273,12 +273,8 @@ export class SkeletonQueryWizard { databaseNwo: string, databaseItems: readonly DatabaseItem[], ): Promise { - const dbItems = databaseItems || []; - const dbs = dbItems.filter( - (db) => - db.language === language && - db.name === databaseNwo && - db.error === undefined, + const dbs = databaseItems.filter( + (db) => db.language === language && db.name === databaseNwo, ); if (dbs.length === 0) { @@ -291,10 +287,7 @@ export class SkeletonQueryWizard { language: string, databaseItems: readonly DatabaseItem[], ): Promise { - const dbItems = databaseItems || []; - const dbs = dbItems.filter( - (db) => db.language === language && db.error === undefined, - ); + const dbs = databaseItems.filter((db) => db.language === language); if (dbs.length === 0) { return undefined; } @@ -308,19 +301,38 @@ export class SkeletonQueryWizard { const defaultDatabaseNwo = QUERY_LANGUAGE_TO_DATABASE_REPO[this.language]; + const dbItems = await this.sortDatabaseItemsByDateAdded( + this.databaseManager.databaseItems, + ); + const defaultDatabaseItem = await this.findDatabaseItemByNwo( this.language, defaultDatabaseNwo, - this.databaseManager.databaseItems, + dbItems, ); if (defaultDatabaseItem !== undefined) { return defaultDatabaseItem; } - return await this.findDatabaseItemByLanguage( - this.language, - this.databaseManager.databaseItems, - ); + return await this.findDatabaseItemByLanguage(this.language, dbItems); + } + + public async sortDatabaseItemsByDateAdded( + databaseItems: readonly DatabaseItem[], + ) { + const validDbItems = databaseItems.filter((db) => db.error === undefined); + + return validDbItems.sort((a, b) => { + if (a.dateAdded === undefined) { + return -1; + } + + if (b.dateAdded === undefined) { + return 1; + } + + return a.dateAdded - b.dateAdded; + }); } } diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts index 87fbbc1e5b3..511bc2e263b 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts @@ -330,37 +330,6 @@ describe("SkeletonQueryWizard", () => { JSON.stringify(mockDbItem), ); }); - - it("should ignore databases with errors", async () => { - const mockDbItem = createMockDB(dir, { - language: "ruby", - dateAdded: 123, - } as FullDatabaseOptions); - const mockDbItem2 = createMockDB(dir, { - language: "javascript", - } as FullDatabaseOptions); - const mockDbItem3 = createMockDB(dir, { - language: "ruby", - dateAdded: 345, - } as FullDatabaseOptions); - - jest.spyOn(mockDbItem, "name", "get").mockReturnValue("mock-name"); - jest.spyOn(mockDbItem3, "name", "get").mockReturnValue(mockDbItem.name); - - jest - .spyOn(mockDbItem, "error", "get") - .mockReturnValue(asError("database go boom!")); - - const databaseItem = await wizard.findDatabaseItemByNwo( - mockDbItem.language, - mockDbItem.name, - [mockDbItem, mockDbItem2, mockDbItem3], - ); - - expect(JSON.stringify(databaseItem)).toEqual( - JSON.stringify(mockDbItem3), - ); - }); }); describe("when the item doesn't exist", () => { @@ -396,32 +365,6 @@ describe("SkeletonQueryWizard", () => { expect(databaseItem).toEqual(mockDbItem); }); - - it("should ignore databases with errors", async () => { - const mockDbItem = createMockDB(dir, { - language: "ruby", - } as FullDatabaseOptions); - const mockDbItem2 = createMockDB(dir, { - language: "javascript", - } as FullDatabaseOptions); - const mockDbItem3 = createMockDB(dir, { - language: "ruby", - } as FullDatabaseOptions); - - jest - .spyOn(mockDbItem, "error", "get") - .mockReturnValue(asError("database go boom!")); - - const databaseItem = await wizard.findDatabaseItemByLanguage("ruby", [ - mockDbItem, - mockDbItem2, - mockDbItem3, - ]); - - expect(JSON.stringify(databaseItem)).toEqual( - JSON.stringify(mockDbItem3), - ); - }); }); describe("when the item doesn't exist", () => { @@ -550,4 +493,65 @@ describe("SkeletonQueryWizard", () => { }); }); }); + + describe("sortDatabaseItemsByDateAdded", () => { + describe("should return a sorted list", () => { + it("should sort the items by dateAdded", async () => { + const mockDbItem = createMockDB(dir, { + dateAdded: 678, + } as FullDatabaseOptions); + const mockDbItem2 = createMockDB(dir, { + dateAdded: 123, + } as FullDatabaseOptions); + const mockDbItem3 = createMockDB(dir, { + dateAdded: undefined, + } as FullDatabaseOptions); + const mockDbItem4 = createMockDB(dir, { + dateAdded: 345, + } as FullDatabaseOptions); + + const sortedList = await wizard.sortDatabaseItemsByDateAdded([ + mockDbItem, + mockDbItem2, + mockDbItem3, + mockDbItem4, + ]); + + expect(sortedList).toEqual([ + mockDbItem3, + mockDbItem2, + mockDbItem4, + mockDbItem, + ]); + }); + + it("should ignore databases with errors", async () => { + const mockDbItem = createMockDB(dir, { + dateAdded: 678, + } as FullDatabaseOptions); + const mockDbItem2 = createMockDB(dir, { + dateAdded: undefined, + } as FullDatabaseOptions); + const mockDbItem3 = createMockDB(dir, { + dateAdded: 345, + } as FullDatabaseOptions); + const mockDbItem4 = createMockDB(dir, { + dateAdded: 123, + } as FullDatabaseOptions); + + jest + .spyOn(mockDbItem, "error", "get") + .mockReturnValue(asError("database go boom!")); + + const sortedList = await wizard.sortDatabaseItemsByDateAdded([ + mockDbItem, + mockDbItem2, + mockDbItem3, + mockDbItem4, + ]); + + expect(sortedList).toEqual([mockDbItem2, mockDbItem4, mockDbItem3]); + }); + }); + }); }); From 02dffe08d5d86eb5e1a8ee24d45dc8377a47efee Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Fri, 14 Apr 2023 16:51:16 +0000 Subject: [PATCH 2/3] Fetch the latest database from the new sorted list And add tests to check this. I've had to adapt the existing `findExistingDatabaseItem` method so receive params so that I'm better able to send it a language and a list of database items. --- .../ql-vscode/src/skeleton-query-wizard.ts | 39 +++++------ .../skeleton-query-wizard.test.ts | 69 +++++++++++++++++++ 2 files changed, 88 insertions(+), 20 deletions(-) diff --git a/extensions/ql-vscode/src/skeleton-query-wizard.ts b/extensions/ql-vscode/src/skeleton-query-wizard.ts index 4ae9c973bc0..1fe3042e65e 100644 --- a/extensions/ql-vscode/src/skeleton-query-wizard.ts +++ b/extensions/ql-vscode/src/skeleton-query-wizard.ts @@ -253,11 +253,18 @@ export class SkeletonQueryWizard { } private async selectOrDownloadDatabase() { + if (this.language === undefined) { + throw new Error("Language is undefined"); + } + if (this.qlPackStoragePath === undefined) { throw new Error("QL Pack storage path is undefined"); } - const existingDatabaseItem = await this.findExistingDatabaseItem(); + const existingDatabaseItem = await this.findExistingDatabaseItem( + this.language, + this.databaseManager.databaseItems, + ); if (existingDatabaseItem) { // select the found database @@ -277,10 +284,7 @@ export class SkeletonQueryWizard { (db) => db.language === language && db.name === databaseNwo, ); - if (dbs.length === 0) { - return undefined; - } - return dbs[0]; + return dbs.pop(); } public async findDatabaseItemByLanguage( @@ -288,25 +292,20 @@ export class SkeletonQueryWizard { databaseItems: readonly DatabaseItem[], ): Promise { const dbs = databaseItems.filter((db) => db.language === language); - if (dbs.length === 0) { - return undefined; - } - return dbs[0]; - } - private async findExistingDatabaseItem() { - if (this.language === undefined) { - throw new Error("Language is undefined"); - } + return dbs.pop(); + } - const defaultDatabaseNwo = QUERY_LANGUAGE_TO_DATABASE_REPO[this.language]; + public async findExistingDatabaseItem( + language: string, + databaseItems: readonly DatabaseItem[], + ): Promise { + const defaultDatabaseNwo = QUERY_LANGUAGE_TO_DATABASE_REPO[language]; - const dbItems = await this.sortDatabaseItemsByDateAdded( - this.databaseManager.databaseItems, - ); + const dbItems = await this.sortDatabaseItemsByDateAdded(databaseItems); const defaultDatabaseItem = await this.findDatabaseItemByNwo( - this.language, + language, defaultDatabaseNwo, dbItems, ); @@ -315,7 +314,7 @@ export class SkeletonQueryWizard { return defaultDatabaseItem; } - return await this.findDatabaseItemByLanguage(this.language, dbItems); + return await this.findDatabaseItemByLanguage(language, dbItems); } public async sortDatabaseItemsByDateAdded( diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts index 511bc2e263b..de868687185 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts @@ -554,4 +554,73 @@ describe("SkeletonQueryWizard", () => { }); }); }); + + describe("findExistingDatabaseItem", () => { + describe("when there are multiple items with the same name", () => { + it("should choose the latest one", async () => { + const mockDbItem = createMockDB(dir, { + language: "javascript", + dateAdded: 456, + } as FullDatabaseOptions); + const mockDbItem2 = createMockDB(dir, { + language: "ruby", + dateAdded: 789, + } as FullDatabaseOptions); + const mockDbItem3 = createMockDB(dir, { + language: "javascript", + dateAdded: 123, + } as FullDatabaseOptions); + const mockDbItem4 = createMockDB(dir, { + language: "javascript", + dateAdded: undefined, + } as FullDatabaseOptions); + + jest + .spyOn(mockDbItem, "name", "get") + .mockReturnValue(QUERY_LANGUAGE_TO_DATABASE_REPO["javascript"]); + jest + .spyOn(mockDbItem2, "name", "get") + .mockReturnValue(QUERY_LANGUAGE_TO_DATABASE_REPO["javascript"]); + + const databaseItem = await wizard.findExistingDatabaseItem( + "javascript", + [mockDbItem, mockDbItem2, mockDbItem3, mockDbItem4], + ); + + expect(JSON.stringify(databaseItem)).toEqual( + JSON.stringify(mockDbItem), + ); + }); + }); + + describe("when there are multiple items with the same language", () => { + it("should choose the latest one", async () => { + const mockDbItem = createMockDB(dir, { + language: "ruby", + dateAdded: 789, + } as FullDatabaseOptions); + const mockDbItem2 = createMockDB(dir, { + language: "javascript", + dateAdded: 456, + } as FullDatabaseOptions); + const mockDbItem3 = createMockDB(dir, { + language: "ruby", + dateAdded: 123, + } as FullDatabaseOptions); + const mockDbItem4 = createMockDB(dir, { + language: "javascript", + dateAdded: undefined, + } as FullDatabaseOptions); + + const databaseItem = await wizard.findExistingDatabaseItem( + "javascript", + [mockDbItem, mockDbItem2, mockDbItem3, mockDbItem4], + ); + + expect(JSON.stringify(databaseItem)).toEqual( + JSON.stringify(mockDbItem2), + ); + }); + }); + }); }); From 02e17516d9331c9b8c7d56b792a56c02d1fba978 Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Tue, 18 Apr 2023 14:10:24 +0000 Subject: [PATCH 3/3] Convert db item methods to be static So that we make it clear we should be passing state as params instead of reading it off `this`. --- .../ql-vscode/src/skeleton-query-wizard.ts | 28 ++++++---- .../skeleton-query-wizard.test.ts | 52 ++++++++++--------- 2 files changed, 45 insertions(+), 35 deletions(-) diff --git a/extensions/ql-vscode/src/skeleton-query-wizard.ts b/extensions/ql-vscode/src/skeleton-query-wizard.ts index 1fe3042e65e..f6c8a38ad57 100644 --- a/extensions/ql-vscode/src/skeleton-query-wizard.ts +++ b/extensions/ql-vscode/src/skeleton-query-wizard.ts @@ -261,10 +261,11 @@ export class SkeletonQueryWizard { throw new Error("QL Pack storage path is undefined"); } - const existingDatabaseItem = await this.findExistingDatabaseItem( - this.language, - this.databaseManager.databaseItems, - ); + const existingDatabaseItem = + await SkeletonQueryWizard.findExistingDatabaseItem( + this.language, + this.databaseManager.databaseItems, + ); if (existingDatabaseItem) { // select the found database @@ -275,7 +276,7 @@ export class SkeletonQueryWizard { } } - public async findDatabaseItemByNwo( + public static async findDatabaseItemByNwo( language: string, databaseNwo: string, databaseItems: readonly DatabaseItem[], @@ -287,7 +288,7 @@ export class SkeletonQueryWizard { return dbs.pop(); } - public async findDatabaseItemByLanguage( + public static async findDatabaseItemByLanguage( language: string, databaseItems: readonly DatabaseItem[], ): Promise { @@ -296,15 +297,17 @@ export class SkeletonQueryWizard { return dbs.pop(); } - public async findExistingDatabaseItem( + public static async findExistingDatabaseItem( language: string, databaseItems: readonly DatabaseItem[], ): Promise { const defaultDatabaseNwo = QUERY_LANGUAGE_TO_DATABASE_REPO[language]; - const dbItems = await this.sortDatabaseItemsByDateAdded(databaseItems); + const dbItems = await SkeletonQueryWizard.sortDatabaseItemsByDateAdded( + databaseItems, + ); - const defaultDatabaseItem = await this.findDatabaseItemByNwo( + const defaultDatabaseItem = await SkeletonQueryWizard.findDatabaseItemByNwo( language, defaultDatabaseNwo, dbItems, @@ -314,10 +317,13 @@ export class SkeletonQueryWizard { return defaultDatabaseItem; } - return await this.findDatabaseItemByLanguage(language, dbItems); + return await SkeletonQueryWizard.findDatabaseItemByLanguage( + language, + dbItems, + ); } - public async sortDatabaseItemsByDateAdded( + public static async sortDatabaseItemsByDateAdded( databaseItems: readonly DatabaseItem[], ) { const validDbItems = databaseItems.filter((db) => db.error === undefined); diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts index de868687185..f083ed83df3 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts @@ -320,7 +320,7 @@ describe("SkeletonQueryWizard", () => { jest.spyOn(mockDbItem, "name", "get").mockReturnValue("mock-name"); - const databaseItem = await wizard.findDatabaseItemByNwo( + const databaseItem = await SkeletonQueryWizard.findDatabaseItemByNwo( mockDbItem.language, mockDbItem.name, [mockDbItem, mockDbItem2], @@ -337,7 +337,7 @@ describe("SkeletonQueryWizard", () => { const mockDbItem = createMockDB(dir); const mockDbItem2 = createMockDB(dir); - const databaseItem = await wizard.findDatabaseItemByNwo( + const databaseItem = await SkeletonQueryWizard.findDatabaseItemByNwo( "ruby", "mock-nwo", [mockDbItem, mockDbItem2], @@ -358,10 +358,11 @@ describe("SkeletonQueryWizard", () => { language: "javascript", } as FullDatabaseOptions); - const databaseItem = await wizard.findDatabaseItemByLanguage("ruby", [ - mockDbItem, - mockDbItem2, - ]); + const databaseItem = + await SkeletonQueryWizard.findDatabaseItemByLanguage("ruby", [ + mockDbItem, + mockDbItem2, + ]); expect(databaseItem).toEqual(mockDbItem); }); @@ -372,10 +373,11 @@ describe("SkeletonQueryWizard", () => { const mockDbItem = createMockDB(dir); const mockDbItem2 = createMockDB(dir); - const databaseItem = await wizard.findDatabaseItemByLanguage("ruby", [ - mockDbItem, - mockDbItem2, - ]); + const databaseItem = + await SkeletonQueryWizard.findDatabaseItemByLanguage("ruby", [ + mockDbItem, + mockDbItem2, + ]); expect(databaseItem).toBeUndefined(); }); @@ -510,12 +512,13 @@ describe("SkeletonQueryWizard", () => { dateAdded: 345, } as FullDatabaseOptions); - const sortedList = await wizard.sortDatabaseItemsByDateAdded([ - mockDbItem, - mockDbItem2, - mockDbItem3, - mockDbItem4, - ]); + const sortedList = + await SkeletonQueryWizard.sortDatabaseItemsByDateAdded([ + mockDbItem, + mockDbItem2, + mockDbItem3, + mockDbItem4, + ]); expect(sortedList).toEqual([ mockDbItem3, @@ -543,12 +546,13 @@ describe("SkeletonQueryWizard", () => { .spyOn(mockDbItem, "error", "get") .mockReturnValue(asError("database go boom!")); - const sortedList = await wizard.sortDatabaseItemsByDateAdded([ - mockDbItem, - mockDbItem2, - mockDbItem3, - mockDbItem4, - ]); + const sortedList = + await SkeletonQueryWizard.sortDatabaseItemsByDateAdded([ + mockDbItem, + mockDbItem2, + mockDbItem3, + mockDbItem4, + ]); expect(sortedList).toEqual([mockDbItem2, mockDbItem4, mockDbItem3]); }); @@ -582,7 +586,7 @@ describe("SkeletonQueryWizard", () => { .spyOn(mockDbItem2, "name", "get") .mockReturnValue(QUERY_LANGUAGE_TO_DATABASE_REPO["javascript"]); - const databaseItem = await wizard.findExistingDatabaseItem( + const databaseItem = await SkeletonQueryWizard.findExistingDatabaseItem( "javascript", [mockDbItem, mockDbItem2, mockDbItem3, mockDbItem4], ); @@ -612,7 +616,7 @@ describe("SkeletonQueryWizard", () => { dateAdded: undefined, } as FullDatabaseOptions); - const databaseItem = await wizard.findExistingDatabaseItem( + const databaseItem = await SkeletonQueryWizard.findExistingDatabaseItem( "javascript", [mockDbItem, mockDbItem2, mockDbItem3, mockDbItem4], );