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: Initialize VaadinServlet after Lookup is available #9500

Merged
merged 6 commits into from
Nov 26, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
36 changes: 25 additions & 11 deletions flow-server/src/main/java/com/vaadin/flow/server/VaadinServlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.vaadin.flow.server;

import javax.servlet.ServletConfig;
import javax.servlet.ServletContext;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
Expand All @@ -27,6 +28,7 @@
import java.util.Properties;

import com.vaadin.flow.component.UI;
import com.vaadin.flow.di.Lookup;
import com.vaadin.flow.function.DeploymentConfiguration;
import com.vaadin.flow.internal.CurrentInstance;
import com.vaadin.flow.server.HandlerHelper.RequestType;
Expand Down Expand Up @@ -64,23 +66,35 @@ public class VaadinServlet extends HttpServlet {
@Override
public void init(ServletConfig servletConfig) throws ServletException {
CurrentInstance.clearAll();
super.init(servletConfig);

try {
servletService = createServletService();
} catch (ServiceException e) {
throw new ServletException("Could not initialize VaadinServlet", e);
if (getServletConfig() == null) {
denis-anisimov marked this conversation as resolved.
Show resolved Hide resolved
super.init(servletConfig);
}

ServletContext servletContext = getServletConfig().getServletContext();
if (new VaadinServletContext(servletContext)
.getAttribute(Lookup.class) == null) {
return;
denis-anisimov marked this conversation as resolved.
Show resolved Hide resolved
}

// Sets current service as it is needed in static file server even
// though there are no request and response.
servletService.setCurrentInstances(null, null);
if (servletService == null) {
denis-anisimov marked this conversation as resolved.
Show resolved Hide resolved
try {
servletService = createServletService();
} catch (ServiceException e) {
throw new ServletException("Could not initialize VaadinServlet",
e);
}

staticFileHandler = createStaticFileHandler(servletService);
// Sets current service as it is needed in static file server even
// though there are no request and response.
servletService.setCurrentInstances(null, null);

servletInitialized();
CurrentInstance.clearAll();
staticFileHandler = createStaticFileHandler(servletService);

servletInitialized();
}

CurrentInstance.clearAll();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ public void nullUrlIsSetToACriticalNotification() {
public void serviceContainsStreamRequestHandler()
throws ServiceException, ServletException {
ServletConfig servletConfig = new MockServletConfig();
servletConfig.getServletContext().setAttribute(Lookup.class.getName(),
Mockito.mock(Lookup.class));
VaadinServlet servlet = new VaadinServlet() {
@Override
protected DeploymentConfiguration createDeploymentConfiguration()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,20 @@
*/
package com.vaadin.flow.server;

import javax.servlet.ServletConfig;
import javax.servlet.ServletContext;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;

import java.util.concurrent.atomic.AtomicBoolean;

import net.jcip.annotations.NotThreadSafe;
import org.junit.Assert;
import org.junit.Test;
import org.mockito.Mockito;

import com.vaadin.flow.di.Lookup;

@NotThreadSafe
public class VaadinServletTest {

Expand Down Expand Up @@ -64,6 +70,86 @@ public void testGetLastPathParameter() {
.getLastPathParameter("http://myhost.com/a;hello/;b=1,c=2/"));
}

@Test
public void init_superInitCalledOnce() throws ServletException {
AtomicBoolean called = new AtomicBoolean();
VaadinServlet servlet = new VaadinServlet() {

@Override
public void init() throws ServletException {
Assert.assertFalse(called.get());
called.set(true);
}
};

ServletConfig config = mockConfig();
servlet.init(config);

Assert.assertTrue(called.get());

servlet.init(mockConfig());

Assert.assertSame(config, servlet.getServletConfig());
}

@Test
public void init_noLookup_servletIsNotInitialized()
throws ServletException {
AtomicBoolean called = new AtomicBoolean();
VaadinServlet servlet = new VaadinServlet() {

@Override
protected void servletInitialized() throws ServletException {
called.set(true);
}
};

ServletConfig config = mockConfig();
servlet.init(config);

Assert.assertFalse(called.get());
}

@Test
public void init_contextHasLookup_servletIsInitialized()
throws ServletException {
AtomicBoolean called = new AtomicBoolean();
VaadinServlet servlet = new VaadinServlet() {

@Override
protected VaadinServletService createServletService()
throws ServletException, ServiceException {
return Mockito.mock(VaadinServletService.class);
}

@Override
protected StaticFileHandler createStaticFileHandler(
VaadinServletService servletService) {
return Mockito.mock(StaticFileHandler.class);
}

@Override
protected void servletInitialized() throws ServletException {
called.set(true);
}
};

ServletConfig config = mockConfig();
ServletContext servletContext = config.getServletContext();
Mockito.when(servletContext.getAttribute(Lookup.class.getName()))
.thenReturn(Mockito.mock(Lookup.class));
servlet.init(config);

Assert.assertTrue(called.get());
}

private ServletConfig mockConfig() {
ServletConfig config = Mockito.mock(ServletConfig.class);
ServletContext context = Mockito.mock(ServletContext.class);
Mockito.when(config.getServletContext()).thenReturn(context);
return config;
}

private HttpServletRequest createRequest(
MockServletServiceSessionSetup mocks, String path) {
HttpServletRequest httpServletRequest = Mockito
Expand Down