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

Issue:
Generated code fails to call the Validate function on embedded structs resulting in incorrect validation
## Problem statement

Referencing schemas doesn't enforce validation of fields.

When a `post` operation defines that its body is `PostRequest`, but `PostRequest` is a reference to `PostFields`, the validation function is empty.

## Swagger specification

```
swagger: '2.0'
info:
  version: 1.0.0
  title: API

paths:
  /test:
    post:
      operationId: testPost
      parameters:
      - in: body
        required: true
        name: postRequest
        schema:
          $ref: '#/definitions/PostRequest'
      responses:
        201:
          description: Test
          schema:
            type: object
            properties:
              msg:
                type: string
                x-isnullable: false
definitions:
  PostRequest:
    $ref: '#/definitions/PostFields'
  PostFields:
    type: object
    required:
    - a
    - b
    properties:
      a:
        type: string
        x-isnullable: false
      b:
        type: string
```

## Steps to reproduce

Assuming the above file is named `swagger.yaml`:

```
swagger generate model -f ./swagger.yaml
```

In the generated code, I'd expect the `Validate` function of the `PostRequest` to validate the `PostFields` struct, but it isn't.

The `PostFields` struct is embedded in the `PostRequest` struct, but the `Validate` method of the `PostFields` is not called by the `Validate` method of the `PostRequest` struct.

This results in the `a` and `b` fields of `PostRequest` not being validated as required.

## What I got

```go
type PostRequest struct {
	PostFields
}

// snip...

// Validate validates this post request
func (m *PostRequest) Validate(formats strfmt.Registry) error {
	return nil
}
```

## What I expected

```go
type PostRequest struct {
	PostFields
}

// snip...

// Validate validates this post request
func (m *PostRequest) Validate(formats strfmt.Registry) error {
        if err := m.PostFields.Validate(formats); err != nil {
		return err
        }
	return nil
}
```

## Environment
version: v0.27.0
commit: 43c2774170504d87b104e3e4d68626aac2cd447d
go version go1.16 darwin/amd64

Pull request:
fix(validations): fixed missing validation in embedded structs
* fixes #2604

This PR fixes the empty Validate methods generated when generating embedded structs (that is, when an object redefines another object with a $ref).

Since the actual need for validation is not known when hitting the embedded struct, always assume a validation.

Exception: when the embedded type is interface{} or io.Reader, we know that we should not call Validate.

