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

gf/net/gclient: The DoRequestObj function does not process the query parameters on non-GET requests #3861

Open
LanceAdd opened this issue Oct 17, 2024 · 10 comments

Comments

@LanceAdd
Copy link
Contributor

Go version

go version go1.22.7 darwin/amd64

GoFrame version

2.7.4

Can this bug be reproduced with the latest release?

Option Yes

What did you do?

package main

import (
	"fmt"
	"github.com/gogf/gf/v2/frame/g"
	"github.com/gogf/gf/v2/os/gctx"
	"time"
)

type ChartUpdateReq struct {
	g.Meta  `path:"/chart/check/{id}" method:"post"`
	Id      string
	Code    string `p:"code"`
	Version string `json:"version"`
}

type ChartUpdateRes struct {
	Name         string    `json:"name"`
	Url          string    `json:"url"`
	Size         int64     `json:"size"`
	Md5          string    `json:"md5"`
	ChartCode    string    `json:"chartCode"`
	ChartVersion time.Time `json:"chartVersion"`
}

func main() {
	req := ChartUpdateReq{Code: "11", Id: "1"}
	res := &ChartUpdateRes{}
	err := g.Client().Prefix("http://127.0.0.1:50000").DoRequestObj(gctx.GetInitCtx(), req, res)
	fmt.Println(err)
	fmt.Println(res)
}

Now URL

http://127.0.0.1:50000/chart/check/1

Expect URL

http://127.0.0.1:50000/chart/check/1?code=11

What did you see happen?

Query parameters (http://xxxx.com?code=11) are built only if content-type is not application/jsonand application/xml.

I think it would be more rigorous for the user to tag the fields in request struct to mark the parameter types (query parameters /json/ path parameters).

These tags are not handled in the current logic, and I'm not sure if this is for some special reason or if you want users to handle the logic themselves. If there is no special reason, I am ready to submit PR to implement my idea

What did you expect to see?

Now URL

http://127.0.0.1:50000/chart/check/1

Expect URL

http://127.0.0.1:50000/chart/check/1?code=11
@LanceAdd LanceAdd added the bug It is confirmed a bug, but don't worry, we'll handle it. label Oct 17, 2024
@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


g.Meta path:"/chart/check/{id}" method:"post"``
Change the method to get or it will be a post request

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


g.Meta path:"/chart/check/{id}" method:"post"`` method is changed to `get`, otherwise it is a `post` request
I know that changing to get will process the query parameters, but the situation I am encountering now requires post/put/patch.
Now under the post request, when there are three parameters: query parameters, path parameters, and request body parameters at the same time, the query parameters will not be constructed at this time, and only the path parameters and request body parameters will be processed. I hope he can do it in the post. Zhong also builds query parameters, which can be distinguished by using tags. I would like to ask whether this was intentionally designed this way or not handled during implementation. If it is just not handled, I will submit a PR to handle it.

@LanceAdd
Copy link
Contributor Author

@wwwfeng 我知道改成get是会去处理查询的参数,但是我现在遇到的情况是需要post/put/patch这三类
现在就是在post的请求下,同时有查询参数、路径参数、请求体参数三种参数时,这时候并不会构建查询参数,只会处理路径参数和请求体参数,我希望他能在post中也去构建查询参数,使用tag标记下就可以区分,想问下这个在实现的时候是故意这么设计的还是没处理,如果只是没处理我就提个PR处理下

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@wwwfeng I know that changing to get will process the query parameters, but the situation I am encountering now requires post/put/patch.
Now under the post request, when there are three parameters: query parameters, path parameters, and request body parameters at the same time, the query parameters will not be constructed at this time, and only the path parameters and request body parameters will be processed. I hope he can do it in the post. Zhong also builds query parameters, which can be distinguished by using tags. I would like to ask whether this was intentionally designed this way or not handled during implementation. If it is just not handled, I will submit a PR to handle it.

@LanceAdd
Copy link
Contributor Author

我看着这个DoRequestObj的注释上也说DoRequestObj does HTTP request using standard request/response object.但是其中的参数并未严格按照规范路由的解析流程,应该用tag标注出来每个参数是路径参数/查询参数/请求体参数然后再去解析这些吧

@gqcn
Copy link
Member

gqcn commented Oct 20, 2024

@wwwfeng 我知道改成get是会去处理查询的参数,但是我现在遇到的情况是需要post/put/patch这三类 现在就是在post的请求下,同时有查询参数、路径参数、请求体参数三种参数时,这时候并不会构建查询参数,只会处理路径参数和请求体参数,我希望他能在post中也去构建查询参数,使用tag标记下就可以区分,想问下这个在实现的时候是故意这么设计的还是没处理,如果只是没处理我就提个PR处理下

你好,目前DoRequestObj只会处理pathbody参数,不会对请求结构体中的in标签进行解析,若有需要环境提pr来支持。

@gqcn gqcn added enhancement help wanted and removed bug It is confirmed a bug, but don't worry, we'll handle it. labels Oct 20, 2024
@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@wwwfeng I know that changing to get will process the query parameters, but the situation I am encountering now requires the three types of post/put/patch. Now under the post request, there are query parameters, path parameters, and requests at the same time. When there are three types of body parameters, query parameters will not be constructed at this time. Only path parameters and request body parameters will be processed. I hope he can also construct query parameters in the post and use tags to distinguish them. I would like to ask Is this intentionally designed this way during implementation or is it not handled? If it is just not handled, I will submit a PR to handle it.

Hello, currently DoRequestObj will only process the path and body parameters, and will not parse the in tag in the request structure. If necessary, the environment will provide pr to support it.

Copy link

Hello @LanceAdd. We like your proposal/feedback and would appreciate a contribution via a Pull Request by you or another community member. We thank you in advance for your contribution and are looking forward to reviewing it!
你好 @LanceAdd。我们喜欢您的提案/反馈,并希望您或其他社区成员通过拉取请求做出贡献。我们提前感谢您的贡献,并期待对其进行审查。

@LanceAdd
Copy link
Contributor Author

LanceAdd commented Nov 8, 2024

@wwwfeng 我知道改成get是会去处理查询的参数,但是我现在遇到的情况是需要post/put/patch这三类 现在就是在post的请求下,同时有查询参数、路径参数、请求体参数三种参数时,这时候并不会构建查询参数,只会处理路径参数和请求体参数,我希望他能在post中也去构建查询参数,使用tag标记下就可以区分,想问下这个在实现的时候是故意这么设计的还是没处理,如果只是没处理我就提个PR处理下

你好,目前DoRequestObj只会处理pathbody参数,不会对请求结构体中的in标签进行解析,若有需要环境提pr来支持。

你好,我在修改现有DoRequestObj 支持这个issues,我个人希望直接使用规范路由请求体作为DoRequestObj的参数,这样方便大家在微服务远程调用中直接使用请求目标项目的req entity,现在其中关于上传文件这部分,现有DoRequestObj底层和其他request一样的prepareRequest,使用@file:, "upload-file=@file:"+path这样的形式进行支持,但是规范路由中则是通过ghttp.UploadFileghttp.UploadFiles配合tag中type:file使用,是否需要保留现有的@file形式,我个人觉得DoRequestObj没必要去兼容这样的形式,但是不清楚大家有没有其他的想法,想问下各位

@github-actions github-actions bot removed the inactive label Nov 8, 2024
@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@wwwfeng I know that changing to get will process the query parameters, but the situation I am encountering now requires post/put/patch. Now under the post request, when there are three parameters: query parameters, path parameters, and request body parameters at the same time, the query parameters will not be constructed at this time, and only the path parameters and request body parameters will be processed. I hope he can do it in the post. Zhong also builds query parameters, which can be distinguished by using tags. I would like to ask whether this was intentionally designed this way or not handled during implementation. If it is just not handled, I will submit a PR to handle it.

Hello, currently DoRequestObj will only process the path and body parameters, and will not parse the in tag in the request structure. If necessary, the environment will provide pr to support it.

Hello, I am modifying the existing DoRequestObj to support this issue. I personally hope to directly use the canonical routing request body as the parameter of DoRequestObj. This will make it easier for everyone to directly use the req entity of the request target project in the microservice remote call. Now it is about uploading files. In this part, the existing DoRequestObj underlying prepareRequest is the same as other requests, using @file:, The form "upload-file=@file:"+path is supported, but in the standard routing, ghttp.UploadFile and ghttp.UploadFiles are used together with type:file in the tag. Do you need to retain it? I personally think there is no need for DoRequestObj to be compatible with the existing @file form, but I don’t know if you have any other ideas. I would like to ask you.

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

Successfully merging a pull request may close this issue.

3 participants