From 4aa7d73c8408e4ff0ece02179abb64412b6fd144 Mon Sep 17 00:00:00 2001 From: Phil Asmar Date: Thu, 8 Aug 2024 09:40:44 -0400 Subject: [PATCH] reply to Norm's comments --- .../Custom/DataModel/ContextBuilder.cs | 115 +++++------------- .../DataModel/IDynamoDBContextBuilder.cs | 29 +++-- .../IntegrationTests/Config/462/App.config | 3 - .../IntegrationTests/DataModelTests.cs | 2 +- .../IntegrationTests/DynamoDBTestsBase.cs | 2 +- .../Custom/DynamoDBContextBuilderTests.cs | 25 ---- 6 files changed, 50 insertions(+), 126 deletions(-) delete mode 100644 sdk/test/Services/DynamoDBv2/UnitTests/Custom/DynamoDBContextBuilderTests.cs diff --git a/sdk/src/Services/DynamoDBv2/Custom/DataModel/ContextBuilder.cs b/sdk/src/Services/DynamoDBv2/Custom/DataModel/ContextBuilder.cs index 3a5f6168d378..f034becc5717 100644 --- a/sdk/src/Services/DynamoDBv2/Custom/DataModel/ContextBuilder.cs +++ b/sdk/src/Services/DynamoDBv2/Custom/DataModel/ContextBuilder.cs @@ -19,36 +19,34 @@ namespace Amazon.DynamoDBv2.DataModel { /// /// Builder that constructs a + /// Using to construct a will implicitly set + /// to true which avoids the DescribeTable call + /// and relies entirely on the DynamoDB attributes set on the .NET classes. + /// If needed, you can revert back to the previous behavior by setting + /// to false using as such: + /// + /// var context = new DynamoDBContextBuilder() + /// .ConfigureContext(x => + /// { + /// x.DisableFetchingTableMetadata = false; + /// }) + /// .Build(); + /// /// #if NET8_0_OR_GREATER [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode(Amazon.DynamoDBv2.Custom.Internal.InternalConstants.RequiresUnreferencedCodeMessage)] #endif public class DynamoDBContextBuilder : IDynamoDBContextBuilder { - /// - /// The object that is built and then returned from - /// - private DynamoDBContext _context; - /// /// The object that is built and then supplied to the returned /// private DynamoDBContextConfig _config; /// - /// Amazon client to use to access DynamoDB + /// A factory method for creating a client /// - private IAmazonDynamoDB _client; - - /// - /// Specifies if an client is constructed or supplied externally - /// - private bool _ownClient = true; - - /// - /// Specifies if the object was disposed to avoid redundant dispose calls - /// - private bool _disposed = false; + private Func _clientFactory; /// /// Creates a builder object to construct a @@ -60,41 +58,17 @@ public DynamoDBContextBuilder() } /// - public IDynamoDBContextBuilder SetDynamoDBClient(IAmazonDynamoDB client) - { - if (_client is null) - { - _client = client; - _ownClient = false; - } - else - { - throw new AmazonDynamoDBException($"Cannot use both '{nameof(SetDynamoDBClient)}' and '{nameof(SetAWSRegion)}'. If you are supplying the DynamoDB client, set the AWS Region directly on the client."); - } - - return this; - } - - /// - public IDynamoDBContextBuilder SetAWSRegion(RegionEndpoint region) + public IDynamoDBContextBuilder ConfigureContext(Action configure) { - if (_client is null) - { - _client = new AmazonDynamoDBClient(region); - _ownClient = true; - } - else - { - throw new AmazonDynamoDBException($"Cannot use both '{nameof(SetDynamoDBClient)}' and '{nameof(SetAWSRegion)}'. If you are supplying the DynamoDB client, set the AWS Region directly on the client."); - } + configure(_config); return this; } /// - public IDynamoDBContextBuilder ConfigureContext(Action configure) + public IDynamoDBContextBuilder WithDynamoDBClient(Func factory) { - configure(_config); + _clientFactory = factory; return this; } @@ -102,52 +76,21 @@ public IDynamoDBContextBuilder ConfigureContext(Action co /// public DynamoDBContext Build() { - if (_client is null) + IAmazonDynamoDB client; + bool ownClient; + + if (_clientFactory is null) { - _client = new AmazonDynamoDBClient(); - _ownClient = true; + client = new AmazonDynamoDBClient(); + ownClient = true; } - - _context = new DynamoDBContext(_client, _ownClient, _config); - - return _context; - } - - /// - /// Disposes of all managed and unmanaged resources. - /// - public void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); - } - - /// - /// Implements the Dispose pattern - /// - /// Whether this object is being disposed via a call to Dispose - /// or garbage collected. - protected virtual void Dispose(bool disposing) - { - if (_disposed) return; - - if (disposing && _context != null) + else { - // Dispose managed resources - _context?.Dispose(); - _context = null; + client = _clientFactory.Invoke(); + ownClient = false; } - // Dispose unmanaged resources if any - _disposed = true; - } - - /// - /// The destructor for the class. - /// - ~DynamoDBContextBuilder() - { - Dispose(false); + return new DynamoDBContext(client, ownClient, _config); } } } diff --git a/sdk/src/Services/DynamoDBv2/Custom/DataModel/IDynamoDBContextBuilder.cs b/sdk/src/Services/DynamoDBv2/Custom/DataModel/IDynamoDBContextBuilder.cs index 70628c3a20eb..20dced34822e 100644 --- a/sdk/src/Services/DynamoDBv2/Custom/DataModel/IDynamoDBContextBuilder.cs +++ b/sdk/src/Services/DynamoDBv2/Custom/DataModel/IDynamoDBContextBuilder.cs @@ -19,20 +19,29 @@ namespace Amazon.DynamoDBv2.DataModel { /// /// Interface for a builder that constructs a + /// Using to construct a will implicitly set + /// to true which avoids the DescribeTable call + /// and relies entirely on the DynamoDB attributes set on the .NET classes. + /// If needed, you can revert back to the previous behavior by setting + /// to false using as such: + /// + /// var context = new DynamoDBContextBuilder() + /// .ConfigureContext(x => + /// { + /// x.DisableFetchingTableMetadata = false; + /// }) + /// .Build(); + /// /// - public interface IDynamoDBContextBuilder : IDisposable + public interface IDynamoDBContextBuilder { /// - /// Sets the DynamoDB client to be used by the that is constructed + /// Supplies a factory method for creating a client. + /// If a factory method is not provided, a new client + /// will be created using the environment to search for credentials and region configuration. /// - /// to use to access DynamoDB. - IDynamoDBContextBuilder SetDynamoDBClient(IAmazonDynamoDB client); - - /// - /// Sets the AWS Region to be used by the client and the constructed - /// - /// The used by the client. - IDynamoDBContextBuilder SetAWSRegion(RegionEndpoint region); + /// Factory method for creating a client + IDynamoDBContextBuilder WithDynamoDBClient(Func factory); /// /// Configures the that is being constructed diff --git a/sdk/test/Services/DynamoDBv2/IntegrationTests/Config/462/App.config b/sdk/test/Services/DynamoDBv2/IntegrationTests/Config/462/App.config index a00d7291d74a..c91128021d54 100644 --- a/sdk/test/Services/DynamoDBv2/IntegrationTests/Config/462/App.config +++ b/sdk/test/Services/DynamoDBv2/IntegrationTests/Config/462/App.config @@ -50,9 +50,6 @@ - - - diff --git a/sdk/test/Services/DynamoDBv2/IntegrationTests/DataModelTests.cs b/sdk/test/Services/DynamoDBv2/IntegrationTests/DataModelTests.cs index b8b3dd286016..aa8da86a6764 100644 --- a/sdk/test/Services/DynamoDBv2/IntegrationTests/DataModelTests.cs +++ b/sdk/test/Services/DynamoDBv2/IntegrationTests/DataModelTests.cs @@ -2495,9 +2495,9 @@ public class AnnotatedEpochEmployee // Hash key [DynamoDBHashKey(StoreAsEpoch = true)] - //[DynamoDBProperty(StoreAsEpoch = true)] public virtual DateTime CreationTime { get; set; } + [DynamoDBProperty(StoreAsEpoch = true)] public DateTime EpochDate2 { get; set; } [DynamoDBProperty(StoreAsEpoch = false)] diff --git a/sdk/test/Services/DynamoDBv2/IntegrationTests/DynamoDBTestsBase.cs b/sdk/test/Services/DynamoDBv2/IntegrationTests/DynamoDBTestsBase.cs index 8a8d43a505c2..1380e87dfad5 100644 --- a/sdk/test/Services/DynamoDBv2/IntegrationTests/DynamoDBTestsBase.cs +++ b/sdk/test/Services/DynamoDBv2/IntegrationTests/DynamoDBTestsBase.cs @@ -93,7 +93,7 @@ public void CleanupTables() public static void CreateContext(DynamoDBEntryConversion conversion, bool isEmptyStringValueEnabled, bool disableFetchingTableMetadata = false) { Context = new DynamoDBContextBuilder() - .SetDynamoDBClient(Client) + .WithDynamoDBClient(() => Client) .ConfigureContext(x => { //x.IgnoreNullValues = true; diff --git a/sdk/test/Services/DynamoDBv2/UnitTests/Custom/DynamoDBContextBuilderTests.cs b/sdk/test/Services/DynamoDBv2/UnitTests/Custom/DynamoDBContextBuilderTests.cs deleted file mode 100644 index 9ce1708913d5..000000000000 --- a/sdk/test/Services/DynamoDBv2/UnitTests/Custom/DynamoDBContextBuilderTests.cs +++ /dev/null @@ -1,25 +0,0 @@ -using Amazon; -using Amazon.DynamoDBv2; -using Amazon.DynamoDBv2.DataModel; -using Microsoft.VisualStudio.TestTools.UnitTesting; - -namespace AWSSDK.UnitTests.DynamoDBv2.NetFramework.Custom -{ - [TestClass] - public class DynamoDBContextBuilderTests - { - [TestMethod] - public void SetClientAndRegion_ThrowsException() - { - var ddbClient = new AmazonDynamoDBClient(); - - Assert.ThrowsException(() => - { - var context = new DynamoDBContextBuilder() - .SetDynamoDBClient(ddbClient) - .SetAWSRegion(RegionEndpoint.USWest2) - .Build(); - }); - } - } -}