2
\$\begingroup\$

I'm an absolute newb when it comes to PowerShell, and this is pretty much my first real script. I'd now like to know ways to improve it and if possible to shorten the code even further. I use it together with an XML file containing links and keywords to download the "latest" version of different applications to keep my deploymentshare up-to-date.

##################################################################################################################
# Set variables
##################################################################################################################

$DownloadFolder = "$PSScriptRoot\Downloads"
$ApplicationShare = "E:\MDTProduction\Applications"
$Time = (Get-Date -Format 'yyyy-MM-dd_HH_mm_ss')
$LogPath = "$PSScriptRoot\DownloadLog_$Time.txt"

##################################################################################################################
# Function to create download folder
##################################################################################################################

function CreateDownloadFolder ()
{
  try
  {
    if (Test-Path "$FinalDownloadFolder")
    {
      Write-Host "$Step1$Skip" -ForegroundColor Yellow
    }
    else
    {
      New-Item "$FinalDownloadFolder" -ItemType Directory -ErrorAction Stop | Out-Null
      Write-Host "$Step1$Done" -ForegroundColor Green
    }
  }
  catch
  {
    $ErrorMessage = $_.Exception.Message
    Write-Host "$Step1$Fail" -ForegroundColor Red
    Write-Information "Failed to create folder with error message: $ErrorMessage" .
    break
  }
}

##################################################################################################################
# Function to download files
##################################################################################################################

function DownloadFiles ()
{
  try
  {
    Write-Progress -Activity "Downloading $FileName" -Status "Please wait..."
    Invoke-WebRequest $DownloadURL -OutFile $FinalDownloadFolder\$Filename -ErrorAction Stop
    Write-Host "$Step2$Done" -ForegroundColor Green
  }
  catch
  {
    $ErrorMessage = $_.Exception.Message
    Write-Information "Failed to download application with error message: $ErrorMessage" .
    Write-Host "$Step2$Fail" -ForegroundColor Red
    break
  }
}

##################################################################################################################
# Function to compare file versions and replace files
##################################################################################################################

function CompareFilesAndReplace ()
{
  try
  {
    $NewFile = (Get-Item $FinalDownloadFolder\$Filename -ErrorAction SilentlyContinue)
    $OldFile = (Get-Item $FinalApplicationShare\*.* -Exclude *.mst, *.ini -ErrorAction SilentlyContinue)
    $NewFileVersion = $NewFile.VersionInfo.FileVersion -replace ",","." -replace " ",""
    $OldFileVersion = $OldFile.VersionInfo.FileVersion -replace ",","." -replace " ",""
    if ($OldFile -and $NewFile)
    {
      if ($NewFileVersion -gt $OldFileVersion)
      {
        Write-Host "$Step3$Done" -ForegroundColor Green
        Remove-Item "$OldFile" -Force -ErrorAction Stop
        Copy-Item "$NewFile" -Destination "$FinalApplicationShare" -Force -ErrorAction Stop
        Write-Host "$Step4$Done" -ForegroundColor Green
        Write-Host "INFO: $NewFileVersion > $OldFileVersion" -ForegroundColor Cyan
        Write-Host "INFO: An update was found, files were replaced..." -ForegroundColor Cyan
      } 
      elseif ($NewFileVersion -eq "" -and $OldFileVersion -eq "")
      {
        Write-Host "$Step3$Done" -ForegroundColor Green
        Remove-Item "$OldFile" -Force -ErrorAction Stop
        Copy-Item "$NewFile" -Destination "$FinalApplicationShare" -Force -ErrorAction Stop
        Write-Host "$Step4$Done" -ForegroundColor Green
        Write-Host "INFO: No version found, took default action to replace files." -ForegroundColor Cyan
      }
      elseif ($NewFileVersion -eq $OldFileVersion)
      {
        Write-Host "$Step3$Done" -ForegroundColor Green
        Write-Host "INFO: Downloaded version $NewFileVersion, current version $OldFileVersion" -ForegroundColor Cyan
        Write-Host "INFO: File versions are equal, keeping existing file." -ForegroundColor Cyan
      }
    }
    else
    {
      Write-Host "INFO: Running instance in download only." -ForegroundColor Cyan
      Write-Information "Unable to find any files in the $FinalApplicationShare directory, if this is supposed to be true disregard this message, otherwise check the path..."
    }
  }
  catch
  {
    $ErrorMessage = $_.Exception.Message
    Write-Information "Failed to query or replace application with error message: $ErrorMessage" .
    Write-Host "$Step3$Fail" -ForegroundColor Red
    break
  }
}


##################################################################################################################
# Begin processing the XML-data and start up the main functions of the script
##################################################################################################################

Start-Transcript -Path "$LogPath" -Force

#Import XML-data
try
{
  [xml]$DownloadSettings = Get-Content "$PSScriptRoot\DownloadSettings.xml"
}
catch
{
  $ErrorMessage = $_.Exception.Message
  Write-Host "Failed to get settings from $DownloadSettings with error message: $ErrorMessage" .

  break
}

#Awesome special effects
$Step1 = "STEP: Create the download folder..."
$Step2 = "STEP: Download the application... "
$Step3 = "STEP: Compare the applications... "
$Step4 = "STEP: Replace the applications... "
$Done = "`t`t`t`t`t`t`t [DONE]"
$Fail = "`t`t`t`t`t`t`t [FAIL]"
$Skip = "`t`t`t`t`t`t`t [SKIP]"

#Process XML-data
foreach ($Application in $DownloadSettings.xml.Application)
{
  $DownloadURL = $Application.DownloadURL
  $FolderName = $Application.ApplicationName
  $Filename = $Application.Filename
  $Keyword = $Application.Keyword 
  $FinalDownloadFolder = "$DownloadFolder\$Foldername\Source"
  $FinalApplicationShare ="$ApplicationShare\$Foldername\Source"

  #Use these settings if a link is present and keyword and filename are not set
  if (($DownloadURL) -and (!$Keyword) -and (!$Filename))
  {
    $Block = "[1]"
    $FileName = $DownloadURL.Split('/')[-1].Split('?')[0] -replace "%20","_"
  }

  #Use these settings if link and keyword has a value, but filename is not set
  elseif (($DownloadURL) -and ($Keyword) -and (!$Filename))
  {
    $Block = "[2]"
    $DownloadURL = ((Invoke-WebRequest $DownloadURL).Links | Where { $_.href -like "*http*$Keyword*" }).href
    $Filename = $DownloadURL.Split('/')[-1].Split('?')[0] -replace "%20","_"
  }

  #Use these settings if link, keyword and filename all have values
  elseif (($DownloadURL) -and ($Keyword) -and ($Filename))
  {
    $Block = "[3]"
    $DownloadURL = ((Invoke-WebRequest $DownloadURL).Links | Where { $_.href -like "*http*$Keyword" -or $_.innerText -like "$Keyword" }).href
  }

  #Output some useful information to the logs, in case of troubleshooting
  Write-Host ""
  Write-Host "$Foldername" -ForegroundColor White;
  Write-Host "-------------------------------------------------------------------"
  Write-Information "ConditionBlock.: $Block"
  Write-Information "DeploymentShare: $FinalApplicationShare"
  Write-Information "DownloadFolder.: $FinalDownloadFolder"
  Write-Information "DownloadURL....: $DownloadURL"
  Write-Information "Filename.......: $Filename"
  Write-Information "Keyword........: $Keyword"

  CreateDownloadFolder
  DownloadFiles
  CompareFilesAndReplace
}

Write-Host
Stop-Transcript 

Sample input XML file used:

<xml>

    <Application>
        <ApplicationName>Install - 7-Zip (Latest) - x64</ApplicationName>
        <DownloadURL>http://www.snapfiles.com/downloads/7zip/dl7zip.html</DownloadURL>
        <FileName></FileName>
        <Keyword>*7z*x64*exe*</Keyword>
    </Application>

    <Application>
        <ApplicationName>Install - Adobe Flash Player (Latest) - x86</ApplicationName>
        <DownloadURL>http://fpdownload.macromedia.com/pub/flashplayer/latest/help/install_flash_player.exe</DownloadURL>
        <FileName></FileName>
        <Keyword></Keyword>
    </Application>

    <Application>
        <ApplicationName>Install - Adobe Flash Player for IE (Latest) - x86</ApplicationName>
        <DownloadURL>http://www.snapfiles.com/downloads/flashplayerie/dlflashplayerie.html</DownloadURL>
        <FileName></FileName>
        <Keyword>*flashplayer*exe*</Keyword>
    </Application>

    <Application>
        <ApplicationName>Install - Citrix Receiver (Latest) - x86</ApplicationName>
        <DownloadURL>https://downloadplugins.citrix.com/Windows/CitrixReceiverWeb.exe</DownloadURL>
        <FileName></FileName>
        <Keyword></Keyword>
    </Application>

    <Application>
        <ApplicationName>Install - FileZilla (Latest) - x64</ApplicationName>
        <DownloadURL>https://sourceforge.net/projects/filezilla/files/latest/download</DownloadURL>
        <FileName></FileName>
        <Keyword>*filezilla*64*exe*</Keyword>
    </Application>

    <Application>
        <ApplicationName>Install - Google Chrome (Latest) - x64</ApplicationName>
        <DownloadURL>https://dl.google.com/dl/chrome/install/googlechromestandaloneenterprise64.msi</DownloadURL>
        <FileName></FileName>
        <Keyword></Keyword>
    </Application>

    <Application>
        <ApplicationName>Install - Java Runtime (Latest) - x86-x64</ApplicationName>
        <DownloadURL>http://www.java.com/en/download/manual.jsp</DownloadURL>
        <FileName>Java_x64_latest.exe</FileName>
        <Keyword>*windows*offline*64*bit*</Keyword>
    </Application>

    <Application>
        <ApplicationName>Install - Java Runtime (Latest) - x86-x64</ApplicationName>
        <DownloadURL>http://www.java.com/en/download/manual.jsp</DownloadURL>
        <FileName>Java_x86_latest.exe</FileName>
        <Keyword>*windows*offline</Keyword>
    </Application>

    <Application>
        <ApplicationName>Install - KeePass (Latest) - x86</ApplicationName>
        <DownloadURL>https://sourceforge.net/projects/keepass/files/latest/download</DownloadURL>
        <FileName></FileName>
        <Keyword>*keepass*setup*exe*</Keyword>
    </Application>

    <Application>
        <ApplicationName>Install - Mozilla FireFox (Latest) - x64</ApplicationName>
        <DownloadURL>http://www.snapfiles.com/downloads/phoenixmoz/dlphoenixmoz.html</DownloadURL>
        <FileName></FileName>
        <Keyword>*win64*firefox*exe*</Keyword>
    </Application>

    <Application>
        <ApplicationName>Install - Notepad++ (Latest) - x64</ApplicationName>
        <DownloadURL>http://www.snapfiles.com/downloads/notepadplus/dlnotepadplus.html</DownloadURL>
        <FileName></FileName>
        <Keyword>*npp*installer*64*</Keyword>
    </Application>

    <Application>
        <ApplicationName>Install - TeamViewer (Latest) - x86</ApplicationName>
        <DownloadURL>http://download.teamviewer.com/download/TeamViewer_Setup.exe</DownloadURL>
        <FileName></FileName>
        <Keyword></Keyword>
    </Application>

</xml>
\$\endgroup\$
1
\$\begingroup\$

Good use of comments and I see some very verbose output being used here. All in all the script is built well. That being said there are a few points worth bringing up. Some carry more weight then others.

Taking advantage of scope

You are using parent variables a lot in this code. While that is perfectly acceptable and that is by design how PowerShell works. Quoting about_scopes

Unless an item is private, the child scope can view the items in the parent scope. And, it can change the items by explicitly specifying the parent scope, but the items are not part of the child scope.

It is not initially clear where the variables you are using are declared. You have some at the start of the script, before your XML loop and inside the loop. If you are going to be using these try and at least keep them together where possible. Better yet....

Function parameters

Your functions could easily be used in other scripts or endeavors. However, they currently are not very scalable with their variables that rely on parent scope. I would look into your functions taking parameters for input. A more in depth start would be to look at [about_functions][2] but a simple example would be for DownloadFiles:

function DownloadFiles{
    param(
        [string]$DownloadUrl,
        [string]$Destination
    )

    # Stuff goes here
}

Now your function calls will be clearer since you know what files you are downloading.

DownloadFiles -DownloadUrl $DownloadURL  -Destination "$FinalDownloadFolder\$Filename"

Error handling

You do this very well. Especially when it comes to downloading files and verifying if your XML attributes are populated. You do miss this two places where you are doing some initial downloads though

$DownloadURL = ((Invoke-WebRequest $DownloadURL).Links | Where { $_.href -
# ... truncated
$DownloadURL = ((Invoke-WebRequest $DownloadURL).Links | Where { $_.href -

Those could fail just as easily and the script would fall flat. You should account for those as well.

String Concatenation for Creating File Paths

 $FinalDownloadFolder = "$DownloadFolder\$Foldername\Source"

You gather both parent folder and filename separately and put them together later. Consider using the [io.path] method Combine() which will make it so you don't have to worry about the presence of a slash in the input.

$FinalDownloadFolder = [IO.Path]::Combine($DownloadFolder, $Foldername, "Source")

Try sub-expressions

Consider this code

  $ErrorMessage = $_.Exception.Message
  Write-Host "Failed to get settings from $DownloadSettings with error message: $ErrorMessage" .

You save the $errorMessage and use it the once in the string on the next line. Why bother saving it when it is already in the pipeline variable?

Write-Host "Failed to get settings from $DownloadSettings with error message: $($_.Exception.Message)"

The sub-expression will allow PowerShell to expand that variable inside the string.

Truthy Falsy consistency

You check variables as booleans which is perfectly fine. I see you using two different styles when you are doing it though

if ($NewFileVersion -gt $OldFileVersion)

and

if (($DownloadURL) -and (!$Keyword) -and (!$Filename))

You don't need to use the brackets in that second case. Just remove them. While they do have there place they are redundant in the above example.

\$\endgroup\$
  • \$\begingroup\$ Thanks @Matt, this was exactly the kind of help I was looking for. I updated the script, did I get it right? \$\endgroup\$ – Dalle Sep 30 '16 at 18:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.