Upgrade from v0.121.0 to v0.122.0 Introduces loss of T.Paths when marshalling to yaml

[mcblair]

Details here: #763 (comment)

[fenollp]

Hey I'm having trouble understanding your issue. Could you tweak #887 to show the bug please? Comment on it or make a PR to it...

[mcblair]

Thanks for whipping up that test for this! Super helpful, here is a reproducible example:

package openapi3_test

import (
	"os"
	"testing"

	"github.com/getkin/kin-openapi/openapi3"
	invopopYaml "github.com/invopop/yaml"
	"github.com/stretchr/testify/require"
	"gopkg.in/yaml.v2"
	v3 "gopkg.in/yaml.v3"
)

func TestIssue883(t *testing.T) {
	spec := `
openapi: '3.0.0'
info:
  version: '1.0.0'
  title: Swagger Petstore
  license:
    name: MIT
servers:
- url: http://petstore.swagger.io/v1
paths:
  /simple:
    get:
      summary: A simple GET request
      responses:
        '200':
          description: OK
`[1:]

	sl := openapi3.NewLoader()
	doc, err := sl.LoadFromData([]byte(spec))
	require.NoError(t, err)
	require.NotNil(t, doc.Paths)

	err = doc.Validate(sl.Context)
	require.NoError(t, err)
	require.NotNil(t, doc.Paths)

	t.Run("Roundtrip yaml.v2", func(t *testing.T) {
		// Test marshaling and unmarshaling with yaml.v2
		testYAML(t, doc, spec, yaml.Marshal, unmarshalYAMLv2)
	})

	t.Run("Roundtrip yaml.v3", func(t *testing.T) {
		// Test marshaling and unmarshaling with yaml.v3
		testYAML(t, doc, spec, v3.Marshal, unmarshalYAMLv3)
	})

	t.Run("Roundtrip invopop/yaml", func(t *testing.T) {
		// Test marshaling and unmarshaling with invopop/yaml
		testYAML(t, doc, spec, invopopYaml.Marshal, unmarshalInvopopYAML)
	})
}

func testYAML(t *testing.T, doc *openapi3.T, originalSpec string, marshalFunc func(interface{}) ([]byte, error), unmarshalFunc func([]byte, interface{}) error) {
	marshalledYaml, err := marshalFunc(doc)
	require.NoError(t, err)

	// Create a temporary file
	tmpFile, err := os.CreateTemp("", "spec-*.yaml")
	require.NoError(t, err)
	defer os.Remove(tmpFile.Name()) // Clean up the file afterwards

	// Write to the temporary file
	_, err = tmpFile.Write(marshalledYaml)
	require.NoError(t, err)
	require.NoError(t, tmpFile.Close())

	// Read from the temporary file
	roundTripped, err := os.ReadFile(tmpFile.Name())
	require.NoError(t, err)

	// Unmarshal using the provided function
	var newDoc openapi3.T
	err = unmarshalFunc(roundTripped, &newDoc)
	require.NoError(t, err)
	require.NotNil(t, newDoc.Paths)

	// Compare original and round-tripped specs
	require.YAMLEq(t, originalSpec, string(roundTripped))
}

func unmarshalYAMLv2(data []byte, v interface{}) error {
	return yaml.Unmarshal(data, v)
}

func unmarshalYAMLv3(data []byte, v interface{}) error {
	return v3.Unmarshal(data, v)
}

func unmarshalInvopopYAML(data []byte, v interface{}) error {
	return invopopYaml.Unmarshal(data, v)
}

I can confirm it is reproducible using yaml.v2 and yaml.v3 and not reproducible using "github.com/invopop/yaml".

[mcblair]

Checking out v0.121.0 of kin-openapi and running those tests yields all 3 passing.

[fenollp]

I see. The thing is yaml v2/v3 packages don't seem to use MarshalYAML => https://pkg.go.dev/gopkg.in/yaml.v3#Marshal

