protobuf-net / protobuf-net.Grpc

GRPC bindings for protobuf-net and grpc-dotnet
Other
868 stars 114 forks source link

Generated .proto from CodeFirst service at runtime #5

Closed pengweiqhca closed 2 years ago

mgravell commented 5 years ago

I'm going to infer a lot here, so... tell me if I'm wrong, but it sounds like you want there to exist some API akin to GetProto/GetSchema in protobuf-net that will allow you to obtain the .proto for a service (along with its dependent messages), via an API presumably originating with the type/types of the service-contracts, i.e.

string proto = Something.GetProto(typeof(IMyService));

is that correct? If so: this is already planned - needs a little work here, but shouldn't be too arduous.

pengweiqhca commented 5 years ago

Should be this, really looking forward to this.

ChristianWeyer commented 4 years ago

Hi @mgravell ,

are you still planning on adding this cool feature? ;-) Thanks!

mgravell commented 4 years ago

Hi @ChristianWeyer - absolutely, yes; this is definitely something I want to do; the challenge is always competing demands for time, etc

ChristianWeyer commented 4 years ago

Great. Can we be of any help?

mgravell commented 4 years ago

In a way you already have been - I kinda rely on noise from people to prioritize, so knowing that more people are needing this helps. I have an unrelated pending PR on protobuf-net that changes (in a compatible way, obvs) the API shape of the schema generator API, so maybe this is a good time for me to try and do this.

For clarity: is there any particular timescale /scenario / etc you're looking for here? And sample scenarios (test cases, effectively) are always helpful.

On Wed, 1 Jul 2020, 20:58 Christian Weyer, notifications@github.com wrote:

Great. Can we be of any help?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/protobuf-net/protobuf-net.Grpc/issues/5#issuecomment-652617466, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEHMH7NVUW7UNI5BWUT33RZOINTANCNFSM4H3O777A .

ChristianWeyer commented 4 years ago

Doing code-first with gRPC for e.g. Blazor WASM applications may indeed boost some parts of client dev productivity. But then, for documentation, test tools, interop etc. we may still need the .protos.

So, while not having a hard timeline or even deadline - I Think having this feature would definitely help to make the story for client web dev with Blazor WASM and .NET Core backends a real nice story in terms of productivity.

What do you think?

mgravell commented 4 years ago

Total agreement; simply time (ultimately, this is one of several "evenings and weekends" things) On the Blazor WASM front: that also ties in with the AOT plans via "generators", so we can prune the ref-emit code

ChristianWeyer commented 4 years ago

Awesome, then we agree ;-)

jfalameda commented 4 years ago

This is definitely a desirable feature for us. In our case, we use BloomRPC to test our endpoints (Postman for protobuf). Not having proto files makes it harder to develop and test. Also, as mentioned ^ up there, it makes it easy to integrate with other languages.

mgravell commented 4 years ago

OK; I have a first stab at the protobuf-net half of the code for this, in the "namespaces" branch; I have a plan to add the gRPC half of this into the new "reflection" package that is in #63 (probably adding the new csproj "now" rather than at merge) - the key thing being that right now I only plan to support this in v3 - IMO it is ok for the "reflection" package to have a hard v3 package reference, but the main lib should stay v2 for now. Example output:

syntax = "proto3";
package mypackage;
import "protobuf-net/protogen.proto"; // custom protobuf-net options
import "google/protobuf/timestamp.proto";
import "google/protobuf/duration.proto";
import "google/protobuf/empty.proto";

message Bar {
   string Value = 1;
}
message Foo {
   .google.protobuf.Timestamp When = 1;
   string Id = 2; // default value could not be applied: 00000000-0000-0000-0000-000000000000
   oneof subtype {
      option (.protobuf_net.oneofopt).isSubType = true;
      Bar Bar = 42;
   }
}
service myService {
   rpc unary (.google.protobuf.Timestamp) returns (.google.protobuf.Empty);
   rpc clientStreaming (Foo) returns (stream Foo);
   rpc serverStreaming (stream Foo) returns (.google.protobuf.Duration);
   rpc fullDuplex (stream Foo) returns (stream Foo);
}

Effectively, protobuf-net.Grpc would have the responsibility for preparing the service contract metadata, then calling into protobuf-net (which knows about data contract metadata).

TFTomSun commented 4 years ago

I am interested in that, too! ... just to create more noise ;-)

Can you please provide a link to the current implementation of the generator? I would like to see whether i could contribute.

mgravell commented 4 years ago

@ChristianWeyer

Can we be of any help?

Kind of tangential, but if you have a positive experience with these tools: maybe take a moment to say so? Your personal and company voice has a lot of respect in relevant circles here, and while MS is happy pushing WCF folks towards gRPC, they're almost entirely silent on anything that isn't the MS approach. Which is tragic when the transition from code-first WCF to code-first gRPC can be so much easier.

