Merge pull request 99designs#542 from Elgarni/add-more-validation-che…

…cks-on-yml-config-file

Add more validation checks on .yml config file

cmd/gen.go

@@ -46,11 +46,6 @@ var genCmd = cli.Command{
 			config.SchemaStr[filename] = string(schemaRaw)
 		}
 
-		if err = config.Check(); err != nil {
-			fmt.Fprintln(os.Stderr, "invalid config format: "+err.Error())
-			os.Exit(1)
-		}
-
 		err = codegen.Generate(*config)
 		if err != nil {
 			fmt.Fprintln(os.Stderr, err.Error())

cmd/init.go

@@ -78,11 +78,6 @@ func GenerateGraphServer(config *codegen.Config, serverFilename string) {
 		config.SchemaStr[filename] = string(schemaRaw)
 	}
 
-	if err := config.Check(); err != nil {
-		fmt.Fprintln(os.Stderr, "invalid config format: "+err.Error())
-		os.Exit(1)
-	}
-
 	if err := codegen.Generate(*config); err != nil {
 		fmt.Fprintln(os.Stderr, err.Error())
 		os.Exit(1)

codegen/codegen.go

@@ -1,6 +1,7 @@
 package codegen
 
 import (
+	"fmt"
 	"log"
 	"os"
 	"path/filepath"
@@ -19,6 +20,9 @@ func Generate(cfg Config) error {
 		return err
 	}
 
+	if err := cfg.check(); err != nil {
+		return fmt.Errorf("invalid config format: " + err.Error())
+	}
 	_ = syscall.Unlink(cfg.Exec.Filename)
 	_ = syscall.Unlink(cfg.Model.Filename)
 

codegen/config.go

@@ -178,7 +178,7 @@ func (c *PackageConfig) IsDefined() bool {
 	return c.Filename != ""
 }
 
-func (cfg *Config) Check() error {
+func (cfg *Config) check() error {
 	if err := cfg.Models.Check(); err != nil {
 		return errors.Wrap(err, "config.models")
 	}
@@ -191,6 +191,29 @@ func (cfg *Config) Check() error {
 	if err := cfg.Resolver.Check(); err != nil {
 		return errors.Wrap(err, "config.resolver")
 	}
+
+	// check packages names against conflict, if present in the same dir
+	// and check filenames for uniqueness
+	packageConfigList := []PackageConfig{
+		cfg.Model,
+		cfg.Exec,
+		cfg.Resolver,
+	}
+	filesMap := make(map[string]bool)
+	pkgConfigsByDir := make(map[string]PackageConfig)
+	for _, current := range packageConfigList {
+		_, fileFound := filesMap[current.Filename]
+		if fileFound {
+			return fmt.Errorf("filename %s defined more than once", current.Filename)
+		}
+		filesMap[current.Filename] = true
+		previous, inSameDir := pkgConfigsByDir[current.Dir()]
+		if inSameDir && current.Package != previous.Package {
+			return fmt.Errorf("filenames %s and %s are in the same directory but have different package definitions", stripPath(current.Filename), stripPath(previous.Filename))
+		}
+		pkgConfigsByDir[current.Dir()] = current
+	}
+
 	return nil
 }
 
@@ -271,3 +294,7 @@ func findCfgInDir(dir string) string {
 	}
 	return ""
 }
+
+func stripPath(path string) string {
+	return filepath.Base(path)
+}

codegen/config_test.go

@@ -79,3 +79,16 @@ func TestReferencedPackages(t *testing.T) {
 	})
 
 }
+
+func TestConfigCheck(t *testing.T) {
+	t.Run("invalid config format due to conflicting package names", func(t *testing.T) {
+		config, err := LoadConfig("testdata/cfg/conflictedPackages.yml")
+		require.NoError(t, err)
+
+		err = config.normalize()
+		require.NoError(t, err)
+
+		err = config.check()
+		require.EqualError(t, err, "filenames exec.go and models.go are in the same directory but have different package definitions")
+	})
+}

codegen/testdata/cfg/conflictedPackages.yml

@@ -0,0 +1,10 @@
+schema:
+  - schema.graphql
+exec:
+  filename: generated/exec.go
+  package: graphql
+model:
+  filename: generated/models.go
+resolver:
+  filename: generated/resolver.go
+  type: Resolver