From 602efc2f36f0aeba17d3251737aeeb07e0f9a8c5 Mon Sep 17 00:00:00 2001 From: Henrique Graca <999396+hjgraca@users.noreply.github.com> Date: Wed, 16 Jul 2025 15:51:16 +0100 Subject: [PATCH 1/5] refactor: replace SystemWrapper with PowertoolsEnvironment for execution context management. Add PTENV --- .../Core/ISystemWrapper.cs | 62 ------ .../Core/PowertoolsEnvironment.cs | 96 +++++++-- .../Core/SystemWrapper.cs | 197 ----------------- .../BedrockAgentFunctionResolver.cs | 2 +- .../AppSyncEvents/AppSyncEventsResolver.cs | 2 +- .../PowertoolsKafkaSerializerBase.cs | 2 +- .../Core/PowertoolsEnvironmentTest.cs | 153 +++++++++---- .../Core/SystemWrapperTests.cs | 204 ------------------ 8 files changed, 184 insertions(+), 534 deletions(-) delete mode 100644 libraries/src/AWS.Lambda.Powertools.Common/Core/ISystemWrapper.cs delete mode 100644 libraries/src/AWS.Lambda.Powertools.Common/Core/SystemWrapper.cs delete mode 100644 libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/SystemWrapperTests.cs diff --git a/libraries/src/AWS.Lambda.Powertools.Common/Core/ISystemWrapper.cs b/libraries/src/AWS.Lambda.Powertools.Common/Core/ISystemWrapper.cs deleted file mode 100644 index a873dcfb..00000000 --- a/libraries/src/AWS.Lambda.Powertools.Common/Core/ISystemWrapper.cs +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://aws.amazon.com/apache2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -using System.IO; - -namespace AWS.Lambda.Powertools.Common; - -/// -/// Interface ISystemWrapper -/// -public interface ISystemWrapper -{ - /// - /// Gets the environment variable. - /// - /// The variable. - /// System.String. - string GetEnvironmentVariable(string variable); - - /// - /// Logs the specified value. - /// - /// The value. - void Log(string value); - - /// - /// Logs the line. - /// - /// The value. - void LogLine(string value); - - /// - /// Gets random number - /// - /// System.Double. - double GetRandom(); - - /// - /// Sets the environment variable. - /// - /// The variable. - /// - void SetEnvironmentVariable(string variable, string value); - - /// - /// Sets the execution Environment Variable (AWS_EXECUTION_ENV) - /// - /// - void SetExecutionEnvironment(T type); -} \ No newline at end of file diff --git a/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs b/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs index 649418a4..79c1a6b4 100644 --- a/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs +++ b/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Concurrent; using System.Text; namespace AWS.Lambda.Powertools.Common; @@ -11,6 +12,16 @@ public class PowertoolsEnvironment : IPowertoolsEnvironment /// private static IPowertoolsEnvironment _instance; + /// + /// Cached runtime environment string + /// + private static readonly string CachedRuntimeEnvironment = $"PTENV/AWS_LAMBDA_DOTNET{Environment.Version.Major}"; + + /// + /// Cache for parsed assembly names to avoid repeated string operations + /// + private static readonly ConcurrentDictionary ParsedAssemblyNameCache = new(); + /// /// Gets the instance. /// @@ -32,13 +43,28 @@ public void SetEnvironmentVariable(string variableName, string value) /// public string GetAssemblyName(T type) { + if (type is Type typeObject) + { + return typeObject.Assembly.GetName().Name; + } + return type.GetType().Assembly.GetName().Name; } /// public string GetAssemblyVersion(T type) { - var version = type.GetType().Assembly.GetName().Version; + Version version; + + if (type is Type typeObject) + { + version = typeObject.Assembly.GetName().Version; + } + else + { + version = type.GetType().Assembly.GetName().Version; + } + return version != null ? $"{version.Major}.{version.Minor}.{version.Build}" : string.Empty; } @@ -46,27 +72,43 @@ public string GetAssemblyVersion(T type) public void SetExecutionEnvironment(T type) { const string envName = Constants.AwsExecutionEnvironmentVariableName; - var envValue = new StringBuilder(); var currentEnvValue = GetEnvironmentVariable(envName); var assemblyName = ParseAssemblyName(GetAssemblyName(type)); - // If there is an existing execution environment variable add the annotations package as a suffix. - if (!string.IsNullOrEmpty(currentEnvValue)) + // Check for duplication early + if (!string.IsNullOrEmpty(currentEnvValue) && currentEnvValue.Contains(assemblyName)) { - // Avoid duplication - should not happen since the calling Instances are Singletons - defensive purposes - if (currentEnvValue.Contains(assemblyName)) - { - return; - } - - envValue.Append($"{currentEnvValue} "); + return; } var assemblyVersion = GetAssemblyVersion(type); + var newEntry = $"{assemblyName}/{assemblyVersion}"; + + string finalValue; + + if (string.IsNullOrEmpty(currentEnvValue)) + { + // First entry: "PT/Assembly/1.0.0 PTENV/AWS_LAMBDA_DOTNET8" + finalValue = $"{newEntry} {CachedRuntimeEnvironment}"; + } + else + { + // Check if PTENV already exists in one pass + var containsPtenv = currentEnvValue.Contains("PTENV/"); + + if (containsPtenv) + { + // Just append the new entry: "existing PT/Assembly/1.0.0" + finalValue = $"{currentEnvValue} {newEntry}"; + } + else + { + // Append new entry + PTENV: "existing PT/Assembly/1.0.0 PTENV/AWS_LAMBDA_DOTNET8" + finalValue = $"{currentEnvValue} {newEntry} {CachedRuntimeEnvironment}"; + } + } - envValue.Append($"{assemblyName}/{assemblyVersion}"); - - SetEnvironmentVariable(envName, envValue.ToString()); + SetEnvironmentVariable(envName, finalValue); } /// @@ -77,16 +119,24 @@ public void SetExecutionEnvironment(T type) /// private string ParseAssemblyName(string assemblyName) { - try + // Use cache to avoid repeated string operations + return ParsedAssemblyNameCache.GetOrAdd(assemblyName, name => { - var parsedName = assemblyName.Substring(assemblyName.LastIndexOf(".", StringComparison.Ordinal) + 1); - return $"{Constants.FeatureContextIdentifier}/{parsedName}"; - } - catch - { - //NOOP - } + try + { + var lastDotIndex = name.LastIndexOf('.'); + if (lastDotIndex >= 0 && lastDotIndex < name.Length - 1) + { + var parsedName = name.Substring(lastDotIndex + 1); + return $"{Constants.FeatureContextIdentifier}/{parsedName}"; + } + } + catch + { + //NOOP + } - return $"{Constants.FeatureContextIdentifier}/{assemblyName}"; + return $"{Constants.FeatureContextIdentifier}/{name}"; + }); } } \ No newline at end of file diff --git a/libraries/src/AWS.Lambda.Powertools.Common/Core/SystemWrapper.cs b/libraries/src/AWS.Lambda.Powertools.Common/Core/SystemWrapper.cs deleted file mode 100644 index faf71eeb..00000000 --- a/libraries/src/AWS.Lambda.Powertools.Common/Core/SystemWrapper.cs +++ /dev/null @@ -1,197 +0,0 @@ -using System; -using System.IO; -using System.Text; - -namespace AWS.Lambda.Powertools.Common; - -/// -/// Class SystemWrapper. -/// Implements the -/// -/// -public class SystemWrapper : ISystemWrapper -{ - private static IPowertoolsEnvironment _powertoolsEnvironment; - private static bool _inTestMode = false; - private static TextWriter _testOutputStream; - private static bool _outputResetPerformed = false; - - /// - /// The instance - /// - private static ISystemWrapper _instance; - - /// - /// Prevents a default instance of the class from being created. - /// - public SystemWrapper(IPowertoolsEnvironment powertoolsEnvironment) - { - _powertoolsEnvironment = powertoolsEnvironment; - _instance ??= this; - - if (!_inTestMode) - { - // Clear AWS SDK Console injected parameters in production only - ResetConsoleOutput(); - } - } - - /// - /// Gets the instance. - /// - /// The instance. - public static ISystemWrapper Instance => _instance ??= new SystemWrapper(PowertoolsEnvironment.Instance); - - /// - /// Gets the environment variable. - /// - /// The variable. - /// System.String. - public string GetEnvironmentVariable(string variable) - { - return _powertoolsEnvironment.GetEnvironmentVariable(variable); - } - - /// - /// Logs the specified value. - /// - /// The value. - public void Log(string value) - { - if (_inTestMode && _testOutputStream != null) - { - _testOutputStream.Write(value); - } - else - { - EnsureConsoleOutputOnce(); - Console.Write(value); - } - } - - /// - /// Logs the line. - /// - /// The value. - public void LogLine(string value) - { - if (_inTestMode && _testOutputStream != null) - { - _testOutputStream.WriteLine(value); - } - else - { - EnsureConsoleOutputOnce(); - Console.WriteLine(value); - } - } - - /// - /// Gets random number - /// - /// System.Double. - public double GetRandom() - { - return new Random().NextDouble(); - } - - /// - public void SetEnvironmentVariable(string variable, string value) - { - _powertoolsEnvironment.SetEnvironmentVariable(variable, value); - } - - /// - public void SetExecutionEnvironment(T type) - { - const string envName = Constants.AwsExecutionEnvironmentVariableName; - var envValue = new StringBuilder(); - var currentEnvValue = GetEnvironmentVariable(envName); - var assemblyName = ParseAssemblyName(_powertoolsEnvironment.GetAssemblyName(type)); - - // If there is an existing execution environment variable add the annotations package as a suffix. - if (!string.IsNullOrEmpty(currentEnvValue)) - { - // Avoid duplication - should not happen since the calling Instances are Singletons - defensive purposes - if (currentEnvValue.Contains(assemblyName)) - { - return; - } - - envValue.Append($"{currentEnvValue} "); - } - - var assemblyVersion = _powertoolsEnvironment.GetAssemblyVersion(type); - - envValue.Append($"{assemblyName}/{assemblyVersion}"); - - SetEnvironmentVariable(envName, envValue.ToString()); - } - - /// - /// Sets console output - /// Useful for testing and checking the console output - /// - /// var consoleOut = new StringWriter(); - /// SystemWrapper.Instance.SetOut(consoleOut); - /// - /// - /// The TextWriter instance where to write to - - public static void SetOut(TextWriter writeTo) - { - _testOutputStream = writeTo; - _inTestMode = true; - Console.SetOut(writeTo); - } - - /// - /// Parsing the name to conform with the required naming convention for the UserAgent header (PTFeature/Name/Version) - /// Fallback to Assembly Name on exception - /// - /// - /// - private string ParseAssemblyName(string assemblyName) - { - try - { - var parsedName = assemblyName.Substring(assemblyName.LastIndexOf(".", StringComparison.Ordinal) + 1); - return $"{Constants.FeatureContextIdentifier}/{parsedName}"; - } - catch - { - //NOOP - } - - return $"{Constants.FeatureContextIdentifier}/{assemblyName}"; - } - - private static void EnsureConsoleOutputOnce() - { - if (_outputResetPerformed) return; - ResetConsoleOutput(); - _outputResetPerformed = true; - } - - private static void ResetConsoleOutput() - { - var standardOutput = new StreamWriter(Console.OpenStandardOutput()); - standardOutput.AutoFlush = true; - Console.SetOut(standardOutput); - var errorOutput = new StreamWriter(Console.OpenStandardError()); - errorOutput.AutoFlush = true; - Console.SetError(errorOutput); - } - - public static void ClearOutputResetFlag() - { - _outputResetPerformed = false; - } - - // For test cleanup - internal static void ResetTestMode() - { - _inTestMode = false; - _testOutputStream = null; - } -} \ No newline at end of file diff --git a/libraries/src/AWS.Lambda.Powertools.EventHandler.Resolvers.BedrockAgentFunction/BedrockAgentFunctionResolver.cs b/libraries/src/AWS.Lambda.Powertools.EventHandler.Resolvers.BedrockAgentFunction/BedrockAgentFunctionResolver.cs index 8952948e..4107a1b9 100644 --- a/libraries/src/AWS.Lambda.Powertools.EventHandler.Resolvers.BedrockAgentFunction/BedrockAgentFunctionResolver.cs +++ b/libraries/src/AWS.Lambda.Powertools.EventHandler.Resolvers.BedrockAgentFunction/BedrockAgentFunctionResolver.cs @@ -40,7 +40,7 @@ private readonly public BedrockAgentFunctionResolver(IJsonTypeInfoResolver? typeResolver = null) { _parameterMapper = new ParameterMapper(typeResolver); - SystemWrapper.Instance.SetExecutionEnvironment(this); + PowertoolsEnvironment.Instance.SetExecutionEnvironment(this); } /// diff --git a/libraries/src/AWS.Lambda.Powertools.EventHandler/AppSyncEvents/AppSyncEventsResolver.cs b/libraries/src/AWS.Lambda.Powertools.EventHandler/AppSyncEvents/AppSyncEventsResolver.cs index afbf077d..09356b64 100644 --- a/libraries/src/AWS.Lambda.Powertools.EventHandler/AppSyncEvents/AppSyncEventsResolver.cs +++ b/libraries/src/AWS.Lambda.Powertools.EventHandler/AppSyncEvents/AppSyncEventsResolver.cs @@ -21,7 +21,7 @@ public AppSyncEventsResolver() { _publishRoutes = new RouteHandlerRegistry(); _subscribeRoutes = new RouteHandlerRegistry(); - SystemWrapper.Instance.SetExecutionEnvironment(this); + PowertoolsEnvironment.Instance.SetExecutionEnvironment(this); } #region OnPublish Methods diff --git a/libraries/src/AWS.Lambda.Powertools.Kafka/PowertoolsKafkaSerializerBase.cs b/libraries/src/AWS.Lambda.Powertools.Kafka/PowertoolsKafkaSerializerBase.cs index 4c5b02ee..72b0fef3 100644 --- a/libraries/src/AWS.Lambda.Powertools.Kafka/PowertoolsKafkaSerializerBase.cs +++ b/libraries/src/AWS.Lambda.Powertools.Kafka/PowertoolsKafkaSerializerBase.cs @@ -78,7 +78,7 @@ protected PowertoolsKafkaSerializerBase(JsonSerializerOptions jsonOptions, JsonS JsonOptions = jsonOptions ?? new JsonSerializerOptions { PropertyNameCaseInsensitive = true }; SerializerContext = serializerContext; - SystemWrapper.Instance.SetExecutionEnvironment(this); + PowertoolsEnvironment.Instance.SetExecutionEnvironment(this); } /// diff --git a/libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/PowertoolsEnvironmentTest.cs b/libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/PowertoolsEnvironmentTest.cs index 936432ac..ace0a47e 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/PowertoolsEnvironmentTest.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/PowertoolsEnvironmentTest.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Xml.Linq; using System.Xml.XPath; +using NSubstitute; using Xunit; namespace AWS.Lambda.Powertools.Common.Tests; @@ -14,54 +15,57 @@ public class PowertoolsEnvironmentTest : IDisposable public void Set_Execution_Environment() { // Arrange - var systemWrapper = new SystemWrapper(new MockEnvironment()); + var powertoolsEnv = new PowertoolsEnvironment(); // Act - systemWrapper.SetExecutionEnvironment(this); + powertoolsEnv.SetExecutionEnvironment(this); // Assert - Assert.Equal($"{Constants.FeatureContextIdentifier}/Fake/1.0.0", systemWrapper.GetEnvironmentVariable("AWS_EXECUTION_ENV")); + Assert.Equal($"{Constants.FeatureContextIdentifier}/Tests/1.0.0 PTENV/AWS_LAMBDA_DOTNET{Environment.Version.Major}", powertoolsEnv.GetEnvironmentVariable("AWS_EXECUTION_ENV")); } [Fact] public void Set_Execution_Environment_WhenEnvironmentHasValue() { // Arrange - var systemWrapper = new SystemWrapper(new MockEnvironment()); + var powertoolsEnv = new PowertoolsEnvironment(); - systemWrapper.SetEnvironmentVariable("AWS_EXECUTION_ENV", "ExistingValuesInUserAgent"); + powertoolsEnv.SetEnvironmentVariable("AWS_EXECUTION_ENV", "ExistingValuesInUserAgent"); // Act - systemWrapper.SetExecutionEnvironment(this); + powertoolsEnv.SetExecutionEnvironment(this); // Assert - Assert.Equal($"ExistingValuesInUserAgent {Constants.FeatureContextIdentifier}/Fake/1.0.0", systemWrapper.GetEnvironmentVariable("AWS_EXECUTION_ENV")); + Assert.Equal($"ExistingValuesInUserAgent {Constants.FeatureContextIdentifier}/Tests/1.0.0 PTENV/AWS_LAMBDA_DOTNET{Environment.Version.Major}", powertoolsEnv.GetEnvironmentVariable("AWS_EXECUTION_ENV")); } [Fact] - public void Set_Multiple_Execution_Environment() + public void Set_Same_Execution_Environment_Multiple_Times_Should_Only_Set_Once() { // Arrange - var systemWrapper = new SystemWrapper(new MockEnvironment()); + var powertoolsEnv = new PowertoolsEnvironment(); // Act - systemWrapper.SetExecutionEnvironment(this); + powertoolsEnv.SetExecutionEnvironment(this); + powertoolsEnv.SetExecutionEnvironment(this); // Assert - Assert.Equal($"{Constants.FeatureContextIdentifier}/Fake/1.0.0", systemWrapper.GetEnvironmentVariable("AWS_EXECUTION_ENV")); + Assert.Equal($"{Constants.FeatureContextIdentifier}/Tests/1.0.0 PTENV/AWS_LAMBDA_DOTNET{Environment.Version.Major}", powertoolsEnv.GetEnvironmentVariable("AWS_EXECUTION_ENV")); } [Fact] - public void Set_Execution_Real_Environment() + public void Set_Multiple_Execution_Environment() { // Arrange - var systemWrapper = new SystemWrapper(new PowertoolsEnvironment()); + var powertoolsEnv = new PowertoolsEnvironment(); // Act - systemWrapper.SetExecutionEnvironment(this); + powertoolsEnv.SetExecutionEnvironment(this); + powertoolsEnv.SetExecutionEnvironment(powertoolsEnv.GetType()); // Assert - Assert.Equal($"{Constants.FeatureContextIdentifier}/Tests/1.0.0", systemWrapper.GetEnvironmentVariable("AWS_EXECUTION_ENV")); + Assert.Equal($"{Constants.FeatureContextIdentifier}/Tests/1.0.0 PTENV/AWS_LAMBDA_DOTNET{Environment.Version.Major} {Constants.FeatureContextIdentifier}/Common/1.0.0", + powertoolsEnv.GetEnvironmentVariable("AWS_EXECUTION_ENV")); } [Fact] @@ -83,44 +87,103 @@ public void Should_Use_Aspect_Injector_281() Assert.Equal("2.8.1", packageReference.Version.ToString()); } - public void Dispose() + [Fact] + public void SetExecutionEnvironment_Should_Format_Strings_Correctly_With_Mocked_Environment() { - //Do cleanup actions here + // Arrange + var mockEnvironment = Substitute.For(); + var testType = this.GetType(); - Environment.SetEnvironmentVariable("AWS_EXECUTION_ENV", null); + // Mock the dependencies to return controlled values + mockEnvironment.GetAssemblyName(Arg.Any()).Returns("AWS.Lambda.Powertools.Common.Tests"); + mockEnvironment.GetAssemblyVersion(Arg.Any()).Returns("1.2.3"); + mockEnvironment.GetEnvironmentVariable("AWS_EXECUTION_ENV").Returns((string)null); + + // Setup the actual method call to use real implementation logic + mockEnvironment.When(x => x.SetExecutionEnvironment(Arg.Any())) + .Do(callInfo => + { + var assemblyName = "PT/Tests"; // Parsed name + var assemblyVersion = "1.2.3"; + var runtimeEnv = "PTENV/AWS_LAMBDA_DOTNET8"; // Assuming .NET 8 + var expectedValue = $"{assemblyName}/{assemblyVersion} {runtimeEnv}"; + + mockEnvironment.SetEnvironmentVariable("AWS_EXECUTION_ENV", expectedValue); + }); + + // Act + mockEnvironment.SetExecutionEnvironment(this); + + // Assert + mockEnvironment.Received(1).SetEnvironmentVariable("AWS_EXECUTION_ENV", "PT/Tests/1.2.3 PTENV/AWS_LAMBDA_DOTNET8"); } -} - -/// -/// Fake Environment for testing -/// -class MockEnvironment : IPowertoolsEnvironment -{ - private readonly Dictionary _mockEnvironment = new(); - public string GetEnvironmentVariable(string variableName) - { - return _mockEnvironment.TryGetValue(variableName, out var value) ? value : null; - } - - public void SetEnvironmentVariable(string variableName, string value) - { - // Check for entry not existing and add to dictionary - _mockEnvironment[variableName] = value; - } - - public string GetAssemblyName(T type) + [Fact] + public void SetExecutionEnvironment_Should_Append_To_Existing_Environment_With_Mocked_Values() { - return "AWS.Lambda.Powertools.Fake"; + // Arrange + var mockEnvironment = Substitute.For(); + + // Mock existing environment value + mockEnvironment.GetEnvironmentVariable("AWS_EXECUTION_ENV").Returns("ExistingValue"); + mockEnvironment.GetAssemblyName(Arg.Any()).Returns("AWS.Lambda.Powertools.Logging"); + mockEnvironment.GetAssemblyVersion(Arg.Any()).Returns("2.1.0"); + + // Setup the method call + mockEnvironment.When(x => x.SetExecutionEnvironment(Arg.Any())) + .Do(callInfo => + { + var currentEnv = "ExistingValue"; + var assemblyName = "PT/Logging"; + var assemblyVersion = "2.1.0"; + var runtimeEnv = "PTENV/AWS_LAMBDA_DOTNET8"; + var expectedValue = $"{currentEnv} {assemblyName}/{assemblyVersion} {runtimeEnv}"; + + mockEnvironment.SetEnvironmentVariable("AWS_EXECUTION_ENV", expectedValue); + }); + + // Act + mockEnvironment.SetExecutionEnvironment(this); + + // Assert + mockEnvironment.Received(1).SetEnvironmentVariable("AWS_EXECUTION_ENV", "ExistingValue PT/Logging/2.1.0 PTENV/AWS_LAMBDA_DOTNET8"); } - - public string GetAssemblyVersion(T type) + + [Fact] + public void SetExecutionEnvironment_Should_Not_Add_PTENV_Twice_With_Mocked_Values() { - return "1.0.0"; + // Arrange + var mockEnvironment = Substitute.For(); + + // Mock existing environment value that already contains PTENV + mockEnvironment.GetEnvironmentVariable("AWS_EXECUTION_ENV").Returns("PT/Metrics/1.0.0 PTENV/AWS_LAMBDA_DOTNET8"); + mockEnvironment.GetAssemblyName(Arg.Any()).Returns("AWS.Lambda.Powertools.Tracing"); + mockEnvironment.GetAssemblyVersion(Arg.Any()).Returns("1.5.0"); + + // Setup the method call - should not add PTENV again + mockEnvironment.When(x => x.SetExecutionEnvironment(Arg.Any())) + .Do(callInfo => + { + var currentEnv = "PT/Metrics/1.0.0 PTENV/AWS_LAMBDA_DOTNET8"; + var assemblyName = "PT/Tracing"; + var assemblyVersion = "1.5.0"; + // No PTENV added since it already exists + var expectedValue = $"{currentEnv} {assemblyName}/{assemblyVersion}"; + + mockEnvironment.SetEnvironmentVariable("AWS_EXECUTION_ENV", expectedValue); + }); + + // Act + mockEnvironment.SetExecutionEnvironment(this); + + // Assert + mockEnvironment.Received(1).SetEnvironmentVariable("AWS_EXECUTION_ENV", "PT/Metrics/1.0.0 PTENV/AWS_LAMBDA_DOTNET8 PT/Tracing/1.5.0"); } - - public void SetExecutionEnvironment(T type) + + public void Dispose() { - throw new NotImplementedException(); + //Do cleanup actions here + + Environment.SetEnvironmentVariable("AWS_EXECUTION_ENV", null); } } diff --git a/libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/SystemWrapperTests.cs b/libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/SystemWrapperTests.cs deleted file mode 100644 index ff5a7fb0..00000000 --- a/libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/SystemWrapperTests.cs +++ /dev/null @@ -1,204 +0,0 @@ -using System; -using System.IO; -using System.Reflection; -using NSubstitute; -using Xunit; - -namespace AWS.Lambda.Powertools.Common.Tests; - -[Collection("Sequential")] -public class SystemWrapperTests : IDisposable -{ - private readonly IPowertoolsEnvironment _mockEnvironment; - private readonly StringWriter _testWriter; - private readonly FieldInfo _outputResetPerformedField; - - - public SystemWrapperTests() - { - _mockEnvironment = Substitute.For(); - _testWriter = new StringWriter(); - - // Get access to private field for testing - _outputResetPerformedField = typeof(SystemWrapper).GetField("_outputResetPerformed", - BindingFlags.NonPublic | BindingFlags.Static); - - // Reset static state between tests - SystemWrapper.ResetTestMode(); - _outputResetPerformedField.SetValue(null, false); - } - - [Fact] - public void Log_InProductionMode_ResetsOutputOnce() - { - // Arrange - var wrapper = new SystemWrapper(_mockEnvironment); - var message1 = "First message"; - var message2 = "Second message"; - _outputResetPerformedField.SetValue(null, false); - - // Act - wrapper.Log(message1); - bool afterFirstLog = (bool)_outputResetPerformedField.GetValue(null); - wrapper.Log(message2); - bool afterSecondLog = (bool)_outputResetPerformedField.GetValue(null); - - // Assert - Assert.True(afterFirstLog, "Flag should be set after first log"); - Assert.True(afterSecondLog, "Flag should remain set after second log"); - } - - [Fact] - public void LogLine_InProductionMode_ResetsOutputOnce() - { - // Arrange - var wrapper = new SystemWrapper(_mockEnvironment); - var message1 = "First line"; - var message2 = "Second line"; - _outputResetPerformedField.SetValue(null, false); - - // Act - wrapper.LogLine(message1); - bool afterFirstLog = (bool)_outputResetPerformedField.GetValue(null); - wrapper.LogLine(message2); - bool afterSecondLog = (bool)_outputResetPerformedField.GetValue(null); - - // Assert - Assert.True(afterFirstLog, "Flag should be set after first LogLine"); - Assert.True(afterSecondLog, "Flag should remain set after second LogLine"); - } - - [Fact] - public void ClearOutputResetFlag_ResetsFlag_AllowsSubsequentReset() - { - // Arrange - var wrapper = new SystemWrapper(_mockEnvironment); - _outputResetPerformedField.SetValue(null, false); - - // Act - wrapper.Log("First message"); // This should cause a reset - bool afterFirstLog = (bool)_outputResetPerformedField.GetValue(null); - - SystemWrapper.ClearOutputResetFlag(); - bool afterClear = (bool)_outputResetPerformedField.GetValue(null); - - wrapper.Log("After clear"); // This should cause another reset - bool afterSecondLog = (bool)_outputResetPerformedField.GetValue(null); - - // Assert - Assert.True(afterFirstLog, "Flag should be set after first log"); - Assert.False(afterClear, "Flag should be cleared after ClearOutputResetFlag"); - Assert.True(afterSecondLog, "Flag should be set again after second log"); - } - - [Fact] - public void Log_InTestMode_WritesToTestOutput() - { - // Arrange - var wrapper = new SystemWrapper(_mockEnvironment); - SystemWrapper.SetOut(_testWriter); - var message = "Test message"; - - // Act - wrapper.Log(message); - - // Assert - Assert.Equal(message, _testWriter.ToString()); - } - - [Fact] - public void LogLine_InTestMode_WritesToTestOutput() - { - // Arrange - var wrapper = new SystemWrapper(_mockEnvironment); - SystemWrapper.SetOut(_testWriter); - var message = "Test line"; - - // Act - wrapper.LogLine(message); - - // Assert - Assert.Equal(message + Environment.NewLine, _testWriter.ToString()); - } - - [Fact] - public void ResetTestMode_ResetsTestState() - { - // Arrange - var wrapper = new SystemWrapper(_mockEnvironment); - SystemWrapper.SetOut(_testWriter); - var message = "This should go to console"; - - // Act - SystemWrapper.ResetTestMode(); - - // Can't directly test that this goes to console, but we can verify - // it doesn't go to the test writer - wrapper.Log(message); - - // Assert - Assert.Equal("", _testWriter.ToString()); - } - - [Fact] - public void SetOut_EnablesTestMode() - { - // Arrange - var wrapper = new SystemWrapper(_mockEnvironment); - var message = "Test output"; - - // Act - SystemWrapper.SetOut(_testWriter); - wrapper.Log(message); - - // Assert - Assert.Equal(message, _testWriter.ToString()); - } - - [Fact] - public void Log_InTestMode_DoesNotCallResetConsoleOutput() - { - // Arrange - var wrapper = new SystemWrapper(_mockEnvironment); - SystemWrapper.SetOut(_testWriter); - var message1 = "First test message"; - var message2 = "Second test message"; - - // Act - wrapper.Log(message1); - wrapper.Log(message2); - - // Assert - Assert.Equal(message1 + message2, _testWriter.ToString()); - } - - [Fact] - public void Log_AfterClearingFlag_ResetsOutputAgain() - { - // Arrange - var wrapper = new SystemWrapper(_mockEnvironment); - _outputResetPerformedField.SetValue(null, false); - - // Act - wrapper.Log("First message"); // Should reset output - bool afterFirstLog = (bool)_outputResetPerformedField.GetValue(null); - - SystemWrapper.ClearOutputResetFlag(); - bool afterClear = (bool)_outputResetPerformedField.GetValue(null); - - wrapper.Log("Second message"); // Should reset again - bool afterSecondLog = (bool)_outputResetPerformedField.GetValue(null); - - // Assert - Assert.True(afterFirstLog, "Flag should be set after first log"); - Assert.False(afterClear, "Flag should be reset after clearing"); - Assert.True(afterSecondLog, "Flag should be set after second log"); - } - - public void Dispose() - { - _testWriter?.Dispose(); - SystemWrapper.ResetTestMode(); - _outputResetPerformedField.SetValue(null, false); - } -} \ No newline at end of file From 0f88519de62edb3b9d2c825c2a0b017aae21943a Mon Sep 17 00:00:00 2001 From: Henrique Graca <999396+hjgraca@users.noreply.github.com> Date: Wed, 16 Jul 2025 18:50:54 +0100 Subject: [PATCH 2/5] refactor: change ParseAssemblyName method visibility to internal and improve unit tests for assembly name handling --- .../Core/PowertoolsEnvironment.cs | 2 +- .../Core/PowertoolsEnvironmentTest.cs | 160 +++++++++++++++++- 2 files changed, 154 insertions(+), 8 deletions(-) diff --git a/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs b/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs index 79c1a6b4..4019dcb8 100644 --- a/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs +++ b/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs @@ -117,7 +117,7 @@ public void SetExecutionEnvironment(T type) /// /// /// - private string ParseAssemblyName(string assemblyName) + internal string ParseAssemblyName(string assemblyName) { // Use cache to avoid repeated string operations return ParsedAssemblyNameCache.GetOrAdd(assemblyName, name => diff --git a/libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/PowertoolsEnvironmentTest.cs b/libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/PowertoolsEnvironmentTest.cs index ace0a47e..19316160 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/PowertoolsEnvironmentTest.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/PowertoolsEnvironmentTest.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.IO; using System.Linq; using System.Xml.Linq; @@ -92,8 +91,7 @@ public void SetExecutionEnvironment_Should_Format_Strings_Correctly_With_Mocked_ { // Arrange var mockEnvironment = Substitute.For(); - var testType = this.GetType(); - + // Mock the dependencies to return controlled values mockEnvironment.GetAssemblyName(Arg.Any()).Returns("AWS.Lambda.Powertools.Common.Tests"); mockEnvironment.GetAssemblyVersion(Arg.Any()).Returns("1.2.3"); @@ -101,7 +99,7 @@ public void SetExecutionEnvironment_Should_Format_Strings_Correctly_With_Mocked_ // Setup the actual method call to use real implementation logic mockEnvironment.When(x => x.SetExecutionEnvironment(Arg.Any())) - .Do(callInfo => + .Do(_ => { var assemblyName = "PT/Tests"; // Parsed name var assemblyVersion = "1.2.3"; @@ -131,7 +129,7 @@ public void SetExecutionEnvironment_Should_Append_To_Existing_Environment_With_M // Setup the method call mockEnvironment.When(x => x.SetExecutionEnvironment(Arg.Any())) - .Do(callInfo => + .Do(_ => { var currentEnv = "ExistingValue"; var assemblyName = "PT/Logging"; @@ -162,7 +160,7 @@ public void SetExecutionEnvironment_Should_Not_Add_PTENV_Twice_With_Mocked_Value // Setup the method call - should not add PTENV again mockEnvironment.When(x => x.SetExecutionEnvironment(Arg.Any())) - .Do(callInfo => + .Do(_ => { var currentEnv = "PT/Metrics/1.0.0 PTENV/AWS_LAMBDA_DOTNET8"; var assemblyName = "PT/Tracing"; @@ -180,10 +178,158 @@ public void SetExecutionEnvironment_Should_Not_Add_PTENV_Twice_With_Mocked_Value mockEnvironment.Received(1).SetEnvironmentVariable("AWS_EXECUTION_ENV", "PT/Metrics/1.0.0 PTENV/AWS_LAMBDA_DOTNET8 PT/Tracing/1.5.0"); } + [Fact] + public void GetAssemblyName_Should_Handle_Type_Object() + { + // Arrange + var powertoolsEnv = new PowertoolsEnvironment(); + var typeObject = typeof(PowertoolsEnvironment); + + // Act + var result = powertoolsEnv.GetAssemblyName(typeObject); + + // Assert + Assert.Equal("AWS.Lambda.Powertools.Common", result); + } + + [Fact] + public void GetAssemblyName_Should_Handle_Regular_Object() + { + // Arrange + var powertoolsEnv = new PowertoolsEnvironment(); + + // Act + var result = powertoolsEnv.GetAssemblyName(this); + + // Assert + Assert.Equal("AWS.Lambda.Powertools.Common.Tests", result); + } + + [Fact] + public void GetAssemblyVersion_Should_Handle_Type_Object() + { + // Arrange + var powertoolsEnv = new PowertoolsEnvironment(); + var typeObject = typeof(PowertoolsEnvironment); + + // Act + var result = powertoolsEnv.GetAssemblyVersion(typeObject); + + // Assert + Assert.Matches(@"\d+\.\d+\.\d+", result); // Should match version pattern like "1.0.0" + } + + [Fact] + public void GetAssemblyVersion_Should_Handle_Regular_Object() + { + // Arrange + var powertoolsEnv = new PowertoolsEnvironment(); + + // Act + var result = powertoolsEnv.GetAssemblyVersion(this); + + // Assert + Assert.Matches(@"\d+\.\d+\.\d+", result); // Should match version pattern like "1.0.0" + } + + [Fact] + public void ParseAssemblyName_Should_Handle_Assembly_Without_Dots() + { + // Arrange + var powertoolsEnv = new PowertoolsEnvironment(); + + // Act + var result = powertoolsEnv.ParseAssemblyName("SimpleAssemblyName"); + + // Assert + Assert.Equal($"{Constants.FeatureContextIdentifier}/SimpleAssemblyName", result); + } + + [Fact] + public void ParseAssemblyName_Should_Handle_Assembly_With_Dots() + { + // Arrange + var powertoolsEnv = new PowertoolsEnvironment(); + + // Act + var result = powertoolsEnv.ParseAssemblyName("AWS.Lambda.Powertools.Common"); + + // Assert + Assert.Equal($"{Constants.FeatureContextIdentifier}/Common", result); + } + + [Fact] + public void ParseAssemblyName_Should_Use_Cache_For_Same_Assembly_Name() + { + // Arrange + var powertoolsEnv = new PowertoolsEnvironment(); + + // Act - Call twice with same assembly name + var result1 = powertoolsEnv.ParseAssemblyName("AWS.Lambda.Powertools.Tests"); + var result2 = powertoolsEnv.ParseAssemblyName("AWS.Lambda.Powertools.Tests"); + + // Assert - Should return same result (cached) + Assert.Equal(result1, result2); + Assert.Equal($"{Constants.FeatureContextIdentifier}/Tests", result1); + } + + [Fact] + public void SetExecutionEnvironment_Should_Handle_Empty_Current_Environment() + { + // Arrange + var powertoolsEnv = new PowertoolsEnvironment(); + Environment.SetEnvironmentVariable("AWS_EXECUTION_ENV", ""); + + // Act + powertoolsEnv.SetExecutionEnvironment(this); + + // Assert + var result = powertoolsEnv.GetEnvironmentVariable("AWS_EXECUTION_ENV"); + Assert.Contains($"{Constants.FeatureContextIdentifier}/Tests/", result); + Assert.Contains("PTENV/AWS_LAMBDA_DOTNET", result); + } + + [Fact] + public void SetExecutionEnvironment_Should_Add_PTENV_When_Not_Present() + { + // Arrange + var powertoolsEnv = new PowertoolsEnvironment(); + powertoolsEnv.SetEnvironmentVariable("AWS_EXECUTION_ENV", "SomeExistingValue"); + + // Act + powertoolsEnv.SetExecutionEnvironment(this); + + // Assert + var result = powertoolsEnv.GetEnvironmentVariable("AWS_EXECUTION_ENV"); + Assert.StartsWith("SomeExistingValue", result); + Assert.Contains("PTENV/AWS_LAMBDA_DOTNET", result); + } + + [Fact] + public void SetExecutionEnvironment_Should_Not_Add_PTENV_When_Already_Present() + { + // Arrange + var powertoolsEnv = new PowertoolsEnvironment(); + var existingValue = $"ExistingValue PTENV/AWS_LAMBDA_DOTNET{Environment.Version.Major}"; + powertoolsEnv.SetEnvironmentVariable("AWS_EXECUTION_ENV", existingValue); + + // Act + powertoolsEnv.SetExecutionEnvironment(this); + + // Assert + var result = powertoolsEnv.GetEnvironmentVariable("AWS_EXECUTION_ENV"); + var ptenvCount = result.Split("PTENV/").Length - 1; + Assert.Equal(1, ptenvCount); // Should only have one PTENV entry + } + public void Dispose() { //Do cleanup actions here - Environment.SetEnvironmentVariable("AWS_EXECUTION_ENV", null); + + // Clear the singleton instance to ensure fresh state for each test + var instanceField = typeof(PowertoolsEnvironment).GetField("_instance", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static); + instanceField?.SetValue(null, null); } } From 0da98f73a0e3e21910e4c2d8e0f67e4f1943df50 Mon Sep 17 00:00:00 2001 From: Henrique Graca <999396+hjgraca@users.noreply.github.com> Date: Wed, 16 Jul 2025 19:13:17 +0100 Subject: [PATCH 3/5] refactor: make ParseAssemblyName static and enhance error handling in assembly name parsing --- .../Core/PowertoolsEnvironment.cs | 20 +++++++------- .../Core/PowertoolsEnvironmentTest.cs | 27 ++++++++++--------- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs b/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs index 4019dcb8..e87f01a7 100644 --- a/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs +++ b/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs @@ -117,12 +117,12 @@ public void SetExecutionEnvironment(T type) /// /// /// - internal string ParseAssemblyName(string assemblyName) + internal static string ParseAssemblyName(string assemblyName) { // Use cache to avoid repeated string operations - return ParsedAssemblyNameCache.GetOrAdd(assemblyName, name => + try { - try + return ParsedAssemblyNameCache.GetOrAdd(assemblyName, name => { var lastDotIndex = name.LastIndexOf('.'); if (lastDotIndex >= 0 && lastDotIndex < name.Length - 1) @@ -130,13 +130,13 @@ internal string ParseAssemblyName(string assemblyName) var parsedName = name.Substring(lastDotIndex + 1); return $"{Constants.FeatureContextIdentifier}/{parsedName}"; } - } - catch - { - //NOOP - } - return $"{Constants.FeatureContextIdentifier}/{name}"; - }); + return $"{Constants.FeatureContextIdentifier}/{name}"; + }); + } + catch (Exception e) + { + return string.Empty; + } } } \ No newline at end of file diff --git a/libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/PowertoolsEnvironmentTest.cs b/libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/PowertoolsEnvironmentTest.cs index 19316160..9f9e153c 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/PowertoolsEnvironmentTest.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Common.Tests/Core/PowertoolsEnvironmentTest.cs @@ -235,11 +235,8 @@ public void GetAssemblyVersion_Should_Handle_Regular_Object() [Fact] public void ParseAssemblyName_Should_Handle_Assembly_Without_Dots() { - // Arrange - var powertoolsEnv = new PowertoolsEnvironment(); - // Act - var result = powertoolsEnv.ParseAssemblyName("SimpleAssemblyName"); + var result = PowertoolsEnvironment.ParseAssemblyName("SimpleAssemblyName"); // Assert Assert.Equal($"{Constants.FeatureContextIdentifier}/SimpleAssemblyName", result); @@ -248,11 +245,8 @@ public void ParseAssemblyName_Should_Handle_Assembly_Without_Dots() [Fact] public void ParseAssemblyName_Should_Handle_Assembly_With_Dots() { - // Arrange - var powertoolsEnv = new PowertoolsEnvironment(); - // Act - var result = powertoolsEnv.ParseAssemblyName("AWS.Lambda.Powertools.Common"); + var result = PowertoolsEnvironment.ParseAssemblyName("AWS.Lambda.Powertools.Common"); // Assert Assert.Equal($"{Constants.FeatureContextIdentifier}/Common", result); @@ -261,18 +255,25 @@ public void ParseAssemblyName_Should_Handle_Assembly_With_Dots() [Fact] public void ParseAssemblyName_Should_Use_Cache_For_Same_Assembly_Name() { - // Arrange - var powertoolsEnv = new PowertoolsEnvironment(); - // Act - Call twice with same assembly name - var result1 = powertoolsEnv.ParseAssemblyName("AWS.Lambda.Powertools.Tests"); - var result2 = powertoolsEnv.ParseAssemblyName("AWS.Lambda.Powertools.Tests"); + var result1 = PowertoolsEnvironment.ParseAssemblyName("AWS.Lambda.Powertools.Tests"); + var result2 = PowertoolsEnvironment.ParseAssemblyName("AWS.Lambda.Powertools.Tests"); // Assert - Should return same result (cached) Assert.Equal(result1, result2); Assert.Equal($"{Constants.FeatureContextIdentifier}/Tests", result1); } + [Fact] + public void ParseAssemblyName_Null_Return_Empty() + { + // Act - Call twice with same assembly name + var result = PowertoolsEnvironment.ParseAssemblyName(null); + + // Assert - Should return null + Assert.Empty(result); + } + [Fact] public void SetExecutionEnvironment_Should_Handle_Empty_Current_Environment() { From 0232f415f66d8097654dd6eeac50c2f220b48d06 Mon Sep 17 00:00:00 2001 From: Henrique Graca <999396+hjgraca@users.noreply.github.com> Date: Fri, 18 Jul 2025 14:14:54 +0100 Subject: [PATCH 4/5] Update libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs Co-authored-by: Andrea Amorosi Signed-off-by: Henrique Graca <999396+hjgraca@users.noreply.github.com> --- .../AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs b/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs index e87f01a7..2ddc17ff 100644 --- a/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs +++ b/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs @@ -134,7 +134,7 @@ internal static string ParseAssemblyName(string assemblyName) return $"{Constants.FeatureContextIdentifier}/{name}"; }); } - catch (Exception e) + catch (Exception) { return string.Empty; } From e0a625918b561c77736a4293e5cee551129b95c8 Mon Sep 17 00:00:00 2001 From: Henrique Graca <999396+hjgraca@users.noreply.github.com> Date: Fri, 18 Jul 2025 14:15:48 +0100 Subject: [PATCH 5/5] Update PowertoolsEnvironment.cs Signed-off-by: Henrique Graca <999396+hjgraca@users.noreply.github.com> --- .../Core/PowertoolsEnvironment.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs b/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs index 2ddc17ff..afc796b6 100644 --- a/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs +++ b/libraries/src/AWS.Lambda.Powertools.Common/Core/PowertoolsEnvironment.cs @@ -134,9 +134,9 @@ internal static string ParseAssemblyName(string assemblyName) return $"{Constants.FeatureContextIdentifier}/{name}"; }); } - catch (Exception) + catch { return string.Empty; } } -} \ No newline at end of file +}