From 46dd19e8ebce81ca6e9b71f22bae7986c24d014a Mon Sep 17 00:00:00 2001 From: Shenglong Li Date: Thu, 12 May 2022 14:26:37 -0700 Subject: [PATCH] Avoid serializing and deserializing template specs (#6833) --- src/Bicep.Core.Samples/DataSetsExtensions.cs | 6 +- .../NoHardcodedEnvironmentUrlsRuleTests.cs | 1 - .../Registry/TemplateSpecRepositoryTests.cs | 96 ++++---- .../Registry/ExternalModuleRegistry.cs | 1 - .../Registry/ITemplateSpecVersionProvider.cs | 14 -- src/Bicep.Core/Registry/TemplateSpecEntity.cs | 229 ------------------ .../Registry/TemplateSpecModuleRegistry.cs | 5 +- .../Registry/TemplateSpecRepository.cs | 11 +- .../Registry/TemplateSpecRepositoryFactory.cs | 2 +- .../Registry/TemplateSpecVersionProvider.cs | 17 -- .../Namespaces/SystemNamespaceType.cs | 1 - 11 files changed, 61 insertions(+), 322 deletions(-) delete mode 100644 src/Bicep.Core/Registry/ITemplateSpecVersionProvider.cs delete mode 100644 src/Bicep.Core/Registry/TemplateSpecEntity.cs delete mode 100644 src/Bicep.Core/Registry/TemplateSpecVersionProvider.cs diff --git a/src/Bicep.Core.Samples/DataSetsExtensions.cs b/src/Bicep.Core.Samples/DataSetsExtensions.cs index 8c8096d84d0..c2f929d0a6b 100644 --- a/src/Bicep.Core.Samples/DataSetsExtensions.cs +++ b/src/Bicep.Core.Samples/DataSetsExtensions.cs @@ -10,7 +10,6 @@ using Bicep.Core.Analyzers.Linter; using Bicep.Core.Configuration; using Bicep.Core.FileSystem; -using Bicep.Core.Json; using Bicep.Core.Modules; using Bicep.Core.Registry; using Bicep.Core.Semantics; @@ -112,13 +111,10 @@ public static ITemplateSpecRepositoryFactory CreateMockTemplateSpecRepositoryFac throw new InvalidOperationException($"Module '{moduleName}' has an invalid target reference '{templateSpecInfo.Metadata.Target}'. Specify a reference to a template spec."); } - var templateSpecElement = JsonElementFactory.CreateElement(templateSpecInfo.ModuleSource); - var templateSpecEntity = TemplateSpecEntity.FromJsonElement(templateSpecElement); - repositoryMocksBySubscription.TryAdd(reference.SubscriptionId, StrictMock.Of()); repositoryMocksBySubscription[reference.SubscriptionId] .Setup(x => x.FindTemplateSpecByIdAsync(reference.TemplateSpecResourceId, It.IsAny())) - .ReturnsAsync(templateSpecEntity); + .ReturnsAsync(new TemplateSpecEntity(templateSpecInfo.ModuleSource)); } var repositoryFactoryMock = StrictMock.Of(); diff --git a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/NoHardcodedEnvironmentUrlsRuleTests.cs b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/NoHardcodedEnvironmentUrlsRuleTests.cs index 1e75d502cbb..fea023e2fac 100644 --- a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/NoHardcodedEnvironmentUrlsRuleTests.cs +++ b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/NoHardcodedEnvironmentUrlsRuleTests.cs @@ -2,7 +2,6 @@ // Licensed under the MIT License. using System.Linq; -using Azure.Deployments.Core.Extensions; using Bicep.Core.Analyzers.Linter.Rules; using Microsoft.VisualStudio.TestTools.UnitTesting; diff --git a/src/Bicep.Core.UnitTests/Registry/TemplateSpecRepositoryTests.cs b/src/Bicep.Core.UnitTests/Registry/TemplateSpecRepositoryTests.cs index 955aa534f49..2ac87469e95 100644 --- a/src/Bicep.Core.UnitTests/Registry/TemplateSpecRepositoryTests.cs +++ b/src/Bicep.Core.UnitTests/Registry/TemplateSpecRepositoryTests.cs @@ -2,10 +2,10 @@ // Licensed under the MIT License. using System; +using System.Text.Json; using System.Threading; using System.Threading.Tasks; using Azure; -using Azure.Core; using Azure.ResourceManager; using Azure.ResourceManager.Resources; using Bicep.Core.Registry; @@ -25,14 +25,14 @@ public class TemplateSpecRepositoryTests [TestMethod] public async Task FindTemplateSpecByIdAsync_TemplateSpecNotFound_ThrowsTemplateSpecException() { - var templateSpecVersionMock = CreateMockTemplateSpecVersion(templateSpecVersionMock => templateSpecVersionMock - .Setup(x => x.GetAsync(It.IsAny())) - .ThrowsAsync(new RequestFailedException(404, "Not found."))); + var templateSpecVersionResourceMock = CreateMockTemplateSpecVersionResource( + mock => mock + .Setup(x => x.GetAsync(It.IsAny())) + .ThrowsAsync(new RequestFailedException(404, "Not found."))); - var clientMock = CreateMockClient(); - var templateSpecVersionProviderMock = CreateMockTemplateSpecVersionProvider(clientMock, templateSpecVersionMock); + var clientMock = CreateMockClient(templateSpecVersionResourceMock); - var repository = new TemplateSpecRepository(clientMock, templateSpecVersionProviderMock); + var repository = new TemplateSpecRepository(clientMock); await Invoking(async () => await repository.FindTemplateSpecByIdAsync(TestTemplateSpecId)) .Should() @@ -43,14 +43,13 @@ await Invoking(async () => await repository.FindTemplateSpecByIdAsync(TestTempla [TestMethod] public async Task FindTemplateSpecByIdAsync_GotUnexpectedRequestFailedException_ConvertsToTemplateSpecException() { - var templateSpecVersionMock = CreateMockTemplateSpecVersion(templateSpecVersionMock => templateSpecVersionMock - .Setup(x => x.GetAsync(It.IsAny())) - .ThrowsAsync(new RequestFailedException("Unexpected error."))); + var templateSpecVersionResourceMock = CreateMockTemplateSpecVersionResource(mock => + mock.Setup(x => x.GetAsync(It.IsAny())) + .ThrowsAsync(new RequestFailedException("Unexpected error."))); - var clientMock = CreateMockClient(); - var templateSpecVersionProviderMock = CreateMockTemplateSpecVersionProvider(clientMock, templateSpecVersionMock); + var clientMock = CreateMockClient(templateSpecVersionResourceMock); - var repository = new TemplateSpecRepository(clientMock, templateSpecVersionProviderMock); + var repository = new TemplateSpecRepository(clientMock); await Invoking(async () => await repository.FindTemplateSpecByIdAsync(TestTemplateSpecId)) .Should() @@ -61,42 +60,42 @@ await Invoking(async () => await repository.FindTemplateSpecByIdAsync(TestTempla [TestMethod] public async Task FindTemplateSpecByIdAsync_TemlateSpecFound_ReturnsTemplateSpec() { - var data = new TemplateSpecVersionData("westus") - { - MainTemplate = new BinaryData("{}") - }; - - var templateSpecVersionMock = CreateMockTemplateSpecVersion( - templateSpecVersionMock => templateSpecVersionMock - .SetupGet(x => x.Data) - .Returns(data), - templateSpecVersionMock => templateSpecVersionMock + var data = new TemplateSpecVersionData("westus"); + var content = JsonSerializer.Serialize(data, new JsonSerializerOptions { WriteIndented = true }); + + var templateSpecVersionResourceMock = CreateMockTemplateSpecVersionResource( + mock => mock .Setup(x => x.GetAsync(It.IsAny())) - .ReturnsAsync(CreateMockResponse(templateSpecVersionMock.Object))); + .ReturnsAsync(CreateMockResponse(content))); - var clientMock = CreateMockClient(); - var templateSpecVersionProviderMock = CreateMockTemplateSpecVersionProvider(clientMock, templateSpecVersionMock); + var clientMock = CreateMockClient(templateSpecVersionResourceMock); - var repository = new TemplateSpecRepository(clientMock, templateSpecVersionProviderMock); + var repository = new TemplateSpecRepository(clientMock); var templateSpec = await repository.FindTemplateSpecByIdAsync(TestTemplateSpecId); - templateSpec.MainTemplate.GetRawText().Should().Be("{}"); + templateSpec.Content.ReplaceLineEndings().Should().Be(@"{ + ""Location"": { + ""Name"": ""westus"", + ""DisplayName"": ""West US"" + }, + ""Tags"": {}, + ""Description"": null, + ""LinkedTemplates"": [], + ""Metadata"": null, + ""MainTemplate"": null, + ""UiFormDefinition"": null, + ""Id"": null, + ""Name"": null, + ""ResourceType"": { + ""Namespace"": null, + ""Type"": null + }, + ""SystemData"": null +}".ReplaceLineEndings()); } - private ITemplateSpecVersionProvider CreateMockTemplateSpecVersionProvider( - ArmClient armClient, - TemplateSpecVersionResource templateSpecVersion) - { - var templateSpecVersionProvider = StrictMock.Of(); - templateSpecVersionProvider - .Setup(x => x.GetTemplateSpecVersion(armClient, It.IsAny())) - .Returns(templateSpecVersion); - - return templateSpecVersionProvider.Object; - } - - private static TemplateSpecVersionResource CreateMockTemplateSpecVersion(params Action>[] setUpTemplateSpecVersionMockActions) + private static TemplateSpecVersionResource CreateMockTemplateSpecVersionResource(params Action>[] setUpTemplateSpecVersionMockActions) { var templateSpecVersionMock = StrictMock.Of(); @@ -108,18 +107,23 @@ private static TemplateSpecVersionResource CreateMockTemplateSpecVersion(params return templateSpecVersionMock.Object; } - private static ArmClient CreateMockClient() + private static ArmClient CreateMockClient(TemplateSpecVersionResource resource) { var clientMock = StrictMock.Of(); + clientMock.Setup(x => x.GetResourceClient(It.IsAny>())) + .Returns(resource); + return clientMock.Object; } - private static Response CreateMockResponse(T value) + private static Response CreateMockResponse(string content) { - var responseMock = StrictMock.Of>(); - responseMock.SetupGet(m => m.Value).Returns(value); - responseMock.Setup(m => m.GetRawResponse()).Returns(StrictMock.Of().Object); + var rawResponseMock = StrictMock.Of(); + rawResponseMock.SetupGet(x => x.Content).Returns(BinaryData.FromString(content)); + + var responseMock = StrictMock.Of>(); + responseMock.Setup(m => m.GetRawResponse()).Returns(rawResponseMock.Object); return responseMock.Object; } diff --git a/src/Bicep.Core/Registry/ExternalModuleRegistry.cs b/src/Bicep.Core/Registry/ExternalModuleRegistry.cs index 42fb9ae2951..02223944ae1 100644 --- a/src/Bicep.Core/Registry/ExternalModuleRegistry.cs +++ b/src/Bicep.Core/Registry/ExternalModuleRegistry.cs @@ -16,7 +16,6 @@ namespace Bicep.Core.Registry { public abstract class ExternalModuleRegistry : ModuleRegistry where TModuleReference : ModuleReference - where TModuleEntity : class { // if we're unable to acquire a lock on the module directory in the cache, we will retry until this timeout is reached private static readonly TimeSpan ModuleDirectoryContentionTimeout = TimeSpan.FromSeconds(5); diff --git a/src/Bicep.Core/Registry/ITemplateSpecVersionProvider.cs b/src/Bicep.Core/Registry/ITemplateSpecVersionProvider.cs deleted file mode 100644 index 00c8cebc73d..00000000000 --- a/src/Bicep.Core/Registry/ITemplateSpecVersionProvider.cs +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using Azure.Core; -using Azure.ResourceManager; -using Azure.ResourceManager.Resources; - -namespace Bicep.Core.Registry -{ - public interface ITemplateSpecVersionProvider - { - TemplateSpecVersionResource GetTemplateSpecVersion(ArmClient armClient, ResourceIdentifier resourceIdentifier); - } -} diff --git a/src/Bicep.Core/Registry/TemplateSpecEntity.cs b/src/Bicep.Core/Registry/TemplateSpecEntity.cs deleted file mode 100644 index ac1bb719f0e..00000000000 --- a/src/Bicep.Core/Registry/TemplateSpecEntity.cs +++ /dev/null @@ -1,229 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using System.Buffers; -using System.Text; -using System.Text.Encodings.Web; -using System.Text.Json; -using Azure.Core; -using Azure.ResourceManager.Resources; -using Bicep.Core.Extensions; -using Bicep.Core.Json; - -namespace Bicep.Core.Registry -{ - public class TemplateSpecEntity - { - public TemplateSpecEntity(ResourceIdentifier id, string name, ResourceType type, JsonElement systemData, string location, JsonElement? tags, string? description, JsonElement? linkedTemplates, JsonElement? metadata, JsonElement mainTemplate, JsonElement? uiFormDefinition) - { - this.Id = id; - this.Name = name; - this.Type = type; - this.SystemData = systemData; - this.Location = location; - this.Tags = tags; - this.Description = description; - this.LinkedTemplates = linkedTemplates; - this.Metadata = metadata; - this.MainTemplate = mainTemplate; - this.UiFormDefinition = uiFormDefinition; - } - - public ResourceIdentifier Id { get; } - - public string Name { get; } - - public ResourceType Type { get; } - - public JsonElement SystemData { get; } - - public string Location { get; } - - public JsonElement? Tags { get; } - - public string? Description { get; } - - public JsonElement? LinkedTemplates { get; } - - public JsonElement? Metadata { get; } - - public JsonElement MainTemplate { get; } - - public JsonElement? UiFormDefinition { get; } - - public static TemplateSpecEntity FromSdkModel(TemplateSpecVersionData model) => new( - model.Id, - model.Name, - model.ResourceType, - JsonElementFactory.CreateElement(model.SystemData), - model.Location, - JsonElementFactory.CreateNullableElement(model.Tags), - model.Description, - JsonElementFactory.CreateNullableElement(model.LinkedTemplates), - JsonElementFactory.CreateNullableElement(model.Metadata), - JsonElementFactory.CreateElement(model.MainTemplate), - JsonElementFactory.CreateNullableElement(model.UiFormDefinition)); - - public static TemplateSpecEntity FromJsonElement(JsonElement element) - { - string id = ""; - string name = ""; - string type = ""; - string location = ""; - JsonElement? tags = default; - JsonElement systemData = default; - - // properties.* - string? description = default; - JsonElement? linkedTemplates = default; - JsonElement? metadata = default; - JsonElement mainTemplate = default; - JsonElement? uiFormDefinition = default; - - foreach (var topLevelProperty in element.EnumerateObject()) - { - switch (topLevelProperty.Name) - { - case "id": - id = topLevelProperty.Value.ToNonNullString(); - break; - - case "name": - name = topLevelProperty.Value.ToNonNullString(); - break; - - case "type": - type = topLevelProperty.Value.ToNonNullString(); - break; - - case "location": - location = topLevelProperty.Value.ToNonNullString(); - break; - - case "tags" when topLevelProperty.Value.IsNotNullValue(): - tags = topLevelProperty.Value; - break; - - case "systemData": - systemData = topLevelProperty.Value; - break; - - case "properties": - foreach (var property in topLevelProperty.Value.EnumerateObject()) - { - switch (property.Name) - { - case "description": - description = property.Value.GetString(); - break; - - case "linkedTemplates" when property.Value.IsNotNullValue(): - linkedTemplates = property.Value; - break; - - case "metadata" when property.Value.IsNotNullValue(): - metadata = property.Value; - break; - - case "mainTemplate": - mainTemplate = property.Value; - break; - - case "uiFormDefinition" when property.Value.IsNotNullValue(): - uiFormDefinition = property.Value; - break; - - default: - break; - } - } - - break; - - default: - break; - } - } - var resourceIdentifier = new ResourceIdentifier(id); - return new(resourceIdentifier, name, type, systemData, location, tags, description, linkedTemplates, metadata, mainTemplate, uiFormDefinition); - } - - public string ToUtf8Json() - { - var bufferWriter = new ArrayBufferWriter(); - var writterOptions = new JsonWriterOptions - { - // Using UnsafeRelaxedJsonEscaping to avoid escaping HTML-sensitive characters such as <, >, &, and '. - // It is safe to use it because we are dealing with responses from the Azure SDK, - // and the output will not be emitted into an HTML page or a