Skip to content

Fix critical comparison operator bug and improve error handling in DOrcDeployModule #7

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.

Sign up for GitHub

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

Open
Copilot wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Fix critical comparison operator bug and improve error handling in DOrcDeployModule #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
03ee07e
Initial plan
Copilot Oct 23, 2025
dd77bff
Fix critical bugs: comparison operator and empty catch blocks
Copilot Oct 23, 2025
4ffe779
Add error handling and missing branches
Copilot Oct 23, 2025
08fc2cb
Add test cases for critical bug fixes
Copilot Oct 23, 2025
893d283
Add comprehensive code review documentation
Copilot Oct 23, 2025
f8ee73b
Remove REVIEW_SUMMARY.md as requested in code review
Copilot Oct 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 46 additions & 21 deletions DOrcDeployModule.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,19 @@ function Invoke-VsDbCmd {
}

function SendEmailToDOrcSupport([string] $StrSubject) {
$Msg = New-Object Net.Mail.MailMessage
$Smtp = New-Object Net.Mail.SmtpClient($DOrcSupportEmailSMTPServer)
$Msg.From = $DOrcSupportEmailFrom
$Msg.To.Add($DOrcSupportEmailTo)
$Msg.Subject = $StrSubject
$Smtp.Send($Msg)
$Smtp = $null
$Msg = $null
try {
$Msg = New-Object Net.Mail.MailMessage
$Smtp = New-Object Net.Mail.SmtpClient($DOrcSupportEmailSMTPServer)
$Msg.From = $DOrcSupportEmailFrom
$Msg.To.Add($DOrcSupportEmailTo)
$Msg.Subject = $StrSubject
$Smtp.Send($Msg)
$Smtp = $null
$Msg = $null
}
catch {
Write-Warning "Failed to send email notification: $($_.Exception.Message)"
}
}

function GetDateReverse() {
Expand All @@ -68,7 +73,18 @@ function CheckDiskSpace([string[]] $servers, [int] $minMB = 100) {
Write-Host " Checking disk space..."
foreach ($server in $servers) {
$serv = "[" + $server.Trim() + "]"
$ntfsVolumes = Get-WmiObject -Class win32_volume -cn $server | Where-Object {($_.FileSystem -eq "NTFS") -and ($_.driveletter)}
try {
$ntfsVolumes = Get-WmiObject -Class win32_volume -cn $server | Where-Object {($_.FileSystem -eq "NTFS") -and ($_.driveletter)}
}
catch {
Write-Warning "Failed to query disk space on $serv : $($_.Exception.Message)"
$bolSpaceCheckOK = $false
continue
}
if (-not $ntfsVolumes) {
Write-Warning "No NTFS volumes found on $serv"
continue
}
foreach ($ntfsVolume in $ntfsVolumes) {
#Considered checking for the existance of the pagefile file but it could be on C: which we care about (either orphaned or current)
if ($ntfsVolume.DriveLetter -eq "P:") { write-host " " $ntfsVolume.DriveLetter "Skipped..."}
Expand Down Expand Up @@ -464,7 +480,7 @@ function GetDbInfoByTypeForEnv([string] $strEnvironment, [string] $strType) {
$EnvId=(Invoke-RestMethod -Uri $uri -method Get -Credential $credentials -ContentType 'application/json').EnvironmentId
$uri=$RefDataApiUrl + 'RefDataEnvironmentsDetails/' + $EnvId
$table=Invoke-RestMethod -Uri $uri -method Get -Credential $credentials -ContentType 'application/json'
$table=$table.DbServers | where {$_.Type -eq $strType} | Select Name, ServerName
$table=$table.DbServers | Where-Object {$_.Type -eq $strType} | Select-Object Name, ServerName
$strResult="Invalid"
if ($table.Name.count -eq 0) {
Write-Host "No entries returned for $strEnvironment $strType"
Expand Down Expand Up @@ -674,7 +690,7 @@ function DeleteRabbit([string]$mode, [string[]]$deleteStrings, [string]$RabbitUs
$exchanges = Invoke-RestMethod $url -Credential $Credentials -DisableKeepAlive -ErrorAction Continue -Method Get
$deleteExchanges = @()

foreach ($exchange in $exchanges | where {-not ($_.name.Contains('amq.'))}) {
foreach ($exchange in $exchanges | Where-Object {-not ($_.name.Contains('amq.'))}) {
if ($deleteString.Length -gt 0) {
if ($exchange.name.Contains($deleteString)) {
$deleteExchanges += $exchange
Expand All @@ -695,7 +711,7 @@ function DeleteRabbit([string]$mode, [string[]]$deleteStrings, [string]$RabbitUs
Write-Host "Deleted exchange: " $deleteExchange.name
}
}
elseif ($mode = 'queue') {
elseif ($mode -eq 'queue') {
$url = "http://$([System.Web.HttpUtility]::UrlEncode($Server)):$([System.Web.HttpUtility]::UrlEncode($RabbitAPIPort))/api/queues/%2f/"

$queues = Invoke-RestMethod $url -Credential $Credentials -DisableKeepAlive -ErrorAction Continue -Method Get
Expand Down Expand Up @@ -734,6 +750,9 @@ function DeleteRabbit([string]$mode, [string[]]$deleteStrings, [string]$RabbitUs
}
}
}
else {
Write-Warning "Invalid mode specified: $mode. Valid values are 'exchange' or 'queue'."
}
}
}

Expand Down Expand Up @@ -894,7 +913,7 @@ function Stop-Services {
$strRemServiceName = $null
$strRemServiceStatus = $null
$strServiceNameGen = $strService + "*"
$oService = Get-Service $strServiceNameGen -computer $strComputer | select -First 1 #ensure only 1 service processed at a time
$oService = Get-Service $strServiceNameGen -computer $strComputer | Select-Object -First 1 #ensure only 1 service processed at a time
$strRemServiceName = $oService.Name
$strRemServiceStatus = $oService.Status
$oService = $null
Expand All @@ -911,7 +930,7 @@ function Stop-Services {
if ($strRemServiceStatus -eq "Stopped") {break}
if ($i -eq $retryCount) {
$ServicePID = $null
$ServicePID = (get-wmiobject win32_service -computername $strComputer | where { $_.name -eq $strService}).processID
$ServicePID = (get-wmiobject win32_service -computername $strComputer | Where-Object { $_.name -eq $strService}).processID
write-host " Killing PID:" $ServicePID
Invoke-Command -ComputerName $strComputer {param($ServicePID) Stop-Process $ServicePID -Force} -Args $ServicePID -ErrorAction SilentlyContinue
}
Expand Down Expand Up @@ -1095,7 +1114,7 @@ function IsMSIx64([string] $strWiXToolsDir, [string] $strMSIFullName) {

function GetRemPSCompName([string] $strServerName) {
$Result = ""
$Result = icm -ComputerName $strServerName {Get-Content env:computername} -erroraction SilentlyContinue
$Result = Invoke-Command -ComputerName $strServerName {Get-Content env:computername} -erroraction SilentlyContinue
return $Result
}

Expand Down Expand Up @@ -2494,8 +2513,8 @@ function Invoke-RemoteProcess([string] $serverName, [string] $strExecutable, [st
Invoke-Command -session $session { $Process = New-Object System.Diagnostics.Process }
Invoke-Command -session $session { $Process.StartInfo = $ProcessInfo }
Invoke-Command -session $session { $Process.Start()}
Invoke-Command -session $session { try { $out = $Process.StandardOutput.ReadToEndAsync() } catch {} }
Invoke-Command -session $session { try { $outErr = $Process.StandardError.ReadToEndAsync() } catch {} }
Invoke-Command -session $session { try { $out = $Process.StandardOutput.ReadToEndAsync() } catch { Write-Verbose "Could not read StandardOutput" } }
Invoke-Command -session $session { try { $outErr = $Process.StandardError.ReadToEndAsync() } catch { Write-Verbose "Could not read StandardError" } }
Invoke-Command -session $session { $Process.WaitForExit() }
if (![String]::IsNullOrEmpty($strExecutableChild)) {
do {
Expand Down Expand Up @@ -3013,7 +3032,7 @@ function Get-ServerIDs
$serverName = $Row.Server_Name.Trim()
$serverType = $Row.Application_Server_Name.Trim()
try {
[guid]$id = icm $serverName {(get-wmiobject Win32_ComputerSystemProduct).UUID} -ErrorAction Stop
[guid]$id = Invoke-Command $serverName {(get-wmiobject Win32_ComputerSystemProduct).UUID} -ErrorAction Stop
$result += $serverName.ToUpper() + ":" + $id + ";"
}
catch {
Expand Down Expand Up @@ -3041,7 +3060,7 @@ function Check-ServerIDsDifferent
$serverName = $Row.Server_Name.Trim()
$serverType = $Row.Application_Server_Name.Trim()
try {
[guid]$id = icm $serverName {(get-wmiobject Win32_ComputerSystemProduct).UUID} -ErrorAction Stop
[guid]$id = Invoke-Command $serverName {(get-wmiobject Win32_ComputerSystemProduct).UUID} -ErrorAction Stop
}
catch {
Write-Host "[Get-ServerIDs] Server $ServerName not reachable. This is expected for a new build."
Expand Down Expand Up @@ -3284,6 +3303,10 @@ function CheckBackup ([string] $SourceInstance, [string] $SourceDB, [string] $Re
}
else {return $false}
}
else {
Write-Warning "Unexpected RestoreMode: $RestoreMode after validation"
return $false
}
}

Function Get-MSHotfix {
Expand Down Expand Up @@ -3770,12 +3793,14 @@ Function Check-IsCitrixServer {
$os = Get-CimInstance -ComputerName $compName -ClassName Win32_OperatingSystem -Property *
$compOS = $os.Caption
$os = $null
} catch { }
} catch {
Write-Verbose "Could not retrieve OS information for $compName : $($_.Exception.Message)"
}
if ($compOS -eq "unknown") {
write-host "[Check-IsCitrixServer] Unable to identify O/S on: $compName"
} elseif ($compOS.ToLower() -match "server"){
write-host "[Check-IsCitrixServer] $compName is $compOS"
$rkCitrixValueCount = icm -ComputerName $compName { $rkCitrix = get-item HKLM:\SOFTWARE\Citrix -ErrorAction SilentlyContinue ; return $rkCitrix.ValueCount }
$rkCitrixValueCount = Invoke-Command -ComputerName $compName { $rkCitrix = get-item HKLM:\SOFTWARE\Citrix -ErrorAction SilentlyContinue ; return $rkCitrix.ValueCount }
if ($rkCitrixValueCount -gt 0) {
write-host "[Check-IsCitrixServer] $compName is a Citrix Server..."
$result = $true
Expand Down
39 changes: 39 additions & 0 deletions DOrcDeployModule.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,45 @@
$here = Split-Path -Parent $MyInvocation.MyCommand.Path
Import-Module "$here\DOrcDeployModule.psm1" -ErrorAction Stop

Describe "DeleteRabbit Mode Parameter Tests" {
Context "Valid mode parameter" {
BeforeAll {
Mock -CommandName Invoke-RestMethod -MockWith { return @() }
Mock -CommandName ConvertTo-SecureString -MockWith { return "SecurePassword" }
Mock -CommandName Add-Type -MockWith { }
}

It "Should handle 'exchange' mode correctly" {
{ DeleteRabbit -mode 'exchange' -deleteStrings @('test') -RabbitUserName 'user' -RabbitPassword 'pass' -RabbitAPIPort '15672' -Server 'localhost' } | Should -Not -Throw
}

It "Should handle 'queue' mode correctly" {
{ DeleteRabbit -mode 'queue' -deleteStrings @('test') -RabbitUserName 'user' -RabbitPassword 'pass' -RabbitAPIPort '15672' -Server 'localhost' } | Should -Not -Throw
}
}

Context "Invalid mode parameter" {
BeforeAll {
Mock -CommandName Invoke-RestMethod -MockWith { return @() }
Mock -CommandName ConvertTo-SecureString -MockWith { return "SecurePassword" }
Mock -CommandName Add-Type -MockWith { }
}

It "Should not throw for invalid mode (warning is displayed instead)" {
# The function now handles invalid modes gracefully with a warning instead of throwing
{ DeleteRabbit -mode 'invalid' -deleteStrings @('test') -RabbitUserName 'user' -RabbitPassword 'pass' -RabbitAPIPort '15672' -Server 'localhost' -WarningAction SilentlyContinue } | Should -Not -Throw
}
}
}

Describe "CheckBackup RestoreMode Parameter Tests" {
Context "Invalid RestoreMode value" {
It "Should throw for invalid RestoreMode" {
{ CheckBackup -SourceInstance 'localhost' -SourceDB 'TestDB' -RestoreMode 'invalid' } | Should -Throw "wrong RestoreMode*"
}
}
}

Describe "Get-DorcCredSSPStatus tests" {
Context "Computer reachable"{
Context "Returns an object with all the expected properties"{
Expand Down
Loading