mirror of
https://github.com/lancedb/lancedb.git
synced 2026-11-13 22:26:53 +00:00
feat!: accept only $ as the identifier delimiter
The delimiter setting comes from the Lance REST catalog standard, which carries it so other catalogs can adopt the identifier grammar. LanceDB services split on `$` and nothing else, so a configuration naming another value produced identifiers no service would take apart the way the caller meant. `ClientConfig::id_delimiter` stays, and is now refused unless it is `$`, reported where the configuration was written. Fixing the delimiter removes what the alternative needed: the `?delimiter=` query parameter, the per-client delimiter field, and the refusal of a component containing the delimiter -- `$` is outside the character set every object name admits, so no validated component can hold one. BREAKING CHANGE: a client configured with an `id_delimiter` other than `$` is now refused at construction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UfmeJ533rQDnPBkMtjerV6
This commit is contained in:
co-authored by
Claude Opus 5
parent
fa24bf322c
commit
5ba2acf730
@@ -93,6 +93,9 @@ pub struct ClientConfig {
|
||||
pub retry_config: Option<RetryConfig>,
|
||||
pub timeout_config: Option<TimeoutConfig>,
|
||||
pub extra_headers: Option<HashMap<String, String>>,
|
||||
/// The delimiter joining a namespace path and a name into one object
|
||||
/// identifier. `"$"` is the only supported value, and leaving this unset is
|
||||
/// how to get it; anything else is rejected when the connection is created.
|
||||
pub id_delimiter: Option<String>,
|
||||
pub tls_config: Option<TlsConfig>,
|
||||
/// User identifier for tracking purposes.
|
||||
|
||||
@@ -166,7 +166,10 @@ class ClientConfig:
|
||||
extra_headers: Optional[dict]
|
||||
Additional headers to include in requests.
|
||||
id_delimiter: Optional[str]
|
||||
The delimiter to use when constructing object identifiers.
|
||||
The delimiter joining a namespace path and a name into one object
|
||||
identifier. ``"$"`` is the only supported value, and leaving this
|
||||
unset is how to get it; anything else is rejected when the connection
|
||||
is created.
|
||||
tls_config: Optional[TlsConfig]
|
||||
TLS/mTLS configuration for secure connections.
|
||||
header_provider: Optional[HeaderProvider]
|
||||
|
||||
@@ -90,8 +90,9 @@ pub struct ClientConfig {
|
||||
pub user_agent: String,
|
||||
// TODO: how to configure request ids?
|
||||
pub extra_headers: HashMap<String, String>,
|
||||
/// The delimiter to use when constructing object identifiers.
|
||||
/// If not default, passes as query parameter.
|
||||
/// The delimiter joining a namespace path and a name into one object
|
||||
/// identifier. [`ID_DELIMITER`] is the only accepted value; any other is
|
||||
/// refused by [`ClientConfig::validate`].
|
||||
pub id_delimiter: Option<String>,
|
||||
/// TLS configuration for mTLS support
|
||||
pub tls_config: Option<TlsConfig>,
|
||||
@@ -307,7 +308,6 @@ pub struct RestfulLanceDbClient<S: HttpSend = Sender> {
|
||||
host: String,
|
||||
pub(crate) retry_config: ResolvedRetryConfig,
|
||||
pub(crate) sender: S,
|
||||
pub(crate) id_delimiter: String,
|
||||
pub(crate) header_provider: Option<Arc<dyn HeaderProvider>>,
|
||||
/// Connection-level read consistency interval. Drives the
|
||||
/// `x-lancedb-min-timestamp` freshness header sent on read requests.
|
||||
@@ -330,7 +330,6 @@ impl<S: HttpSend> std::fmt::Debug for RestfulLanceDbClient<S> {
|
||||
.field("host", &self.host)
|
||||
.field("retry_config", &self.retry_config)
|
||||
.field("sender", &self.sender)
|
||||
.field("id_delimiter", &self.id_delimiter)
|
||||
.field(
|
||||
"header_provider",
|
||||
&self.header_provider.as_ref().map(|_| "Some(...)"),
|
||||
@@ -453,39 +452,26 @@ enum BodyLogging {
|
||||
///
|
||||
/// An empty delimiter is refused for the same reason from the other side: it
|
||||
/// joins `["a", "b"]` into `ab`, which no split recovers.
|
||||
fn validate_id_delimiter(delimiter: String) -> Result<String> {
|
||||
if delimiter.is_empty() {
|
||||
return Err(Error::InvalidInput {
|
||||
message: "id_delimiter must not be empty: an identifier joined by nothing cannot be \
|
||||
split back into a namespace path and a name"
|
||||
.to_string(),
|
||||
});
|
||||
}
|
||||
// The root namespace is addressed by the bare delimiter, so a delimiter that
|
||||
// is itself a relative segment makes the root listing route disappear:
|
||||
// `/v1/namespace/./secret/list` resolves to `/v1/namespace/secret/list`.
|
||||
if delimiter == "." || delimiter == ".." {
|
||||
/// The delimiter joining a namespace path and a name into the `{id}` a route
|
||||
/// addresses, and the only one a LanceDB service splits on.
|
||||
///
|
||||
/// The identifier grammar comes from the Lance REST catalog standard, which
|
||||
/// carries a delimiter setting so other catalogs can adopt it; LanceDB itself
|
||||
/// has one value. Fixing it here is what lets an identifier be joined and split
|
||||
/// back apart without further checks: `$` is outside the character set every
|
||||
/// object name admits, so no name can contain one.
|
||||
pub(crate) const ID_DELIMITER: &str = "$";
|
||||
|
||||
fn validate_id_delimiter(delimiter: &str) -> Result<()> {
|
||||
if delimiter != ID_DELIMITER {
|
||||
return Err(Error::InvalidInput {
|
||||
message: format!(
|
||||
"id_delimiter '{delimiter}' is a relative path segment: the root namespace is \
|
||||
addressed by the delimiter alone, so a request for it would resolve to a \
|
||||
different route"
|
||||
"id_delimiter '{delimiter}' is not supported: '{ID_DELIMITER}' is the only \
|
||||
delimiter LanceDB services split an identifier on"
|
||||
),
|
||||
});
|
||||
}
|
||||
if let Some(unsafe_character) = delimiter
|
||||
.chars()
|
||||
.find(|character| matches!(character, '/' | '?' | '#' | '%' | '\\'))
|
||||
{
|
||||
return Err(Error::InvalidInput {
|
||||
message: format!(
|
||||
"id_delimiter '{delimiter}' contains {unsafe_character:?}, which is URL path \
|
||||
syntax: a delimiter is written into the path raw, so one that divides or ends \
|
||||
the path changes which route a request reaches"
|
||||
),
|
||||
});
|
||||
}
|
||||
Ok(delimiter)
|
||||
Ok(())
|
||||
}
|
||||
|
||||
impl ClientConfig {
|
||||
@@ -496,7 +482,9 @@ impl ClientConfig {
|
||||
/// response later. Public so a caller assembling a config can ask the same
|
||||
/// question without connecting.
|
||||
pub fn validate(&self) -> Result<()> {
|
||||
validate_id_delimiter(self.id_delimiter.clone().unwrap_or_else(|| "$".to_string()))?;
|
||||
if let Some(delimiter) = &self.id_delimiter {
|
||||
validate_id_delimiter(delimiter)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
@@ -629,10 +617,6 @@ impl RestfulLanceDbClient<Sender> {
|
||||
host,
|
||||
retry_config,
|
||||
sender: Sender,
|
||||
id_delimiter: client_config
|
||||
.id_delimiter
|
||||
.clone()
|
||||
.unwrap_or("$".to_string()),
|
||||
header_provider: client_config.header_provider,
|
||||
read_consistency_interval,
|
||||
max_bytes_per_request,
|
||||
@@ -790,22 +774,12 @@ impl<S: HttpSend> RestfulLanceDbClient<S> {
|
||||
|
||||
pub fn get(&self, uri: &str) -> RequestBuilder {
|
||||
let full_uri = format!("{}{}", self.host, uri);
|
||||
let builder = self.client.get(full_uri);
|
||||
self.add_id_delimiter_query_param(builder)
|
||||
self.client.get(full_uri)
|
||||
}
|
||||
|
||||
pub fn post(&self, uri: &str) -> RequestBuilder {
|
||||
let full_uri = format!("{}{}", self.host, uri);
|
||||
let builder = self.client.post(full_uri);
|
||||
self.add_id_delimiter_query_param(builder)
|
||||
}
|
||||
|
||||
fn add_id_delimiter_query_param(&self, req: RequestBuilder) -> RequestBuilder {
|
||||
if self.id_delimiter != "$" {
|
||||
req.query(&[("delimiter", self.id_delimiter.clone())])
|
||||
} else {
|
||||
req
|
||||
}
|
||||
self.client.post(full_uri)
|
||||
}
|
||||
|
||||
/// Apply dynamic headers from the header provider if configured
|
||||
@@ -1148,7 +1122,6 @@ pub mod test_utils {
|
||||
sender: MockSender {
|
||||
f: Arc::new(wrapper),
|
||||
},
|
||||
id_delimiter: "$".to_string(),
|
||||
header_provider: None,
|
||||
read_consistency_interval,
|
||||
max_bytes_per_request: None,
|
||||
@@ -1175,7 +1148,6 @@ pub mod test_utils {
|
||||
sender: MockSender {
|
||||
f: Arc::new(wrapper),
|
||||
},
|
||||
id_delimiter: config.id_delimiter.unwrap_or_else(|| "$".to_string()),
|
||||
header_provider: config.header_provider,
|
||||
read_consistency_interval: None,
|
||||
max_bytes_per_request: config
|
||||
@@ -1190,27 +1162,38 @@ pub mod test_utils {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
/// A delimiter is written into the path raw, so one that is path syntax
|
||||
/// changes the shape of a route rather than its content. Refusing a
|
||||
/// component that contains the delimiter cannot catch this: neither
|
||||
/// component contains one.
|
||||
/// The identifier grammar has one delimiter. A configuration naming any
|
||||
/// other is refused where it was written, rather than producing identifiers
|
||||
/// no service would split the way the caller meant.
|
||||
#[test]
|
||||
fn test_a_delimiter_that_is_path_syntax_is_refused() {
|
||||
for delimiter in ["/", "?", "#", "%", "a/b", "", ".", "..", "/../../function/"] {
|
||||
let error = super::validate_id_delimiter(delimiter.to_string())
|
||||
.expect_err("a delimiter that is path syntax must be refused");
|
||||
fn test_a_delimiter_other_than_the_supported_one_is_refused() {
|
||||
for delimiter in ["/", "?", "#", "%", "", ".", "..", "-", "_", "|", "::", "$$"] {
|
||||
let error = super::validate_id_delimiter(delimiter)
|
||||
.expect_err("only the supported delimiter may be configured");
|
||||
assert!(
|
||||
error.to_string().contains("id_delimiter"),
|
||||
"{delimiter:?}: {error}"
|
||||
);
|
||||
}
|
||||
// The default, and other separators that are not path syntax, stand.
|
||||
// A period is fine inside a longer delimiter -- only the exact relative
|
||||
// segments are a problem.
|
||||
for delimiter in ["$", "-", "_", "|", "::", ".x.", "..."] {
|
||||
super::validate_id_delimiter(delimiter.to_string())
|
||||
.unwrap_or_else(|error| panic!("{delimiter:?} must be accepted: {error}"));
|
||||
super::validate_id_delimiter(super::ID_DELIMITER).unwrap();
|
||||
}
|
||||
|
||||
/// Leaving it unset is how nearly every caller reaches the same delimiter.
|
||||
#[test]
|
||||
fn test_an_unset_delimiter_is_the_supported_one() {
|
||||
super::ClientConfig::default().validate().unwrap();
|
||||
super::ClientConfig {
|
||||
id_delimiter: Some(super::ID_DELIMITER.to_string()),
|
||||
..Default::default()
|
||||
}
|
||||
.validate()
|
||||
.unwrap();
|
||||
super::ClientConfig {
|
||||
id_delimiter: Some("-".to_string()),
|
||||
..Default::default()
|
||||
}
|
||||
.validate()
|
||||
.expect_err("a configured delimiter other than the supported one must be refused");
|
||||
}
|
||||
|
||||
use super::*;
|
||||
@@ -1513,7 +1496,6 @@ mod tests {
|
||||
host: "https://example.com".to_string(),
|
||||
retry_config: RetryConfig::default().try_into().unwrap(),
|
||||
sender: Sender,
|
||||
id_delimiter: "+".to_string(),
|
||||
header_provider: Some(Arc::new(provider) as Arc<dyn HeaderProvider>),
|
||||
read_consistency_interval: None,
|
||||
max_bytes_per_request: None,
|
||||
@@ -1552,7 +1534,6 @@ mod tests {
|
||||
host: "https://example.com".to_string(),
|
||||
retry_config: RetryConfig::default().try_into().unwrap(),
|
||||
sender: Sender,
|
||||
id_delimiter: "+".to_string(),
|
||||
header_provider: Some(Arc::new(provider) as Arc<dyn HeaderProvider>),
|
||||
read_consistency_interval: None,
|
||||
max_bytes_per_request: None,
|
||||
@@ -1617,7 +1598,6 @@ mod tests {
|
||||
host: "https://example.com".to_string(),
|
||||
retry_config: RetryConfig::default().try_into().unwrap(),
|
||||
sender: Sender,
|
||||
id_delimiter: "+".to_string(),
|
||||
header_provider: Some(Arc::new(provider) as Arc<dyn HeaderProvider>),
|
||||
read_consistency_interval: None,
|
||||
max_bytes_per_request: None,
|
||||
|
||||
+41
-165
@@ -37,7 +37,8 @@ use crate::table::BaseTable;
|
||||
use crate::utils::{reject_relative_segment, validate_namespace_name, validate_table_name};
|
||||
|
||||
use super::client::{
|
||||
ClientConfig, HeaderProvider, HttpSend, RequestResultExt, RestfulLanceDbClient, Sender,
|
||||
ClientConfig, HeaderProvider, HttpSend, ID_DELIMITER, RequestResultExt, RestfulLanceDbClient,
|
||||
Sender,
|
||||
};
|
||||
use super::sql::SqlClient;
|
||||
use super::table::RemoteTable;
|
||||
@@ -381,7 +382,7 @@ impl<S: HttpSend> RemoteDatabase<S> {
|
||||
name: &str,
|
||||
namespace_path: &[String],
|
||||
) -> Result<(String, Response)> {
|
||||
let identifier = build_table_identifier(name, namespace_path, &self.client.id_delimiter)?;
|
||||
let identifier = build_table_identifier(name, namespace_path)?;
|
||||
let cache_key = build_cache_key(name, namespace_path);
|
||||
let req = self.client.post(&format!("/v1/table/{}/drop/", identifier));
|
||||
let (request_id, resp) = self.client.send(req).await?;
|
||||
@@ -405,8 +406,7 @@ impl<S: HttpSend> RemoteDatabase<S> {
|
||||
&self,
|
||||
request: &TableNamesRequest,
|
||||
) -> Result<(Vec<String>, ServerVersion)> {
|
||||
let namespace_id =
|
||||
build_namespace_identifier(&request.namespace_path, &self.client.id_delimiter)?;
|
||||
let namespace_id = build_namespace_identifier(&request.namespace_path)?;
|
||||
let path = format!("/v1/namespace/{}/table/list", namespace_id);
|
||||
|
||||
let mut names = Vec::new();
|
||||
@@ -514,16 +514,13 @@ impl From<&CreateTableMode> for &'static str {
|
||||
}
|
||||
|
||||
/// The path segment addressing one table: its namespace path and name.
|
||||
fn build_table_identifier(name: &str, namespace: &[String], delimiter: &str) -> Result<String> {
|
||||
fn build_table_identifier(name: &str, namespace: &[String]) -> Result<String> {
|
||||
for segment in namespace {
|
||||
validate_namespace_name(segment)?;
|
||||
reject_delimiter(segment, delimiter, "namespace segment")?;
|
||||
}
|
||||
validate_table_name(name)?;
|
||||
reject_delimiter(name, delimiter, "table name")?;
|
||||
Ok(join_identifier(
|
||||
namespace.iter().map(String::as_str).chain([name]),
|
||||
delimiter,
|
||||
))
|
||||
}
|
||||
|
||||
@@ -534,33 +531,7 @@ fn build_table_identifier(name: &str, namespace: &[String], delimiter: &str) ->
|
||||
/// untouched and is then resolved away.
|
||||
fn build_function_identifier(name: &str) -> Result<String> {
|
||||
reject_relative_segment("Function name", name)?;
|
||||
Ok(join_identifier([name].into_iter(), ""))
|
||||
}
|
||||
|
||||
/// Refuse a component holding the delimiter that is about to separate it from
|
||||
/// the others.
|
||||
///
|
||||
/// The join is what decides identity, and it is only reversible while no
|
||||
/// component contains the separator. The default `$` is outside every name
|
||||
/// charset here, so this never fires for it. A *configured* delimiter may be
|
||||
/// any string, and the readable choices -- `.`, `-`, `_` -- are all inside
|
||||
/// those charsets: under `id_delimiter="."` the namespace `["prod.vision"]`
|
||||
/// and the namespace `["prod", "vision"]` join to one identifier, and a caller
|
||||
/// addressing the first would reach the object belonging to the second.
|
||||
///
|
||||
/// There is no escaping of the joined form that recovers a boundary the
|
||||
/// delimiter has already erased, so this refuses rather than encodes.
|
||||
fn reject_delimiter(component: &str, delimiter: &str, what: &str) -> Result<()> {
|
||||
if !delimiter.is_empty() && component.contains(delimiter) {
|
||||
return Err(Error::InvalidInput {
|
||||
message: format!(
|
||||
"{what} '{component}' contains the configured identifier delimiter \
|
||||
'{delimiter}', so the namespace path and the name it joins could not be told \
|
||||
apart"
|
||||
),
|
||||
});
|
||||
}
|
||||
Ok(())
|
||||
Ok(join_identifier([name].into_iter()))
|
||||
}
|
||||
|
||||
/// The path segment addressing one Secret: its namespace path and name.
|
||||
@@ -570,15 +541,8 @@ fn reject_delimiter(component: &str, delimiter: &str, what: &str) -> Result<()>
|
||||
/// point.
|
||||
///
|
||||
/// [`Connection`]: crate::connection::Connection
|
||||
fn build_secret_identifier(name: &str, namespace: &[String], delimiter: &str) -> Result<String> {
|
||||
for segment in namespace {
|
||||
reject_delimiter(segment, delimiter, "Secret namespace path segment")?;
|
||||
}
|
||||
reject_delimiter(name, delimiter, "Secret name")?;
|
||||
Ok(join_identifier(
|
||||
namespace.iter().map(String::as_str).chain([name]),
|
||||
delimiter,
|
||||
))
|
||||
fn build_secret_identifier(name: &str, namespace: &[String]) -> String {
|
||||
join_identifier(namespace.iter().map(String::as_str).chain([name]))
|
||||
}
|
||||
|
||||
/// Join validated components into the `{id}` a route addresses: each
|
||||
@@ -594,43 +558,33 @@ fn build_secret_identifier(name: &str, namespace: &[String], delimiter: &str) ->
|
||||
/// URL's shape. It does not cover `.` and `..`, which are unreserved too and
|
||||
/// are resolved after decoding -- those are refused outright, by the validator
|
||||
/// each object has.
|
||||
fn join_identifier<'a>(components: impl Iterator<Item = &'a str>, delimiter: &str) -> String {
|
||||
fn join_identifier<'a>(components: impl Iterator<Item = &'a str>) -> String {
|
||||
components
|
||||
.map(|component| urlencoding::encode(component).into_owned())
|
||||
.collect::<Vec<_>>()
|
||||
.join(delimiter)
|
||||
.join(ID_DELIMITER)
|
||||
}
|
||||
|
||||
/// The namespace a Secret listing is scoped to.
|
||||
///
|
||||
/// The root is the bare delimiter, as it is for every other object's listing.
|
||||
fn build_secret_namespace_identifier(namespace: &[String], delimiter: &str) -> Result<String> {
|
||||
for segment in namespace {
|
||||
reject_delimiter(segment, delimiter, "Secret namespace path segment")?;
|
||||
}
|
||||
fn build_secret_namespace_identifier(namespace: &[String]) -> String {
|
||||
if namespace.is_empty() {
|
||||
return Ok(delimiter.to_string());
|
||||
return ID_DELIMITER.to_string();
|
||||
}
|
||||
Ok(join_identifier(
|
||||
namespace.iter().map(String::as_str),
|
||||
delimiter,
|
||||
))
|
||||
join_identifier(namespace.iter().map(String::as_str))
|
||||
}
|
||||
|
||||
/// The path segment addressing one namespace.
|
||||
fn build_namespace_identifier(namespace: &[String], delimiter: &str) -> Result<String> {
|
||||
fn build_namespace_identifier(namespace: &[String]) -> Result<String> {
|
||||
for segment in namespace {
|
||||
validate_namespace_name(segment)?;
|
||||
reject_delimiter(segment, delimiter, "namespace segment")?;
|
||||
}
|
||||
if namespace.is_empty() {
|
||||
// According to the namespace spec, use delimiter to represent root namespace
|
||||
return Ok(delimiter.to_string());
|
||||
return Ok(ID_DELIMITER.to_string());
|
||||
}
|
||||
Ok(join_identifier(
|
||||
namespace.iter().map(String::as_str),
|
||||
delimiter,
|
||||
))
|
||||
Ok(join_identifier(namespace.iter().map(String::as_str)))
|
||||
}
|
||||
|
||||
/// Build a secure cache key using length prefixes.
|
||||
@@ -767,11 +721,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
&self,
|
||||
request: CreateMaterializedViewRequest,
|
||||
) -> Result<Job> {
|
||||
let identifier = build_table_identifier(
|
||||
&request.name,
|
||||
&request.namespace_path,
|
||||
&self.client.id_delimiter,
|
||||
)?;
|
||||
let identifier = build_table_identifier(&request.name, &request.namespace_path)?;
|
||||
let req = self
|
||||
.client
|
||||
.post(&format!("/v1/materialized_view/{identifier}/create"))
|
||||
@@ -814,7 +764,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
name: &str,
|
||||
namespace_path: &[String],
|
||||
) -> Result<Job> {
|
||||
let identifier = build_table_identifier(name, namespace_path, &self.client.id_delimiter)?;
|
||||
let identifier = build_table_identifier(name, namespace_path)?;
|
||||
let request = self
|
||||
.client
|
||||
.post(&format!("/v1/materialized_view/{identifier}/drop"));
|
||||
@@ -852,7 +802,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
page_token: Option<String>,
|
||||
}
|
||||
|
||||
let namespace_id = build_namespace_identifier(namespace_path, &self.client.id_delimiter)?;
|
||||
let namespace_id = build_namespace_identifier(namespace_path)?;
|
||||
let path = format!("/v1/namespace/{namespace_id}/materialized_view/list");
|
||||
let mut views = Vec::new();
|
||||
let mut page_token: Option<String> = None;
|
||||
@@ -927,7 +877,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
}
|
||||
|
||||
async fn list_functions(&self) -> Result<Vec<FunctionVersion>> {
|
||||
let namespace_id = build_namespace_identifier(&[], &self.client.id_delimiter)?;
|
||||
let namespace_id = build_namespace_identifier(&[])?;
|
||||
let path = format!("/v1/namespace/{namespace_id}/function/list");
|
||||
let mut functions = Vec::new();
|
||||
let mut page_token: Option<String> = None;
|
||||
@@ -987,7 +937,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
value: &str,
|
||||
namespace_path: &[String],
|
||||
) -> Result<()> {
|
||||
let secret_id = build_secret_identifier(name, namespace_path, &self.client.id_delimiter)?;
|
||||
let secret_id = build_secret_identifier(name, namespace_path);
|
||||
self.post_secret_write(
|
||||
&format!("/v1/secret/{secret_id}/create"),
|
||||
&RemoteCreateSecretRequest { value },
|
||||
@@ -996,7 +946,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
}
|
||||
|
||||
async fn alter_secret(&self, name: &str, value: &str, namespace_path: &[String]) -> Result<()> {
|
||||
let secret_id = build_secret_identifier(name, namespace_path, &self.client.id_delimiter)?;
|
||||
let secret_id = build_secret_identifier(name, namespace_path);
|
||||
self.post_secret_write(
|
||||
&format!("/v1/secret/{secret_id}/alter"),
|
||||
&RemoteAlterSecretRequest { value },
|
||||
@@ -1005,8 +955,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
}
|
||||
|
||||
async fn list_secrets(&self, namespace_path: &[String]) -> Result<Vec<String>> {
|
||||
let namespace_id =
|
||||
build_secret_namespace_identifier(namespace_path, &self.client.id_delimiter)?;
|
||||
let namespace_id = build_secret_namespace_identifier(namespace_path);
|
||||
let path = format!("/v1/namespace/{namespace_id}/secret/list");
|
||||
let mut names = Vec::new();
|
||||
let mut page_token: Option<String> = None;
|
||||
@@ -1039,7 +988,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
}
|
||||
|
||||
async fn drop_secret(&self, name: &str, namespace_path: &[String]) -> Result<()> {
|
||||
let secret_id = build_secret_identifier(name, namespace_path, &self.client.id_delimiter)?;
|
||||
let secret_id = build_secret_identifier(name, namespace_path);
|
||||
let req = self.client.post(&format!("/v1/secret/{secret_id}/drop"));
|
||||
let (request_id, response) = self.client.send(req).await?;
|
||||
self.client.check_response(&request_id, response).await?;
|
||||
@@ -1047,7 +996,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
}
|
||||
|
||||
async fn describe_secret(&self, name: &str, namespace_path: &[String]) -> Result<SecretInfo> {
|
||||
let secret_id = build_secret_identifier(name, namespace_path, &self.client.id_delimiter)?;
|
||||
let secret_id = build_secret_identifier(name, namespace_path);
|
||||
let req = self
|
||||
.client
|
||||
.post(&format!("/v1/secret/{secret_id}/describe"));
|
||||
@@ -1178,8 +1127,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
};
|
||||
|
||||
for table in &tables {
|
||||
let table_identifier =
|
||||
build_table_identifier(table, &request.namespace_path, &self.client.id_delimiter)?;
|
||||
let table_identifier = build_table_identifier(table, &request.namespace_path)?;
|
||||
let cache_key = build_cache_key(table, &request.namespace_path);
|
||||
let remote_table = Arc::new(RemoteTable::new(
|
||||
self.client.clone(),
|
||||
@@ -1195,7 +1143,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
|
||||
async fn list_tables(&self, request: ListTablesRequest) -> Result<ListTablesResponse> {
|
||||
let namespace_parts = request.id.as_deref().unwrap_or(&[]);
|
||||
let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter)?;
|
||||
let namespace_id = build_namespace_identifier(namespace_parts)?;
|
||||
let mut req = self
|
||||
.client
|
||||
.get(&format!("/v1/namespace/{}/table/list", namespace_id));
|
||||
@@ -1215,8 +1163,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
// Cache the tables for future use
|
||||
let namespace_vec = namespace_parts.to_vec();
|
||||
for table in &response.tables {
|
||||
let table_identifier =
|
||||
build_table_identifier(table, &namespace_vec, &self.client.id_delimiter)?;
|
||||
let table_identifier = build_table_identifier(table, &namespace_vec)?;
|
||||
let cache_key = build_cache_key(table, &namespace_vec);
|
||||
let remote_table = Arc::new(RemoteTable::new(
|
||||
self.client.clone(),
|
||||
@@ -1234,11 +1181,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
async fn create_table(&self, mut request: CreateTableRequest) -> Result<Arc<dyn BaseTable>> {
|
||||
let body = stream_as_body(request.data.scan_as_stream())?;
|
||||
|
||||
let identifier = build_table_identifier(
|
||||
&request.name,
|
||||
&request.namespace_path,
|
||||
&self.client.id_delimiter,
|
||||
)?;
|
||||
let identifier = build_table_identifier(&request.name, &request.namespace_path)?;
|
||||
let req = self
|
||||
.client
|
||||
.post(&format!("/v1/table/{}/create/", identifier))
|
||||
@@ -1290,11 +1233,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
}
|
||||
let rsp = self.client.check_response(&request_id, rsp).await?;
|
||||
let version = parse_server_version(&request_id, &rsp)?;
|
||||
let table_identifier = build_table_identifier(
|
||||
&request.name,
|
||||
&request.namespace_path,
|
||||
&self.client.id_delimiter,
|
||||
)?;
|
||||
let table_identifier = build_table_identifier(&request.name, &request.namespace_path)?;
|
||||
let cache_key = build_cache_key(&request.name, &request.namespace_path);
|
||||
let table = Arc::new(RemoteTable::new(
|
||||
self.client.clone(),
|
||||
@@ -1309,11 +1248,8 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
}
|
||||
|
||||
async fn clone_table(&self, request: CloneTableRequest) -> Result<Arc<dyn BaseTable>> {
|
||||
let table_identifier = build_table_identifier(
|
||||
&request.target_table_name,
|
||||
&request.target_namespace_path,
|
||||
&self.client.id_delimiter,
|
||||
)?;
|
||||
let table_identifier =
|
||||
build_table_identifier(&request.target_table_name, &request.target_namespace_path)?;
|
||||
|
||||
let remote_request = RemoteCloneTableRequest {
|
||||
source_location: request.source_uri,
|
||||
@@ -1354,11 +1290,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
}
|
||||
|
||||
async fn open_table(&self, request: OpenTableRequest) -> Result<Arc<dyn BaseTable>> {
|
||||
let identifier = build_table_identifier(
|
||||
&request.name,
|
||||
&request.namespace_path,
|
||||
&self.client.id_delimiter,
|
||||
)?;
|
||||
let identifier = build_table_identifier(&request.name, &request.namespace_path)?;
|
||||
let cache_key = build_cache_key(&request.name, &request.namespace_path);
|
||||
|
||||
// We describe the table to confirm it exists before moving on.
|
||||
@@ -1374,11 +1306,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
let rsp = self.client.check_response(&request_id, rsp).await?;
|
||||
let version = parse_server_version(&request_id, &rsp)?;
|
||||
let describe_body = rsp.text().await.ok();
|
||||
let table_identifier = build_table_identifier(
|
||||
&request.name,
|
||||
&request.namespace_path,
|
||||
&self.client.id_delimiter,
|
||||
)?;
|
||||
let table_identifier = build_table_identifier(&request.name, &request.namespace_path)?;
|
||||
let table = Arc::new(RemoteTable::new(
|
||||
self.client.clone(),
|
||||
request.name.clone(),
|
||||
@@ -1405,8 +1333,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
cur_namespace_path: &[String],
|
||||
new_namespace_path: &[String],
|
||||
) -> Result<()> {
|
||||
let current_identifier =
|
||||
build_table_identifier(current_name, cur_namespace_path, &self.client.id_delimiter)?;
|
||||
let current_identifier = build_table_identifier(current_name, cur_namespace_path)?;
|
||||
let current_cache_key = build_cache_key(current_name, cur_namespace_path);
|
||||
let new_cache_key = build_cache_key(new_name, new_namespace_path);
|
||||
|
||||
@@ -1470,7 +1397,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
request: ListNamespacesRequest,
|
||||
) -> Result<ListNamespacesResponse> {
|
||||
let namespace_parts = request.id.as_deref().unwrap_or(&[]);
|
||||
let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter)?;
|
||||
let namespace_id = build_namespace_identifier(namespace_parts)?;
|
||||
let mut req = self
|
||||
.client
|
||||
.get(&format!("/v1/namespace/{}/list", namespace_id));
|
||||
@@ -1492,7 +1419,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
request: CreateNamespaceRequest,
|
||||
) -> Result<CreateNamespaceResponse> {
|
||||
let namespace_parts = request.id.as_deref().unwrap_or(&[]);
|
||||
let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter)?;
|
||||
let namespace_id = build_namespace_identifier(namespace_parts)?;
|
||||
let mut req = self
|
||||
.client
|
||||
.post(&format!("/v1/namespace/{}/create", namespace_id));
|
||||
@@ -1520,7 +1447,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
|
||||
async fn drop_namespace(&self, request: DropNamespaceRequest) -> Result<DropNamespaceResponse> {
|
||||
let namespace_parts = request.id.as_deref().unwrap_or(&[]);
|
||||
let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter)?;
|
||||
let namespace_id = build_namespace_identifier(namespace_parts)?;
|
||||
let mut req = self
|
||||
.client
|
||||
.post(&format!("/v1/namespace/{}/drop", namespace_id));
|
||||
@@ -1551,7 +1478,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
request: DescribeNamespaceRequest,
|
||||
) -> Result<DescribeNamespaceResponse> {
|
||||
let namespace_parts = request.id.as_deref().unwrap_or(&[]);
|
||||
let namespace_id = build_namespace_identifier(namespace_parts, &self.client.id_delimiter)?;
|
||||
let namespace_id = build_namespace_identifier(namespace_parts)?;
|
||||
let req = self
|
||||
.client
|
||||
.post(&format!("/v1/namespace/{}/describe", namespace_id))
|
||||
@@ -1570,7 +1497,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
async fn namespace_client(&self) -> Result<Arc<dyn lance_namespace::LanceNamespace>> {
|
||||
// Create a RestNamespace pointing to the same remote host with the same authentication headers
|
||||
let mut builder = lance_namespace_impls::RestNamespaceBuilder::new(self.client.host())
|
||||
.delimiter(&self.client.id_delimiter)
|
||||
.delimiter(ID_DELIMITER)
|
||||
.headers(self.namespace_headers.clone());
|
||||
|
||||
if let Some(context_provider) = &self.namespace_context_provider {
|
||||
@@ -1606,7 +1533,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
|
||||
let mut properties = HashMap::new();
|
||||
properties.insert("uri".to_string(), self.client.host().to_string());
|
||||
properties.insert("delimiter".to_string(), self.client.id_delimiter.clone());
|
||||
properties.insert("delimiter".to_string(), ID_DELIMITER.to_string());
|
||||
for (key, value) in &self.namespace_headers {
|
||||
properties.insert(format!("header.{}", key), value.clone());
|
||||
}
|
||||
@@ -3507,57 +3434,6 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_a_configured_delimiter_is_refused_inside_a_component() {
|
||||
use std::sync::{Arc, Mutex};
|
||||
let reached = Arc::new(Mutex::new(false));
|
||||
let flag = reached.clone();
|
||||
let conn = Connection::new_with_handler_and_config(
|
||||
move |_| {
|
||||
*flag.lock().unwrap() = true;
|
||||
http::Response::builder().status(200).body("{}").unwrap()
|
||||
},
|
||||
ClientConfig {
|
||||
id_delimiter: Some(".".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
let error = conn
|
||||
.drop_secret("openai", &["prod.vision".to_string()])
|
||||
.await
|
||||
.expect_err("a component holding the delimiter must be refused");
|
||||
assert!(error.to_string().contains("delimiter"), "{error}");
|
||||
assert!(!*reached.lock().unwrap(), "the request was sent anyway");
|
||||
|
||||
// The same two names as separate segments are the ordinary case.
|
||||
conn.drop_secret("openai", &["prod".to_string(), "vision".to_string()])
|
||||
.await
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_a_configured_delimiter_is_refused_inside_a_table_component() {
|
||||
use std::sync::{Arc, Mutex};
|
||||
let reached = Arc::new(Mutex::new(false));
|
||||
let flag = reached.clone();
|
||||
let conn = Connection::new_with_handler_and_config(
|
||||
move |_| {
|
||||
*flag.lock().unwrap() = true;
|
||||
http::Response::builder().status(200).body("{}").unwrap()
|
||||
},
|
||||
ClientConfig {
|
||||
id_delimiter: Some(".".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
let error = conn
|
||||
.drop_table("t", &["prod.vision".to_string()])
|
||||
.await
|
||||
.expect_err("a segment holding the delimiter must be refused");
|
||||
assert!(error.to_string().contains("delimiter"), "{error}");
|
||||
assert!(!*reached.lock().unwrap(), "the request was sent anyway");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_create_and_alter_secret_send_the_value_in_the_request_body() {
|
||||
for (route, call) in [
|
||||
|
||||
Reference in New Issue
Block a user