From f3b3b0fe8aba19f4ce5b04da0cc6571b12a07529 Mon Sep 17 00:00:00 2001 From: Marco Collovati Date: Mon, 25 May 2026 16:23:27 +0200 Subject: [PATCH 1/2] refactor: extract `@VaadinSessionScoped` activation into managed bean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the activation predicate and storage-access logic out of `VaadinSessionScopedContext` into a nested `ContextualStorageManager` managed bean. The context becomes a thin delegator. As a side effect, applications may replace the bean via CDI `@Specializes` to adjust behavior — though the manager is intentionally undocumented and overriding it is at the integrator's own risk. Adds integration tests covering the strict default and the lenient `@Specializes` scenario from a background thread that does not hold the session lock. References #506 --- .../LenientSessionContextManager.java | 59 +++++ .../SessionContextSpecializesView.java | 126 +++++++++++ .../itest/SessionContextSpecializesTest.java | 76 +++++++ .../cdi/itest/SessionContextStrictTest.java | 80 +++++++ .../java/com/vaadin/cdi/VaadinExtension.java | 6 +- .../context/VaadinSessionScopedContext.java | 88 ++++++-- .../cdi/context/SessionContextTest.java | 207 +++++++++++++++++- 7 files changed, 612 insertions(+), 30 deletions(-) create mode 100644 vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/LenientSessionContextManager.java create mode 100644 vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/SessionContextSpecializesView.java create mode 100644 vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextSpecializesTest.java create mode 100644 vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextStrictTest.java diff --git a/vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/LenientSessionContextManager.java b/vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/LenientSessionContextManager.java new file mode 100644 index 00000000..ea71e138 --- /dev/null +++ b/vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/LenientSessionContextManager.java @@ -0,0 +1,59 @@ +/* + * Copyright 2000-2018 Vaadin Ltd. + * + * Licensed 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 com.vaadin.cdi.itest.sessioncontextspecializes; + +import java.util.concurrent.atomic.AtomicReference; + +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.context.spi.Contextual; +import jakarta.enterprise.inject.Specializes; + +import com.vaadin.cdi.context.VaadinSessionScopedContext; +import com.vaadin.cdi.util.ContextualStorage; +import com.vaadin.flow.server.VaadinSession; + +/** + * Replaces the framework-default + * {@link VaadinSessionScopedContext.ContextualStorageManager} via + * {@code @Specializes} to activate the {@code @VaadinSessionScoped} context + * whenever a {@link VaadinSession} is set on the current thread, and to + * acquire the session lock around storage access when the calling thread + * does not already hold it. + */ +@ApplicationScoped +@Specializes +public class LenientSessionContextManager + extends VaadinSessionScopedContext.ContextualStorageManager { + + @Override + protected boolean isActive() { + return VaadinSession.getCurrent() != null; + } + + @Override + protected ContextualStorage getContextualStorage(Contextual contextual, + boolean createIfNotExist) { + VaadinSession session = VaadinSession.getCurrent(); + if (session.hasLock()) { + return super.getContextualStorage(contextual, createIfNotExist); + } + AtomicReference ref = new AtomicReference<>(); + session.accessSynchronously(() -> ref + .set(super.getContextualStorage(contextual, createIfNotExist))); + return ref.get(); + } +} diff --git a/vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/SessionContextSpecializesView.java b/vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/SessionContextSpecializesView.java new file mode 100644 index 00000000..88bfd946 --- /dev/null +++ b/vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/SessionContextSpecializesView.java @@ -0,0 +1,126 @@ +/* + * Copyright 2000-2018 Vaadin Ltd. + * + * Licensed 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 com.vaadin.cdi.itest.sessioncontextspecializes; + +import jakarta.annotation.PostConstruct; +import jakarta.enterprise.context.ContextNotActiveException; +import jakarta.enterprise.event.Event; +import jakarta.enterprise.event.Observes; +import jakarta.inject.Inject; + +import com.vaadin.cdi.annotation.CdiComponent; +import com.vaadin.cdi.annotation.VaadinSessionScoped; +import com.vaadin.cdi.itest.Counter; +import com.vaadin.flow.component.html.Div; +import com.vaadin.flow.component.html.NativeButton; +import com.vaadin.flow.router.Route; +import com.vaadin.flow.server.VaadinSession; + +/** + * Background-thread probe used by both strict-default and {@code @Specializes} + * integration tests. From a thread that has only set the + * {@link VaadinSession} thread-local — without acquiring the session lock — + * the view calls a method on a {@code @VaadinSessionScoped} bean and fires a + * CDI event observed by the same bean. + *

