Skip to content

Commit

Permalink
Fix duplicated filter/sorting type names (#2530)
Browse files Browse the repository at this point in the history
  • Loading branch information
PascalSenn committed Nov 4, 2020
1 parent e65e07a commit 60c8fb9
Show file tree
Hide file tree
Showing 23 changed files with 464 additions and 142 deletions.
17 changes: 7 additions & 10 deletions src/HotChocolate/Core/src/Types/Types/NamedTypeBase.cs
Expand Up @@ -81,23 +81,20 @@ public Type RuntimeType
_directives = directives;
}

protected void SetTypeIdentity(Type typeDefinition)
protected void SetTypeIdentity(Type typeDefinitionOrIdentity)
{
if (typeDefinition is null)
if (typeDefinitionOrIdentity is null)
{
throw new ArgumentNullException(nameof(typeDefinition));
throw new ArgumentNullException(nameof(typeDefinitionOrIdentity));
}

if (!typeDefinition.IsGenericTypeDefinition)
if (!typeDefinitionOrIdentity.IsGenericTypeDefinition)
{
throw new ArgumentException(
"The type definition must be a generic type definition.",
nameof(typeDefinition));
TypeIdentity = typeDefinitionOrIdentity;
}

if (RuntimeType != typeof(object))
else if (RuntimeType != typeof(object))
{
TypeIdentity = typeDefinition.MakeGenericType(RuntimeType);
TypeIdentity = typeDefinitionOrIdentity.MakeGenericType(RuntimeType);
}
}
}
Expand Down
22 changes: 13 additions & 9 deletions src/HotChocolate/Data/src/Data/Filters/FilterInputType.cs
Expand Up @@ -40,6 +40,18 @@ public FilterInputType(Action<IFilterInputTypeDescriptor> configure)
return descriptor.CreateDefinition();
}

protected override void OnRegisterDependencies(
ITypeDiscoveryContext context,
InputObjectTypeDefinition definition)
{
base.OnRegisterDependencies(context, definition);
if (definition is FilterInputTypeDefinition { EntityType: { } } filterDefinition)
{
SetTypeIdentity(typeof(FilterInputType<>)
.MakeGenericType(filterDefinition.EntityType));
}
}

protected virtual void Configure(IFilterInputTypeDescriptor descriptor)
{
}
Expand All @@ -66,7 +78,7 @@ protected virtual void Configure(IFilterInputTypeDescriptor descriptor)
{
fields.Add(new AndField(context.DescriptorContext, def.Scope));
}

if (definition is FilterInputTypeDefinition { UseOr: true } defOr)
{
fields.Add(new OrField(context.DescriptorContext, defOr.Scope));
Expand All @@ -85,14 +97,6 @@ protected virtual void Configure(IFilterInputTypeDescriptor descriptor)
}
}

protected override void OnRegisterDependencies(
ITypeDiscoveryContext context,
InputObjectTypeDefinition definition)
{
base.OnRegisterDependencies(context, definition);
SetTypeIdentity(typeof(FilterInputType<>));
}

// we are disabling the default configure method so
// that this does not lead to confusion.
protected sealed override void Configure(
Expand Down
Expand Up @@ -129,7 +129,9 @@ public IFilterOperationFieldDescriptor Operation(int operationId)
if (fieldDescriptor is null)
{
fieldDescriptor = FilterOperationFieldDescriptor.New(
Context, operationId, Definition.Scope);
Context,
operationId,
Definition.Scope);
Operations.Add(fieldDescriptor);
}

Expand Down Expand Up @@ -160,7 +162,9 @@ public IFilterInputTypeDescriptor Ignore(int operationId)
if (fieldDescriptor is null)
{
fieldDescriptor = FilterOperationFieldDescriptor.New(
Context, operationId, Definition.Scope);
Context,
operationId,
Definition.Scope);
Operations.Add(fieldDescriptor);
}

Expand All @@ -177,7 +181,9 @@ public IFilterInputTypeDescriptor Ignore(NameString name)
if (fieldDescriptor is null)
{
fieldDescriptor = FilterFieldDescriptor.New(
Context, name, Definition.Scope);
Context,
name,
Definition.Scope);
Fields.Add(fieldDescriptor);
}

Expand Down
Expand Up @@ -46,21 +46,28 @@ public class QueryableFilterInterceptor
argumentVisitor(valueNode, filterInputType, false);

