Azure / azure-cosmos-table-dotnet

.NET SDK for Azure Cosmos Table API
14 stars 6 forks source link

TableEntityAdapter thorws Null Reference when <T> doesn't have a property in the underlying table #23

Closed fuzzzerd closed 4 years ago

fuzzzerd commented 4 years ago

We are continuously addressing and improving the SDK. Please describe your issue along with the following information:

** SDK version

Confirmed in Nuget Package

** Issue reproduce steps

  1. Create a POCO model class and a ModelAdapter class that inherits TableEntityAdatper.
  2. Add a field to the underlying Azure table that does not exist on the model
  3. Run a TableQuery<ModelAdapter> query and note the null reference

** Whether the issue is consistent or sporadic

Consistent.

** Environment Summary

ASP.NET Core, Azure App Service

** Any other context, such as stack trace or log.

NullReferenceException: Object reference not set to an instance of an object.

Microsoft.Azure.Cosmos.Table.EntityPropertyConverter.SetProperty(object root, string propertyPath, object propertyValue, EntityPropertyConverterOptions entityPropertyConverterOptions, OperationContext operationContext) StorageException: Object reference not set to an instance of an object.

Microsoft.Azure.Cosmos.Table.RestExecutor.TableCommand.Executor.ExecuteAsync(RESTCommand cmd, IRetryPolicy policy, OperationContext operationContext, CancellationToken token)

NullReferenceException: Object reference not set to an instance of an object.

Microsoft.Azure.Cosmos.Table.EntityPropertyConverter.SetProperty(object root, string propertyPath, object propertyValue, EntityPropertyConverterOptions entityPropertyConverterOptions, OperationContext operationContext) Microsoft.Azure.Cosmos.Table.EntityPropertyConverter+<>cDisplayClass4_0.b0(T current, KeyValuePair<string, EntityProperty> kvp) System.Linq.Enumerable.Aggregate<TSource, TAccumulate>(IEnumerable source, TAccumulate seed, Func<TAccumulate, TSource, TAccumulate> func) Microsoft.Azure.Cosmos.Table.EntityPropertyConverter.ConvertBack(IDictionary<string, EntityProperty> flattenedEntityProperties, EntityPropertyConverterOptions entityPropertyConverterOptions, OperationContext operationContext) Microsoft.Azure.Cosmos.Table.TableEntityAdapter.ReadEntity(IDictionary<string, EntityProperty> properties, OperationContext operationContext) Microsoft.Azure.Cosmos.Table.EntityUtilities.ResolveEntityByType(string partitionKey, string rowKey, DateTimeOffset timestamp, IDictionary<string, EntityProperty> properties, string etag) Microsoft.Azure.Cosmos.Table.RestExecutor.TableCommand.TableOperationHttpResponseParsers.ReadAndResolve(string etag, Dictionary<string, object> props, Func<string, string, DateTimeOffset, IDictionary<string, EntityProperty>, string, T> resolver, TableRequestOptions options) Microsoft.Azure.Cosmos.Table.RestExecutor.TableCommand.TableOperationHttpResponseParsers.TableQueryPostProcessGenericAsync<TElement, TQueryType>(Stream responseStream, Func<string, string, DateTimeOffset, IDictionary<string, EntityProperty>, string, TElement> resolver, HttpResponseMessage resp, TableRequestOptions options, OperationContext ctx, CancellationToken cancellationToken) Microsoft.Azure.Cosmos.Table.RestExecutor.TableCommand.TableQueryRESTCommandGenerator+<>cDisplayClass1_0<TInput, TResult>+<b2>d.MoveNext() Microsoft.Azure.Cosmos.Table.RestExecutor.TableCommand.Executor.ProcessEndOfRequestAsync(ExecutionState executionState, CancellationToken cancellationToken) Microsoft.Azure.Cosmos.Table.RestExecutor.TableCommand.Executor.ExecuteAsync(RESTCommand cmd, IRetryPolicy policy, OperationContext operationContext, CancellationToken token)

Unsure if related, but seems very similar to this issue: https://github.com/Azure/azure-storage-net/issues/659

PaulCheng commented 4 years ago

What's your sdk version, can you please share repro code snippet?

PaulCheng commented 4 years ago

This is the same known issue as the post referenced. The proposed fix was never merged into master. We will re-triage this bug during the next release. If your Model class can extend TableEntity, the missing property scenario is handled properly.

fuzzzerd commented 4 years ago

What's your sdk version,

.NET Core v3.1.100

can you please share repro code snippet?

Yes, I'm using a TableEntityAdapter and not inheriting TableEntity.

public class DataModel  // poco, currently not using TableEntity
{
    public string Name { get; set; }
    // an so on
}
public class DataModelAdapter : TableEntityAdapter<DataModel>
{
    public DataModelAdapter() { }

    public DataModelAdapter(string partitionKey, DataModel model) : base(model)
    {
        this.PartitionKey = BuildPartitionKey(partitionKey);
        this.RowKey = model.Name
    }

    public static string BuildPartitionKey(string partitionKey)
    {
        return $"{partitionKey}";
    }
}

// simple case, just query by partition and row key, the query does not seem to impact the result
var partitionKeyFilter = TableQuery.CombineFilters(
    TableQuery.GenerateFilterCondition("PartitionKey", QueryComparisons.Equal, partitionKey),
    TableOperators.And,
    TableQuery.GenerateFilterCondition("RowKey", QueryComparisons.Equal, rowKey)
);
var query = new TableQuery<DataModelAdapter>().Where(partitionKeyFilter);
var results = await _table.ExecuteQuerySegmentedAsync(query, null);

In this case, if the underlying Azure Table has only one field called Name everything works fine; however, as mentioned if another field is added to the table, this calling code blows up unless DataModel has a matching property.

In this particular case, I do have the ability to change my base model to inherit TableEntity but I have future cases where that will not be possible, as my application will not be defining those models.

thanga91 commented 4 years ago

Any idea when this will be resolved ?

sivethe commented 4 years ago

Triaged and accepted for the v1.0.9 release

PaulCheng commented 4 years ago

This had been addressed in v1.0.7. That said, TableEntityAdapter is not the usage pattern we want to support going forward. We'll mark it as obsolete in v.1.0.9 and remove in v2.0.0+.