fix(schema): expose repeated field and expand array query params#415
fix(schema): expose repeated field and expand array query params#415anshul-garg27 wants to merge 4 commits intogoogleworkspace:mainfrom
Conversation
Two related fixes for repeated/array query parameters: 1. `gws schema` now includes `"repeated": true` in parameter output when the Discovery Document marks a parameter as repeated. This lets users know which params accept multiple values. 2. When `--params` contains a JSON array for a parameter marked `repeated: true`, the executor now expands it into multiple query parameters (e.g. `?h=Subject&h=Date&h=From`) instead of stringifying the array as a single value. The query_params type changes from HashMap<String, String> to Vec<(String, String)> to support multiple entries with the same key. Closes googleworkspace#300
🦋 Changeset detectedLatest commit: fe787dd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue where repeated query parameters were not correctly handled, leading to API calls failing or behaving unexpectedly. By enhancing both the schema representation and the query parameter processing logic, the CLI now accurately communicates and executes requests involving repeated fields, improving reliability and usability for developers interacting with APIs that utilize such parameters. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly implements support for repeated query parameters by changing query_params from a HashMap to a Vec<(String, String)> and expanding array values for repeated fields. The schema output is also updated to expose the repeated property. The changes are well-tested. I've added one suggestion to improve validation by providing a clearer error message when a user incorrectly passes an array for a non-repeated parameter, which will prevent confusing API errors.
Address review feedback: print a warning to stderr when a JSON array is provided for a parameter not marked as repeated, since the array will be stringified rather than expanded. Directs users to gws schema to check which parameters accept arrays.
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request successfully implements the exposure of the repeated field in gws schema output and expands JSON arrays into multiple query parameters for repeated fields. The changes are well-contained within src/executor.rs and src/schema.rs, with appropriate updates to existing tests and the addition of new tests to cover the new functionality. The code is clear and directly addresses the described problem, enhancing the CLI's capability to handle complex API parameters correctly.
The test `test_build_url_repeated_query_param_expands_array` uses MethodParameter but it was not imported in the test module's use statement, causing a compilation error in CI.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request correctly exposes the repeated field in the schema output and updates build_url to expand array values into multiple query parameters. However, there is a critical issue in how these query parameters are applied to the HTTP request. The current implementation in build_http_request will cause only the last query parameter to be sent, defeating the purpose of this change. I've added a critical review comment with a suggested fix.
Consolidate query parameters (including pageToken) into a single request.query() call to ensure repeated parameters are sent correctly instead of being overwritten by successive calls.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request addresses issues with repeated/array query parameters by modifying the schema output to include a repeated flag and updating the request executor to expand JSON arrays into multiple query parameters. The data structure for query parameters has been changed from a HashMap to a Vec<(String, String)> to support repeated keys. The changes are accompanied by new tests. I have reviewed the changes and found no issues of high or critical severity.
Summary
Two related fixes for repeated/array query parameters:
gws schemanow includes"repeated": truein parameter output when the Discovery Document marks a parameter as repeated--paramscontains a JSON array for arepeatedparameter, the executor expands it into multiple query parameters instead of stringifying the arrayProblem
gws schema gmail.users.messages.getshowedmetadataHeadersasstringwith no indication it accepts multiple values. Passing{"metadataHeaders": ["Subject", "Date"]}had no effect because the array was stringified as a single value.Solution
Schema (
src/schema.rs):Executor (
src/executor.rs):query_paramsfromHashMap<String, String>toVec<(String, String)>to support multiple entries with the same keyrepeated: trueand the value is a JSON array, each element becomes a separate query parameter entryBefore:
?metadataHeaders=["Subject","Date","From"](ignored by API)After:
?metadataHeaders=Subject&metadataHeaders=Date&metadataHeaders=FromTest plan
param_to_jsonwithrepeated: trueincludes field in outputparam_to_jsonwithrepeated: falseomits fieldbuild_urlexpands JSON array into repeated query paramsVec<(String, String)>typeCloses #300