Expression instance = context.PopInstance();
if (filterContext.Errors.Count == 0 &&
filterContext.TryCreateLambda(out LambdaExpression? expression))
if (filterContext.Errors.Count == 0)
{
context.PushInstance(
Expression.Call(
typeof(Enumerable),
nameof(Enumerable.Where),
new[] { filterInputType.EntityType.Source },
instance,
(Expression)expression));
if (filterContext.TryCreateLambda(out LambdaExpression? expression))
{
context.PushInstance(
Expression.Call(
typeof(Enumerable),
nameof(Enumerable.Where),
new[] { filterInputType.EntityType.Source },
instance,
(Expression)expression));
}
else
{
context.PushInstance(instance);
}
}
else
{
context.PushInstance(
Expression.Constant(Array.CreateInstance(filterInputType.RuntimeType, 0)));
Expression.Constant(
Array.CreateInstance(filterInputType.EntityType.Source, 0)));
context.ReportError(
ProjectionProvider_CouldNotProjectFiltering(valueNode));
}
Expand Down
Expand Up @@ -53,7 +53,8 @@ public class QueryableSortInterceptor
else
{
context.PushInstance(
Expression.Constant(Array.CreateInstance(sortInputType.RuntimeType, 0)));
Expression.Constant(
Array.CreateInstance(sortInputType.EntityType.Source, 0)));
context.ReportError(
ProjectionProvider_CouldNotProjectSorting(valueNode));
}
Expand Down
20 changes: 12 additions & 8 deletions src/HotChocolate/Data/src/Data/Sorting/SortInputType.cs
Expand Up @@ -44,6 +44,18 @@ protected virtual void Configure(ISortInputTypeDescriptor descriptor)
{
}

protected override void OnRegisterDependencies(
ITypeDiscoveryContext context,
InputObjectTypeDefinition definition)
{
base.OnRegisterDependencies(context, definition);
if (definition is SortInputTypeDefinition { EntityType: { } } sortDefinition)
{
SetTypeIdentity(
typeof(SortInputType<>).MakeGenericType(sortDefinition.EntityType));
}
}

protected override void OnCompleteType(
ITypeCompletionContext context,
InputObjectTypeDefinition definition)
Expand Down Expand Up @@ -71,14 +83,6 @@ protected virtual void Configure(ISortInputTypeDescriptor descriptor)
}
}

protected override void OnRegisterDependencies(
ITypeDiscoveryContext context,
InputObjectTypeDefinition definition)
{
base.OnRegisterDependencies(context, definition);
SetTypeIdentity(typeof(SortInputType<>));
}

// we are disabling the default configure method so
// that this does not lead to confusion.
protected sealed override void Configure(
Expand Down
Expand Up @@ -140,7 +140,9 @@ public ISortInputTypeDescriptor Ignore(NameString name)
if (fieldDescriptor is null)
{
fieldDescriptor = SortFieldDescriptor.New(
Context, name, Definition.Scope);
Context,
name,
Definition.Scope);
Fields.Add(fieldDescriptor);
}

