This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch feature/WW-3576-session-map-staleness in repository https://gitbox.apache.org/repos/asf/struts.git
commit 456144416b23e4802547815c7686403e12c05314 Author: Lukasz Lenart <[email protected]> AuthorDate: Sun Oct 18 09:08:46 2026 +0200 WW-3576 feat(core): reconcile a session sandbox with per-action conflict resolution Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../action/BackgroundSessionConflictAware.java | 37 +++++ .../org/apache/struts2/action/SessionConflict.java | 31 ++++ .../struts2/action/SessionConflictResolution.java | 80 +++++++++ .../interceptor/exec/SessionSandboxReconciler.java | 113 +++++++++++++ .../exec/SessionSandboxReconcilerTest.java | 181 +++++++++++++++++++++ 5 files changed, 442 insertions(+) diff --git a/core/src/main/java/org/apache/struts2/action/BackgroundSessionConflictAware.java b/core/src/main/java/org/apache/struts2/action/BackgroundSessionConflictAware.java new file mode 100644 index 000000000..adfd0f2fb --- /dev/null +++ b/core/src/main/java/org/apache/struts2/action/BackgroundSessionConflictAware.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.action; + +/** + * Actions run by the execAndWait interceptor that want to decide how a session key is reconciled when the + * background process wrote it while another request changed it should implement this interface. + * + * Without it, the value the session holds at reconciliation is kept and the background's write is dropped. + */ +public interface BackgroundSessionConflictAware { + + /** + * Decides the value of one conflicting session key. Called once per key, on the polling request. + * + * @param conflict the key with its snapshot, current and background values. + * @return how the key is reconciled. + */ + SessionConflictResolution resolveSessionConflict(SessionConflict conflict); + +} diff --git a/core/src/main/java/org/apache/struts2/action/SessionConflict.java b/core/src/main/java/org/apache/struts2/action/SessionConflict.java new file mode 100644 index 000000000..5af7ccaae --- /dev/null +++ b/core/src/main/java/org/apache/struts2/action/SessionConflict.java @@ -0,0 +1,31 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.action; + +/** + * A session key the execAndWait background process wrote while another request changed it. A {@code null} + * value means the key was absent. + * + * @param key the session attribute name. + * @param snapshotValue the value when the background process started. + * @param currentValue the value the session holds now. + * @param backgroundValue the value the background process left, {@code null} if it removed the key. + */ +public record SessionConflict(String key, Object snapshotValue, Object currentValue, Object backgroundValue) { +} diff --git a/core/src/main/java/org/apache/struts2/action/SessionConflictResolution.java b/core/src/main/java/org/apache/struts2/action/SessionConflictResolution.java new file mode 100644 index 000000000..cf3e1bef6 --- /dev/null +++ b/core/src/main/java/org/apache/struts2/action/SessionConflictResolution.java @@ -0,0 +1,80 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.action; + +/** + * How a {@link BackgroundSessionConflictAware} action reconciles one conflicting session key. + */ +public final class SessionConflictResolution { + + private static final SessionConflictResolution KEEP_CURRENT = new SessionConflictResolution(Kind.KEEP_CURRENT, null); + private static final SessionConflictResolution USE_BACKGROUND = new SessionConflictResolution(Kind.USE_BACKGROUND, null); + private static final SessionConflictResolution REMOVE = new SessionConflictResolution(Kind.REPLACE, null); + + private enum Kind { KEEP_CURRENT, USE_BACKGROUND, REPLACE } + + private final Kind kind; + private final Object replacement; + + private SessionConflictResolution(Kind kind, Object replacement) { + this.kind = kind; + this.replacement = replacement; + } + + /** + * @return a resolution keeping the value the session holds now. + */ + public static SessionConflictResolution keepCurrent() { + return KEEP_CURRENT; + } + + /** + * @return a resolution taking the background process's value, removing the key if it removed it. + */ + public static SessionConflictResolution useBackground() { + return USE_BACKGROUND; + } + + /** + * @param value the value to store, {@code null} removes the key. + * @return a resolution storing the given value. + */ + public static SessionConflictResolution replaceWith(Object value) { + return new SessionConflictResolution(Kind.REPLACE, value); + } + + /** + * @return a resolution removing the key. + */ + public static SessionConflictResolution remove() { + return REMOVE; + } + + /** + * @param conflict the conflict this resolution answers. + * @return the value the key holds after reconciliation, {@code null} meaning removed. + */ + public Object resolve(SessionConflict conflict) { + return switch (kind) { + case KEEP_CURRENT -> conflict.currentValue(); + case USE_BACKGROUND -> conflict.backgroundValue(); + case REPLACE -> replacement; + }; + } +} diff --git a/core/src/main/java/org/apache/struts2/interceptor/exec/SessionSandboxReconciler.java b/core/src/main/java/org/apache/struts2/interceptor/exec/SessionSandboxReconciler.java new file mode 100644 index 000000000..0f76f283b --- /dev/null +++ b/core/src/main/java/org/apache/struts2/interceptor/exec/SessionSandboxReconciler.java @@ -0,0 +1,113 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.interceptor.exec; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpSession; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.struts2.action.BackgroundSessionConflictAware; +import org.apache.struts2.action.SessionConflict; + +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Objects; +import java.util.Set; + +/** + * Applies the writes an execAndWait background process logged in its {@link SandboxedSessionMap} to the + * session of the polling request. Writes are discarded when that session is not the one the process started + * with. A key changed elsewhere since the snapshot keeps its current value unless the action implements + * {@link BackgroundSessionConflictAware}. + */ +public final class SessionSandboxReconciler { + + private static final Logger LOG = LogManager.getLogger(SessionSandboxReconciler.class); + + private SessionSandboxReconciler() { + } + + public static void reconcile(SandboxedSessionMap sandbox, HttpServletRequest request, Object action) { + HttpSession real = request.getSession(false); + if (real == null || !real.getId().equals(sandbox.getStartSessionId())) { + LOG.debug("Discarding the session writes of background process [{}]: its session is gone", sandbox.getProcessName()); + return; + } + List<SessionOperation> operations = sandbox.getOperations(); + int lastInvalidate = lastInvalidateIndex(operations); + if (lastInvalidate >= 0) { + real.invalidate(); + HttpSession fresh = request.getSession(true); + for (String key : touchedKeys(operations.subList(lastInvalidate + 1, operations.size()))) { + write(fresh, key, sandbox.get(key)); + } + return; + } + for (String key : touchedKeys(operations)) { + reconcileKey(sandbox, real, key, action); + } + } + + private static void reconcileKey(SandboxedSessionMap sandbox, HttpSession real, String key, Object action) { + Object current = real.getAttribute(key); + Object snapshot = sandbox.getSnapshotValue(key); + Object background = sandbox.get(key); + if (Objects.equals(current, snapshot)) { + write(real, key, background); + } else if (action instanceof BackgroundSessionConflictAware aware) { + SessionConflict conflict = new SessionConflict(key, snapshot, current, background); + Object resolved = aware.resolveSessionConflict(conflict).resolve(conflict); + if (resolved != current) { + write(real, key, resolved); + } + } else { + LOG.debug("Background process [{}] keeps the current value of session key [{}] changed by another request", + sandbox.getProcessName(), key); + } + } + + private static int lastInvalidateIndex(List<SessionOperation> operations) { + for (int i = operations.size() - 1; i >= 0; i--) { + if (operations.get(i) instanceof SessionOperation.Invalidate) { + return i; + } + } + return -1; + } + + private static Set<String> touchedKeys(List<SessionOperation> operations) { + Set<String> keys = new LinkedHashSet<>(); + for (SessionOperation operation : operations) { + if (operation instanceof SessionOperation.Put put) { + keys.add(put.key()); + } else if (operation instanceof SessionOperation.Remove removal) { + keys.add(removal.key()); + } + } + return keys; + } + + private static void write(HttpSession session, String key, Object value) { + if (value == null) { + session.removeAttribute(key); + } else { + session.setAttribute(key, value); + } + } +} diff --git a/core/src/test/java/org/apache/struts2/interceptor/exec/SessionSandboxReconcilerTest.java b/core/src/test/java/org/apache/struts2/interceptor/exec/SessionSandboxReconcilerTest.java new file mode 100644 index 000000000..9d9dcd4d7 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/interceptor/exec/SessionSandboxReconcilerTest.java @@ -0,0 +1,181 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.interceptor.exec; + +import jakarta.servlet.http.HttpSession; +import junit.framework.TestCase; +import org.apache.struts2.action.BackgroundSessionConflictAware; +import org.apache.struts2.action.SessionConflict; +import org.apache.struts2.action.SessionConflictResolution; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpSession; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.function.Function; + +public class SessionSandboxReconcilerTest extends TestCase { + + private MockHttpSession live; + private MockHttpServletRequest request; + + @Override + protected void setUp() throws Exception { + super.setUp(); + live = new MockHttpSession(); + request = new MockHttpServletRequest(); + request.setSession(live); + } + + public void testOnlyLoggedKeysAreApplied() { + live.setAttribute("other", "1"); + SandboxedSessionMap sandbox = new SandboxedSessionMap(live, "p"); + live.setAttribute("other", "changed"); + sandbox.put("b", "2"); + + SessionSandboxReconciler.reconcile(sandbox, request, new Object()); + + assertEquals("changed", live.getAttribute("other")); + assertEquals("2", live.getAttribute("b")); + } + + public void testSessionIdChangeDiscardsEverything() { + SandboxedSessionMap sandbox = new SandboxedSessionMap(live, "p"); + sandbox.put("b", "2"); + live.invalidate(); + HttpSession fresh = request.getSession(true); + + SessionSandboxReconciler.reconcile(sandbox, request, new Object()); + + assertNull(fresh.getAttribute("b")); + } + + public void testNoSessionDiscardsEverything() { + SandboxedSessionMap sandbox = new SandboxedSessionMap(live, "p"); + sandbox.put("b", "2"); + live.invalidate(); + + SessionSandboxReconciler.reconcile(sandbox, request, new Object()); + + assertNull(request.getSession(false)); + } + + public void testConflictKeepsCurrentByDefault() { + SandboxedSessionMap sandbox = conflictOnA(); + + SessionSandboxReconciler.reconcile(sandbox, request, new Object()); + + assertEquals("live", live.getAttribute("a")); + } + + public void testHookUseBackground() { + SandboxedSessionMap sandbox = conflictOnA(); + + SessionSandboxReconciler.reconcile(sandbox, request, resolving(conflict -> SessionConflictResolution.useBackground())); + + assertEquals("bg", live.getAttribute("a")); + } + + public void testHookReplaceWith() { + SandboxedSessionMap sandbox = conflictOnA(); + + SessionSandboxReconciler.reconcile(sandbox, request, resolving(conflict -> SessionConflictResolution.replaceWith("merged"))); + + assertEquals("merged", live.getAttribute("a")); + } + + public void testHookRemove() { + SandboxedSessionMap sandbox = conflictOnA(); + + SessionSandboxReconciler.reconcile(sandbox, request, resolving(conflict -> SessionConflictResolution.remove())); + + assertNull(live.getAttribute("a")); + } + + public void testHookSeesAllFourValues() { + SandboxedSessionMap sandbox = conflictOnA(); + List<SessionConflict> seen = new ArrayList<>(); + + SessionSandboxReconciler.reconcile(sandbox, request, resolving(conflict -> { + seen.add(conflict); + return SessionConflictResolution.keepCurrent(); + })); + + assertEquals(List.of(new SessionConflict("a", "1", "live", "bg")), seen); + } + + public void testRemovalOfAKeyChangedElsewhereIsAConflict() { + live.setAttribute("a", "1"); + SandboxedSessionMap sandbox = new SandboxedSessionMap(live, "p"); + live.setAttribute("a", "live"); + sandbox.remove("a"); + + SessionSandboxReconciler.reconcile(sandbox, request, new Object()); + + assertEquals("live", live.getAttribute("a")); + } + + public void testBackgroundInvalidateStartsAFreshSession() { + live.setAttribute("a", "1"); + SandboxedSessionMap sandbox = new SandboxedSessionMap(live, "p"); + live.setAttribute("a", "live"); + sandbox.invalidate(); + sandbox.put("user", "u"); + List<SessionConflict> seen = new ArrayList<>(); + + SessionSandboxReconciler.reconcile(sandbox, request, resolving(conflict -> { + seen.add(conflict); + return SessionConflictResolution.keepCurrent(); + })); + + HttpSession fresh = request.getSession(false); + assertTrue(live.isInvalid()); + assertNotSame(live, fresh); + assertEquals(List.of("user"), Collections.list(fresh.getAttributeNames())); + assertEquals("u", fresh.getAttribute("user")); + assertEquals(List.of(), seen); + } + + public void testHookExceptionPropagates() { + SandboxedSessionMap sandbox = conflictOnA(); + IllegalArgumentException thrown = new IllegalArgumentException("boom"); + + try { + SessionSandboxReconciler.reconcile(sandbox, request, resolving(conflict -> { + throw thrown; + })); + fail("expected the hook's exception"); + } catch (IllegalArgumentException e) { + assertSame(thrown, e); + } + } + + private SandboxedSessionMap conflictOnA() { + live.setAttribute("a", "1"); + SandboxedSessionMap sandbox = new SandboxedSessionMap(live, "p"); + live.setAttribute("a", "live"); + sandbox.put("a", "bg"); + return sandbox; + } + + private static BackgroundSessionConflictAware resolving(Function<SessionConflict, SessionConflictResolution> resolver) { + return resolver::apply; + } +}
