Skip to content

Commit

Permalink
reply to Norm's comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@philasmar
philasmar committed Aug 9, 2024
1 parent 4476800 commit 4aa7d73
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 126 deletions.
115 changes: 29 additions & 86 deletions sdk/src/Services/DynamoDBv2/Custom/DataModel/ContextBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,36 +19,34 @@ namespace Amazon.DynamoDBv2.DataModel
{
/// <summary>
/// Builder that constructs a <see cref="DynamoDBContext"/>
/// Using <see cref="DynamoDBContextBuilder"/> to construct a <see cref="DynamoDBContext"/> will implicitly set
/// <see cref="DynamoDBContextConfig.DisableFetchingTableMetadata"/> 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 <see cref="DynamoDBContextConfig.DisableFetchingTableMetadata"/>
/// to false using <see cref="DynamoDBContextBuilder.ConfigureContext(Action{DynamoDBContextConfig})"/> as such:
/// <code>
/// var context = new DynamoDBContextBuilder()
/// .ConfigureContext(x =>
/// {
/// x.DisableFetchingTableMetadata = false;
/// })
/// .Build();
/// </code>
/// </summary>
#if NET8_0_OR_GREATER
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode(Amazon.DynamoDBv2.Custom.Internal.InternalConstants.RequiresUnreferencedCodeMessage)]
#endif
public class DynamoDBContextBuilder : IDynamoDBContextBuilder
{
/// <summary>
/// The <see cref="DynamoDBContext"/> object that is built and then returned from <see cref="Build"/>
/// </summary>
private DynamoDBContext _context;

/// <summary>
/// The <see cref="DynamoDBContextConfig"/> object that is built and then supplied to the returned <see cref="DynamoDBContext"/>
/// </summary>
private DynamoDBContextConfig _config;

/// <summary>
/// Amazon client to use to access DynamoDB
/// A factory method for creating a <see cref="IAmazonDynamoDB"/> client
/// </summary>
private IAmazonDynamoDB _client;

/// <summary>
/// Specifies if an <see cref="IAmazonDynamoDB"/> client is constructed or supplied externally
/// </summary>
private bool _ownClient = true;

/// <summary>
/// Specifies if the object was disposed to avoid redundant dispose calls
/// </summary>
private bool _disposed = false;
private Func<IAmazonDynamoDB> _clientFactory;

/// <summary>
/// Creates a builder object to construct a <see cref="DynamoDBContext"/>
Expand All @@ -60,94 +58,39 @@ public DynamoDBContextBuilder()
}

/// <inheritdoc/>
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;
}

/// <inheritdoc/>
public IDynamoDBContextBuilder SetAWSRegion(RegionEndpoint region)
public IDynamoDBContextBuilder ConfigureContext(Action<DynamoDBContextConfig> 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;
}

/// <inheritdoc/>
public IDynamoDBContextBuilder ConfigureContext(Action<DynamoDBContextConfig> configure)
public IDynamoDBContextBuilder WithDynamoDBClient(Func<IAmazonDynamoDB> factory)
{
configure(_config);
_clientFactory = factory;

return this;
}

/// <inheritdoc/>
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;
}

/// <summary>
/// Disposes of all managed and unmanaged resources.
/// </summary>
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}

/// <summary>
/// Implements the Dispose pattern
/// </summary>
/// <param name="disposing">Whether this object is being disposed via a call to Dispose
/// or garbage collected.</param>
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;
}

/// <summary>
/// The destructor for the class.
/// </summary>
~DynamoDBContextBuilder()
{
Dispose(false);
return new DynamoDBContext(client, ownClient, _config);
}
}
}
29 changes: 19 additions & 10 deletions sdk/src/Services/DynamoDBv2/Custom/DataModel/IDynamoDBContextBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,29 @@ namespace Amazon.DynamoDBv2.DataModel
{
/// <summary>
/// Interface for a builder that constructs a <see cref="DynamoDBContext"/>
/// Using <see cref="IDynamoDBContextBuilder"/> to construct a <see cref="DynamoDBContext"/> will implicitly set
/// <see cref="DynamoDBContextConfig.DisableFetchingTableMetadata"/> 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 <see cref="DynamoDBContextConfig.DisableFetchingTableMetadata"/>
/// to false using <see cref="IDynamoDBContextBuilder.ConfigureContext(Action{DynamoDBContextConfig})"/> as such:
/// <code>
/// var context = new DynamoDBContextBuilder()
/// .ConfigureContext(x =>
/// {
/// x.DisableFetchingTableMetadata = false;
/// })
/// .Build();
/// </code>
/// </summary>
public interface IDynamoDBContextBuilder : IDisposable
public interface IDynamoDBContextBuilder
{
/// <summary>
/// Sets the DynamoDB client to be used by the <see cref="DynamoDBContext"/> that is constructed
/// Supplies a factory method for creating a <see cref="IAmazonDynamoDB"/> client.
/// If a factory method is not provided, a new <see cref="IAmazonDynamoDB"/> client
/// will be created using the environment to search for credentials and region configuration.
/// </summary>
/// <param name="client"><see cref="IAmazonDynamoDB"/> to use to access DynamoDB.</param>
IDynamoDBContextBuilder SetDynamoDBClient(IAmazonDynamoDB client);

/// <summary>
/// Sets the AWS Region to be used by the <see cref="IAmazonDynamoDB"/> client and the constructed <see cref="DynamoDBContext"/>
/// </summary>
/// <param name="region">The <see cref="RegionEndpoint"/> used by the <see cref="IAmazonDynamoDB"/> client.</param>
IDynamoDBContextBuilder SetAWSRegion(RegionEndpoint region);
/// <param name="factory">Factory method for creating a <see cref="IAmazonDynamoDB"/> client</param>
IDynamoDBContextBuilder WithDynamoDBClient(Func<IAmazonDynamoDB> factory);

/// <summary>
/// Configures the <see cref="DynamoDBContext"/> that is being constructed
Expand Down
3 changes: 0 additions & 3 deletions sdk/test/Services/DynamoDBv2/IntegrationTests/Config/462/App.config
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,6 @@
<map type="AWSSDK_DotNet.IntegrationTests.Tests.DynamoDB.DynamoDBTests+EpochEmployee, AWSSDK.IntegrationTests.DynamoDBv2.NetFramework" targetTable="HashRangeTable">
<property name="EpochDate2" storeAsEpoch="true" />
</map>
<map type="AWSSDK_DotNet.IntegrationTests.Tests.DynamoDB.DynamoDBTests+AnnotatedEpochEmployee, AWSSDK.IntegrationTests.DynamoDBv2.NetFramework" targetTable="NumericHashRangeTable">
<property name="EpochDate2" storeAsEpoch="true" />
</map>
</mappings>
</dynamoDBContext>
</dynamoDB>
Expand Down
2 changes: 1 addition & 1 deletion sdk/test/Services/DynamoDBv2/IntegrationTests/DataModelTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
2 changes: 1 addition & 1 deletion sdk/test/Services/DynamoDBv2/IntegrationTests/DynamoDBTestsBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
25 changes: 0 additions & 25 deletions sdk/test/Services/DynamoDBv2/UnitTests/Custom/DynamoDBContextBuilderTests.cs
View file

This file was deleted.

0 comments on commit 4aa7d73

Please sign in to comment.