Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for issue #20092 #20103

Merged
merged 6 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;

import org.slf4j.Logger;
Expand Down Expand Up @@ -621,14 +622,21 @@ public Registration addUIInitListener(UIInitListener listener) {
/**
* Adds a listener that gets notified when a Vaadin service session that has
* been initialized for this service is destroyed.
*
* <p>
* The session being destroyed is locked and its UIs have been removed when
* the listeners are called.
*
* <p>
* This method delivers notifications for all associated sessions. To be
* notified for only one specific session, use
* {@link VaadinSession#addSessionDestroyListener}.
*
* @param listener
* the vaadin service session destroy listener
* @return a handle that can be used for removing the listener
* @see #addSessionInitListener(SessionInitListener)
* @see VaadinSession#addSessionDestroyListener
*/
public Registration addSessionDestroyListener(
SessionDestroyListener listener) {
Expand Down Expand Up @@ -684,18 +692,19 @@ public void fireSessionDestroy(VaadinSession vaadinSession) {
}
SessionDestroyEvent event = new SessionDestroyEvent(
VaadinService.this, session);
for (SessionDestroyListener listener : sessionDestroyListeners) {
try {
listener.sessionDestroy(event);
} catch (Exception e) {
/*
* for now, use the session error handler; in the future,
* could have an API for using some other handler for
* session init and destroy listeners
*/
session.getErrorHandler().error(new ErrorEvent(e));
}
}
Stream.concat(session.destroyListeners.stream(),
sessionDestroyListeners.stream()).forEach(listener -> {
try {
listener.sessionDestroy(event);
} catch (Exception e) {
/*
* for now, use the session error handler; in the
* future, could have an API for using some other
* handler for session init and destroy listeners
*/
session.getErrorHandler().error(new ErrorEvent(e));
}
});

session.setState(VaadinSessionState.CLOSED);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Queue;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.Future;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
Expand All @@ -47,6 +49,7 @@
import com.vaadin.flow.internal.CurrentInstance;
import com.vaadin.flow.internal.StateNode;
import com.vaadin.flow.server.startup.ApplicationConfiguration;
import com.vaadin.flow.shared.Registration;
import com.vaadin.flow.shared.communication.PushMode;

import jakarta.servlet.http.HttpSession;
Expand Down Expand Up @@ -78,6 +81,8 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {

volatile boolean sessionClosedExplicitly = false;

final List<SessionDestroyListener> destroyListeners = new CopyOnWriteArrayList<>();

/**
* Configuration for the session.
*/
Expand Down Expand Up @@ -489,6 +494,31 @@ public Collection<RequestHandler> getRequestHandlers() {
return Collections.unmodifiableCollection(requestHandlers);
}

/**
* Adds a listener that gets notified when this session is destroyed.
*
* <p>
* This session will be locked and its {@link UI}s will have been removed
* when the listener is called.
*
* <p>
* If this session is already closed, no notification is delivered.
*
* <p>
* This method only delivers notifications for this session. To also be
* notified about other sessions, use
* {@link VaadinService#addSessionDestroyListener}.
*
* @param listener
* the session destroy listener
* @return a handle that can be used for removing the listener
* @see VaadinService#addSessionDestroyListener
*/
public Registration addSessionDestroyListener(
SessionDestroyListener listener) {
return Registration.addAndRemove(destroyListeners, listener);
}

/**
* Gets the currently used session. The current session is automatically
* defined when processing requests to the server (see {@link ThreadLocal})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ public class SpringVaadinServletService extends VaadinServletService {

private final transient ApplicationContext context;

private final Registration serviceDestroyRegistration;

static final String SPRING_BOOT_WEBPROPERTIES_CLASS = "org.springframework.boot.autoconfigure.web.WebProperties";

/**
Expand All @@ -66,11 +64,6 @@ public SpringVaadinServletService(VaadinServlet servlet,
ApplicationContext context) {
super(servlet, deploymentConfiguration);
this.context = context;
SessionDestroyListener listener = event -> sessionDestroyed(
event.getSession());
Registration registration = addSessionDestroyListener(listener);
serviceDestroyRegistration = addServiceDestroyListener(
event -> serviceDestroyed(registration));
}

@Override
Expand Down Expand Up @@ -105,21 +98,13 @@ public void init() throws ServiceException {
uiInitListeners.values().forEach(this::addUIInitListener);
}

// This method should be removed when the deprecated class
// SpringVaadinSession is removed
@Override
protected VaadinSession createVaadinSession(VaadinRequest request) {
return new SpringVaadinSession(this);
}

private void sessionDestroyed(VaadinSession session) {
assert session instanceof SpringVaadinSession;
((SpringVaadinSession) session).fireSessionDestroy();
}

private void serviceDestroyed(Registration registration) {
registration.remove();
serviceDestroyRegistration.remove();
}

@Override
public URL getStaticResource(String path) {
URL resource = super.getStaticResource(path);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,27 +15,21 @@
*/
package com.vaadin.flow.spring;

import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;

import com.vaadin.flow.server.SessionDestroyEvent;
import com.vaadin.flow.server.SessionDestroyListener;
import com.vaadin.flow.server.SessionInitListener;
import com.vaadin.flow.server.VaadinService;
import com.vaadin.flow.server.VaadinSession;

/**
* Vaadin session implementation for Spring.
*
* @author Vaadin Ltd
*
* @deprecated No replacement planned
*/
@Deprecated(forRemoval = true)
public class SpringVaadinSession extends VaadinSession {

private final List<SessionDestroyListener> destroyListeners = new CopyOnWriteArrayList<>();

/**
* Creates a new VaadinSession tied to a VaadinService.
* Creates a new SpringVaadinSession tied to a VaadinService.
*
* @param service
* the Vaadin service for the new session
Expand All @@ -48,10 +42,7 @@ public SpringVaadinSession(VaadinService service) {
* Handles destruction of the session.
*/
public void fireSessionDestroy() {
SessionDestroyEvent event = new SessionDestroyEvent(getService(), this);
destroyListeners.stream()
.forEach(listener -> listener.sessionDestroy(event));
destroyListeners.clear();
getService().fireSessionDestroy(this);
}

/**
Expand All @@ -70,7 +61,7 @@ public void fireSessionDestroy() {
* the vaadin service session destroy listener
*/
public void addDestroyListener(SessionDestroyListener listener) {
destroyListeners.add(listener);
this.addSessionDestroyListener(listener);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
import com.vaadin.flow.server.VaadinServletContext;
import com.vaadin.flow.server.VaadinSession;
import com.vaadin.flow.shared.Registration;
import com.vaadin.flow.spring.SpringVaadinSession;
import com.vaadin.flow.spring.annotation.RouteScopeOwner;

/**
Expand All @@ -72,22 +71,13 @@ private static class RouteStoreWrapper implements Serializable {

private final VaadinSession session;

private final Registration sessionDestroyListenerRegistration;

private final Map<String, RouteBeanStore> routeStores;

private RouteStoreWrapper(VaadinSession session) {
assert session.hasLock();
this.session = session;
session.addSessionDestroyListener(event -> destroy());
routeStores = new HashMap<>();
if (session instanceof SpringVaadinSession) {
sessionDestroyListenerRegistration = null;
((SpringVaadinSession) session)
.addDestroyListener(event -> destroy());
} else {
sessionDestroyListenerRegistration = session.getService()
.addSessionDestroyListener(event -> destroy());
}
}

private RouteBeanStore getBeanStore(UI ui) {
Expand Down Expand Up @@ -146,9 +136,6 @@ private void destroy() {
routeStores.clear();
} finally {
session.unlock();
if (sessionDestroyListenerRegistration != null) {
sessionDestroyListenerRegistration.remove();
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import com.vaadin.flow.server.VaadinSession;
import com.vaadin.flow.shared.Registration;
import com.vaadin.flow.spring.SpringVaadinSession;

/**
* Implementation of Spring's
Expand All @@ -40,31 +39,15 @@ public class VaadinSessionScope extends AbstractScope {

private static class SessionBeanStore extends BeanStore {

private final Registration sessionDestroyListenerRegistration;

private SessionBeanStore(VaadinSession session) {
super(session);
if (session instanceof SpringVaadinSession) {
sessionDestroyListenerRegistration = null;
((SpringVaadinSession) session)
.addDestroyListener(event -> destroy());
} else {
sessionDestroyListenerRegistration = session.getService()
.addSessionDestroyListener(event -> destroy());
}
session.addSessionDestroyListener(event -> destroy());
}

@Override
Void doDestroy() {
try {
getVaadinSession().setAttribute(BeanStore.class, null);
super.doDestroy();
} finally {
if (sessionDestroyListenerRegistration != null) {
sessionDestroyListenerRegistration.remove();
}
}
return null;
getVaadinSession().setAttribute(BeanStore.class, null);
return super.doDestroy();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import com.vaadin.flow.component.UI;
import com.vaadin.flow.server.VaadinSession;
import com.vaadin.flow.shared.Registration;
import com.vaadin.flow.spring.SpringVaadinSession;

/**
* Implementation of Spring's
Expand All @@ -48,22 +47,13 @@ private static class UIStoreWrapper

private final VaadinSession session;

private final Registration sessionDestroyListenerRegistration;

private final Map<Integer, BeanStore> uiStores;

private UIStoreWrapper(VaadinSession session) {
assert session.hasLock();
uiStores = new HashMap<>();
this.session = session;
if (session instanceof SpringVaadinSession) {
sessionDestroyListenerRegistration = null;
((SpringVaadinSession) session)
.addDestroyListener(event -> destroy());
} else {
sessionDestroyListenerRegistration = session.getService()
.addSessionDestroyListener(event -> destroy());
}
session.addSessionDestroyListener(event -> destroy());
}

@Override
Expand Down Expand Up @@ -96,9 +86,6 @@ private void destroy() {
uiStores.clear();
} finally {
session.unlock();
if (sessionDestroyListenerRegistration != null) {
sessionDestroyListenerRegistration.remove();
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.Collections;
import java.util.Properties;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Supplier;

import org.junit.After;
Expand All @@ -35,7 +36,6 @@
import com.vaadin.flow.server.VaadinSession;
import com.vaadin.flow.server.VaadinSessionState;
import com.vaadin.flow.server.startup.ApplicationConfiguration;
import com.vaadin.flow.spring.SpringVaadinSession;

import static org.mockito.Mockito.doCallRealMethod;
import static org.mockito.Mockito.times;
Expand All @@ -46,12 +46,18 @@ public abstract class AbstractScopeTest {

private VaadinSession session;

public static class TestSession extends SpringVaadinSession {
public static class TestSession extends VaadinSession {

private final ReentrantLock lock = new ReentrantLock();

public TestSession() {
super(Mockito.mock(VaadinService.class));
super(Mockito.spy(VaadinService.class));
}

@Override
public ReentrantLock getLockInstance() {
return this.lock;
}
}

@After
Expand Down Expand Up @@ -131,7 +137,7 @@ protected void registerDestructionCallback_currentScopeIsSet_objectIsStored(

@SuppressWarnings("unchecked")
protected VaadinSession mockSession() {
SpringVaadinSession session = Mockito.mock(TestSession.class,
VaadinSession session = Mockito.mock(TestSession.class,
Mockito.withSettings().useConstructor());
doCallRealMethod().when(session).setAttribute(Mockito.any(Class.class),
Mockito.any());
Expand Down
Loading
Loading