From af0a890516b6302b60710b4f54e58b68d88b1857 Mon Sep 17 00:00:00 2001 From: sunghyunkang1111 <114709653+sunghyunkang1111@users.noreply.github.com> Date: Mon, 7 Apr 2025 10:45:29 -0500 Subject: [PATCH] Pk missing fix (#2094) * fix partition key missing not being able to load the document * Implement E2E tests for documents with different partitionkeys * Implement E2E tests for documents with different partitionkeys * Implement E2E tests for documents with different partitionkeys * Updated snapshot --- 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, 259 insertions(+), 21 deletions(-) create mode 100644 test/sql/document.spec.ts create mode 100644 test/sql/testCases.ts diff --git a/playwright.config.ts b/playwright.config.ts index 4c5ad3c14..5279a0660 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -1,5 +1,4 @@ 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 c24ac08e8..5ef058f3f 100644 --- a/src/Explorer/Tabs/DocumentsTabV2/DocumentsTabV2.tsx +++ b/src/Explorer/Tabs/DocumentsTabV2/DocumentsTabV2.tsx @@ -1037,6 +1037,7 @@ 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); @@ -2083,8 +2088,8 @@ export const DocumentsTabComponent: React.FunctionComponent -
-
+
+
{!isPreferredApiMongoDB && SELECT * FROM c } -
+
-
+
{isTabActive && selectedDocumentContent && selectedRows.size <= 1 && (
SELECT * FROM c @@ -65,6 +67,7 @@ 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", () => { @@ -89,6 +96,18 @@ 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()", () => { @@ -201,18 +220,6 @@ 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, @@ -225,5 +232,52 @@ 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 f0b39e4e2..07822a422 100644 --- a/src/Utils/QueryUtils.ts +++ b/src/Utils/QueryUtils.ts @@ -47,6 +47,7 @@ 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) => { @@ -61,8 +62,13 @@ export function buildDocumentsQueryPartitionProjections( projectedProperty += `[${projection}]`; } }); - - projections.push(`${collectionAlias}${projectedProperty}`); + const fullAccess = `${collectionAlias}${projectedProperty}`; + if (!isSystemPartitionKey) { + const wrappedProjection = `IIF(IS_DEFINED(${fullAccess}), ${fullAccess}, {})`; + projections.push(wrappedProjection); + } else { + projections.push(fullAccess); + } } return projections.join(","); @@ -118,7 +124,7 @@ export const extractPartitionKeyValues = ( documentContent: any, partitionKeyDefinition: PartitionKeyDefinition, ): PartitionKey[] => { - if (!partitionKeyDefinition.paths || partitionKeyDefinition.paths.length === 0) { + if (!partitionKeyDefinition.paths || partitionKeyDefinition.paths.length === 0 || partitionKeyDefinition.systemKey) { return undefined; } @@ -130,6 +136,8 @@ export const extractPartitionKeyValues = ( if (value !== undefined) { partitionKeyValues.push(value); + } else { + partitionKeyValues.push({}); } }); diff --git a/test/fx.ts b/test/fx.ts index 661d55897..14ad0acbb 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,6 +37,7 @@ export enum TestAccount { Mongo = "Mongo", Mongo32 = "Mongo32", SQL = "SQL", + SQLReadOnly = "SQLReadOnly", } export const defaultAccounts: Record = { @@ -46,6 +47,7 @@ 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"; @@ -214,6 +216,25 @@ 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; }; @@ -232,6 +253,12 @@ 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. @@ -294,6 +321,10 @@ 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 new file mode 100644 index 000000000..10877702a --- /dev/null +++ b/test/sql/document.spec.ts @@ -0,0 +1,45 @@ +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 new file mode 100644 index 000000000..0f18bb183 --- /dev/null +++ b/test/sql/testCases.ts @@ -0,0 +1,88 @@ +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", + ], + }, +];