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

Implementation And Design Smells #2521

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ public class MeteorServiceProcessor implements Processor<Servlet> {
@Override
public void handle(AtmosphereFramework framework, Class<Servlet> annotatedClass) {
try {
ReflectorServletProcessor r = framework.newClassInstance(ReflectorServletProcessor.class, ReflectorServletProcessor.class);

Class<ReflectorServletProcessor> processorClass = ReflectorServletProcessor.class;
ReflectorServletProcessor r = framework.newClassInstance(processorClass, processorClass);
r.setServletClassName(annotatedClass.getName());
LinkedList<AtmosphereInterceptor> l = new LinkedList<>();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package org.atmosphere.config;

import jakarta.websocket.Session;

public class WebSocketSessionConfiguration {
private final Session session;
private final int maxBinaryBufferSize;
private final int webSocketIdleTimeoutMs;
private final int maxTextBufferSize;

public WebSocketSessionConfiguration(Session session, int maxBinaryBufferSize, int webSocketIdleTimeoutMs, int maxTextBufferSize) {
this.session = session;
this.maxBinaryBufferSize = maxBinaryBufferSize;
this.webSocketIdleTimeoutMs = webSocketIdleTimeoutMs;
this.maxTextBufferSize = maxTextBufferSize;
}

public void configure() {
if (maxBinaryBufferSize != -1) {
session.setMaxBinaryMessageBufferSize(maxBinaryBufferSize);
}
if (webSocketIdleTimeoutMs != -1) {
session.setMaxIdleTimeout(webSocketIdleTimeoutMs);
}
if (maxTextBufferSize != -1) {
session.setMaxTextMessageBufferSize(maxTextBufferSize);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import jakarta.websocket.MessageHandler;
import jakarta.websocket.Session;
import jakarta.websocket.server.HandshakeRequest;
import org.atmosphere.config.WebSocketSessionConfiguration;
import org.atmosphere.container.version.JSR356WebSocket;
import org.atmosphere.cpr.ApplicationConfig;
import org.atmosphere.cpr.AtmosphereFramework;
Expand Down Expand Up @@ -117,9 +118,11 @@ public void onOpen(Session session, final EndpointConfig endpointConfig) {
return;
}

if (maxBinaryBufferSize != -1) session.setMaxBinaryMessageBufferSize(maxBinaryBufferSize);
if (webSocketIdleTimeoutMs != -1) session.setMaxIdleTimeout(webSocketIdleTimeoutMs);
if (maxTextBufferSize != -1) session.setMaxTextMessageBufferSize(maxTextBufferSize);

WebSocketSessionConfiguration sessionConfigurator = new WebSocketSessionConfiguration(
session, maxBinaryBufferSize, webSocketIdleTimeoutMs, maxTextBufferSize
);
sessionConfigurator.configure();

webSocket = new JSR356WebSocket(session, framework.getAtmosphereConfig());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public void onStartup(Set<Class<?>> classes, final ServletContext c) {
force = false;
}

if (force || l.size() == size && resolver.testClassExists(DefaultAsyncSupportResolver.JSR356_WEBSOCKET)) {
if (force || l.size() == size && resolver.testClassExists("jakarta.websocket.Endpoint")) {
try {
framework.setAsyncSupport(new JSR356AsyncSupport(framework.getAtmosphereConfig(), c));
} catch (IllegalStateException ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,68 @@
*/
public class DefaultAsyncSupportResolver implements AsyncSupportResolver {

private static final Logger logger = LoggerFactory.getLogger(DefaultAsyncSupportResolver.class);
private static Logger logger = LoggerFactory.getLogger(DefaultAsyncSupportResolver.class);

public final static String SERVLET_30 = "jakarta.servlet.AsyncListener";
public final static String NETTY = "org.jboss.netty.channel.Channel";
public final static String JSR356_WEBSOCKET = "jakarta.websocket.Endpoint";
private static String SERVLET_30 = "jakarta.servlet.AsyncListener";
private static String NETTY = "org.jboss.netty.channel.Channel";
private static String JSR356_WEBSOCKET = "jakarta.websocket.Endpoint";

private final AtmosphereConfig config;

private final boolean suppress356;
private AtmosphereConfig config;
private boolean suppress356;

public DefaultAsyncSupportResolver(final AtmosphereConfig config) {
this.config = config;
this.suppress356 =
Boolean.parseBoolean(config.getInitParameter(ApplicationConfig.WEBSOCKET_SUPPRESS_JSR356));
this.suppress356 = Boolean.parseBoolean(config.getInitParameter(ApplicationConfig.WEBSOCKET_SUPPRESS_JSR356));
}

// Getter and Setter Methods for the variables
public static String getServlet30() {
return SERVLET_30;
}

public static void setServlet30(String servlet30) {
SERVLET_30 = servlet30;
}

public static String getNetty() {
return NETTY;
}

public static void setNetty(String netty) {
NETTY = netty;
}

public static String getJsr356WebSocket() {
return JSR356_WEBSOCKET;
}

public static void setJsr356WebSocket(String jsr356WebSocket) {
JSR356_WEBSOCKET = jsr356WebSocket;
}

public AtmosphereConfig getConfig() {
return config;
}

public void setConfig(AtmosphereConfig config) {
this.config = config;
}

public boolean isSuppress356() {
return suppress356;
}

public void setSuppress356(boolean suppress356) {
this.suppress356 = suppress356;
}

// Getter and Setter for logger
public static Logger getLogger() {
return logger;
}

public static void setLogger(Logger logger) {
DefaultAsyncSupportResolver.logger = logger;
}

/**
Expand All @@ -59,7 +107,7 @@ public DefaultAsyncSupportResolver(final AtmosphereConfig config) {
protected boolean testClassExists(final String testClass) {
try {
final boolean exists = testClass != null && testClass.length() > 0 && IOUtils.loadClass(null, testClass) != null;
logger.debug(exists ? "Found {}" : "Not found {}", testClass);
getLogger().debug(exists ? "Found {}" : "Not found {}", testClass);
return exists;
} catch (Exception ex) {
return false;
Expand All @@ -74,23 +122,22 @@ protected boolean testClassExists(final String testClass) {
public List<Class<? extends AsyncSupport>> detectContainersPresent() {
return new LinkedList<Class<? extends AsyncSupport>>() {
{
if (testClassExists(NETTY))
if (testClassExists(getNetty())) {
add(NettyCometSupport.class);
}
}
};
}

public List<Class<? extends AsyncSupport>> detectWebSocketPresent(final boolean useNativeIfPossible, final boolean useServlet30Async) {

return new LinkedList<Class<? extends AsyncSupport>>() {
{
if (useServlet30Async && !useNativeIfPossible) {

if (!suppress356 && testClassExists(JSR356_WEBSOCKET)) {
if (!isSuppress356() && testClassExists(getJsr356WebSocket())) {
add(JSR356AsyncSupport.class);
}
} else {
if (!suppress356 && testClassExists(JSR356_WEBSOCKET)) {
if (!isSuppress356() && testClassExists(getJsr356WebSocket())) {
add(JSR356AsyncSupport.class);
}
}
Expand All @@ -105,10 +152,10 @@ public List<Class<? extends AsyncSupport>> detectWebSocketPresent(final boolean
* @return
*/
public AsyncSupport defaultCometSupport(final boolean preferBlocking) {
if (!preferBlocking && testClassExists(SERVLET_30)) {
return new Servlet30CometSupport(config);
if (!preferBlocking && testClassExists(getServlet30())) {
return new Servlet30CometSupport(getConfig());
} else {
return new BlockingIOCometSupport(config);
return new BlockingIOCometSupport(getConfig());
}
}

Expand All @@ -123,13 +170,13 @@ public AsyncSupport defaultCometSupport(final boolean preferBlocking) {
public AsyncSupport newCometSupport(final Class<? extends AsyncSupport> targetClass) {
try {
return targetClass.getDeclaredConstructor(new Class[]{AtmosphereConfig.class})
.newInstance(config);
.newInstance(getConfig());
} catch (final Exception e) {
logger.warn("Failed to create AsyncSupport class: {}, error: {}", targetClass, e);
getLogger().warn("Failed to create AsyncSupport class: {}, error: {}", targetClass, e);

Throwable cause = e.getCause();
if (cause != null) {
logger.error("Real error: {}", cause.getMessage(), cause);
getLogger().error("Real error: {}", cause.getMessage(), cause);
}
return null;
}
Expand All @@ -139,12 +186,12 @@ public AsyncSupport newCometSupport(final String targetClassFQN) {
try {
ClassLoader cl = Thread.currentThread().getContextClassLoader();
return (AsyncSupport) cl.loadClass(targetClassFQN)
.getDeclaredConstructor(new Class[]{AtmosphereConfig.class}).newInstance(config);
.getDeclaredConstructor(new Class[]{AtmosphereConfig.class}).newInstance(getConfig());
} catch (final Exception e) {
logger.error("Failed to create AsyncSupport class: {}, error: {}", targetClassFQN, e);
getLogger().error("Failed to create AsyncSupport class: {}, error: {}", targetClassFQN, e);
Throwable cause = e.getCause();
if (cause != null) {
logger.error("Real error: {}", cause.getMessage(), cause);
getLogger().error("Real error: {}", cause.getMessage(), cause);
}
throw new IllegalArgumentException("Unable to create " + targetClassFQN, e);
}
Expand Down Expand Up @@ -174,7 +221,7 @@ public AsyncSupport resolve(boolean useNativeIfPossible, boolean defaultToBlocki
AsyncSupport cs = null;

// Validate the value for old Servlet Container.
useServlet30Async = testClassExists(SERVLET_30);
useServlet30Async = testClassExists(getServlet30());

if (!defaultToBlocking) {
List<Class<? extends AsyncSupport>> l = detectWebSocketPresent(useNativeIfPossible, useServlet30Async);
Expand All @@ -201,7 +248,7 @@ public AsyncSupport resolveWebSocket(final java.util.List<Class<? extends AsyncS
* This method is called to determine which native comet support to the used.
*
* @param available
* @return the result of @link {resolveMultipleNativeSupportConflict} if there are more than 1 item in the list of available ontainers
* @return the result of @link {resolveMultipleNativeSupportConflict} if there are more than 1 item in the list of available containers
*/
protected AsyncSupport resolveNativeCometSupport(final java.util.List<Class<? extends AsyncSupport>> available) {
if (available == null || available.isEmpty()) return null;
Expand All @@ -221,7 +268,8 @@ protected AsyncSupport resolveMultipleNativeSupportConflict(final List<Class<? e
}

b.append(" until you do, Atmosphere will use:").append(available.get(0));
logger.warn("{}", b.toString());
getLogger().warn("{}", b.toString());
return newCometSupport(available.get(0));
}
}

21 changes: 18 additions & 3 deletions modules/cpr/src/main/java/org/atmosphere/util/IOUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,24 @@ public static boolean isBodyBinary(AtmosphereRequest request) {
}

public static boolean isBodyEmpty(Object o) {
if (o != null && (String.class.isAssignableFrom(o.getClass()) && ((String) o).isEmpty())) return true;
assert o != null;
return Byte[].class.isAssignableFrom(o.getClass()) && ((Byte[]) o).length == 0;
if (o == null) {
return false;
}

boolean isEmptyString = isStringAndEmpty(o);
boolean isEmptyByteArray = isByteArrayAndEmpty(o);

return isEmptyString || isEmptyByteArray;
}

private static boolean isStringAndEmpty(Object o) {
boolean isString = String.class.isAssignableFrom(o.getClass());
return isString && ((String) o).isEmpty();
}

private static boolean isByteArrayAndEmpty(Object o) {
boolean isByteArray = Byte[].class.isAssignableFrom(o.getClass());
return isByteArray && ((Byte[]) o).length == 0;
}

public static StringBuilder readEntirelyAsString(AtmosphereResource r) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public void testAsyncSupportClassNotFoundDefaultsToBlockingIOIfServlet30IsNotAva
.resolveNativeCometSupport(anyList());
doReturn(false)
.when(defaultAsyncSupportResolver)
.testClassExists(DefaultAsyncSupportResolver.SERVLET_30);
.testClassExists("jakarta.servlet.AsyncListener");

Assert.assertEquals(
defaultAsyncSupportResolver.resolve(useNativeIfPossible, defaultToBlocking, useServlet30Async).getClass(),
Expand Down