And if you have a negative experience, maybe say that too! Except maybe tell me first so I can fix the problem 😜 (like I'm trying with the above)

mgravell commented 4 years ago

An initial version of this is implemented by #108, and is available in version 1.0.110, which should be on NuGet shortly.

Requires protobuf-net.Grpc.Reflection, which in turn takes a protobuf-net v3 dependency (normally we only insist on v2)

Usage is:

var generator = new SchemaGenerator(); // optional controls on here, we can add more add needed
var schema = generator.GetSchema<IMyService>(); // there is also a non-generic overload that takes Type

Let me know how it goes! We can tweak this, etc

mgravell commented 4 years ago

@ChristianWeyer this related new thing may also be along your interests - think "mex" in WCF terms

ChristianWeyer commented 4 years ago

Kind of tangential, but if you have a positive experience with these tools: maybe take a moment to say so? Your personal and company voice has a lot of respect in relevant circles here, and while MS is happy pushing WCF folks towards gRPC, they're almost entirely silent on anything that isn't the MS approach. Which is tragic when the transition from code-first WCF to code-first gRPC can be so much easier.

And if you have a negative experience, maybe say that too! Except maybe tell me first so I can fix the problem 😜 (like I'm trying with the above)

Yes, I will write a blog post about using Blazuor WASM with gRPC in the coming weeks and will surely address protobuf-net.Grpc. I can hardly imagine .NET devs writing tons of .proto files in the first place 😇

Thanks for all the hard work you do @mgravell ! ✌🏼

ChristianWeyer commented 4 years ago

An initial version of this is implemented by #108, and is available in version 1.0.110, which should be on NuGet shortly.

Requires protobuf-net.Grpc.Reflection, which in turn takes a protobuf-net v3 dependency (normally we only insist on v2)

Usage is:

var generator = new SchemaGenerator(); // optional controls on here, we can add more add needed
var schema = generator.GetSchema<IMyService>(); // there is also a non-generic overload that takes Type

Let me know how it goes! We can tweak this, etc

Cool, thanks for the hint. I just tried this with a very simple contract:

[ServiceContract]
public interface IConferencesService
{
    Task<IEnumerable<ConferenceOverview>> ListConferencesAsync();
}

And this was the result:

syntax = "proto3";
package Shared.Contracts;
import "protobuf-net/bcl.proto"; // schema for protobuf-net's handling of core .NET types
import "google/protobuf/empty.proto";

message ConferenceOverview {
   .bcl.Guid ID = 1; // default value could not be applied: 00000000-0000-0000-0000-000000000000
   string Title = 2;
}
service ConferencesService {
   rpc ListConferences (IEnumerable_ConferenceOverview) returns (.google.protobuf.Empty);
}

... hm, I think there is room for improvement... 😉

Thanks.

ChristianWeyer commented 4 years ago

@ChristianWeyer this related new thing may also be along your interests - think "mex" in WCF terms

Hehehe, things are coming back. Great stuff. Thanks.

mgravell commented 4 years ago

Re the snafu:

image

was fixed over the weekend; I'll get that deployed, along with the gRPC bits that rev the libs

Re the Guid: I strongly advise, if possible, taking a protobuf-net v3 dependency and using

[CompatibilityLevel(CompatibilityLevel(CompatibilityLevel.Level300)]

or higher; this changes how a few common types like Guid are encoded, and will make it much easier to work in a cross-platform way. You can apply [CompatibilityLevel] at the assembly, module, type, or member, but for green-field things: assembly or module is the simplest. See here for more info.

(edit: d'oh, silly me; if you're using this API, you already have a v3 dependency, so: should be simple)

(edit edit: I wonder if I should add a hint on CompatibilityLevel to the .proto output)

mgravell commented 4 years ago
Task<IEnumerable<ConferenceOverview>> ListConferencesAsync();

heh, I'm actually kinda surprised that one worked at all :) but: two ways to improve this today: @ChristianWeyer

  1. use IAsyncEnumerable<T>

    IAsyncEnumerable<ConferenceOverview> ListConferencesAsync();
  2. send a message the has a set:

    Task<ListConferencesResult> ListConferencesAsync();

    where:

    [DataContract]
    public class ListConferencesResult
    {
        [ProtoMember(1)]
        public List<ConferenceOverview> Conferences {get;} = new List<ConferenceOverview>();
    }

Note that protobuf-net is silently doing "2" under the covers, but it has no good way of inventing a name, so that's where IEnumerable_ConferenceOverview came from; it should have included an invented message definition for IEnumerable_ConferenceOverview though - I'll add a test and get that fixed.

Added a test locally, and for:

        [ServiceContract]
        public interface IConferencesService
        {
            Task<IEnumerable<ConferenceOverview>> ListConferencesEnumerable();
            IAsyncEnumerable<ConferenceOverview> ListConferencesAsyncEnumerable();
            Task<ListConferencesResult> ListConferencesWrapped();
        }

I currently get:

syntax = "proto3";
package protobuf_net.Grpc.Reflection.Test;
import "google/protobuf/empty.proto";

message ConferenceOverview {
   string ID = 1; // default value could not be applied: 00000000-0000-0000-0000-000000000000
   string Title = 2;
}
message ListConferencesResult {
   repeated ConferenceOverview Conferences = 1;
}
service ConferencesService {
   rpc ListConferencesAsyncEnumerable (.google.protobuf.Empty) returns (stream ConferenceOverview);
   rpc ListConferencesEnumerable (.google.protobuf.Empty) returns (IEnumerable_ConferenceOverview);
   rpc ListConferencesWrapped (.google.protobuf.Empty) returns (ListConferencesResult);
}

I will investigate where the missing IEnumerable_ConferenceOverview declaration has gone, but fundamentally: it looks just like ListConferencesResult.

mgravell commented 4 years ago

treatment of IEnumerable<T> is resolved in protobuf-net here; new output:

syntax = "proto3";
package protobuf_net.Grpc.Reflection.Test;
import "google/protobuf/empty.proto";

message ConferenceOverview {
   string ID = 1; // default value could not be applied: 00000000-0000-0000-0000-000000000000
   string Title = 2;
}
message IEnumerable_ConferenceOverview {
   repeated ConferenceOverview items = 1;
}
service ConferencesService {
   rpc ListConferencesEnumerable (.google.protobuf.Empty) returns (IEnumerable_ConferenceOverview);
}
ChristianWeyer commented 4 years ago

Just updated to the latest packages and now get this:

image

This is the contract code:

[ServiceContract]
    public interface IConferencesService
    {
        Task<IEnumerable<ConferenceOverview>> ListConferencesAsync();
        Task<ConferenceDetails> GetConferenceDetailsAsync(Guid id);
        Task<ConferenceDetails> AddNewConferenceAsync(ConferenceDetails conference);
    }
mgravell commented 4 years ago

You're like a fun maze of corner cases :)

The "make everything simple" fix here is to use a DTO for all inputs and outputs. In a lot of cases, it simply won't allow you to use a primitive as an input/output, but Guid happens to fall into an obscure intersection of things that look transmittable. Now I need to figure out what the "correct" (or at least, most predictable) behaviour is here. Let me think on that.

mgravell commented 4 years ago

Thankfully, I haven't yet merged the PR related to value-type parameters and multiple parameters, so the problem scenarios (sending Guid or decimal) do not currently work. This is fortunate as it means we can make it work properly!

So, my proposal is:

Doing this conveniently may require me to add [CompatibilityLevel] in the 2.4 branch of protobuf-net, but it would only go up to level 200 there.

mgravell commented 4 years ago

Oh and to emphasize: the "today" fix is - send a DTO.

Kurounin commented 4 years ago

@mgravell I noticed that when you override the name in the ServiceContract (e.g. [ServiceContract(Name = "CustomName")]) the generated proto still contains package namespace, even though the RPC route is simply /CustomName/MethodName.

To get it working correctly I added the namespace in the name, so it reads like [ServiceContract(Name = "namespace.CustomName")] which generates matching routes /namespace.CustomName/MethodName.

JohnGalt1717 commented 4 years ago

Using this package it appears to work well, however optional doesn't appear to be generated. (i.e. optional string xxx = 1) as I would expect.

JohnGalt1717 commented 4 years ago

Also, It would be nice if you could specify multiple services at once (i.e. array of types) and have it generate all of the shared protos etc in one or merge all of it together into a single file. Often you'll have multiple services, each with a service contract, and this will work right now, but protoc freaks out if you do all of the files generated or if you do them one after another it will create dupicates of the exact same shared DataContracts.

mgravell commented 4 years ago

Are you generating proto2 or proto3? Proto3 doesn't use "optional", except in the still-experimental bits.

On Wed, 19 Aug 2020, 21:00 James Hancock, notifications@github.com wrote:

Also, It would be nice if you could specify multiple services at once (i.e. array of types) and have it generate all of the shared protos etc in one or merge all of it together into a single file. Often you'll have multiple services, each with a service contract, and this will work right now, but protoc freaks out if you do all of the files generated or if you do them one after another it will create dupicates of the exact same shared DataContracts.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/protobuf-net/protobuf-net.Grpc/issues/5#issuecomment-676631982, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEHMFO76L6XOUZCXBT26LSBQVPFANCNFSM4H3O777A .

JohnGalt1717 commented 4 years ago

@mgravell proto3. Was hoping that it would generate the optional so that I could use experimental features.

But my biggest issue is having multiple services duplicates shared DataContracts which causes havoc while using protoc to generate clients like Dart etc. because it thinks that they're different contracts because they're in different files. It really needs to allow taking multiple ServiceContracts and then generate protos where there is a messages.proto and then a proto for each service that references the messages.proto that was created so that it eliminates the duplication.

mgravell commented 4 years ago

the current protobuf-net parser knows about "optional" in proto3, but I have not updated the schema generator; as for the other bit: yes, I have a plan there to add a better (more controlled) way of specifying origins for external protos - on the list.

JohnGalt1717 commented 4 years ago

@mgravell So what do you think about making this a dotnet global tool that can take assemblies that you want to gen from and spit out protos? (and allow for it later to handle generating full clients in other languages as plugins)

The reason I ask, is that it would be nice to be able to create a build event that generated the protos. (I'd really love to have it generate swagger-grpc too and have the protos downloadable on demand)

bjorkstromm commented 4 years ago

@mgravell So what do you think about making this a dotnet global tool that can take assemblies that you want to gen from and spit out protos? (and allow for it later to handle generating full clients in other languages as plugins)

The reason I ask, is that it would be nice to be able to create a build event that generated the protos. (I'd really love to have it generate swagger-grpc too and have the protos downloadable on demand)

IIRC @mgravell talked about integrating this with protogen, which IMO is an excellent in idea.

ThaDaVos commented 3 years ago

I tried testing above too, but before running the code I am already getting build errors as after adding the package ProtoBuf.Grpc.Reflection or the ASP.NET Core version, the System.ServiceModel package/namespace seems to have lost it's references to ServiceContract so I end up with this after the package:

image

While before it was: image

The difference, no red squirlies of missing class references - am I using the wrong namespace or did something strange happen?

Or should I be doing the reflection/generation part in a separate project? (Still need to split everything up eventually though)

mgravell commented 3 years ago

You're missing https://www.nuget.org/packages/System.ServiceModel.Primitives

On Thu, 29 Apr 2021, 23:16 ThaDaVos, @.***> wrote:

I tried testing above too, but before running the code I am already getting build errors as after adding the package ProtoBuf.Grpc.Reflection or the ASP.NET Core version, the System.ServiceModel package/namespace seems to have lost it's references to ServiceContract so I end up with this after the package: [image: image] https://user-images.githubusercontent.com/5251028/116624850-fa7a7180-a948-11eb-9e5b-034a45a55441.png

While before it was: [image: image] https://user-images.githubusercontent.com/5251028/116624983-28f84c80-a949-11eb-90ae-82d065e69203.png

The difference, no red squirlies of missing class references - am I using the wrong namespace or did something strange happen?

Or should I be doing the reflection/generation part in a separate project? (Still need to split everything up eventually though)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/protobuf-net/protobuf-net.Grpc/issues/5#issuecomment-829631410, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEHMCGBMEBE5FLGOPLCX3TLHLE7ANCNFSM4H3O777A .

ThaDaVos commented 3 years ago

Thanks @mgravell - that fixes it

murugaratham commented 3 years ago

Couldn't find any relevant stackoverflow discussion, but is there anywhere that document how do i get the generated .proto file?

I have existing web apis that i am exploring on enabling grpc and wanted to try out milkman or any quick tools that developers can shorten the dev loop

ThaDaVos commented 3 years ago

@murugaratham - the only documentation as far as I know there is, is this comment: https://github.com/protobuf-net/protobuf-net.Grpc/issues/5#issuecomment-653854482

I did it as follows: image

bjorkstromm commented 3 years ago

Couldn't find any relevant stackoverflow discussion, but is there anywhere that document how do i get the generated .proto file?

I have existing web apis that i am exploring on enabling grpc and wanted to try out milkman or any quick tools that developers can shorten the dev loop

If you use gRPC Server Reflection, then there are tools that can work with that. Check out grpcurl or grpcui. I've also written a simple .NET Global tool for this extracting .proto files from gRPC Server Reflection enabled services, Checkout dotnet-grpc-cli if you're interested in that.

Enabling gRPC Server reflection to your protobuf-net.Grpc enabled service is straight-forward.

Install NuGet package. https://www.nuget.org/packages/protobuf-net.Grpc.AspNetCore.Reflection/

Register services. https://github.com/protobuf-net/protobuf-net.Grpc/blob/2060c3ce9618a53d1e3cf5e4af75ae9c0d6b9e44/examples/pb-net-grpc/Server_CS/Startup.cs#L22

Map the reflection endpoint. https://github.com/protobuf-net/protobuf-net.Grpc/blob/2060c3ce9618a53d1e3cf5e4af75ae9c0d6b9e44/examples/pb-net-grpc/Server_CS/Startup.cs#L43

If you just want the .proto files generated, add the code that @ThaDaVos suggested somewhere in startup behind a flag.