Expand Down
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using System.Linq;
using HotChocolate.Data.Filters;
Expand Down Expand Up @@ -248,11 +249,12 @@ public void FilterInputType_Should_ThrowException_WhenNoConventionIsRegistered()
{
// arrange
ISchemaBuilder builder = SchemaBuilder.New()
.AddQueryType(c =>
c.Name("Query")
.Field("foo")
.Resolve(new List<Foo>())
.UseFiltering("Foo"));
.AddQueryType(
c =>
c.Name("Query")
.Field("foo")
.Resolve(new List<Foo>())
.UseFiltering("Foo"));

// act
// assert
Expand All @@ -265,18 +267,32 @@ public void FilterInputType_Should_ThrowException_WhenNoConventionIsRegisteredDe
{
// arrange
ISchemaBuilder builder = SchemaBuilder.New()
.AddQueryType(c =>
c.Name("Query")
.Field("foo")
.Resolve(new List<Foo>())
.UseFiltering());
.AddQueryType(
c =>
c.Name("Query")
.Field("foo")
.Resolve(new List<Foo>())
.UseFiltering());

// act
// assert
SchemaException exception = Assert.Throws<SchemaException>(() => builder.Create());
exception.Message.MatchSnapshot();
}

[Fact]
public void FilterInputType_Should_UseCustomFilterType_When_Nested()
{
// arrange
ISchemaBuilder builder = SchemaBuilder.New()
.AddFiltering()
.AddQueryType<UserQueryType>();

// act
// assert
builder.Create().Print().MatchSnapshot();
}

public class FooDirectiveType
: DirectiveType<FooDirective>
{
Expand Down Expand Up @@ -333,5 +349,34 @@ public class Author
[GraphQLNonNullType]
public string Name { get; set; }
}

public class User
{
public int Id { get; set; }

public string Name { get; set; } = default!;

public List<User> Friends { get; set; } = default!;
}

public class UserFilterInputType : FilterInputType<User>
{
protected override void Configure(IFilterInputTypeDescriptor<User> descriptor)
{
descriptor.Ignore(x => x.Id);
}
}

public class UserQueryType : ObjectType<User>
{
protected override void Configure(IObjectTypeDescriptor<User> descriptor)
{
descriptor.Name(nameof(Query));
descriptor
.Field("foo")
.Resolve(new List<User>())
.UseFiltering<UserFilterInputType>();
}
}
}
}
@@ -0,0 +1,54 @@
schema {
query: Query
}

type Query {
foo(where: UserFilterInput): [Query]
id: Int!
name: String!
friends: [Query!]!
}

input ListFilterInputOfFilterInputTypeOfUserFilterInput {
all: UserFilterInput
none: UserFilterInput
some: UserFilterInput
any: Boolean
}

input StringOperationFilterInput {
and: [StringOperationFilterInput!]
or: [StringOperationFilterInput!]
eq: String
neq: String
contains: String
ncontains: String
in: [String]
nin: [String]
startsWith: String
nstartsWith: String
endsWith: String
nendsWith: String
}

input UserFilterInput {
and: [UserFilterInput!]
or: [UserFilterInput!]
name: StringOperationFilterInput
friends: ListFilterInputOfFilterInputTypeOfUserFilterInput
}

"The `@defer` directive may be provided for fragment spreads and inline fragments to inform the executor to delay the execution of the current fragment to indicate deprioritization of the current fragment. A query with `@defer` directive will cause the request to potentially return multiple responses, where non-deferred data is delivered in the initial response and data deferred is delivered in a subsequent response. `@include` and `@skip` take precedence over `@defer`."
directive @defer("If this argument label has a value other than null, it will be passed on to the result of this defer directive. This label is intended to give client applications a way to identify to which fragment a deferred result belongs to." label: String "Deferred when true." if: Boolean) on FRAGMENT_SPREAD | INLINE_FRAGMENT

"The @deprecated directive is used within the type system definition language to indicate deprecated portions of a GraphQL service’s schema,such as deprecated fields on a type or deprecated enum values."
directive @deprecated("Deprecations include a reason for why it is deprecated, which is formatted using Markdown syntax (as specified by CommonMark)." reason: String = "No longer supported") on FIELD_DEFINITION | ENUM_VALUE

"Directs the executor to include this field or fragment only when the `if` argument is true."
directive @include("Included when true." if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT

"Directs the executor to skip this field or fragment when the `if` argument is true."
directive @skip("Skipped when true." if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT

"The `@stream` directive may be provided for a field of `List` type so that the backend can leverage technology such as asynchronous iterators to provide a partial list in the initial response, and additional list items in subsequent responses. `@include` and `@skip` take precedence over `@stream`."
directive @stream("If this argument label has a value other than null, it will be passed on to the result of this stream directive. This label is intended to give client applications a way to identify to which fragment a streamed result belongs to." label: String "The initial elements that shall be send down to the consumer." initialCount: Int! "Streamed when true." if: Boolean!) on FIELD
@@ -1,19 +1,30 @@
{
"errors": [
{
"message": "Unexpected Execution Error",
"locations": [
{
"line": 3,
"column": 29
}
],
"path": [
"root"
]
}
],
"data": {
"root": null
"root": [
{
"foo": {
"objectSet": [
{
"foo": {
"barString": "a",
"barShort": 12
}
}
]
}
},
{
"foo": {
"objectSet": [
{
"foo": {
"barString": "d",
"barShort": 14
}
}
]
}
}
]
}
}

0 comments on commit 60c8fb9

Please sign in to comment.