(unomi) 05/07: UNOMI-973: Do not read a setting the router has not published yet as a refusal

[js...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jsinovassin pushed a commit to branch UNOMI-973-review-followups
in repository https://gitbox.apache.org/repos/asf/unomi.git

commit 1a223bda4f752eef2ce6536420107f835a5daba3
Author: jsinovassin <js...@jahia.com>
AuthorDate: Tue Sep 1 18:37:17 2026 +0200

    UNOMI-973: Do not read a setting the router has not published yet as a refusal
    
    The REST endpoint validates against the settings the router's Camel context
    publishes on start-up, but it answers as soon as its own component is
    satisfied, which can be first. getProperty then returns null, an empty scheme
    allow-list follows from it, and a perfectly good configuration is answered
    400 "endpoint scheme 'file' is not allowed" -- pointing its author at a
    scheme that was never the problem.
    
    An absent setting now answers 503 with what is actually the matter, so the
    caller retries instead of correcting a configuration that is already right.
    Nothing is stored either way.
---
 .../rest/AbstractConfigurationServiceEndpoint.java | 21 +++++++++++++++++---
 .../rest/ConfigurationEndpointValidationTest.java  | 23 ++++++++++++++++++++++
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/AbstractConfigurationServiceEndpoint.java b/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/AbstractConfigurationServiceEndpoint.java
index 207aa4d7c..26db48e61 100644
--- a/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/AbstractConfigurationServiceEndpoint.java
+++ b/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/AbstractConfigurationServiceEndpoint.java
@@ -44,13 +44,28 @@ public abstract class AbstractConfigurationServiceEndpoint<T> {
      * but a log line to show for it. Refusing here gives the caller the reason while it can still act
      * on it, and keeps the configuration out of the store.
      *
+     * <p>Answers {@code 503} instead while the router has yet to publish its settings, since a
+     * configuration cannot be judged against settings that are not there yet.
+     *
      * @param endpointUri              the endpoint URI the configuration names
      * @param permittedBaseDirsProperty the shared property holding the base directories for this direction
      */
     protected void refuseIfEndpointCannotBeHonoured(String endpointUri, String permittedBaseDirsProperty) {
-        String refusal = EndpointValidator.validate(endpointUri,
-                (String) configSharingService.getProperty(RouterConstants.CONFIG_ALLOWED_ENDPOINTS),
-                (String) configSharingService.getProperty(permittedBaseDirsProperty));
+        String allowedSchemes = (String) configSharingService.getProperty(RouterConstants.CONFIG_ALLOWED_ENDPOINTS);
+        String permittedBaseDirs = (String) configSharingService.getProperty(permittedBaseDirsProperty);
+        if (allowedSchemes == null || permittedBaseDirs == null) {
+            // The router's Camel context publishes both on start-up, and this endpoint answers before
+            // it has. An absent setting is not an empty allow-list: reading it as one would refuse a
+            // legitimate configuration, and blame its scheme for it. Say the truth instead -- there is
+            // nothing to validate against yet -- so the caller can retry rather than correct a
+            // configuration that is already right.
+            String unavailable = "the router is still starting up: no endpoint can be validated yet";
+            throw new ServiceUnavailableException(unavailable,
+                    Response.status(Response.Status.SERVICE_UNAVAILABLE)
+                            .type(MediaType.TEXT_PLAIN).entity(unavailable).build());
+        }
+
+        String refusal = EndpointValidator.validate(endpointUri, allowedSchemes, permittedBaseDirs);
         if (refusal != null) {
             throw new BadRequestException(refusal, Response.status(Response.Status.BAD_REQUEST)
                     .type(MediaType.TEXT_PLAIN).entity(refusal).build());
diff --git a/extensions/router/router-rest/src/test/java/org/apache/unomi/router/rest/ConfigurationEndpointValidationTest.java b/extensions/router/router-rest/src/test/java/org/apache/unomi/router/rest/ConfigurationEndpointValidationTest.java
index b93adf829..b5df90d11 100644
--- a/extensions/router/router-rest/src/test/java/org/apache/unomi/router/rest/ConfigurationEndpointValidationTest.java
+++ b/extensions/router/router-rest/src/test/java/org/apache/unomi/router/rest/ConfigurationEndpointValidationTest.java
@@ -100,6 +100,19 @@ public class ConfigurationEndpointValidationTest {
         assertTrue("the configuration should have been stored", importConfigurations.contains("in-bounds"));
     }
 
+    @Test
+    public void savingARecurrentImportBeforeTheRouterPublishedItsSettingsIsNotRefused() throws Exception {
+        // the router's Camel context has not started yet, so it has published nothing
+        ImportConfigurationServiceEndPoint starting = new ImportConfigurationServiceEndPoint();
+        starting.setImportConfigurationService(importConfigurations);
+        starting.setConfigSharingService(new InMemoryConfigSharingService());
+
+        ImportConfiguration configuration = recurrentImport(fileUri(permittedImportDir, "?fileName=profiles.csv"));
+
+        assertUnavailable(() -> starting.saveConfiguration(configuration));
+        assertFalse("nothing is stored while the endpoint cannot be judged", importConfigurations.contains("in-bounds"));
+    }
+
     @Test
     public void savingARecurrentImportWhoseSourceIsOutsideThePermittedBaseDirsIsRefused() {
         ImportConfiguration configuration = recurrentImport(fileUri(arbitraryDir, "?fileName=profiles.csv"));
@@ -162,6 +175,16 @@ public class ConfigurationEndpointValidationTest {
      * A refused configuration answers {@code 400 Bad Request}, and says why: the caller has to be able
      * to correct the endpoint from the answer alone.
      */
+    private void assertUnavailable(Runnable save) {
+        try {
+            save.run();
+            fail("saving the configuration should not have been answered yet");
+        } catch (WebApplicationException e) {
+            assertEquals("settings that are not published yet make the service unavailable, not the "
+                    + "configuration wrong", 503, e.getResponse().getStatus());
+        }
+    }
+
     private void assertRefused(Runnable save) {
         try {
             save.run();