gogo / letmegrpc

[maintainer wanted] generates a web form gui from a grpc specification
BSD 3-Clause "New" or "Revised" License
419 stars 48 forks source link

protoc-gen-gofast generates different names #25

Open tgulacsi opened 8 years ago

tgulacsi commented 8 years ago

db_dealer.zip protoc-gen-gofast generates DbDealer_UtolsoMod from the proto name db_dealer__utolso_mod.

The protoc-gen-letmegrpc plugin uses the names from the proto unchanged. I've tried to replicate what gofast does (vanity and vanity/command) but seems a little bit magic, and when it worked, it did not compile, missed the "Serve" function from the html plugin.

Please help me sort this out!

package main

import (
    "strings"

    "github.com/gogo/letmegrpc/html"
    "github.com/gogo/protobuf/proto"
    "github.com/gogo/protobuf/protoc-gen-gogo/generator"
    "github.com/gogo/protobuf/vanity/command"
)

func main() {
    req := command.Read()
    //files := req.GetProtoFile()
    //files = vanity.FilterFiles(files, vanity.NotGoogleProtobufDescriptorProto)
    //vanity.ForEachFile(files, vanity.TurnOnMarshalerAll)
    //vanity.ForEachFile(files, vanity.TurnOnSizerAll)
    //vanity.ForEachFile(files, vanity.TurnOnUnmarshalerAll)

    //resp := command.Generate(req)
    //command.Write(resp)

    gen := generator.New()
    gen.Request = req
    gen.CommandLineParameters(gen.Request.GetParameter())
    gen.WrapTypes()
    gen.SetPackageNames()
    gen.BuildTypeNameMap()
    gen.GeneratePlugin(html.New())
        gen.Response = resp

    for i := 0; i < len(gen.Response.File); i++ {
        gen.Response.File[i].Name = proto.String(strings.Replace(*gen.Response.File[i].Name, ".pb.go", ".letmegrpc.go", -1))
    }

    // Send back the results.
    command.Write(gen.Response)
}
awalterschulze commented 8 years ago

Ok so letmegrpc does not call the CamelCase function. Basically if you find the place where this function should be called you can fix it properly.

The easiest fix though is just to change you message name to match the camel cased name.

tgulacsi commented 8 years ago

I went back to proto-gen-go, and read & implemented the Protocol Buffers Style Guide (CamelCase for message and service names, snake_case for field names), and now letmegrpc starts flawlessly, it's only complaint is a missing GZIPDecompressor - I'll try to add that.

But very possible that the error is not in letmegrpc, but protoc-gen-gofast (gogoprotobuf), as it handled non-CamelCase names very inconsistently...

awalterschulze commented 8 years ago

protoc-gen-gofast should generate almost exactly the same code protoc-gen-go Could you maybe show me a the diff between the two code generators' generated code and the source proto file?

tgulacsi commented 8 years ago

I haven't said that protoc-gen-go produced different code, just that I went back to it to get as close to a working example as I can.

I've included the generated .pb.go in db_dealer.zip in the first comment - for example, db_dealer__utolso_mod becomes "/db_dealer.db_dealerutolso_mod/DbDealer_UtolsoMod" in its Handler's UnaryServerInfo, but in the Client, Invoke calls "/db_dealer.db_dealerutolso_mod/db_dealer__utolso_mod", which does not match...

As I'm generating the .proto file, the change to use CamelCase is not a problem.

awalterschulze commented 8 years ago

How did you go back to get as close to a working example as possible? Did you change your message names? Why did you need to go back to protoc-gen-go vs protoc-gen-gofast?

tgulacsi commented 8 years ago
  1. I've replaced protoc-gen-gofast with protoc-gen-go, with no help,
  2. changed service and message names to be CamelCase - this helped.
  3. now I've tried protoc-gen-gofast, and this works, too!

Now I created a minimal example .proto file, and you're right, gofast and go both create the same naming inconsistencies: Client Invokes "/db_dealer.db_dealer/db_dealer__utolso_mod", but Handler uses "/db_dealer.db_dealer/DbDealer_UtolsoMod" in UnaryServerInfo.FullMethod. db_dealer-proto.zip

awalterschulze commented 8 years ago

Ok so now that you've contributed do you think you are ready to fix this as well?

tgulacsi commented 8 years ago

Aye aye, Sir! PTAL https://github.com/gogo/protobuf/issues/203

Walter Schulze notifications@github.com ezt írta (időpont: 2016. szept. 24., Szo, 11:39):

Ok so now that you've contributed do you think you are ready to fix this as well?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/gogo/letmegrpc/issues/25#issuecomment-249355738, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPoSnJRiQ3EkN0jlhL_JZ0XlLa1uGFtks5qtO-4gaJpZM4KBj2u .

awalterschulze commented 8 years ago

I think this fix should happen in letmegrpc and not in gogoprotobuf.

tgulacsi commented 8 years ago

But that inconsistency is in gogoprotobuf's gprc code generator!

awalterschulze commented 8 years ago

This code is consistently being generated in the same way by go_out, gofast_out and gogo_out