Merge branch '7.8'

Author: at055612

CHANGELOG.md

@@ -12,6 +12,20 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 DO NOT ADD CHANGES HERE - ADD THEM USING log_change.sh
 ~~~
 
+* Issue **#4831** : Fix Data Retention -> Impact Summary not showing any data.
+
+* Issue **#4829** : Fix stuck searches.
+
+* Issue **#4830** : Use a cache rather than sessions to maintain auth flow state to avoid creating unnecessary sessions.
+
+* Issue **#4842** : Fix null session when doing OIDC code flow with KeyCloak.
+
+* Issue **#4844** : Fix issue where vis parent table filters are not applied to the right data values.
+
+* Issue **#4837** : Change the fetching of OIDC config to use jersey client instead of Apache http client. The yaml properties `appConfig.security.authentication.openId.httpClient` and `proxyConfig.security.authentication.openId.httpClient` have been removed. Configuration of the jersey client is now done using `jerseyClients.OPEN_ID.` (see https://gchq.github.io/stroom-docs/docs/install-guide/configuration/stroom-and-proxy/common-configuration/#jersey-http-client-configuration).
+
+* Issue **#4849** : Fix the default forwarding queue config so that it retries for HTTP and not for FILE. Add the config prop `queue.queueAndRetryEnabled` to control whether forwarding is queued with retry handling or not. Add the config prop `atomicMoveEnabled` to `forwardFileDestinations` items to allow disabling of atomic file moves when using a remote file system that doesn't support atomic moves.
+
 
 ## [v7.9-beta.6] - 2025-04-07
 

stroom-config/stroom-config-app/src/test/resources/stroom/config/app/expected.yaml

@@ -880,7 +880,6 @@ appConfig:
         clientSecret: null
         expectedSignerPrefixes: []
         formTokenRequest: true
-        httpClient: null
         identityProviderType: "INTERNAL_IDP"
         issuer: null
         jwksUri: null

stroom-proxy/stroom-proxy-app/src/main/java/stroom/proxy/app/ProxyOpenIdConfig.java

@@ -2,7 +2,6 @@
 
 import stroom.security.openid.api.AbstractOpenIdConfig;
 import stroom.security.openid.api.IdpType;
-import stroom.util.http.HttpClientConfiguration;
 import stroom.util.shared.IsProxyConfig;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
@@ -42,8 +41,7 @@ public ProxyOpenIdConfig(
             @JsonProperty("validIssuers") final Set<String> validIssuers,
             @JsonProperty("uniqueIdentityClaim") final String uniqueIdentityClaim,
             @JsonProperty("userDisplayNameClaim") final String userDisplayNameClaim,
-            @JsonProperty(PROP_NAME_EXPECTED_SIGNER_PREFIXES) final Set<String> expectedSignerPrefixes,
-            @JsonProperty("httpClient") final HttpClientConfiguration httpClient) {
+            @JsonProperty(PROP_NAME_EXPECTED_SIGNER_PREFIXES) final Set<String> expectedSignerPrefixes) {
 
         super(identityProviderType,
                 openIdConfigurationEndpoint,
@@ -62,8 +60,7 @@ public ProxyOpenIdConfig(
                 validIssuers,
                 uniqueIdentityClaim,
                 userDisplayNameClaim,
-                expectedSignerPrefixes,
-                httpClient);
+                expectedSignerPrefixes);
     }
 
     @JsonIgnore
@@ -113,7 +110,6 @@ public ProxyOpenIdConfig withIdentityProviderType(final IdpType identityProvider
                 getValidIssuers(),
                 getUniqueIdentityClaim(),
                 getUserDisplayNameClaim(),
-                getExpectedSignerPrefixes(),
-                getHttpClient());
+                getExpectedSignerPrefixes());
     }
 }

stroom-proxy/stroom-proxy-app/src/main/java/stroom/proxy/app/handler/ForwardException.java

@@ -3,6 +3,7 @@
 import stroom.meta.api.AttributeMap;
 import stroom.meta.api.StandardHeaderArguments;
 import stroom.proxy.StroomStatusCode;
+import stroom.proxy.app.handler.HttpSender.ResponseStatus;
 import stroom.util.shared.NullSafe;
 
 import java.util.Objects;
