From a3bfc89318f6c819cec4eb3d2b38aa8a179e285c Mon Sep 17 00:00:00 2001 From: sunghyunkang1111 <114709653+sunghyunkang1111@users.noreply.github.com> Date: Wed, 9 Apr 2025 13:04:44 -0500 Subject: [PATCH] Revert "Pk missing fix (#2094)" (#2099) This reverts commit af0a890516b6302b60710b4f54e58b68d88b1857. --- playwright.config.ts | 1 + .../Tabs/DocumentsTabV2/DocumentsTabV2.tsx | 17 +--- .../DocumentsTabV2.test.tsx.snap | 4 - src/Utils/QueryUtils.test.ts | 78 +++------------- src/Utils/QueryUtils.ts | 14 +-- test/fx.ts | 33 +------ test/sql/document.spec.ts | 45 ---------- test/sql/testCases.ts | 88 ------------------- 8 files changed, 21 insertions(+), 259 deletions(-) delete mode 100644 test/sql/document.spec.ts delete mode 100644 test/sql/testCases.ts diff --git a/playwright.config.ts b/playwright.config.ts index 5279a0660..4c5ad3c14 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -1,4 +1,5 @@ import { defineConfig, devices } from "@playwright/test"; + /** * See https://playwright.dev/docs/test-configuration. */ diff --git a/src/Explorer/Tabs/DocumentsTabV2/DocumentsTabV2.tsx b/src/Explorer/Tabs/DocumentsTabV2/DocumentsTabV2.tsx index 65424d881..cef323b0e 100644 --- a/src/Explorer/Tabs/DocumentsTabV2/DocumentsTabV2.tsx +++ b/src/Explorer/Tabs/DocumentsTabV2/DocumentsTabV2.tsx @@ -1046,7 +1046,6 @@ export const DocumentsTabComponent: React.FunctionComponent { - // in case of any kind of failures of accidently changing partition key, restore the original - // so that when user navigates away from current document and comes back, - // it doesnt fail to load due to using the invalid partition keys - selectedDocumentId.partitionKeyValue = originalPartitionKeyValue; onExecutionErrorChange(true); const errorMessage = getErrorMessage(error); useDialog.getState().showOkModalDialog("Update document failed", errorMessage); @@ -2099,8 +2094,8 @@ export const DocumentsTabComponent: React.FunctionComponent -
-
+
+
{!isPreferredApiMongoDB && SELECT * FROM c } -
+
-
+
{isTabActive && selectedDocumentContent && selectedRows.size <= 1 && (
SELECT * FROM c @@ -67,7 +65,6 @@ exports[`Documents tab (noSql API) when rendered should render the page 1`] = ` preferredSize="35%" >
{ version: 2, }; }; - const generatePartitionKeysForPaths = (paths: string[]): DataModels.PartitionKey => { - return { - paths: paths, - kind: "Hash", - version: 2, - }; - }; describe("buildDocumentsQueryPartitionProjections()", () => { it("should return empty string if partition key is undefined", () => { @@ -96,18 +89,6 @@ describe("Query Utils", () => { expect(query).toContain("c.id"); }); - - it("should always include {} for any missing partition keys", () => { - const query = QueryUtils.buildDocumentsQuery( - "", - ["a", "b", "c"], - generatePartitionKeysForPaths(["/a", "/b", "/c"]), - [], - ); - expect(query).toContain('IIF(IS_DEFINED(c["a"]), c["a"], {})'); - expect(query).toContain('IIF(IS_DEFINED(c["b"]), c["b"], {})'); - expect(query).toContain('IIF(IS_DEFINED(c["c"]), c["c"], {})'); - }); }); describe("queryPagesUntilContentPresent()", () => { @@ -220,6 +201,18 @@ describe("Query Utils", () => { expect(expectedPartitionKeyValues).toContain(documentContent["Category"]); }); + it("should extract no partition key values in the case nested partition key", () => { + const singlePartitionKeyDefinition: PartitionKeyDefinition = { + kind: PartitionKeyKind.Hash, + paths: ["/Location.type"], + }; + const partitionKeyValues: PartitionKey[] = extractPartitionKeyValues( + documentContent, + singlePartitionKeyDefinition, + ); + expect(partitionKeyValues.length).toBe(0); + }); + it("should extract all partition key values for hierarchical and nested partition keys", () => { const mixedPartitionKeyDefinition: PartitionKeyDefinition = { kind: PartitionKeyKind.MultiHash, @@ -232,52 +225,5 @@ describe("Query Utils", () => { expect(partitionKeyValues.length).toBe(2); expect(partitionKeyValues).toEqual(["United States", "Point"]); }); - - it("if any partition key is null or empty string, the partitionKeyValues shall match", () => { - const newDocumentContent = { - ...documentContent, - ...{ - Country: null, - Location: { - type: "", - coordinates: [-121.49, 46.206], - }, - }, - }; - - const mixedPartitionKeyDefinition: PartitionKeyDefinition = { - kind: PartitionKeyKind.MultiHash, - paths: ["/Country", "/Location/type"], - }; - const partitionKeyValues: PartitionKey[] = extractPartitionKeyValues( - newDocumentContent, - mixedPartitionKeyDefinition, - ); - expect(partitionKeyValues.length).toBe(2); - expect(partitionKeyValues).toEqual([null, ""]); - }); - - it("if any partition key doesn't exist, it should still set partitionkey value as {}", () => { - const newDocumentContent = { - ...documentContent, - ...{ - Country: null, - Location: { - coordinates: [-121.49, 46.206], - }, - }, - }; - - const mixedPartitionKeyDefinition: PartitionKeyDefinition = { - kind: PartitionKeyKind.MultiHash, - paths: ["/Country", "/Location/type"], - }; - const partitionKeyValues: PartitionKey[] = extractPartitionKeyValues( - newDocumentContent, - mixedPartitionKeyDefinition, - ); - expect(partitionKeyValues.length).toBe(2); - expect(partitionKeyValues).toEqual([null, {}]); - }); }); }); diff --git a/src/Utils/QueryUtils.ts b/src/Utils/QueryUtils.ts index 07822a422..f0b39e4e2 100644 --- a/src/Utils/QueryUtils.ts +++ b/src/Utils/QueryUtils.ts @@ -47,7 +47,6 @@ export function buildDocumentsQueryPartitionProjections( for (const index in partitionKey.paths) { // TODO: Handle "/" in partition key definitions const projectedProperties: string[] = partitionKey.paths[index].split("/").slice(1); - const isSystemPartitionKey: boolean = partitionKey.systemKey || false; let projectedProperty = ""; projectedProperties.forEach((property: string) => { @@ -62,13 +61,8 @@ export function buildDocumentsQueryPartitionProjections( projectedProperty += `[${projection}]`; } }); - const fullAccess = `${collectionAlias}${projectedProperty}`; - if (!isSystemPartitionKey) { - const wrappedProjection = `IIF(IS_DEFINED(${fullAccess}), ${fullAccess}, {})`; - projections.push(wrappedProjection); - } else { - projections.push(fullAccess); - } + + projections.push(`${collectionAlias}${projectedProperty}`); } return projections.join(","); @@ -124,7 +118,7 @@ export const extractPartitionKeyValues = ( documentContent: any, partitionKeyDefinition: PartitionKeyDefinition, ): PartitionKey[] => { - if (!partitionKeyDefinition.paths || partitionKeyDefinition.paths.length === 0 || partitionKeyDefinition.systemKey) { + if (!partitionKeyDefinition.paths || partitionKeyDefinition.paths.length === 0) { return undefined; } @@ -136,8 +130,6 @@ export const extractPartitionKeyValues = ( if (value !== undefined) { partitionKeyValues.push(value); - } else { - partitionKeyValues.push({}); } }); diff --git a/test/fx.ts b/test/fx.ts index 14ad0acbb..661d55897 100644 --- a/test/fx.ts +++ b/test/fx.ts @@ -26,7 +26,7 @@ export function getAzureCLICredentials(): AzureCliCredential { export async function getAzureCLICredentialsToken(): Promise { const credentials = getAzureCLICredentials(); - const token = (await credentials.getToken("https://management.core.windows.net//.default"))?.token || ""; + const token = (await credentials.getToken("https://management.core.windows.net//.default")).token; return token; } @@ -37,7 +37,6 @@ export enum TestAccount { Mongo = "Mongo", Mongo32 = "Mongo32", SQL = "SQL", - SQLReadOnly = "SQLReadOnly", } export const defaultAccounts: Record = { @@ -47,7 +46,6 @@ export const defaultAccounts: Record = { [TestAccount.Mongo]: "github-e2etests-mongo", [TestAccount.Mongo32]: "github-e2etests-mongo32", [TestAccount.SQL]: "github-e2etests-sql", - [TestAccount.SQLReadOnly]: "github-e2etests-sql-readonly", }; export const resourceGroupName = process.env.DE_TEST_RESOURCE_GROUP ?? "de-e2e-tests"; @@ -216,25 +214,6 @@ export class QueryTab { } } -export class DocumentsTab { - documentsFilter: Locator; - documentsListPane: Locator; - documentResultsPane: Locator; - resultsEditor: Editor; - - constructor( - public frame: Frame, - public tabId: string, - public tab: Locator, - public locator: Locator, - ) { - this.documentsFilter = this.locator.getByTestId("DocumentsTab/Filter"); - this.documentsListPane = this.locator.getByTestId("DocumentsTab/DocumentsPane"); - this.documentResultsPane = this.locator.getByTestId("DocumentsTab/ResultsPane"); - this.resultsEditor = new Editor(this.frame, this.documentResultsPane.getByTestId("EditorReact/Host/Loaded")); - } -} - type PanelOpenOptions = { closeTimeout?: number; }; @@ -253,12 +232,6 @@ export class DataExplorer { return new QueryTab(this.frame, tabId, tab, queryTab); } - documentsTab(tabId: string): DocumentsTab { - const tab = this.tab(tabId); - const documentsTab = tab.getByTestId("DocumentsTab"); - return new DocumentsTab(this.frame, tabId, tab, documentsTab); - } - /** Select the primary global command button. * * There's only a single "primary" button, but we still require you to pass the label to confirm you're selecting the right button. @@ -321,10 +294,6 @@ export class DataExplorer { return await this.waitForNode(`${databaseId}/${containerId}`); } - async waitForContainerItemsNode(databaseId: string, containerId: string): Promise { - return await this.waitForNode(`${databaseId}/${containerId}/Items`); - } - /** Select the tree node with the specified id */ treeNode(id: string): TreeNode { return new TreeNode(this.frame.getByTestId(`TreeNode:${id}`), this.frame, id); diff --git a/test/sql/document.spec.ts b/test/sql/document.spec.ts deleted file mode 100644 index 10877702a..000000000 --- a/test/sql/document.spec.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { expect, test } from "@playwright/test"; - -import { DataExplorer, DocumentsTab, TestAccount } from "../fx"; -import { documentTestCases } from "./testCases"; - -let explorer: DataExplorer = null!; -let documentsTab: DocumentsTab = null!; - -for (const { name, databaseId, containerId, expectedDocumentIds } of documentTestCases) { - test.describe(`Test Documents with ${name}`, () => { - test.beforeEach("Open documents tab", async ({ page }) => { - explorer = await DataExplorer.open(page, TestAccount.SQLReadOnly); - - const containerNode = await explorer.waitForContainerNode(databaseId, containerId); - await containerNode.expand(); - - const containerMenuNode = await explorer.waitForContainerItemsNode(databaseId, containerId); - await containerMenuNode.element.click(); - - documentsTab = explorer.documentsTab("tab0"); - - await documentsTab.documentsFilter.waitFor(); - await documentsTab.documentsListPane.waitFor(); - await expect(documentsTab.resultsEditor.locator).toBeAttached({ timeout: 60 * 1000 }); - }); - - for (const docId of expectedDocumentIds) { - test.describe(`Document ID: ${docId}`, () => { - test(`should load and view document ${docId}`, async () => { - const span = documentsTab.documentsListPane.getByText(docId, { exact: true }).nth(0); - await span.waitFor(); - await expect(span).toBeVisible(); - - await span.click(); - await expect(documentsTab.resultsEditor.locator).toBeAttached({ timeout: 60 * 1000 }); - - const resultText = await documentsTab.resultsEditor.text(); - const resultData = JSON.parse(resultText!); - expect(resultText).not.toBeNull(); - expect(resultData?.id).toEqual(docId); - }); - }); - } - }); -} diff --git a/test/sql/testCases.ts b/test/sql/testCases.ts deleted file mode 100644 index 0f18bb183..000000000 --- a/test/sql/testCases.ts +++ /dev/null @@ -1,88 +0,0 @@ -type ContainerTestCase = { - name: string; - databaseId: string; - containerId: string; - expectedDocumentIds: string[]; -}; - -export const documentTestCases: ContainerTestCase[] = [ - { - name: "System Partition Key", - databaseId: "e2etests-sql-readonly", - containerId: "systemPartitionKey", - expectedDocumentIds: ["systempartition"], - }, - { - name: "Single Partition Key", - databaseId: "e2etests-sql-readonly", - containerId: "singlePartitionKey", - expectedDocumentIds: [ - "singlePartitionKey", - "singlePartitionKey_empty_string", - "singlePartitionKey_null", - "singlePartitionKey_missing", - ], - }, - { - name: "Single Nested Partition Key", - databaseId: "e2etests-sql-readonly", - containerId: "singleNestedPartitionKey", - expectedDocumentIds: [ - "singlePartitionKey_nested", - "singlePartitionKey_nested_empty_string", - "singlePartitionKey_nested_null", - "singlePartitionKey_nested_missing", - ], - }, - { - name: "2-Level Hierarchical Partition Key", - databaseId: "e2etests-sql-readonly", - containerId: "twoLevelPartitionKey", - expectedDocumentIds: [ - "twoLevelPartitionKey_value_empty", - "twoLevelPartitionKey_value_null", - "twoLevelPartitionKey_value_missing", - "twoLevelPartitionKey_empty_null", - "twoLevelPartitionKey_null_missing", - "twoLevelPartitionKey_missing_value", - ], - }, - { - name: "2-Level Hierarchical Nested Partition Key", - databaseId: "e2etests-sql-readonly", - containerId: "twoLevelNestedPartitionKey", - expectedDocumentIds: [ - "twoLevelNestedPartitionKey_nested_value_empty", - "twoLevelNestedPartitionKey_nested_null_missing", - "twoLevelNestedPartitionKey_nested_missing_value", - ], - }, - { - name: "3-Level Hierarchical Partition Key", - databaseId: "e2etests-sql-readonly", - containerId: "threeLevelPartitionKey", - expectedDocumentIds: [ - "threeLevelPartitionKey_value_empty_null", - "threeLevelPartitionKey_value_null_missing", - "threeLevelPartitionKey_value_missing_null", - "threeLevelPartitionKey_null_empty_value", - "threeLevelPartitionKey_missing_value_value", - "threeLevelPartitionKey_empty_value_missing", - ], - }, - { - name: "3-Level Nested Hierarchical Partition Key", - databaseId: "e2etests-sql-readonly", - containerId: "threeLevelNestedPartitionKey", - expectedDocumentIds: [ - "threeLevelNestedPartitionKey_nested_empty_value_null", - "threeLevelNestedPartitionKey_nested_null_value_missing", - "threeLevelNestedPartitionKey_nested_missing_value_null", - "threeLevelNestedPartitionKey_nested_null_empty_missing", - "threeLevelNestedPartitionKey_nested_value_missing_empty", - "threeLevelNestedPartitionKey_nested_missing_null_empty", - "threeLevelNestedPartitionKey_nested_empty_null_value", - "threeLevelNestedPartitionKey_nested_value_null_empty", - ], - }, -];