Skip to content

Mark old IGraphQLBuilder extension methods obsolete and create new ones #602

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
Dec 27, 2021
Merged

Mark old IGraphQLBuilder extension methods obsolete and create new ones #602

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
bc14056
Mark old graphqlbuilder methods obsolete and create new ones
Shane32 Jul 19, 2021
93a7829
Merge remote-tracking branch 'origin/master' into builder2
Shane32 Jul 19, 2021
e964eb3
Update
Shane32 Jul 19, 2021
cc0c7ab
Update samples
Shane32 Jul 19, 2021
bab5d80
Update tests
Shane32 Jul 19, 2021
9bb8d5b
Update apiapprovals
Shane32 Jul 19, 2021
3467c7e
Update to latest beta version
Shane32 Aug 1, 2021
7eb458e
Fix sample
Shane32 Aug 1, 2021
014c99b
More updates
Shane32 Aug 1, 2021
bfed979
Update apiapprovals
Shane32 Aug 1, 2021
ce8d62c
Update readme
Shane32 Aug 1, 2021
59d99bf
Update GraphQL nuget packages
Shane32 Aug 18, 2021
2ba24ce
Merge branch 'master' into builder2
Shane32 Oct 30, 2021
6e4d190
Apply suggestions from code review
Shane32 Oct 30, 2021
6e793d8
Update
Shane32 Oct 30, 2021
2cbec77
Merge branch 'master' into builder2
Shane32 Nov 7, 2021
8ca601f
Merge branch 'master' into builder2
sungam3r Nov 28, 2021
81c2a39
Merge
Shane32 Dec 12, 2021
a80f23d
Mrege
Shane32 Dec 12, 2021
e3c0b8b
Merge branch 'master' into builder2
Shane32 Dec 26, 2021
ff555cf
Update src/Core/Core.csproj
Shane32 Dec 26, 2021
bffb064
Update src/Authorization.AspNetCore/Authorization.AspNetCore.csproj
Shane32 Dec 26, 2021
2373bc7
Updates
Shane32 Dec 27, 2021
8286d77
Fix package reference
Shane32 Dec 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@

[Oo]bj/
[Bb]in/

*.received.txt
24 changes: 15 additions & 9 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,14 @@ public void ConfigureServices(IServiceCollection services)
// Add GraphQL services and configure options
services
.AddSingleton<IChat, Chat>()
.AddSingleton<ChatSchema>()
.AddGraphQL((options, provider) =>
.AddSingleton<ChatSchema>();

MicrosoftDI.GraphQLBuilderExtensions.AddGraphQL(services)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently, this is a temporary inconvenience.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Although it is binary compatible it is probably not source compatible. There should be notes somewhere in here to that effect.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(That may happen when using GraphQL 4.6.1 regardless of the changes in this PR.)

Copy link
Member

@sungam3r sungam3r Dec 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be notes somewhere

I suggest to use GraphQL.NET documentation site as a single place for projects' documentation. I would to make new sidebar section for server project.
изображение

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

graphql-dotnet/graphql-dotnet#2817

.AddServer(true)
.ConfigureExecution(options =>
{
options.EnableMetrics = Environment.IsDevelopment();
var logger = provider.GetRequiredService<ILogger<Startup>>();
var logger = options.RequestServices.GetRequiredService<ILogger<Startup>>();
options.UnhandledExceptionDelegate = ctx => logger.LogError("{Error} occurred", ctx.OriginalException.Message);
})
// Add required services for GraphQL request/response de/serialization
Expand All @@ -140,7 +143,7 @@ public void ConfigureServices(IServiceCollection services)
.AddErrorInfoProvider(opt => opt.ExposeExceptionStackTrace = Environment.IsDevelopment())
.AddWebSockets() // Add required services for web socket support
.AddDataLoader() // Add required services for DataLoader support
.AddGraphTypes(typeof(ChatSchema)) // Add all IGraphType implementors in assembly which ChatSchema exists
.AddGraphTypes(typeof(ChatSchema).Assembly) // Add all IGraphType implementors in assembly which ChatSchema exists
}

public void Configure(IApplicationBuilder app)
Expand Down Expand Up @@ -177,11 +180,14 @@ public void ConfigureServices(IServiceCollection services)
services
.AddRouting()
.AddSingleton<IChat, Chat>()
.AddSingleton<ChatSchema>()
.AddGraphQL((options, provider) =>
.AddSingleton<ChatSchema>();

MicrosoftDI.GraphQLBuilderExtensions.AddGraphQL(services)
.AddServer(true)
.ConfigureExecution(options =>
{
options.EnableMetrics = Environment.IsDevelopment();
var logger = provider.GetRequiredService<ILogger<Startup>>();
var logger = options.RequestServices.GetRequiredService<ILogger<Startup>>();
options.UnhandledExceptionDelegate = ctx => logger.LogError("{Error} occurred", ctx.OriginalException.Message);
})
// It is required when both GraphQL HTTP and GraphQL WebSockets middlewares are mapped to the same endpoint (by default 'graphql').
Expand All @@ -192,7 +198,7 @@ public void ConfigureServices(IServiceCollection services)
.AddErrorInfoProvider(opt => opt.ExposeExceptionStackTrace = Environment.IsDevelopment())
.AddWebSockets() // Add required services for web socket support
.AddDataLoader() // Add required services for DataLoader support
.AddGraphTypes(typeof(ChatSchema)); // Add all IGraphType implementors in assembly which ChatSchema exists
.AddGraphTypes(typeof(ChatSchema).Assembly); // Add all IGraphType implementors in assembly which ChatSchema exists
}

public void Configure(IApplicationBuilder app)
Expand All @@ -202,7 +208,7 @@ public void Configure(IApplicationBuilder app)

// this is required for ASP.NET Core routing
app.UseRouting();

