search

OData / odata.net

ODataLib: Open Data Protocol - .NET Libraries and Frameworks
https://docs.microsoft.com/odata
Other
686 stars 349 forks source link

ArgumentException on FilterClause property. "System.ArgumentException: An item with the same key has already been added." #216

Closed Xiaodong431 closed 9 years ago

Xiaodong431 commented 9 years ago

Current web api version and odata version: WebApi: 5.2 OData: 6.7

The query doesn't have $filter operator, but when my code check Filter property like below code, the FilterClause property throws exception. This exception only occurs once in thousands of requests:

        var gridSelection =
            this.queryConverter.GetMtQuery(
                options.Filter == null ? null : options.Filter.FilterClause,
                options.OrderBy == null ? null : options.OrderBy.OrderByClause,
                options.Skip == null ? null : new int?(options.Skip.Value),
                options.Top == null ? null : new int?(options.Top.Value)
                );

Query URL:

https://api.bingads.microsoft.com/ODataApi/Advertiser/V2/Customers(16285205)/Accounts(45102217)/Keywords?startdate=06/04/2015&enddate=06/04/2015&$select=Id,Text,Bid,Status,DeliveryStatus,PerformanceMetrics,QualityScore,BidEstimate&$expand=Campaign($select=Id,Name),AdGroup($select=Id,Name)&$top=5&$orderby=PerformanceMetrics/Clicks+desc&

The detail exception:

System.ArgumentException: An item with the same key has already been added. at System.Collections.Generic.Dictionary2.Insert(TKey key, TValue value, Boolean add) at Microsoft.OData.Edm.EnumHelper.GetCachedValuesAndNames(IEdmEnumType enumType, UInt64[]& values, String[]& names, Boolean getValues, Boolean getNames) at Microsoft.OData.Edm.EnumHelper.TryParseEnum(IEdmEnumType enumType, String value, Boolean ignoreCase, Int64& parseResult) at Microsoft.OData.Core.UriParser.Parsers.EnumBinder.TryParseEnum(IEdmEnumType enumType, String value, ODataEnumValue& enumValue) at Microsoft.OData.Core.UriParser.Parsers.EnumBinder.TryBindIdentifier(String identifier, IEdmEnumTypeReference typeReference, IEdmModel modelWhenNoTypeReference, QueryNode& boundEnum) at Microsoft.OData.Core.UriParser.Parsers.DottedIdentifierBinder.BindDottedIdentifier(DottedIdentifierToken dottedIdentifierToken) at Microsoft.OData.Core.UriParser.Parsers.MetadataBinder.Bind(QueryToken token) at Microsoft.OData.Core.UriParser.Parsers.BinaryOperatorBinder.BindBinaryOperator(BinaryOperatorToken binaryOperatorToken) at Microsoft.OData.Core.UriParser.Parsers.MetadataBinder.Bind(QueryToken token) at Microsoft.OData.Core.UriParser.Parsers.BinaryOperatorBinder.GetOperandFromToken(BinaryOperatorKind operatorKind, QueryToken queryToken) at Microsoft.OData.Core.UriParser.Parsers.BinaryOperatorBinder.BindBinaryOperator(BinaryOperatorToken binaryOperatorToken) at Microsoft.OData.Core.UriParser.Parsers.MetadataBinder.Bind(QueryToken token) at Microsoft.OData.Core.UriParser.Parsers.BinaryOperatorBinder.GetOperandFromToken(BinaryOperatorKind operatorKind, QueryToken queryToken) at Microsoft.OData.Core.UriParser.Parsers.BinaryOperatorBinder.BindBinaryOperator(BinaryOperatorToken binaryOperatorToken) at Microsoft.OData.Core.UriParser.Parsers.MetadataBinder.Bind(QueryToken token) at Microsoft.OData.Core.UriParser.Parsers.BinaryOperatorBinder.GetOperandFromToken(BinaryOperatorKind operatorKind, QueryToken queryToken) at Microsoft.OData.Core.UriParser.Parsers.BinaryOperatorBinder.BindBinaryOperator(BinaryOperatorToken binaryOperatorToken) at Microsoft.OData.Core.UriParser.Parsers.MetadataBinder.Bind(QueryToken token) at Microsoft.OData.Core.UriParser.Parsers.BinaryOperatorBinder.GetOperandFromToken(BinaryOperatorKind operatorKind, QueryToken queryToken) at Microsoft.OData.Core.UriParser.Parsers.BinaryOperatorBinder.BindBinaryOperator(BinaryOperatorToken binaryOperatorToken) at Microsoft.OData.Core.UriParser.Parsers.MetadataBinder.Bind(QueryToken token) at Microsoft.OData.Core.UriParser.Parsers.FilterBinder.BindFilter(QueryToken filter) at Microsoft.OData.Core.UriParser.ODataQueryOptionParser.ParseFilterImplementation(String filter, ODataUriParserConfiguration configuration, IEdmType elementType, IEdmNavigationSource navigationSource) at Microsoft.OData.Core.UriParser.ODataQueryOptionParser.ParseFilter() at System.Web.OData.Query.FilterQueryOption.get_FilterClause() at Microsoft.BingAds.Api.Repositories.Validators.EntityQueryOptionsValidator1.Validate(ODataQueryOptions1 options) in d:\dbs\sh\adamt_n\0603_170258_2\cmd\2\private\Campaign\MT\Source\BingAds.Api\BingAds.Api.Repository\Validators\EntityQueryOptionsValidator.cs:line 46 at Microsoft.BingAds.Api.Service.Controllers.KeywordsController.<GetKeywords>d__0.MoveNext() in d:\dbs\sh\adamt_n\0603_170258_2\cmd\2\private\Campaign\MT\Source\BingAds.Api\BingAds.Api.Service\Controllers\KeywordsController.cs:line 55 --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Threading.Tasks.TaskHelpersExtensions.<CastToObject>d__31.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter1.GetResult() at System.Web.Http.Controllers.ApiControllerActionInvoker.<InvokeActionAsyncCore>d__0.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter1.GetResult() at System.Web.Http.Filters.ActionFilterAttribute.d5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Web.Http.Filters.ActionFilterAttribute.d5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.d0.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult() at System.Web.Http.Controllers.ActionFilterResult.d2.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult() at System.Web.Http.Controllers.ExceptionFilterResult.d__0.MoveNext(). Reserved 3: Reserved1:

karataliu commented 9 years ago

This could be reproduced with a model containing an enum type that contains two identical names, when parsing involves enum value, this exception would occur:

var model = new EdmModel();
var color = new EdmEnumType("ns", "color");
color.AddMember("s1", new EdmIntegerConstant(1));
color.AddMember("s1", new EdmIntegerConstant(2));
model.AddElement(color);
var pan = new EdmEntityType("ns", "pan");
pan.AddKeys( pan.AddStructuralProperty("id", EdmPrimitiveTypeKind.Int32, false));
pan.AddStructuralProperty("backColor", new EdmEnumTypeReference(color, false));
model.AddElement(pan);
IEnumerable<EdmError> errors;
// {AlreadyDefined : Each member name of an enum type must be unique. Enum member name 's1' is already defined.}
model.Validate(out errors);
var parser = new ODataQueryOptionParser(model, pan, null, new Dictionary<string, string> { { "$filter", "ns.color's1' eq backColor" } });
var filter = parser.ParseFilter();

This would cause: System.ArgumentException: An item with the same key has already been added. at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource) at System.Collections.Generic.Dictionary2.Insert(TKey key, TValue value, Boolean add) at System.Collections.Generic.Dictionary2.Add(TKey key, TValue value) at Microsoft.OData.Edm.EnumHelper.GetEnumValuesAndNames(IEdmEnumType enumType, UInt64[]& values, String[]& names, Boolean getValues, Boolean getNames) in EnumHelper.cs: line 273 at Microsoft.OData.Edm.EnumHelper.GetCachedValuesAndNames(IEdmEnumType enumType, UInt64[]& values, String[]& names, Boolean getValues, Boolean getNames) in EnumHelper.cs: line 253 at Microsoft.OData.Edm.EnumHelper.TryParseEnum(IEdmEnumType enumType, String value, Boolean ignoreCase, ref Int64 parseResult) in EnumHelper.cs: line 63 at Microsoft.OData.Core.UriParser.Parsers.EnumBinder.TryParseEnum(IEdmEnumType enumType, String value, ref ODataEnumValue enumValue) in EnumBinder.cs: line 128 at Microsoft.OData.Core.UriParser.Parsers.EnumBinder.TryBindIdentifier(String identifier, IEdmEnumTypeReference typeReference, IEdmModel modelWhenNoTypeReference, ref QueryNode boundEnum) in EnumBinder.cs: line 105 at Microsoft.OData.Core.UriParser.Parsers.EnumBinder.TryBindDottedIdentifierAsEnum(DottedIdentifierToken dottedIdentifierToken, SingleValueNode parent, BindingState state, ref QueryNode boundEnum) in EnumBinder.cs: line 50 at Microsoft.OData.Core.UriParser.Parsers.DottedIdentifierBinder.BindDottedIdentifier(DottedIdentifierToken dottedIdentifierToken) in DottedIdentifierBinder.cs: line 75 at Microsoft.OData.Core.UriParser.Parsers.MetadataBinder.BindCast(DottedIdentifierToken dottedIdentifierToken) in MetadataBinder.cs: line 289 at Microsoft.OData.Core.UriParser.Parsers.MetadataBinder.Bind(QueryToken token) in MetadataBinder.cs: line 176 at Microsoft.OData.Core.UriParser.Parsers.BinaryOperatorBinder.GetOperandFromToken(BinaryOperatorKind operatorKind, QueryToken queryToken) in BinaryOperatorBinder.cs: line 91 at Microsoft.OData.Core.UriParser.Parsers.BinaryOperatorBinder.BindBinaryOperator(BinaryOperatorToken binaryOperatorToken) in BinaryOperatorBinder.cs: line 53 at Microsoft.OData.Core.UriParser.Parsers.MetadataBinder.BindBinaryOperator(BinaryOperatorToken binaryOperatorToken) in MetadataBinder.cs: line 267 at Microsoft.OData.Core.UriParser.Parsers.MetadataBinder.Bind(QueryToken token) in MetadataBinder.cs: line 164 at Microsoft.OData.Core.UriParser.Parsers.FilterBinder.BindFilter(QueryToken filter) in FilterBinder.cs: line 52 at Microsoft.OData.Core.UriParser.ODataQueryOptionParser.ParseFilterImplementation(String filter, ODataUriParserConfiguration configuration, IEdmType elementType, IEdmNavigationSource navigationSource) in ODataQueryOptionParser.cs: line 281 at Microsoft.OData.Core.UriParser.ODataQueryOptionParser.ParseFilter() in ODataQueryOptionParser.cs: line 123 at Microsoft.Test.OData.Query.TDD.Tests.Semantic.FilterAndOrderByFunctionalTests.ParseEnumConstantOrderBy() in FilterAndOrderByFunctionalTests.cs: line 512

@Xiaodong431 Looks like this is due to using a wrong model (containing an enum type with duplicate member names) for Uri parsing. And such issues could be detected with model validation. I'd suggest conducting model validation and check whether duplicate member names exist in the model first, especially when you have the case with dynamic model building.

Xiaodong431 commented 9 years ago

My issue might not be caused by dynamic model or duplicate member names.

My filter type is a static enum type, not dynamic model. It doesn't have duplicate member names. I use totally 10 types of filter strings on thousands of queries. But only less than 0.01% query has this issue. The query string is actually like this (previous given query string was truncated):

https://api.bingads.microsoft.com/ODataApi/Advertiser/V2/Customers(16285205)/Accounts(45102217)/Keywords?startdate=06/04/2015&enddate=06/04/2015&$select=Id,Text,Bid,Status,DeliveryStatus,PerformanceMetrics,QualityScore,BidEstimate&$expand=Campaign($select=Id,Name),AdGroup($select=Id,Name)&$top=5&$orderby=PerformanceMetrics/Clicks+desc&$filter=(DeliveryStatus+eq+Enum.DeliveryStatus'AdGroupDraft'+or+DeliveryStatus+eq+Enum.DeliveryStatus'EditorialUnderAppeal'+or+DeliveryStatus+eq+Enum.DeliveryStatus'EditorialPending'+or+DeliveryStatus+eq+Enum.DeliveryStatus'CampaignUserPaused'+or+DeliveryStatus+eq+Enum.DeliveryStatus'AdGroupUserPaused')&$count=true

congysu commented 9 years ago

@Xiaodong431 How can we contact you? You can feel free to send an email to odatafeedback@microsoft.com. Thanks.

karataliu commented 9 years ago

Thanks for the information. Just found the following code which conducts operation on dictionary without lock caused this issue. https://github.com/OData/odata.net/blob/ae0dd29c1cf430255a8ec9c4225b4745e25cad64/src/Microsoft.OData.Edm/ExtensionMethods/EnumHelper.cs#L282-L298 Have added this to Backlog.

Xiaodong431 commented 9 years ago

Thanks for investigation. May I know which version of odata and webapi would the bug fix be shipped? We might need to upgrade the 2 framework when it is fixed.

You can contact me by xijian@microsoft.com for more detail.

congysu commented 9 years ago

@Xiaodong431 Can you help verify if it resolve the issue your encountered? https://www.nuget.org/packages/Microsoft.OData.Core/6.13.0-rc