Replace observable references with just promises (#5278)

* replace observable references with just promises

* add tests for searching in dataview

* add comments

* work through respecting max matches

* fix tests

* fix strict null checks

src/sql/base/browser/ui/table/tableDataView.ts

@@ -3,9 +3,6 @@
  *  Licensed under the Source EULA. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { Observable } from 'rxjs/Observable';
-import { Observer } from 'rxjs/Observer';
-
 import { Event, Emitter } from 'vs/base/common/event';
 import * as types from 'vs/base/common/types';
 import { compare as stringCompare } from 'vs/base/common/strings';
@@ -50,7 +47,6 @@ export class TableDataView<T extends Slick.SlickData> implements IDisposableData
 	//Used when filtering is enabled, _allData holds the complete set of data.
 	private _allData: Array<T>;
 	private _findArray: Array<IFindPosition>;
-	private _findObs: Observable<IFindPosition> | undefined;
 	private _findIndex: number;
 	private _filterEnabled: boolean;
 
@@ -78,6 +74,7 @@ export class TableDataView<T extends Slick.SlickData> implements IDisposableData
 			this._data = new Array<T>();
 		}
 
+		// @todo @anthonydresser 5/1/19 theres a lot we could do by just accepting a regex as a exp rather than accepting a full find function
 		this._sortFn = _sortFn ? _sortFn : defaultSort;
 
 		this._filterFn = _filterFn ? _filterFn : (dataToFilter) => dataToFilter;
@@ -155,7 +152,7 @@ export class TableDataView<T extends Slick.SlickData> implements IDisposableData
 		this._onRowCountChange.fire(this.getLength());
 	}
 