app.UseEndpoints(endpoints =>
{
// map websocket middleware for ChatSchema at default path /graphql
Expand Down
1 change: 1 addition & 0 deletions samples/Samples.Server/Samples.Server.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
<ItemGroup>
<PackageReference Include="Serilog.AspNetCore" Version="4.0.0" />
<PackageReference Include="Serilog.Sinks.Console" Version="4.0.1" />
<PackageReference Include="GraphQL.MicrosoftDI" Version="4.6.1" />
</ItemGroup>

<ItemGroup>
Expand Down
21 changes: 13 additions & 8 deletions samples/Samples.Server/Startup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System.Collections.Generic;
using GraphQL.DataLoader;
using GraphQL.Execution;
using GraphQL.Samples.Schemas.Chat;
using GraphQL.Server;
using GraphQL.Server.Ui.Altair;
Expand Down Expand Up @@ -29,20 +31,23 @@ public Startup(IConfiguration configuration, IWebHostEnvironment environment)
// This method gets called by the runtime. Use this method to add services to the container.
public void ConfigureServices(IServiceCollection services)
{
services
.AddSingleton<IChat, Chat>()
.AddSingleton<ChatSchema>()
.AddGraphQL((options, provider) =>
services.AddSingleton<IChat, Chat>();

MicrosoftDI.GraphQLBuilderExtensions.AddGraphQL(services)
.AddServer(true)
.AddSchema<ChatSchema>()
.ConfigureExecution(options =>
{
options.EnableMetrics = Environment.IsDevelopment();
var logger = provider.GetRequiredService<ILogger<Startup>>();
var logger = options.RequestServices.GetRequiredService<ILogger<Startup>>();
options.UnhandledExceptionDelegate = ctx => logger.LogError("{Error} occurred", ctx.OriginalException.Message);
})
.AddSystemTextJson(deserializerSettings => { }, serializerSettings => { })
.AddErrorInfoProvider<CustomErrorInfoProvider>(opt => opt.ExposeExceptionStackTrace = Environment.IsDevelopment())
.AddSystemTextJson()
.AddErrorInfoProvider<CustomErrorInfoProvider>()
.Configure<ErrorInfoProviderOptions>(opt => opt.ExposeExceptionStackTrace = Environment.IsDevelopment())
.AddWebSockets()
.AddDataLoader()
.AddGraphTypes(typeof(ChatSchema));
.AddGraphTypes(typeof(ChatSchema).Assembly);
}

// This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
Expand Down
23 changes: 14 additions & 9 deletions samples/Samples.Server/StartupWithRouting.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System.Collections.Generic;
using GraphQL.DataLoader;
using GraphQL.Execution;
using GraphQL.Samples.Schemas.Chat;
using GraphQL.Server;
using GraphQL.Server.Ui.Altair;
Expand Down Expand Up @@ -29,22 +31,25 @@ public StartupWithRouting(IConfiguration configuration, IWebHostEnvironment envi
// This method gets called by the runtime. Use this method to add services to the container.
public void ConfigureServices(IServiceCollection services)
{
services
.AddRouting()
.AddSingleton<IChat, Chat>()
.AddSingleton<ChatSchema>()
.AddGraphQL((options, provider) =>
services.AddRouting();
services.AddSingleton<IChat, Chat>();

MicrosoftDI.GraphQLBuilderExtensions.AddGraphQL(services)
.AddServer(true)
.AddSchema<ChatSchema>()
.ConfigureExecution(options =>
{
options.EnableMetrics = Environment.IsDevelopment();
var logger = provider.GetRequiredService<ILogger<Startup>>();
var logger = options.RequestServices.GetRequiredService<ILogger<Startup>>();
options.UnhandledExceptionDelegate = ctx => logger.LogError("{Error} occurred", ctx.OriginalException.Message);
})
.AddDefaultEndpointSelectorPolicy()
.AddSystemTextJson(deserializerSettings => { }, serializerSettings => { })
.AddErrorInfoProvider(opt => opt.ExposeExceptionStackTrace = Environment.IsDevelopment())
.AddSystemTextJson()
.AddErrorInfoProvider<CustomErrorInfoProvider>()
.Configure<ErrorInfoProviderOptions>(opt => opt.ExposeExceptionStackTrace = Environment.IsDevelopment())
.AddWebSockets()
.AddDataLoader()
.AddGraphTypes(typeof(ChatSchema));
.AddGraphTypes(typeof(ChatSchema).Assembly);
}

// This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
Expand Down
6 changes: 3 additions & 3 deletions src/All/All.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
<ProjectReference Include="..\Ui.Playground\Ui.Playground.csproj" />
<ProjectReference Include="..\Ui.Voyager\Ui.Voyager.csproj" />

<PackageReference Include="GraphQL.MemoryCache" Version="4.1.0" />
<PackageReference Include="GraphQL.MicrosoftDI" Version="4.1.0" />
<PackageReference Include="GraphQL.SystemReactive" Version="4.1.0" />
<PackageReference Include="GraphQL.MemoryCache" Version="4.6.1" />
<PackageReference Include="GraphQL.MicrosoftDI" Version="4.6.1" />
<PackageReference Include="GraphQL.SystemReactive" Version="4.6.1" />
</ItemGroup>

</Project>
33 changes: 33 additions & 0 deletions src/Authorization.AspNetCore/GraphQLBuilderAuthorizationExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#nullable enable

using System;
using GraphQL.Server.Authorization.AspNetCore;
using GraphQL.Validation;
Expand All @@ -14,6 +16,7 @@ public static class GraphQLBuilderAuthorizationExtensions
/// </summary>
/// <param name="builder">The GraphQL builder.</param>
/// <returns>Reference to the passed <paramref name="builder"/>.</returns>
[Obsolete]
public static IGraphQLBuilder AddGraphQLAuthorization(this IGraphQLBuilder builder)
=> builder.AddGraphQLAuthorization(options => { });

Expand All @@ -23,6 +26,7 @@ public static IGraphQLBuilder AddGraphQLAuthorization(this IGraphQLBuilder build
/// <param name="builder">The GraphQL builder.</param>
/// <param name="options">An action delegate to configure the provided <see cref="AuthorizationOptions"/>.</param>
/// <returns>Reference to the passed <paramref name="builder"/>.</returns>
[Obsolete]
public static IGraphQLBuilder AddGraphQLAuthorization(this IGraphQLBuilder builder, Action<AuthorizationOptions> options)
{
builder.Services.TryAddTransient<IClaimsPrincipalAccessor, DefaultClaimsPrincipalAccessor>();
Expand All @@ -33,5 +37,34 @@ public static IGraphQLBuilder AddGraphQLAuthorization(this IGraphQLBuilder build

return builder;
}

/// <summary>
/// Adds the GraphQL authorization.
/// </summary>
/// <param name="builder">The GraphQL builder.</param>
/// <returns>Reference to the passed <paramref name="builder"/>.</returns>
public static DI.IGraphQLBuilder AddGraphQLAuthorization(this DI.IGraphQLBuilder builder)
=> builder.AddGraphQLAuthorization(_ => { });

/// <summary>
/// Adds the GraphQL authorization.
/// </summary>
/// <param name="builder">The GraphQL builder.</param>
/// <param name="configure">An action delegate to configure the provided <see cref="AuthorizationOptions"/>.</param>
/// <returns>Reference to the passed <paramref name="builder"/>.</returns>
public static DI.IGraphQLBuilder AddGraphQLAuthorization(this DI.IGraphQLBuilder builder, Action<AuthorizationOptions>? configure)
{
if (!(builder is IServiceCollection services))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I see...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove : IServiceCollection from GraphQLBuilder and add that:

public interface IServiceCollectionProvider
{
    public IServiceCollection ServiceCollection { get; }
}

Then

internal class GraphQLBuilder : GraphQLBuilderBase, IServiceRegister, IServiceCollectionProvider
{
        public override IServiceRegister Services => this;

        public IServiceCollection ServiceCollection { get; }
}

+ remove

 int ICollection<ServiceDescriptor>.Count => ServiceCollection.Count;
        bool ICollection<ServiceDescriptor>.IsReadOnly => ServiceCollection.IsReadOnly;
        ServiceDescriptor IList<ServiceDescriptor>.this[int index] { get => ServiceCollection[index]; set => ServiceCollection[index] = value; }
        int IList<ServiceDescriptor>.IndexOf(ServiceDescriptor item) => ServiceCollection.IndexOf(item);
        void IList<ServiceDescriptor>.Insert(int index, ServiceDescriptor item) => ServiceCollection.Insert(index, item);
        void IList<ServiceDescriptor>.RemoveAt(int index) => ServiceCollection.RemoveAt(index);
        void ICollection<ServiceDescriptor>.Add(ServiceDescriptor item) => ServiceCollection.Add(item);
        void ICollection<ServiceDescriptor>.Clear() => ServiceCollection.Clear();
        bool ICollection<ServiceDescriptor>.Contains(ServiceDescriptor item) => ServiceCollection.Contains(item);
        void ICollection<ServiceDescriptor>.CopyTo(ServiceDescriptor[] array, int arrayIndex) => ServiceCollection.CopyTo(array, arrayIndex);
        bool ICollection<ServiceDescriptor>.Remove(ServiceDescriptor item) => ServiceCollection.Remove(item);
        IEnumerator<ServiceDescriptor> IEnumerable<ServiceDescriptor>.GetEnumerator() => ServiceCollection.GetEnumerator();
        IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable)ServiceCollection).GetEnumerator();

Then cast builder.Services to IServiceCollectionProvider

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine (for v5 obviously). This is the 3rd option I mentioned. It will require a dependency on GraphQL.MicrosoftDI from the server project. Since it only works for Microsoft DI, the dependency makes sense.

GraphQLBuilder should remain public so that if someone wants to add additional initialization code to RegisterDefaultServices, they don't need to rewrite the entire class. There's no reason to make it internal/private.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But... then project needs a reference to IServiceCollectionProvider... So... Let's just move : IServiceCollection from IGraphQLBuilder to IServiceRegister.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If OK I'll do a PR to graphql-net/develop

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GraphQLBuilder should remain public so that if someone wants to add additional initialization code to RegisterDefaultServices, they don't need to rewrite the entire class. There's no reason to make it internal/private.

OK.

It will require a dependency on GraphQL.MicrosoftDI from the server project.

I think we will be able to go without it.

throw new NotSupportedException("This method only supports the MicrosoftDI implementation of IGraphQLBuilder.");
services.TryAddTransient<IClaimsPrincipalAccessor, DefaultClaimsPrincipalAccessor>();
services.AddHttpContextAccessor();
if (configure != null)
services.AddAuthorizationCore(configure);
else
services.AddAuthorizationCore();
builder.AddValidationRule<AuthorizationValidationRule>();

return builder;
}
}
}
65 changes: 65 additions & 0 deletions src/Core/BasicGraphQLExecuter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
using System;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using GraphQL.Instrumentation;
using GraphQL.Types;
using Microsoft.Extensions.Options;

namespace GraphQL.Server
{
public class BasicGraphQLExecuter<TSchema> : IGraphQLExecuter<TSchema>
where TSchema : ISchema
{
public TSchema Schema { get; }

private readonly IDocumentExecuter _documentExecuter;
private readonly GraphQLOptions _options;

public BasicGraphQLExecuter(
TSchema schema,
IDocumentExecuter documentExecuter,
IOptions<GraphQLOptions> options)
{
Schema = schema;

_documentExecuter = documentExecuter;
_options = options.Value;
}

public virtual async Task<ExecutionResult> ExecuteAsync(string operationName, string query, Inputs variables, IDictionary<string, object> context, IServiceProvider requestServices, CancellationToken cancellationToken = default)
{
var start = DateTime.UtcNow;

var options = GetOptions(operationName, query, variables, context, requestServices, cancellationToken);
var result = await _documentExecuter.ExecuteAsync(options);

if (options.EnableMetrics)
{
result.EnrichWithApolloTracing(start);
}

return result;
}

protected virtual ExecutionOptions GetOptions(string operationName, string query, Inputs variables, IDictionary<string, object> context, IServiceProvider requestServices, CancellationToken cancellationToken)
{
var opts = new ExecutionOptions
{
Schema = Schema,
OperationName = operationName,
Query = query,
Inputs = variables,
UserContext = context,
CancellationToken = cancellationToken,
ComplexityConfiguration = _options.ComplexityConfiguration,
EnableMetrics = _options.EnableMetrics,
UnhandledExceptionDelegate = _options.UnhandledExceptionDelegate,
MaxParallelExecutionCount = _options.MaxParallelExecutionCount,
RequestServices = requestServices,
};

return opts;
}
}
}
4 changes: 2 additions & 2 deletions src/Core/Core.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

<ItemGroup>
<!--TODO: remove SystemReactive in v6 -->
<PackageReference Include="GraphQL.SystemReactive" Version="4.1.0" />
<PackageReference Include="GraphQL.DataLoader" Version="4.1.0" />
<PackageReference Include="GraphQL.SystemReactive" Version="4.6.1" />
<PackageReference Include="GraphQL.DataLoader" Version="4.6.1" />
<PackageReference Include="Microsoft.Extensions.Options" Version="3.1.0" />
</ItemGroup>

Expand Down
1 change: 1 addition & 0 deletions src/Core/DefaultGraphQLExecuter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace GraphQL.Server
{
[Obsolete]
public class DefaultGraphQLExecuter<TSchema> : IGraphQLExecuter<TSchema>
where TSchema : ISchema
{
Expand Down
Loading