Skip to content

Commit

Permalink
Sort tags by explicit Swagger annotation first
Browse files Browse the repository at this point in the history 
I believe it what most people would expect, since the order of
tags extracted from methods is harder to control, especially if
swagger file is merged from multiple proto files.
  • Loading branch information
@same-id
same-id committed Sep 11, 2023
1 parent abc1a23 commit c8111e2
Show file tree
Hide file tree
Showing 2 changed files with 250 additions and 4 deletions.
53 changes: 49 additions & 4 deletions protoc-gen-openapiv2/internal/genopenapi/template.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1746,10 +1746,6 @@ func applyTemplate(p param) (*openapiSwaggerObject, error) {
panic(err)
}

if !p.reg.GetDisableServiceTags() {
s.Tags = append(s.Tags, renderServiceTags(p.Services, p.reg)...)
}

messages := messageMap{}
streamingMessages := messageMap{}
enums := enumMap{}
Expand Down Expand Up @@ -2017,6 +2013,10 @@ func applyTemplate(p param) (*openapiSwaggerObject, error) {
// should be added here, once supported in the proto.
}

if !p.reg.GetDisableServiceTags() {
s.Tags = mergeTags(s.Tags, renderServiceTags(p.Services, p.reg))
}

// Finally add any references added by users that aren't
// otherwise rendered.
if err := addCustomRefs(s.Definitions, p.reg, customRefs); err != nil {
Expand All @@ -2026,6 +2026,51 @@ func applyTemplate(p param) (*openapiSwaggerObject, error) {
return &s, nil
}

func mergeTags(existingTags []openapiTagObject, tags []openapiTagObject) []openapiTagObject {
for _, tag := range tags {
matched := false
for i, existingTag := range existingTags {
if existingTag.Name == tag.Name {
if existingTag.Description == "" {
existingTags[i].Description = tag.Description
}
if existingTag.ExternalDocs == nil {
existingTags[i].ExternalDocs = tag.ExternalDocs
} else if tag.ExternalDocs != nil {
if existingTag.ExternalDocs.Description == "" {
existingTags[i].ExternalDocs.Description = tag.ExternalDocs.Description
}
if existingTag.ExternalDocs.URL == "" {
existingTags[i].ExternalDocs.URL = tag.ExternalDocs.URL
}
}
if existingTag.extensions == nil {
existingTags[i].extensions = tag.extensions
} else if tag.extensions != nil {
for _, ext := range tag.extensions {
matchedExt := false
for _, existingExt := range existingTag.extensions {
if existingExt.key == ext.key {
matchedExt = true
break
}
}
if !matchedExt {
existingTags[i].extensions = append(existingTags[i].extensions, ext)
}
}
}
matched = true
break
}
}
if !matched {
existingTags = append(existingTags, tag)
}
}
return existingTags
}

func processExtensions(inputExts map[string]*structpb.Value) ([]extension, error) {
exts := make([]extension, 0, len(inputExts))
for k, v := range inputExts {
Expand Down
201 changes: 201 additions & 0 deletions protoc-gen-openapiv2/internal/genopenapi/template_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10096,3 +10096,204 @@ func TestUpdatePaths(t *testing.T) {
})
}
}

func MustMarshal(v interface{}) []byte {
b, err := json.Marshal(v)
if err != nil {
panic(err)
}
return b
}

