Don't use Deserr for ExternalDocumentId, instead convert to error afterward

This commit is contained in:
Louis Dureuil 2025-03-03 16:51:34 +01:00
parent 17bf82235d
commit 31ef7adef5
No known key found for this signature in database
2 changed files with 18 additions and 18 deletions

View File

@ -5,7 +5,7 @@ use index_scheduler::IndexScheduler;
use meilisearch_types::deserr::query_params::Param; use meilisearch_types::deserr::query_params::Param;
use meilisearch_types::deserr::{DeserrJsonError, DeserrQueryParamError}; use meilisearch_types::deserr::{DeserrJsonError, DeserrQueryParamError};
use meilisearch_types::error::deserr_codes::*; use meilisearch_types::error::deserr_codes::*;
use meilisearch_types::error::{ErrorCode as _, ResponseError}; use meilisearch_types::error::ResponseError;
use meilisearch_types::index_uid::IndexUid; use meilisearch_types::index_uid::IndexUid;
use meilisearch_types::keys::actions; use meilisearch_types::keys::actions;
use meilisearch_types::serde_cs::vec::CS; use meilisearch_types::serde_cs::vec::CS;
@ -111,7 +111,7 @@ pub async fn similar_get(
) -> Result<HttpResponse, ResponseError> { ) -> Result<HttpResponse, ResponseError> {
let index_uid = IndexUid::try_from(index_uid.into_inner())?; let index_uid = IndexUid::try_from(index_uid.into_inner())?;
let query = params.0.try_into()?; let query = params.0.into();
let mut aggregate = SimilarAggregator::<SimilarGET>::from_query(&query); let mut aggregate = SimilarAggregator::<SimilarGET>::from_query(&query);
@ -295,10 +295,8 @@ impl std::convert::TryFrom<String> for RankingScoreThresholdGet {
} }
} }
impl TryFrom<SimilarQueryGet> for SimilarQuery { impl From<SimilarQueryGet> for SimilarQuery {
type Error = ResponseError; fn from(
fn try_from(
SimilarQueryGet { SimilarQueryGet {
id, id,
offset, offset,
@ -311,7 +309,7 @@ impl TryFrom<SimilarQueryGet> for SimilarQuery {
embedder, embedder,
ranking_score_threshold, ranking_score_threshold,
}: SimilarQueryGet, }: SimilarQueryGet,
) -> Result<Self, Self::Error> { ) -> Self {
let filter = match filter { let filter = match filter {
Some(f) => match serde_json::from_str(&f) { Some(f) => match serde_json::from_str(&f) {
Ok(v) => Some(v), Ok(v) => Some(v),
@ -320,10 +318,8 @@ impl TryFrom<SimilarQueryGet> for SimilarQuery {
None => None, None => None,
}; };
Ok(SimilarQuery { SimilarQuery {
id: id.0.try_into().map_err(|code: InvalidSimilarId| { id: serde_json::Value::String(id.0),
ResponseError::from_msg(code.to_string(), code.error_code())
})?,
offset: offset.0, offset: offset.0,
limit: limit.0, limit: limit.0,
filter, filter,
@ -333,6 +329,6 @@ impl TryFrom<SimilarQueryGet> for SimilarQuery {
show_ranking_score: show_ranking_score.0, show_ranking_score: show_ranking_score.0,
show_ranking_score_details: show_ranking_score_details.0, show_ranking_score_details: show_ranking_score_details.0,
ranking_score_threshold: ranking_score_threshold.map(|x| x.0), ranking_score_threshold: ranking_score_threshold.map(|x| x.0),
}) }
} }
} }

View File

@ -635,7 +635,7 @@ impl SearchQueryWithIndex {
pub struct SimilarQuery { pub struct SimilarQuery {
#[deserr(error = DeserrJsonError<InvalidSimilarId>)] #[deserr(error = DeserrJsonError<InvalidSimilarId>)]
#[schema(value_type = String)] #[schema(value_type = String)]
pub id: ExternalDocumentId, pub id: serde_json::Value,
#[deserr(default = DEFAULT_SEARCH_OFFSET(), error = DeserrJsonError<InvalidSimilarOffset>)] #[deserr(default = DEFAULT_SEARCH_OFFSET(), error = DeserrJsonError<InvalidSimilarOffset>)]
pub offset: usize, pub offset: usize,
#[deserr(default = DEFAULT_SEARCH_LIMIT(), error = DeserrJsonError<InvalidSimilarLimit>)] #[deserr(default = DEFAULT_SEARCH_LIMIT(), error = DeserrJsonError<InvalidSimilarLimit>)]
@ -657,8 +657,7 @@ pub struct SimilarQuery {
pub ranking_score_threshold: Option<RankingScoreThresholdSimilar>, pub ranking_score_threshold: Option<RankingScoreThresholdSimilar>,
} }
#[derive(Debug, Clone, PartialEq, Deserr)] #[derive(Debug, Clone, PartialEq)]
#[deserr(try_from(Value) = TryFrom::try_from -> InvalidSimilarId)]
pub struct ExternalDocumentId(String); pub struct ExternalDocumentId(String);
impl AsRef<str> for ExternalDocumentId { impl AsRef<str> for ExternalDocumentId {
@ -674,7 +673,7 @@ impl ExternalDocumentId {
} }
impl TryFrom<String> for ExternalDocumentId { impl TryFrom<String> for ExternalDocumentId {
type Error = InvalidSimilarId; type Error = milli::UserError;
fn try_from(value: String) -> Result<Self, Self::Error> { fn try_from(value: String) -> Result<Self, Self::Error> {
serde_json::Value::String(value).try_into() serde_json::Value::String(value).try_into()
@ -682,10 +681,10 @@ impl TryFrom<String> for ExternalDocumentId {
} }
impl TryFrom<Value> for ExternalDocumentId { impl TryFrom<Value> for ExternalDocumentId {
type Error = InvalidSimilarId; type Error = milli::UserError;
fn try_from(value: Value) -> Result<Self, Self::Error> { fn try_from(value: Value) -> Result<Self, Self::Error> {
Ok(Self(milli::documents::validate_document_id_value(value).map_err(|_| InvalidSimilarId)?)) Ok(Self(milli::documents::validate_document_id_value(value)?))
} }
} }
@ -1597,6 +1596,11 @@ pub fn perform_similar(
ranking_score_threshold, ranking_score_threshold,
} = query; } = query;
let id: ExternalDocumentId = id.try_into().map_err(|error| {
let msg = format!("Invalid value at `.id`: {error}");
ResponseError::from_msg(msg, Code::InvalidSimilarId)
})?;
// using let-else rather than `?` so that the borrow checker identifies we're always returning here, // using let-else rather than `?` so that the borrow checker identifies we're always returning here,
// preventing a use-after-move // preventing a use-after-move
let Some(internal_id) = index.external_documents_ids().get(&rtxn, &id)? else { let Some(internal_id) = index.external_documents_ids().get(&rtxn, &id)? else {