diff --git a/Actions/.Modules/ReadSettings.psm1 b/Actions/.Modules/ReadSettings.psm1 index ea3bbe350..354f9af9b 100644 --- a/Actions/.Modules/ReadSettings.psm1 +++ b/Actions/.Modules/ReadSettings.psm1 @@ -16,7 +16,11 @@ $CustomTemplateProjectSettingsFile = Join-Path '.github' $CustomTemplateProjectS function MergeCustomObjectIntoOrderedDictionary { Param( [System.Collections.Specialized.OrderedDictionary] $dst, - [PSCustomObject] $src + [PSCustomObject] $src, + # Settings already explicitly overwritten by a prior user source. Template sources must not re-apply these. + [string[]] $overwrittenSettings = @(), + # True when the source file is a template-managed file (*.doNotEdit.json). Used together with $overwrittenSettings. + [bool] $isTemplateSource = $false ) # If the src object contains property 'overwriteSettings' (list of settings), remove these settings from the dst object, so that they can be re-added with the new value later on @@ -42,6 +46,11 @@ function MergeCustomObjectIntoOrderedDictionary { return } + # Template sources must not re-introduce a property that was explicitly overwritten by a user source + if ($isTemplateSource -and $overwrittenSettings -contains $prop) { + return + } + $srcProp = $src."$prop" $srcPropType = $srcProp.GetType().Name if (-not $dst.Contains($prop)) { @@ -67,6 +76,11 @@ function MergeCustomObjectIntoOrderedDictionary { @($dst.Keys) | ForEach-Object { $prop = $_ if ($src.PSObject.Properties.Name -eq $prop) { + # Template sources must not overwrite a setting that was explicitly overwritten by a user source + if ($isTemplateSource -and $overwrittenSettings -contains $prop) { + OutputDebug "Skipping template merge of explicitly overwritten setting $prop" + return + } $dstProp = $dst."$prop" $srcProp = $src."$prop" $dstPropType = $dstProp.GetType().Name @@ -479,11 +493,30 @@ function ReadSettings { } } + # Tracks settings names that were explicitly overwritten by a user-controlled source. + # Template sources (*.doNotEdit.json) that appear later in the merge order must not re-apply these. + $overwrittenSettings = @() + foreach($settingsObject in $settingsObjects) { $settingsJson = $settingsObject.Settings if ($settingsJson) { + $isTemplateSource = $settingsObject.Source -like "*.doNotEdit.json" OutputDebug "Applying settings from $($settingsObject.Source) ($($settingsObject.Type))" - MergeCustomObjectIntoOrderedDictionary -dst $settings -src $settingsJson + MergeCustomObjectIntoOrderedDictionary -dst $settings -src $settingsJson -overwrittenSettings $overwrittenSettings -isTemplateSource $isTemplateSource + + # After a non-template source is merged, record which settings it declared as overwritten. + # Subsequent template sources must not re-apply those settings. + # A setting is only considered overwritten when the source both declares it in overwriteSettings + # and also provides the property value. This mirrors the guard in MergeCustomObjectIntoOrderedDictionary + # that only removes a destination setting when the source contains the property as well. + if (-not $isTemplateSource -and $settingsJson.PSObject.Properties.Name -contains "overwriteSettings") { + $settingsJson.overwriteSettings | ForEach-Object { + if ($_ -notin $overwrittenSettings -and $settingsJson.PSObject.Properties.Name -contains $_) { + $overwrittenSettings += $_ + } + } + } + if ($settingsJson.PSObject.Properties.Name -eq "ConditionalSettings") { foreach($conditionalSetting in $settingsJson.ConditionalSettings) { if ("$conditionalSetting" -ne "") { @@ -505,7 +538,7 @@ function ReadSettings { } if ($conditionMet) { OutputDebug "Applying conditional settings for $($conditions -join ", ")" - MergeCustomObjectIntoOrderedDictionary -dst $settings -src $conditionalSetting.settings + MergeCustomObjectIntoOrderedDictionary -dst $settings -src $conditionalSetting.settings -overwrittenSettings $overwrittenSettings -isTemplateSource $isTemplateSource } } } diff --git a/Tests/ReadSettings.Test.ps1 b/Tests/ReadSettings.Test.ps1 index 24fdb69c9..4294c29e7 100644 --- a/Tests/ReadSettings.Test.ps1 +++ b/Tests/ReadSettings.Test.ps1 @@ -460,7 +460,130 @@ InModuleScope ReadSettings { # Allows testing of private functions $dst.PSObject.Properties.Name | Should -Not -Contain 'overwriteSettings' } - It 'Multiple conditionalSettings with same array setting are merged (all entries kept)' { + It 'overwriteSettings in a user source prevents subsequent template sources from re-merging that setting' { + Mock Write-Host { } + Mock Out-Host { } + + Push-Location + $tempName = Join-Path ([System.IO.Path]::GetTempPath()) ([Guid]::NewGuid().ToString()) + $githubFolder = Join-Path $tempName ".github" + $projectALGoFolder = Join-Path $tempName "Project/$ALGoFolderName" + + New-Item $githubFolder -ItemType Directory | Out-Null + New-Item $projectALGoFolder -ItemType Directory | Out-Null + + try { + # Template repo settings add probing paths + @{ "appDependencyProbingPaths" = @( @{ "repo" = "template-repo"; "version" = "latest" } ) } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder $CustomTemplateRepoSettingsFileName) -Encoding utf8 -Force + + # User repo settings explicitly overwrite appDependencyProbingPaths to empty + @{ "overwriteSettings" = @("appDependencyProbingPaths"); "appDependencyProbingPaths" = @() } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder "AL-Go-Settings.json") -Encoding utf8 -Force + + # Template project settings also have probing paths (this is the source of the bug) + @{ "appDependencyProbingPaths" = @( @{ "repo" = "template-project-repo"; "version" = "latest" } ) } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder $CustomTemplateProjectSettingsFileName) -Encoding utf8 -Force + + $ENV:ALGoOrgSettings = '' + $ENV:ALGoRepoSettings = '' + + $settings = ReadSettings -baseFolder $tempName -project 'Project' -repoName 'repo' -workflowName '' -branchName '' -userName '' + + # appDependencyProbingPaths must remain empty: the user's overwriteSettings must win over both template sources + $settings.appDependencyProbingPaths | Should -BeNullOrEmpty + } + finally { + Pop-Location + Remove-Item $tempName -Recurse -Force + } + } + + It 'overwriteSettings in a user source still allows subsequent user sources to contribute to that setting' { + Mock Write-Host { } + Mock Out-Host { } + + Push-Location + $tempName = Join-Path ([System.IO.Path]::GetTempPath()) ([Guid]::NewGuid().ToString()) + $githubFolder = Join-Path $tempName ".github" + $projectALGoFolder = Join-Path $tempName "Project/$ALGoFolderName" + + New-Item $githubFolder -ItemType Directory | Out-Null + New-Item $projectALGoFolder -ItemType Directory | Out-Null + + try { + # Template repo settings add probing paths + @{ "appDependencyProbingPaths" = @( @{ "repo" = "template-repo"; "version" = "latest" } ) } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder $CustomTemplateRepoSettingsFileName) -Encoding utf8 -Force + + # User repo settings explicitly overwrite appDependencyProbingPaths to a specific set + @{ "overwriteSettings" = @("appDependencyProbingPaths"); "appDependencyProbingPaths" = @( @{ "repo" = "user-repo"; "version" = "1.0" } ) } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder "AL-Go-Settings.json") -Encoding utf8 -Force + + # Template project settings also have probing paths (must be ignored) + @{ "appDependencyProbingPaths" = @( @{ "repo" = "template-project-repo"; "version" = "latest" } ) } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder $CustomTemplateProjectSettingsFileName) -Encoding utf8 -Force + + # User project settings add one more probing path (must be merged on top of the user repo overwrite) + @{ "appDependencyProbingPaths" = @( @{ "repo" = "project-repo"; "version" = "2.0" } ) } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $projectALGoFolder "settings.json") -Encoding utf8 -Force + + $ENV:ALGoOrgSettings = '' + $ENV:ALGoRepoSettings = '' + + $settings = ReadSettings -baseFolder $tempName -project 'Project' -repoName 'repo' -workflowName '' -branchName '' -userName '' + + # Only user-repo and project-repo entries must be present; template entries must be absent + $settings.appDependencyProbingPaths.Count | Should -Be 2 + $settings.appDependencyProbingPaths.repo | Should -Contain 'user-repo' + $settings.appDependencyProbingPaths.repo | Should -Contain 'project-repo' + $settings.appDependencyProbingPaths.repo | Should -Not -Contain 'template-repo' + $settings.appDependencyProbingPaths.repo | Should -Not -Contain 'template-project-repo' + } + finally { + Pop-Location + Remove-Item $tempName -Recurse -Force + } + } + + It 'overwriteSettings in a user source without the property value does not block template sources from merging that setting' { + Mock Write-Host { } + Mock Out-Host { } + + Push-Location + $tempName = Join-Path ([System.IO.Path]::GetTempPath()) ([Guid]::NewGuid().ToString()) + $githubFolder = Join-Path $tempName ".github" + $projectALGoFolder = Join-Path $tempName "Project/$ALGoFolderName" + + New-Item $githubFolder -ItemType Directory | Out-Null + New-Item $projectALGoFolder -ItemType Directory | Out-Null + + try { + # Template repo settings add probing paths + @{ "appDependencyProbingPaths" = @( @{ "repo" = "template-repo"; "version" = "latest" } ) } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder $CustomTemplateRepoSettingsFileName) -Encoding utf8 -Force + + # User repo settings declare overwriteSettings for appDependencyProbingPaths but do NOT provide the property value. + # This must not block template sources from contributing appDependencyProbingPaths. + @{ "overwriteSettings" = @("appDependencyProbingPaths") } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder "AL-Go-Settings.json") -Encoding utf8 -Force + + $ENV:ALGoOrgSettings = '' + $ENV:ALGoRepoSettings = '' + + $settings = ReadSettings -baseFolder $tempName -project 'Project' -repoName 'repo' -workflowName '' -branchName '' -userName '' + + # appDependencyProbingPaths must contain the template entry because the user source did not provide a value + $settings.appDependencyProbingPaths | Should -Not -BeNullOrEmpty + $settings.appDependencyProbingPaths.repo | Should -Contain 'template-repo' + } + finally { + Pop-Location + Remove-Item $tempName -Recurse -Force + } + } + + Mock Write-Host { } Mock Out-Host { }