+ * Outcomes are recorded via the application-scoped {@link Counter} so that + * failures on the session-scoped proxy don't lose information: + * {@link #DIRECT_CALL_COUNT} is incremented if the proxy call succeeds, + * {@link #OBSERVED_COUNT} if the observer is invoked, and + * {@link #ERROR_COUNT} for each {@link RuntimeException} caught by the + * background thread. + */ +@Route("") +@CdiComponent +public class SessionContextSpecializesView extends Div { + + public static final String FIREBTN_ID = "firebtn"; + public static final String OBSERVED_COUNT = "specializesObserved"; + public static final String DIRECT_CALL_COUNT = "specializesDirectCall"; + public static final String ERROR_COUNT = "specializesError"; + public static final String UNEXPECTED_ERROR_COUNT = "specializesUnexpectedError"; + + @Inject + private SessionScopedObserver observer; + + @Inject + private Event eventBus; + + @Inject + private Counter counter; + + @PostConstruct + private void init() { + NativeButton fireBtn = new NativeButton("fire-background"); + fireBtn.addClickListener(click -> { + VaadinSession session = VaadinSession.getCurrent(); + Thread thread = new Thread(() -> { + VaadinSession previous = VaadinSession.getCurrent(); + VaadinSession.setCurrent(session); + try { + try { + // Touch a @VaadinSessionScoped bean directly from a + // background thread that does NOT hold the session + // lock. + observer.recordDirectCall(); + } catch (ContextNotActiveException e) { + counter.increment(ERROR_COUNT); + } catch (RuntimeException e) { + counter.increment(UNEXPECTED_ERROR_COUNT); + } + try { + // Fire a CDI event; the observer is on a + // @VaadinSessionScoped bean. + eventBus.fire(new BackgroundEvent()); + } catch (ContextNotActiveException e) { + counter.increment(ERROR_COUNT); + } catch (RuntimeException e) { + counter.increment(UNEXPECTED_ERROR_COUNT); + } + } finally { + VaadinSession.setCurrent(previous); + } + }, "background-event-fire"); + thread.start(); + try { + thread.join(5000); + } catch (InterruptedException ignored) { + Thread.currentThread().interrupt(); + } + }); + fireBtn.setId(FIREBTN_ID); + add(fireBtn); + } + + public static class BackgroundEvent { + } + + @VaadinSessionScoped + public static class SessionScopedObserver { + + @Inject + private Counter counter; + + public void recordDirectCall() { + counter.increment(DIRECT_CALL_COUNT); + } + + private void onBackgroundEvent(@Observes BackgroundEvent event) { + counter.increment(OBSERVED_COUNT); + } + } +} diff --git a/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextSpecializesTest.java b/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextSpecializesTest.java new file mode 100644 index 00000000..ce864a28 --- /dev/null +++ b/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextSpecializesTest.java @@ -0,0 +1,76 @@ +/* + * Copyright 2000-2018 Vaadin Ltd. + * + * Licensed 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 com.vaadin.cdi.itest; + +import java.io.IOException; + +import com.vaadin.cdi.itest.sessioncontextspecializes.LenientSessionContextManager; +import com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView; +import org.jboss.arquillian.container.test.api.Deployment; +import org.jboss.shrinkwrap.api.spec.WebArchive; +import org.junit.Before; +import org.junit.Test; + +import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.DIRECT_CALL_COUNT; +import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.ERROR_COUNT; +import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.FIREBTN_ID; +import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.OBSERVED_COUNT; +import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.UNEXPECTED_ERROR_COUNT; + +/** + * Verifies that an application can take {@code @VaadinSessionScoped} beans + * into use from a background thread that only sets the + * {@link com.vaadin.flow.server.VaadinSession} thread-local — without + * holding the session lock — by providing a {@code @Specializes} lenient + * {@link com.vaadin.cdi.context.VaadinSessionScopedContext.ContextualStorageManager}. + *

+ * Reproduces the user-facing scenarios from issues #495 and #506. + */ +public class SessionContextSpecializesTest extends AbstractCdiTest { + + @Deployment(testable = false) + public static WebArchive deployment() { + return ArchiveProvider.createWebArchive( + "session-context-specializes", + SessionContextSpecializesView.class, + SessionContextSpecializesView.BackgroundEvent.class, + SessionContextSpecializesView.SessionScopedObserver.class, + LenientSessionContextManager.class); + } + + @Before + public void setUp() throws Exception { + resetCounts(); + open(); + } + + @Test + public void backgroundThread_firesEventAndCallsBean_specializesAllowsIt() + throws IOException { + assertCountEquals(0, OBSERVED_COUNT); + assertCountEquals(0, DIRECT_CALL_COUNT); + assertCountEquals(0, ERROR_COUNT); + assertCountEquals(0, UNEXPECTED_ERROR_COUNT); + + click(FIREBTN_ID); + + assertCountEquals(1, DIRECT_CALL_COUNT); + assertCountEquals(1, OBSERVED_COUNT); + assertCountEquals(0, ERROR_COUNT); + assertCountEquals(0, UNEXPECTED_ERROR_COUNT); + } +} diff --git a/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextStrictTest.java b/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextStrictTest.java new file mode 100644 index 00000000..b2f64ff6 --- /dev/null +++ b/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextStrictTest.java @@ -0,0 +1,80 @@ +/* + * Copyright 2000-2018 Vaadin Ltd. + * + * Licensed 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 com.vaadin.cdi.itest; + +import java.io.IOException; + +import com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView; +import org.jboss.arquillian.container.test.api.Deployment; +import org.jboss.shrinkwrap.api.spec.WebArchive; +import org.junit.Before; +import org.junit.Test; + +import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.DIRECT_CALL_COUNT; +import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.ERROR_COUNT; +import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.FIREBTN_ID; +import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.OBSERVED_COUNT; +import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.UNEXPECTED_ERROR_COUNT; +import static org.junit.Assert.assertTrue; + +/** + * Verifies the framework's supported activation condition for + * {@code @VaadinSessionScoped}: when the current thread has set the + * {@link com.vaadin.flow.server.VaadinSession} thread-local but does not hold + * the session lock, the context is inactive — bean lookups fail and observer + * methods on session-scoped beans are not invoked. Regression guard for the + * lock check introduced in {@code VaadinSessionScopedContext.isActive()}. + */ +public class SessionContextStrictTest extends AbstractCdiTest { + + @Deployment(testable = false) + public static WebArchive deployment() { + return ArchiveProvider.createWebArchive( + "session-context-strict", + SessionContextSpecializesView.class, + SessionContextSpecializesView.BackgroundEvent.class, + SessionContextSpecializesView.SessionScopedObserver.class); + } + + @Before + public void setUp() throws Exception { + resetCounts(); + open(); + } + + @Test + public void backgroundThread_withoutSpecializes_contextInactive() + throws IOException { + assertCountEquals(0, OBSERVED_COUNT); + assertCountEquals(0, DIRECT_CALL_COUNT); + assertCountEquals(0, ERROR_COUNT); + assertCountEquals(0, UNEXPECTED_ERROR_COUNT); + + click(FIREBTN_ID); + + // Neither the proxy call nor the observer dispatch succeeded: + assertCountEquals(0, DIRECT_CALL_COUNT); + assertCountEquals(0, OBSERVED_COUNT); + // At least one of the two attempts threw ContextNotActiveException + // because the context is inactive on an unlocked thread. + assertTrue("Expected at least one ContextNotActiveException, got " + + getCount(ERROR_COUNT), getCount(ERROR_COUNT) >= 1); + // Any other RuntimeException would indicate something went wrong + // unrelated to the inactive context — guard against false positives. + assertCountEquals(0, UNEXPECTED_ERROR_COUNT); + } +} diff --git a/vaadin-cdi/src/main/java/com/vaadin/cdi/VaadinExtension.java b/vaadin-cdi/src/main/java/com/vaadin/cdi/VaadinExtension.java index 1b6d8f24..a9e59179 100644 --- a/vaadin-cdi/src/main/java/com/vaadin/cdi/VaadinExtension.java +++ b/vaadin-cdi/src/main/java/com/vaadin/cdi/VaadinExtension.java @@ -50,6 +50,7 @@ public class VaadinExtension implements Extension { private VaadinServiceScopedContext serviceScopedContext; + private VaadinSessionScopedContext sessionScopedContext; private UIScopedContext uiScopedContext; private RouteScopedContext routeScopedContext; private Set beanInfoSet = new HashSet<>(); @@ -62,11 +63,11 @@ private void storeBeanValidationInfo(@Observes ProcessBean processBean) { private void addContexts(@Observes AfterBeanDiscovery afterBeanDiscovery, BeanManager beanManager) { serviceScopedContext = new VaadinServiceScopedContext(beanManager); + sessionScopedContext = new VaadinSessionScopedContext(beanManager); uiScopedContext = new UIScopedContext(beanManager); routeScopedContext = new RouteScopedContext(beanManager); addContext(afterBeanDiscovery, serviceScopedContext, null); - addContext(afterBeanDiscovery, - new VaadinSessionScopedContext(beanManager), null); + addContext(afterBeanDiscovery, sessionScopedContext, null); addContext(afterBeanDiscovery, uiScopedContext, NormalUIScoped.class); addContext(afterBeanDiscovery, routeScopedContext, NormalRouteScoped.class); @@ -77,6 +78,7 @@ private void addContexts(@Observes AfterBeanDiscovery afterBeanDiscovery, private void initializeContexts(@Observes AfterDeploymentValidation adv, BeanManager beanManager) { serviceScopedContext.init(beanManager); + sessionScopedContext.init(beanManager); uiScopedContext.init(beanManager); routeScopedContext.init(beanManager, uiScopedContext::isActive); } diff --git a/vaadin-cdi/src/main/java/com/vaadin/cdi/context/VaadinSessionScopedContext.java b/vaadin-cdi/src/main/java/com/vaadin/cdi/context/VaadinSessionScopedContext.java index 564159e1..ee154d3a 100644 --- a/vaadin-cdi/src/main/java/com/vaadin/cdi/context/VaadinSessionScopedContext.java +++ b/vaadin-cdi/src/main/java/com/vaadin/cdi/context/VaadinSessionScopedContext.java @@ -15,13 +15,16 @@ */ package com.vaadin.cdi.context; +import jakarta.enterprise.context.ApplicationScoped; import jakarta.enterprise.context.spi.Contextual; import jakarta.enterprise.inject.spi.BeanManager; +import jakarta.inject.Inject; import java.lang.annotation.Annotation; import com.vaadin.cdi.annotation.VaadinSessionScoped; import com.vaadin.cdi.util.AbstractContext; +import com.vaadin.cdi.util.BeanProvider; import com.vaadin.cdi.util.ContextUtils; import com.vaadin.cdi.util.ContextualStorage; import com.vaadin.flow.server.VaadinSession; @@ -31,35 +34,35 @@ *

* Stores contextuals in {@link VaadinSession}. Other Vaadin CDI contexts are * stored in the corresponding {@link VaadinSessionScoped} context. + *

+ * The context is active when the current {@link VaadinSession} is bound to the + * calling thread and that thread holds the session lock — i.e. when the code + * runs inside an active Vaadin request, a {@code UI#access} block, or a + * {@code VaadinSession#access} block. Code running on a background thread that + * has only set {@link VaadinSession#setCurrent(VaadinSession)} without + * acquiring the session lock is operating outside the supported usage of the + * framework; behavior in that case is not guaranteed. * * @since 3.0 */ public class VaadinSessionScopedContext extends AbstractContext { - private final BeanManager beanManager; - private static final String ATTRIBUTE_NAME = VaadinSessionScopedContext.class - .getName(); + + private ContextualStorageManager contextManager; public VaadinSessionScopedContext(BeanManager beanManager) { super(beanManager); - this.beanManager = beanManager; + } + + public void init(BeanManager beanManager) { + contextManager = BeanProvider.getContextualReference(beanManager, + ContextualStorageManager.class, false); } @Override protected ContextualStorage getContextualStorage(Contextual contextual, boolean createIfNotExist) { - VaadinSession session = VaadinSession.getCurrent(); - ContextualStorage storage = findContextualStorage(session); - if (storage == null && createIfNotExist) { - storage = new ContextualStorage(beanManager, false, true); - session.setAttribute(ATTRIBUTE_NAME, storage); - } - return storage; - } - - private static ContextualStorage findContextualStorage( - VaadinSession session) { - // session lock is checked inside - return (ContextualStorage) session.getAttribute(ATTRIBUTE_NAME); + return contextManager.getContextualStorage(contextual, + createIfNotExist); } @Override @@ -69,15 +72,11 @@ public Class getScope() { @Override public boolean isActive() { - VaadinSession session = VaadinSession.getCurrent(); - return session != null && session.hasLock(); + return contextManager != null && contextManager.isActive(); } public static void destroy(VaadinSession session) { - ContextualStorage storage = findContextualStorage(session); - if (storage != null) { - AbstractContext.destroyAllActive(storage); - } + ContextualStorageManager.destroy(session); } /** @@ -97,4 +96,47 @@ public static boolean guessContextIsUndeployed() { && !ContextUtils.isContextActive(VaadinSessionScoped.class)); } + /** + * For internal use only. + */ + @ApplicationScoped + public static class ContextualStorageManager { + + protected static final String ATTRIBUTE_NAME = VaadinSessionScopedContext.class + .getName(); + + @Inject + private BeanManager beanManager; + + protected boolean isActive() { + VaadinSession session = VaadinSession.getCurrent(); + return session != null && session.hasLock(); + } + + protected ContextualStorage getContextualStorage( + Contextual contextual, boolean createIfNotExist) { + VaadinSession session = VaadinSession.getCurrent(); + ContextualStorage storage = findContextualStorage(session); + if (storage == null && createIfNotExist) { + storage = new ContextualStorage(beanManager, false, true); + session.setAttribute(ATTRIBUTE_NAME, storage); + } + return storage; + } + + private static ContextualStorage findContextualStorage( + VaadinSession session) { + // session lock is checked inside + return (ContextualStorage) session.getAttribute(ATTRIBUTE_NAME); + } + + private static void destroy(VaadinSession session) { + ContextualStorage storage = findContextualStorage(session); + if (storage != null) { + AbstractContext.destroyAllActive(storage); + } + } + + } + } diff --git a/vaadin-cdi/src/test/java/com/vaadin/cdi/context/SessionContextTest.java b/vaadin-cdi/src/test/java/com/vaadin/cdi/context/SessionContextTest.java index 511fb05c..0ecf176c 100644 --- a/vaadin-cdi/src/test/java/com/vaadin/cdi/context/SessionContextTest.java +++ b/vaadin-cdi/src/test/java/com/vaadin/cdi/context/SessionContextTest.java @@ -16,15 +16,31 @@ package com.vaadin.cdi.context; import jakarta.enterprise.context.ContextNotActiveException; +import jakarta.enterprise.context.spi.Contextual; +import jakarta.enterprise.inject.spi.BeanManager; + +import java.lang.reflect.Field; +import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; import com.vaadin.cdi.annotation.VaadinSessionScoped; +import com.vaadin.cdi.context.VaadinSessionScopedContext.ContextualStorageManager; import com.vaadin.cdi.util.BeanProvider; +import com.vaadin.cdi.util.ContextualStorage; +import com.vaadin.flow.server.Command; import com.vaadin.flow.server.VaadinSession; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class SessionContextTest @@ -53,10 +69,7 @@ public void get_sessionExistsButNotLocked_contextNotActive() { VaadinSession session = context.getSession(); when(session.hasLock()).thenReturn(false); - VaadinSessionScopedContext sessionContext = new VaadinSessionScopedContext( - weld.select() - .select(jakarta.enterprise.inject.spi.BeanManager.class) - .get()); + VaadinSessionScopedContext sessionContext = newContext(); assertFalse(sessionContext.isActive()); @@ -67,8 +80,192 @@ public void get_sessionExistsButNotLocked_contextNotActive() { }); } + @Test + public void defaultManager_sessionLocked_isActive() { + SessionUnderTestContext context = new SessionUnderTestContext(); + context.activate(); + VaadinSession session = context.getSession(); + when(session.hasLock()).thenReturn(true); + + ContextualStorageManager manager = lookupManager(); + assertTrue(manager.isActive()); + } + + @Test + public void context_nullSession_notActive() { + VaadinSession.setCurrent(null); + VaadinSessionScopedContext sessionContext = newContext(); + assertFalse(sessionContext.isActive()); + } + + @Test + public void defaultManager_storageAccess_returnsStorage() { + SessionUnderTestContext context = new SessionUnderTestContext(); + context.activate(); + VaadinSession session = context.getSession(); + when(session.hasLock()).thenReturn(true); + + VaadinSessionScopedContext sessionContext = newContext(); + ContextualStorage storage = sessionContext.getContextualStorage(null, + true); + assertNotNull(storage); + verify(session, never()).accessSynchronously(any(Command.class)); + } + + @Test + public void lenientSubclass_isActiveWithoutLock() { + SessionUnderTestContext context = new SessionUnderTestContext(); + context.activate(); + VaadinSession session = context.getSession(); + when(session.hasLock()).thenReturn(false); + + ContextualStorageManager lenient = injectBeanManager( + new ContextualStorageManager() { + @Override + protected boolean isActive() { + return VaadinSession.getCurrent() != null; + } + }); + + VaadinSessionScopedContext sessionContext = newContext(lenient); + assertTrue(sessionContext.isActive()); + } + + @Test + public void lenientSubclass_storageAccess_wrappedInAccessSynchronously() { + SessionUnderTestContext context = new SessionUnderTestContext(); + context.activate(); + VaadinSession session = context.getSession(); + when(session.hasLock()).thenReturn(false); + doAnswer(inv -> { + ((Command) inv.getArgument(0)).execute(); + return null; + }).when(session).accessSynchronously(any(Command.class)); + + ContextualStorageManager lenient = injectBeanManager( + new ContextualStorageManager() { + @Override + protected boolean isActive() { + return VaadinSession.getCurrent() != null; + } + + @Override + protected ContextualStorage getContextualStorage( + Contextual c, boolean create) { + VaadinSession s = VaadinSession.getCurrent(); + if (s.hasLock()) { + return super.getContextualStorage(c, create); + } + AtomicReference ref = new AtomicReference<>(); + s.accessSynchronously(() -> ref + .set(super.getContextualStorage(c, create))); + return ref.get(); + } + }); + + VaadinSessionScopedContext sessionContext = newContext(lenient); + ContextualStorage storage = sessionContext.getContextualStorage(null, + true); + + assertNotNull(storage); + verify(session).accessSynchronously(any(Command.class)); + } + + @Test + public void lenientSubclass_storageAccess_lockHeldFastPath() { + SessionUnderTestContext context = new SessionUnderTestContext(); + context.activate(); + VaadinSession session = context.getSession(); + when(session.hasLock()).thenReturn(true); + + ContextualStorageManager lenient = injectBeanManager( + new ContextualStorageManager() { + @Override + protected boolean isActive() { + return VaadinSession.getCurrent() != null; + } + + @Override + protected ContextualStorage getContextualStorage( + Contextual c, boolean create) { + VaadinSession s = VaadinSession.getCurrent(); + if (s.hasLock()) { + return super.getContextualStorage(c, create); + } + AtomicReference ref = new AtomicReference<>(); + s.accessSynchronously(() -> ref + .set(super.getContextualStorage(c, create))); + return ref.get(); + } + }); + + VaadinSessionScopedContext sessionContext = newContext(lenient); + ContextualStorage storage = sessionContext.getContextualStorage(null, + true); + + assertNotNull(storage); + // With the lock held, lenient strategy avoids the extra wrap. + verify(session, never()).accessSynchronously(any(Command.class)); + // A second lookup must return the same storage instance. + assertSame(storage, sessionContext.getContextualStorage(null, false)); + } + + @Test + public void contextIsActive_whenInitNotCalled_returnsFalse() { + // Guards against NPE if isActive() is somehow invoked before + // VaadinExtension.initializeContexts has run. + SessionUnderTestContext context = new SessionUnderTestContext(); + context.activate(); + BeanManager beanManager = weld.select().select(BeanManager.class).get(); + VaadinSessionScopedContext bare = new VaadinSessionScopedContext( + beanManager); + assertFalse(bare.isActive()); + } + + private VaadinSessionScopedContext newContext() { + BeanManager beanManager = weld.select().select(BeanManager.class).get(); + VaadinSessionScopedContext sessionContext = new VaadinSessionScopedContext( + beanManager); + sessionContext.init(beanManager); + return sessionContext; + } + + private VaadinSessionScopedContext newContext( + ContextualStorageManager manager) { + BeanManager beanManager = weld.select().select(BeanManager.class).get(); + VaadinSessionScopedContext sessionContext = new VaadinSessionScopedContext( + beanManager); + try { + Field f = VaadinSessionScopedContext.class + .getDeclaredField("contextManager"); + f.setAccessible(true); + f.set(sessionContext, manager); + } catch (ReflectiveOperationException e) { + throw new AssertionError("Failed to install test manager", e); + } + return sessionContext; + } + + private ContextualStorageManager lookupManager() { + return BeanProvider.getContextualReference( + weld.select().select(BeanManager.class).get(), + ContextualStorageManager.class, false); + } + + private static M injectBeanManager( + M target) { + try { + Field f = ContextualStorageManager.class + .getDeclaredField("beanManager"); + f.setAccessible(true); + f.set(target, Mockito.mock(BeanManager.class)); + return target; + } catch (ReflectiveOperationException e) { + throw new AssertionError("Failed to inject BeanManager", e); + } + } + @VaadinSessionScoped public static class SessionScopedTestBean extends TestBean { } - } From 634cdd4f2f7eeb4fe3aac3dd6dfbbbd71ad7cbc1 Mon Sep 17 00:00:00 2001 From: Marco Collovati Date: Wed, 3 Jun 2026 08:47:36 +0300 Subject: [PATCH 2/2] test: remove background-thread join from session context itests `SessionContextSpecializesView` joined the spawned background thread with a 5 s timeout: in the lenient `@Specializes` deployment the thread waits in `VaadinSession#accessSynchronously` for the session lock held by the joining request thread, so an unbounded join would deadlock. Every click stalled the full 5 s and the assertions raced the still-running thread. The click listener now only starts the thread, removing the lock inversion entirely. The thread increments a completion counter in its `finally` block, and the tests synchronize on it via a new `waitForCount()` helper before asserting. --- .../LenientSessionContextManager.java | 13 +++++----- .../SessionContextSpecializesView.java | 26 +++++++++---------- .../com/vaadin/cdi/itest/AbstractCdiTest.java | 11 ++++++++ .../itest/SessionContextSpecializesTest.java | 22 +++++++++------- .../cdi/itest/SessionContextStrictTest.java | 13 +++++++--- 5 files changed, 52 insertions(+), 33 deletions(-) diff --git a/vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/LenientSessionContextManager.java b/vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/LenientSessionContextManager.java index ea71e138..ac14dd93 100644 --- a/vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/LenientSessionContextManager.java +++ b/vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/LenientSessionContextManager.java @@ -13,15 +13,14 @@ * License for the specific language governing permissions and limitations under * the License. */ - package com.vaadin.cdi.itest.sessioncontextspecializes; -import java.util.concurrent.atomic.AtomicReference; - import jakarta.enterprise.context.ApplicationScoped; import jakarta.enterprise.context.spi.Contextual; import jakarta.enterprise.inject.Specializes; +import java.util.concurrent.atomic.AtomicReference; + import com.vaadin.cdi.context.VaadinSessionScopedContext; import com.vaadin.cdi.util.ContextualStorage; import com.vaadin.flow.server.VaadinSession; @@ -30,9 +29,9 @@ * Replaces the framework-default * {@link VaadinSessionScopedContext.ContextualStorageManager} via * {@code @Specializes} to activate the {@code @VaadinSessionScoped} context - * whenever a {@link VaadinSession} is set on the current thread, and to - * acquire the session lock around storage access when the calling thread - * does not already hold it. + * whenever a {@link VaadinSession} is set on the current thread, and to acquire + * the session lock around storage access when the calling thread does not + * already hold it. */ @ApplicationScoped @Specializes @@ -46,7 +45,7 @@ protected boolean isActive() { @Override protected ContextualStorage getContextualStorage(Contextual contextual, - boolean createIfNotExist) { + boolean createIfNotExist) { VaadinSession session = VaadinSession.getCurrent(); if (session.hasLock()) { return super.getContextualStorage(contextual, createIfNotExist); diff --git a/vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/SessionContextSpecializesView.java b/vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/SessionContextSpecializesView.java index 88bfd946..7bdf8827 100644 --- a/vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/SessionContextSpecializesView.java +++ b/vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/SessionContextSpecializesView.java @@ -13,7 +13,6 @@ * License for the specific language governing permissions and limitations under * the License. */ - package com.vaadin.cdi.itest.sessioncontextspecializes; import jakarta.annotation.PostConstruct; @@ -32,17 +31,19 @@ /** * Background-thread probe used by both strict-default and {@code @Specializes} - * integration tests. From a thread that has only set the - * {@link VaadinSession} thread-local — without acquiring the session lock — - * the view calls a method on a {@code @VaadinSessionScoped} bean and fires a - * CDI event observed by the same bean. + * integration tests. From a thread that has only set the {@link VaadinSession} + * thread-local — without acquiring the session lock — the view calls a method + * on a {@code @VaadinSessionScoped} bean and fires a CDI event observed by the + * same bean. *

* Outcomes are recorded via the application-scoped {@link Counter} so that * failures on the session-scoped proxy don't lose information: * {@link #DIRECT_CALL_COUNT} is incremented if the proxy call succeeds, - * {@link #OBSERVED_COUNT} if the observer is invoked, and - * {@link #ERROR_COUNT} for each {@link RuntimeException} caught by the - * background thread. + * {@link #OBSERVED_COUNT} if the observer is invoked, and {@link #ERROR_COUNT} + * for each {@link RuntimeException} caught by the background thread. + * {@link #DONE_COUNT} is incremented when the background thread finishes, + * regardless of outcome — tests synchronize on it before asserting the other + * counters. */ @Route("") @CdiComponent @@ -53,6 +54,7 @@ public class SessionContextSpecializesView extends Div { public static final String DIRECT_CALL_COUNT = "specializesDirectCall"; public static final String ERROR_COUNT = "specializesError"; public static final String UNEXPECTED_ERROR_COUNT = "specializesUnexpectedError"; + public static final String DONE_COUNT = "specializesBackgroundDone"; @Inject private SessionScopedObserver observer; @@ -93,14 +95,12 @@ private void init() { } } finally { VaadinSession.setCurrent(previous); + // Completion signal: tests wait for this counter before + // asserting the other ones. + counter.increment(DONE_COUNT); } }, "background-event-fire"); thread.start(); - try { - thread.join(5000); - } catch (InterruptedException ignored) { - Thread.currentThread().interrupt(); - } }); fireBtn.setId(FIREBTN_ID); add(fireBtn); diff --git a/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/AbstractCdiTest.java b/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/AbstractCdiTest.java index 9f9afced..43b99784 100644 --- a/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/AbstractCdiTest.java +++ b/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/AbstractCdiTest.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.io.UncheckedIOException; import java.net.URL; import org.jboss.arquillian.container.test.api.RunAsClient; @@ -76,6 +77,16 @@ protected void assertCountEquals(int expectedCount, String counter) Assert.assertEquals(expectedCount, getCount(counter)); } + protected void waitForCount(int expectedCount, String counter) { + waitUntil(driver -> { + try { + return getCount(counter) == expectedCount; + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }, 10); + } + protected void assertTextEquals(String expectedText, String elementId) { Assert.assertEquals(expectedText, getText(elementId)); } diff --git a/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextSpecializesTest.java b/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextSpecializesTest.java index ce864a28..1edc7053 100644 --- a/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextSpecializesTest.java +++ b/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextSpecializesTest.java @@ -13,29 +13,30 @@ * License for the specific language governing permissions and limitations under * the License. */ - package com.vaadin.cdi.itest; import java.io.IOException; -import com.vaadin.cdi.itest.sessioncontextspecializes.LenientSessionContextManager; -import com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView; import org.jboss.arquillian.container.test.api.Deployment; import org.jboss.shrinkwrap.api.spec.WebArchive; import org.junit.Before; import org.junit.Test; +import com.vaadin.cdi.itest.sessioncontextspecializes.LenientSessionContextManager; +import com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView; + import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.DIRECT_CALL_COUNT; +import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.DONE_COUNT; import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.ERROR_COUNT; import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.FIREBTN_ID; import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.OBSERVED_COUNT; import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.UNEXPECTED_ERROR_COUNT; /** - * Verifies that an application can take {@code @VaadinSessionScoped} beans - * into use from a background thread that only sets the - * {@link com.vaadin.flow.server.VaadinSession} thread-local — without - * holding the session lock — by providing a {@code @Specializes} lenient + * Verifies that an application can take {@code @VaadinSessionScoped} beans into + * use from a background thread that only sets the + * {@link com.vaadin.flow.server.VaadinSession} thread-local — without holding + * the session lock — by providing a {@code @Specializes} lenient * {@link com.vaadin.cdi.context.VaadinSessionScopedContext.ContextualStorageManager}. *

* Reproduces the user-facing scenarios from issues #495 and #506. @@ -44,8 +45,7 @@ public class SessionContextSpecializesTest extends AbstractCdiTest { @Deployment(testable = false) public static WebArchive deployment() { - return ArchiveProvider.createWebArchive( - "session-context-specializes", + return ArchiveProvider.createWebArchive("session-context-specializes", SessionContextSpecializesView.class, SessionContextSpecializesView.BackgroundEvent.class, SessionContextSpecializesView.SessionScopedObserver.class, @@ -68,6 +68,10 @@ public void backgroundThread_firesEventAndCallsBean_specializesAllowsIt() click(FIREBTN_ID); + // The background thread updates the counters asynchronously; wait + // for its completion signal before asserting. + waitForCount(1, DONE_COUNT); + assertCountEquals(1, DIRECT_CALL_COUNT); assertCountEquals(1, OBSERVED_COUNT); assertCountEquals(0, ERROR_COUNT); diff --git a/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextStrictTest.java b/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextStrictTest.java index b2f64ff6..b672b626 100644 --- a/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextStrictTest.java +++ b/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextStrictTest.java @@ -13,18 +13,19 @@ * License for the specific language governing permissions and limitations under * the License. */ - package com.vaadin.cdi.itest; import java.io.IOException; -import com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView; import org.jboss.arquillian.container.test.api.Deployment; import org.jboss.shrinkwrap.api.spec.WebArchive; import org.junit.Before; import org.junit.Test; +import com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView; + import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.DIRECT_CALL_COUNT; +import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.DONE_COUNT; import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.ERROR_COUNT; import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.FIREBTN_ID; import static com.vaadin.cdi.itest.sessioncontextspecializes.SessionContextSpecializesView.OBSERVED_COUNT; @@ -43,8 +44,7 @@ public class SessionContextStrictTest extends AbstractCdiTest { @Deployment(testable = false) public static WebArchive deployment() { - return ArchiveProvider.createWebArchive( - "session-context-strict", + return ArchiveProvider.createWebArchive("session-context-strict", SessionContextSpecializesView.class, SessionContextSpecializesView.BackgroundEvent.class, SessionContextSpecializesView.SessionScopedObserver.class); @@ -66,6 +66,11 @@ public void backgroundThread_withoutSpecializes_contextInactive() click(FIREBTN_ID); + // The background thread updates the counters asynchronously; wait + // for its completion signal so the zero-assertions below are + // meaningful instead of passing on a not-yet-run thread. + waitForCount(1, DONE_COUNT); + // Neither the proxy call nor the observer dispatch succeeded: assertCountEquals(0, DIRECT_CALL_COUNT); assertCountEquals(0, OBSERVED_COUNT);