Skip to content
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.

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

gRPC transcoding public API #40792

Closed
JamesNK opened this issue Mar 21, 2022 · 3 comments · Fixed by #42422
Closed

gRPC transcoding public API #40792

JamesNK opened this issue Mar 21, 2022 · 3 comments · Fixed by #42422
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-grpc Includes: GRPC wire-up, templates
Milestone

Comments

@JamesNK
Copy link
Member

JamesNK commented Mar 21, 2022

Background and Motivation

gRPC transcoding allows apps to expose gRPC services as if they were RESTful APIs.

Proposed API

namespace Microsoft.AspNetCore.Grpc.JsonTranscoding
{
    public sealed class GrpcJsonTranscodingOptions
    {
        public GrpcJsonSettings JsonSettings { get; set; }
        public Google.Protobuf.Reflection.TypeRegistry TypeRegistry { get; set; }
    }

    public sealed class GrpcJsonSettings
    {
        public bool WriteIndented { get; set; } = false;
        public bool WriteEnumsAsIntegers { get; set; } = false;
        public bool WriteInt64AsStrings { get; set; } = false;
        public bool SkipDefaultValues { get; set; } = false;
    }

    public sealed class GrpcJsonTranscodingMetadata
    {
        public GrpcTranscodingMetadata(Google.Protobuf.Reflection.MethodDescriptor methodDescriptor, Google.Api.HttpRule httpRule);

        public Google.Api.HttpRule HttpRule { get; }
        public Google.Protobuf.Reflection.MethodDescriptor MethodDescriptor { get; }
    }
}

namespace Microsoft.Extensions.DependencyInjection
{
    public static class GrpcTranscodingServiceExtensions
    {
        public static IGrpcServerBuilder AddJsonTranscoding(this IGrpcServerBuilder builder);
        public static IGrpcServerBuilder AddJsonTranscoding(this IGrpcServerBuilder builder, Action<GrpcJsonTranscodingOptions> configureOptions);
    }
}

Usage Examples

// Old and busted
services.AddGrpc(options =>
    {
        options.EnableDetailedErrors = true;
    });
services.AddGrpcJsonTranscoding(options =>
    {
        options.JsonSettings.WriteIndented = true;
    });

// New hotness
services
    .AddGrpc(options => options.EnableDetailedErrors = true)
    .AddJsonTranscoding(options => options.JsonSettings.WriteIndented = true);

// Maybe future?
services.AddGrpc()
   .AddJsonTranscoding()
   .AddJsonTranscoding<GreeterService>(options => options.JsonSettings.WriteIndented = true);

Alternative Designs

Risks

@JamesNK JamesNK added area-grpc Includes: GRPC wire-up, templates api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Mar 21, 2022
@JamesNK JamesNK self-assigned this Mar 21, 2022
@JamesNK JamesNK added api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Mar 21, 2022
@ghost
Copy link

ghost commented Mar 21, 2022

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@JamesNK
Copy link
Member Author

JamesNK commented Mar 21, 2022

Note: Will review in a separate meeting.

@halter73
Copy link
Member

halter73 commented Mar 21, 2022

Api review:

  • Change *Transcoding* -> *JsonTranscoding* in all APIs, namespaces and package names.
  • Can TypeRegistry go on the main options type?
  • Can we expose JsonSerializerOptions on JsonSettings?
    • Not for now
  • JsonSettings -> GrpcJsonSettings
  • WriteDefaultValues -> IgnoreDefaultValues (or SkipDefaultValues)
  • Make extension method on IGrpcServerBuilder instead of IServiceCollection
+ namespace Microsoft.AspNetCore.Grpc.JsonTranscoding;
+
+ public sealed class GrpcJsonTranscodingOptions
+ {
+     public GrpcJsonSettings JsonSettings { get; set; }
+     public Google.Protobuf.Reflection.TypeRegistry TypeRegistry { get; set; }
+ }
+ 
+ public sealed class GrpcJsonSettings
+ {
+     public bool WriteIndented { get; set; } = false;
+     public bool WriteEnumsAsIntegers { get; set; } = false;
+     public bool WriteInt64AsStrings { get; set; } = false;
+     public bool IgnoreDefaultValues { get; set; } = false;
+ }
+ 
+ public sealed class GrpcJsonTranscodingMetadata
+ {
+     public GrpcTranscodingMetadata(Google.Protobuf.Reflection.MethodDescriptor methodDescriptor, Google.Api.HttpRule httpRule);
+ 
+     public Google.Api.HttpRule HttpRule { get; }
+     public Google.Protobuf.Reflection.MethodDescriptor MethodDescriptor { get; }
+ }
+ 
+ namespace Microsoft.Extensions.DependencyInjection;
+
+ public static class GrpcTranscodingServiceExtensions
+ {
+     public static IGrpcServerBuilder AddJsonTranscoding(this IGrpcServerBuilder builder);
+     public static IGrpcServerBuilder AddJsonTranscoding(this IGrpcServerBuilder builder, Action<GrpcJsonTranscodingOptions> configureOptions);
+ }

@halter73 halter73 added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Mar 21, 2022
@rafikiassumani-msft rafikiassumani-msft added this to the 7.0-preview5 milestone Apr 21, 2022
@dotnet dotnet locked as resolved and limited conversation to collaborators Jul 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-grpc Includes: GRPC wire-up, templates
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants