From 6ec1fc100f43a28ee3e21c3a85a0389183a8aee6 Mon Sep 17 00:00:00 2001 From: vallabh Date: Fri, 27 Jan 2023 05:44:42 +0000 Subject: [PATCH 1/3] removeDuplicateTags func introduced to fix #2514 --- plugin/modelgen/models.go | 31 +++++++++++ plugin/modelgen/models_test.go | 54 +++++++++++++++++++ .../modelgen/out_struct_pointers/generated.go | 4 +- 3 files changed, 87 insertions(+), 2 deletions(-) diff --git a/plugin/modelgen/models.go b/plugin/modelgen/models.go index a7388b911a7..ef7a218b933 100644 --- a/plugin/modelgen/models.go +++ b/plugin/modelgen/models.go @@ -415,9 +415,40 @@ func GoTagFieldHook(td *ast.Definition, fd *ast.FieldDefinition, f *Field) (*Fie f.Tag = f.Tag + " " + strings.Join(args, " ") } + f.Tag = removeDuplicateTags(f.Tag) + return f, nil } +// removeDuplicateTags removes duplicate tags +func removeDuplicateTags(t string) string { + + tt := strings.Split(t, " ") + + if len(tt) > 0 { + tagMap := map[string]string{} + returnTags := "" + + for _, ti := range tt { + kv := strings.Split(ti, ":") + if len(kv) > 0 { + tagMap[kv[0]] = kv[1] + } + } + + for k, v := range tagMap { + if len(returnTags) > 0 { + returnTags += " " + } + returnTags += k + ":" + v + } + return returnTags + + } + + return t +} + // GoFieldHook applies the goField directive to the generated Field f. func GoFieldHook(td *ast.Definition, fd *ast.FieldDefinition, f *Field) (*Field, error) { args := make([]string, 0) diff --git a/plugin/modelgen/models_test.go b/plugin/modelgen/models_test.go index d4a77414d90..c10babdb32d 100644 --- a/plugin/modelgen/models_test.go +++ b/plugin/modelgen/models_test.go @@ -350,3 +350,57 @@ func goBuild(t *testing.T, path string) error { return nil } + +func TestRemoveDuplicate(t *testing.T) { + type args struct { + t string + } + tests := []struct { + name string + args args + want string + }{ + { + name: "Duplicate Test with 1", + args: args{ + t: "json:\"name\"", + }, + want: "json:\"name\"", + }, + { + name: "Duplicate Test with 2", + args: args{ + t: "json:\"name\" json:\"name2\"", + }, + want: "json:\"name2\"", + }, + { + name: "Duplicate Test with 3", + args: args{ + t: "json:\"name\" json:\"name2\" json:\"name3\"", + }, + want: "json:\"name3\"", + }, + { + name: "Duplicate Test with 3 and 1 unrelated", + args: args{ + t: "json:\"name\" something:\"name2\" json:\"name3\"", + }, + want: "json:\"name3\" something:\"name2\"", + }, + { + name: "Duplicate Test with 3 and 2 unrelated", + args: args{ + t: "something:\"name1\" json:\"name\" something:\"name2\" json:\"name3\"", + }, + want: "something:\"name2\" json:\"name3\"", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := removeDuplicateTags(tt.args.t); got != tt.want { + t.Errorf("removeDuplicate() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/plugin/modelgen/out_struct_pointers/generated.go b/plugin/modelgen/out_struct_pointers/generated.go index 7b414f6dd5e..955049d7b91 100644 --- a/plugin/modelgen/out_struct_pointers/generated.go +++ b/plugin/modelgen/out_struct_pointers/generated.go @@ -89,8 +89,8 @@ type CyclicalB struct { type FieldMutationHook struct { Name *string `json:"name" anotherTag:"tag" database:"FieldMutationHookname"` - Enum *ExistingEnum `json:"enum" yetAnotherTag:"12" database:"FieldMutationHookenum"` - NoVal *string `json:"noVal" yaml:"noVal" repeated:"true" database:"FieldMutationHooknoVal"` + Enum *ExistingEnum `yetAnotherTag:"12" json:"enum" database:"FieldMutationHookenum"` + NoVal *string `repeated:"true" json:"noVal" yaml:"noVal" database:"FieldMutationHooknoVal"` Repeated *string `json:"repeated" someTag:"value" repeated:"true" database:"FieldMutationHookrepeated"` } From 06b8609b1b687b9da58c1fc6a58028624510a3ef Mon Sep 17 00:00:00 2001 From: Steve Coffman Date: Fri, 27 Jan 2023 12:53:55 -0500 Subject: [PATCH 2/3] Change to prepend goTag directive Signed-off-by: Steve Coffman --- plugin/modelgen/models.go | 33 +++++++++++++-------------------- plugin/modelgen/models_test.go | 8 ++++---- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/plugin/modelgen/models.go b/plugin/modelgen/models.go index ef7a218b933..851c4b53db5 100644 --- a/plugin/modelgen/models.go +++ b/plugin/modelgen/models.go @@ -388,7 +388,7 @@ func (m *Plugin) generateFields(cfg *config.Config, schemaType *ast.Definition) return fields, nil } -// GoTagFieldHook applies the goTag directive to the generated Field f. When applying the Tag to the field, the field +// GoTagFieldHook prepend the goTag directive to the generated Field f. When applying the Tag to the field, the field // name is used when no value argument is present. func GoTagFieldHook(td *ast.Definition, fd *ast.FieldDefinition, f *Field) (*Field, error) { args := make([]string, 0) @@ -412,7 +412,7 @@ func GoTagFieldHook(td *ast.Definition, fd *ast.FieldDefinition, f *Field) (*Fie } if len(args) > 0 { - f.Tag = f.Tag + " " + strings.Join(args, " ") + f.Tag = strings.Join(args, " ") + " " + f.Tag } f.Tag = removeDuplicateTags(f.Tag) @@ -422,31 +422,24 @@ func GoTagFieldHook(td *ast.Definition, fd *ast.FieldDefinition, f *Field) (*Fie // removeDuplicateTags removes duplicate tags func removeDuplicateTags(t string) string { - + processed := make(map[string]bool) tt := strings.Split(t, " ") + returnTags := "" - if len(tt) > 0 { - tagMap := map[string]string{} - returnTags := "" - - for _, ti := range tt { - kv := strings.Split(ti, ":") - if len(kv) > 0 { - tagMap[kv[0]] = kv[1] - } + for _, ti := range tt { + kv := strings.Split(ti, ":") + if len(kv) == 0 || processed[kv[0]] { + continue } - for k, v := range tagMap { - if len(returnTags) > 0 { - returnTags += " " - } - returnTags += k + ":" + v + processed[kv[0]] = true + if len(returnTags) > 0 { + returnTags = returnTags + " " } - return returnTags - + returnTags = returnTags + kv[0] + ":" + kv[1] } - return t + return returnTags } // GoFieldHook applies the goField directive to the generated Field f. diff --git a/plugin/modelgen/models_test.go b/plugin/modelgen/models_test.go index c10babdb32d..8058d5f5985 100644 --- a/plugin/modelgen/models_test.go +++ b/plugin/modelgen/models_test.go @@ -372,28 +372,28 @@ func TestRemoveDuplicate(t *testing.T) { args: args{ t: "json:\"name\" json:\"name2\"", }, - want: "json:\"name2\"", + want: "json:\"name\"", }, { name: "Duplicate Test with 3", args: args{ t: "json:\"name\" json:\"name2\" json:\"name3\"", }, - want: "json:\"name3\"", + want: "json:\"name\"", }, { name: "Duplicate Test with 3 and 1 unrelated", args: args{ t: "json:\"name\" something:\"name2\" json:\"name3\"", }, - want: "json:\"name3\" something:\"name2\"", + want: "json:\"name\" something:\"name2\"", }, { name: "Duplicate Test with 3 and 2 unrelated", args: args{ t: "something:\"name1\" json:\"name\" something:\"name2\" json:\"name3\"", }, - want: "something:\"name2\" json:\"name3\"", + want: "something:\"name1\" json:\"name\"", }, } for _, tt := range tests { From c83aa08e16780459072569a65211d7713a502f8b Mon Sep 17 00:00:00 2001 From: Steve Coffman Date: Fri, 27 Jan 2023 13:32:00 -0500 Subject: [PATCH 3/3] Fix test for field_hooks_are_applied to prepend Signed-off-by: Steve Coffman --- plugin/modelgen/models.go | 14 ++++++-------- plugin/modelgen/models_test.go | 10 +++++----- plugin/modelgen/out/generated.go | 8 ++++---- plugin/modelgen/out_struct_pointers/generated.go | 6 +++--- 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/plugin/modelgen/models.go b/plugin/modelgen/models.go index 851c4b53db5..c18548ed7ea 100644 --- a/plugin/modelgen/models.go +++ b/plugin/modelgen/models.go @@ -388,8 +388,9 @@ func (m *Plugin) generateFields(cfg *config.Config, schemaType *ast.Definition) return fields, nil } -// GoTagFieldHook prepend the goTag directive to the generated Field f. When applying the Tag to the field, the field -// name is used when no value argument is present. +// GoTagFieldHook prepends the goTag directive to the generated Field f. +// When applying the Tag to the field, the field +// name is used if no value argument is present. func GoTagFieldHook(td *ast.Definition, fd *ast.FieldDefinition, f *Field) (*Field, error) { args := make([]string, 0) for _, goTag := range fd.Directives.ForNames("goTag") { @@ -412,15 +413,12 @@ func GoTagFieldHook(td *ast.Definition, fd *ast.FieldDefinition, f *Field) (*Fie } if len(args) > 0 { - f.Tag = strings.Join(args, " ") + " " + f.Tag + f.Tag = removeDuplicateTags(strings.Join(args, " ") + " " + f.Tag) } - f.Tag = removeDuplicateTags(f.Tag) - return f, nil } -// removeDuplicateTags removes duplicate tags func removeDuplicateTags(t string) string { processed := make(map[string]bool) tt := strings.Split(t, " ") @@ -434,9 +432,9 @@ func removeDuplicateTags(t string) string { processed[kv[0]] = true if len(returnTags) > 0 { - returnTags = returnTags + " " + returnTags += " " } - returnTags = returnTags + kv[0] + ":" + kv[1] + returnTags += kv[0] + ":" + kv[1] } return returnTags diff --git a/plugin/modelgen/models_test.go b/plugin/modelgen/models_test.go index 8058d5f5985..bb08a379db9 100644 --- a/plugin/modelgen/models_test.go +++ b/plugin/modelgen/models_test.go @@ -82,14 +82,14 @@ func TestModelGeneration(t *testing.T) { fileText := string(file) expectedTags := []string{ - `json:"name" anotherTag:"tag"`, - `json:"enum" yetAnotherTag:"12"`, - `json:"noVal" yaml:"noVal"`, - `json:"repeated" someTag:"value" repeated:"true"`, + `anotherTag:"tag" json:"name"`, + `yetAnotherTag:"12" json:"enum"`, + `yaml:"noVal" repeated:"true" json:"noVal"`, + `someTag:"value" repeated:"true" json:"repeated"`, } for _, tag := range expectedTags { - require.True(t, strings.Contains(fileText, tag)) + require.True(t, strings.Contains(fileText, tag), tag) } }) diff --git a/plugin/modelgen/out/generated.go b/plugin/modelgen/out/generated.go index c968c466dc4..32d1c0a1ca1 100644 --- a/plugin/modelgen/out/generated.go +++ b/plugin/modelgen/out/generated.go @@ -106,10 +106,10 @@ type CyclicalB struct { } type FieldMutationHook struct { - Name *string `json:"name" anotherTag:"tag" database:"FieldMutationHookname"` - Enum *ExistingEnum `json:"enum" yetAnotherTag:"12" database:"FieldMutationHookenum"` - NoVal *string `json:"noVal" yaml:"noVal" repeated:"true" database:"FieldMutationHooknoVal"` - Repeated *string `json:"repeated" someTag:"value" repeated:"true" database:"FieldMutationHookrepeated"` + Name *string `anotherTag:"tag" json:"name" database:"FieldMutationHookname"` + Enum *ExistingEnum `yetAnotherTag:"12" json:"enum" database:"FieldMutationHookenum"` + NoVal *string `yaml:"noVal" repeated:"true" json:"noVal" database:"FieldMutationHooknoVal"` + Repeated *string `someTag:"value" repeated:"true" json:"repeated" database:"FieldMutationHookrepeated"` } type ImplArrayOfA struct { diff --git a/plugin/modelgen/out_struct_pointers/generated.go b/plugin/modelgen/out_struct_pointers/generated.go index 955049d7b91..6d429db1e6f 100644 --- a/plugin/modelgen/out_struct_pointers/generated.go +++ b/plugin/modelgen/out_struct_pointers/generated.go @@ -88,10 +88,10 @@ type CyclicalB struct { } type FieldMutationHook struct { - Name *string `json:"name" anotherTag:"tag" database:"FieldMutationHookname"` + Name *string `anotherTag:"tag" json:"name" database:"FieldMutationHookname"` Enum *ExistingEnum `yetAnotherTag:"12" json:"enum" database:"FieldMutationHookenum"` - NoVal *string `repeated:"true" json:"noVal" yaml:"noVal" database:"FieldMutationHooknoVal"` - Repeated *string `json:"repeated" someTag:"value" repeated:"true" database:"FieldMutationHookrepeated"` + NoVal *string `yaml:"noVal" repeated:"true" json:"noVal" database:"FieldMutationHooknoVal"` + Repeated *string `someTag:"value" repeated:"true" json:"repeated" database:"FieldMutationHookrepeated"` } type ImplArrayOfA struct {