Powershell zip subfolders recursively, conditionally

Question

This is a progression of the script I posted here: Zip the contents of subfolders, conditionally

It does this:

1. Determines all subfolders recursively

2. Checks each subfolder for files older than 31 days which aren't .zip

3. If such files found, creates a new folder within, labelled with the date

4. Moves the files to the new folder

5. Creates a zipped copy of the new folder

6. Deletes the new folder and its contents

There's error-handling and logging built-in

---

#Run under Powershell v1 or v2

#Pass the target log folder as a parameter: '-LogFolder Path\to\logfolder'
param(
    [parameter(Mandatory=$true)]
    [ValidateScript({
        if (Test-Path $_ -PathType Container) {$true}
        else {Throw "Folder $_ not found"}})]
    [String]
    $LogFolder
)

#Get working directory, for outputting log file
$WorkingDirectory = (pwd).path

#Print datestamp to the log file
Get-date >> "$WorkingDirectory\OutputLog.txt"
"Log directory: $LogFolder" >> "$WorkingDirectory\OutputLog.txt"

#Record total size of the target log folder before compression
$Logsize = Get-ChildItem -recurse $LogFolder | Measure-Object -property length -sum
$MBsize = "{0:N2}" -f ($Logsize.sum / 1MB) + " MB"
"Initial Log folder size: $MBsize" >> "$WorkingDirectory\OutputLog.txt"

#Verify that the Zip.dll is present
$testzip = Test-Path .\ICSharpCode.SharpZipLib.dll

#If the Zip.dll is found, bind it to a variable and load it, otherwise exit
#In other words, if the script cannot load the Zip module, it will do nothing
if ($testzip -eq $True){
$ZipModule = Get-ChildItem .\ICSharpCode.SharpZipLib.dll | Select -ExpandProperty FullName
[void][System.Reflection.Assembly]::LoadFrom($ZipModule)
}
Else{
"Zip dll not found or couldn't be loaded. 
Check file is present and unblocked. 
Exiting" >> "$WorkingDirectory\OutputLog.txt" 
Exit
}

#Get all subfolders
$subfolders = Get-ChildItem $LogFolder -Recurse | 
Where-Object { $_.PSIsContainer -and $_.fullname -notmatch "\\jsonTemplates\\verifier\\?" }

ForEach ($s in $subfolders) {

    $path = $s  #$s variable contains each folder
    $path 
    Set-Location $path.FullName

    $fullpath = $path.FullName

    #Get all items older than 31 days, exclude zip files and folders
    $items = Get-ChildItem -Exclude *.zip | 
    Where-Object {$_.LastWriteTime -lt (Get-date).AddDays(-31) 
    -and -not $_.psIsContainer}

    #Verify that there are such items in this directory, catch errors
    if ( $(Try { Test-Path $items } 
    Catch { "Cannot find items in $fullpath. 
    Sub-folders will be processed afterwards. 
    ERROR: $_" >>  "$WorkingDirectory\OutputLog.txt"  }) ) {

        $date = Get-Date -Format 'yyyy-MM-dd_HH-mm'
        $newpath = "$path-$date"
        $newpath
        $newfld = New-Item -ItemType Directory -name $newpath

        $src = $newfld.FullName

        #move items to newly-created folder
        Move-Item $items -destination $src 

        $dest = "$src.zip"
        "Compressing $src to $dest"  >>  "$WorkingDirectory\OutputLog.txt"  

        #the following block zips the folder
        try{
            $zip = New-Object ICSharpCode.SharpZipLib.Zip.FastZip
            $zip.CreateZip($dest, $src, $true, ".*")
            Remove-Item $src -force -recurse
        }
        catch { 
            "Folder could not be compressed. Removal of $src ABORTED. 
            ERROR: $_" >> "$WorkingDirectory/OutputLog.txt" 
        }
    }

}

#Record total size of the target log folder after compression
$Logsize = Get-ChildItem -recurse $LogFolder | Measure-Object -property length -sum
$MBsize = "{0:N2}" -f ($Logsize.sum / 1MB) + " MB"
"Final Log folder size: $MBsize" >> "$WorkingDirectory\OutputLog.txt"

Set-Location $LogFolder

Answer 1

[ValidateScript({
    if (Test-Path $_ -PathType Container) {$true}
    else {Throw "Folder $_ not found"}})]

Test-Path already returns a [bool] which is what is expected by [ValidateScript()] so this can be simplified by removing the if/else:

[ValidateScript(
    (Test-Path $_ -PathType Container) -or $(throw "Folder $_ not found")
)]

---

pwd is an alias for Get-Location. I always recommend avoiding aliases and abbreviations in reusable scripts.

---

if ($testzip -eq $True)

This can be simplified to:

if ($testzip)

---

Else{
"Zip dll not found or couldn't be loaded. 
Check file is present and unblocked. 
Exiting" >> "$WorkingDirectory\OutputLog.txt" 
Exit
}

This should probably be thrown as an exception:

else {
    $e = "Zip dll not found or couldn't be loaded. 
    Check file is present and unblocked. 
    Exiting" 
    $e >> "$WorkingDirectory\OutputLog.txt" 
    throw [System.IO.FileNotFoundException]$e
}

---

$path = $s  #$s variable contains each folder
$path 
Set-Location $path.FullName

There are a two things I want to address here.

First, the middle line:

$path

This is implicitly calling Write-Object $path which in your script is probably writing this out to the console, and that's probably the desired behavior. You should be aware that this actually makes it the return value of the current scope/scriptblock you're in, which means it's part of the return value of your script.

You might not care about that right now, and it might never matter because you'll never call this script from something that checks its output, but it's bad practice (unless you very explicitly do want that to be part or all of its return value).

If all you want is to show the path on the screen, use Write-Host. Or even better, add [CmdletBinding()] above your param() block and then use Write-Verbose. That will only show the output when someone calls the script with -Verbose.

Second, I believe you should avoid changing the current path when possible. If it's not possible or it's impractical, then at least make use of Push-Location and Pop-Location along with try/finally so that you restore the original working directory.

---

if ( $(Try { Test-Path $items } 
    Catch { "Cannot find items in $fullpath. 
    Sub-folders will be processed afterwards. 
    ERROR: $_" >>  "$WorkingDirectory\OutputLog.txt"  }) ) {

This is confusing. Test-Path should not throw an exception; it should return $false when the path doesn't exist, so your catch should never run. In addition, if it did run, the entire expression might still return $true making the if always satisfied.

Seems this would be better written as:

if ( Test-Path $items ) {
    # ... do stuff
} else {
    "Cannot find items in $fullpath. 
    Sub-folders will be processed afterwards." >>  "$WorkingDirectory\OutputLog.txt"
}

---

You should wrap your logging into a function instead of having "Some message" >> "$WorkingDirectory\OutputLog.txt" strewn about the code. If you ever have to When you have to update that it will be a pain.

Consider something like this:

function Write-Log {
[CmdletBinding()]
param(
    [Parameter(
        Mandatory=$true
    )]
    [ValidateNotNullOrEmpty()]
    [String]
    $Message ,

    [Parameter(
        Mandatory=$true
    )]
    [ValidateNotNullOrEmpty()]
    [ValidateScript( { $_ | Test-Path -PathType Leaf -IsValid } )]
    [String]
    $LogPath
)
    $Message  >>  $LogPath
}

Now you can do:

Write-Log -Message "Compressing $src to $dest" -LogPath "$WorkingDirectory\OutputLog.txt"

And, you can do this at the top of the script (with v3 or higher):

$PSDefaultParameterValues = @{
    "Write-Log:LogPath" = "$WorkingDirectory\OutputLog.txt"
}

And then just do:

Write-Log -Message "Compressing $src to $dest"

Of course you could also make -LogPath optional, and give it a default value if you must (for v2 and below compatibility).

Even if you have to supply the log file every time though, so what, you're already doing that. And wrapping this in a function gives you more options for changing how its written later (like automatically adding a timestamp to each log entry).