grpc-gateway icon indicating copy to clipboard operation
grpc-gateway copied to clipboard

protoc-gen-swagger: No way to specify 0 as minimum on floating point values

Open goodspark opened this issue 6 years ago • 7 comments

Steps you follow to reproduce the error:

  1. Generate a swagger file from the following proto file:
syntax = "proto3";
package mytest;
import "protoc-gen-swagger/options/annotations.proto";
service MyTest {
  rpc MyTest (MyTestRequest) returns (MyTestResponse);
}
message MyTestRequest {}
message MyTestResponse {
  double value = 1  [(grpc.gateway.protoc_gen_swagger.options.openapiv2_field) = {
    minimum: 0,
    maximum: 36.22,
  }];
}

Basically I want to specify that value is valid for the [0, 36.22] range.

The JSON (snippet) output:

"mytest.MyTestResponse": {
    "properties": {
        "value": {
            "format": "double", 
            "maximum": 36.22, 
            "type": "number"
        }
    }, 
    "type": "object"
}

What did you expect to happen instead:

I expect the JSON file to look like:

"mytest.MyTestResponse": {
    "properties": {
        "value": {
            "format": "double", 
            "minimum": 0.0,
            "maximum": 36.22, 
            "type": "number"
        }
    }, 
    "type": "object"
}

The intent here is to be able to pass this to a tool that supports swagger and have it properly display that the minimum is 0.

What's your theory on why it isn't working:

I suspect since 0 is a "default" value, protoc-gen-swagger can't tell the difference between unset and set-to-default. And thus the minimum gets excluded.

goodspark avatar Aug 12 '19 02:08 goodspark

This is a really interesting bug. I think your analysis is correct, but I'm not sure I understand why. The proto extension documentation includes this helpful missive:

Protocol Buffers also allows you to define and use your own options. This is an advanced feature which most people don't need. If you do think you need to create your own options, see the Proto2 Language Guide for details. Note that creating custom options uses extensions, which are permitted only for custom options in proto3.

I believe that means that when you're interacting with extensions they are proto2 (weird!). Doesn't that mean that we would have the HasFoo methods on the proto objects? I think that means we could just check to see if the value was set. @johanbrandhorst, am I way off base here?

achew22 avatar Aug 12 '19 06:08 achew22

I agree that the analysis sounds right, but I'm not sure there's anything we can do here without breaking someone. If we take the default value to mean "0", it'll break anyone who hasn't specified a minimum. If not, we get this bug. It would be interesting to see exactly what data is passed to the generator, it might be we need a flag for this. Alternatively... Maybe set the minimum to epsilon, epsilon > 0 😬😬😬?

johanbrandhorst avatar Aug 12 '19 08:08 johanbrandhorst

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Oct 11 '19 08:10 stale[bot]

Commenting so the bot doesn't close it. Per above comments, there's no easy/straight forward fix, but I'd like to keep this open, unless you all strongly object.

goodspark avatar Oct 11 '19 17:10 goodspark

A flag for MinimumSet or using strings, so you can tell the difference between "0" and "" are hacks, but I guess would maybe work?

zoidyzoidzoid avatar Oct 23 '19 22:10 zoidyzoidzoid