Skip to content

Commit 6824972

Browse files
shyjsarahjerry-024
authored andcommitted
fix(api): honor REST config overrides
Match the Java REST client merge semantics: treat null config maps as empty, apply defaults then client options then overrides, and remove entries whose final override value is null.
1 parent 7afff74 commit 6824972

1 file changed

Lines changed: 164 additions & 7 deletions

File tree

crates/paimon/src/api/api_response.rs

Lines changed: 164 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
//!
2020
//! This module contains all response structures used in REST API calls.
2121
22-
use serde::{Deserialize, Serialize};
22+
use serde::{Deserialize, Deserializer, Serialize};
2323
use std::collections::HashMap;
2424

2525
use crate::catalog::{Function, FunctionDefinition, ViewSchema};
@@ -266,31 +266,78 @@ impl GetDatabaseResponse {
266266
}
267267
}
268268

269-
/// Response containing configuration defaults.
269+
/// Response containing server-provided configuration defaults and overrides.
270270
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
271271
#[serde(rename_all = "camelCase")]
272272
pub struct ConfigResponse {
273273
/// Default configuration values.
274+
#[serde(default, deserialize_with = "deserialize_config_defaults")]
274275
pub defaults: HashMap<String, String>,
276+
/// Configuration values that override client-provided options.
277+
///
278+
/// A null value removes the corresponding option after all maps are merged.
279+
#[serde(default, deserialize_with = "deserialize_null_to_default")]
280+
pub overrides: HashMap<String, Option<String>>,
281+
}
282+
283+
fn deserialize_null_to_default<'de, D, T>(deserializer: D) -> Result<T, D::Error>
284+
where
285+
D: Deserializer<'de>,
286+
T: Deserialize<'de> + Default,
287+
{
288+
Ok(Option::<T>::deserialize(deserializer)?.unwrap_or_default())
289+
}
290+
291+
fn deserialize_config_defaults<'de, D>(deserializer: D) -> Result<HashMap<String, String>, D::Error>
292+
where
293+
D: Deserializer<'de>,
294+
{
295+
let defaults =
296+
Option::<HashMap<String, Option<String>>>::deserialize(deserializer)?.unwrap_or_default();
297+
// Java filters null values after merging. Defaults have the lowest precedence, so dropping
298+
// their null entries here produces the same result while preserving the existing field type.
299+
Ok(defaults
300+
.into_iter()
301+
.filter_map(|(key, value)| value.map(|value| (key, value)))
302+
.collect())
275303
}
276304

277305
impl ConfigResponse {
278306
/// Create a new ConfigResponse.
279307
pub fn new(defaults: HashMap<String, String>) -> Self {
280-
Self { defaults }
308+
Self {
309+
defaults,
310+
overrides: HashMap::new(),
311+
}
281312
}
282313

283-
/// Merge these defaults with the provided Options.
284-
/// User options take precedence over defaults.
314+
/// Merge server defaults and overrides with the provided Options.
315+
/// Overrides take precedence over user options, which take precedence over defaults.
285316
pub fn merge_options(&self, options: &crate::common::Options) -> crate::common::Options {
286317
let mut merged = self.defaults.clone();
287318
merged.extend(options.to_map().clone());
319+
self.apply_overrides(&mut merged);
288320
crate::common::Options::from_map(merged)
289321
}
290322

291-
/// Convert to Options struct.
323+
/// Convert server-provided defaults and overrides to Options.
292324
pub fn to_options(&self) -> crate::common::Options {
293-
crate::common::Options::from_map(self.defaults.clone())
325+
let mut options = self.defaults.clone();
326+
self.apply_overrides(&mut options);
327+
crate::common::Options::from_map(options)
328+
}
329+
330+
fn apply_overrides(&self, options: &mut HashMap<String, String>) {
331+
for (key, value) in &self.overrides {
332+
match value {
333+
Some(value) => {
334+
options.insert(key.clone(), value.clone());
335+
}
336+
None => {
337+
options.remove(key);
338+
}
339+
}
340+
}
294341
}
295342
}
296343

