Skip to content

Commit

Permalink
Issue #4903 - Improved behavior for Custom ServerEndpointConfig.Confi…
Browse files Browse the repository at this point in the history 
…gurator

Signed-off-by: Joakim Erdfelt <[email protected]>
  • Loading branch information
@joakime
joakime committed Jun 10, 2020
1 parent cbda92a commit 9bfc168
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 9 deletions.
2 changes: 1 addition & 1 deletion ...src/main/java/org/eclipse/jetty/websocket/jsr356/server/ContainerDefaultConfigurator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public <T> T getEndpointInstance(Class<T> endpointClass) throws InstantiationExc
{
// Since this is started via a ServiceLoader, this class has no Scope or context
// that can be used to obtain a ObjectFactory from.
return endpointClass.getDeclaredConstructor().newInstance();
return endpointClass.getConstructor().newInstance();
}
catch (Exception e)
{
Expand Down
40 changes: 36 additions & 4 deletions ...-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/ServerContainer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,22 @@ public EndpointInstance newClientEndpointInstance(Object endpoint, ServerEndpoin
@Override
public void addEndpoint(Class<?> endpointClass) throws DeploymentException
{
if (endpointClass == null)
{
throw new DeploymentException("Unable to deploy null endpoint class");
}

if (isStarted() || isStarting())
{
if (!ReflectUtils.isDefaultConstructable(endpointClass))
{
throw new DeploymentException("Cannot access default constructor for the class: " + endpointClass.getName());
}

if (LOG.isDebugEnabled())
{
LOG.debug("addEndpoint({})", endpointClass);
}
ServerEndpointMetadata metadata = getServerEndpointMetadata(endpointClass, null);
addEndpoint(metadata);
}
Expand All @@ -136,20 +150,38 @@ public void addEndpoint(Class<?> endpointClass) throws DeploymentException
}
}

private void addEndpoint(ServerEndpointMetadata metadata) throws DeploymentException
private void addEndpoint(ServerEndpointMetadata metadata)
{
if (!ReflectUtils.isDefaultConstructable(metadata.getEndpointClass()))
throw new DeploymentException("Cannot access default constructor for the Endpoint class");

JsrCreator creator = new JsrCreator(this, metadata, this.configuration.getFactory().getExtensionFactory());
this.configuration.addMapping("uri-template|" + metadata.getPath(), creator);
}

@Override
public void addEndpoint(ServerEndpointConfig config) throws DeploymentException
{
if (config == null)
{
throw new DeploymentException("Unable to deploy null ServerEndpointConfig");
}

Class<?> endpointClass = config.getEndpointClass();
if (endpointClass == null)
{
throw new DeploymentException("Unable to deploy null endpoint class from ServerEndpointConfig: " + config.getClass().getName());
}

if (isStarted() || isStarting())
{
ServerEndpointConfig.Configurator configurator = config.getConfigurator();
// only validate constructor and class modifiers on non-custom configurators
if (configurator instanceof ContainerDefaultConfigurator)
{
if (!ReflectUtils.isDefaultConstructable(endpointClass))
{
throw new DeploymentException("Cannot access default constructor for the class: " + endpointClass.getName());
}
}

if (LOG.isDebugEnabled())
{
LOG.debug("addEndpoint({}) path={} endpoint={}", config, config.getPath(), config.getEndpointClass());
Expand Down
66 changes: 62 additions & 4 deletions ...ver-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/PrivateEndpointTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.util.component.LifeCycle;
import org.eclipse.jetty.websocket.api.util.WSURI;
import org.eclipse.jetty.websocket.jsr356.server.deploy.WebSocketServerContainerInitializer;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -41,6 +42,7 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.instanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;

public class PrivateEndpointTest
Expand Down Expand Up @@ -94,6 +96,26 @@ public void onMessage(String message)
}
}

public static class CustomEndpoint extends Endpoint implements MessageHandler.Whole<String>
{
public CustomEndpoint(String id)
{
// This is a valid no-default-constructor implementation, and can be added via a custom
// ServerEndpointConfig.Configurator
}

@Override
public void onOpen(Session session, EndpointConfig config)
{
session.addMessageHandler(this);
}

@Override
public void onMessage(String message)
{
}
}

@SuppressWarnings("InnerClassMayBeStatic")
public class ServerSocketNonStatic extends Endpoint implements MessageHandler.Whole<String>
{
Expand Down Expand Up @@ -131,11 +153,47 @@ private void onMessage(String message)
public void testEndpoint()
{
RuntimeException error = assertThrows(RuntimeException.class, () ->
start(container -> container.addEndpoint(ServerEndpointConfig.Builder.create(ServerSocket.class, "/").build())));
{
ServerEndpointConfig config = ServerEndpointConfig.Builder.create(ServerSocket.class, "/").build();
start(container -> container.addEndpoint(config));
});

assertThat(error.getCause(), instanceOf(DeploymentException.class));
DeploymentException deploymentException = (DeploymentException)error.getCause();
assertThat(deploymentException.getMessage(), containsString("Cannot access default constructor"));
}

@Test
public void testCustomEndpoint() throws Exception
{
ServerEndpointConfig config = ServerEndpointConfig.Builder.create(CustomEndpoint.class, "/")
.configurator(new ServerEndpointConfig.Configurator()
{
@SuppressWarnings("unchecked")
@Override
public <T> T getEndpointInstance(Class<T> endpointClass)
{
return (T)new CustomEndpoint("server");
}
}).build();
start(container -> container.addEndpoint(config));

Session session = client.connectToServer(new CustomEndpoint("client"), WSURI.toWebsocket(server.getURI().resolve("/")));
assertNotNull(session);
}

@Test
public void testCustomEndpointNoConfigurator()
{
RuntimeException error = assertThrows(RuntimeException.class, () ->
{
ServerEndpointConfig config = ServerEndpointConfig.Builder.create(CustomEndpoint.class, "/").build();
start(container -> container.addEndpoint(config));
});

assertThat(error.getCause(), instanceOf(DeploymentException.class));
DeploymentException deploymentException = (DeploymentException)error.getCause();
assertThat(deploymentException.getMessage(), containsString("Cannot access default constructor for the Endpoint class"));
assertThat(deploymentException.getMessage(), containsString("Cannot access default constructor"));
}

@Test
Expand All @@ -146,7 +204,7 @@ public void testInnerEndpoint()

assertThat(error.getCause(), instanceOf(DeploymentException.class));
DeploymentException deploymentException = (DeploymentException)error.getCause();
assertThat(deploymentException.getMessage(), containsString("Cannot access default constructor for the Endpoint class"));
assertThat(deploymentException.getMessage(), containsString("Cannot access default constructor"));
}

@Test
Expand All @@ -157,7 +215,7 @@ public void testAnnotatedEndpoint()

assertThat(error.getCause(), instanceOf(DeploymentException.class));
DeploymentException deploymentException = (DeploymentException)error.getCause();
assertThat(deploymentException.getMessage(), containsString("Cannot access default constructor for the Endpoint class"));
assertThat(deploymentException.getMessage(), containsString("Cannot access default constructor"));
}

@Test
Expand Down

0 comments on commit 9bfc168

Please sign in to comment.