Disallow duplicate link titles (#46)

* Disallow duplicate link titles

This change came about as a result of the realization that we are dropping numerous client API calls as a part of client generation, both here in schematic and in the ruby version of this tool, Heroics.

While a number of PRs have been initiated to fix the underlying heroku platform api schema itself, these two tools should raise an error and should not proceed to generate an incomplete client in the event of duplicate link titles.

During the discussion here (https://github.com/heroku/api/pull/7787#issuecomment-299169974) we are attempting to do the following:
- fix all of the (sub)schemas of the platform API.
- update schematic and heroics to throw an error when duplicates exist.
- regenerate the Golang and ruby clients, incrementing the major version in order to reflect the breaking changes (title changes -> method name changes) that will be introduced.

* Add tests; lowercase title for compare

- lowercase the title to ensure good comparison
- added test cases
- fixed an issue raise by tests

* Dont’t panic, just return early w/error

CONTRIBUTORS

@@ -1,3 +1,4 @@
 Blake Gentry <[email protected]>
 Bo Jeans <[email protected]>
 Timothée Peignier <[email protected]>
+Chris Johnson <[email protected]>

gen.go

@@ -70,6 +70,10 @@ func (s *Schema) Generate() ([]byte, error) {
 			Definition: schema,
 		}
 
+		if !s.CheckForDuplicateTitles() {
+			return nil, fmt.Errorf("duplicate titles detected for %s", context.Name)
+		}
+
 		templates.ExecuteTemplate(&buf, "struct.tmpl", context)
 		templates.ExecuteTemplate(&buf, "funcs.tmpl", context)
 	}
@@ -237,22 +241,24 @@ func (s *Schema) Values(name string, l *Link) []string {
 	return values
 }
 
-// UniqueLinks returns a list of links, unique by title.
+// CheckForDuplicateTitles ensures that all titles are unique for a schema.
 //
-// If more than one link in a given schema has the same title, only the one
-// appearing last will appear in this list. This matches the behavior of the
-// heroics Ruby gem and avoids generating structs and funcs with duplicate
-// names.
-func (s *Schema) UniqueLinks() []*Link {
+// If more than one link in a given schema has the same title, we cannot
+// accurately generate the client from the schema. Although it's not strictly a
+// schema violation, it needs to be fixed before the client can be properly
+// generated.
+func (s *Schema) CheckForDuplicateTitles() bool {
 	titles := map[string]bool{}
 	var uniqueLinks []*Link
 	for _, link := range s.Links {
-		if _, ok := titles[link.Title]; !ok {
+		title := strings.ToLower(link.Title)
+		if _, ok := titles[title]; !ok {
 			uniqueLinks = append(uniqueLinks, link)
 		}
-		titles[link.Title] = true
+		titles[title] = true
 	}
-	return uniqueLinks
+
+	return len(uniqueLinks) != len(s.Links)
 }
 
 // URL returns schema base URL.

gen_test.go

@@ -616,3 +616,63 @@ func TestValues(t *testing.T) {
 		}
 	}
 }
+
+var linkTitleTests = []struct {
+	Schema   *Schema
+	Expected bool
+}{
+	{
+		Schema: &Schema{
+			Title: "Selfreferencing",
+			Type:  "object",
+			Links: []*Link{
+				{
+					Title: "Create",
+				},
+				{
+					Title: "Create",
+				},
+			},
+		},
+		Expected: true,
+	},
+	{
+		Schema: &Schema{
+			Title: "Selfreferencing",
+			Type:  "object",
+			Links: []*Link{
+				{
+					Title: "update",
+				},
+				{
+					Title: "Update",
+				},
+			},
+		},
+		Expected: true,
+	},
+	{
+		Schema: &Schema{
+			Title: "Selfreferencing",
+			Type:  "object",
+			Links: []*Link{
+				{
+					Title: "Create",
+				},
+				{
+					Title: "Delete",
+				},
+			},
+		},
+		Expected: false,
+	},
+}
+
+func TestLinkTitles(t *testing.T) {
+	for i, lt := range linkTitleTests {
+		resp := lt.Schema.CheckForDuplicateTitles()
+		if resp != lt.Expected {
+			t.Errorf("%d: wants %v, got %v", i, lt.Expected, resp)
+		}
+	}
+}

templates/funcs.tmpl

@@ -1,6 +1,6 @@
 {{$Name := .Name}}
 {{$Def := .Definition}}
-{{range .Definition.UniqueLinks}}
+{{range .Definition.Links}}
   {{if .AcceptsCustomType}}
    type {{paramType $Name .}} {{linkGoType .}}
   {{end}}
@@ -19,4 +19,3 @@
     {{end}}
   }
 {{end}}
-

templates/templates.go

@@ -6,7 +6,7 @@ var templates = map[string]string{"field.tmpl": `{{initialCap .Name}} {{.Type}}
 `,
 	"funcs.tmpl": `{{$Name := .Name}}
 {{$Def := .Definition}}
-{{range .Definition.UniqueLinks}}
+{{range .Definition.Links}}
   {{if .AcceptsCustomType}}
    type {{paramType $Name .}} {{linkGoType .}}
   {{end}}
@@ -263,4 +263,3 @@ func Parse(t *template.Template) (*template.Template, error) {
 	}
 	return t, nil
 }
-