OverrideTableName is not applied when passed in DynamoDBContext over DynamoDBOperationConfig

michaelmatveev commented 5 years ago

Expected Behavior

Suppose I create DynamoDBContext like follows

        private readonly string _tableName;
        private DynamoDBContext GetDbContext()
        {
            var dynamoDbOperationConfig = new DynamoDBOperationConfig
            {
                OverrideTableName = _tableName            
            };
            return new DynamoDBContext(_client, dynamoDbOperationConfig);            
        }

then I try to use context expecting to save an item with a class name different then a table name

       public async Task SaveItemAsync<T>(T item)
        {
            using (var context = GetDbContext())
            {
                await context.SaveAsync(item);
            }
        }

Current Behavior

Get an error

Result StackTrace:  
at Amazon.Runtime.Internal.HttpErrorResponseExceptionHandler.HandleException(IExecutionContext executionContext, HttpErrorResponseException exception) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\ErrorHandler\HttpErrorResponseExceptionHandler.cs:line 60
   at Amazon.Runtime.Internal.ErrorHandler.ProcessException(IExecutionContext executionContext, Exception exception) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\ErrorHandler\ErrorHandler.cs:line 212
   at Amazon.Runtime.Internal.ErrorHandler.InvokeSync(IExecutionContext executionContext) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\ErrorHandler\ErrorHandler.cs:line 78
   at Amazon.Runtime.Internal.CallbackHandler.InvokeSync(IExecutionContext executionContext) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\Handlers\CallbackHandler.cs:line 46
   at Amazon.Runtime.Internal.EndpointDiscoveryHandler.InvokeSync(IExecutionContext executionContext) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\Handlers\EndpointDiscoveryHandler.cs:line 44
   at Amazon.Runtime.Internal.RetryHandler.InvokeSync(IExecutionContext executionContext) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\RetryHandler\RetryHandler.cs:line 74
   at Amazon.Runtime.Internal.CallbackHandler.InvokeSync(IExecutionContext executionContext) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\Handlers\CallbackHandler.cs:line 46
   at Amazon.Runtime.Internal.CallbackHandler.InvokeSync(IExecutionContext executionContext) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\Handlers\CallbackHandler.cs:line 46
   at Amazon.Runtime.Internal.ErrorCallbackHandler.InvokeSync(IExecutionContext executionContext) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\Handlers\ErrorCallbackHandler.cs:line 39
   at Amazon.Runtime.Internal.MetricsHandler.InvokeSync(IExecutionContext executionContext) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\Handlers\MetricsHandler.cs:line 40
   at Amazon.Runtime.Internal.RuntimePipeline.InvokeSync(IExecutionContext executionContext) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\RuntimePipeline.cs:line 136
   at Amazon.Runtime.AmazonServiceClient.Invoke[TResponse](AmazonWebServiceRequest request, InvokeOptionsBase options) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\AmazonServiceClient.cs:line 203
   at Amazon.DynamoDBv2.DocumentModel.Table.DescribeTable(String tableName) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Services\DynamoDBv2\Custom\DocumentModel\Table.cs:line 328
   at Amazon.Runtime.Internal.Util.Cache`2.GetValueHelper(TKey key, Boolean& isStaleItem, Func`2 creator) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Internal\Util\SdkCache.cs:line 428
   at Amazon.DynamoDBv2.DocumentModel.Table.LoadTableInfo() in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Services\DynamoDBv2\Custom\DocumentModel\Table.cs:line 133
   at Amazon.DynamoDBv2.DataModel.DynamoDBContext.GetUnconfiguredTable(String tableName) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Services\DynamoDBv2\Custom\DataModel\ContextInternal.cs:line 129
   at Amazon.DynamoDBv2.DataModel.DynamoDBContext.GetTargetTable(ItemStorageConfig storageConfig, DynamoDBFlatConfig flatConfig, DynamoDBConsumer consumer) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Services\DynamoDBv2\Custom\DataModel\ContextInternal.cs:line 111
   at Amazon.DynamoDBv2.DataModel.DynamoDBContext.SaveHelperAsync[T](T value, DynamoDBOperationConfig operationConfig, CancellationToken cancellationToken) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Services\DynamoDBv2\Custom\DataModel\Context.cs:line 280
   at Os33.RedHook.BackupService.Common.Storage.DynamoDbProvider.SaveItemAsync[T](T item) in C:\Users\mmatveev\source\repos\OS33.RedHook.Backup\Os33.RedHook.BackupService.Common\Storage\DynamoDbProvider.cs:line 35
   at Os33.RedHook.BackupService.IntTests.DynamoDbProviderTests.SaveItemsToTableQueryThenByKeyAndDelete() in C:\Users\mmatveev\source\repos\OS33.RedHook.Backup\tests\Os33.RedHook.BackupService.IntTests\DynamoDbProviderTests.cs:line 32
