From fd8067d9638425473e8b7a30190b1f0975e0ba5c Mon Sep 17 00:00:00 2001 From: Laurent Nguyen Date: Thu, 6 Mar 2025 10:44:41 +0100 Subject: [PATCH 1/3] For all control plane operations, do not use ARM for Fabric. Enable "delete container" for fabric native. --- src/Common/dataAccess/createCollection.ts | 3 ++- src/Common/dataAccess/deleteCollection.ts | 3 ++- src/Common/dataAccess/readDatabaseOffer.ts | 7 ++++--- src/Common/dataAccess/updateCollection.ts | 4 +++- src/Common/dataAccess/updateOffer.ts | 3 ++- src/Explorer/ContextMenuButtonFactory.tsx | 2 +- 6 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/Common/dataAccess/createCollection.ts b/src/Common/dataAccess/createCollection.ts index b5afd70a3..78ef4f488 100644 --- a/src/Common/dataAccess/createCollection.ts +++ b/src/Common/dataAccess/createCollection.ts @@ -1,4 +1,5 @@ import { ContainerRequest, ContainerResponse, DatabaseRequest, DatabaseResponse, RequestOptions } from "@azure/cosmos"; +import { isFabricNative } from "Platform/Fabric/FabricUtil"; import { AuthType } from "../../AuthType"; import * as DataModels from "../../Contracts/DataModels"; import { useDatabases } from "../../Explorer/useDatabases"; @@ -24,7 +25,7 @@ export const createCollection = async (params: DataModels.CreateCollectionParams ); try { let collection: DataModels.Collection; - if (userContext.authType === AuthType.AAD && !userContext.features.enableSDKoperations) { + if (!isFabricNative() && userContext.authType === AuthType.AAD && !userContext.features.enableSDKoperations) { if (params.createNewDatabase) { const createDatabaseParams: DataModels.CreateDatabaseParams = { autoPilotMaxThroughput: params.autoPilotMaxThroughput, diff --git a/src/Common/dataAccess/deleteCollection.ts b/src/Common/dataAccess/deleteCollection.ts index f83126dd1..e786e5ea7 100644 --- a/src/Common/dataAccess/deleteCollection.ts +++ b/src/Common/dataAccess/deleteCollection.ts @@ -1,3 +1,4 @@ +import { isFabric } from "Platform/Fabric/FabricUtil"; import { AuthType } from "../../AuthType"; import { userContext } from "../../UserContext"; import { deleteCassandraTable } from "../../Utils/arm/generatedClients/cosmos/cassandraResources"; @@ -12,7 +13,7 @@ import { handleError } from "../ErrorHandlingUtils"; export async function deleteCollection(databaseId: string, collectionId: string): Promise { const clearMessage = logConsoleProgress(`Deleting container ${collectionId}`); try { - if (userContext.authType === AuthType.AAD && !userContext.features.enableSDKoperations) { + if (userContext.authType === AuthType.AAD && !userContext.features.enableSDKoperations && !isFabric()) { await deleteCollectionWithARM(databaseId, collectionId); } else { await client().database(databaseId).container(collectionId).delete(); diff --git a/src/Common/dataAccess/readDatabaseOffer.ts b/src/Common/dataAccess/readDatabaseOffer.ts index c0c695ad1..ba8019367 100644 --- a/src/Common/dataAccess/readDatabaseOffer.ts +++ b/src/Common/dataAccess/readDatabaseOffer.ts @@ -1,4 +1,4 @@ -import { isFabric, isFabricMirroredKey } from "Platform/Fabric/FabricUtil"; +import { isFabric, isFabricMirroredKey, isFabricNative } from "Platform/Fabric/FabricUtil"; import { AuthType } from "../../AuthType"; import { Offer, ReadDatabaseOfferParams } from "../../Contracts/DataModels"; import { userContext } from "../../UserContext"; @@ -11,8 +11,9 @@ import { handleError } from "../ErrorHandlingUtils"; import { readOfferWithSDK } from "./readOfferWithSDK"; export const readDatabaseOffer = async (params: ReadDatabaseOfferParams): Promise => { - if (isFabricMirroredKey()) { - // TODO This works, but is very slow, because it requests the token, so we skip for now + if (isFabricMirroredKey() || isFabricNative()) { + // For Fabric Mirroring, it is slow, because it requests the token and we don't need it. + // For Fabric Native, it is not supported. console.error("Skiping readDatabaseOffer for Fabric"); return undefined; } diff --git a/src/Common/dataAccess/updateCollection.ts b/src/Common/dataAccess/updateCollection.ts index 15515f5b5..263960663 100644 --- a/src/Common/dataAccess/updateCollection.ts +++ b/src/Common/dataAccess/updateCollection.ts @@ -1,4 +1,5 @@ import { ContainerDefinition, RequestOptions } from "@azure/cosmos"; +import { isFabric } from "Platform/Fabric/FabricUtil"; import { AuthType } from "../../AuthType"; import { Collection } from "../../Contracts/DataModels"; import { userContext } from "../../UserContext"; @@ -36,7 +37,8 @@ export async function updateCollection( if ( userContext.authType === AuthType.AAD && !userContext.features.enableSDKoperations && - userContext.apiType !== "Tables" + userContext.apiType !== "Tables" && + !isFabric() ) { collection = await updateCollectionWithARM(databaseId, collectionId, newCollection); } else { diff --git a/src/Common/dataAccess/updateOffer.ts b/src/Common/dataAccess/updateOffer.ts index 4d26ca68d..cd20fcd4c 100644 --- a/src/Common/dataAccess/updateOffer.ts +++ b/src/Common/dataAccess/updateOffer.ts @@ -1,4 +1,5 @@ import { OfferDefinition, RequestOptions } from "@azure/cosmos"; +import { isFabric } from "Platform/Fabric/FabricUtil"; import { AuthType } from "../../AuthType"; import { Offer, SDKOfferDefinition, ThroughputBucket, UpdateOfferParams } from "../../Contracts/DataModels"; import { userContext } from "../../UserContext"; @@ -56,7 +57,7 @@ export const updateOffer = async (params: UpdateOfferParams): Promise => const clearMessage = logConsoleProgress(`Updating offer for ${offerResourceText}`); try { - if (userContext.authType === AuthType.AAD && !userContext.features.enableSDKoperations) { + if (userContext.authType === AuthType.AAD && !userContext.features.enableSDKoperations && !isFabric()) { if (params.collectionId) { updatedOffer = await updateCollectionOfferWithARM(params); } else if (userContext.apiType === "Tables") { diff --git a/src/Explorer/ContextMenuButtonFactory.tsx b/src/Explorer/ContextMenuButtonFactory.tsx index 890e2f86f..8108cbbb2 100644 --- a/src/Explorer/ContextMenuButtonFactory.tsx +++ b/src/Explorer/ContextMenuButtonFactory.tsx @@ -146,7 +146,7 @@ export const createCollectionContextMenuButton = ( }); } - if (configContext.platform !== Platform.Fabric) { + if (!isFabric() || (isFabric() && !userContext.fabricContext?.isReadOnly)) { items.push({ iconSrc: DeleteCollectionIcon, onClick: (lastFocusedElement?: React.RefObject) => { From 7045125ddc14f8af1a269dd1c6889d38cf244d24 Mon Sep 17 00:00:00 2001 From: Laurent Nguyen Date: Thu, 6 Mar 2025 11:20:23 +0100 Subject: [PATCH 2/3] Fix unit test --- src/Explorer/Tree/treeNodeUtil.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Explorer/Tree/treeNodeUtil.test.ts b/src/Explorer/Tree/treeNodeUtil.test.ts index 2c7af8021..0af149edd 100644 --- a/src/Explorer/Tree/treeNodeUtil.test.ts +++ b/src/Explorer/Tree/treeNodeUtil.test.ts @@ -559,6 +559,7 @@ describe("createDatabaseTreeNodes", () => { updateUserContext({ fabricContext: { artifactType: CosmosDbArtifactType.MIRRORED_KEY, + isReadOnly: false, } as FabricContext, }); }, From fa8ada91945349f40ed2ebe72ea3f581fa4fd323 Mon Sep 17 00:00:00 2001 From: Laurent Nguyen Date: Thu, 6 Mar 2025 12:38:22 +0100 Subject: [PATCH 3/3] Fix tre note tests with proper fabric config. Add new fabric non-readonly test. --- .../__snapshots__/treeNodeUtil.test.ts.snap | 171 +++++++++++++++++- src/Explorer/Tree/treeNodeUtil.test.ts | 75 +++++--- 2 files changed, 218 insertions(+), 28 deletions(-) diff --git a/src/Explorer/Tree/__snapshots__/treeNodeUtil.test.ts.snap b/src/Explorer/Tree/__snapshots__/treeNodeUtil.test.ts.snap index 787443041..9814a72fb 100644 --- a/src/Explorer/Tree/__snapshots__/treeNodeUtil.test.ts.snap +++ b/src/Explorer/Tree/__snapshots__/treeNodeUtil.test.ts.snap @@ -740,7 +740,7 @@ exports[`createDatabaseTreeNodes generates the correct tree structure for the Mo ] `; -exports[`createDatabaseTreeNodes generates the correct tree structure for the SQL API, on Fabric 1`] = ` +exports[`createDatabaseTreeNodes generates the correct tree structure for the SQL API, on Fabric non read-only 1`] = ` [ { "children": [ @@ -753,6 +753,12 @@ exports[`createDatabaseTreeNodes generates the correct tree structure for the SQ "label": "New SQL Query", "onClick": [Function], }, + { + "iconSrc": {}, + "label": "Delete Container", + "onClick": [Function], + "styleClass": "deleteCollectionMenuItem", + }, ], "iconSrc": , + "isExpanded": true, + "isSelected": [Function], + "label": "standardCollection", + "onClick": [Function], + "onCollapsed": [Function], + "onContextMenuOpen": [Function], + "onExpanded": [Function], + }, + { + "children": undefined, + "className": "collectionNode", + "contextMenu": [ + { + "iconSrc": {}, + "label": "New SQL Query", + "onClick": [Function], + }, + ], + "iconSrc": , + "isExpanded": true, + "isSelected": [Function], + "label": "conflictsCollection", + "onClick": [Function], + "onCollapsed": [Function], + "onContextMenuOpen": [Function], + "onExpanded": [Function], + }, + ], + "className": "databaseNode", + "contextMenu": undefined, + "iconSrc": , + "isExpanded": true, + "isSelected": [Function], + "label": "standardDb", + "onCollapsed": [Function], + "onContextMenuOpen": [Function], + "onExpanded": [Function], + }, + { + "children": [ + { + "children": undefined, + "className": "collectionNode", + "contextMenu": [ + { + "iconSrc": {}, + "label": "New SQL Query", + "onClick": [Function], + }, + ], + "iconSrc": , + "isExpanded": true, + "isSelected": [Function], + "label": "sampleItemsCollection", + "onClick": [Function], + "onCollapsed": [Function], + "onContextMenuOpen": [Function], + "onExpanded": [Function], + }, + ], + "className": "databaseNode", + "contextMenu": undefined, + "iconSrc": , + "isExpanded": true, + "isSelected": [Function], + "label": "sharedDatabase", + "onCollapsed": [Function], + "onContextMenuOpen": [Function], + "onExpanded": [Function], + }, + { + "children": [ + { + "children": undefined, + "className": "collectionNode", + "contextMenu": [ + { + "iconSrc": {}, + "label": "New SQL Query", + "onClick": [Function], + }, + ], + "iconSrc": , + "isExpanded": true, + "isSelected": [Function], + "label": "schemaCollection", + "onClick": [Function], + "onCollapsed": [Function], + "onContextMenuOpen": [Function], + "onExpanded": [Function], + }, + { + "className": "loadMoreNode", + "label": "load more", + "onClick": [Function], + }, + ], + "className": "databaseNode", + "contextMenu": undefined, + "iconSrc": , + "isExpanded": true, + "isSelected": [Function], + "label": "giganticDatabase", + "onCollapsed": [Function], + "onContextMenuOpen": [Function], + "onExpanded": [Function], + }, +] +`; + exports[`createDatabaseTreeNodes generates the correct tree structure for the SQL API, on Portal 1`] = ` [ { @@ -972,7 +1135,7 @@ exports[`createDatabaseTreeNodes generates the correct tree structure for the SQ }, ], "isSelected": [Function], - "label": "mockSproc3", + "label": "mockSproc4", "onClick": [Function], }, ], @@ -990,7 +1153,7 @@ exports[`createDatabaseTreeNodes generates the correct tree structure for the SQ }, ], "isSelected": [Function], - "label": "mockUdf3", + "label": "mockUdf4", "onClick": [Function], }, ], @@ -1008,7 +1171,7 @@ exports[`createDatabaseTreeNodes generates the correct tree structure for the SQ }, ], "isSelected": [Function], - "label": "mockTrigger3", + "label": "mockTrigger4", "onClick": [Function], }, ], diff --git a/src/Explorer/Tree/treeNodeUtil.test.ts b/src/Explorer/Tree/treeNodeUtil.test.ts index 0af149edd..f9298a428 100644 --- a/src/Explorer/Tree/treeNodeUtil.test.ts +++ b/src/Explorer/Tree/treeNodeUtil.test.ts @@ -17,7 +17,7 @@ import { } from "Explorer/Tree/treeNodeUtil"; import { useDatabases } from "Explorer/useDatabases"; import { useSelectedNode } from "Explorer/useSelectedNode"; -import { FabricContext, updateUserContext } from "UserContext"; +import { FabricContext, updateUserContext, UserContext } from "UserContext"; import PromiseSource from "Utils/PromiseSource"; import { useSidePanel } from "hooks/useSidePanel"; import { useTabs } from "hooks/useTabs"; @@ -361,9 +361,30 @@ describe("createDatabaseTreeNodes", () => { }); }); - it.each<[string, Platform, boolean, Partial]>([ - ["the SQL API, on Fabric", Platform.Fabric, false, { capabilities: [], enableMultipleWriteLocations: true }], - ["the SQL API, on Portal", Platform.Portal, false, { capabilities: [], enableMultipleWriteLocations: true }], + it.each<[string, Platform, boolean, Partial, Partial]>([ + [ + "the SQL API, on Fabric read-only", + Platform.Fabric, + false, + { capabilities: [], enableMultipleWriteLocations: true }, + { fabricContext: { isReadOnly: true } as FabricContext }, + ], + [ + "the SQL API, on Fabric non read-only", + Platform.Fabric, + false, + { capabilities: [], enableMultipleWriteLocations: true }, + { fabricContext: { isReadOnly: false } as FabricContext }, + ], + [ + "the SQL API, on Portal", + Platform.Portal, + false, + { capabilities: [], enableMultipleWriteLocations: true }, + { + fabricContext: undefined, + }, + ], [ "the Cassandra API, serverless, on Hosted", Platform.Hosted, @@ -374,6 +395,7 @@ describe("createDatabaseTreeNodes", () => { { name: CapabilityNames.EnableServerless, description: "" }, ], }, + { fabricContext: undefined }, ], [ "the Mongo API, with Notebooks and Phoenix features, on Emulator", @@ -382,26 +404,31 @@ describe("createDatabaseTreeNodes", () => { { capabilities: [{ name: CapabilityNames.EnableMongo, description: "" }], }, + { fabricContext: undefined }, ], - ])("generates the correct tree structure for %s", (_, platform, isNotebookEnabled, dbAccountProperties) => { - useNotebook.setState({ isPhoenixFeatures: isNotebookEnabled }); - updateConfigContext({ platform }); - updateUserContext({ - databaseAccount: { - properties: { - enableMultipleWriteLocations: true, - ...dbAccountProperties, - }, - } as unknown as DataModels.DatabaseAccount, - }); - const nodes = createDatabaseTreeNodes( - explorer, - isNotebookEnabled, - useDatabases.getState().databases, - refreshActiveTab, - ); - expect(nodes).toMatchSnapshot(); - }); + ])( + "generates the correct tree structure for %s", + (_, platform, isNotebookEnabled, dbAccountProperties, userContext) => { + useNotebook.setState({ isPhoenixFeatures: isNotebookEnabled }); + updateConfigContext({ platform }); + updateUserContext({ + ...userContext, + databaseAccount: { + properties: { + enableMultipleWriteLocations: true, + ...dbAccountProperties, + }, + } as unknown as DataModels.DatabaseAccount, + }); + const nodes = createDatabaseTreeNodes( + explorer, + isNotebookEnabled, + useDatabases.getState().databases, + refreshActiveTab, + ); + expect(nodes).toMatchSnapshot(); + }, + ); // The above tests focused on the tree structure. The below tests focus on some core behaviors of the nodes. // They are not exhaustive, because exhaustive tests here require a lot of mocking and can become very brittle. @@ -559,7 +586,7 @@ describe("createDatabaseTreeNodes", () => { updateUserContext({ fabricContext: { artifactType: CosmosDbArtifactType.MIRRORED_KEY, - isReadOnly: false, + isReadOnly: true, } as FabricContext, }); },