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

Issue:
Custom principal import is missing from generated server code
## Problem statement
I generate my swagger server using `./swagger generate server --spec=api/swagger.yaml -P auth.Agent`. I expect the agent gets added as an argument to my handler func. This used to work fine but now the generated server doesn't compile because it doesn't import my `auth` package.

Recently, I get this:

```go
// Code generated by go-swagger; DO NOT EDIT.

package cat

// This file was generated by the swagger tool.
// Editing this file might prove futile when you re-run the generate command

import (
	"net/http"

	"github.com/go-openapi/runtime/middleware"
)

// GetCatHandlerFunc turns a function with the right signature into a get cat handler
type GetCatHandlerFunc func(GetCatParams, *auth.Agent) middleware.Responder

...
```

previously I used to get this:
```go
// Code generated by go-swagger; DO NOT EDIT.

package cat

// This file was generated by the swagger tool.
// Editing this file might prove futile when you re-run the generate command

import (
	"net/http"

	"github.com/go-openapi/runtime/middleware"
	"github.com/my/project/path/to/auth"
        // ^^^notice this is not in the previous snippet ^^^
)

// GetCatHandlerFunc turns a function with the right signature into a get cat handler
type GetCatHandlerFunc func(GetCatParams, *auth.Agent) middleware.Responder

...
```

## Environment
swagger version: 0.23.0
go version: 1.13.10
OS: macOS Mojave 10.14.6


Pull request:
fixed broken imports when specifying principal in external package


* fixes #2283

NOTE:
* default principal is "interface{}"
* principal specified with simple package (e.g. auth.Principal) is assumed to be imported from target generation (base import)
* principal specified with fully qualified name (e.g. github.com/myproject/auth.Principal) is imported from that location

Also in this commit:
* fixes package alias for when specifying existing external models
* more tests on default imports

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>

Patch:
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..e86d626d 100644
--- a/generator/operation.go
+++ b/generator/operation.go
@@ -92,7 +92,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 +157,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 +1211,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 +1248,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..10064f36 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,6 +724,24 @@ 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)
@@ -755,13 +789,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 {
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)
+		})
+	}
+}