Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 13 commits into from
Jun 19, 2018
2 changes: 2 additions & 0 deletions src/constants.ts
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"];
4 changes: 3 additions & 1 deletion src/docdb/editors/DocDBDocumentNodeEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 14 additions & 7 deletions src/docdb/tree/DocDBDocumentTreeItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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'),
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -95,6 +101,7 @@ export class DocDBDocumentTreeItem implements IAzureTreeItem {
}
});
});
await this.refreshLabel();
return this.document;
}
}
Expand Down
1 change: 1 addition & 0 deletions src/mongo/editors/MongoCollectionNodeEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/mongo/editors/MongoDocumentNodeEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 8 additions & 4 deletions src/mongo/editors/MongoFindOneResultEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the other issue I saw. Why was the return removed here?

Copy link
Contributor Author

@PrashanthCorp PrashanthCorp May 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad. We need to kick off the refresh before returning, but after updating the value.

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 {
Expand Down
12 changes: 10 additions & 2 deletions src/mongo/tree/MongoDocumentTreeItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 } {
Expand All @@ -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 });
Expand All @@ -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;
}

Expand Down
15 changes: 15 additions & 0 deletions src/utils/vscodeUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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"]);
}
32 changes: 32 additions & 0 deletions test/documentLabel.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});


});