Repository: go-swagger/go-swagger (go)

Issue:
"generate operation" returns "no operations were selected"
## Problem statement
```
$ ./swagger generate operation --name=
2020/04/26 20:25:14 validating spec /home/borisov/tmp/swagger/swagger.yml
2020/04/26 20:25:15 preprocessing spec with option:  minimal flattening
no operations were selected
```
If path has `operationId` defined then nothing is generated. If that property is removed then code is generated correctly:

```
$ ./swagger generate operation --name=
2020/04/26 20:31:21 validating spec /home/borisov/tmp/swagger/swagger.yml
2020/04/26 20:31:21 preprocessing spec with option:  minimal flattening
2020/04/26 20:31:21 rendering 4 templates for operation 
2020/04/26 20:31:21 name field GetFoo
2020/04/26 20:31:21 package field operations
2020/04/26 20:31:21 creating generated file "get_foo_parameters.go" in "restapi/operations" as parameters
2020/04/26 20:31:21 executed template asset:serverParameter
2020/04/26 20:31:21 name field GetFoo
2020/04/26 20:31:21 package field operations
2020/04/26 20:31:21 creating generated file "get_foo_urlbuilder.go" in "restapi/operations" as urlbuilder
2020/04/26 20:31:21 executed template asset:serverUrlbuilder
2020/04/26 20:31:21 name field GetFoo
2020/04/26 20:31:21 package field operations
2020/04/26 20:31:21 creating generated file "get_foo_responses.go" in "restapi/operations" as responses
2020/04/26 20:31:21 executed template asset:serverResponses
2020/04/26 20:31:21 name field GetFoo
2020/04/26 20:31:21 package field operations
2020/04/26 20:31:21 creating generated file "get_foo.go" in "restapi/operations" as handler
2020/04/26 20:31:21 executed template asset:serverOperation
2020/04/26 20:31:21 Generation completed!

For this generation to compile you need to have some packages in your GOPATH:

        * github.com/go-openapi/runtime

You can get these now with: go get -u -f ./...
```

## Swagger specification
```
$ cat swagger.yml 
consumes:
- application/json
info:
  title: Swagger
  version: 0.1.0
paths:
  /foo:
    get:
      operationId: list
      responses:
        200:
          description: Successful response
          schema:
              type: object
              required:
                - data
              properties:
                data:
                  type: string
produces:
- application/json
schemes:
- http
swagger: "2.0"
```
```
$ ./swagger validate swagger.yml 
2020/04/26 20:22:27 
The swagger spec at "swagger.yml" is valid against swagger specification 2.0
```

## Environment
```
$ ./swagger version
version: v0.23.0
commit: 3596b40bedd4f06cc918767098568456d2ec2370
```
```
$ go version
go version go1.14.2 linux/amd64
```

Pull request:
fix(cli): filter out empty args passed by CLI to select operations
* Also fixed similar issue with selected models

* fixes #2280

Also in this commit:
* ordered operations in dump data

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>