@@ -12,15 +13,18 @@ public class ForwardException extends RuntimeException {
     private final StroomStatusCode stroomStatusCode;
     private final String feedName;
     private final boolean isRecoverable;
+    private final int httpResponseCode; // In the case of UNKNOWN_ERROR, this may differ from 999
 
     private ForwardException(final StroomStatusCode stroomStatusCode,
                              final AttributeMap attributeMap,
                              final String message,
+                             final int httpResponseCode,
                              final boolean isRecoverable,
                              final Throwable cause) {
         super(message, cause);
         this.isRecoverable = isRecoverable;
         this.stroomStatusCode = stroomStatusCode;
+        this.httpResponseCode = httpResponseCode;
         this.feedName = NullSafe.get(
                 attributeMap,
                 attrMap -> attrMap.get(StandardHeaderArguments.FEED));
@@ -30,26 +34,52 @@ public static ForwardException recoverable(final StroomStatusCode stroomStatusCo
                                                final AttributeMap attributeMap,
                                                final String message,
                                                final Throwable cause) {
-        return new ForwardException(stroomStatusCode, attributeMap, message, true, cause);
+        return new ForwardException(
+                stroomStatusCode,
+                attributeMap,
+                message,
+                stroomStatusCode.getHttpCode(),
+                true,
+                cause);
     }
 
-    public static ForwardException recoverable(final StroomStatusCode stroomStatusCode,
+    public static ForwardException recoverable(final ResponseStatus responseStatus,
                                                final AttributeMap attributeMap) {
-        Objects.requireNonNull(stroomStatusCode);
-        return new ForwardException(stroomStatusCode, attributeMap, stroomStatusCode.getMessage(), true, null);
+        Objects.requireNonNull(responseStatus);
+        final StroomStatusCode stroomStatusCode = responseStatus.stroomStatusCode();
+        return new ForwardException(
+                stroomStatusCode,
+                attributeMap,
+                Objects.requireNonNullElse(responseStatus.message(), stroomStatusCode.getMessage()),
+                responseStatus.httpResponseCode(),
+                true,
+                null);
     }
 
     public static ForwardException nonRecoverable(final StroomStatusCode stroomStatusCode,
                                                   final AttributeMap attributeMap,
                                                   final String message,
                                                   final Throwable cause) {
-        return new ForwardException(stroomStatusCode, attributeMap, message, false, cause);
+        return new ForwardException(
+                stroomStatusCode,
+                attributeMap,
+                message,
+                stroomStatusCode.getHttpCode(),
+                false,
+                cause);
     }
 
-    public static ForwardException nonRecoverable(final StroomStatusCode stroomStatusCode,
+    public static ForwardException nonRecoverable(final ResponseStatus responseStatus,
                                                   final AttributeMap attributeMap) {
-        Objects.requireNonNull(stroomStatusCode);
-        return new ForwardException(stroomStatusCode, attributeMap, stroomStatusCode.getMessage(), false, null);
+        Objects.requireNonNull(responseStatus);
+        final StroomStatusCode stroomStatusCode = responseStatus.stroomStatusCode();
+        return new ForwardException(
+                stroomStatusCode,
+                attributeMap,
+                Objects.requireNonNullElse(responseStatus.message(), stroomStatusCode.getMessage()),
+                responseStatus.httpResponseCode(),
+                false,
+                null);
     }
 
     public boolean isRecoverable() {
@@ -59,4 +89,8 @@ public boolean isRecoverable() {
     public String getFeedName() {
         return feedName;
     }
+
+    public int getHttpResponseCode() {
+        return httpResponseCode;
+    }
 }

stroom-proxy/stroom-proxy-app/src/main/java/stroom/proxy/app/handler/ForwardFileConfig.java

@@ -21,29 +21,33 @@ public final class ForwardFileConfig
         implements IsProxyConfig, ForwarderConfig {
 
     public static final String PROP_NAME_SUB_PATH_TEMPLATE = "subPathTemplate";
+    public static final String PROP_NAME_ATOMIC_MOVE_ENABLED = "atomicMoveEnabled";
     public static final TemplatingMode DEFAULT_TEMPLATING_MODE = TemplatingMode.REPLACE_UNKNOWN_PARAMS;
 
     private static final String DEFAULT_SUB_PATH_TEMPLATE = "${year}${month}${day}/${feed}";
+    private static final boolean DEFAULT_ATOMIC_MOVE_ENABLED = true;
     private static final LivenessCheckMode DEFAULT_LIVENESS_CHECK_MODE = LivenessCheckMode.READ;
 
     private final boolean enabled;
     private final boolean instant;
     private final String name;
     private final String path;
     private final PathTemplateConfig subPathTemplate;
-    private final ForwardQueueConfig forwardQueueConfig;
+    private final ForwardFileQueueConfig forwardQueueConfig;
     private final String livenessCheckPath;
     private final LivenessCheckMode livenessCheckMode;
+    private final boolean atomicMoveEnabled;
 
     public ForwardFileConfig() {
         enabled = true;
         instant = false;
         name = null;
         path = null;
         subPathTemplate = null;
-        forwardQueueConfig = null; // Assume local file forwarder by default, so no queue config needed
+        forwardQueueConfig = new ForwardFileQueueConfig();
         livenessCheckPath = null;
         livenessCheckMode = LivenessCheckMode.READ;
+        atomicMoveEnabled = DEFAULT_ATOMIC_MOVE_ENABLED;
     }
 
     @SuppressWarnings("unused")
@@ -53,17 +57,19 @@ public ForwardFileConfig(@JsonProperty("enabled") final boolean enabled,
                              @JsonProperty("name") final String name,
                              @JsonProperty("path") final String path,
                              @JsonProperty(PROP_NAME_SUB_PATH_TEMPLATE) final PathTemplateConfig subPathTemplate,
-                             @JsonProperty("queue") final ForwardQueueConfig forwardQueueConfig,
+                             @JsonProperty("queue") final ForwardFileQueueConfig forwardQueueConfig,
                              @JsonProperty("livenessCheckPath") final String livenessCheckPath,
-                             @JsonProperty("livenessCheckMode") final LivenessCheckMode livenessCheckMode) {
+                             @JsonProperty("livenessCheckMode") final LivenessCheckMode livenessCheckMode,
+                             @JsonProperty(PROP_NAME_ATOMIC_MOVE_ENABLED) final Boolean atomicMoveEnabled) {
         this.enabled = enabled;
         this.instant = instant;
         this.name = name;
         this.path = path;
         this.subPathTemplate = Objects.requireNonNullElse(subPathTemplate, PathTemplateConfig.DISABLED);
-        this.forwardQueueConfig = forwardQueueConfig;
+        this.forwardQueueConfig = Objects.requireNonNullElseGet(forwardQueueConfig, ForwardFileQueueConfig::new);
         this.livenessCheckPath = livenessCheckPath;
         this.livenessCheckMode = Objects.requireNonNullElse(livenessCheckMode, DEFAULT_LIVENESS_CHECK_MODE);
+        this.atomicMoveEnabled = Objects.requireNonNullElse(atomicMoveEnabled, DEFAULT_ATOMIC_MOVE_ENABLED);
     }
 
     private ForwardFileConfig(final Builder builder) {
@@ -75,6 +81,7 @@ private ForwardFileConfig(final Builder builder) {
         forwardQueueConfig = builder.forwardQueueConfig;
         livenessCheckPath = builder.livenessCheckPath;
         livenessCheckMode = builder.livenessCheckMode;
+        atomicMoveEnabled = builder.atomicMoveEnabled;
     }
 
     /**
@@ -130,13 +137,14 @@ public PathTemplateConfig getSubPathTemplate() {
         return subPathTemplate;
     }
 
+    @NotNull
     @Override
     @JsonProperty("queue")
     @JsonPropertyDescription("Adds multi-threading and retry control to this forwarder. Can be set to null " +
                              "for a local file forwarder, but should be populated if the file forwarder is " +
                              "forwarding to a remote file system that may fail. Defaults to null as a " +
                              "local file forwarder is assumed.")
-    public ForwardQueueConfig getForwardQueueConfig() {
+    public ForwardFileQueueConfig getForwardQueueConfig() {
         return forwardQueueConfig;
     }
 
@@ -169,10 +177,24 @@ public String getLivenessCheckPath() {
         return livenessCheckPath;
     }
 
+    @JsonPropertyDescription(
+            "The type of liveness check to perform (READ|WRITE). " +
+            "READ will attempt to read the file/dir specified in livenessCheckPath. " +
+            "WRITE will attempt to touch the file specified in livenessCheckPath.")
     public LivenessCheckMode getLivenessCheckMode() {
         return livenessCheckMode;
     }
 
+    @JsonPropertyDescription(
+            "Stroom-Proxy will attempt to move files onto the forward destination using an atomic move. " +
+            "This ensures that the move does not happen more than once. If an atomic move is not possible, " +
+            "e.g. the destination is a remote file system that does not support an atomic move, then it will " +
+            "fall back to a non-atomic move with the risk of it happening more than once. If you see warnings " +
+            "in the logs or know the file system will not support atomic moves then set this to false.")
+    public boolean isAtomicMoveEnabled() {
+        return atomicMoveEnabled;
+    }
+
     @Override
     public boolean equals(final Object o) {
         if (this == o) {
@@ -189,7 +211,8 @@ public boolean equals(final Object o) {
                && Objects.equals(subPathTemplate, that.subPathTemplate)
                && Objects.equals(forwardQueueConfig, that.forwardQueueConfig)
                && Objects.equals(livenessCheckPath, that.livenessCheckPath)
-               && livenessCheckMode == that.livenessCheckMode;
+               && livenessCheckMode == that.livenessCheckMode
+               && atomicMoveEnabled == that.atomicMoveEnabled;
     }
 
     @Override
@@ -201,7 +224,8 @@ public int hashCode() {
                 subPathTemplate,
                 forwardQueueConfig,
                 livenessCheckPath,
-                livenessCheckMode);
+                livenessCheckMode,
+                atomicMoveEnabled);
     }
 
     @Override
@@ -215,6 +239,7 @@ public String toString() {
                ", forwardQueueConfig=" + forwardQueueConfig +
                ", livenessCheckPath='" + livenessCheckPath + '\'' +
                ", livenessCheckMode=" + livenessCheckMode +
+               ", atomicMoveEnabled=" + atomicMoveEnabled +
                '}';
     }
 
@@ -232,6 +257,7 @@ public static Builder builder(final ForwardFileConfig copy) {
         builder.forwardQueueConfig = copy.getForwardQueueConfig();
         builder.livenessCheckPath = copy.getLivenessCheckPath();
         builder.livenessCheckMode = copy.getLivenessCheckMode();
+        builder.atomicMoveEnabled = copy.isAtomicMoveEnabled();
         return builder;
     }
 
@@ -241,14 +267,15 @@ public static Builder builder(final ForwardFileConfig copy) {
 
     public static final class Builder {
 
-        public String livenessCheckPath;
-        public LivenessCheckMode livenessCheckMode;
+        private String livenessCheckPath;
+        private LivenessCheckMode livenessCheckMode;
+        private boolean atomicMoveEnabled;
         private boolean enabled;
         private boolean instant;
         private String name;
         private String path;
         private PathTemplateConfig subPathTemplate;
-        private ForwardQueueConfig forwardQueueConfig;
+        private ForwardFileQueueConfig forwardQueueConfig;
 
         private Builder() {
         }
@@ -293,7 +320,7 @@ public Builder withSubPathTemplate(final PathTemplateConfig subPathTemplate) {
             return this;
         }
 
-        public Builder withForwardQueueConfig(final ForwardQueueConfig forwardQueueConfig) {
+        public Builder withForwardQueueConfig(final ForwardFileQueueConfig forwardQueueConfig) {
             this.forwardQueueConfig = forwardQueueConfig;
             return this;
         }
@@ -308,6 +335,11 @@ public Builder withLivenessCheckMode(final LivenessCheckMode livenessCheckMode)
             return this;
         }
 
+        public Builder withAtomicMoveEnabled(final boolean atomicMoveEnabled) {
+            this.atomicMoveEnabled = atomicMoveEnabled;
+            return this;
+        }
+
         public ForwardFileConfig build() {
             return new ForwardFileConfig(this);
         }

stroom-proxy/stroom-proxy-app/src/main/java/stroom/proxy/app/handler/ForwardFileDestinationFactoryImpl.java

@@ -6,11 +6,13 @@
 import stroom.util.io.PathCreator;
 import stroom.util.logging.LambdaLogger;
 import stroom.util.logging.LambdaLoggerFactory;
+import stroom.util.logging.LogUtil;
 
 import jakarta.inject.Inject;
 import jakarta.inject.Singleton;
 
 import java.nio.file.Path;
+import java.util.Objects;
 
 @Singleton
 public class ForwardFileDestinationFactoryImpl implements ForwardFileDestinationFactory {
@@ -62,20 +64,16 @@ public ForwardDestination create(final ForwardFileConfig config) {
     private ForwardDestination getWrappedForwardDestination(final ForwardFileConfig config,
                                                             final ForwardFileDestinationImpl forwardFileDestination) {
         final ForwardQueueConfig forwardQueueConfig = config.getForwardQueueConfig();
-        final ForwardDestination destination;
-        if (forwardQueueConfig != null) {
-            // We have queue config so wrap out ultimate destination with some queue/retry logic
-            destination = new RetryingForwardDestination(
-                    forwardQueueConfig,
-                    forwardFileDestination,
-                    dataDirProvider,
-                    pathCreator,
-                    dirQueueFactory,
-                    proxyServices,
-                    fileStores);
-        } else {
-            destination = forwardFileDestination;
-        }
-        return destination;
+        Objects.requireNonNull(forwardQueueConfig, () -> LogUtil.message(
+                "No forwardQueueConfig set for destination '{}'", config.getName()));
+        // We have queue config so wrap out ultimate destination with some queue/retry logic
+        return new RetryingForwardDestination(
+                forwardQueueConfig,
+                forwardFileDestination,
+                dataDirProvider,
+                pathCreator,
+                dirQueueFactory,
+                proxyServices,
+                fileStores);
     }
 }