From 7f9a651f1da6fdd7b62f409a61baaaac33f0e33f Mon Sep 17 00:00:00 2001 From: Dan Larsen Date: Thu, 6 Jan 2022 18:16:22 +0100 Subject: [PATCH] Fix embedded messages (#280) * Handle readonly and writeonly for all versions above draft-06 * protoc-gen-jsonschema: fix embedded messages with embedded messages All schemas in definitions and definitions from embedded messages will now be written on the root schema. * Fix naming of sub messages * Fix failing test --- .../examples/tests/embedded/message.proto | 46 ++++++++++ .../tests/embedded/schemas_json/Message.json | 60 +++++++++++++ .../tests/embedded/schemas_proto/Message.json | 60 +++++++++++++ .../tests/embedded/testdata_json/Message.json | 27 ++++++ .../tests/mapfields/schemas_json/Message.json | 10 +-- .../mapfields/schemas_proto/Message.json | 10 +-- .../generator/json-schema.go | 84 ++++++++++++------- apps/protoc-gen-jsonschema/plugin_test.go | 1 + go.mod | 2 +- go.sum | 4 + 10 files changed, 263 insertions(+), 41 deletions(-) create mode 100644 apps/protoc-gen-jsonschema/examples/tests/embedded/message.proto create mode 100644 apps/protoc-gen-jsonschema/examples/tests/embedded/schemas_json/Message.json create mode 100644 apps/protoc-gen-jsonschema/examples/tests/embedded/schemas_proto/Message.json create mode 100644 apps/protoc-gen-jsonschema/examples/tests/embedded/testdata_json/Message.json diff --git a/apps/protoc-gen-jsonschema/examples/tests/embedded/message.proto b/apps/protoc-gen-jsonschema/examples/tests/embedded/message.proto new file mode 100644 index 0000000..399b11b --- /dev/null +++ b/apps/protoc-gen-jsonschema/examples/tests/embedded/message.proto @@ -0,0 +1,46 @@ +// Copyright 2021 Google LLC. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +syntax = "proto3"; + +package tests.embedded.message.v1; + +import "google/api/annotations.proto"; + +option go_package = "github.com/google/gnostic/apps/protoc-gen-openapi/examples/tests/embedded/message/v1;message"; + +service Messaging { + rpc UpdateMessage(Message) returns (Message) { + option (google.api.http) = { + post : "/v1/messages/{message_id}" + body : "*" + }; + } +} + +message Message { + string message_id = 1; + message Comment { + string id = 1; + string content = 2; + message Reply { + string id = 1; + string comment_id = 2; + string content = 3; + } + repeated Reply replies = 3; + } + repeated Comment comments = 2; +} diff --git a/apps/protoc-gen-jsonschema/examples/tests/embedded/schemas_json/Message.json b/apps/protoc-gen-jsonschema/examples/tests/embedded/schemas_json/Message.json new file mode 100644 index 0000000..e9e1a22 --- /dev/null +++ b/apps/protoc-gen-jsonschema/examples/tests/embedded/schemas_json/Message.json @@ -0,0 +1,60 @@ +{ + "title": "Message", + "$id": "http://example.com/schemas/Message.json", + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "messageId": { + "title": "messageId", + "type": "string" + }, + "comments": { + "title": "comments", + "type": "array", + "items": { + "$ref": "#/definitions/Message_Comment" + } + } + }, + "definitions": { + "Message_Comment_Reply": { + "title": "Reply", + "type": "object", + "properties": { + "id": { + "title": "id", + "type": "string" + }, + "commentId": { + "title": "commentId", + "type": "string" + }, + "content": { + "title": "content", + "type": "string" + } + } + }, + "Message_Comment": { + "title": "Comment", + "type": "object", + "properties": { + "id": { + "title": "id", + "type": "string" + }, + "content": { + "title": "content", + "type": "string" + }, + "replies": { + "title": "replies", + "type": "array", + "items": { + "$ref": "#/definitions/Message_Comment_Reply" + } + } + } + } + } +} diff --git a/apps/protoc-gen-jsonschema/examples/tests/embedded/schemas_proto/Message.json b/apps/protoc-gen-jsonschema/examples/tests/embedded/schemas_proto/Message.json new file mode 100644 index 0000000..c5e75ed --- /dev/null +++ b/apps/protoc-gen-jsonschema/examples/tests/embedded/schemas_proto/Message.json @@ -0,0 +1,60 @@ +{ + "title": "Message", + "$id": "http://example.com/schemas/Message.json", + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "message_id": { + "title": "message_id", + "type": "string" + }, + "comments": { + "title": "comments", + "type": "array", + "items": { + "$ref": "#/definitions/Message_Comment" + } + } + }, + "definitions": { + "Message_Comment_Reply": { + "title": "Reply", + "type": "object", + "properties": { + "id": { + "title": "id", + "type": "string" + }, + "comment_id": { + "title": "comment_id", + "type": "string" + }, + "content": { + "title": "content", + "type": "string" + } + } + }, + "Message_Comment": { + "title": "Comment", + "type": "object", + "properties": { + "id": { + "title": "id", + "type": "string" + }, + "content": { + "title": "content", + "type": "string" + }, + "replies": { + "title": "replies", + "type": "array", + "items": { + "$ref": "#/definitions/Message_Comment_Reply" + } + } + } + } + } +} diff --git a/apps/protoc-gen-jsonschema/examples/tests/embedded/testdata_json/Message.json b/apps/protoc-gen-jsonschema/examples/tests/embedded/testdata_json/Message.json new file mode 100644 index 0000000..9757bbe --- /dev/null +++ b/apps/protoc-gen-jsonschema/examples/tests/embedded/testdata_json/Message.json @@ -0,0 +1,27 @@ +{ + "messageId": "123abc", + "comments": [ + { + "id": "abc123", + "content": "first comment", + "replies": [ + { + "id": "xyz123", + "commentId": "abc123", + "content": "first reply to first comment" + } + ] + }, + { + "id": "123abc", + "content": "second comment", + "replies": [ + { + "id": "123xyz", + "commentId": "123abc", + "content": "first reply to second comment" + } + ] + } + ] +} \ No newline at end of file diff --git a/apps/protoc-gen-jsonschema/examples/tests/mapfields/schemas_json/Message.json b/apps/protoc-gen-jsonschema/examples/tests/mapfields/schemas_json/Message.json index e12d0f3..11a73bf 100644 --- a/apps/protoc-gen-jsonschema/examples/tests/mapfields/schemas_json/Message.json +++ b/apps/protoc-gen-jsonschema/examples/tests/mapfields/schemas_json/Message.json @@ -14,7 +14,7 @@ "$ref": "http://example.com/schemas/AnotherMessage.json" }, "subMessage": { - "$ref": "#/definitions/SubMessage" + "$ref": "#/definitions/Message_SubMessage" }, "stringList": { "title": "stringList", @@ -27,7 +27,7 @@ "title": "subMessageList", "type": "array", "items": { - "$ref": "#/definitions/SubMessage" + "$ref": "#/definitions/Message_SubMessage" } }, "anotherMessageList": { @@ -55,7 +55,7 @@ "title": "subMessagesMap", "type": "object", "additionalProperties": { - "$ref": "#/definitions/SubMessage" + "$ref": "#/definitions/Message_SubMessage" } }, "anotherMessagesMap": { @@ -74,10 +74,8 @@ } }, "definitions": { - "SubMessage": { + "Message_SubMessage": { "title": "SubMessage", - "$id": "/schemas/SubMessage", - "$schema": "http://json-schema.org/draft-07/schema#", "type": "object", "properties": { "id": { diff --git a/apps/protoc-gen-jsonschema/examples/tests/mapfields/schemas_proto/Message.json b/apps/protoc-gen-jsonschema/examples/tests/mapfields/schemas_proto/Message.json index 2a949ac..f2ed560 100644 --- a/apps/protoc-gen-jsonschema/examples/tests/mapfields/schemas_proto/Message.json +++ b/apps/protoc-gen-jsonschema/examples/tests/mapfields/schemas_proto/Message.json @@ -14,7 +14,7 @@ "$ref": "http://example.com/schemas/AnotherMessage.json" }, "sub_message": { - "$ref": "#/definitions/SubMessage" + "$ref": "#/definitions/Message_SubMessage" }, "string_list": { "title": "string_list", @@ -27,7 +27,7 @@ "title": "sub_message_list", "type": "array", "items": { - "$ref": "#/definitions/SubMessage" + "$ref": "#/definitions/Message_SubMessage" } }, "another_message_list": { @@ -55,7 +55,7 @@ "title": "sub_messages_map", "type": "object", "additionalProperties": { - "$ref": "#/definitions/SubMessage" + "$ref": "#/definitions/Message_SubMessage" } }, "another_messages_map": { @@ -74,10 +74,8 @@ } }, "definitions": { - "SubMessage": { + "Message_SubMessage": { "title": "SubMessage", - "$id": "/schemas/SubMessage", - "$schema": "http://json-schema.org/draft-07/schema#", "type": "object", "properties": { "id": { diff --git a/apps/protoc-gen-jsonschema/generator/json-schema.go b/apps/protoc-gen-jsonschema/generator/json-schema.go index b1bcb7d..eebc114 100644 --- a/apps/protoc-gen-jsonschema/generator/json-schema.go +++ b/apps/protoc-gen-jsonschema/generator/json-schema.go @@ -18,7 +18,6 @@ package generator import ( "fmt" "log" - "net/url" "regexp" "strings" @@ -113,20 +112,24 @@ func (g *JSONSchemaGenerator) formatFieldName(field *protogen.Field) string { return field.Desc.JSONName() } -// schemaReferenceForTypeName returns a JSON Schema definitions reference. -func (g *JSONSchemaGenerator) schemaReferenceForTypeName(typeName string) string { - parts := strings.Split(typeName, ".") - lastPart := parts[len(parts)-1] - return "#/definitions/" + g.formatMessageNameString(lastPart) -} +// messageDefinitionName builds the full schema definition name of a message. +func messageDefinitionName(desc protoreflect.MessageDescriptor) string { + name := string(desc.Name()) + + pkg := string(desc.ParentFile().Package()) + parentName := desc.Parent().FullName() + if len(parentName) > len(pkg) { + parentName = parentName[len(pkg)+1:] + name = fmt.Sprintf("%s.%s", parentName, name) + } -// fullMessageTypeName builds the full type name of a message. -func fullMessageTypeName(message protoreflect.MessageDescriptor) string { - name := string(message.Name()) - return "." + string(message.ParentFile().Package()) + "." + name + return strings.Replace(name, ".", "_", -1) } -func (g *JSONSchemaGenerator) schemaOrReferenceForType(typeName string) *jsonschema.Schema { +func (g *JSONSchemaGenerator) schemaOrReferenceForType(desc protoreflect.MessageDescriptor) *jsonschema.Schema { + // Create the full typeName + typeName := fmt.Sprintf(".%s.%s", desc.ParentFile().Package(), desc.Name()) + switch typeName { case ".google.protobuf.Timestamp": @@ -157,7 +160,8 @@ func (g *JSONSchemaGenerator) schemaOrReferenceForType(typeName string) *jsonsch return nil } - ref := g.schemaReferenceForTypeName(typeName) + typeName = messageDefinitionName(desc) + ref := "#/definitions/" + g.formatMessageNameString(typeName) return &jsonschema.Schema{Ref: &ref} } @@ -179,17 +183,17 @@ func (g *JSONSchemaGenerator) schemaOrReferenceForField(field protoreflect.Field switch kind { case protoreflect.MessageKind: - typeName := fullMessageTypeName(field.Message()) - - kindSchema = g.schemaOrReferenceForType(typeName) + kindSchema = g.schemaOrReferenceForType(field.Message()) if kindSchema == nil { return nil } - if kindSchema.Ref != nil && !refInDefinitions(*kindSchema.Ref, definitions) { - ref := strings.Replace(*kindSchema.Ref, "#/definitions/", *g.conf.BaseURL, 1) - ref += ".json" - kindSchema.Ref = &ref + if kindSchema.Ref != nil { + if !refInDefinitions(*kindSchema.Ref, definitions) { + ref := strings.Replace(*kindSchema.Ref, "#/definitions/", *g.conf.BaseURL, 1) + ref += ".json" + kindSchema.Ref = &ref + } } case protoreflect.StringKind: @@ -268,13 +272,27 @@ func (g *JSONSchemaGenerator) buildSchemasFromMessages(messages []*protogen.Mess // Any embedded messages will be created as definitions if message.Messages != nil { - subSchemas := g.buildSchemasFromMessages(message.Messages) - for i, subSchema := range subSchemas { - idURL, _ := url.Parse(*subSchema.Value.ID) - path := strings.TrimSuffix(idURL.Path, ".json") - subSchemas[i].Value.ID = &path + if schema.Value.Definitions == nil { + schema.Value.Definitions = &[]*jsonschema.NamedSchema{} + } + + for _, subMessage := range message.Messages { + subSchemas := g.buildSchemasFromMessages([]*protogen.Message{subMessage}) + if len(subSchemas) != 1 { + continue + } + subSchema := subSchemas[0] + subSchema.Value.ID = nil + subSchema.Value.Schema = nil + subSchema.Name = messageDefinitionName(subMessage.Desc) + + if subSchema.Value.Definitions != nil { + *schema.Value.Definitions = append(*schema.Value.Definitions, *subSchema.Value.Definitions...) + subSchema.Value.Definitions = nil + } + + *schema.Value.Definitions = append(*schema.Value.Definitions, subSchemas...) } - schema.Value.Definitions = &subSchemas } if message.Desc.IsMapEntry() { @@ -289,7 +307,7 @@ func (g *JSONSchemaGenerator) buildSchemasFromMessages(messages []*protogen.Mess } // Handle readonly and writeonly properties, if the schema version can handle it. - if strings.TrimSuffix(*schema.Value.Schema, "#") == "http://json-schema.org/draft-07/schema" { + if getSchemaVersion(schema.Value) >= "07" { t := true // Check the field annotations to see if this is a readonly field. extension := proto.GetExtension(field.Desc.Options(), annotations.E_FieldBehavior) @@ -313,7 +331,6 @@ func (g *JSONSchemaGenerator) buildSchemasFromMessages(messages []*protogen.Mess // Do not add title for ref values if fieldSchema.Ref == nil { fieldSchema.Title = &fieldName - } // Get the field description from the comments. @@ -338,6 +355,17 @@ func (g *JSONSchemaGenerator) buildSchemasFromMessages(messages []*protogen.Mess return schemas } +var reSchemaVersion = regexp.MustCompile(`https*://json-schema.org/draft[/-]([^/]+)/schema`) + +func getSchemaVersion(schema *jsonschema.Schema) string { + schemaSchema := *schema.Schema + matches := reSchemaVersion.FindStringSubmatch(schemaSchema) + if len(matches) == 2 { + return matches[1] + } + return "" +} + func refInDefinitions(ref string, definitions *[]*jsonschema.NamedSchema) bool { if definitions == nil { return false diff --git a/apps/protoc-gen-jsonschema/plugin_test.go b/apps/protoc-gen-jsonschema/plugin_test.go index 473a020..07526d2 100644 --- a/apps/protoc-gen-jsonschema/plugin_test.go +++ b/apps/protoc-gen-jsonschema/plugin_test.go @@ -36,6 +36,7 @@ var jsonschemaTests = []struct { {name: "Google Library example", path: "examples/google/example/library/v1/", pkg: "google.example.library.v1", protofile: "library.proto"}, {name: "Map fields", path: "examples/tests/mapfields/", pkg: "tests.mapfields.message.v1", protofile: "message.proto"}, {name: "JSON options", path: "examples/tests/jsonoptions/", pkg: "", protofile: "message.proto"}, + {name: "Embedded messages", path: "examples/tests/embedded/", pkg: "", protofile: "message.proto"}, } func TestJSONSchemaProtobufNaming(t *testing.T) { diff --git a/go.mod b/go.mod index baf6894..6f29a39 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.12 require ( github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815 - github.com/flowstack/go-jsonschema v0.0.0-20211209084258-c17707834089 + github.com/flowstack/go-jsonschema v0.1.2-0.20220104092834-36c9f7194585 github.com/golang/protobuf v1.5.2 github.com/kr/pretty v0.2.0 // indirect github.com/stoewer/go-strcase v1.2.0 diff --git a/go.sum b/go.sum index 7a9e6a0..53fb0d3 100644 --- a/go.sum +++ b/go.sum @@ -13,6 +13,10 @@ github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.m github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/flowstack/go-jsonschema v0.0.0-20211209084258-c17707834089 h1:fitJoueZBxhxndujAiIEGTJ7J8+EDQN72ZDb2fuJPpA= github.com/flowstack/go-jsonschema v0.0.0-20211209084258-c17707834089/go.mod h1:yL7fNggx1o8rm9RlgXv7hTBWxdBM0rVwpMwimd3F3N0= +github.com/flowstack/go-jsonschema v0.1.1 h1:dCrjGJRXIlbDsLAgTJZTjhwUJnnxVWl1OgNyYh5nyDc= +github.com/flowstack/go-jsonschema v0.1.1/go.mod h1:yL7fNggx1o8rm9RlgXv7hTBWxdBM0rVwpMwimd3F3N0= +github.com/flowstack/go-jsonschema v0.1.2-0.20220104092834-36c9f7194585 h1:5tr8qTErHQ8K5a1tDhb4lBoMaN5Ky69tr+tcqJ+dtbw= +github.com/flowstack/go-jsonschema v0.1.2-0.20220104092834-36c9f7194585/go.mod h1:yL7fNggx1o8rm9RlgXv7hTBWxdBM0rVwpMwimd3F3N0= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=