Document labels: use fields other than _id wherever possible (#569)

* Update third party notices for mongodb-extended-json. * Document labels: use fields other than _id wherever possible * Remove the number of calls to toString in finding the label * Force refresh a document's tree item post-update. Fixes #145 * Document labels : make private * Add setter for tsc -p * Changes based on feedback * Based on feedback * tsc fix * Feedback + tests * Add field to documentLabelFields. Kicks off CLA.
microsoft · Jun 19, 2018 · e6fa015 · e6fa015
1 parent 3ea8080
commit e6fa015
Show file tree

Hide file tree

Showing 9 changed files with 88 additions and 15 deletions.
diff --git a/src/constants.ts b/src/constants.ts
@@ -34,3 +34,5 @@ export const defaultStoredProcedure =
 };` ;
 
 export const emptyPartitionKeyValue = {};
+
+export const documentLabelFields = ["name", "Name", "NAME", "ID", "UUID", "Id", "id", "_id", "uuid"];
diff --git a/src/docdb/editors/DocDBDocumentNodeEditor.ts b/src/docdb/editors/DocDBDocumentNodeEditor.ts
@@ -24,7 +24,9 @@ export class DocDBDocumentNodeEditor implements ICosmosEditor<RetrievedDocument>
     }
 
     public async update(document: RetrievedDocument): Promise<RetrievedDocument> {
-        return await this._documentNode.treeItem.update(document);
+        const updatedDoc = await this._documentNode.treeItem.update(document);
+        await this._documentNode.refresh();
+        return updatedDoc;
     }
 
     public get id(): string {

diff --git a/src/docdb/tree/DocDBDocumentTreeItem.ts b/src/docdb/tree/DocDBDocumentTreeItem.ts
@@ -9,6 +9,7 @@ import { IAzureNode, IAzureTreeItem, UserCancelledError, DialogResponses } from
 import { RetrievedDocument, DocumentClient } from 'documentdb';
 import { DocDBCollectionTreeItem } from './DocDBCollectionTreeItem';
 import { emptyPartitionKeyValue } from '../../constants';
+import { getDocumentTreeItemLabel } from '../../utils/vscodeUtils';
 
 /**
  * Represents a Cosmos DB DocumentDB (SQL) document
@@ -18,23 +19,24 @@ export class DocDBDocumentTreeItem implements IAzureTreeItem {
     public readonly contextValue: string = DocDBDocumentTreeItem.contextValue;
     public readonly commandId: string = 'cosmosDB.openDocument';
 
-    private readonly partitionKeyValue: string | undefined | Object;
-
+    private readonly _partitionKeyValue: string | undefined | Object;
+    private _label: string;
     private _document: RetrievedDocument;
     private _collection: DocDBCollectionTreeItem;
 
     constructor(collection: DocDBCollectionTreeItem, document: RetrievedDocument) {
         this._collection = collection;
         this._document = document;
-        this.partitionKeyValue = this.getPartitionKeyValue();
+        this._partitionKeyValue = this.getPartitionKeyValue();
+        this._label = getDocumentTreeItemLabel(this._document);
     }
 
     public get id(): string {
         return this.document.id;
     }
 
-    public get label(): string {
-        return this.document.id;
+    public async refreshLabel(): Promise<void> {
+        this._label = getDocumentTreeItemLabel(this._document);
     }
 
     public get link(): string {
@@ -45,6 +47,10 @@ export class DocDBDocumentTreeItem implements IAzureTreeItem {
         return this._document;
     }
 
+    get label(): string {
+        return this._label;
+    }
+
     public get iconPath(): string | vscode.Uri | { light: string | vscode.Uri; dark: string | vscode.Uri } {
         return {
             light: path.join(__filename, '..', '..', '..', '..', '..', 'resources', 'icons', 'theme-agnostic', 'Document.svg'),
@@ -57,7 +63,7 @@ export class DocDBDocumentTreeItem implements IAzureTreeItem {
         const result = await vscode.window.showWarningMessage(message, { modal: true }, DialogResponses.deleteResponse, DialogResponses.cancel);
         if (result === DialogResponses.deleteResponse) {
             const client = this._collection.getDocumentClient();
-            const options = { partitionKey: this.partitionKeyValue };
+            const options = { partitionKey: this._partitionKeyValue };
             await new Promise((resolve, reject) => {
                 // Disabling type check in the next line. This helps ensure documents having no partition key value
                 // can still pass an empty object when required. It looks like a disparity between the type settings outlined here
@@ -80,7 +86,7 @@ export class DocDBDocumentTreeItem implements IAzureTreeItem {
             throw new Error(`The "_self" and "_etag" fields are required to update a document`);
         }
         else {
-            let options = { accessCondition: { type: 'IfMatch', condition: newData._etag }, partitionKey: this.partitionKeyValue };
+            let options = { accessCondition: { type: 'IfMatch', condition: newData._etag }, partitionKey: this._partitionKeyValue };
             this._document = await new Promise<RetrievedDocument>((resolve, reject) => {
                 client.replaceDocument(
                     _self,
@@ -95,6 +101,7 @@ export class DocDBDocumentTreeItem implements IAzureTreeItem {
                         }
                     });
             });
+            await this.refreshLabel();
             return this.document;
         }
     }

diff --git a/src/mongo/editors/MongoCollectionNodeEditor.ts b/src/mongo/editors/MongoCollectionNodeEditor.ts
@@ -38,6 +38,7 @@ export class MongoCollectionNodeEditor implements ICosmosEditor<IMongoDocument[]
             const documentNode = documentNodes.find((node) => node.treeItem.document._id.toString() === updatedDoc._id.toString());
             if (documentNode) {
                 documentNode.treeItem.document = updatedDoc;
+                await documentNode.refresh();
             }
         }
     }

diff --git a/src/mongo/editors/MongoDocumentNodeEditor.ts b/src/mongo/editors/MongoDocumentNodeEditor.ts
@@ -25,7 +25,9 @@ export class MongoDocumentNodeEditor implements ICosmosEditor<IMongoDocument> {
     }
 
     public async update(document: IMongoDocument): Promise<IMongoDocument> {
-        return await this._documentNode.treeItem.update(document);
+        const updatedDoc = await this._documentNode.treeItem.update(document);
+        await this._documentNode.refresh();
+        return updatedDoc;
     }
 
     public get id(): string {

diff --git a/src/mongo/editors/MongoFindOneResultEditor.ts b/src/mongo/editors/MongoFindOneResultEditor.ts
@@ -34,12 +34,16 @@ export class MongoFindOneResultEditor implements ICosmosEditor<IMongoDocument> {
 
     public async update(newDocument: IMongoDocument): Promise<IMongoDocument> {
         const node = await this._tree.findNode(this.id);
+        let result: IMongoDocument;
         if (node) {
-            return (<IAzureNode<MongoDocumentTreeItem>>node).treeItem.update(newDocument);
+            result = await (<IAzureNode<MongoDocumentTreeItem>>node).treeItem.update(newDocument);
+            node.refresh();
+        } else {
+            // If the node isn't cached already, just update it to Mongo directly (without worrying about updating the tree)
+            const db = await this._databaseNode.treeItem.getDb();
+            result = await MongoDocumentTreeItem.update(db.collection(this._collectionName), newDocument);
         }
-        // If the node isn't cached already, just update it to Mongo directly (without worrying about updating the tree)
-        const db = await this._databaseNode.treeItem.getDb();
-        return await MongoDocumentTreeItem.update(db.collection(this._collectionName), newDocument);
+        return result;
     }
 
     public get id(): string {

diff --git a/src/mongo/tree/MongoDocumentTreeItem.ts b/src/mongo/tree/MongoDocumentTreeItem.ts
@@ -8,6 +8,7 @@ import * as vscode from 'vscode';
 import * as path from 'path';
 import { Collection, ObjectID, DeleteWriteOpResultObject, UpdateWriteOpResult } from 'mongodb';
 import { IAzureTreeItem, IAzureNode, UserCancelledError, DialogResponses } from 'vscode-azureextensionui';
+import { getDocumentTreeItemLabel } from '../../utils/vscodeUtils';
 
 export interface IMongoDocument {
     _id: string | ObjectID;
@@ -23,19 +24,25 @@ export class MongoDocumentTreeItem implements IAzureTreeItem {
     public readonly commandId: string = 'cosmosDB.openDocument';
     public document: IMongoDocument;
 
+    private _label;
     private _collection: Collection;
 
     constructor(document: IMongoDocument, collection: Collection) {
         this.document = document;
+        this._label = getDocumentTreeItemLabel(this.document);
         this._collection = collection;
     }
 
     get id(): string {
         return this.document._id.toString();
     }
 
+    public async refreshLabel(): Promise<void> {
+        this._label = getDocumentTreeItemLabel(this.document);
+    }
+
     get label(): string {
-        return this.document._id.toString();
+        return this._label;
     }
 
     public get iconPath(): string | vscode.Uri | { light: string | vscode.Uri; dark: string | vscode.Uri } {
@@ -46,7 +53,7 @@ export class MongoDocumentTreeItem implements IAzureTreeItem {
     }
 
     public async deleteTreeItem(_node: IAzureNode): Promise<void> {
-        const message: string = `Are you sure you want to delete document '${this.label}'?`;
+        const message: string = `Are you sure you want to delete document '${this._label}'?`;
         const result = await vscode.window.showWarningMessage(message, { modal: true }, DialogResponses.deleteResponse, DialogResponses.cancel);
         if (result === DialogResponses.deleteResponse) {
             const result: DeleteWriteOpResultObject = await this._collection.deleteOne({ "_id": this.document._id });
@@ -60,6 +67,7 @@ export class MongoDocumentTreeItem implements IAzureTreeItem {
 
     public async update(newDocument: IMongoDocument): Promise<IMongoDocument> {
         this.document = await MongoDocumentTreeItem.update(this._collection, newDocument);
+        await this.refreshLabel();
         return this.document;
     }
 

diff --git a/src/utils/vscodeUtils.ts b/src/utils/vscodeUtils.ts
@@ -9,6 +9,9 @@ import * as vscode from 'vscode';
 import { IAzureNode, IAzureTreeItem } from 'vscode-azureextensionui';
 import { MongoAccountTreeItem } from '../mongo/tree/MongoAccountTreeItem';
 import { DocDBAccountTreeItemBase } from '../docdb/tree/DocDBAccountTreeItemBase';
+import { IMongoDocument } from '../mongo/tree/MongoDocumentTreeItem';
+import { RetrievedDocument } from 'documentdb';
+import { documentLabelFields } from '../constants';
 
 const outputChannel = vscode.window.createOutputChannel("Azure CosmosDB");
 
@@ -101,3 +104,15 @@ export function getNodeEditorLabel(node: IAzureNode): string {
 function isAccountTreeItem(treeItem: IAzureTreeItem): boolean {
     return (treeItem instanceof MongoAccountTreeItem) || (treeItem instanceof DocDBAccountTreeItemBase);
 }
+
+export function getDocumentTreeItemLabel(document: IMongoDocument | RetrievedDocument): string {
+    for (let field of documentLabelFields) {
+        if (document.hasOwnProperty(field)) {
+            let value = document[field];
+            if (value !== undefined && typeof value !== 'object') {
+                return String(value);
+            }
+        }
+    }
+    return String(document["_id"]);
+}
diff --git a/test/documentLabel.test.ts b/test/documentLabel.test.ts
@@ -0,0 +1,32 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+
+import * as assert from 'assert';
+import { getDocumentTreeItemLabel } from '../src/utils/vscodeUtils';
+
+suite("Document Label Tests", () => {
+    test("Non-zero number", () => {
+        const doc = { name: 4, _id: "12345678901234567890123456789012" };
+        assert.equal(getDocumentTreeItemLabel(doc), 4);
+    });
+
+    test("zero (number)", () => {
+        const doc = { name: 0, _id: "12345678901234567890123456789012" };
+        assert.equal(getDocumentTreeItemLabel(doc), 0);
+    });
+
+    test("Empty string", () => {
+        const doc = { name: "", _id: "" };
+        assert.equal(getDocumentTreeItemLabel(doc), "");
+    });
+
+    test("Null", () => {
+        const doc = { name: null, _id: "12345678901234567890123456789012" };
+        assert.equal(getDocumentTreeItemLabel(doc), doc._id);
+    });
+
+
+});
Original file line number	Diff line number	Diff line change
Expand Up		@@ -34,3 +34,5 @@ export const defaultStoredProcedure =
		};` ;

		export const emptyPartitionKeyValue = {};

		export const documentLabelFields = ["name", "Name", "NAME", "ID", "UUID", "Id", "id", "_id", "uuid"];