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

Issue with XML Attribute Handling for Nested Objects #317

Open
DimShadoWWW opened this issue Dec 26, 2024 · 8 comments
Open

Issue with XML Attribute Handling for Nested Objects #317

DimShadoWWW opened this issue Dec 26, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@DimShadoWWW
Copy link

DimShadoWWW commented Dec 26, 2024

To Reproduce
Steps to reproduce the behavior:

Example code:

package main

import (
	"fmt"
	"net/http"

	"github.com/go-fuego/fuego"
	"github.com/go-fuego/fuego/option"
	"github.com/go-fuego/fuego/param"
)

type Health struct {
	Status string `json:"status" xml:"status" example:"ok"`
}

type MyInput struct {
	Name string `json:"name" xml:"name,attr" validate:"required"  example:"Hello, Carmack"`
}

type MyOutput struct {
	Message MyInput `json:"message" xml:"message"`
}

func main() {
	s := fuego.NewServer(
		fuego.WithAddr("0.0.0.0:8080"),
	)

	fuego.Get(s, "/health", func(c fuego.ContextNoBody) (Health, error) {
		return Health{Status: "ok"}, nil
	}, option.Description("Health check"),
		option.Summary("Returns a status message"),
		option.OperationID("health"),
		option.Tags("health"),
		option.AddResponse(http.StatusOK, "Health check", fuego.Response{
			ContentTypes: []string{"application/xml", "application/json"},
			Type:         Health{},
		}),
	)

	// Automatically generates OpenAPI documentation for this route
	fuego.Post(s, "/user/{user}", myController,
		option.Description("This route does something..."),
		option.Summary("This is my summary"),
		option.Tags("MyTag"), // A tag is set by default according to the return type (can be deactivated)
		// option.Deprecated(),  // Marks the route as deprecated in the OpenAPI spec
		option.Query("name", "Declares a query parameter with default value", param.Default("Carmack")),
		option.Header("Authorization", "Bearer token", param.Required()),
		optionPagination,
	)

	s.Run()
}

func myController(c fuego.ContextWithBody[MyInput]) (MyOutput, error) {
	body, err := c.Body()
	if err != nil {
		return MyOutput{}, err
	}

	return MyOutput{
		Message: MyInput{
			Name: fmt.Sprintf("Hello, %s", body.Name),
		},
	}, nil
}

var optionPagination = option.Group(
	option.QueryInt("page", "Page number", param.Default(1), param.Example("1st page", 1), param.Example("42nd page", 42)),
	option.QueryInt("perPage", "Number of items per page"),
)

Output XML example in /user/{user} endpoint:

<?xml version="1.0" encoding="UTF-8"?>
<MyOutput>
	<message>
		<name>string</name>
	</message>
</MyOutput>

Here, "name" appears as a child tag instead of being an attribute.

Expected behavior

Expected output in XML (from /user/{user} endpoint):

<?xml version="1.0" encoding="UTF-8"?>
<MyOutput>
	<message name="Hello, Carmack">
	</message>
</MyOutput>

Framework version:

github.com/go-fuego/fuego v0.17.0

Go version:
v1.23.4

Additional context

The issue seems to stem from the way the XML struct tags are being handled for nested objects. The expected behavior aligns with how the struct is defined:

type MyInput struct {
	Name string `json:"name" xml:"name,attr" validate:"required" example:"Hello, Carmack"`
}

Relevant schemas from the generated OpenAPI:

        "MyInput": {
            "description": "MyInput schema",
            "properties": {
                "name": {
                    "example": "Hello, Carmack",
                    "type": "string"
                }
            },
            "required": [
                "name"
            ],
            "type": "object"
        },
        "MyOutput": {
            "description": "MyOutput schema",
            "properties": {
                "message": {
                    "properties": {
                        "name": {
                            "type": "string"
                        }
                    },
                    "type": "object"
                }
            },
            "type": "object"
        },
        "unknown-interface": {
            "description": "unknown-interface schema"
        }

The generated OpenAPI does not reflect the attribute correctly in the message field.

For more details, you can refer to the attached
openapi.json

@DimShadoWWW DimShadoWWW added the bug Something isn't working label Dec 26, 2024
@EwenQuim
Copy link
Member

Thank you for the report!

I don't know where this bug can come from. We use the stdlib encoding/xml. Are the tags you're using defined by the stdlib?

@DimShadoWWW
Copy link
Author

DimShadoWWW commented Dec 28, 2024

Hello @EwenQuim ,

I uploaded the example code to testapi.

I used customOpenAPI and customBaseRoute in api/oapi/custom.go to overwrite fuego.OpenAPI and fuego.BaseRoute. This way I was able to insert a custome made parseStructTags that managed Xml tag names and attributes using OpenAPI's xml in this way:

          "status": {
            "example": "ok",
            "type": "string",
            "xml": {
              "attribute": true,
              "name": "Status"
            }
          }

