Add method to retrieve all users from Firebase Auth#44
Conversation
WalkthroughAdds project-scoped Firebase Auth URL building, token retrieval helpers, per-request auth for GET/POST, and concurrent batch user listing (partitioned workers, pagination, HashMap dedup). Introduces a BatchGetResponse model and a single-line URL formatting tweak in the Google token endpoint. Two new public methods added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Caller
participant FAC as FirebaseAuthClient
participant Auth as Google OAuth
participant API as Identity Toolkit
Note over Caller,FAC: Caller -> get_all_users_concurrently(num_workers)
Caller->>FAC: get_all_users_concurrently(n)
activate FAC
FAC->>FAC: build worker partitions (0-9A-Za-z)
FAC->>FAC: spawn workers
par worker loop
loop per-page
FAC->>FAC: get_access_token()
FAC->>Auth: POST token endpoint (client creds)
Auth-->>FAC: access_token
FAC->>API: GET accounts:batchGet?maxResults&nextPageToken&filter (Authorization header)
API-->>FAC: BatchGetResponse { users?, nextPageToken? }
FAC->>FAC: collect users
end
and
end
FAC->>FAC: merge & deduplicate (HashMap by uid)
FAC-->>Caller: Vec<User>
deactivate FAC
sequenceDiagram
autonumber
participant FAC as FirebaseAuthClient
participant Auth as Google OAuth
FAC->>FAC: get_access_token()
activate FAC
FAC->>Auth: POST token endpoint (client creds)
Auth-->>FAC: access_token | error
deactivate FAC
Note over FAC: Tokens used by auth_get/auth_post via Authorization: Bearer <token>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Comment |
0ec4a9a to
471ece4
Compare
471ece4 to
0d674d4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/auth/credential/api_auth_token.rs(1 hunks)src/auth/mod.rs(5 hunks)src/auth/models/mod.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/auth/credential/api_auth_token.rs (1)
src/auth/mod.rs (1)
url(54-56)
src/auth/mod.rs (2)
src/auth/credential/api_auth_token.rs (3)
new(19-25)get_access_token(27-38)res(73-74)src/auth/credential/token_handler.rs (1)
new(18-23)
🔇 Additional comments (15)
src/auth/credential/api_auth_token.rs (1)
55-55: LGTM - URL formatting simplified.The change from a multiline
format!block to a single-line format is cleaner and more readable without changing the functionality.src/auth/models/mod.rs (1)
14-19: LGTM - Clean model for batch response.The
BatchGetResponsestruct is well-structured with proper serde attributes. ThecamelCaserenaming and optional fields align perfectly with Firebase's API response format.src/auth/mod.rs (13)
1-1: Good addition for deduplication.Adding
HashMapimport is necessary for the deduplication logic in the concurrent user retrieval.
10-10: LGTM - Model import added.The
BatchGetResponseimport is correctly added to support the new batch retrieval functionality.
31-31: LGTM - Project ID field added.The
project_idfield is properly extracted from the service account and stored for use in project-scoped URLs.Also applies to: 42-42, 50-50
58-65: LGTM - Helper method for project URLs.The
project_urlhelper method correctly constructs project-scoped Firebase API URLs using the stored project ID.
67-78: LGTM - Access token retrieval method.The
get_access_tokenmethod properly wraps the token manager call with error logging. The error handling preserves the original error while adding trace logging.
80-94: LGTM - Auth POST method.The
auth_postmethod correctly retrieves an access token and creates a request builder with the Authorization header. Good separation of concerns.
96-110: LGTM - Auth GET method.The
auth_getmethod mirrors theauth_postpattern perfectly, providing consistent authorization header handling for GET requests.
345-353: LGTM - Simple wrapper method.The
get_all_usersmethod is a clean wrapper that defaults to single-threaded execution viaget_all_users_concurrently(1).
355-369: Good input validation and documentation.The method signature and docs clearly explain the worker limits and API rate limits. The clamping of
num_workersis a good safety measure.
371-382: LGTM - Clean URL builder.The
make_pagination_urlhelper function correctly constructs pagination URLs with optional next page tokens.
384-402: Smart partitioning strategy.The character-based partitioning using alphanumeric ranges is clever - it distributes users across workers based on UID prefixes. The range calculation handles the last chunk properly to include all remaining characters.
404-444: Solid worker implementation.The
run_workerfunction handles pagination correctly, processes batch responses properly, and has good boundary checking with themaxparameter to prevent overlap between workers.
326-327: auth_post usage is correct; no additional direct self.client.{post,get} calls were found.
f90202b to
84683b2
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/auth/mod.rs (2)
36-39: Set a client-wide timeout.reqwest defaults to no timeout. Add one to prevent hanging on network stalls.
let client = reqwest::Client::builder() .https_only(true) + .timeout(std::time::Duration::from_secs(30)) .build() .context("Failed to create HTTP client")?;
749-749: Typo in log format string.There’s a stray apostrophe in the error log.
- tracing::error!("{:?}'", &err); + tracing::error!("{:?}", &err);
♻️ Duplicate comments (1)
src/auth/mod.rs (1)
459-461: Nice: efficient dedup via into_values().This addresses the prior review about avoiding the extra key Vec.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/auth/mod.rs(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/auth/mod.rs (1)
src/auth/credential/api_auth_token.rs (2)
new(19-25)get_access_token(27-38)
🔇 Additional comments (4)
src/auth/mod.rs (4)
58-65: Project-scoped URL helper looks good.Correct path shape for v1 project-scoped endpoints.
420-427: Error message clarity.“Failed to get all users” is fine, but echoing the response status/body is already handled by response_error. All good—just noting we’re consistent.
345-353: Wrapper method is fine.get_all_users delegating to the concurrent variant (now sequential pager) is clean.
355-462: No changes needed; pagination semantics are correct. The Identity Toolkit API usesmaxResults(up to 1000) andnextPageToken, and you must use the opaque token returned by the API—your implementation aligns with these specs.
| async fn get_access_token(&self) -> Result<String, FirebaseError> { | ||
| let access_token = self | ||
| .api_auth_token_manager | ||
| .get_access_token() | ||
| .await | ||
| .map_err(|e| { | ||
| tracing::error!("Failed to get access token: {}", e); | ||
| tracing::error!("Failed to get access token: {e}"); | ||
| e | ||
| })?; | ||
|
|
||
| Ok(access_token) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Minor: avoid double-logging errors.
You log in map_err and again via response_error at call sites. Consider downgrading this log to debug to reduce noise.
- tracing::error!("Failed to get access token: {e}");
+ tracing::debug!("Failed to get access token: {e}");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async fn get_access_token(&self) -> Result<String, FirebaseError> { | |
| let access_token = self | |
| .api_auth_token_manager | |
| .get_access_token() | |
| .await | |
| .map_err(|e| { | |
| tracing::error!("Failed to get access token: {}", e); | |
| tracing::error!("Failed to get access token: {e}"); | |
| e | |
| })?; | |
| Ok(access_token) | |
| } | |
| async fn get_access_token(&self) -> Result<String, FirebaseError> { | |
| let access_token = self | |
| .api_auth_token_manager | |
| .get_access_token() | |
| .await | |
| .map_err(|e| { | |
| tracing::debug!("Failed to get access token: {e}"); | |
| e | |
| })?; | |
| Ok(access_token) | |
| } |
🤖 Prompt for AI Agents
In src/auth/mod.rs around lines 67 to 78, the map_err closure currently logs the
error at error level which causes duplicate noisy logs at call sites; change
that tracing::error! call to tracing::debug! (or remove the log entirely) so the
error is still mapped through unchanged but only gets logged at callers or in
debug mode, keeping the map_err closure as a pass-through that does not produce
redundant error-level logs.
| /// Creates a new `GET` request builder with the `Authorization` header set | ||
| /// to an authorized admin access token. | ||
| async fn auth_get( | ||
| &self, | ||
| url: impl AsRef<str>, | ||
| ) -> Result<reqwest::RequestBuilder, FirebaseError> { | ||
| let access_token = self.get_access_token().await?; | ||
|
|
||
| let builder = self | ||
| .client | ||
| .get(url.as_ref()) | ||
| .header("Authorization", format!("Bearer {access_token}")); | ||
|
|
||
| Ok(builder) | ||
| } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Optional: set Accept header for JSON responses.
Not required, but can make intent explicit.
- .get(url.as_ref())
+ .get(url.as_ref())
+ .header(reqwest::header::ACCEPT, "application/json")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Creates a new `GET` request builder with the `Authorization` header set | |
| /// to an authorized admin access token. | |
| async fn auth_get( | |
| &self, | |
| url: impl AsRef<str>, | |
| ) -> Result<reqwest::RequestBuilder, FirebaseError> { | |
| let access_token = self.get_access_token().await?; | |
| let builder = self | |
| .client | |
| .get(url.as_ref()) | |
| .header("Authorization", format!("Bearer {access_token}")); | |
| Ok(builder) | |
| } | |
| /// Creates a new `GET` request builder with the `Authorization` header set | |
| /// to an authorized admin access token. | |
| async fn auth_get( | |
| &self, | |
| url: impl AsRef<str>, | |
| ) -> Result<reqwest::RequestBuilder, FirebaseError> { | |
| let access_token = self.get_access_token().await?; | |
| let builder = self | |
| .client | |
| .get(url.as_ref()) | |
| .header(reqwest::header::ACCEPT, "application/json") | |
| .header("Authorization", format!("Bearer {access_token}")); | |
| Ok(builder) | |
| } |
🤖 Prompt for AI Agents
In src/auth/mod.rs around lines 96 to 111, the auth_get builder sets the
Authorization header but does not declare the expected response content type;
add an Accept: application/json header to the request builder so intent is
explicit. Update the chain that builds `builder` to include an
`.header("Accept", "application/json")` alongside the existing Authorization
header and return the resulting builder unchanged.
| /// Retrieve all users in Firebase Auth using `num_workers` concurrent workers. See | ||
| /// [`get_all_users`](Self::get_all_users) for more info. | ||
| /// | ||
| /// The number of workers should be between 1 and 62. If you pass a number outside | ||
| /// this range, it will be clamped. | ||
| /// | ||
| /// Keep in mind that, at the time of writing, the Identity Toolkit API limit is 500 | ||
| /// requests/second per service account. This method will fire off multiple requests | ||
| /// simultaneously. See the up-to-date limits in the [official docs](https://firebase.google.com/docs/auth/limits). | ||
| #[tracing::instrument(name = "Get all users concurrently", skip_all)] |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Docs: adjust wording after dropping sharding.
The docstring still advertises concurrent workers. Update to reflect sequential paging or document why concurrency isn’t supported for listing.
🤖 Prompt for AI Agents
In src/auth/mod.rs around lines 355 to 364, the docstring still describes
concurrent workers and sharding for listing users; update it to reflect the
current sequential paging implementation (or explicitly state why concurrency
was removed/not supported). Replace references to "num_workers" and "concurrent
workers" with wording that explains the method pages sequentially through
results, mention the API rate limits and that requests are issued one page at a
time, and/or add a brief note explaining why concurrent listing isn't used
(e.g., lack of sharded endpoints or to respect per-service-account rate limits).
| /// Retrieve all users in Firebase Auth using `num_workers` concurrent workers. See | ||
| /// [`get_all_users`](Self::get_all_users) for more info. | ||
| /// | ||
| /// The number of workers should be between 1 and 62. If you pass a number outside | ||
| /// this range, it will be clamped. | ||
| /// | ||
| /// Keep in mind that, at the time of writing, the Identity Toolkit API limit is 500 | ||
| /// requests/second per service account. This method will fire off multiple requests | ||
| /// simultaneously. See the up-to-date limits in the [official docs](https://firebase.google.com/docs/auth/limits). | ||
| #[tracing::instrument(name = "Get all users concurrently", skip_all)] | ||
| pub async fn get_all_users_concurrently( | ||
| &self, | ||
| num_workers: u8, | ||
| ) -> Result<Vec<User>, FirebaseError> { | ||
| let base_url = self.project_url("/accounts:batchGet"); | ||
|
|
||
| fn make_pagination_url( | ||
| base_url: &str, | ||
| max_results: usize, | ||
| next_page_token: Option<&str>, | ||
| ) -> String { | ||
| format!( | ||
| "{base_url}?maxResults={max_results}{}", | ||
| next_page_token | ||
| .map(|token| format!("&nextPageToken={token}")) | ||
| .unwrap_or_else(|| { "".to_string() }) | ||
| ) | ||
| } | ||
|
|
||
| let legal = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" | ||
| .chars() | ||
| .collect::<Vec<_>>(); | ||
|
|
||
| let num_workers = (num_workers as usize).clamp(1, legal.len()); | ||
|
|
||
| let chunk_size = legal.len() / num_workers; | ||
| let mut ranges = Vec::new(); | ||
|
|
||
| for i in 0..num_workers { | ||
| let min = legal[i * chunk_size]; | ||
| let max = if i < num_workers - 1 { | ||
| legal[(i + 1) * chunk_size] | ||
| } else { | ||
| (*legal.last().unwrap() as u8 + 1) as char | ||
| }; | ||
|
|
||
| ranges.push((min, max)); | ||
| } | ||
|
|
||
| async fn run_worker( | ||
| client: &FirebaseAuthClient, | ||
| range: (char, char), | ||
| base_url: &str, | ||
| ) -> Result<Vec<User>, FirebaseError> { | ||
| const MAX_RESULTS: usize = 1000; | ||
| let (min, max) = (range.0.to_string(), range.1.to_string()); | ||
|
|
||
| let mut all_users = Vec::new(); | ||
| let mut next_page_token = Some(min); | ||
| loop { | ||
| let url = make_pagination_url(base_url, MAX_RESULTS, next_page_token.as_deref()); | ||
|
|
||
| let res = client | ||
| .auth_get(url) | ||
| .await? | ||
| .header("Content-Type", "application/json") | ||
| .send() | ||
| .await | ||
| .context("Failed to send get all users request")?; | ||
|
|
||
| if !res.status().is_success() { | ||
| return Err(response_error("Failed to get all users", res).await); | ||
| } | ||
|
|
||
| let res_body: BatchGetResponse = | ||
| res.json().await.context("Failed to read response JSON")?; | ||
|
|
||
| if let Some(mut users) = res_body.users { | ||
| all_users.append(&mut users); | ||
| } | ||
|
|
||
| next_page_token = res_body.next_page_token.map(|t| t.to_string()); | ||
|
|
||
| if next_page_token.is_none() || next_page_token.as_deref() >= Some(&max) { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| Ok(all_users) | ||
| } | ||
|
|
||
| let futures = ranges | ||
| .into_iter() | ||
| .map(|range| run_worker(self, range, &base_url)); | ||
|
|
||
| let worker_results = futures::future::try_join_all(futures).await?; | ||
|
|
||
| let mut deduped = HashMap::new(); | ||
| for users in worker_results { | ||
| for user in users { | ||
| deduped.insert(user.uid.clone(), user); | ||
| } | ||
| } | ||
|
|
||
| let all_users = deduped.into_values().collect(); | ||
|
|
||
| Ok(all_users) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Concurrency approach is incorrect: nextPageToken is opaque (this will miss/duplicate users).
You're sharding by ASCII ranges and seeding nextPageToken with arbitrary chars. Per Google APIs, nextPageToken (or pageToken) is an opaque cursor returned by the server; you can't manufacture starting tokens. This will either 400, start all workers at the beginning, or silently drop/dup pages. Replace with sequential pagination.
Apply this diff to switch to a single sequential pager (keeps the public API, ignores num_workers):
@@
- #[tracing::instrument(name = "Get all users concurrently", skip_all)]
- pub async fn get_all_users_concurrently(
- &self,
- num_workers: u8,
- ) -> Result<Vec<User>, FirebaseError> {
- let base_url = self.project_url("/accounts:batchGet");
-
- fn make_pagination_url(
- base_url: &str,
- max_results: usize,
- next_page_token: Option<&str>,
- ) -> String {
- format!(
- "{base_url}?maxResults={max_results}{}",
- next_page_token
- .map(|token| format!("&nextPageToken={token}"))
- .unwrap_or_else(|| { "".to_string() })
- )
- }
-
- let legal = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
- .chars()
- .collect::<Vec<_>>();
-
- let num_workers = (num_workers as usize).clamp(1, legal.len());
-
- let chunk_size = legal.len() / num_workers;
- let mut ranges = Vec::new();
-
- for i in 0..num_workers {
- let min = legal[i * chunk_size];
- let max = if i < num_workers - 1 {
- legal[(i + 1) * chunk_size]
- } else {
- (*legal.last().unwrap() as u8 + 1) as char
- };
-
- ranges.push((min, max));
- }
-
- async fn run_worker(
- client: &FirebaseAuthClient,
- range: (char, char),
- base_url: &str,
- ) -> Result<Vec<User>, FirebaseError> {
- const MAX_RESULTS: usize = 1000;
- let (min, max) = (range.0.to_string(), range.1.to_string());
-
- let mut all_users = Vec::new();
- let mut next_page_token = Some(min);
- loop {
- let url = make_pagination_url(base_url, MAX_RESULTS, next_page_token.as_deref());
-
- let res = client
- .auth_get(url)
- .await?
- .header("Content-Type", "application/json")
- .send()
- .await
- .context("Failed to send get all users request")?;
-
- if !res.status().is_success() {
- return Err(response_error("Failed to get all users", res).await);
- }
-
- let res_body: BatchGetResponse =
- res.json().await.context("Failed to read response JSON")?;
-
- if let Some(mut users) = res_body.users {
- all_users.append(&mut users);
- }
-
- next_page_token = res_body.next_page_token.map(|t| t.to_string());
-
- if next_page_token.is_none() || next_page_token.as_deref() >= Some(&max) {
- break;
- }
- }
-
- Ok(all_users)
- }
-
- let futures = ranges
- .into_iter()
- .map(|range| run_worker(self, range, &base_url));
-
- let worker_results = futures::future::try_join_all(futures).await?;
-
- let mut deduped = HashMap::new();
- for users in worker_results {
- for user in users {
- deduped.insert(user.uid.clone(), user);
- }
- }
-
- let all_users = deduped.into_values().collect();
-
- Ok(all_users)
- }
+ #[tracing::instrument(name = "Get all users concurrently", skip_all)]
+ pub async fn get_all_users_concurrently(
+ &self,
+ _num_workers: u8,
+ ) -> Result<Vec<User>, FirebaseError> {
+ let base_url = self.project_url("/accounts:batchGet");
+ const MAX_RESULTS: usize = 1000;
+
+ let mut all_users = Vec::new();
+ let mut next_page_token: Option<String> = None;
+
+ loop {
+ let mut req = self.auth_get(&base_url).await?;
+ req = req.query(&[("maxResults", MAX_RESULTS)]);
+ if let Some(ref token) = next_page_token {
+ req = req.query(&[("nextPageToken", token)]);
+ }
+
+ let res = req
+ .send()
+ .await
+ .context("Failed to send get all users request")?;
+
+ if !res.status().is_success() {
+ return Err(response_error("Failed to get all users", res).await);
+ }
+
+ let res_body: BatchGetResponse =
+ res.json().await.context("Failed to read response JSON")?;
+
+ if let Some(mut users) = res_body.users {
+ all_users.append(&mut users);
+ }
+
+ next_page_token = res_body.next_page_token;
+ if next_page_token.is_none() {
+ break;
+ }
+ }
+
+ Ok(all_users)
+ }If you still want concurrency, you’ll need a different strategy (e.g., a single pager that feeds a bounded channel to N workers for per-user follow-up work). The listing itself can’t be sharded by token.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Retrieve all users in Firebase Auth using `num_workers` concurrent workers. See | |
| /// [`get_all_users`](Self::get_all_users) for more info. | |
| /// | |
| /// The number of workers should be between 1 and 62. If you pass a number outside | |
| /// this range, it will be clamped. | |
| /// | |
| /// Keep in mind that, at the time of writing, the Identity Toolkit API limit is 500 | |
| /// requests/second per service account. This method will fire off multiple requests | |
| /// simultaneously. See the up-to-date limits in the [official docs](https://firebase.google.com/docs/auth/limits). | |
| #[tracing::instrument(name = "Get all users concurrently", skip_all)] | |
| pub async fn get_all_users_concurrently( | |
| &self, | |
| num_workers: u8, | |
| ) -> Result<Vec<User>, FirebaseError> { | |
| let base_url = self.project_url("/accounts:batchGet"); | |
| fn make_pagination_url( | |
| base_url: &str, | |
| max_results: usize, | |
| next_page_token: Option<&str>, | |
| ) -> String { | |
| format!( | |
| "{base_url}?maxResults={max_results}{}", | |
| next_page_token | |
| .map(|token| format!("&nextPageToken={token}")) | |
| .unwrap_or_else(|| { "".to_string() }) | |
| ) | |
| } | |
| let legal = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" | |
| .chars() | |
| .collect::<Vec<_>>(); | |
| let num_workers = (num_workers as usize).clamp(1, legal.len()); | |
| let chunk_size = legal.len() / num_workers; | |
| let mut ranges = Vec::new(); | |
| for i in 0..num_workers { | |
| let min = legal[i * chunk_size]; | |
| let max = if i < num_workers - 1 { | |
| legal[(i + 1) * chunk_size] | |
| } else { | |
| (*legal.last().unwrap() as u8 + 1) as char | |
| }; | |
| ranges.push((min, max)); | |
| } | |
| async fn run_worker( | |
| client: &FirebaseAuthClient, | |
| range: (char, char), | |
| base_url: &str, | |
| ) -> Result<Vec<User>, FirebaseError> { | |
| const MAX_RESULTS: usize = 1000; | |
| let (min, max) = (range.0.to_string(), range.1.to_string()); | |
| let mut all_users = Vec::new(); | |
| let mut next_page_token = Some(min); | |
| loop { | |
| let url = make_pagination_url(base_url, MAX_RESULTS, next_page_token.as_deref()); | |
| let res = client | |
| .auth_get(url) | |
| .await? | |
| .header("Content-Type", "application/json") | |
| .send() | |
| .await | |
| .context("Failed to send get all users request")?; | |
| if !res.status().is_success() { | |
| return Err(response_error("Failed to get all users", res).await); | |
| } | |
| let res_body: BatchGetResponse = | |
| res.json().await.context("Failed to read response JSON")?; | |
| if let Some(mut users) = res_body.users { | |
| all_users.append(&mut users); | |
| } | |
| next_page_token = res_body.next_page_token.map(|t| t.to_string()); | |
| if next_page_token.is_none() || next_page_token.as_deref() >= Some(&max) { | |
| break; | |
| } | |
| } | |
| Ok(all_users) | |
| } | |
| let futures = ranges | |
| .into_iter() | |
| .map(|range| run_worker(self, range, &base_url)); | |
| let worker_results = futures::future::try_join_all(futures).await?; | |
| let mut deduped = HashMap::new(); | |
| for users in worker_results { | |
| for user in users { | |
| deduped.insert(user.uid.clone(), user); | |
| } | |
| } | |
| let all_users = deduped.into_values().collect(); | |
| Ok(all_users) | |
| } | |
| #[tracing::instrument(name = "Get all users concurrently", skip_all)] | |
| pub async fn get_all_users_concurrently( | |
| &self, | |
| _num_workers: u8, | |
| ) -> Result<Vec<User>, FirebaseError> { | |
| let base_url = self.project_url("/accounts:batchGet"); | |
| const MAX_RESULTS: usize = 1000; | |
| let mut all_users = Vec::new(); | |
| let mut next_page_token: Option<String> = None; | |
| loop { | |
| let mut req = self.auth_get(&base_url).await?; | |
| req = req.query(&[("maxResults", MAX_RESULTS)]); | |
| if let Some(ref token) = next_page_token { | |
| req = req.query(&[("nextPageToken", token)]); | |
| } | |
| let res = req | |
| .send() | |
| .await | |
| .context("Failed to send get all users request")?; | |
| if !res.status().is_success() { | |
| return Err(response_error("Failed to get all users", res).await); | |
| } | |
| let res_body: BatchGetResponse = | |
| res.json().await.context("Failed to read response JSON")?; | |
| if let Some(mut users) = res_body.users { | |
| all_users.append(&mut users); | |
| } | |
| next_page_token = res_body.next_page_token; | |
| if next_page_token.is_none() { | |
| break; | |
| } | |
| } | |
| Ok(all_users) | |
| } |
🤖 Prompt for AI Agents
In src/auth/mod.rs around lines 355 to 462, the current implementation shards by
ASCII ranges and seeds nextPageToken with fabricated values which is invalid
because nextPageToken is an opaque server-issued cursor and this will
miss/duplicate pages or error; replace the whole concurrent sharding logic with
a single sequential pager that ignores num_workers: call the same
/accounts:batchGet endpoint in a loop, starting with no page token, build each
page URL with the returned next_page_token until it is None, accumulate users
into a HashMap to dedupe by uid, and return the collected values; remove the
legal/ranges/chunking and run_worker logic and update the docstring to note
num_workers is ignored (or clamp it but not used), or alternatively implement a
producer-consumer where one pager feeds a bounded channel to workers if you need
concurrency for per-user processing.
| /// Keep in mind that, at the time of writing, the Identity Toolkit API limit is 500 | ||
| /// requests/second per service account. This method will fire off multiple requests | ||
| /// simultaneously. See the up-to-date limits in the [official docs](https://firebase.google.com/docs/auth/limits). | ||
| #[tracing::instrument(name = "Get all users concurrently", skip_all)] |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider bounding maximum workers even if reintroducing concurrency later.
If you bring back concurrency for other flows, cap it (e.g., min(num_workers, 16)) and use a Semaphore to avoid bursts.
🤖 Prompt for AI Agents
In src/auth/mod.rs around lines 361 to 364, the comment and instrumented method
currently allow unbounded concurrent requests; when reintroducing concurrency
cap the maximum workers to avoid exceeding API limits (for example set
max_workers = min(num_workers, 16)) and protect concurrent launches with a
tokio::sync::Semaphore (acquire a permit per task and release after completion)
so bursts are limited and the request rate stays within quota.
🛠️ Refactor suggestion
Add retries/backoff and a per-request timeout to respect API limits.
You note 500 req/s, but there’s no retry/backoff or timeout. Add exponential backoff on 429/5xx and a sane timeout to avoid hung tasks.
Would you like a small helper (with jitter) wired into auth_get/auth_post?
| fn make_pagination_url( | ||
| base_url: &str, | ||
| max_results: usize, | ||
| next_page_token: Option<&str>, | ||
| ) -> String { | ||
| format!( | ||
| "{base_url}?maxResults={max_results}{}", | ||
| next_page_token | ||
| .map(|token| format!("&nextPageToken={token}")) | ||
| .unwrap_or_else(|| { "".to_string() }) | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t hand-roll query strings; URL-encode tokens or use .query().
Manually formatting nextPageToken risks broken URLs if the token contains reserved chars. Use reqwest’s .query(...) instead.
Apply this minimal change if you keep the current structure:
- let url = make_pagination_url(base_url, MAX_RESULTS, next_page_token.as_deref());
- let res = client
- .auth_get(url)
- .await?
- .header("Content-Type", "application/json")
- .send()
+ let mut req = client.auth_get(base_url).await?;
+ req = req.query(&[("maxResults", MAX_RESULTS)]);
+ if let Some(ref t) = next_page_token { req = req.query(&[("nextPageToken", t)]); }
+ let res = req
+ .send()
.await
.context("Failed to send get all users request")?;Committable suggestion skipped: line range outside the PR's diff.
| let res = client | ||
| .auth_get(url) | ||
| .await? | ||
| .header("Content-Type", "application/json") |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Drop Content-Type on GET.
GETs don’t have a body; setting Content-Type is misleading. If anything, set Accept: application/json.
- .header("Content-Type", "application/json")
+ // Optionally: .header(reqwest::header::ACCEPT, "application/json")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .header("Content-Type", "application/json") | |
| // Optionally: .header(reqwest::header::ACCEPT, "application/json") |
🤖 Prompt for AI Agents
In src/auth/mod.rs around line 420, the request builder sets "Content-Type:
application/json" on a GET which is incorrect; remove the
.header("Content-Type", "application/json") call and, if the server expects JSON
responses, add .header("Accept", "application/json") instead so the GET is not
advertising a body content type but still expresses it expects JSON.
1 participant
Adds two new methods to the Firebase Auth client:
get_all_users, which retrieves all users from Firebase Auth (the multi-user version ofget_user)get_all_users_concurrently, which retrieves the users using a specified number of workers, which can speed up the retrieval significantly for large user setsSummary by CodeRabbit