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

read_only not working with references. #1137

Closed
SimonSikstrom opened this issue Feb 17, 2020 · 2 comments · Fixed by #3082
Closed

read_only not working with references. #1137

SimonSikstrom opened this issue Feb 17, 2020 · 2 comments · Fixed by #3082

Comments

@SimonSikstrom
Copy link

So I want to be able to define messages in other messages as read_only fields when generating openapi doc, however, that does not seem possible at the moment.

Given example:


syntax = "proto3";

import "protoc-gen-swagger/options/annotations.proto";
import "google/api/annotations.proto";

service SomeService {
  // Create new Some
  rpc SomeService(SomeServiceRequest) returns (SomeMessage) {
    option (google.api.http) = {
      post: "/do-something"
      body: "body"
    };
  }
}

message SomeServiceRequest {
  SomeMessage body = 1;
}

message SomeMessage {
  // Readonly id, works fine here
  string id = 1 [(grpc.gateway.protoc_gen_swagger.options.openapiv2_field) = {read_only: true}];

  // OtherReadOnlyMessage readonly, does not work.
  OtherReadOnlyMessage field = 2 [(grpc.gateway.protoc_gen_swagger.options.openapiv2_field) = {read_only: true}];

  // OtherReadOnlyMessage not readonly
  OtherReadOnlyMessage other = 3;
}

// Message I want to use in multiple places
message OtherReadOnlyMessage {
  string whatever = 1;
}

It will output something like this:

{
  "swagger": "2.0",
  "info": {
    "title": "readonly.proto",
    "version": "version not set"
  },
  "consumes": [
    "application/json"
  ],
  "produces": [
    "application/json"
  ],
  "paths": {
    "/do-something": {
      "post": {
        "summary": "Create A new Some",
        "operationId": "SomeService",
        "responses": {
          "200": {
            "description": "A successful response.",
            "schema": {
              "$ref": "#/definitions/SomeMessage"
            }
          },
          "default": {
            "description": "An unexpected error response",
            "schema": {
              "$ref": "#/definitions/runtimeError"
            }
          }
        },
        "parameters": [
          {
            "name": "body",
            "in": "body",
            "required": true,
            "schema": {
              "$ref": "#/definitions/SomeMessage"
            }
          }
        ],
        "tags": [
          "SomeService"
        ]
      }
    }
  },
  "definitions": {
    "OtherReadOnlyMessage": {
      "type": "object",
      "properties": {
        "whatever": {
          "type": "string"
        }
      },
      "title": "Message I want to use in two places"
    },
    "SomeMessage": {
      "type": "object",
      "properties": {
        "id": {
          "type": "string",
          "title": "Readonly id, works",
          "readOnly": true
        },
        "field": {
          "$ref": "#/definitions/OtherReadOnlyMessage",
          "title": "OtherReadOnlyMessage readonly",
          "readOnly": true
        },
        "other": {
          "$ref": "#/definitions/OtherReadOnlyMessage",
          "title": "OtherReadOnlyMessage not readonly"
        }
      }
    },
...
  }
}

The readOnly field is set correctly but according to swagger docs:

Any sibling elements of a $ref are ignored. This is because $ref works by replacing itself and everything on its level with the definition it is pointing at.

So its not reading and listing this property as read only in the end.

It seems like common practice is to instead of directly using $ref is to use allOf of the $ref instead, then the readOnly property would be used.

So this

...
"field": {
          "$ref": "#/definitions/OtherReadOnlyMessage",
          "title": "OtherReadOnlyMessage readonly",
          "readOnly": true
        },
...

Should in that case become this:

...
        "field": {
          "title": "OtherReadOnlyMessage readonly",
          "readOnly": true,
          "allOf": [
            { "$ref": "#/definitions/OtherReadOnlyMessage" }
          ]
        },
...

Thoughts?

@johanbrandhorst
Copy link
Collaborator

Hey @SimonSikstrom, thanks for the detailed issue report! Your analysis sounds reasonable, and also your proposed solution. Would you be interested in contributing this fix?

@SimonSikstrom
Copy link
Author

SimonSikstrom commented Feb 17, 2020

Thanks for the quick response! Sure I could have a look and see if this is something I could solve in a reasonable timeframe. I'll link upcoming PR if I have something ready.

same-id added a commit to same-id/grpc-gateway that referenced this issue Dec 15, 2022
same-id added a commit to same-id/grpc-gateway that referenced this issue Dec 20, 2022
As described here:

grpc-ecosystem#1137

This partially reverts the recent commit:

a0fe8d7

Which doesn't solve the problem for read_only being set on the field.

This feature will require setting the "use_allof_for_refs" to true
since this breaks existing compatibility with swagger-codegen generated
files from the AllOf'd schema - type is now "object" and not $ref.

Test added + ReDoc now works with read_only message/enum fields.
same-id added a commit to same-id/grpc-gateway that referenced this issue Dec 21, 2022
As described here:

grpc-ecosystem#1137

This partially reverts the recent commit:

a0fe8d7

Which doesn't solve the problem for read_only being set on the field.

This feature will require setting the "use_allof_for_refs" to true
since this breaks existing compatibility with swagger-codegen generated
files from the AllOf'd schema - type is now "object" and not $ref.

Test added + ReDoc now works with read_only message/enum fields.
same-id added a commit to same-id/grpc-gateway that referenced this issue Dec 21, 2022
As described here:

grpc-ecosystem#1137

This partially reverts the recent commit:

a0fe8d7

Which doesn't solve the problem for read_only being set on the field.

This feature will require setting the "use_allof_for_refs" to true
since this breaks existing compatibility with swagger-codegen generated
files from the AllOf'd schema - type is now "object" and not $ref.

Test added + ReDoc now works with read_only message/enum fields.
johanbrandhorst pushed a commit that referenced this issue Dec 21, 2022
As described here:

#1137

This partially reverts the recent commit:

a0fe8d7

Which doesn't solve the problem for read_only being set on the field.

This feature will require setting the "use_allof_for_refs" to true
since this breaks existing compatibility with swagger-codegen generated
files from the AllOf'd schema - type is now "object" and not $ref.

Test added + ReDoc now works with read_only message/enum fields.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants