Simplify FromRequest::from_request for QueryConfigParams (#802)

It seems that the code introduces a few unnecessary local structs to parse http query parameters into. But these structs are the same as the final QueryType-nested structs. So, we can parse the params into those structs directly and make the code simpler Co-authored-by: Artem Ignatyev <[email protected]>
private-attribution · Oct 13, 2023 · 64f6fc2 · 64f6fc2
1 parent c75dfd7
commit 64f6fc2
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 72 deletions.
diff --git a/src/helpers/transport/query.rs b/src/helpers/transport/query.rs
@@ -250,6 +250,7 @@ pub struct IpaQueryConfig {
     /// input report format in which all fields are secret-shared. This option is provided
     /// only for development and testing purposes and may be removed in the future.
     #[cfg_attr(feature = "clap", arg(long))]
+    #[serde(default)]
     pub plaintext_match_keys: bool,
 }
 

diff --git a/src/net/http_serde.rs b/src/net/http_serde.rs
@@ -80,20 +80,14 @@ pub mod echo {
 }
 
 pub mod query {
-    use std::{
-        fmt::{Display, Formatter},
-        num::NonZeroU32,
-    };
+    use std::fmt::{Display, Formatter};
 
     use async_trait::async_trait;
     use axum::extract::{FromRequest, Query, RequestParts};
 
     use crate::{
         ff::FieldType,
-        helpers::query::{
-            ContributionBits, IpaQueryConfig, QueryConfig, QuerySize, QueryType,
-            SparseAggregateQueryConfig,
-        },
+        helpers::query::{QueryConfig, QuerySize, QueryType},
         net::Error,
     };
 
@@ -129,71 +123,21 @@ pub mod query {
             let query_type = match query_type.as_str() {
                 #[cfg(any(test, feature = "cli", feature = "test-fixture"))]
                 QueryType::TEST_MULTIPLY_STR => Ok(QueryType::TestMultiply),
-                QueryType::SEMIHONEST_IPA_STR | QueryType::MALICIOUS_IPA_STR => {
-                    #[derive(serde::Deserialize)]
-                    struct IPAQueryConfigParam {
-                        per_user_credit_cap: u32,
-                        max_breakdown_key: u32,
-                        attribution_window_seconds: Option<NonZeroU32>,
-                        num_multi_bits: u32,
-                        #[serde(default)]
-                        plaintext_match_keys: bool,
-                    }
-                    let Query(IPAQueryConfigParam {
-                        per_user_credit_cap,
-                        max_breakdown_key,
-                        attribution_window_seconds,
-                        num_multi_bits,
-                        plaintext_match_keys,
-                    }) = req.extract().await?;
-
-                    match query_type.as_str() {
-                        QueryType::SEMIHONEST_IPA_STR => {
-                            Ok(QueryType::SemiHonestIpa(IpaQueryConfig {
-                                per_user_credit_cap,
-                                max_breakdown_key,
-                                attribution_window_seconds,
-                                num_multi_bits,
-                                plaintext_match_keys,
-                            }))
-                        }
-                        QueryType::MALICIOUS_IPA_STR => {
-                            Ok(QueryType::MaliciousIpa(IpaQueryConfig {
-                                per_user_credit_cap,
-                                max_breakdown_key,
-                                attribution_window_seconds,
-                                num_multi_bits,
-                                plaintext_match_keys,
-                            }))
-                        }
-                        &_ => unreachable!(),
-                    }
+                QueryType::SEMIHONEST_IPA_STR => {
+                    let Query(q) = req.extract().await?;
+                    Ok(QueryType::SemiHonestIpa(q))
                 }
-                QueryType::SEMIHONEST_AGGREGATE_STR | QueryType::MALICIOUS_AGGREGATE_STR => {
-                    #[derive(serde::Deserialize)]
-                    struct AggregateQueryConfigParam {
-                        contribution_bits: ContributionBits,
-                        num_contributions: u32,
-                    }
-                    let Query(AggregateQueryConfigParam {
-                        contribution_bits,
-                        num_contributions,
-                    }) = req.extract().await?;
-                    match query_type.as_str() {
-                        QueryType::SEMIHONEST_AGGREGATE_STR => Ok(
-                            QueryType::SemiHonestSparseAggregate(SparseAggregateQueryConfig {
-                                contribution_bits,
-                                num_contributions,
-                            }),
-                        ),
-                        QueryType::MALICIOUS_AGGREGATE_STR => Ok(
-                            QueryType::MaliciousSparseAggregate(SparseAggregateQueryConfig {
-                                contribution_bits,
-                                num_contributions,
-                            }),
-                        ),
-                        &_ => unreachable!(),
-                    }
+                QueryType::MALICIOUS_IPA_STR => {
+                    let Query(q) = req.extract().await?;
+                    Ok(QueryType::MaliciousIpa(q))
+                }
+                QueryType::SEMIHONEST_AGGREGATE_STR => {
+                    let Query(q) = req.extract().await?;
+                    Ok(QueryType::SemiHonestSparseAggregate(q))
+                }
+                QueryType::MALICIOUS_AGGREGATE_STR => {
+                    let Query(q) = req.extract().await?;
+                    Ok(QueryType::MaliciousSparseAggregate(q))
                 }
                 other => Err(Error::bad_query_value("query_type", other)),
             }?;