This can indeed be seen as a regression: by introducing private fields (maps in Paths, Callbacks, Responses)

Struct fields are only marshalled if they are exported

Changing to https://github.com/invopop/yaml fixes this, while still depending on yaml.v3.
Is this closure enough for you? Or is it impossible in your context to make the switch?

[mcblair]

I see. The thing is yaml v2/v3 packages don't seem to use MarshalYAML => https://pkg.go.dev/gopkg.in/yaml.v3#Marshal

This can indeed be seen as a regression: by introducing private fields (maps in Paths, Callbacks, Responses)

Struct fields are only marshalled if they are exported

Changing to https://github.com/invopop/yaml fixes this, while still depending on yaml.v3. Is this closure enough for you? Or is it impossible in your context to make the switch?

Apologies for the delay, spent a few weeks in the Philippines for the holidays 🇵🇭

We decided to switch entirely to invopop/yaml but then stumbled upon a new issue: invopop/yaml#12 (comment).

Kind of crazy: go with canonical go YAML and the billion-dollar mistake… or go with invopop YAML which flips some behaviors upside down, silently.

[sonu27]

Having the same issue, had to update my open API merger after PathItems were made private, but now paths are empty.

package main

import (
	"fmt"
	"github.com/getkin/kin-openapi/openapi3"
	"github.com/rs/zerolog/log"
	"gopkg.in/yaml.v3"
	"os"
	"path/filepath"
)

func main() {
	base := openapi3.T{
		OpenAPI: "3.0.3",
		Info: &openapi3.Info{
			Title:   "Alfred API",
			Version: "1.0",
		},
		Servers: openapi3.Servers{
			&openapi3.Server{
				Description: "Current",
				URL:         "",
			},
		},
		Paths: &openapi3.Paths{},
		Components: &openapi3.Components{
			Schemas: openapi3.Schemas{},
		},
	}

	err := generate(&base, "./openapi.yaml")
	if err != nil {
		log.Fatal().Err(err).Msg("generating openapi spec")
	}
}

func generate(base *openapi3.T, path string) error {
	files, err := filepath.Glob("./svc/api-*/openapi.yaml")
	if err != nil {
		return fmt.Errorf("finding files: %w", err)
	}

	loader := openapi3.NewLoader()

	for _, file := range files {
		fmt.Printf("processing file: %s\n", file)
		doc, err := loader.LoadFromFile(file)
		if err != nil {
			return fmt.Errorf("loading file: %w", err)
		}

		// loop through paths and add to base, checking for duplicates
		for key, pathItem := range doc.Paths.Map() {
			if base.Paths.Find(key) != nil {
				return fmt.Errorf("duplicate path found: %s", key)
			}
			base.Paths.Set(key, pathItem)
		}

		// loop through schemas and add to base, checking for duplicates
		for name, schema := range doc.Components.Schemas {
			if _, ok := base.Components.Schemas[name]; ok {
				return fmt.Errorf("duplicate schema found: %s", name)
			}
			base.Components.Schemas[name] = schema
		}
	}

	b, err := yaml.Marshal(&base)
	if err != nil {
		return fmt.Errorf("marshalling base: %w", err)
	}

	f, err := os.Create(path)
	if err != nil {
		return fmt.Errorf("file not created: %w", err)
	}

	_, err = f.Write(b)
	if err != nil {
		return fmt.Errorf("writing file: %w", err)
	}
	fmt.Printf("file created: %s\n", path)

	return nil
}

[fenollp]

Please @sonu27 use the YAML lib this lib uses: invopop's.

I'm closing this issue and suggest #849 to discuss a different yaml lib.

This current issue is resolved: #883 (comment)

[sonu27]

That library has its own issues (see above)

[fenollp]

Does it? It ser/de YAML based on JSON tags, as intended.

[sonu27]

Ah fair enough :)
Just switched to it. Will add a comment here: #849