Skip to content

Commit 7b89c15

Browse files
fix(migration): Address best-practices review for Get-DbaCmObject
- Thread-safe writes to ConnectionHost.Connections via CacheConnection helper - Case-insensitive __ExtendedStatus comparison matching PS1 -like behavior - enabledProtocols string no longer starts with misleading "None" prefix - CimErrorInfo fields changed to read-only properties - ProcessRecord uses local variable instead of mutating ComputerName parameter Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 7caa53c commit 7b89c15

1 file changed

Lines changed: 37 additions & 30 deletions

File tree

project/dbatools/Commands/GetDbaCmObjectCommand.cs

Lines changed: 37 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -107,14 +107,14 @@ protected override void BeginProcessing()
107107
/// </summary>
108108
protected override void ProcessRecord()
109109
{
110-
if (ComputerName == null)
110+
DbaCmConnectionParameter[] targets = ComputerName;
111+
if (targets == null || targets.Length == 0)
111112
{
112113
// Default to local computer when not specified
113-
string localName = Environment.MachineName;
114-
ComputerName = new DbaCmConnectionParameter[] { new DbaCmConnectionParameter(localName) };
114+
targets = new DbaCmConnectionParameter[] { new DbaCmConnectionParameter(Environment.MachineName) };
115115
}
116116

117-
foreach (DbaCmConnectionParameter connectionObject in ComputerName)
117+
foreach (DbaCmConnectionParameter connectionObject in targets)
118118
{
119119
if (!connectionObject.Success)
120120
{
@@ -183,15 +183,16 @@ protected override void ProcessRecord()
183183
}
184184

185185
// Build the enabled protocols description
186-
string enabledProtocols = "None";
186+
List<string> protocolList = new List<string>();
187187
if (connection.CimRM != ManagementConnectionProtocolState.Disabled)
188-
enabledProtocols += ", CimRM";
188+
protocolList.Add("CimRM");
189189
if (connection.CimDCOM != ManagementConnectionProtocolState.Disabled)
190-
enabledProtocols += ", CimDCOM";
190+
protocolList.Add("CimDCOM");
191191
if (connection.Wmi != ManagementConnectionProtocolState.Disabled)
192-
enabledProtocols += ", Wmi";
192+
protocolList.Add("Wmi");
193193
if (connection.PowerShellRemoting != ManagementConnectionProtocolState.Disabled)
194-
enabledProtocols += ", PowerShellRemoting";
194+
protocolList.Add("PowerShellRemoting");
195+
string enabledProtocols = protocolList.Count > 0 ? String.Join(", ", protocolList) : "None";
195196

196197
// Create list of excluded connection types
197198
ManagementConnectionType excludedTypes = ManagementConnectionType.None;
@@ -214,8 +215,7 @@ protected override void ProcessRecord()
214215
}
215216
catch (Exception ex)
216217
{
217-
if (!_disableCache)
218-
ConnectionHost.Connections[computer] = connection;
218+
CacheConnection(computer, connection);
219219
StopFunction(
220220
String.Format("[{0}] Unable to find a connection to the target system. Ensure the name is typed correctly, and the server allows any of the following protocols: {1}",
221221
computer, enabledProtocols),
@@ -275,8 +275,7 @@ private bool ProcessCimRM(ManagementConnection connection, string computer, PSCr
275275
null);
276276
connection.ReportSuccess(ManagementConnectionType.CimRM);
277277
connection.AddGoodCredential(cred);
278-
if (!_disableCache)
279-
ConnectionHost.Connections[computer] = connection;
278+
CacheConnection(computer, connection);
280279

281280
WriteEnumerable(result);
282281
return true;
@@ -293,8 +292,7 @@ private bool ProcessCimRM(ManagementConnection connection, string computer, PSCr
293292
if (errorDetails.BadCredentials)
294293
{
295294
connection.AddBadCredential(cred);
296-
if (!_disableCache)
297-
ConnectionHost.Connections[computer] = connection;
295+
CacheConnection(computer, connection);
298296
StopFunction(
299297
String.Format("[{0}] Invalid connection credentials", computer),
300298
errorRecord: new ErrorRecord(ex, "BadCredentials", ErrorCategory.AuthenticationError, computer),
@@ -350,8 +348,7 @@ private bool ProcessCimDCOM(ManagementConnection connection, string computer, PS
350348
null);
351349
connection.ReportSuccess(ManagementConnectionType.CimDCOM);
352350
connection.AddGoodCredential(cred);
353-
if (!_disableCache)
354-
ConnectionHost.Connections[computer] = connection;
351+
CacheConnection(computer, connection);
355352

356353
WriteEnumerable(result);
357354
return true;
@@ -368,8 +365,7 @@ private bool ProcessCimDCOM(ManagementConnection connection, string computer, PS
368365
if (errorDetails.BadCredentials)
369366
{
370367
connection.AddBadCredential(cred);
371-
if (!_disableCache)
372-
ConnectionHost.Connections[computer] = connection;
368+
CacheConnection(computer, connection);
373369
StopFunction(
374370
String.Format("[{0}] Invalid connection credentials", computer),
375371
errorRecord: new ErrorRecord(ex, "BadCredentials", ErrorCategory.AuthenticationError, computer),
@@ -453,8 +449,7 @@ private bool ProcessWmi(ManagementConnection connection, string computer, PSCred
453449
null);
454450
connection.ReportSuccess(ManagementConnectionType.Wmi);
455451
connection.AddGoodCredential(cred);
456-
if (!_disableCache)
457-
ConnectionHost.Connections[computer] = connection;
452+
CacheConnection(computer, connection);
458453

459454
return true;
460455
}
@@ -472,8 +467,7 @@ private bool ProcessWmi(ManagementConnection connection, string computer, PSCred
472467
if (reason == "UnauthorizedAccessException")
473468
{
474469
connection.AddBadCredential(cred);
475-
if (!_disableCache)
476-
ConnectionHost.Connections[computer] = connection;
470+
CacheConnection(computer, connection);
477471
StopFunction(
478472
String.Format("[{0}] Invalid connection credentials", computer),
479473
errorRecord: new ErrorRecord(ex, "BadCredentials", ErrorCategory.AuthenticationError, computer),
@@ -566,8 +560,7 @@ private bool ProcessPSRemoting(ManagementConnection connection, string computer,
566560
null);
567561
connection.ReportSuccess(ManagementConnectionType.PowerShellRemoting);
568562
connection.AddGoodCredential(cred);
569-
if (!_disableCache)
570-
ConnectionHost.Connections[computer] = connection;
563+
CacheConnection(computer, connection);
571564

572565
return true;
573566
}
@@ -657,7 +650,7 @@ internal static CimErrorInfo ResolveCimError(Exception exception, string compute
657650
break;
658651
}
659652
}
660-
if (originalError != null && originalError.Contains("__ExtendedStatus"))
653+
if (originalError != null && originalError.IndexOf("__ExtendedStatus", StringComparison.OrdinalIgnoreCase) >= 0)
661654
{
662655
message = String.Format("[{0}] Something went wrong when looking for {1}, in {2}. This often indicates issues with the target system.", computerName, className, ns);
663656
}
@@ -780,6 +773,20 @@ internal static bool IsProviderLoadFailure(Exception ex)
780773

781774
#region Utility
782775

776+
/// <summary>
777+
/// Stores a connection in the cache in a thread-safe manner.
778+
/// </summary>
779+
private void CacheConnection(string computer, ManagementConnection connection)
780+
{
781+
if (!_disableCache)
782+
{
783+
lock (ConnectionHost.Connections)
784+
{
785+
ConnectionHost.Connections[computer] = connection;
786+
}
787+
}
788+
}
789+
783790
/// <summary>
784791
/// Writes objects from a CIM result (which may be IEnumerable) to the pipeline.
785792
/// </summary>
@@ -812,16 +819,16 @@ private void WriteEnumerable(object result)
812819
internal class CimErrorInfo
813820
{
814821
/// <summary>CIM error code</summary>
815-
public int ErrorCode;
822+
public int ErrorCode { get; }
816823

817824
/// <summary>Human-readable error message</summary>
818-
public string Message;
825+
public string Message { get; }
819826

820827
/// <summary>Whether the error indicates a connection problem (should try next protocol)</summary>
821-
public bool BadConnection;
828+
public bool BadConnection { get; }
822829

823830
/// <summary>Whether the error indicates invalid credentials</summary>
824-
public bool BadCredentials;
831+
public bool BadCredentials { get; }
825832

826833
/// <summary>
827834
/// Creates a new CimErrorInfo instance.

0 commit comments

Comments
 (0)