--- End of stack trace from previous location where exception was thrown ---
----- Inner Stack Trace -----
   at Amazon.Runtime.HttpWebRequestMessage.GetResponse() in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\HttpHandler\_mobile\HttpRequestMessageFactory.cs:line 503
   at Amazon.Runtime.Internal.HttpHandler`1.InvokeSync(IExecutionContext executionContext) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\HttpHandler\HttpHandler.cs:line 85
   at Amazon.Runtime.Internal.Unmarshaller.InvokeSync(IExecutionContext executionContext) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\Handlers\Unmarshaller.cs:line 50
   at Amazon.Runtime.Internal.ErrorHandler.InvokeSync(IExecutionContext executionContext) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\ErrorHandler\ErrorHandler.cs:line 72
Result Message: 
Amazon.DynamoDBv2.Model.ResourceNotFoundException : Requested resource not found: Table: TestItem not found
---- Amazon.Runtime.Internal.HttpErrorResponseException : Exception of type 'Amazon.Runtime.Internal.HttpErrorResponseException' was thrown.

Possible Solution

Fix in the \sdk\src\Services\DynamoDBv2\Custom\DataModel\Configs.cs:

        public DynamoDBFlatConfig(DynamoDBOperationConfig operationConfig, DynamoDBContextConfig contextConfig)
        {
            if (operationConfig == null)
                operationConfig = _emptyOperationConfig;
            if (contextConfig == null)
                contextConfig = _emptyContextConfig;

            bool consistentRead = operationConfig.ConsistentRead ?? contextConfig.ConsistentRead ?? false;
            bool skipVersionCheck = operationConfig.SkipVersionCheck ?? contextConfig.SkipVersionCheck ?? false;
            bool ignoreNullValues = operationConfig.IgnoreNullValues ?? contextConfig.IgnoreNullValues ?? false;
            string overrideTableName =
                !string.IsNullOrEmpty(operationConfig.OverrideTableName) ? operationConfig.OverrideTableName : string.Empty; // should be a value passed in DynamoDBOperationConfig from constructor

Steps to Reproduce (for bugs)

    public class DynamoDbProvider : IDisposable
    {
        private readonly string _tableName;
        private readonly IAmazonDynamoDB _client;

        public DynamoDbProvider(string tableName, IAmazonDynamoDB client)
        {
            _tableName = tableName;
            _client = client;
        }

        public async Task SaveItemAsync<T>(T item)
        {
            using (var context = GetDbContext())
            {
                await context.SaveAsync(item);
            }
        }

        public void Dispose()
        {
            _client.Dispose();
        }

        private DynamoDBContext GetDbContext()
        {
            var dynamoDbOperationConfig = new DynamoDBOperationConfig
            {
                OverrideTableName = _tableName,
            };
            return new DynamoDBContext(_client, dynamoDbOperationConfig);            
        }
    }

and the test

        private class TestItem
        {
            public string accountId { get; set; }
            public string region { get; set; }
            public string destinationRegion { get; set; }
            public string roleArn { get; set; }
        }
        [Fact]
        public async Task SaveItemsToTableQueryThenByKeyAndDelete()
        {
            using (var dbProvider = new DynamoDbProvider("real-test-item-table-name", new AmazonDynamoDBClient()))
            {
                var id = Guid.NewGuid();

                var item1 = new TestItem { accountId = $"acc-1-{id}", region = "reg-1", destinationRegion = "reg-2", roleArn = "arn-1" };
               await dbProvider.SaveItemAsync(item1);
           }
    }

Context

Workaround is to pass DynamoDBOperationConfig to each operation

Your Environment

AWSSDK.Core version used: 3.3.31.11
Operating System and version: win10
Visual Studio version: VS 2017 professional
Targeted .NET platform: .NET Core

.NET Core Info

.NET Core version used for development: .NET Core 2.1
.NET Core version installed in the environment where application runs: .NET Core SDK (reflecting any global.json): Version: 2.1.503 Commit: 4c506e0f35

Runtime Environment: OS Name: Windows Server 2016 Standard OS Version: 10.0.14393 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\2.1.503\

Host (useful for support): Version: 2.1.7 Commit: cca5d72d48

.NET Core SDKs installed: 1.1.9 [C:\Program Files\dotnet\sdk] 2.1.2 [C:\Program Files\dotnet\sdk] 2.1.200 [C:\Program Files\dotnet\sdk] 2.1.202 [C:\Program Files\dotnet\sdk] 2.1.300 [C:\Program Files\dotnet\sdk] 2.1.400 [C:\Program Files\dotnet\sdk] 2.1.402 [C:\Program Files\dotnet\sdk] 2.1.503 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed: Microsoft.AspNetCore.All 2.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.App 2.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 1.0.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 1.1.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.0.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]

klaytaybai commented 5 years ago

Thanks for reporting this. I was able to reproduce, and I'm looking into how to fix it.

klaytaybai commented 5 years ago

Hi @michaelmatveev, you can get this to work if you instead use


await context.SaveAsync(item, new DynamoDBOperationConfig
{
    OverrideTableName = _tableName,
});

klaytaybai commented 5 years ago

I'm concerned that a proper fix to change https://github.com/aws/aws-sdk-net/blob/b691e46e57a3e24477e6a5fa2e849da44db7002f/sdk/src/Services/DynamoDBv2/Custom/DataModel/Configs.cs#L298-L299 to use the contextConfig's value as a backup would be a breaking change. I'll keep this open until I get the team's opinion on this.

normj commented 4 years ago

I see this issue was closed by our new GitHub automation but I wanted to put some more context into this issue.

You are passing an instance of DynamoDBOperationConfig into the constructor of DynamoDBContext. The DynamoDBOperationConfig type is the one that has the OverrideTable property but DynamoDBContext takes in the base class DynamoDBContextConfig so it has no knowledge of the OverrideTable property. The DynamoDBOperationConfig type is meant to be used when calling the individual methods like the SaveAsync call you are doing so it can override that particular for the POCO being saved to a specific table. The terrible confusion is an artifact of probably a bad design on our part to use inheritance between `DynamoDBContextConfig and DynamoDBOperationConfig but at this point we can't change that without very significant breaking changes.

If you are looking for away to do global table mapping changes take a look at using the AWSConfigsDynamoDB.Context.AddMapping calls. Here is a snippet I did in a recent demo.

https://github.com/aws-samples/cloudmosaic-dotnet/blob/reinvent-2019/Application/API/CloudMosaic.API/Startup.cs#L36-L47

ashovlin commented 2 months ago

In our next major version of the SDK (development underway now, see #3209), we've separated the hierarchy between DynamoDBContextConfig and DynamoDBOperationConfig. You will no longer be able to pass an operation-level config in to the context-level constructor, where OverrideTableName is not honored.

var dynamoDbOperationConfig = new DynamoDBOperationConfig
{
    OverrideTableName = _tableName            
};

var context = new DynamoDBContext(_client, dynamoDbOperationConfig);  // no longer possible in V4

We didn't want to take a single override table name as an input to the context object, since one can reuse a context object with multiple tables. If you do want to override table names on an application-level, you can use the TableAlias or TypeMapping collections on AWSConfigsDynamoDB.Context.

aws / aws-sdk-net