Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generates unnecessary definitions when struct inherits the fields from another #81

Open
SVilgelm opened this issue Nov 22, 2021 · 2 comments

Comments

@SVilgelm
Copy link
Contributor

I'm currently using very outdated library v1.1.1-0.20190620131940-98aa997b1687, and I want to upgrade to latest, but I was really surprised when the swagger validate shows much more warnings with new version.

Here is an example of two structs:

type Parent struct {
	FieldA string
}

type Child struct {
	Parent
	FieldB int
}

with v1.1.1 it generates just one definition:

{
  "definitions": {
    "restfulspec.Child": {
      "required": [
        "FieldA",
        "FieldB"
      ],
      "properties": {
        "FieldA": {
          "type": "string"
        },
        "FieldB": {
          "type": "integer",
          "format": "int32"
        }
      }
    }
  }
}

but v2.6.1 generates two definitions:

{
  "definitions": {
    "restfulspec.Child": {
      "required": [
        "FieldA",
        "FieldB"
      ],
      "properties": {
        "FieldA": {
          "type": "string"
        },
        "FieldB": {
          "type": "integer",
          "format": "int32"
        }
      }
    },
    "restfulspec.Parent": {
      "required": [
        "FieldA"
      ],
      "properties": {
        "FieldA": {
          "type": "string"
        }
      }
    }
  }
}

and the swagger validate shows a warning:

WARNING: definition "#/definitions/restfulspec.Parents" is not used anywhere

It would be great if you can exclude fix the issue.
Here is the test case:

type Parent struct {
	FieldA string
}

type Child struct {
	Parent
	FieldB int
}

func TestParentChildArray(t *testing.T) {
	db := definitionBuilder{Definitions: spec.Definitions{}, Config: Config{}}
	db.addModelFrom(Child{})
	s := spec.Schema{
		SchemaProps: spec.SchemaProps{
			Definitions: db.Definitions,
		},
	}
	_, ok := db.Definitions["restfulspec.Parent"]
	if ok {
		data, _ := json.MarshalIndent(s, "", "  ")
		t.Log(string(data))
		t.Fail()
	}
}
@emicklei
Copy link
Owner

emicklei commented Dec 7, 2021

Thank you for providing the example test.
The lame excuse for current behaviour is that the code base had many contributions that have not been reviewed that carefully. I will have a look at this case to see where the flow is off.

@SVilgelm
Copy link
Contributor Author

SVilgelm commented Dec 7, 2021

no worries, it is not something major, just some warnings, that can be avoided. You know what I think, maybe we can add a supporting for allOf (it is par of v2) for such cases
so instead of including all fields from Parent, we can create Parent definition and then create Child with allOf: [{$ref: Parent}, {...Child...}]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants