Simplify update endpoint to use same default URI as create

Author: rdettai

docs/reference/rest-api.md

@@ -348,7 +348,7 @@ Updating the doc mapping doesn't reindex existing data. Queries and results are
 |---------------------|--------------------|-----------------------------------------------------------------------------------------------------------------------|---------------------------------------|
 | `version`           | `String`           | Config format version, use the same as your Quickwit version.                                                         | _required_                            |
 | `index_id`          | `String`           | Index ID, must be the same index as in the request URI.                                                               | _required_                            |
-| `index_uri`         | `String`           | Defines where the index files are stored. Cannot be updated.                                                          | `{current_index_uri}`                 |
+| `index_uri`         | `String`           | Defines where the index files are stored. Cannot be updated.                                                          | `{default_index_root_uri}/{index_id}`                 |
 | `doc_mapping`       | `DocMapping`       | Doc mapping object as specified in the [index config docs](../configuration/index-config.md#doc-mapping).             | _required_                            |
 | `indexing_settings` | `IndexingSettings` | Indexing settings object as specified in the [index config docs](../configuration/index-config.md#indexing-settings). |                                       |
 | `search_settings`   | `SearchSettings`   | Search settings object as specified in the [index config docs](../configuration/index-config.md#search-settings).     |                                       |

quickwit/quickwit-common/src/uri.rs

@@ -169,30 +169,23 @@ impl Uri {
     }
 
     /// Returns the parent URI.
-    ///
-    /// When the URI points to an object store, this method makes sure that the
-    /// result targets to a well defined bucket, e.g `s3://` returns `None`.
-    ///
     /// Does not apply to PostgreSQL URIs.
     pub fn parent(&self) -> Option<Uri> {
-        self.parent_unchecked().filter(|parent| {
-            let path = parent.path();
-            match self.protocol() {
-                Protocol::S3 => path.components().count() >= 1,
-                Protocol::Azure => path.components().count() >= 2,
-                Protocol::Google => path.components().count() >= 1,
-                _ => true,
-            }
-        })
-    }
-
-    /// Same as [Self::parent()] but without the additional check on object stores.
-    pub fn parent_unchecked(&self) -> Option<Uri> {
         if self.protocol().is_database() {
             return None;
         }
         let path = self.path();
         let protocol = self.protocol();
+
+        if protocol == Protocol::S3 && path.components().count() < 2 {
+            return None;
+        }
+        if protocol == Protocol::Azure && path.components().count() < 3 {
+            return None;
+        }
+        if protocol == Protocol::Google && path.components().count() < 2 {
+            return None;
+        }
         let parent_path = path.parent()?;
 
         Some(Self {
@@ -604,23 +597,11 @@ mod tests {
             "ram:///foo"
         );
         assert!(Uri::for_test("s3://bucket").parent().is_none());
-        assert_eq!(
-            Uri::for_test("s3://bucket").parent_unchecked().unwrap(),
-            "s3://"
-        );
         assert!(Uri::for_test("s3://bucket/").parent().is_none());
-        assert_eq!(
-            Uri::for_test("s3://bucket/").parent_unchecked().unwrap(),
-            "s3://"
-        );
         assert_eq!(
             Uri::for_test("s3://bucket/foo").parent().unwrap(),
             "s3://bucket"
         );
-        assert_eq!(
-            Uri::for_test("s3://bucket/foo").parent_unchecked().unwrap(),
-            "s3://bucket"
-        );
         assert_eq!(
             Uri::for_test("s3://bucket/foo/").parent().unwrap(),
             "s3://bucket"

quickwit/quickwit-config/src/index_config/serialize.rs

@@ -68,18 +68,17 @@ pub fn load_index_config_from_user_config(
 ///
 /// Ensures that the new configuration is valid in itself and compared to the
 /// current index config. If the new configuration omits some fields, the
-/// default values will be used, not those of the current index config. The only
-/// exception is the index_uri because it cannot be updated.
+/// default values will be used, not those of the current index config.
 pub fn load_index_config_update(
     config_format: ConfigFormat,
     index_config_bytes: &[u8],
-    current_index_parent_dir: &Uri,
+    default_index_root_uri: &Uri,
     current_index_config: &IndexConfig,
 ) -> anyhow::Result<IndexConfig> {
     let mut new_index_config = load_index_config_from_user_config(
         config_format,
         index_config_bytes,
-        current_index_parent_dir,
+        default_index_root_uri,
     )?;
     ensure!(
         current_index_config.index_id == new_index_config.index_id,

quickwit/quickwit-serve/src/index_api/index_resource.rs

@@ -286,13 +286,15 @@ pub async fn create_index(
 
 pub fn update_index_handler(
     metastore: MetastoreServiceClient,
+    node_config: Arc<NodeConfig>,
 ) -> impl Filter<Extract = (impl warp::Reply,), Error = Rejection> + Clone {
     warp::path!("indexes" / String)
         .and(warp::put())
         .and(extract_config_format())
         .and(warp::body::content_length_limit(1024 * 1024))
         .and(warp::filters::body::bytes())
         .and(with_arg(metastore))
+        .and(with_arg(node_config))
         .then(update_index)
         .map(log_failure("failed to update index"))
         .and(extract_format_from_qs())
@@ -325,6 +327,7 @@ pub async fn update_index(
     config_format: ConfigFormat,
     index_config_bytes: Bytes,
     metastore: MetastoreServiceClient,
+    node_config: Arc<NodeConfig>,
 ) -> Result<IndexMetadata, IndexServiceError> {
     info!(index_id = %target_index_id, "update-index");
 
@@ -336,14 +339,10 @@ pub async fn update_index(
     let index_uid = current_index_metadata.index_uid.clone();
     let current_index_config = current_index_metadata.into_index_config();
 
-    let current_index_parent_dir = &current_index_config
-        .index_uri
-        .parent_unchecked()
-        .ok_or_else(|| IndexServiceError::Internal("index URI should have a parent".to_string()))?;
     let new_index_config = load_index_config_update(
         config_format,
         &index_config_bytes,
-        current_index_parent_dir,
+        &node_config.default_index_root_uri,
         &current_index_config,
     )
     .map_err(IndexServiceError::InvalidConfig)?;

quickwit/quickwit-serve/src/index_api/rest_handler.rs

@@ -89,8 +89,11 @@ pub fn index_management_handlers(
     // Indexes handlers.
     get_index_metadata_handler(index_service.metastore())
         .or(list_indexes_metadata_handler(index_service.metastore()))
-        .or(create_index_handler(index_service.clone(), node_config))
-        .or(update_index_handler(index_service.metastore()))
+        .or(create_index_handler(
+            index_service.clone(),
+            node_config.clone(),
+        ))
+        .or(update_index_handler(index_service.metastore(), node_config))
         .or(clear_index_handler(index_service.clone()))
         .or(delete_index_handler(index_service.clone()))
         .boxed()