diff --git a/src/sql/platform/connection/common/connectionProfileGroup.ts b/src/sql/platform/connection/common/connectionProfileGroup.ts index f1e2904024..b27484d55d 100644 --- a/src/sql/platform/connection/common/connectionProfileGroup.ts +++ b/src/sql/platform/connection/common/connectionProfileGroup.ts @@ -19,8 +19,8 @@ export interface IConnectionProfileGroup { export class ConnectionProfileGroup extends Disposable implements IConnectionProfileGroup { - public children: ConnectionProfileGroup[] = []; - public connections: ConnectionProfile[] = []; + private _childGroups: ConnectionProfileGroup[] = []; + private _childConnections: ConnectionProfile[] = []; public parentId?: string; private _isRenamed = false; public constructor( @@ -42,9 +42,9 @@ export class ConnectionProfileGroup extends Disposable implements IConnectionPro public toObject(): IConnectionProfileGroup { let subgroups = undefined; - if (this.children.length > 0) { + if (this._childGroups.length > 0) { subgroups = []; - this.children.forEach((group) => { + this._childGroups.forEach((group) => { subgroups.push(group.toObject()); }); } @@ -75,8 +75,24 @@ export class ConnectionProfileGroup extends Disposable implements IConnectionPro this._isRenamed = val; } + public get children(): ConnectionProfileGroup[] | undefined { + return this._childGroups; + } + + public set children(children: ConnectionProfileGroup[] | undefined) { + this._childGroups = children ?? []; + } + + public get connections(): ConnectionProfile[] | undefined { + return this._childConnections; + } + + public set connections(connections: ConnectionProfile[] | undefined) { + this._childConnections = connections ?? []; + } + public hasChildren(): boolean { - if (this.children.length > 0 || this.connections.length > 0) { + if (this._childGroups.length > 0 || this._childConnections.length > 0) { return true; } return false; @@ -86,12 +102,12 @@ export class ConnectionProfileGroup extends Disposable implements IConnectionPro * Returns true if all connections in the tree have valid options using the correct capabilities */ public get hasValidConnections(): boolean { - let invalidConnections = find(this.connections, c => !c.isConnectionOptionsValid); + let invalidConnections = find(this._childConnections, c => !c.isConnectionOptionsValid); if (invalidConnections !== undefined) { return false; } else { let childrenAreValid: boolean = true; - this.children.forEach(element => { + this._childGroups.forEach(element => { let isChildValid = element.hasValidConnections; if (!isChildValid) { childrenAreValid = false; @@ -103,11 +119,11 @@ export class ConnectionProfileGroup extends Disposable implements IConnectionPro public getChildren(): (ConnectionProfile | ConnectionProfileGroup)[] { let allChildren: (ConnectionProfile | ConnectionProfileGroup)[] = []; - this.connections.forEach((conn) => { + this._childConnections.forEach((conn) => { allChildren.push(conn); }); - this.children.forEach((group) => { + this._childGroups.forEach((group) => { allChildren.push(group); }); return allChildren; @@ -120,22 +136,22 @@ export class ConnectionProfileGroup extends Disposable implements IConnectionPro return other.id === this.id; } - public addConnections(connections: ConnectionProfile[]): void { - connections.forEach((conn) => { - this.connections = this.connections.filter((curConn) => { return curConn.id !== conn.id; }); + public addConnections(connections: ConnectionProfile[] | undefined): void { + connections?.forEach((conn) => { + this._childConnections = this._childConnections.filter((curConn) => { return curConn.id !== conn.id; }); conn.parent = this; this._register(conn); - this.connections.push(conn); + this._childConnections.push(conn); }); } - public addGroups(groups: ConnectionProfileGroup[]): void { - groups.forEach((group) => { - this.children = this.children.filter((grp) => { return group.id !== grp.id; }); + public addGroups(groups: ConnectionProfileGroup[] | undefined): void { + groups?.forEach((group) => { + this._childGroups = this._childGroups.filter((grp) => { return group.id !== grp.id; }); group.parent = this; this._register(group); - this.children.push(group); + this._childGroups.push(group); }); } diff --git a/src/sql/platform/connection/common/utils.ts b/src/sql/platform/connection/common/utils.ts index efa2bf09e5..79f0ab9b58 100644 --- a/src/sql/platform/connection/common/utils.ts +++ b/src/sql/platform/connection/common/utils.ts @@ -117,14 +117,14 @@ export function generateUriWithPrefix(connection: IConnectionProfile, prefix: st export function findProfileInGroup(og: IConnectionProfile, groups: ConnectionProfileGroup[]): ConnectionProfile | undefined { for (let group of groups) { - for (let conn of group.connections) { + for (let conn of group.connections!) { if (conn.id === og.id) { return conn; } } if (group.hasChildren()) { - let potentialReturn = findProfileInGroup(og, group.children); + let potentialReturn = findProfileInGroup(og, group.children!); if (potentialReturn) { return potentialReturn; } diff --git a/src/sql/platform/connection/test/common/connectionProfileGroup.test.ts b/src/sql/platform/connection/test/common/connectionProfileGroup.test.ts index e46deb6f24..3a39bad856 100644 --- a/src/sql/platform/connection/test/common/connectionProfileGroup.test.ts +++ b/src/sql/platform/connection/test/common/connectionProfileGroup.test.ts @@ -5,6 +5,8 @@ import { ConnectionProfileGroup } from 'sql/platform/connection/common/connectionProfileGroup'; import * as assert from 'assert'; +import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile'; +import { TestCapabilitiesService } from 'sql/platform/capabilities/test/common/testCapabilitiesService'; suite('SQL ConnectionProfileGroup tests', () => { let root: ConnectionProfileGroup; @@ -14,12 +16,17 @@ suite('SQL ConnectionProfileGroup tests', () => { let group1Node: ConnectionProfileGroup; let group11Node: ConnectionProfileGroup; let group2Node: ConnectionProfileGroup; + let emptyGroup: ConnectionProfileGroup; + let connectionProfile: ConnectionProfile; + setup(() => { root = new ConnectionProfileGroup(ConnectionProfileGroup.RootGroupName, undefined, ConnectionProfileGroup.RootGroupName, undefined, undefined); group1Node = new ConnectionProfileGroup(Groups1, root, Groups1, undefined, undefined); group2Node = new ConnectionProfileGroup(Groups2, root, Groups2, undefined, undefined); group11Node = new ConnectionProfileGroup(Groups11, root, Groups11, undefined, undefined); + emptyGroup = new ConnectionProfileGroup(ConnectionProfileGroup.RootGroupName, undefined, ConnectionProfileGroup.RootGroupName); + connectionProfile = new ConnectionProfile(new TestCapabilitiesService(), undefined!); root.addGroups([group1Node]); group1Node.addGroups([group11Node]); root.addGroups([group2Node]); @@ -142,4 +149,66 @@ suite('SQL ConnectionProfileGroup tests', () => { let actual = ConnectionProfileGroup.sameGroupName(name1, name2); assert.deepEqual(actual, expected); }); + + test('test behavior when children is set to undefined', () => { + emptyGroup.children = undefined; + assert(emptyGroup.hasChildren() === false, 'Group should report no children after setting children to undefined'); + const obj = emptyGroup.toObject(); + assert.equal(obj.id, emptyGroup.id, 'toObject result has wrong id'); + assert.equal(obj.name, emptyGroup.name, 'toObject result has wrong name'); + assert(emptyGroup.hasValidConnections === true, 'Expected group to have valid connections'); + const children = emptyGroup.getChildren(); + assert(children.length === 0, 'Expected group to have 0 children'); + }); + + test('test behavior when connections is set to undefined', () => { + emptyGroup.connections = undefined; + assert(emptyGroup.hasChildren() === false, 'Group should report no children after setting connections to undefined'); + const obj = emptyGroup.toObject(); + assert.equal(obj.id, emptyGroup.id, 'toObject result has wrong id'); + assert.equal(obj.name, emptyGroup.name, 'toObject result has wrong name'); + assert(emptyGroup.hasValidConnections === true, 'Expected group to have valid connections'); + const children = emptyGroup.getChildren(); + assert.equal(children.length, 0, 'Expected group to have 0 children'); + }); + + test('test behavior with 1 child group', () => { + emptyGroup.addGroups([group1Node]); + assert(emptyGroup.hasChildren() === true, 'Group should have children if 1 child group is added'); + assert(emptyGroup.hasValidConnections === true, 'Expected group to have valid connections'); + const children = emptyGroup.getChildren(); + assert.equal(children.length, 1, 'Expected group to have 1 child'); + assert.equal(children[0].id, group1Node.id, 'Expected group child to be group1Node'); + }); + + test('test behavior with 1 child connection', () => { + emptyGroup.addConnections([connectionProfile]); + assert(emptyGroup.hasChildren(), 'Group should have children if 1 child group is added'); + assert(emptyGroup.hasValidConnections === false, 'Expected group not to have valid connections'); + const children = emptyGroup.getChildren(); + assert.equal(children.length, 1, 'Expected group to have 1 child'); + assert.equal(children[0].id, connectionProfile.id, 'Expected group child to be connectionProfile'); + }); + + test('adding undefined groups does nothing', () => { + emptyGroup.addGroups(undefined); + assert(emptyGroup.hasChildren() === false, 'Expected group not to have any children'); + // Verify adding undefined doesn't modify existing children + emptyGroup.addGroups([group1Node]); + emptyGroup.addGroups(undefined); + const children = emptyGroup.getChildren(); + assert.equal(children.length, 1, 'Expected group to have 1 child still'); + assert.equal(children[0].id, group1Node.id, 'Expected group child to be group1Node'); + }); + + test('adding undefined connections does nothing', () => { + emptyGroup.addConnections(undefined); + assert(emptyGroup.hasChildren() === false, 'Expected group not to have any children'); + // Verify adding undefined doesn't modify existing children + emptyGroup.addConnections([connectionProfile]); + emptyGroup.addConnections(undefined); + const children = emptyGroup.getChildren(); + assert.equal(children.length, 1, 'Expected group to have 1 child still'); + assert.equal(children[0].id, connectionProfile.id, 'Expected group child to be connectionProfile'); + }); });