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

Projects.SetConfig endpoint fails with 400 Bad Request due to ConfigInput.MaxObjectSizeLimit zero value not being omitted #111

Open
wuhuang26 opened this issue May 27, 2022 · 5 comments · May be fixed by #112

Comments

@wuhuang26
Copy link

I create TestProjectsService_SetConfig func to test SetConfig requests api fail
error info:
failed: 400 Bad Request
Invalid application/json in request

I find ConfigInput field is MaxObjectSizeLimitInfo, change type string can be success...

@dmitshur
Copy link
Collaborator

Thanks for reporting.

Since the field is optional, MaxObjectSizeLimitInfo is definitely not right as it'll always be included despite the omitempty tag, while *MaxObjectSizeLimitInfo would allow the zero value to be properly omitted.

The documentation for the ConfigInput entity says:

The max object size limit of this project as a MaxObjectSizeLimitInfo entity.
If set to 0, the max object size limit is removed.
If not set, this setting is not updated.

The first and last paragraph suggests that *MaxObjectSizeLimitInfo would be right. However, the "if set to 0" part suggests maybe it can be a string or integer value too? To allow that, I think we'd need to use interface{} and let the user provide the desired type.

It's also possible the documentation is incorrect, though, since the example request shows a string being used:

"max_object_size_limit": "10m",

@dmitshur dmitshur changed the title Aboud ConfigInput field MaxObjectSizeLimit problem Projects.SetConfig endpoint fails with 400 Bad Request due to ConfigInput.MaxObjectSizeLimit zero value not being omitted May 27, 2022
@dmitshur dmitshur added the bug label May 27, 2022
@dmitshur dmitshur linked a pull request May 27, 2022 that will close this issue
@andygrunwald
Copy link
Owner

From the first look, interfae{} looks like the way to go here.
Proper inline documentation would be very useful to have to describe the possible values.

@dmitshur
Copy link
Collaborator

@andygrunwald It should be possible to infer the type from Gerrit's code. If ProjectInput.java is the right place to look, then using string as implemented in PR #112 looks right (and Gerrit REST API documentation needs updating).

@wuhuang26
Copy link
Author

wuhuang26 commented May 30, 2022 via email

@wuhuang26
Copy link
Author

I found that the configinfo setting parameter does not take effect, such as State、SubmitType、Actions

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