Help with a script

Hello all,

I am working on a script to find and delete emails in exchange online.

I’ve been browsing through the Powershell books wrote by Don and so far I have managed to come up with this.

[pre]

#Search-ABMEmails by MessageID
#Usage: Search.ps1 -MessageID [ID] | Delete-ABMEmails
function Search-ABMEmails {
[CmdletBinding()]
Param(
# MessageID
[Parameter(ValueFromPipeline=$true)]
[Parameter(ValueFromPipelineByPropertyName=$true)]
[Parameter(Mandatory=$true)]
[string[]]$MessageID
)#Param
Begin{}
Process{
$script:Trace=Get-MessageTrace-MessageID "$MessageID"|`
Select-Object-Propery @{name="Identity";expression={$_.RecipientAddress}}, SenderAddress,Subject |`
Format-Table-AutoSize
$script:RecipientAddress= [scriptblock]::Create($Trace.Identity)
$script:SenderAddress= [scriptblock]::Create($Trace.SenderAddress)
$script:Subject= [scriptblock]::Create($Trace.Subject)
Write-Output$Trace
}#Process
End{}
}#Search-ABMEmails
function Remove-ABMEmails {
foreach ($Identityin$Trace){
Search-Mailbox-Identity $RecipientAddress|`
Search-Query {Subject:"$Subject" AND From:$SenderAddress} -DeleteContent #Search-Mailbox -DeleteContent
}#foreach
}#Delete-ABMEmails
[/pre]

The command should work like this;

Search.ps1 -MessageID [ID] with the option to pipe it to the other function for deletion.

I have the impression that this will fail, still haven’t mastered enough Powershell knowledge.

I would be grateful if any of you PS experts could provide any advice on how to make this work. :slight_smile: :slight_smile: :slight_smile:

OK, so you have a couple things.

Here’s what you did well:

  1. Good work on naming your functions correctly and using approved verbs.
  2. Your methodology is mostly sound (some errors we'll get to in a moment) and you're using some stuff a lot of people consider advanced, and using it properly, so nice work!
Here's what you need to be mindful of and work on:
  1. Personally, I like to keep things very tidy, so I know what I'm looking at. Proper indentation means a lot less unnecessary comments that usually end up detracting from the readability of the code further.
  2. Use of multiple `[Parameter()]` attributes. Each attribute is applied separately, and usually to separate parameter sets. In the absence of a defined parameter set name for each attribute on a parameter, I'm not sure the behaviour is well-defined, but I'd expect either errors or it just picking one at random to apply. If you want all the properties you're supplying to apply at the same time, you need to put them all in the same attribute, separated by commas.
    • Also, unless you're doing work with V2 of PowerShell, you don't need to specify `=$true` for most attribute properties. The presence of their name is sufficient for PS to infer you want them to be `$true`.
  3. Watch your spacing between a command and the parameter names. `Select-Object` is the command, `-Property` is a parameter name. `Select-Object-Property` will throw an error, as it will be read as the entire thing being the command name and that command won't be found. `Select-Object -Property` is how it needs to be.
    • Looking at the patterns of how this turns up, it might just be a copy-paste artifact. Sometimes it's easier to put the code in a gist and link it in your forum posts to embed it. The native code blocks here aren't always perfect. :)
  4. Don't use `$script:` or other specific scoping unless you absolutely must. Breaking out of the function scope like that is very rarely necessary. In most cases, you want to take input with parameters, and send output along the output stream. Any other side effects from a function are largely invisible until you discover exactly what it changed. This can create a bit of a debugging nightmare if you have multiple functions changing the same value; avoid it as much as you can, you'll tend to create headaches for yourself a lot otherwise.
    • In this case, it seems the specific scope declarations are not really needed and can simply be removed to avoid accidentally sharing values between functions declared in the same script.
  5. Don't use backticks to continue lines, period. Learn and make good use of PowerShell's natural line continuations instead. Mark Kraus' blog post goes through why this is and what the alternatives are in a number of situations.
  6. Try to be consistent in your style and spacing. Some places are missing spaces, others not. The better your consistency when writing a script, the easier time you'll have reading it again later and debugging it when something goes bad.
  7. You're using `Format-Table` in a function. Don't. `Format-*` commands destroy the data in order to produce pretty-looking text. Once data has been passed through such a command, you can no longer access its property values as you're trying to do.
  8. Be very careful with using `"$Variable"` to pass values along. Not only is it not needed in most cases, you're also automatically converting the value to a string. If you happen to get array input (which you are permitting, since your parameter has `[string[]]` "string array" type) you will get potentially unusable values to pass along to the command you're sending that to.
  9. Don't create script blocks unnecessarily. Script blocks are used to store executable code for later use; they're not really for values. For what you have happening here, chances are the custom object output from Select-Object is sufficient.
  10. Don't share values between functions. It seems harmless now, but it's a habit that will really bite later on as things you write get a bit more involved. Debugging functions that share values is a huge headache. Instead, have one function call the other. Sometimes this means you need to take the same parameters for both functions, so that you can pass them along appropriately.
  11. Get in the habit of always using named blocks for your functions. Typically if a function doesn't name its blocks, it will automatically be assigned to the `end{}` block, but it's always better to be explicit about your intent.
  12. `-SearchQuery` is a parameter for `Search-Mailbox`, not another command to pipe to.
    • Also, that parameter takes a query string, not a scriptblock. Bad habit to get into, although it is (unfortunately) encouraged by some folks. Best to work with the right data type so you can avoid nasty surprises.
  13. This delves into some slightly more advanced stuff, but when working with pipeline-capable functions that accept array parameters, it's usually best to add an inner `foreach() {}` loop over that parameter, in case people specify the value directly as is still permissible.
With all of that in mind (sorry about the wall of text, just trying to make sure I explain the why and not just the how), here's how I'd do those functions, more or less:

https://gist.github.com/vexx32/c162230e5352816fac90df23cc7b6bd6

Also, when in doubt, don’t be afraid to google for the documentation. It can help quite a bit, and there’s a lot of it out there with plenty of useful examples.