From 26818831de525bc4377d505d23a500af898ebcb7 Mon Sep 17 00:00:00 2001 From: Marco Collovati Date: Mon, 8 Jun 2026 16:30:32 +0300 Subject: [PATCH] refactor: extract `@VaadinSessionScoped` activation into managed bean (#515) 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 | 58 +++++ .../SessionContextSpecializesView.java | 126 ++++++++++ .../com/vaadin/cdi/itest/AbstractCdiTest.java | 11 + .../itest/SessionContextSpecializesTest.java | 80 +++++++ .../cdi/itest/SessionContextStrictTest.java | 85 +++++++ .../java/com/vaadin/cdi/VaadinExtension.java | 6 +- .../context/VaadinSessionScopedContext.java | 111 ++++++--- .../cdi/context/SessionContextTest.java | 223 +++++++++++++++++- 8 files changed, 653 insertions(+), 47 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..ac14dd93 --- /dev/null +++ b/vaadin-cdi-itest/src/main/java/com/vaadin/cdi/itest/sessioncontextspecializes/LenientSessionContextManager.java @@ -0,0 +1,58 @@ +/* + * 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.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; + +/** + * 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..7bdf8827 --- /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. + * {@link #DONE_COUNT} is incremented when the background thread finishes, + * regardless of outcome — tests synchronize on it before asserting the other + * counters. + */ +@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"; + public static final String DONE_COUNT = "specializesBackgroundDone"; + + @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); + // Completion signal: tests wait for this counter before + // asserting the other ones. + counter.increment(DONE_COUNT); + } + }, "background-event-fire"); + thread.start(); + }); + 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/AbstractCdiTest.java b/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/AbstractCdiTest.java index 50d87677..0ca46eb5 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 @@ -20,6 +20,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; @@ -77,6 +78,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 new file mode 100644 index 00000000..1edc7053 --- /dev/null +++ b/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextSpecializesTest.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 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 + * {@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); + + // 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); + 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..b672b626 --- /dev/null +++ b/vaadin-cdi-itest/src/test/java/com/vaadin/cdi/itest/SessionContextStrictTest.java @@ -0,0 +1,85 @@ +/* + * 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 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; +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); + + // 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); + // 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 13004e28..4ab9d2d6 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 49b03ac5..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 @@ -13,51 +13,56 @@ * License for the specific language governing permissions and limitations under * the License. */ - package com.vaadin.cdi.context; -import java.lang.annotation.Annotation; - +import jakarta.enterprise.context.ApplicationScoped; import jakarta.enterprise.context.spi.Contextual; import jakarta.enterprise.inject.spi.BeanManager; -import com.vaadin.cdi.util.ContextUtils; -import com.vaadin.cdi.util.AbstractContext; -import com.vaadin.cdi.util.ContextualStorage; +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; /** * Context for {@link VaadinSessionScoped @VaadinSessionScoped} beans. *

- * Stores contextuals in {@link VaadinSession}. - * Other Vaadin CDI contexts are stored in the corresponding {@link VaadinSessionScoped} context. + * 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; } - @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; + public void init(BeanManager beanManager) { + contextManager = BeanProvider.getContextualReference(beanManager, + ContextualStorageManager.class, false); } - private static ContextualStorage findContextualStorage(VaadinSession session) { - // session lock is checked inside - return (ContextualStorage) session.getAttribute(ATTRIBUTE_NAME); + @Override + protected ContextualStorage getContextualStorage(Contextual contextual, + boolean createIfNotExist) { + return contextManager.getContextualStorage(contextual, + createIfNotExist); } @Override @@ -67,31 +72,71 @@ 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); } /** * Guess whether this context is undeployed. * - * Tomcat expires sessions after contexts are undeployed. - * Need this guess to prevent exceptions when try to - * properly destroy contexts on session expiration. + * Tomcat expires sessions after contexts are undeployed. Need this guess to + * prevent exceptions when try to properly destroy contexts on session + * expiration. * * @return true when context is not active, but sure it should */ public static boolean guessContextIsUndeployed() { - // Given there is a current VaadinSession, we should have an active context, + // Given there is a current VaadinSession, we should have an active + // context, // except we get here after the application is undeployed. return (VaadinSession.getCurrent() != null && !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 b53c3734..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 @@ -13,21 +13,38 @@ * License for the specific language governing permissions and limitations under * the License. */ - package com.vaadin.cdi.context; import jakarta.enterprise.context.ContextNotActiveException; -import org.junit.jupiter.api.Test; +import jakarta.enterprise.context.spi.Contextual; +import jakarta.enterprise.inject.spi.BeanManager; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.when; +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; -public class SessionContextTest extends AbstractContextTest { +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 + extends AbstractContextTest { @Override protected UnderTestContext newContextUnderTest() { @@ -52,21 +69,203 @@ 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()); assertThrows(ContextNotActiveException.class, () -> { - SessionScopedTestBean ref = - BeanProvider.getContextualReference(SessionScopedTestBean.class); + SessionScopedTestBean ref = BeanProvider + .getContextualReference(SessionScopedTestBean.class); ref.getState(); }); } + @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 { } - }