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

Nested header on input #301

Open
victoraugustolls opened this issue Mar 12, 2024 · 13 comments
Open

Nested header on input #301

victoraugustolls opened this issue Mar 12, 2024 · 13 comments
Labels
question Further information is requested

Comments

@victoraugustolls
Copy link
Sponsor Contributor

victoraugustolls commented Mar 12, 2024

I see that it is a common pattern with Huma that the body, both in the input and output, is to be created inside the nested struct field Body. Is it possible to do the same with headers and path/query parameters? Something like:

type MyInput struct {
    Headers struct {
        UserIP string `header:"user-ip"`
    }
    Body struct {
        Name string `json:"name"`
    }
}
@danielgtaylor danielgtaylor added the question Further information is requested label Mar 12, 2024
@danielgtaylor
Copy link
Owner

@victoraugustolls this isn't currently possible. They are just added at the top level, like:

type MyInput struct {
    UserIP  string `header:"User-Ip"`
    Header2 string `header:"Header2"`
    Header3 int    `header:"Header3"`
    Body struct {
        Name string `json:"name"`
    }
}

Any reason you need additional grouping?

@victoraugustolls
Copy link
Sponsor Contributor Author

I was thinking about header reuse, which allows structures to be reused on inputs. Do you think it makes sense?

@bclements
Copy link
Sponsor

I really like the grouping idea myself. It follows your established pattern to use Body. If we are upvoting..I'd love this as a feature.

@Insei
Copy link
Contributor

Insei commented Mar 12, 2024

@victoraugustolls you can use composition in you case, like:

type MyInputHeaders struct {
    UserIP  string `header:"User-Ip"`
    Header2 string `header:"Header2"`
    Header3 int    `header:"Header3"`
}
type MyInput struct {
   MyInputHeaders
   Body struct {
        Name string `json:"name"`
   }
}

It's a not perfect solution, but I already use this method for each paginated lists in my APIs and it's work well

@victoraugustolls
Copy link
Sponsor Contributor Author

victoraugustolls commented Mar 12, 2024

I now we can it just not feels that follows a pattern as with the body like @bclements mentioned!

One example of how this can help is when we are accessing the values of the input. Imagine a case where I have everything: headers, body, path/query parameters...

In such a case I know I am using a body variable because I access it with input.Body.Value, but this is not true for the rest! In this scenario the user needs to look at the input in the case that it does not know by heart.

@victoraugustolls
Copy link
Sponsor Contributor Author

If @danielgtaylor agrees with it, I can open a PR implementing this for other structures other than the body!

@danielgtaylor
Copy link
Owner

I'm not opposed to the idea; seems several people are interested in it as a code organization strategy. It'll likely be slightly less efficient as reflection is used to set the fields and you have to go into the Headers struct but it's a tiny difference. As long as it is optional this seems like it would be a nice feature to have.

@victoraugustolls
Copy link
Sponsor Contributor Author

@danielgtaylor I'm working on a pull request for it, but I might only be able to finish it next week!

@ssoroka
Copy link
Contributor

ssoroka commented Apr 15, 2024

I'd actually like to make Body optional, and I think it might make sense to make both Body and Headers optional.
I don't think it'd be too confusing to intuit what each parameter does from the tags, eg all header are headers, json are clearly body, (path and query things, right?), and anything left over can probably be assumed to be body.

It shouldn't make much difference to the reflection. If Headers is defined, look there for headers, otherwise look at the top level. If Body is defined, look there for body fields, otherwise look at the top level.

@victoraugustolls
Copy link
Sponsor Contributor Author

@danielgtaylor given @ssoroka comment, I also feel like that for every possible part of the request we could have it in the root of the struct or in special fields like Body, Header, Query and Param. Do we need a discussion for this or would you be okay with this proposal?

@danielgtaylor
Copy link
Owner

@victoraugustolls @ssoroka I'm not sure this will work for the body. In a scenario like this the JSON unmarshal call would not know what to do:

type ExampleRequest struct {
  Part1 struct {
    Field1 string `json:"field1"`
  }
  Part2 struct {
    Field2 string `json:"field2"`
  }
}

There's also the complication that fields without a json tag are still unmarshaled, and we need to support other formats like CBOR/YAML/etc.

@victoraugustolls
Copy link
Sponsor Contributor Author

@danielgtaylor fair enough, did not think it through before tagging you! Thanks for the quick response, will work on the previous approach. I had some rough recent couple of weeks at works but I think I can take the time now :)

@victoraugustolls
Copy link
Sponsor Contributor Author

victoraugustolls commented May 15, 2024

I'm currently investigating the best approach to set nested fields inside the input using reflection. I tried to modify the least I could in the existing code, but that makes me create a lot of turnarounds, so some refactor in existing code might be needed to implement this in a cleaner manner.

This is the part of the code that @danielgtaylor mentioned in #393 so I'm handling it with care!

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

No branches or pull requests

5 participants