Skip to content

Commit

Permalink
fix(rust): Trim trailing backslash in url storage options (delta-io#2656
Browse files Browse the repository at this point in the history
)

This trims trailing backslash if present in a storage option's
value if it's key ends with `_URL` (e.g. HOST_URL) or if the value
itself seems to be a url (i.e. if it contains `://`).

This also adds supporting test for this fix.
  • Loading branch information
omkar-foss committed Aug 14, 2024
1 parent c446b12 commit 99a346a
Showing 1 changed file with 54 additions and 1 deletion.
55 changes: 54 additions & 1 deletion crates/core/src/table/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,23 @@ impl DeltaTableBuilder {
/// - [S3 options](https://docs.rs/object_store/latest/object_store/aws/enum.AmazonS3ConfigKey.html#variants)
/// - [Google options](https://docs.rs/object_store/latest/object_store/gcp/enum.GoogleConfigKey.html#variants)
pub fn with_storage_options(mut self, storage_options: HashMap<String, String>) -> Self {
self.storage_options = Some(storage_options);
self.storage_options = Some(
storage_options
.clone()
.into_iter()
.map(
|(k, v)| {
let needs_trim = v.contains("://") ||
k.to_lowercase().ends_with("_url");
if needs_trim {
(k.to_owned(), v.trim_end_matches('/').to_owned())
} else {
(k, v)
}
}
)
.collect()
);
self
}

Expand Down Expand Up @@ -561,4 +577,41 @@ mod tests {
DeltaTableBuilder::from_valid_uri("this://is.nonsense")
.expect_err("this should be an error");
}

#[test]
fn test_writer_storage_opts_url_trim() {
let cases = [
// Trim Case 1 - Key indicating a url
("SOMETHING_URL", "something://else/", "something://else"),
// Trim Case 2 - Value https url ending with slash
("SOMETHING", "http://something:port/", "http://something:port"),
// Trim Case 3 - Value https url ending with slash
("SOMETHING", "https://something:port/", "https://something:port"),
// Trim Case 4 - JDBC MySQL url with slash
(
"SOME_JDBC_PREFIX",
"jdbc:mysql://mysql.db.server:3306/",
"jdbc:mysql://mysql.db.server:3306"),
// Trim Case 5 - S3A file system link
("SOME_S3_LINK", "s3a://bucket-name/", "s3a://bucket-name"),
// No Trim Case 6 - Not a url but ending with slash
("SOME_RANDOM_STRING", "a1b2c3d4e5f#/", "a1b2c3d4e5f#/"),
// No Trim Case 7 - Some value not a url
(
"SOME_VALUE",
"/ This is some value 123 /",
"/ This is some value 123 /"
),
];
for (key, val, expected) in cases {
let table_uri = Url::parse("memory:///test/tests/data/delta-0.8.0").unwrap();
let mut storage_opts = HashMap::<String, String>::new();
storage_opts.insert(key.to_owned(), val.to_owned());

let table = DeltaTableBuilder::from_uri(table_uri)
.with_storage_options(storage_opts);
let found_opts = table.storage_options();
assert_eq!(expected, found_opts.0.get(key).unwrap());
}
}
}

0 comments on commit 99a346a

Please sign in to comment.