-
Notifications
You must be signed in to change notification settings - Fork 22
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
830 : Add limit and metadata version params in v2 settings get end-point #838
Conversation
Map<String, TreeNode<String, Location>> treeNodeHashMap = null; | ||
|
||
if (StringUtils.isBlank(team) && StringUtils.isBlank(providerId) && StringUtils.isBlank(locationId) | ||
&& StringUtils.isBlank(teamId) && StringUtils.isBlank(team) && StringUtils.isBlank(serverVersion)) { | ||
&& StringUtils.isBlank(teamId) && StringUtils.isBlank(team) && StringUtils.isBlank(serverVersion) | ||
&& StringUtils.isBlank(metadataVersion)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wont this break the current functionality StringUtils.isBlank(metadataVersion)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check is to ensure that all parameters can not be null. Hence, added meta version field here as well. @bennsimon
Please let me know if there is anything incorrect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a new parameter so for the devs that were supplying the other params only it should work. i.e metadataVersion
should be optional to not break the implementation for others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, metadata_version
and limit
request params are not required fields. This check is to ensure that all request parameters can not be null, we need to supply at least a single request param from the given list. Thus added the check.
Should I remove validation from here? @bennsimon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leave it
Resolves #830