@@ -450,6 +497,116 @@ impl AuthTableQueryResponse {
450497
mod tests {
451498
use super::*;
452499

500+
#[test]
501+
fn test_config_response_overrides_client_options() {
502+
let response: ConfigResponse = serde_json::from_str(
503+
r#"{
504+
"defaults": {
505+
"default-only": "default",
506+
"client-wins": "default",
507+
"override-wins": "default"
508+
},
509+
"overrides": {
510+
"override-only": "override",
511+
"override-wins": "override"
512+
}
513+
}"#,
514+
)
515+
.unwrap();
516+
let client_options = crate::common::Options::from_map(HashMap::from([
517+
("client-only".to_string(), "client".to_string()),
518+
("client-wins".to_string(), "client".to_string()),
519+
("override-wins".to_string(), "client".to_string()),
520+
]));
521+
522+
let merged = response.merge_options(&client_options);
523+
524+
assert_eq!(merged.get("default-only"), Some(&"default".to_string()));
525+
assert_eq!(merged.get("client-only"), Some(&"client".to_string()));
526+
assert_eq!(merged.get("client-wins"), Some(&"client".to_string()));
527+
assert_eq!(merged.get("override-only"), Some(&"override".to_string()));
528+
assert_eq!(merged.get("override-wins"), Some(&"override".to_string()));
529+
}
530+
531+
#[test]
532+
fn test_config_response_without_overrides_is_backward_compatible() {
533+
let response: ConfigResponse =
534+
serde_json::from_str(r#"{"defaults": {"warehouse": "s3://warehouse"}}"#).unwrap();
535+
536+
assert!(response.overrides.is_empty());
537+
assert_eq!(
538+
response.to_options().get("warehouse"),
539+
Some(&"s3://warehouse".to_string())
540+
);
541+
}
542+
543+
#[test]
544+
fn test_config_response_accepts_null_maps() {
545+
let response: ConfigResponse =
546+
serde_json::from_str(r#"{"defaults": null, "overrides": null}"#).unwrap();
547+
let client_options = crate::common::Options::from_map(HashMap::from([(
548+
"client-only".to_string(),
549+
"client".to_string(),
550+
)]));
551+
552+
let merged = response.merge_options(&client_options);
553+
554+
assert_eq!(merged.get("client-only"), Some(&"client".to_string()));
555+
}
556+
557+
#[test]
558+
fn test_config_response_filters_null_values_after_merge() {
559+
let response: ConfigResponse = serde_json::from_str(
560+
r#"{
561+
"defaults": {
562+
"default-null": null,
563+
"client-replaces-null": null,
564+
"override-removes": "default",
565+
"override-wins": "default"
566+
},
567+
"overrides": {
568+
"override-null-only": null,
569+
"override-removes": null,
570+
"override-wins": "override"
571+
}
572+
}"#,
573+
)
574+
.unwrap();
575+
let client_options = crate::common::Options::from_map(HashMap::from([
576+
("client-only".to_string(), "client".to_string()),
577+
("client-replaces-null".to_string(), "client".to_string()),
578+
("override-removes".to_string(), "client".to_string()),
579+
("override-wins".to_string(), "client".to_string()),
580+
]));
581+
582+
let merged = response.merge_options(&client_options);
583+
584+
assert!(!merged.contains("default-null"));
585+
assert_eq!(
586+
merged.get("client-replaces-null"),
587+
Some(&"client".to_string())
588+
);
589+
assert_eq!(merged.get("client-only"), Some(&"client".to_string()));
590+
assert!(!merged.contains("override-null-only"));
591+
assert!(!merged.contains("override-removes"));
592+
assert_eq!(merged.get("override-wins"), Some(&"override".to_string()));
593+
}
594+
595+
#[test]
596+
fn test_config_response_to_options_applies_overrides() {
597+
let response: ConfigResponse = serde_json::from_str(
598+
r#"{
599+
"defaults": {"data-token.enabled": "false"},
600+
"overrides": {"data-token.enabled": "true"}
601+
}"#,
602+
)
603+
.unwrap();
604+
605+
let options = response.to_options();
606+
607+
assert_eq!(options.get("data-token.enabled"), Some(&"true".to_string()));
608+
}
609+
453610
#[test]
454611
fn test_auth_table_query_response_deserialization() {
455612
// A restricted grant: pin the exact wire field names. A drift in either

0 commit comments

Comments
 (0)