Patch:
diff --git a/cmd/swagger/commands/generate/operation.go b/cmd/swagger/commands/generate/operation.go
index 60600dc0..dc56babd 100644
--- a/cmd/swagger/commands/generate/operation.go
+++ b/cmd/swagger/commands/generate/operation.go
@@ -59,7 +59,7 @@ type Operation struct {
 	NoResponses  bool `long:"skip-responses" description:"when present will not generate the response model struct"`
 	NoURLBuilder bool `long:"skip-url-builder" description:"when present will not generate a URL builder"`
 
-	Name []string `long:"name" short:"n" required:"true" description:"the operations to generate, repeat for multiple (defaults to all). Same as --operations"`
+	Name []string `long:"name" short:"n" description:"the operations to generate, repeat for multiple (defaults to all). Same as --operations"`
 }
 
 func (o Operation) apply(opts *generator.GenOpts) {
@@ -94,7 +94,7 @@ You can get these now with: go get -u -f %s/...
 
 // Execute generates a model file
 func (o *Operation) Execute(args []string) error {
-	if o.Shared.DumpData && (len(o.Name) > 1 || len(o.Operations.Operations) > 1) {
+	if o.Shared.DumpData && len(append(o.Name, o.Operations.Operations...)) > 1 {
 		return errors.New("only 1 operation at a time is supported for dumping data")
 	}
 
diff --git a/fixtures/bugs/2278/fixture-2278.yaml b/fixtures/bugs/2278/fixture-2278.yaml
new file mode 100644
index 00000000..db691427
--- /dev/null
+++ b/fixtures/bugs/2278/fixture-2278.yaml
@@ -0,0 +1,25 @@
+---
+swagger: "2.0"
+info:
+  title: foo api
+  version: "1.0.0"
+paths:
+  /version:
+    get:
+      responses:
+        "200":
+          description: foo version
+          schema:
+            type: string
+      summary: foo version
+      operationId: showVersion
+  /withtags:
+    get:
+      tags: [ "sometag" ]
+      responses:
+        "200":
+          description: foo tagged version
+          schema:
+            type: string
+      summary: foo tagged version
+      operationId: showTaggedVersion
diff --git a/generator/bindata.go b/generator/bindata.go
index 89461f8d..342af6c1 100644
--- a/generator/bindata.go
+++ b/generator/bindata.go
@@ -58,7 +58,7 @@ import (
 func bindataRead(data []byte, name string) ([]byte, error) {
 	gz, err := gzip.NewReader(bytes.NewBuffer(data))
 	if err != nil {
-		return nil, fmt.Errorf("read %q: %w", name, err)
+		return nil, fmt.Errorf("read %q: %v", name, err)
 	}
 
 	var buf bytes.Buffer
@@ -66,7 +66,7 @@ func bindataRead(data []byte, name string) ([]byte, error) {
 	clErr := gz.Close()
 
 	if err != nil {
-		return nil, fmt.Errorf("read %q: %w", name, err)
+		return nil, fmt.Errorf("read %q: %v", name, err)
 	}
 	if clErr != nil {
 		return nil, err
@@ -1019,9 +1019,6 @@ var _bindata = map[string]func() (*asset, error){
 	"templates/validation/structfield.gotmpl":                     templatesValidationStructfieldGotmpl,
 }
 
-// AssetDebug is true if the assets were built with the debug flag enabled.
-const AssetDebug = false
-
 // AssetDir returns the file names below a certain
 // directory embedded in the file by go-bindata.
 // For example if you run go-bindata on data/... and data contains the
diff --git a/generator/client.go b/generator/client.go
index 38b0fb47..65915bc6 100644
--- a/generator/client.go
+++ b/generator/client.go
@@ -59,7 +59,7 @@ func GenerateClient(name string, modelNames, operationIDs []string, opts *GenOpt
 		ServerPackage:     opts.LanguageOpts.ManglePackagePath(opts.ServerPackage, defaultServerTarget),
 		ClientPackage:     opts.LanguageOpts.ManglePackagePath(opts.ClientPackage, defaultClientTarget),
 		OperationsPackage: opts.LanguageOpts.ManglePackagePath(opts.ClientPackage, defaultClientTarget),
-		Principal:         opts.Principal,
+		Principal:         opts.PrincipalAlias(),
 		DefaultScheme:     opts.DefaultScheme,
 		DefaultProduces:   opts.DefaultProduces,
 		DefaultConsumes:   opts.DefaultConsumes,
diff --git a/generator/operation.go b/generator/operation.go
index 0ea896d8..6c6b4c84 100644
--- a/generator/operation.go
+++ b/generator/operation.go
@@ -53,7 +53,8 @@ func sortedResponses(input map[int]spec.Response) responses {
 	return res
 }
 
-// GenerateServerOperation generates a parameter model, parameter validator, http handler implementations for a given operation
+// GenerateServerOperation generates a parameter model, parameter validator, http handler implementations for a given operation.
+//
 // It also generates an operation handler interface that uses the parameter model for handling a valid request.
 // Allows for specifying a list of tags to include only certain tags for the generation
 func GenerateServerOperation(operationNames []string, opts *GenOpts) error {
@@ -92,7 +93,7 @@ func GenerateServerOperation(operationNames []string, opts *GenOpts) error {
 			Operation:            *operation,
 			SecurityRequirements: analyzed.SecurityRequirementsFor(operation),
 			SecurityDefinitions:  analyzed.SecurityDefinitionsFor(operation),
-			Principal:            opts.Principal,
+			Principal:            opts.PrincipalAlias(),
 			Target:               filepath.Join(opts.Target, filepath.FromSlash(serverPackage)),
 			Base:                 opts.Target,
 			Tags:                 opts.Tags,
@@ -157,7 +158,7 @@ func (o *operationGenerator) Generate() error {
 
 	bldr := codeGenOpBuilder{
 		ModelsPackage:       o.ModelsPackage,
-		Principal:           o.Principal,
+		Principal:           o.GenOpts.PrincipalAlias(),
 		Target:              o.Target,
 		DefaultImports:      defaultImports,
 		Imports:             imports,
@@ -1211,13 +1212,29 @@ func maxInt(a, b int) int {
 // deconflictTag ensures generated packages for operations based on tags do not conflict
 // with other imports
 func deconflictTag(seenTags []string, pkg string) string {
+	return deconflictPkg(pkg, func(pkg string) string { return renameOperationPackage(seenTags, pkg) })
+}
+
+// deconflictPrincipal ensures that whenever an external principal package is added, it doesn't conflict
+// with standard inports
+func deconflictPrincipal(pkg string) string {
+	switch pkg {
+	case "principal":
+		return renamePrincipalPackage(pkg)
+	default:
+		return deconflictPkg(pkg, renamePrincipalPackage)
+	}
+}
+
+// deconflictPkg renames package names which conflict with standard imports
+func deconflictPkg(pkg string, renamer func(string) string) string {
 	switch pkg {
 	case "api", "httptransport":
 		fallthrough
 	case "errors", "runtime", "middleware", "security", "spec", "strfmt", "loads", "swag", "validate":
 		fallthrough
 	case "tls", "http", "fmt", "strings", "log":
-		return renameOperationPackage(seenTags, pkg)
+		return renamer(pkg)
 	}
 	return pkg
 }
@@ -1232,3 +1249,7 @@ func renameOperationPackage(seenTags []string, pkg string) string {
 	}
 	return current
 }
+
+func renamePrincipalPackage(pkg string) string {
+	return "auth"
+}
diff --git a/generator/shared.go b/generator/shared.go
index b8ae3049..c84e9c5f 100644
--- a/generator/shared.go
+++ b/generator/shared.go
@@ -397,6 +397,10 @@ func (g *GenOpts) EnsureDefaults() error {
 	// always include validator with models
 	g.IncludeValidator = g.IncludeModel
 
+	if g.Principal == "" {
+		g.Principal = iface
+	}
+
 	g.defaultsEnsured = true
 	return nil
 }
@@ -685,14 +689,26 @@ func (g *GenOpts) defaultImports() map[string]string {
 	defaultImports := make(map[string]string, 50)
 
 	if g.ExistingModels == "" {
+		// generated models
 		importPath := path.Join(
 			baseImport,
 			g.LanguageOpts.ManglePackagePath(g.ModelPackage, defaultModelsTarget))
 		defaultImports[g.LanguageOpts.ManglePackageName(g.ModelPackage, defaultModelsTarget)] = importPath
 	} else {
-		// TODO(fredbi): mangle existing model pkg aliases
+		// external models
 		importPath := g.LanguageOpts.ManglePackagePath(g.ExistingModels, "")
-		defaultImports[importAlias(importPath)] = importPath
+		defaultImports["models"] = importPath
+	}
+
+	alias, _, target := g.resolvePrincipal()
+	if alias != "" {
+		if pth, _ := path.Split(target); pth != "" {
+			// if principal is specified with an path, generate this import
+			defaultImports[alias] = target
+		} else {
+			// if principal is specified with a relative path, assume it is located in generated target
+			defaultImports[alias] = path.Join(baseImport, target)
+		}
 	}
 	return defaultImports
 }
@@ -708,12 +724,31 @@ func (g *GenOpts) initImports(operationsPackage string) map[string]string {
 	return imports
 }
 
+// PrincipalAlias returns an aliased type to the principal
+func (g *GenOpts) PrincipalAlias() string {
+	_, principal, _ := g.resolvePrincipal()
+	return principal
+}
+
+func (g *GenOpts) resolvePrincipal() (string, string, string) {
+	dotLocation := strings.LastIndex(g.Principal, ".")
+	if dotLocation < 0 {
+		return "", g.Principal, ""
+	}
+
+	// handle possible conflicts with injected principal package
+	// NOTE(fred): we do not check here for conflicts with packages created from operation tags, only standard imports
+	alias := deconflictPrincipal(importAlias(g.Principal[:dotLocation]))
+	return alias, alias + g.Principal[dotLocation:], g.Principal[:dotLocation]
+}
+
 func fileExists(target, name string) bool {
 	_, err := os.Stat(filepath.Join(target, name))
 	return !os.IsNotExist(err)
 }
 
 func gatherModels(specDoc *loads.Document, modelNames []string) (map[string]spec.Schema, error) {
+	modelNames = pruneEmpty(modelNames)
 	models, mnc := make(map[string]spec.Schema), len(modelNames)
 	defs := specDoc.Spec().Definitions
 
@@ -755,13 +790,17 @@ func titleOrDefault(specDoc *loads.Document, name, defaultName string) string {
 }
 
 func mainNameOrDefault(specDoc *loads.Document, name, defaultName string) string {
-	// _test won't do as main server name
+	// *_test won't do as main server name
 	return strings.TrimSuffix(titleOrDefault(specDoc, name, defaultName), "Test")
 }
 
 func appNameOrDefault(specDoc *loads.Document, name, defaultName string) string {
-	// _test_api, _api_test, _test, _api won't do as app names
-	return strings.TrimSuffix(strings.TrimSuffix(strings.TrimSuffix(titleOrDefault(specDoc, name, defaultName), "Test"), "API"), "Test")
+	// *_test won't do as app names
+	name = strings.TrimSuffix(titleOrDefault(specDoc, name, defaultName), "Test")
+	if name == "" {
+		name = swag.ToGoName(defaultName)
+	}
+	return name
 }
 
 type opRef struct {
@@ -779,6 +818,7 @@ func (o opRefs) Swap(i, j int)      { o[i], o[j] = o[j], o[i] }
 func (o opRefs) Less(i, j int) bool { return o[i].Key < o[j].Key }
 
 func gatherOperations(specDoc *analysis.Spec, operationIDs []string) map[string]opRef {
+	operationIDs = pruneEmpty(operationIDs)
 	var oprefs opRefs
 
 	for method, pathItem := range specDoc.Operations() {
diff --git a/generator/structs.go b/generator/structs.go
index 23660ec0..6a7d44a7 100644
--- a/generator/structs.go
+++ b/generator/structs.go
@@ -403,13 +403,20 @@ func (g GenStatusCodeResponses) Swap(i, j int)      { g[i], g[j] = g[j], g[i] }
 func (g GenStatusCodeResponses) Less(i, j int) bool { return g[i].Code < g[j].Code }
 
 // MarshalJSON marshals these responses to json
+//
+// This is used by DumpData.
 func (g GenStatusCodeResponses) MarshalJSON() ([]byte, error) {
 	if g == nil {
 		return nil, nil
 	}
+	responses := make(GenStatusCodeResponses, len(g))
+	copy(responses, g)
+	// order marshalled output
+	sort.Sort(responses)
+
 	var buf bytes.Buffer
 	buf.WriteRune('{')
-	for i, v := range g {
+	for i, v := range responses {
 		rb, err := json.Marshal(v)
 		if err != nil {
 			return nil, err
diff --git a/generator/support.go b/generator/support.go
index bb9e0c11..9889cd62 100644
--- a/generator/support.go
+++ b/generator/support.go
@@ -96,7 +96,7 @@ func newAppGenerator(name string, modelNames, operationIDs []string, opts *GenOp
 		ServerPackage:     opts.LanguageOpts.ManglePackagePath(opts.ServerPackage, defaultServerTarget),
 		ClientPackage:     opts.LanguageOpts.ManglePackagePath(opts.ClientPackage, defaultClientTarget),
 		OperationsPackage: filepath.Join(opts.LanguageOpts.ManglePackagePath(opts.ServerPackage, defaultServerTarget), apiPackage),
-		Principal:         opts.Principal,
+		Principal:         opts.PrincipalAlias(),
 		DefaultScheme:     opts.DefaultScheme,
 		DefaultProduces:   opts.DefaultProduces,
 		DefaultConsumes:   opts.DefaultConsumes,
@@ -196,9 +196,6 @@ func (a *appGenerator) GenerateSupport(ap *GenApp) error {
 }
 
 func (a *appGenerator) makeSecuritySchemes() GenSecuritySchemes {
-	if a.Principal == "" {
-		a.Principal = "interface{}"
-	}
 	requiredSecuritySchemes := make(map[string]spec.SecurityScheme, len(a.Analyzed.RequiredSecuritySchemes()))
 	for _, scheme := range a.Analyzed.RequiredSecuritySchemes() {
 		if req, ok := a.SpecDoc.Spec().SecurityDefinitions[scheme]; ok && req != nil {
@@ -262,7 +259,7 @@ func (a *appGenerator) makeCodegenApp() (GenApp, error) {
 
 		bldr := codeGenOpBuilder{
 			ModelsPackage:    a.ModelsPackage,
-			Principal:        a.Principal,
+			Principal:        a.GenOpts.PrincipalAlias(),
 			Target:           a.Target,
 			DefaultImports:   defaultImports,
 			Imports:          imports,
@@ -418,7 +415,7 @@ func (a *appGenerator) makeCodegenApp() (GenApp, error) {
 		Models:              genModels,
 		Operations:          genOps,
 		OperationGroups:     opGroups,
-		Principal:           a.Principal,
+		Principal:           a.GenOpts.PrincipalAlias(),
 		SwaggerJSON:         generateReadableSpec(jsonb),
 		FlatSwaggerJSON:     generateReadableSpec(flatjsonb),
 		ExcludeSpec:         a.GenOpts.ExcludeSpec,


Test patch:
diff --git a/generator/build_test.go b/generator/build_test.go
index aef6d7c0..979639ec 100644
--- a/generator/build_test.go
+++ b/generator/build_test.go
@@ -42,6 +42,9 @@ func TestGenerateAndBuild(t *testing.T) {
 		"issue 2111": {
 			"../fixtures/bugs/2111/fixture-2111.yaml",
 		},
+		"issue 2278": {
+			"../fixtures/bugs/2278/fixture-2278.yaml",
+		},
 	}
 
 	for name, cas := range cases {
diff --git a/generator/client_test.go b/generator/client_test.go
index e309c91b..e56d3db6 100644
--- a/generator/client_test.go
+++ b/generator/client_test.go
@@ -266,6 +266,11 @@ func TestClient(t *testing.T) {
 				assert.True(t, fileExists(target, "operations"))
 			},
 		},
+		{
+			name:      "name with trailing API",
+			spec:      filepath.Join("..", "fixtures", "bugs", "2278", "fixture-2278.yaml"),
+			wantError: false,
+		},
 	}
 
 	for i, tt := range tests {
diff --git a/generator/shared_test.go b/generator/shared_test.go
index c54592d4..d89a7f41 100644
--- a/generator/shared_test.go
+++ b/generator/shared_test.go
@@ -511,6 +511,8 @@ func TestShared_AppNameOrDefault(t *testing.T) {
 	require.NotNil(t, specDoc.Spec().Info)
 	specDoc.Spec().Info.Title = "    "
 	assert.Equal(t, "Xyz", appNameOrDefault(specDoc, "  ", "xyz"))
+	specDoc.Spec().Info.Title = "test"
+	assert.Equal(t, "Xyz", appNameOrDefault(specDoc, "  ", "xyz"))
 }
 
 func TestShared_GatherModel(t *testing.T) {
@@ -560,3 +562,127 @@ func TestShared_DumpWrongData(t *testing.T) {
 		B: "xyz",
 	}))
 }
+
+func TestResolvePrincipal(t *testing.T) {
+	for _, toPin := range []struct {
+		Title     string
+		Principal string
+		Expected  []string
+	}{
+		{
+			Title: "defaults", Principal: "",
+			Expected: []string{"", "interface{}", ""},
+		},
+		{
+			Title: "with base import", Principal: "auth.Principal",
+			Expected: []string{"auth", "auth.Principal", "auth"},
+		},
+		{
+			Title: "with full import", Principal: "github.com/myproject/auth.Principal",
+			Expected: []string{"auth", "auth.Principal", "github.com/myproject/auth"},
+		},
+		{
+			Title: "with name conflict", Principal: "github.com/myproject/middleware.Principal",
+			Expected: []string{"auth", "auth.Principal", "github.com/myproject/middleware"},
+		},
+		{
+			Title: "with name conflict (2)", Principal: "github.com/myproject/principal.Principal",
+			Expected: []string{"auth", "auth.Principal", "github.com/myproject/principal"},
+		},
+	} {
+		fixture := toPin
+		t.Run(fixture.Title, func(t *testing.T) {
+			t.Parallel()
+			opts := &GenOpts{Principal: fixture.Principal}
+			err := opts.EnsureDefaults()
+			require.NoError(t, err)
+			alias, principal, target := opts.resolvePrincipal()
+			require.Equal(t, fixture.Expected[0], alias)
+			require.Equal(t, fixture.Expected[1], principal)
+			require.Equal(t, fixture.Expected[2], target)
+		})
+	}
+}
+
+func TestDefaultImports(t *testing.T) {
+	for i, toPin := range []struct {
+		Title    string
+		Opts     *GenOpts
+		Expected map[string]string
+	}{
+		{
+			Title: "defaults",
+			Opts:  &GenOpts{},
+			Expected: map[string]string{
+				"models": "github.com/go-swagger/go-swagger/generator/models",
+			},
+		},
+		{
+			Title: "with base import",
+			Opts: &GenOpts{
+				Principal: "ext.Principal",
+			},
+			Expected: map[string]string{
+				"ext":    "github.com/go-swagger/go-swagger/generator/ext",
+				"models": "github.com/go-swagger/go-swagger/generator/models",
+			},
+		},
+		{
+			Title: "with full import",
+			Opts: &GenOpts{
+				Principal: "github.com/myproject/identity.Principal",
+			},
+			Expected: map[string]string{
+				"identity": "github.com/myproject/identity",
+				"models":   "github.com/go-swagger/go-swagger/generator/models",
+			},
+		},
+		{
+			Title: "with name conflict",
+			Opts: &GenOpts{
+				Principal: "github.com/myproject/middleware.Principal",
+			},
+			Expected: map[string]string{
+				"auth":   "github.com/myproject/middleware",
+				"models": "github.com/go-swagger/go-swagger/generator/models",
+			},
+		},
+		{
+			Title: "with name conflict (2)",
+			Opts: &GenOpts{
+				Principal: "github.com/myproject/principal.Principal",
+			},
+			Expected: map[string]string{
+				"auth":   "github.com/myproject/principal",
+				"models": "github.com/go-swagger/go-swagger/generator/models",
+			},
+		},
+		{
+			Title: "alternate target for models",
+			Opts: &GenOpts{
+				ModelPackage: "target/bespoke",
+			},
+			Expected: map[string]string{
+				"bespoke": "github.com/go-swagger/go-swagger/generator/target/bespoke",
+			},
+		},
+		{
+			Title: "with existing models",
+			Opts: &GenOpts{
+				ExistingModels: "github.com/myproject/target/bespoke",
+			},
+			Expected: map[string]string{
+				"models": "github.com/myproject/target/bespoke",
+			},
+		},
+	} {
+		fixture := toPin
+		t.Run(fixture.Title, func(t *testing.T) {
+			t.Parallel()
+			err := fixture.Opts.EnsureDefaults()
+			require.NoError(t, err)
+			imports := fixture.Opts.defaultImports()
+			require.EqualValuesf(t, fixture.Expected, imports, "unexpected imports generated with fixture %q[%d]", fixture.Title, i)
+		})
+	}
+}