From 7d394222022613afcbae9b7dbc3083eb7ce0c1fe Mon Sep 17 00:00:00 2001 From: Andreas Wiede Date: Wed, 6 Feb 2019 15:31:25 -0500 Subject: [PATCH 1/5] Fix typo in README Fix typo in README in selection example directory to point to the selection example, not the todo example. --- example/selection/readme.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/example/selection/readme.md b/example/selection/readme.md index e8dc2e8e183..f37be6eb6a9 100644 --- a/example/selection/readme.md +++ b/example/selection/readme.md @@ -1,10 +1,10 @@ -### todo app +### selection app This is the simplest example of a graphql server. to run this server ```bash -go run ./example/todo/server/server.go +go run ./example/selection/server/server.go ``` -and open http://localhost:8081 in your browser +and open http://localhost:8086 in your browser From 3645cd3ecf6c11cde04e65493afb7d3db34e04dd Mon Sep 17 00:00:00 2001 From: Abdullah Algarni Date: Sat, 9 Feb 2019 14:45:37 +0300 Subject: [PATCH 2/5] Add more validation checks on .yml config file --- codegen/config.go | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/codegen/config.go b/codegen/config.go index f9df24fb3f3..7326727014c 100644 --- a/codegen/config.go +++ b/codegen/config.go @@ -191,6 +191,47 @@ 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 i, current := range packageConfigList { + if i == 0 { + filesMap[current.Filename] = true + pkgConfigsByDir[current.Dir()] = []PackageConfig{current} + continue + } + _, fileFound := filesMap[current.Filename] + if fileFound { + return fmt.Errorf("filename %s defined more than once", current.Filename) + } + filesMap[current.Filename] = true + prevPkgList, inSameDir := pkgConfigsByDir[current.Dir()] + if inSameDir { + for _, previous := range prevPkgList { + if current.Package != previous.Package { + eitherPackageEmpty := previous.Package != "" || current.Package != ""; + if eitherPackageEmpty { + if current.Package == filepath.Base(current.Dir()) && previous.Package == "" { + break; + } + if previous.Package == filepath.Base(previous.Dir()) && current.Package == "" { + break + } + return fmt.Errorf("filenames %s and %s are in the same directory but have different package definitions", current.Filename, previous.Filename) + } + } + } + } + pkgConfigsByDir[current.Dir()] = append(pkgConfigsByDir[current.Dir()], current) + } + return nil } From 8ac0f6e4e0ae01485bf240286533c2cc1da42d81 Mon Sep 17 00:00:00 2001 From: Abdullah Algarni Date: Sat, 9 Feb 2019 15:58:36 +0300 Subject: [PATCH 3/5] Removed redundant semicolons --- codegen/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codegen/config.go b/codegen/config.go index 7326727014c..4319953ba18 100644 --- a/codegen/config.go +++ b/codegen/config.go @@ -216,10 +216,10 @@ func (cfg *Config) Check() error { if inSameDir { for _, previous := range prevPkgList { if current.Package != previous.Package { - eitherPackageEmpty := previous.Package != "" || current.Package != ""; + eitherPackageEmpty := previous.Package != "" || current.Package != "" if eitherPackageEmpty { if current.Package == filepath.Base(current.Dir()) && previous.Package == "" { - break; + break } if previous.Package == filepath.Base(previous.Dir()) && current.Package == "" { break From b56cb659d36b090b0c26277c061d2b9a4ed2c4a9 Mon Sep 17 00:00:00 2001 From: Abdullah Algarni Date: Mon, 11 Feb 2019 17:37:02 +0300 Subject: [PATCH 4/5] Refactored config check so that it runs after being normalized --- cmd/gen.go | 5 ----- cmd/init.go | 5 ----- codegen/codegen.go | 4 ++++ codegen/config.go | 36 +++++++++++------------------------- 4 files changed, 15 insertions(+), 35 deletions(-) diff --git a/cmd/gen.go b/cmd/gen.go index 3842f02b37d..97bd6342f1e 100644 --- a/cmd/gen.go +++ b/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()) diff --git a/cmd/init.go b/cmd/init.go index 1e7c18b9327..6ad7e58c811 100644 --- a/cmd/init.go +++ b/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) diff --git a/codegen/codegen.go b/codegen/codegen.go index 773e3db7cc4..8aadd48cd2d 100644 --- a/codegen/codegen.go +++ b/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) diff --git a/codegen/config.go b/codegen/config.go index 4319953ba18..9826ca5dfe5 100644 --- a/codegen/config.go +++ b/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") } @@ -200,36 +200,18 @@ func (cfg *Config) Check() error { cfg.Resolver, } filesMap := make(map[string]bool) - pkgConfigsByDir := make(map[string][]PackageConfig) - for i, current := range packageConfigList { - if i == 0 { - filesMap[current.Filename] = true - pkgConfigsByDir[current.Dir()] = []PackageConfig{current} - continue - } + 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 - prevPkgList, inSameDir := pkgConfigsByDir[current.Dir()] - if inSameDir { - for _, previous := range prevPkgList { - if current.Package != previous.Package { - eitherPackageEmpty := previous.Package != "" || current.Package != "" - if eitherPackageEmpty { - if current.Package == filepath.Base(current.Dir()) && previous.Package == "" { - break - } - if previous.Package == filepath.Base(previous.Dir()) && current.Package == "" { - break - } - return fmt.Errorf("filenames %s and %s are in the same directory but have different package definitions", current.Filename, previous.Filename) - } - } - } + 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()] = append(pkgConfigsByDir[current.Dir()], current) + pkgConfigsByDir[current.Dir()] = current } return nil @@ -312,3 +294,7 @@ func findCfgInDir(dir string) string { } return "" } + +func stripPath(path string) string { + return filepath.Base(path) +} From dc925c462705af9e61357d038b6a3f869f1f3157 Mon Sep 17 00:00:00 2001 From: Abdullah Algarni Date: Mon, 11 Feb 2019 17:39:04 +0300 Subject: [PATCH 5/5] Added a test for config checking --- codegen/config_test.go | 13 +++++++++++++ codegen/testdata/cfg/conflictedPackages.yml | 10 ++++++++++ 2 files changed, 23 insertions(+) create mode 100644 codegen/testdata/cfg/conflictedPackages.yml diff --git a/codegen/config_test.go b/codegen/config_test.go index a97b9d20f56..09a20abf54d 100644 --- a/codegen/config_test.go +++ b/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") + }) +} diff --git a/codegen/testdata/cfg/conflictedPackages.yml b/codegen/testdata/cfg/conflictedPackages.yml new file mode 100644 index 00000000000..ec6ceea515a --- /dev/null +++ b/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