Patch:
diff --git a/fixtures/bugs/2604/2604.yaml b/fixtures/bugs/2604/2604.yaml
new file mode 100644
index 00000000..1b78bc3f
--- /dev/null
+++ b/fixtures/bugs/2604/2604.yaml
@@ -0,0 +1,49 @@
+swagger: '2.0'
+info:
+  version: 1.0.0
+  title: API
+
+paths:
+  /test:
+    post:
+      operationId: testPost
+      parameters:
+      - in: body
+        required: true
+        name: postRequest
+        schema:
+          $ref: '#/definitions/PostRequest'
+      responses:
+        201:
+          description: Test
+          schema:
+            type: object
+            properties:
+              msg:
+                type: string
+                x-isnullable: false
+definitions:
+  PostRequest:
+    $ref: '#/definitions/PostFields'
+  PostFields:
+    type: object
+    required:
+    - a
+    - b
+    properties:
+      a:
+        type: string
+        x-isnullable: false
+      b:
+        type: string
+  Composed:
+    $ref: '#/definitions/NoValidations'
+  NoValidations:
+    type: object
+    properties:
+      c:
+        type: string
+  ComposedIface:
+    $ref: '#/definitions/Interface'
+  Interface:
+    type: object
diff --git a/generator/model.go b/generator/model.go
index a30bd04f..c57546a6 100644
--- a/generator/model.go
+++ b/generator/model.go
@@ -441,7 +441,6 @@ type schemaGenContext struct {
 	AdditionalProperty         bool
 	Untyped                    bool
 	Named                      bool
-	RefHandled                 bool
 	IsVirtual                  bool
 	IsTuple                    bool
 	IncludeValidator           bool
@@ -1670,8 +1669,7 @@ func (sg *schemaGenContext) shortCircuitNamedRef() (bool, error) {
 	// NOTE: this assumes that all $ref point to a definition,
 	// i.e. the spec is canonical, as guaranteed by minimal flattening.
 	//
-	// TODO: RefHandled is actually set nowhere
-	if sg.RefHandled || !sg.Named || sg.Schema.Ref.String() == "" {
+	if !sg.Named || sg.Schema.Ref.String() == "" {
 		return false, nil
 	}
 	debugLogAsJSON("short circuit named ref: %q", sg.Schema.Ref.String(), sg.Schema)
@@ -1682,6 +1680,8 @@ func (sg *schemaGenContext) shortCircuitNamedRef() (bool, error) {
 	// check if the $ref points to a simple type or polymorphic (base) type.
 	//
 	// If this is the case, just realias this simple type, without creating a struct.
+	//
+	// In templates this case is identified by .IsSuperAlias = true
 	asch, era := analysis.Schema(analysis.SchemaOpts{
 		Root:     sg.TypeResolver.Doc.Spec(),
 		BasePath: sg.TypeResolver.Doc.SpecFilePath(),
@@ -1732,10 +1732,16 @@ func (sg *schemaGenContext) shortCircuitNamedRef() (bool, error) {
 	}
 
 	// Aliased object: use golang struct composition.
+	// Covers case of a type redefinition like:
+	// thistype:
+	//   $ref: #/definitions/othertype
+	//
 	// This is rendered as a struct with type field, i.e. :
 	// Alias struct {
 	//		AliasedType
 	// }
+	//
+	// In templates, the schema is composed like AllOf.
 	nullableOverride := sg.GenSchema.IsNullable
 
 	tpe := resolvedType{}
@@ -1748,17 +1754,26 @@ func (sg *schemaGenContext) shortCircuitNamedRef() (bool, error) {
 	tpe.IsAnonymous = false
 	tpe.IsNullable = sg.TypeResolver.isNullable(&sg.Schema)
 
-	item := sg.NewCompositionBranch(sg.Schema, 0)
-	if err := item.makeGenSchema(); err != nil {
+	branch := sg.NewCompositionBranch(sg.Schema, 0)
+	if err := branch.makeGenSchema(); err != nil {
 		return true, err
 	}
 	sg.GenSchema.resolvedType = tpe
 	sg.GenSchema.IsNullable = sg.GenSchema.IsNullable || nullableOverride
 	// prevent format from bubbling up in composed type
-	item.GenSchema.IsCustomFormatter = false
+	branch.GenSchema.IsCustomFormatter = false
+
+	sg.MergeResult(branch, true)
+
+	tpx, ers := sg.TypeResolver.ResolveSchema(&sg.Schema, false, true)
+	if ers != nil {
+		return false, ers
+	}
+	// we don't know the actual validation status yet. So assume true,
+	// unless we can infer that no Validate() method will be present
+	branch.GenSchema.HasValidations = !tpx.IsInterface && !tpx.IsStream
+	sg.GenSchema.AllOf = append(sg.GenSchema.AllOf, branch.GenSchema)
 
-	sg.MergeResult(item, true)
-	sg.GenSchema.AllOf = append(sg.GenSchema.AllOf, item.GenSchema)
 	return true, nil
 }
 
@@ -1972,6 +1987,7 @@ func (sg *schemaGenContext) makeGenSchema() error {
 		return err
 	}
 	if returns {
+		// short circuited on a resolved $ref
 		return nil
 	}
 	debugLogAsJSON("after short circuit named ref", sg.Schema)
@@ -2033,6 +2049,8 @@ func (sg *schemaGenContext) makeGenSchema() error {
 			log.Printf("INFO: type %s is external, with inferred spec type %s, referred to as %s", sg.GenSchema.Name, sg.GenSchema.GoType, extType)
 			sg.GenSchema.GoType = extType
 			sg.GenSchema.AliasedType = extType
+
+			// short circuit schema building for external types
 			return nil
 		}
 		// TODO: case for embedded types as anonymous definitions
@@ -2112,5 +2130,6 @@ func (sg *schemaGenContext) makeGenSchema() error {
 		(gs.IsTuple || gs.IsComplexObject || gs.IsAdditionalProperties || (gs.IsPrimitive && gs.IsAliased && gs.IsCustomFormatter && !strings.Contains(gs.Zero(), `("`)))
 
 	debugLog("finished gen schema for %q", sg.Name)
+
 	return nil
 }


Test patch:
diff --git a/generator/moreschemavalidation_fixtures_test.go b/generator/moreschemavalidation_fixtures_test.go
index 6735df5f..61fceac8 100644
--- a/generator/moreschemavalidation_fixtures_test.go
+++ b/generator/moreschemavalidation_fixtures_test.go
@@ -11116,11 +11116,12 @@ func initFixtureRealiasedTypes() {
 	thisRun.AddExpectations("hs2.go", []string{
 		`type Hs2 struct {`,
 		`	Hs0`,
-		// empty validation
-		"func (m *Hs2) Validate(formats strfmt.Registry) error {\n	return nil\n}",
+		// NOT empty validation
+		"func (m *Hs2) Validate(formats strfmt.Registry) error {",
+		"if err := m.Hs0.Validate(formats); err != nil {",
 	},
 		// not expected
-		todo,
+		[]string{"TODO", "func (m *Hs2) Validate(formats strfmt.Registry) error {\n	return nil\n}"},
 		// output in log
 		noLines,
 		noLines)
@@ -11184,11 +11185,12 @@ func initFixtureRealiasedTypes() {
 	thisRun.AddExpectations("g2.go", []string{
 		`type G2 struct {`,
 		`	G0`,
-		// empty validation
-		"func (m *G2) Validate(formats strfmt.Registry) error {\n	return nil\n}",
+		// NOT empty validation
+		"func (m *G2) Validate(formats strfmt.Registry) error {",
+		"if err := m.G0.Validate(formats); err != nil {",
 	},
 		// not expected
-		todo,
+		[]string{"TODO", "func (m *G2) Validate(formats strfmt.Registry) error {\n	return nil\n}"},
 		// output in log
 		noLines,
 		noLines)
@@ -11227,11 +11229,12 @@ func initFixtureRealiasedTypes() {
 	thisRun.AddExpectations("g0.go", []string{
 		`type G0 struct {`,
 		`	G1`,
-		// empty validation
-		"func (m *G0) Validate(formats strfmt.Registry) error {\n	return nil\n}",
+		// NOT empty validation
+		"func (m *G0) Validate(formats strfmt.Registry) error {",
+		"if err := m.G1.Validate(formats); err != nil {",
 	},
 		// not expected
-		todo,
+		[]string{"TODO", "func (m *G0) Validate(formats strfmt.Registry) error {\n	return nil\n}"},
 		// output in log
 		noLines,
 		noLines)
@@ -11344,11 +11347,12 @@ func initFixtureRealiasedTypes() {
 	thisRun.AddExpectations("g2v.go", []string{
 		`type G2v struct {`,
 		`	G0v`,
-		// empty validation
-		"func (m *G2v) Validate(formats strfmt.Registry) error {\n	return nil\n}",
+		// NOT empty validation
+		"func (m *G2v) Validate(formats strfmt.Registry) error {",
+		"if err := m.G0v.Validate(formats); err != nil",
 	},
 		// not expected
-		todo,
+		[]string{"TODO", "func (m *G2v) Validate(formats strfmt.Registry) error {\n      return nil\n}"},
 		// output in log
 		noLines,
 		noLines)
@@ -11439,11 +11443,12 @@ func initFixtureRealiasedTypes() {
 	thisRun.AddExpectations("g0v.go", []string{
 		`type G0v struct {`,
 		`	G1v`,
-		// empty validation
-		"func (m *G0v) Validate(formats strfmt.Registry) error {\n	return nil\n}",
+		// NOT empty validation
+		"func (m *G0v) Validate(formats strfmt.Registry) error {",
+		"if err := m.G1v.Validate(formats); err != nil {",
 	},
 		// not expected
-		todo,
+		[]string{"TODO", "func (m *G0v) Validate(formats strfmt.Registry) error {\n	return nil\n}"},
 		// output in log
 		noLines,
 		noLines)
@@ -11452,11 +11457,12 @@ func initFixtureRealiasedTypes() {
 	thisRun.AddExpectations("hs0.go", []string{
 		`type Hs0 struct {`,
 		`	Hsubtype1`,
-		// empty validation
-		"func (m *Hs0) Validate(formats strfmt.Registry) error {\n	return nil\n}",
+		// NOT empty validation
+		"func (m *Hs0) Validate(formats strfmt.Registry) error {",
+		"if err := m.Hsubtype1.Validate(formats); err != nil {",
 	},
 		// not expected
-		todo,
+		[]string{"TODO", "func (m *Hs0) Validate(formats strfmt.Registry) error {\n	return nil\n}"},
 		// output in log
 		noLines,
 		noLines)
@@ -11491,6 +11497,76 @@ func initFixture1993() {
 		noLines)
 }
 
+func initFixture2604() {
+	f := newModelFixture("../fixtures/bugs/2604/2604.yaml", "additional cases for embedded structs")
+	thisRun := f.AddRun(false).WithMinimalFlatten(true)
+
+	thisRun.AddExpectations("composed.go", []string{
+		`type Composed struct {`,
+		`	NoValidations`,
+		`func (m *Composed) Validate(formats strfmt.Registry) error {`,
+		`if err := m.NoValidations.Validate(formats); err != nil {`,
+	},
+		// not expected
+		todo,
+		// output in log
+		noLines,
+		noLines)
+
+	thisRun.AddExpectations("composed_iface.go", []string{
+		`type ComposedIface = Interface`,
+	},
+		// not expected
+		todo,
+		// output in log
+		noLines,
+		noLines)
+
+	thisRun.AddExpectations("interface.go", []string{
+		`type Interface interface{}`,
+	},
+		// not expected
+		todo,
+		// output in log
+		noLines,
+		noLines)
+
+	thisRun.AddExpectations("no_validations.go", []string{
+		`type NoValidations struct {`,
+		// empty validation
+		"func (m *NoValidations) Validate(formats strfmt.Registry) error {\n	return nil\n}",
+	},
+		// not expected
+		todo,
+		// output in log
+		noLines,
+		noLines)
+
+	thisRun.AddExpectations("post_fields.go", []string{
+		`type PostFields struct {`,
+		`func (m *PostFields) Validate(formats strfmt.Registry) error {`,
+		`if err := m.validateA(formats); err != nil {`,
+		`if err := m.validateB(formats); err != nil {`,
+	},
+		// not expected
+		todo,
+		// output in log
+		noLines,
+		noLines)
+
+	thisRun.AddExpectations("post_request.go", []string{
+		`type PostRequest struct {`,
+		`PostFields`,
+		`func (m *PostRequest) Validate(formats strfmt.Registry) error {`,
+		`if err := m.PostFields.Validate(formats); err != nil {`,
+	},
+		// not expected
+		todo,
+		// output in log
+		noLines,
+		noLines)
+}
+
 func initFixture2364() {
 	f := newModelFixture("../fixtures/bugs/2364/fixture-2364.yaml", "test non-nullable allOf")
 	thisRun := f.AddRun(false).WithMinimalFlatten(true)
diff --git a/generator/moreschemavalidation_test.go b/generator/moreschemavalidation_test.go
index 4b741822..dd59129d 100644
--- a/generator/moreschemavalidation_test.go
+++ b/generator/moreschemavalidation_test.go
@@ -286,6 +286,9 @@ func initModelFixtures() {
 
 	// map of nullable array
 	initFixture2494()
+
+	// additional cases for embedded struct
+	initFixture2604()
 }
 
 /* Template initTxxx() to prepare and load a fixture: