Make sure route conflicts are resolved

golem-worker-service-base/src/api/common.rs

@@ -134,6 +134,12 @@ mod conversion {
                 ApiDeploymentError::DeploymentConflict(conflict) => {
                     ApiEndpointError::already_exists(format!("Deployment conflict: {}", conflict))
                 }
+                ApiDeploymentError::ConflictingDefinitions(conflicts) => {
+                    ApiEndpointError::already_exists(format!(
+                        "Conflicting API definitions during deployment: {}",
+                        conflicts.join(", ")
+                    ))
+                }
             }
         }
     }

golem-worker-service-base/src/api/custom_http_request_api.rs

@@ -9,7 +9,7 @@ use poem::{Body, Endpoint, Request, Response};
 use tracing::{error, info};
 
 use crate::http::{ApiInputPath, InputHttpRequest};
-use crate::service::api_definition_lookup::ApiDefinitionLookup;
+use crate::service::api_definition_lookup::ApiDefinitionsLookup;
 
 use crate::worker_binding::WorkerBindingResolver;
 use crate::worker_bridge_execution::WorkerRequestExecutor;
@@ -21,15 +21,15 @@ pub struct CustomHttpRequestApi {
     pub evaluator: Arc<dyn Evaluator + Sync + Send>,
     pub worker_metadata_fetcher: Arc<dyn WorkerMetadataFetcher + Sync + Send>,
     pub api_definition_lookup_service:
-        Arc<dyn ApiDefinitionLookup<InputHttpRequest, HttpApiDefinition> + Sync + Send>,
+        Arc<dyn ApiDefinitionsLookup<InputHttpRequest, HttpApiDefinition> + Sync + Send>,
 }
 
 impl CustomHttpRequestApi {
     pub fn new(
         worker_request_executor_service: Arc<dyn WorkerRequestExecutor + Sync + Send>,
         worker_metadata_fetcher: Arc<dyn WorkerMetadataFetcher + Sync + Send>,
         api_definition_lookup_service: Arc<
-            dyn ApiDefinitionLookup<InputHttpRequest, HttpApiDefinition> + Sync + Send,
+            dyn ApiDefinitionsLookup<InputHttpRequest, HttpApiDefinition> + Sync + Send,
         >,
     ) -> Self {
         let evaluator = Arc::new(DefaultEvaluator::from_worker_request_executor(
@@ -83,7 +83,7 @@ impl CustomHttpRequestApi {
             req_body: json_request_body,
         };
 
-        let api_definition = match self
+        let possible_api_definitions = match self
             .api_definition_lookup_service
             .get(api_request.clone())
             .await
@@ -97,18 +97,15 @@ impl CustomHttpRequestApi {
             }
         };
 
-        match api_request.resolve(&api_definition).await {
+        match api_request.resolve(&possible_api_definitions).await {
             Ok(resolved_worker_request) => {
                 resolved_worker_request
                     .execute_with::<poem::Response>(&self.evaluator, &self.worker_metadata_fetcher)
                     .await
             }
 
             Err(msg) => {
-                error!(
-                    "API request id: {} - request error: {}",
-                    &api_definition.id, msg
-                );
+                error!("Failed to resolve the API definition; error: {}", msg);
 
                 Response::builder()
                     .status(StatusCode::METHOD_NOT_ALLOWED)

golem-worker-service-base/src/http/http_request.rs

@@ -313,7 +313,10 @@ mod tests {
         let evaluator = get_test_evaluator();
         let worker_metadata_fetcher = get_test_metadata_fetcher("golem:it/api/get-cart-contents");
 
-        let resolved_route = api_request.resolve(api_specification).await.unwrap();
+        let resolved_route = api_request
+            .resolve(&vec![api_specification.clone()])
+            .await
+            .unwrap();
 
         resolved_route
             .execute_with(&evaluator, &worker_metadata_fetcher)
@@ -951,7 +954,7 @@ mod tests {
                 function_params,
             );
 
-            let resolved_route = api_request.resolve(&api_specification).await;
+            let resolved_route = api_request.resolve(&vec![api_specification]).await;
 
             let result = resolved_route.map(|x| x.worker_detail);
 
@@ -985,7 +988,7 @@ mod tests {
                 expression,
             );
 
-            let resolved_route = api_request.resolve(&api_specification).await.unwrap();
+            let resolved_route = api_request.resolve(&vec![api_specification]).await.unwrap();
 
             assert_eq!(
                 resolved_route.worker_detail.idempotency_key,

golem-worker-service-base/src/repo/api_deployment_repo.rs

@@ -1,4 +1,5 @@
 use std::collections::HashMap;
+use std::fmt::Display;
 use std::sync::Mutex;
 
 use crate::api_definition::{ApiDefinitionId, ApiDeployment, ApiSite, ApiSiteString};
@@ -40,6 +41,15 @@ pub trait ApiDeploymentRepo<Namespace: ApiNamespace> {
 pub enum ApiDeploymentRepoError {
     Internal(anyhow::Error),
 }
+
+impl Display for ApiDeploymentRepoError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            ApiDeploymentRepoError::Internal(e) => write!(f, "Internal error: {}", e),
+        }
+    }
+}
+
 impl From<RedisError> for ApiDeploymentRepoError {
     fn from(err: RedisError) -> Self {
         ApiDeploymentRepoError::Internal(anyhow::Error::new(err))

golem-worker-service-base/src/service/api_definition_lookup.rs

@@ -2,12 +2,14 @@ use std::fmt::Display;
 
 use async_trait::async_trait;
 
+// TODO; We could optimise this further
+// to pick the exact API Definition (instead of a vector),
+// by doing route resolution at this stage rather than
+// delegating that task to worker-binding resolver.
+// However, requires lot more work.
 #[async_trait]
-pub trait ApiDefinitionLookup<Input, ApiDefinition> {
-    async fn get(
-        &self,
-        input_http_request: Input,
-    ) -> Result<ApiDefinition, ApiDefinitionLookupError>;
+pub trait ApiDefinitionsLookup<Input, ApiDefinition> {
+    async fn get(&self, input: Input) -> Result<Vec<ApiDefinition>, ApiDefinitionLookupError>;
 }
 
 pub struct ApiDefinitionLookupError(pub String);