Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Commit

Permalink
throw an error on unknown fields specified in the _geo field
Browse files Browse the repository at this point in the history
  • Loading branch information
irevoire committed Jan 24, 2023
1 parent 1c4b1b3 commit de3c4f1
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 0 deletions.
2 changes: 2 additions & 0 deletions milli/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and undersco
pub enum GeoError {
#[error("The `_geo` field in the document with the id: `{document_id}` is not an object. Was expecting an object with the `_geo.lat` and `_geo.lng` fields but instead got `{value}`.")]
NotAnObject { document_id: Value, value: Value },
#[error("The `_geo` field in the document with the id: `{document_id}` contains the following unexpected fields: `{value}`.")]
UnexpectedExtraFields { document_id: Value, value: Value },
#[error("Could not find latitude nor longitude in the document with the id: `{document_id}`. Was expecting `_geo.lat` and `_geo.lng` fields.")]
MissingLatitudeAndLongitude { document_id: Value },
#[error("Could not find latitude in the document with the id: `{document_id}`. Was expecting a `_geo.lat` field.")]
Expand Down
31 changes: 31 additions & 0 deletions milli/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2326,4 +2326,35 @@ pub(crate) mod tests {

db_snap!(index, geo_faceted_documents_ids); // ensure that no more document was inserted
}

#[test]
fn unexpected_extra_fields_in_geo_field() {
let index = TempIndex::new();

index
.update_settings(|settings| {
settings.set_primary_key("id".to_string());
settings.set_filterable_fields(HashSet::from(["_geo".to_string()]));
})
.unwrap();

let err = index
.add_documents(
documents!({ "id" : "doggo", "_geo": { "lat": 1, "lng": 2, "doggo": "are the best" }}),
)
.unwrap_err();
insta::assert_display_snapshot!(err, @r###"The `_geo` field in the document with the id: `"\"doggo\""` contains the following unexpected fields: `{"doggo":"are the best"}`."###);

db_snap!(index, geo_faceted_documents_ids); // ensure that no documents were inserted

// multiple fields and complex values
let err = index
.add_documents(
documents!({ "id" : "doggo", "_geo": { "lat": 1, "lng": 2, "doggo": "are the best", "and": { "all": ["cats", { "are": "beautiful" } ] } } }),
)
.unwrap_err();
insta::assert_display_snapshot!(err, @r###"The `_geo` field in the document with the id: `"\"doggo\""` contains the following unexpected fields: `{"and":{"all":["cats",{"are":"beautiful"}]},"doggo":"are the best"}`."###);

db_snap!(index, geo_faceted_documents_ids); // ensure that no documents were inserted
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: milli/src/index.rs
---
[]
4 changes: 4 additions & 0 deletions milli/src/update/index_documents/enrich.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,10 @@ pub fn validate_geo_from_json(id: &DocumentId, bytes: &[u8]) -> Result<StdResult
Value::Object(mut object) => match (object.remove("lat"), object.remove("lng")) {
(Some(lat), Some(lng)) => {
match (extract_finite_float_from_value(lat), extract_finite_float_from_value(lng)) {
(Ok(_), Ok(_)) if !object.is_empty() => Ok(Err(UnexpectedExtraFields {
document_id: debug_id(),
value: object.into(),
})),
(Ok(_), Ok(_)) => Ok(Ok(())),
(Err(value), Ok(_)) => Ok(Err(BadLatitude { document_id: debug_id(), value })),
(Ok(_), Err(value)) => Ok(Err(BadLongitude { document_id: debug_id(), value })),
Expand Down

0 comments on commit de3c4f1

Please sign in to comment.