prepend goTag directive on struct tags and omit overridden duplicate …

…struct tags per #2514 (#2533)

* removeDuplicateTags func introduced to fix #2514

* Change to prepend goTag directive

Signed-off-by: Steve Coffman <steve@khanacademy.org>

* Fix test for field_hooks_are_applied to prepend

Signed-off-by: Steve Coffman <steve@khanacademy.org>

---------

Signed-off-by: Steve Coffman <steve@khanacademy.org>
Co-authored-by: vallabh <vallabh.joshi@gmail.com>

plugin/modelgen/models.go

@@ -388,8 +388,9 @@ 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
-// 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,12 +413,33 @@ func GoTagFieldHook(td *ast.Definition, fd *ast.FieldDefinition, f *Field) (*Fie
 	}
 
 	if len(args) > 0 {
-		f.Tag = f.Tag + " " + strings.Join(args, " ")
+		f.Tag = removeDuplicateTags(strings.Join(args, " ") + " " + f.Tag)
 	}
 
 	return f, nil
 }
 
+func removeDuplicateTags(t string) string {
+	processed := make(map[string]bool)
+	tt := strings.Split(t, " ")
+	returnTags := ""
+
+	for _, ti := range tt {
+		kv := strings.Split(ti, ":")
+		if len(kv) == 0 || processed[kv[0]] {
+			continue
+		}
+
+		processed[kv[0]] = true
+		if len(returnTags) > 0 {
+			returnTags += " "
+		}
+		returnTags += kv[0] + ":" + kv[1]
+	}
+
+	return returnTags
+}
+
 // 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)

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)
 		}
 	})
 
@@ -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:\"name\"",
+		},
+		{
+			name: "Duplicate Test with 3",
+			args: args{
+				t: "json:\"name\" json:\"name2\" json:\"name3\"",
+			},
+			want: "json:\"name\"",
+		},
+		{
+			name: "Duplicate Test with 3 and 1 unrelated",
+			args: args{
+				t: "json:\"name\" something:\"name2\" json:\"name3\"",
+			},
+			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:\"name1\" json:\"name\"",
+		},
+	}
+	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)
+			}
+		})
+	}
+}