notifications - add tests

src/vs/workbench/common/notifications.ts

@@ -35,7 +35,7 @@ export interface INotificationChangeEvent {
 }
 
 class NoOpNotification implements INotificationHandle {
-	dispose() { }
+	public dispose(): void { }
 }
 
 export class NotificationsModel implements INotificationsModel {
@@ -69,13 +69,42 @@ export class NotificationsModel implements INotificationsModel {
 			return item; // return early if this is a no-op
 		}
 
-		// Add to list as first entry (TODO@notification dedupe!)
+		// Deduplicate
+		const duplicate = this.findNotification(item);
+		if (duplicate) {
+			duplicate.dispose();
+		}
+
+		// Add to list as first entry
 		this._notifications.splice(0, 0, item);
 
 		// Events
 		this._onDidNotificationChange.fire({ item, index: 0, kind: NotificationChangeType.ADD });
 
-		return item;
+		// Wrap into handles
+		return {
+			dispose: () => this.disposeItem(item)
+		};
+	}
+
+	private disposeItem(item: INotificationViewItem): void {
+		const liveItem = this.findNotification(item);
+		if (liveItem && liveItem !== item) {
+			liveItem.dispose(); // item could have been replaced with another one, make sure to dispose the live item
+		} else {
+			item.dispose(); // otherwise just dispose the item that was passed in
+		}
+	}
+
+	private findNotification(item: INotificationViewItem): INotificationViewItem {
+		for (let i = 0; i < this._notifications.length; i++) {
+			const notification = this._notifications[i];
+			if (notification.equals(item)) {
+				return notification;
+			}
+		}
+
+		return void 0;
 	}
 
 	private createViewItem(notification: INotification): INotificationViewItem | NoOpNotification {
@@ -126,6 +155,8 @@ export interface INotificationViewItem {
 	collapse(): void;
 
 	dispose(): void;
+
+	equals(item: INotificationViewItem);
 }
 
 export function isNotificationViewItem(obj: any): obj is INotificationViewItem {
@@ -171,10 +202,10 @@ export class NotificationViewItem implements INotificationViewItem {
 			message.value = `${message.value.substr(0, NotificationViewItem.MAX_MESSAGE_LENGTH)}...`;
 		}
 
-		return new NotificationViewItem(severity, message, notification.source, notification.actions || []);
+		return new NotificationViewItem(severity, message, notification.source, notification.actions);
 	}
 
-	private constructor(private _severity: Severity, private _message: IMarkdownString, private _source: string, private _actions: IAction[]) {
+	private constructor(private _severity: Severity, private _message: IMarkdownString, private _source: string, private _actions: IAction[] = []) {
 		this.toDispose = [];
 		this._expanded = _actions.length > 0;
 
@@ -242,4 +273,26 @@ export class NotificationViewItem implements INotificationViewItem {
 
 		this.toDispose = dispose(this.toDispose);
 	}
+
+	public equals(other: INotificationViewItem): boolean {
+		if (this._source !== other.source) {
+			return false;
+		}
+
+		if (this._actions.length !== other.actions.length) {
+			return false;
+		}
+
+		if (this._message.value !== other.message.value) {
+			return false;
+		}
+
+		for (let i = 0; i < this._actions.length; i++) {
+			if (this._actions[i].id !== other.actions[i].id) {
+				return false;
+			}
+		}
+
+		return true;
+	}
 }
\ No newline at end of file

src/vs/workbench/test/common/notifications.test.ts

+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+'use strict';
+
+import * as assert from 'assert';
+import { NotificationsModel, NotificationViewItem, INotificationChangeEvent, NotificationChangeType } from 'vs/workbench/common/notifications';
+import { Severity } from 'vs/platform/message/common/message';
+import { Action } from 'vs/base/common/actions';
+import { INotification } from 'vs/platform/notification/common/notification';
+
+suite('Notifications', () => {
+
+	test('Items', () => {
+
+		// Invalid
+		assert.ok(!NotificationViewItem.create({ severity: Severity.Error, message: '' }));
+		assert.ok(!NotificationViewItem.create({ severity: Severity.Error, message: null }));
+		assert.ok(!NotificationViewItem.create({ severity: Severity.Error, message: { value: '', isTrusted: true } }));
+
+		// Duplicates
+		let item1 = NotificationViewItem.create({ severity: Severity.Error, message: 'Error Message' });
+		let item2 = NotificationViewItem.create({ severity: Severity.Error, message: 'Error Message' });
+		let item3 = NotificationViewItem.create({ severity: Severity.Info, message: 'Info Message' });
+		let item4 = NotificationViewItem.create({ severity: Severity.Error, message: 'Error Message', source: 'Source' });
+		let item5 = NotificationViewItem.create({ severity: Severity.Error, message: 'Error Message', actions: [new Action('id', 'label')] });
+
+		assert.equal(item1.equals(item1), true);
+		assert.equal(item2.equals(item2), true);
+		assert.equal(item3.equals(item3), true);
+		assert.equal(item4.equals(item4), true);
+		assert.equal(item5.equals(item5), true);
+
+		assert.equal(item1.equals(item2), true);
+		assert.equal(item1.equals(item3), false);
+		assert.equal(item1.equals(item4), false);
+		assert.equal(item1.equals(item5), false);
+
+		// Message Box
+		assert.equal(item5.canCollapse, false);
+		assert.equal(item5.expanded, true);
+
+		// Events
+		let called = 0;
+		item1.onDidChange(() => {
+			called++;
+		});
+
+		item1.expand();
+		item1.expand();
+		item1.collapse();
+		item1.collapse();
+
+		assert.equal(called, 2);
+
+		called = 0;
+		item1.onDidDispose(() => {
+			called++;
+		});
+
+		item1.dispose();
+		assert.equal(called, 1);
+	});
+
+	test('Model', () => {
+		const model = new NotificationsModel();
+
+		let lastEvent: INotificationChangeEvent;
+		model.onDidNotificationChange(e => {
+			lastEvent = e;
+		});
+
+		let item1: INotification = { severity: Severity.Error, message: 'Error Message', actions: [new Action('id', 'label')] };
+		let item2: INotification = { severity: Severity.Warning, message: 'Warning Message', source: 'Some Source' };
+		let item2Duplicate: INotification = { severity: Severity.Warning, message: 'Warning Message', source: 'Some Source' };
+		let item3: INotification = { severity: Severity.Info, message: 'Info Message' };
+
+		let item1Handle = model.notify(item1);
+		assert.equal(lastEvent.item.severity, item1.severity);
+		assert.equal(lastEvent.item.message.value, item1.message);
+		assert.equal(lastEvent.index, 0);
+		assert.equal(lastEvent.kind, NotificationChangeType.ADD);
+
+		let item2Handle = model.notify(item2);
+		assert.equal(lastEvent.item.severity, item2.severity);
+		assert.equal(lastEvent.item.message.value, item2.message);
+		assert.equal(lastEvent.index, 0);
+		assert.equal(lastEvent.kind, NotificationChangeType.ADD);
+
+		model.notify(item3);
+		assert.equal(lastEvent.item.severity, item3.severity);
+		assert.equal(lastEvent.item.message.value, item3.message);
+		assert.equal(lastEvent.index, 0);
+		assert.equal(lastEvent.kind, NotificationChangeType.ADD);
+
+		assert.equal(model.notifications.length, 3);
+
+		item1Handle.dispose();
+		assert.equal(model.notifications.length, 2);
+		assert.equal(lastEvent.item.severity, item1.severity);
+		assert.equal(lastEvent.item.message.value, item1.message);
+		assert.equal(lastEvent.index, 2);
+		assert.equal(lastEvent.kind, NotificationChangeType.REMOVE);
+
+		model.notify(item2Duplicate);
+		assert.equal(model.notifications.length, 2);
+		assert.equal(lastEvent.item.severity, item2Duplicate.severity);
+		assert.equal(lastEvent.item.message.value, item2Duplicate.message);
+		assert.equal(lastEvent.index, 0);
+		assert.equal(lastEvent.kind, NotificationChangeType.ADD);
+
+		item2Handle.dispose();
+		assert.equal(model.notifications.length, 1);
+		assert.equal(lastEvent.item.severity, item2Duplicate.severity);
+		assert.equal(lastEvent.item.message.value, item2Duplicate.message);
+		assert.equal(lastEvent.index, 0);
+		assert.equal(lastEvent.kind, NotificationChangeType.REMOVE);
+
+		model.notifications[0].expand();
+		assert.equal(lastEvent.item.severity, item3.severity);
+		assert.equal(lastEvent.item.message.value, item3.message);
+		assert.equal(lastEvent.index, 0);
+		assert.equal(lastEvent.kind, NotificationChangeType.CHANGE);
+	});
+});
\ No newline at end of file