From 97959b83e7901e7718ac9a41920e9fcd6643bafd Mon Sep 17 00:00:00 2001 From: Hisham Bin Ateya Date: Fri, 11 Oct 2024 10:00:19 +0300 Subject: [PATCH] Don't use JObject.Merge in OpenId module (#16865) --- .../OpenIdServerDeploymentSource.cs | 13 +-- .../OpenIdValidationDeploymentSource.cs | 12 +- .../OpenIdServerDeploymentSourceTests.cs | 103 +++++++----------- 3 files changed, 47 insertions(+), 81 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Deployment/OpenIdServerDeploymentSource.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Deployment/OpenIdServerDeploymentSource.cs index 9df97d11aef..db7f2374f0b 100644 --- a/src/OrchardCore.Modules/OrchardCore.OpenId/Deployment/OpenIdServerDeploymentSource.cs +++ b/src/OrchardCore.Modules/OrchardCore.OpenId/Deployment/OpenIdServerDeploymentSource.cs @@ -55,15 +55,10 @@ protected override async Task ProcessAsync(OpenIdServerDeploymentStep step, Depl RequireProofKeyForCodeExchange = settings.RequireProofKeyForCodeExchange, }; - // Use nameof(OpenIdServerSettings) as name, - // to match the recipe step. - var obj = new JsonObject + result.Steps.Add(new JsonObject { - ["name"] = nameof(OpenIdServerSettings), - }; - - obj.Merge(JObject.FromObject(settingsModel)); - - result.Steps.Add(obj); + ["name"] = "OpenIdServerSettings", + ["OpenIdServerSettings"] = JObject.FromObject(settingsModel), + }); } } diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Deployment/OpenIdValidationDeploymentSource.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Deployment/OpenIdValidationDeploymentSource.cs index d6f341fad47..f0a782f5a10 100644 --- a/src/OrchardCore.Modules/OrchardCore.OpenId/Deployment/OpenIdValidationDeploymentSource.cs +++ b/src/OrchardCore.Modules/OrchardCore.OpenId/Deployment/OpenIdValidationDeploymentSource.cs @@ -19,12 +19,10 @@ protected override async Task ProcessAsync(OpenIdValidationDeploymentStep step, { var validationSettings = await _openIdValidationService.GetSettingsAsync(); - // The 'name' property should match the related recipe step name. - var jObject = new JsonObject { ["name"] = nameof(OpenIdValidationSettings) }; - - // Merge settings as the recipe step doesn't use a child property. - jObject.Merge(JObject.FromObject(validationSettings)); - - result.Steps.Add(jObject); + result.Steps.Add(new JsonObject + { + ["name"] = "OpenIdValidationSettings", + ["OpenIdValidationSettings"] = JObject.FromObject(validationSettings), + }); } } diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.OpenId/OpenIdServerDeploymentSourceTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.OpenId/OpenIdServerDeploymentSourceTests.cs index a2c12a9309c..e70b3c3749c 100644 --- a/test/OrchardCore.Tests/Modules/OrchardCore.OpenId/OpenIdServerDeploymentSourceTests.cs +++ b/test/OrchardCore.Tests/Modules/OrchardCore.OpenId/OpenIdServerDeploymentSourceTests.cs @@ -1,7 +1,5 @@ -using System.Text.Json; using System.Text.Json.Nodes; using OrchardCore.Deployment; -using OrchardCore.Json.Serialization; using OrchardCore.OpenId.Deployment; using OrchardCore.OpenId.Recipes; using OrchardCore.OpenId.Services; @@ -13,6 +11,36 @@ namespace OrchardCore.Tests.Modules.OrchardCore.OpenId; public class OpenIdServerDeploymentSourceTests { + [Fact] + public async Task ServerDeploymentSourceIsReadableByRecipe() + { + // Arrange + var settings = CreateSettings("https://deploy.localhost", OpenIdServerSettings.TokenFormat.JsonWebToken, true); + var openIdServerService = new Mock(); + openIdServerService + .Setup(m => m.GetSettingsAsync()) + .ReturnsAsync(settings); + + openIdServerService + .Setup(m => m.LoadSettingsAsync()) + .ReturnsAsync(settings); + + var fileBuilder = new MemoryFileBuilder(); + var descriptor = new RecipeDescriptor(); + var result = new DeploymentPlanResult(fileBuilder, descriptor); + var deploymentSource = new OpenIdServerDeploymentSource(openIdServerService.Object); + + // Act + await deploymentSource.ProcessDeploymentStepAsync(new OpenIdServerDeploymentStep(), result); + await result.FinalizeAsync(); + + // Assert + await ExecuteRecipeAsync(openIdServerService.Object, fileBuilder, descriptor); + + var updatedSettings = await openIdServerService.Object.LoadSettingsAsync(); + Assert.Equal(settings, updatedSettings); + } + private static OpenIdServerSettings CreateSettings(string authority, OpenIdServerSettings.TokenFormat tokenFormat, bool initializeAllProperties) { var result = new OpenIdServerSettings @@ -54,78 +82,23 @@ private static OpenIdServerSettings CreateSettings(string authority, OpenIdServe return result; } - private static Mock CreateServerServiceWithSettingsMock(OpenIdServerSettings settings) - { - var serverService = new Mock(); - - serverService - .Setup(m => m.GetSettingsAsync()) - .ReturnsAsync(settings); - - serverService - .Setup(m => m.LoadSettingsAsync()) - .ReturnsAsync(settings); - - return serverService; - } - - [Fact] - public async Task ServerDeploymentSourceIsReadableByRecipe() + private static async Task ExecuteRecipeAsync( + IOpenIdServerService openIdServerService, + MemoryFileBuilder fileBuilder, + RecipeDescriptor recipeDescriptor) { - // Arrange var recipeFile = "Recipe.json"; - - var expectedSettings = CreateSettings("https://deploy.localhost", OpenIdServerSettings.TokenFormat.JsonWebToken, true); - var deployServerServiceMock = CreateServerServiceWithSettingsMock(expectedSettings); - - var actualSettings = CreateSettings("https://recipe.localhost", OpenIdServerSettings.TokenFormat.DataProtection, false); - var recipeServerServiceMock = CreateServerServiceWithSettingsMock(actualSettings); - - var settingsProperties = typeof(OpenIdServerSettings) - .GetProperties(); - - foreach (var property in settingsProperties) - { - Assert.NotEqual( - property.GetValue(expectedSettings), - property.GetValue(actualSettings)); - } - - var fileBuilder = new MemoryFileBuilder(); - var descriptor = new RecipeDescriptor(); - var result = new DeploymentPlanResult(fileBuilder, descriptor); - - var jsonOptions = new JsonSerializerOptions(JOptions.Base); - jsonOptions.Converters.Add(System.Text.Json.Serialization.DynamicJsonConverter.Instance); - jsonOptions.Converters.Add(PathStringJsonConverter.Instance); - - var deploymentSource = new OpenIdServerDeploymentSource(deployServerServiceMock.Object); - - // Act - await deploymentSource.ProcessDeploymentStepAsync(new OpenIdServerDeploymentStep(), result); - await result.FinalizeAsync(); - - var deploy = JsonNode.Parse( - fileBuilder.GetFileContents( - recipeFile, - Encoding.UTF8)); + var content = fileBuilder.GetFileContents(recipeFile, Encoding.UTF8); + var deploy = JsonNode.Parse(content); var recipeContext = new RecipeExecutionContext { - RecipeDescriptor = descriptor, + RecipeDescriptor = recipeDescriptor, Name = deploy["steps"][0].Value("name"), Step = (JsonObject)deploy["steps"][0], }; - var recipeStep = new OpenIdServerSettingsStep(recipeServerServiceMock.Object); + var recipeStep = new OpenIdServerSettingsStep(openIdServerService); await recipeStep.ExecuteAsync(recipeContext); - - // Assert - foreach (var property in settingsProperties) - { - Assert.Equal( - property.GetValue(expectedSettings), - property.GetValue(actualSettings)); - } } }