func TestMergeTags(t *testing.T) {
testCases := [...]struct {
testName string
existingTags []openapiTagObject
newTags []openapiTagObject
expectedMergedTags []openapiTagObject
}{
{
testName: "Simple merge.",
existingTags: []openapiTagObject{{
Name: "tag1",
Description: "tag1 description",
}},
newTags: []openapiTagObject{{
Name: "tag2",
Description: "tag2 description",
}},
expectedMergedTags: []openapiTagObject{{
Name: "tag1",
Description: "tag1 description",
}, {
Name: "tag2",
Description: "tag2 description",
}},
},
{
testName: "Merge description",
existingTags: []openapiTagObject{{
Name: "tag1",
Description: "tag1 description",
}, {
Name: "tag2",
}, {
Name: "tag3",
Description: "tag3 description",
}},
newTags: []openapiTagObject{{
Name: "tag2",
Description: "tag2 description",
}},
expectedMergedTags: []openapiTagObject{{
Name: "tag1",
Description: "tag1 description",
}, {
Name: "tag2",
Description: "tag2 description",
}, {
Name: "tag3",
Description: "tag3 description",
}},
},
{
testName: "Merge external docs",
existingTags: []openapiTagObject{{
Name: "tag1",
ExternalDocs: &openapiExternalDocumentationObject{},
}, {
Name: "tag2",
}, {
Name: "tag3",
ExternalDocs: &openapiExternalDocumentationObject{
Description: "tag3 description",
},
}, {
Name: "tag4",
ExternalDocs: &openapiExternalDocumentationObject{
URL: "tag4 url",
},
}},
newTags: []openapiTagObject{{
Name: "tag1",
ExternalDocs: &openapiExternalDocumentationObject{
Description: "tag1 description",
},
}, {
Name: "tag2",
ExternalDocs: &openapiExternalDocumentationObject{
Description: "tag2 description",
URL: "tag2 url",
},
}, {
Name: "tag3",
ExternalDocs: &openapiExternalDocumentationObject{
Description: "ignored tag3 description",
URL: "tag3 url",
},
}, {
Name: "tag4",
ExternalDocs: &openapiExternalDocumentationObject{
Description: "tag4 description",
},
}},
expectedMergedTags: []openapiTagObject{{
Name: "tag1",
ExternalDocs: &openapiExternalDocumentationObject{
Description: "tag1 description",
},
}, {
Name: "tag2",
ExternalDocs: &openapiExternalDocumentationObject{
Description: "tag2 description",
URL: "tag2 url",
},
}, {
Name: "tag3",
ExternalDocs: &openapiExternalDocumentationObject{
Description: "tag3 description",
URL: "tag3 url",
},
}, {
Name: "tag4",
ExternalDocs: &openapiExternalDocumentationObject{
Description: "tag4 description",
URL: "tag4 url",
},
}},
},
{
testName: "Merge extensions",
existingTags: []openapiTagObject{{
Name: "tag1",
extensions: []extension{{key: "x-key1", value: MustMarshal("key1 extension")}},
}, {
Name: "tag2",
extensions: []extension{
{key: "x-key1", value: MustMarshal("key1 extension")},
{key: "x-key2", value: MustMarshal("key2 extension")},
},
}, {
Name: "tag3",
extensions: []extension{
{key: "x-key1", value: MustMarshal("key1 extension")},
},
}, {
Name: "tag4",
extensions: nil,
}},
newTags: []openapiTagObject{{
Name: "tag1",
extensions: []extension{{key: "x-key2", value: MustMarshal("key2 extension")}},
}, {
Name: "tag2",
extensions: []extension{
{key: "x-key1", value: MustMarshal("key1 extension")},
{key: "x-key2", value: MustMarshal("ignored key2 extension")},
{key: "x-key3", value: MustMarshal("key3 extension")},
},
}, {
Name: "tag3",
extensions: nil,
}, {
Name: "tag4",
extensions: []extension{
{key: "x-key1", value: MustMarshal("key1 extension")},
},
}},
expectedMergedTags: []openapiTagObject{{
Name: "tag1",
extensions: []extension{
{key: "x-key1", value: MustMarshal("key1 extension")},
{key: "x-key2", value: MustMarshal("key2 extension")},
},
}, {
Name: "tag2",
extensions: []extension{
{key: "x-key1", value: MustMarshal("key1 extension")},
{key: "x-key2", value: MustMarshal("key2 extension")},
{key: "x-key3", value: MustMarshal("key3 extension")},
},
}, {
Name: "tag3",
extensions: []extension{
{key: "x-key1", value: MustMarshal("key1 extension")},
},
}, {
Name: "tag4",
extensions: []extension{
{key: "x-key1", value: MustMarshal("key1 extension")},
},
}},
},
}
for _, tc := range testCases {
tc := tc
t.Run(tc.testName, func(t *testing.T) {
mergedTags := mergeTags(tc.existingTags, tc.newTags)
if !reflect.DeepEqual(tc.expectedMergedTags, mergedTags) {
t.Fatalf("%s: Tags not correctly merged. Want %#v, got %#v", tc.testName, tc.expectedMergedTags, mergedTags)
}
})
}
}

0 comments on commit c8111e2

Please sign in to comment.