-	find(exp: string, maxMatches: number = 0): Thenable<IFindPosition> {
+	find(exp: string, maxMatches?: number): Promise<IFindPosition> {
 		if (!this._findFn) {
 			return Promise.reject(new Error('no find function provided'));
 		}
@@ -163,35 +160,45 @@ export class TableDataView<T extends Slick.SlickData> implements IDisposableData
 		this._findIndex = 0;
 		this._onFindCountChange.fire(this._findArray.length);
 		if (exp) {
-			this._findObs = Observable.create((observer: Observer<IFindPosition>) => {
-				for (let i = 0; i < this._data.length; i++) {
-					let item = this._data[i];
-					let result = this._findFn!(item, exp);
-					if (result) {
-						result.forEach(pos => {
-							let index = { col: pos, row: i };
-							this._findArray.push(index);
-							observer.next(index);
-							this._onFindCountChange.fire(this._findArray.length);
-						});
-						if (maxMatches > 0 && this._findArray.length > maxMatches) {
-							break;
-						}
-					}
-				}
-			});
-			return this._findObs!.take(1).toPromise().then(() => {
-				return this._findArray[this._findIndex];
+			return new Promise<IFindPosition>((resolve) => {
+				const disp = this.onFindCountChange(e => {
+					resolve(this._findArray[e - 1]);
+					disp.dispose();
+				});
+				this._startSearch(exp, maxMatches);
 			});
 		} else {
 			return Promise.reject(new Error('no expression'));
 		}
 	}
 
+	private _startSearch(exp: string, maxMatches: number = 0): void {
+		for (let i = 0; i < this._data.length; i++) {
+			const item = this._data[i];
+			const result = this._findFn!(item, exp);
+			let breakout = false;
+			if (result) {
+				for (let j = 0; j < result.length; j++) {
+					const pos = result[j];
+					const index = { col: pos, row: i };
+					this._findArray.push(index);
+					this._onFindCountChange.fire(this._findArray.length);
+					if (maxMatches > 0 && this._findArray.length === maxMatches) {
+						breakout = true;
+						break;
+					}
+				}
+			}
+
+			if (breakout) {
+				break;
+			}
+		}
+	}
+
 	clearFind() {
 		this._findArray = new Array<IFindPosition>();
 		this._findIndex = 0;
-		this._findObs = undefined;
 		this._onFindCountChange.fire(this._findArray.length);
 	}
 
@@ -242,6 +249,5 @@ export class TableDataView<T extends Slick.SlickData> implements IDisposableData
 		this._data = [];
 		this._allData = [];
 		this._findArray = [];
-		this._findObs = undefined;
 	}
 }

src/sql/base/test/browser/ui/table/tableDataView.test.ts

@@ -0,0 +1,196 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the Source EULA. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as assert from 'assert';
+import { TableDataView } from 'sql/base/browser/ui/table/tableDataView';
+
+suite('TableDataView', () => {
+	test('Data can be filtered and filter can be cleared', () => {
+		const rowCount = 10;
+		const columnCount = 5;
+		const originalData = populateData(rowCount, columnCount);
+
+		let filteredRowCount = 5;
+		const obj = new TableDataView(originalData, undefined, undefined, (data: any[]) => {
+			return populateData(filteredRowCount, columnCount);
+		});
+
+		let rowCountEventInvokeCount = 0;
+		let filterStateChangeEventInvokeCount = 0;
+		let rowCountEventParameter;
+		obj.onRowCountChange((count) => {
+			rowCountEventInvokeCount++;
+			rowCountEventParameter = count;
+		});
+
+		obj.onFilterStateChange(() => {
+			filterStateChangeEventInvokeCount++;
+		});
+
+		let verify = (expectedRowCountChangeInvokeCount: number,
+			expectedDataLength: number,
+			expectedNonFilteredDataLength: number,
+			expectedFilterStateChangeInvokeCount: number,
+			stepName: string,
+			verifyRowCountEventParameter: boolean = true) => {
+			assert.equal(rowCountEventInvokeCount, expectedRowCountChangeInvokeCount, 'RowCountChange event count - ' + stepName);
+			if (verifyRowCountEventParameter) {
+				assert.equal(rowCountEventParameter, expectedDataLength, 'Row count passed by RowCountChange event - ' + stepName);
+			}
+			assert.equal(obj.getLength(), expectedDataLength, 'Data length - ' + stepName);
+			assert.equal(obj.getLengthNonFiltered(), expectedNonFilteredDataLength, 'Length for all data - ' + stepName);
+			assert.equal(filterStateChangeEventInvokeCount, expectedFilterStateChangeInvokeCount, 'FilterStateChange event count - ' + stepName);
+		};
+
+		verify(0, rowCount, rowCount, 0, 'after initialization', false);
+
+		obj.filter();
+
+		verify(0, filteredRowCount, rowCount, 1, 'after filtering', false);
+
+		const additionalRowCount = 20;
+		const additionalData = populateData(additionalRowCount, columnCount);
+		obj.push(additionalData);
+
+		verify(1, filteredRowCount * 2, rowCount + additionalRowCount, 1, 'after adding more data');
+
+		obj.clearFilter();
+
+		verify(1, rowCount + additionalRowCount, rowCount + additionalRowCount, 2, 'after clearing filter', false);
+
+		//From this point on, nothing matches the filter criteria
+		filteredRowCount = 0;
+
+		obj.filter();
+		verify(1, 0, rowCount + additionalRowCount, 3, 'after 2nd filtering', false);
+
+		obj.push(additionalData);
+		verify(2, 0, rowCount + additionalRowCount + additionalRowCount, 3, 'after 2nd adding more data');
+
+		obj.clearFilter();
+		verify(2, rowCount + additionalRowCount + additionalRowCount, rowCount + additionalRowCount + additionalRowCount, 4, 'after 2nd clearing filter', false);
+
+		obj.clearFilter();
+		verify(2, rowCount + additionalRowCount + additionalRowCount, rowCount + additionalRowCount + additionalRowCount, 4, 'calling clearFilter() multiple times', false);
+	});
+
+	test('Search can find items', async () => {
+		const rowCount = 10;
+		const columnCount = 5;
+		const originalData = populateData(rowCount, columnCount);
+
+		const searchFn = (val: { [x: string]: string }, exp: string): Array<number> => {
+			const ret = new Array<number>();
+			for (let i = 0; i < columnCount; i++) {
+				const colVal = val[getColumnName(i)];
+				if (colVal && colVal.toLocaleLowerCase().includes(exp.toLocaleLowerCase())) {
+					ret.push(i);
+				}
+			}
+			return ret;
+		};
+
+		const dataView = new TableDataView(originalData, searchFn);
+
+		let findValue = await dataView.find('row 2');
+		assert.deepEqual(findValue, { row: 2, col: 0 });
+		findValue = await dataView.findNext();
+		assert.deepEqual(findValue, { row: 2, col: 1 });
+		findValue = await dataView.findNext();
+		assert.deepEqual(findValue, { row: 2, col: 2 });
+		findValue = await dataView.findNext();
+		assert.deepEqual(findValue, { row: 2, col: 3 });
+		findValue = await dataView.findNext();
+		assert.deepEqual(findValue, { row: 2, col: 4 });
+		// find will loop around once it reaches the end
+		findValue = await dataView.findNext();
+		assert.deepEqual(findValue, { row: 2, col: 0 });
+	});
+
+	test('Search fails correctly', async () => {
+		const rowCount = 10;
+		const columnCount = 5;
+		const originalData = populateData(rowCount, columnCount);
+
+		const searchFn = (val: { [x: string]: string }, exp: string): Array<number> => {
+			const ret = new Array<number>();
+			for (let i = 0; i < columnCount; i++) {
+				const colVal = val[getColumnName(i)];
+				if (colVal && colVal.toLocaleLowerCase().includes(exp.toLocaleLowerCase())) {
+					ret.push(i);
+				}
+			}
+			return ret;
+		};
+
+		const dataView = new TableDataView(originalData, searchFn);
+
+		try {
+			// we haven't started a search so we should throw
+			await dataView.findNext();
+			assert.fail();
+		} catch (e) {
+
+		}
+
+		await dataView.find('row 2');
+		dataView.clearFind();
+
+		try {
+			// we cleared the search and haven't started a new search so we should throw
+			await dataView.findNext();
+			assert.fail();
+		} catch (e) {
+
+		}
+	});
+
+	test('Search respects max finds', async () => {
+		const rowCount = 10;
+		const columnCount = 5;
+		const originalData = populateData(rowCount, columnCount);
+
+		const searchFn = (val: { [x: string]: string }, exp: string): Array<number> => {
+			const ret = new Array<number>();
+			for (let i = 0; i < columnCount; i++) {
+				const colVal = val[getColumnName(i)];
+				if (colVal && colVal.toLocaleLowerCase().includes(exp.toLocaleLowerCase())) {
+					ret.push(i);
+				}
+			}
+			return ret;
+		};
+
+		const dataView = new TableDataView(originalData, searchFn);
+
+		let findValue = await dataView.find('row 2', 2);
+		assert.deepEqual(findValue, { row: 2, col: 0 });
+		findValue = await dataView.findNext();
+		assert.deepEqual(findValue, { row: 2, col: 1 });
+		// find will loop around once it reaches the end
+		findValue = await dataView.findNext();
+		assert.deepEqual(findValue, { row: 2, col: 0 });
+	});
+});
+
+function populateData(row: number, column: number): any[] {
+	let data: Array<{ [key: string]: string }> = [];
+	for (let i: number = 0; i < row; i++) {
+		let row = {};
+		for (let j: number = 0; j < column; j++) {
+			row[getColumnName(j)] = getCellValue(i, j);
+		}
+		data.push(row);
+	}
+	return data;
+}
+
+function getColumnName(index: number): string {
+	return `column${index}`;
+}
+
+function getCellValue(row: number, column: number): string {
+	return `row ${row} column ${column}`;
+}