but it worked only in parent structure, on childs it didn't work:
struct:

type Health struct {
	Status string `json:"status" xml:"Status,attr" example:"ok"`

	Input MyInput `json:"input" xml:"input"`
}

type MyInput struct {
	Name   string    `json:"name" xml:"name,attr" validate:"required"  example:"Hello, Carmack"`
	Values []MyValue `json:"values" xml:"values"`
}

type MyValue struct {
	Value string `json:"value" xml:"value,attr" example:"example value"`
}

results in attribute only in Health and not in MyInput and MyValue:

            "Health": {
                "description": "Health schema",
                "properties": {
                    "input": {
                        "properties": {
                            "name": {
                                "type": "string"
                            },
                            "values": {
                                "items": {
                                    "properties": {
                                        "value": {
                                            "type": "string"
                                        }
                                    },
                                    "type": "object"
                                },
                                "type": "array"
                            }
                        },
                        "type": "object",
                        "xml": {
                            "name": "input"
                        }
                    },
                    "status": {
                        "example": "ok",
                        "type": "string",
                        "xml": {
                            "attribute": true,
                            "name": "Status"
                        }
                    }
                },
                "type": "object"
            },

This is just a proof of concept to understand the extent of changes required before attempting to implement them in 'fuego'.

@dylanhitt
Copy link
Collaborator

dylanhitt commented Dec 29, 2024

Hmm, I looked into this some. The output seems correct. At least it aligns with this playground.

I believed you'd need something like this to get the structure you desire.

package main

import (
	"encoding/xml"
	"fmt"
	"os"
)

type MyInput struct {
	Message string `xml:",chardata"`
	Name    string `xml:"name,attr"`
}

type MyOutput struct {
	Message myInput `json:"message" xml:"message"`
}

func main() {
	x := &MyOutput{Message: myInput{Name: "Hello, Carmack"}}

	enc := xml.NewEncoder(os.Stdout)
	enc.Indent("", "  ")
	if err := enc.Encode(x); err != nil {
		fmt.Printf("error: %v\n", err)
	}

}

output:

<MyOutput>
  <message name="Hello, Carmack"></message>
</MyOutput>

@DimShadoWWW
Copy link
Author

The problem is not encoding to XML, but on the generated OpenAPI documentation.

I think I found the problem, editing openapi.go and adding:


		// If the property is an object, process its own tags
		if propertyValue.Type != nil {
			if propertyValue.Type.Is(openapi3.TypeObject) {
				// Recursively process the struct tags of the inner object
				parseStructTags(field.Type, schemaRef.Value.Properties[jsonFieldName])
			}
		}

		// Xml attributes
		xmlTag, ok := field.Tag.Lookup("xml")
		if ok {
			xmlTagName := strings.Split(xmlTag, ",")[0] // remove omitempty, etc
			if xmlTagName == "-" {
				continue
			}
			if xmlTagName == "" {
				xmlTagName = field.Name
			}

			propertyValue.XML = &openapi3.XML{
				Name: xmlTagName,
			}

			xmlTags := strings.Split(xmlTag, ",")
			if slices.Contains(xmlTags, "attr") {
				propertyValue.XML.Attribute = true
			}
		}

just after:

		propertyCopy := *property
		propertyValue := *propertyCopy.Value

The XML attributes are defined as attributes in OpenAPI documentation.

I will try to check other edge situations before a PR.

@dylanhitt
Copy link
Collaborator

OIC. You may want to base off of #319

@DimShadoWWW
Copy link
Author

DimShadoWWW commented Dec 30, 2024

@dylanhitt, I updated fuego to the latest commit ce494eb and removed all my customizations from the example code in testapi.

However, the bug persists—the XML information is still not being handled properly. Here is the current output:

      "Health": {
        "description": "Health schema",
        "properties": {
          "input": {
            "properties": {
              "name": {
                "example": "Hello, Carmack",
                "type": "string"
              },
              "values": {
                "items": {
                  "properties": {
                    "value": {
                      "type": "string"
                    }
                  },
                  "type": "object"
                },
                "type": "array"
              }
            },
            "required": [
              "name"
            ],
            "type": "object"
          },
          "status": {
            "example": "ok",
            "type": "string"
          }
        },
        "type": "object"
      },

I noticed that commit cfe0e19 addresses nested struct tags for JSON, but parseStructTags does not currently include logic to handle XML information in the OpenAPI schema, or a way to include customizations.

@dylanhitt
Copy link
Collaborator

Yeah. I was just saying you should base you should rebase on #319 which is now just in main. #319 isn't going this fix the issue. You seemed to bo the write track with properly parsing the tags.

@DimShadoWWW
Copy link
Author

PR #328 provides foundational support for XML schema definitions in OpenAPI documentation, aligning with the use cases where XML-based APIs require precise schema representation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants