Get-MofSchemaObject: Rewrite function to use DscClassCache ImportClasses

[Daniël (danielboth)]

Pull Request (PR) description

This is a rewrite of the Get-MofSchemaObject function. Instead of parsing text this uses the ImportClasses method of [Microsoft.PowerShell.DesiredStateConfiguration.Internal.DscClassCache] to parse the schema.mof. The output is a 1-1 copy (as close as possible) of what the text based parsing used to produce.

This Pull Request (PR) fixes the following issues

None

Task list

• Added an entry under the Unreleased section of the change log in the CHANGELOG.md.
  Entry should say what was changed, and how that affects users (if applicable).
• Documentation added/updated in README.md.
• Comment-based help added/updated for all new/changed functions.
• Localization strings added/updated in all localization files as appropriate.
• Unit tests added/updated.
• New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

---

This change is 

[Codecov (codecov-io)]

Codecov Report

Merging #354 into dev will increase coverage by 4%.
The diff coverage is 96%.

@@         Coverage Diff         @@
##            dev   #354   +/-   ##
===================================
+ Coverage    80%    84%   +4%     
===================================
  Files        12     12           
  Lines      1806   1749   -57     
  Branches      2      2           
===================================
+ Hits       1448   1476   +28     
+ Misses      356    271   -85     
  Partials      2      2

[Daniël (danielboth)]

Have been writing some tests as well, here the updated function differs from the current one, the current one would fail like this:

 [-] Should get property MultipleValues with all correct properties 12ms
   Expected strings to be the same, but they were different.
   Expected length: 11
   Actual length:   6
   Strings differ at index 6.
   Expected: 'StringArray'
   But was:  'String'
   86:             $property.DataType | Should -Be $DataType

 [-] Should get property Credential with all correct properties 7ms
   Expected strings to be the same, but they were different.
   Expected length: 8
   Actual length:   6
   Strings differ at index 0.
   Expected: 'Instance'
   But was:  'String'
   86:             $property.DataType | Should -Be $DataType

[Daniel Scott-Raynsford (PlagueHO)]

Reviewed 1 of 2 files at r1, 2 of 2 files at r2.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on Daniël (@danielboth))

---

DscResource.DocumentationHelper/MofHelper.psm1, line 38 at r2 (raw file):

        $filePath = (Resolve-Path -Path $FileName).Path
        $tempFilePath = Join-Path -Path $env:TEMP -ChildPath "$((New-Guid).Guid).tmp"

Nitpick, non-blocking: Could be worth prefixing the filename with something to indicate it belongs to this process (E.g. DscMofHelper_Guid)

---

DscResource.DocumentationHelper/MofHelper.psm1, line 43 at r2 (raw file):

        Set-Content -LiteralPath $tempFilePath -Value $rawContent -ErrorAction Stop

        #.NET methods don't like PowerShell drives

Can you add space after #?

---

DscResource.DocumentationHelper/MofHelper.psm1, line 61 at r2 (raw file):

    finally
    {
        Remove-Item -LiteralPath $tempFilePath -Force

Should assign this to $null because Remove-Item returns objects.

---

DscResource.DocumentationHelper/MofHelper.psm1, line 68 at r2 (raw file):

        $state = switch ($property.flags)
        {
            {$_ -band [Microsoft.Management.Infrastructure.CimFlags]::Key}      {'Key'}

Nitpick, non-blocking: This doesn't align to style guide, but I don't think we've got a style example that quite aligns here and my thoughts are that this is pretty readable this way. Johan Ljunggren (@johlju)? Do we try and get this to align to style guide or leave it?

---

Tests/Unit/MofHelper.Tests.ps1, line 8 at r2 (raw file):

InModuleScope -ModuleName 'MofHelper' {

Can you remove blank line?

[Johan Ljunggren (johlju)]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on Daniël (@danielboth))

---

DscResource.DocumentationHelper/MofHelper.psm1, line 68 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick, non-blocking: This doesn't align to style guide, but I don't think we've got a style example that quite aligns here and my thoughts are that this is pretty readable this way. Johan Ljunggren (@johlju)? Do we try and get this to align to style guide or leave it?

I have written such code like any other switch statement. If this was a "normal" switch statement it would be written like this

$state = switch
{
    'hello'
    {
         'key'
    }
}

So the code would be written

        $state = switch ($property.flags)
        {
            {$_ -band [Microsoft.Management.Infrastructure.CimFlags]::Key}      
            {
                'Key'
            }
           
            {$_ -band [Microsoft.Management.Infrastructure.CimFlags]::Required}
            {
                'Required'
            }
            
            {$_ -band [Microsoft.Management.Infrastructure.CimFlags]::ReadOnly}
            {
                'Read'
            }
            
            default                                                             
            {
                'Write'
            }
        }

I don't mind the way it looks now, but if we add a custom rule to this I think the rule would be written to enforce the braces like written above, because we can't write to complex rules. 🤔

But we can resolve that when that days comes. So Daniël (@danielboth) suggest to leave as-is for now. 🙂

[Johan Ljunggren (johlju)]

Daniel Scott-Raynsford (@PlagueHO) Remember we can't merge this just yet. gaelcolas (@gaelcolas) does not want any changes to DscResource.Tests just now since he is in the process of changing the pipeline. 🙂

gaelcolas (@gaelcolas) do you want/need this change implemented (merged) "now" to help you, or should we hold off until you are done?

[Daniël (danielboth)]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on Daniel Scott-Raynsford (@PlagueHO))

---

DscResource.DocumentationHelper/MofHelper.psm1, line 43 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you add space after #?

Done.

---

DscResource.DocumentationHelper/MofHelper.psm1, line 61 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Should assign this to $null because Remove-Item returns objects.

That's our friend New-Item, Remove-Item does not return objects.

---

DscResource.DocumentationHelper/MofHelper.psm1, line 68 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

I have written such code like any other switch statement. If this was a "normal" switch statement it would be written like this

$state = switch
{
    'hello'
    {
         'key'
    }
}

So the code would be written

        $state = switch ($property.flags)
        {
            {$_ -band [Microsoft.Management.Infrastructure.CimFlags]::Key}      
            {
                'Key'
            }
           
            {$_ -band [Microsoft.Management.Infrastructure.CimFlags]::Required}
            {
                'Required'
            }
            
            {$_ -band [Microsoft.Management.Infrastructure.CimFlags]::ReadOnly}
            {
                'Read'
            }
            
            default                                                             
            {
                'Write'
            }
        }

I don't mind the way it looks now, but if we add a custom rule to this I think the rule would be written to enforce the braces like written above, because we can't write to complex rules. 🤔

But we can resolve that when that days comes. So Daniël (@danielboth) suggest to leave as-is for now. 🙂

Ok, will leave this for now.

---

Tests/Unit/MofHelper.Tests.ps1, line 8 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you remove blank line?

Done.

[Johan Ljunggren (johlju)]

Looks like Daniel Scott-Raynsford (@PlagueHO) already talked to gaelcolas (@gaelcolas) about this in Slack so it sounded like it can be merged when the review is done.

[gaelcolas (gaelcolas)]

That's fine, I'm more on the pssa rules side right now

[Daniel Scott-Raynsford (PlagueHO)]

Reviewed 2 of 2 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved

[Daniel Scott-Raynsford (PlagueHO)]

Reviewable status: complete! all files reviewed, all discussions resolved

[Daniel Scott-Raynsford (PlagueHO)]

Reviewed 1 of 1 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved