Support Optimistic Concurrency
benhysell opened this issue · 4 comments
Edit - removed pull request, updated issue here.
Hack MS SQL Test project again just to quickly show issue.
Attempting to support optimistic concurrency:
Midway down the page - Add an Optimistic Concurrency Property to the Department Entity
The tldr, label one of your fields with [Timestamp] with the type of byte[] and SQL will ensure optimistic concurrency.
In the hacked ms sql test project I added
[Timestamp]
public byte[] ServerTimestamp { get; set; }
To User, and tried to run LookupSingleEntity(), test throws an exception trying to InitializeUserSchema when I add the ServerTimestamp field.
Stack trace:
System.IndexOutOfRangeException : Index was outside the bounds of the array.
at GraphQL.Net.GraphQLField.NewInternal[TArgs](GraphQLSchema schema, String name, Func2 exprFunc, Type fieldCLRType, Delegate mutationFunc) in C:\Jobs\graphql-net\GraphQL.Net\GraphQLField.cs:line 72 at GraphQL.Net.GraphQLField.New[TContext,TArgs](GraphQLSchema schema, String name, Func
2 exprFunc, Type fieldCLRType, Action2 mutationFunc) in C:\Jobs\graphql-net\GraphQL.Net\GraphQLField.cs:line 65 at GraphQL.Net.GraphQLTypeBuilder
2.AddFieldInternal[TArgs,TField](String name, Func2 exprFunc, Action
2 mutation) in C:\Jobs\graphql-net\GraphQL.Net\GraphQLTypeBuilder.cs:line 39
at GraphQL.Net.GraphQLTypeBuilder2.AddField[TArgs,TField](String name, Func
2 exprFunc) in C:\Jobs\graphql-net\GraphQL.Net\GraphQLTypeBuilder.cs:line 31
at GraphQL.Net.GraphQLTypeBuilder2.AddField[TArgs,TField](String name, TArgs shape, Func
2 exprFunc) in C:\Jobs\graphql-net\GraphQL.Net\GraphQLTypeBuilder.cs:line 22
at GraphQL.Net.GraphQLTypeBuilder2.AddField[TField](String name, Expression
1 expr) in C:\Jobs\graphql-net\GraphQL.Net\GraphQLTypeBuilder.cs:line 87
at GraphQL.Net.GraphQLTypeBuilder2.AddField[TField](Expression
1 expr) in C:\Jobs\graphql-net\GraphQL.Net\GraphQLTypeBuilder.cs:line 72
at Tests.EF.EntityFrameworkMsSqlTests.InitializeUserSchema(GraphQLSchema`1 schema) in C:\Jobs\graphql-net\Tests.EF\EntityFrameworkMsSqlTests.cs:line 76
at Tests.EF.EntityFrameworkMsSqlTests.CreateDefaultContext() in C:\Jobs\graphql-net\Tests.EF\EntityFrameworkMsSqlTests.cs:line 61
at Tests.EF.EntityFrameworkMsSqlTests.LookupSingleEntity() in C:\Jobs\graphql-net\Tests.EF\EntityFrameworkMsSqlTests.cs:line 128
Hi Ben,
Thanks for reporting this. I suspect the problem lies in detecting whether the field type is a list or not, which we do here: https://github.com/ckimes89/graphql-net/blob/818a9ac736ef924f6661a520c6f9cc037357b863/GraphQL.Net/GraphQLField.cs#L70-L72
byte[]
is assignable to IEnumerable<>
, but it doesn't have a generic type parameter so fieldCLRType.GetGenericArguments()[0]
fails. The logic is also incorrect in this case since we don't really want to consider byte[]
to be a list type. I think we should change this logic to require that the field type be generic before it can be considered a list type. This would also cover strings (which are assignable to IEnumerable<char>
), so we can remove the explicit check for the string type.
Regarding issues vs. pull requests, it should suffice to report an issue with a bit of example code and a stack trace. For example, I was able to pinpoint this problem with just the information in your PR description. If a lot of code is useful to the discussion, then a PR or just a link to the branch in your repo would be good.
I usually think of PRs as just a modification request, so I was confused by the first one you sent yesterday. It made a lot more sense once I got that it was used to show example code.
I've updated NuGet to version 0.2.2 which should include the fix for this. Let me know if it doesn't work.
Works like a charm! Much thanks!