Proto/Swagger HTTP JSON to RPC mapping

[psprings]

Problem

The current HTTP JSON to RPC mapping does not seem to generate proper string replacement logic, so the HTTP request will always result in a 404.

Example

The following would yield a 404

grafeasCfg := &grafeasAPI.Configuration{
    BasePath:      "http://localhost:8080",
    DefaultHeader: make(map[string]string),
    UserAgent:     "Swagger-Codegen/0.1.0/go",
}
grafeasClient := grafeasAPI.NewAPIClient(grafeasCfg)
projectID := "projects/myproject"
notesResp, _, err := grafeasClient.GrafeasV1Beta1Api.ListNotes(context.Background(), projectID, nil)
if err != nil {
    log.Fatal(err)
} else {
    log.Print(notesResp.Notes)
}

While a curl against that same project would yield a 200

$ curl http://localhost:8080/v1beta1/projects/myproject/notes
{"notes":[],"nextPageToken":""}

Proto generated client

The following shows the broken string replacement logic

// create path and map variables
localVarPath := a.client.cfg.BasePath + "/v1beta1/{parent=projects/*}/notes"
localVarPath = strings.Replace(localVarPath, "{"+"parent"+"}", fmt.Sprintf("%v", parent), -1)

Reference to code

Derived from: https://github.com/grafeas/grafeas/blob/6a8d995912a9f10f732e8ffcffbae8830507ed17/proto/v1beta1/grafeas.proto#L121

Simulated

parent := "projects/myproject"
localVarPath := "http://localhost:8080" + "/v1beta1/{parent=projects/*}/notes"
fmt.Println("[CURRENT]  BEFORE REPLACEMENT:", localVarPath)
localVarPath = strings.Replace(localVarPath, "{"+"parent"+"}", fmt.Sprintf("%v", parent), -1)
fmt.Println("[CURRENT]  AFTER REPLACEMENT :", localVarPath)
localVarPath = "http://localhost:8080" + "/v1beta1/{parent}/notes"
fmt.Println("[PROPOSED] BEFORE REPLACEMENT:", localVarPath)
localVarPath = strings.Replace(localVarPath, "{"+"parent"+"}", fmt.Sprintf("%v", parent), -1)
fmt.Println("[PROPOSED] AFTER REPLACEMENT :", localVarPath)
// [CURRENT]  BEFORE REPLACEMENT: http://localhost:8080/v1beta1/{parent=projects/*}/notes
// [CURRENT]  AFTER REPLACEMENT : http://localhost:8080/v1beta1/{parent=projects/*}/notes
// [PROPOSED] BEFORE REPLACEMENT: http://localhost:8080/v1beta1/{parent}/notes
// [PROPOSED] AFTER REPLACEMENT : http://localhost:8080/v1beta1/projects/myproject/notes

Solution

I haven't had time to test yet but changing

rpc ListNotes(ListNotesRequest) returns (ListNotesResponse) {
    option (google.api.http) = {
      get: "/v1beta1/{parent=projects/*}/notes"
    };
  };

to

rpc ListNotes(ListNotesRequest) returns (ListNotesResponse) {
    option (google.api.http) = {
      get: "/v1beta1/{parent}/notes"
    };
  };

should work

[aysylu]

Hi @psprings, thanks so much for filing this issue and for the fix proposal! The underlying issue here is that OpenAPI used by Swagger and GoogleAPI are structured differently. There has been a lot of work to address these issues, but it's still WIP. I think the best way to address this specific issue is to modify the generated library code. Would you be willing to contribute this fix?

[psprings]

@aysylu sorry, didn't see the response until late. I'd be happy to PR a fix!

[aysylu]

@psprings no problem! Looking forward to your PR.:)

[psprings]

@aysylu things have been a little busy, but I had a little bit of time the last night or two to dig into this a little bit.

I threw together an idea for how to modify the spec file(s) as part of the go generate in #15 whenever you have a chance to take a look

I'd be interested in discussing generated folder structure, the inclusion/exclusion of the project.swagger.json file, and if this would make sense to integrate into an upstream CI process (in which case the actual "generation" of the client would occur whenever a tag is created) here or on the PR.