diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java index 44c301ebda..526dc26712 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -445,15 +445,14 @@ public class ConstructorReference extends SpelNodeImpl { public void generateCode(MethodVisitor mv, CodeFlow cf) { ReflectiveConstructorExecutor executor = ((ReflectiveConstructorExecutor) this.cachedExecutor); Constructor constructor = executor.getConstructor(); - String classSlashedDescriptor = constructor.getDeclaringClass().getName().replace('.', '/'); - mv.visitTypeInsn(NEW, classSlashedDescriptor); + String classDesc = constructor.getDeclaringClass().getName().replace('.', '/'); + mv.visitTypeInsn(NEW, classDesc); mv.visitInsn(DUP); // children[0] is the type of the constructor, don't want to include that in argument processing - SpelNodeImpl[] arguments = new SpelNodeImpl[children.length-1]; - System.arraycopy(children, 1, arguments, 0, children.length-1); + SpelNodeImpl[] arguments = new SpelNodeImpl[children.length - 1]; + System.arraycopy(children, 1, arguments, 0, children.length - 1); generateCodeForArguments(mv, cf, constructor, arguments); - mv.visitMethodInsn(INVOKESPECIAL, classSlashedDescriptor, "", - CodeFlow.createSignatureDescriptor(constructor), false); + mv.visitMethodInsn(INVOKESPECIAL, classDesc, "", CodeFlow.createSignatureDescriptor(constructor), false); cf.pushDescriptor(this.exitTypeDescriptor); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java index c876ed4bc1..605910c9f3 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -33,15 +33,15 @@ import org.springframework.expression.spel.support.ReflectionHelper; import org.springframework.util.ReflectionUtils; /** - * A function reference is of the form "#someFunction(a,b,c)". Functions may be defined in - * the context prior to the expression being evaluated or within the expression itself + * A function reference is of the form "#someFunction(a,b,c)". Functions may be defined + * in the context prior to the expression being evaluated or within the expression itself * using a lambda function definition. For example: Lambda function definition in an * expression: "(#max = {|x,y|$x>$y?$x:$y};max(2,3))" Calling context defined function: * "#isEven(37)". Functions may also be static java methods, registered in the context * prior to invocation of the expression. * - *

Functions are very simplistic, the arguments are not part of the definition (right - * now), so the names must be unique. + *

Functions are very simplistic, the arguments are not part of the definition + * (right now), so the names must be unique. * * @author Andy Clement * @since 3.0 @@ -72,7 +72,8 @@ public class FunctionReference extends SpelNodeImpl { // Two possibilities: a lambda function or a Java static method registered as a function if (!(value.getValue() instanceof Method)) { - throw new SpelEvaluationException(SpelMessage.FUNCTION_REFERENCE_CANNOT_BE_INVOKED, this.name, value.getClass()); + throw new SpelEvaluationException( + SpelMessage.FUNCTION_REFERENCE_CANNOT_BE_INVOKED, this.name, value.getClass()); } try { @@ -113,7 +114,8 @@ public class FunctionReference extends SpelNodeImpl { argumentConversionOccurred = ReflectionHelper.convertAllArguments(converter, functionArgs, method); } if (method.isVarArgs()) { - functionArgs = ReflectionHelper.setupArgumentsForVarargsInvocation(method.getParameterTypes(), functionArgs); + functionArgs = + ReflectionHelper.setupArgumentsForVarargsInvocation(method.getParameterTypes(), functionArgs); } try { @@ -160,13 +162,12 @@ public class FunctionReference extends SpelNodeImpl { @Override public boolean isCompilable() { - if (this.method == null || argumentConversionOccurred) { + if (this.method == null || this.argumentConversionOccurred) { return false; } int methodModifiers = this.method.getModifiers(); - if (!Modifier.isStatic(methodModifiers) || - !Modifier.isPublic(methodModifiers) || - !Modifier.isPublic(method.getDeclaringClass().getModifiers())) { + if (!Modifier.isStatic(methodModifiers) || !Modifier.isPublic(methodModifiers) || + !Modifier.isPublic(this.method.getDeclaringClass().getModifiers())) { return false; } for (SpelNodeImpl child : this.children) { @@ -179,9 +180,9 @@ public class FunctionReference extends SpelNodeImpl { @Override public void generateCode(MethodVisitor mv,CodeFlow cf) { - String methodDeclaringClassSlashedDescriptor = this.method.getDeclaringClass().getName().replace('.', '/'); - generateCodeForArguments(mv, cf, method, this.children); - mv.visitMethodInsn(INVOKESTATIC, methodDeclaringClassSlashedDescriptor, this.method.getName(), + String classDesc = this.method.getDeclaringClass().getName().replace('.', '/'); + generateCodeForArguments(mv, cf, this.method, this.children); + mv.visitMethodInsn(INVOKESTATIC, classDesc, this.method.getName(), CodeFlow.createSignatureDescriptor(this.method), false); cf.pushDescriptor(this.exitTypeDescriptor); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java index 254c6ee9e6..0352b56e2b 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -248,6 +248,7 @@ public class Indexer extends SpelNodeImpl { cf.exitCompilationScope(); mv.visitInsn(insn); } + else if (this.indexedType == IndexedType.LIST) { mv.visitTypeInsn(CHECKCAST, "java/util/List"); cf.enterCompilationScope(); @@ -255,6 +256,7 @@ public class Indexer extends SpelNodeImpl { cf.exitCompilationScope(); mv.visitMethodInsn(INVOKEINTERFACE, "java/util/List", "get", "(I)Ljava/lang/Object;", true); } + else if (this.indexedType == IndexedType.MAP) { mv.visitTypeInsn(CHECKCAST, "java/util/Map"); // Special case when the key is an unquoted string literal that will be parsed as @@ -271,27 +273,30 @@ public class Indexer extends SpelNodeImpl { } mv.visitMethodInsn(INVOKEINTERFACE, "java/util/Map", "get", "(Ljava/lang/Object;)Ljava/lang/Object;", true); } + else if (this.indexedType == IndexedType.OBJECT) { ReflectivePropertyAccessor.OptimalPropertyAccessor accessor = (ReflectivePropertyAccessor.OptimalPropertyAccessor) this.cachedReadAccessor; Member member = accessor.member; boolean isStatic = Modifier.isStatic(member.getModifiers()); - String memberDeclaringClassSlashedDescriptor = member.getDeclaringClass().getName().replace('.', '/'); + String classDesc = member.getDeclaringClass().getName().replace('.', '/'); + if (!isStatic) { if (descriptor == null) { cf.loadTarget(mv); } - if (descriptor == null || !memberDeclaringClassSlashedDescriptor.equals(descriptor.substring(1))) { - mv.visitTypeInsn(CHECKCAST, memberDeclaringClassSlashedDescriptor); + if (descriptor == null || !classDesc.equals(descriptor.substring(1))) { + mv.visitTypeInsn(CHECKCAST, classDesc); } } - if (member instanceof Field) { - mv.visitFieldInsn(isStatic ? GETSTATIC : GETFIELD, memberDeclaringClassSlashedDescriptor, - member.getName(), CodeFlow.toJvmDescriptor(((Field) member).getType())); + + if (member instanceof Method) { + mv.visitMethodInsn((isStatic? INVOKESTATIC : INVOKEVIRTUAL), classDesc, member.getName(), + CodeFlow.createSignatureDescriptor((Method) member), false); } else { - mv.visitMethodInsn(isStatic? INVOKESTATIC : INVOKEVIRTUAL, memberDeclaringClassSlashedDescriptor, - member.getName(), CodeFlow.createSignatureDescriptor((Method) member), false); + mv.visitFieldInsn((isStatic ? GETSTATIC : GETFIELD), classDesc, member.getName(), + CodeFlow.toJvmDescriptor(((Field) member).getType())); } } @@ -555,12 +560,8 @@ public class Indexer extends SpelNodeImpl { ReflectivePropertyAccessor.OptimalPropertyAccessor optimalAccessor = (ReflectivePropertyAccessor.OptimalPropertyAccessor) accessor; Member member = optimalAccessor.member; - if (member instanceof Field) { - Indexer.this.exitTypeDescriptor = CodeFlow.toDescriptor(((Field)member).getType()); - } - else { - Indexer.this.exitTypeDescriptor = CodeFlow.toDescriptor(((Method)member).getReturnType()); - } + Indexer.this.exitTypeDescriptor = CodeFlow.toDescriptor(member instanceof Method ? + ((Method) member).getReturnType() : ((Field) member).getType()); } return accessor.read(this.evaluationContext, this.targetObject, this.name); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/SpelNodeImpl.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/SpelNodeImpl.java index ae8da2a1d8..f5a88679ad 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/SpelNodeImpl.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/SpelNodeImpl.java @@ -239,15 +239,15 @@ public abstract class SpelNodeImpl implements SpelNode, Opcodes { // The final parameter may or may not need packaging into an array, or nothing may // have been passed to satisfy the varargs and so something needs to be built. int p = 0; // Current supplied argument being processed - int childcount = arguments.length; + int childCount = arguments.length; // Fulfill all the parameter requirements except the last one - for (p = 0; p < paramDescriptors.length-1;p++) { + for (p = 0; p < paramDescriptors.length - 1; p++) { generateCodeForArgument(mv, cf, arguments[p], paramDescriptors[p]); } - SpelNodeImpl lastchild = (childcount == 0 ? null : arguments[childcount-1]); - String arraytype = paramDescriptors[paramDescriptors.length-1]; + SpelNodeImpl lastchild = (childCount == 0 ? null : arguments[childCount - 1]); + String arraytype = paramDescriptors[paramDescriptors.length - 1]; // Determine if the final passed argument is already suitably packaged in array // form to be passed to the method if (lastchild != null && lastchild.getExitDescriptor().equals(arraytype)) { @@ -256,10 +256,10 @@ public abstract class SpelNodeImpl implements SpelNode, Opcodes { else { arraytype = arraytype.substring(1); // trim the leading '[', may leave other '[' // build array big enough to hold remaining arguments - CodeFlow.insertNewArrayCode(mv, childcount-p, arraytype); + CodeFlow.insertNewArrayCode(mv, childCount - p, arraytype); // Package up the remaining arguments into the array int arrayindex = 0; - while (p < childcount) { + while (p < childCount) { SpelNodeImpl child = arguments[p]; mv.visitInsn(DUP); CodeFlow.insertOptimalLoad(mv, arrayindex++); @@ -280,20 +280,20 @@ public abstract class SpelNodeImpl implements SpelNode, Opcodes { * Ask an argument to generate its bytecode and then follow it up * with any boxing/unboxing/checkcasting to ensure it matches the expected parameter descriptor. */ - protected static void generateCodeForArgument(MethodVisitor mv, CodeFlow cf, SpelNodeImpl argument, String paramDescriptor) { + protected static void generateCodeForArgument(MethodVisitor mv, CodeFlow cf, SpelNodeImpl argument, String paramDesc) { cf.enterCompilationScope(); argument.generateCode(mv, cf); boolean primitiveOnStack = CodeFlow.isPrimitive(cf.lastDescriptor()); // Check if need to box it for the method reference? - if (primitiveOnStack && paramDescriptor.charAt(0) == 'L') { + if (primitiveOnStack && paramDesc.charAt(0) == 'L') { CodeFlow.insertBoxIfNecessary(mv, cf.lastDescriptor().charAt(0)); } - else if (paramDescriptor.length() == 1 && !primitiveOnStack) { - CodeFlow.insertUnboxInsns(mv, paramDescriptor.charAt(0), cf.lastDescriptor()); + else if (paramDesc.length() == 1 && !primitiveOnStack) { + CodeFlow.insertUnboxInsns(mv, paramDesc.charAt(0), cf.lastDescriptor()); } - else if (!cf.lastDescriptor().equals(paramDescriptor)) { + else if (!cf.lastDescriptor().equals(paramDesc)) { // This would be unnecessary in the case of subtyping (e.g. method takes Number but Integer passed in) - CodeFlow.insertCheckCast(mv, paramDescriptor); + CodeFlow.insertCheckCast(mv, paramDesc); } cf.exitCompilationScope(); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java index 8f9aea6048..d080c284f0 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java @@ -238,11 +238,13 @@ public class ReflectionHelper { * @return true if some kind of conversion occurred on the argument * @throws SpelEvaluationException if there is a problem with conversion */ - public static boolean convertAllArguments(TypeConverter converter, Object[] arguments, Method method) throws SpelEvaluationException { - Integer varargsPosition = method.isVarArgs() ? method.getParameterTypes().length-1:null; + public static boolean convertAllArguments(TypeConverter converter, Object[] arguments, Method method) + throws SpelEvaluationException { + + Integer varargsPosition = (method.isVarArgs() ? method.getParameterTypes().length - 1 : null); return convertArguments(converter, arguments, method, varargsPosition); } - + /** * Takes an input set of argument values and converts them to the types specified as the * required parameter types. The arguments are converted 'in-place' in the input array. diff --git a/spring-messaging/src/main/java/org/springframework/messaging/tcp/reactor/AbstractPromiseToListenableFutureAdapter.java b/spring-messaging/src/main/java/org/springframework/messaging/tcp/reactor/AbstractPromiseToListenableFutureAdapter.java index 9874dfd1e6..d48f080a8a 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/tcp/reactor/AbstractPromiseToListenableFutureAdapter.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/tcp/reactor/AbstractPromiseToListenableFutureAdapter.java @@ -56,21 +56,20 @@ abstract class AbstractPromiseToListenableFutureAdapter implements Listena try { registry.success(adapt(result)); } - catch (Throwable t) { - registry.failure(t); + catch (Throwable ex) { + registry.failure(ex); } } }); this.promise.onError(new Consumer() { @Override - public void accept(Throwable t) { - registry.failure(t); + public void accept(Throwable ex) { + registry.failure(ex); } }); } - protected abstract T adapt(S result); @Override public T get() throws InterruptedException { @@ -113,4 +112,7 @@ abstract class AbstractPromiseToListenableFutureAdapter implements Listena this.registry.addFailureCallback(failureCallback); } + + protected abstract T adapt(S result); + } diff --git a/spring-messaging/src/main/java/org/springframework/messaging/tcp/reactor/Reactor11TcpClient.java b/spring-messaging/src/main/java/org/springframework/messaging/tcp/reactor/Reactor11TcpClient.java index 093147fb57..669cf86df2 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/tcp/reactor/Reactor11TcpClient.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/tcp/reactor/Reactor11TcpClient.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -47,7 +47,6 @@ import org.springframework.messaging.tcp.TcpOperations; import org.springframework.util.Assert; import org.springframework.util.concurrent.ListenableFuture; - /** * An implementation of {@link org.springframework.messaging.tcp.TcpOperations} * based on the TCP client support of the Reactor project. @@ -70,33 +69,23 @@ public class Reactor11TcpClient

implements TcpOperations

{ * A constructor that creates a {@link reactor.net.netty.tcp.NettyTcpClient} with * a {@link reactor.event.dispatch.SynchronousDispatcher} as a result of which * network I/O is handled in Netty threads. - * *

Also see the constructor accepting a pre-configured Reactor * {@link reactor.net.tcp.TcpClient}. - * * @param host the host to connect to * @param port the port to connect to * @param codec the codec to use for encoding and decoding the TCP stream */ public Reactor11TcpClient(String host, int port, Codec, Message

> codec) { - - // Revisit in 1.1: is Environment still required w/ sync dispatcher? this.environment = new Environment(new SynchronousDispatcherConfigReader()); - this.tcpClient = new TcpClientSpec, Message

>(REACTOR_TCP_CLIENT_TYPE) - .env(this.environment) - .codec(codec) - .connect(host, port) - .get(); + .env(this.environment).codec(codec).connect(host, port).get(); } /** * A constructor with a pre-configured {@link reactor.net.tcp.TcpClient}. - * *

NOTE: if the client is configured with a thread-creating * dispatcher, you are responsible for shutting down the {@link reactor.core.Environment} * instance with which the client is configured. - * * @param tcpClient the TcpClient to use */ public Reactor11TcpClient(TcpClient, Message

> tcpClient) { @@ -108,7 +97,6 @@ public class Reactor11TcpClient

implements TcpOperations

{ @Override public ListenableFuture connect(TcpConnectionHandler

connectionHandler) { - Promise, Message

>> promise = this.tcpClient.open(); composeConnectionHandling(promise, connectionHandler); @@ -125,7 +113,6 @@ public class Reactor11TcpClient

implements TcpOperations

{ final ReconnectStrategy reconnectStrategy) { Assert.notNull(reconnectStrategy, "ReconnectStrategy must not be null"); - Reconnect reconnect = new Reconnect() { @Override public Tuple2 reconnect(InetSocketAddress address, int attempt) { @@ -162,8 +149,8 @@ public class Reactor11TcpClient

implements TcpOperations

{ connection .when(Throwable.class, new Consumer() { @Override - public void accept(Throwable t) { - connectionHandler.handleFailure(t); + public void accept(Throwable ex) { + connectionHandler.handleFailure(ex); } }) .consume(new Consumer>() { @@ -201,9 +188,9 @@ public class Reactor11TcpClient

implements TcpOperations

{ } } + /** * A ConfigurationReader that enforces the use of a SynchronousDispatcher. - * *

The {@link reactor.core.configuration.PropertiesConfigurationReader} used by * default automatically creates other dispatchers with thread pools that are * not needed. diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/ViewControllerBeanDefinitionParser.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/ViewControllerBeanDefinitionParser.java index 4e847cfd98..9025ff1f40 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/ViewControllerBeanDefinitionParser.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/ViewControllerBeanDefinitionParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,7 +21,6 @@ import java.util.Map; import org.w3c.dom.Element; import org.springframework.beans.factory.config.BeanDefinition; -import org.springframework.beans.factory.config.ConstructorArgumentValues; import org.springframework.beans.factory.parsing.BeanComponentDefinition; import org.springframework.beans.factory.support.ManagedMap; import org.springframework.beans.factory.support.RootBeanDefinition; @@ -131,21 +130,25 @@ class ViewControllerBeanDefinitionParser implements BeanDefinitionParser { } private RootBeanDefinition getRedirectView(Element element, HttpStatus status, Object source) { - ConstructorArgumentValues cavs = new ConstructorArgumentValues(); - cavs.addIndexedArgumentValue(0, element.getAttribute("redirect-url")); - RootBeanDefinition redirectView = new RootBeanDefinition(RedirectView.class, cavs, null); + RootBeanDefinition redirectView = new RootBeanDefinition(RedirectView.class); redirectView.setSource(source); + redirectView.getConstructorArgumentValues().addIndexedArgumentValue(0, element.getAttribute("redirect-url")); + if (status != null) { redirectView.getPropertyValues().add("statusCode", status); } + if (element.hasAttribute("context-relative")) { redirectView.getPropertyValues().add("contextRelative", element.getAttribute("context-relative")); - } else { + } + else { redirectView.getPropertyValues().add("contextRelative", true); } + if (element.hasAttribute("keep-query-params")) { redirectView.getPropertyValues().add("propagateQueryParams", element.getAttribute("keep-query-params")); } + return redirectView; } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/adapter/jetty/JettyWebSocketHandlerAdapter.java b/spring-websocket/src/main/java/org/springframework/web/socket/adapter/jetty/JettyWebSocketHandlerAdapter.java index 1972184f00..6b16b5897c 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/adapter/jetty/JettyWebSocketHandlerAdapter.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/adapter/jetty/JettyWebSocketHandlerAdapter.java @@ -38,7 +38,6 @@ import org.springframework.web.socket.TextMessage; import org.springframework.web.socket.WebSocketHandler; import org.springframework.web.socket.handler.ExceptionWebSocketHandlerDecorator; - /** * Adapts {@link WebSocketHandler} to the Jetty 9 WebSocket API. * @@ -59,8 +58,8 @@ public class JettyWebSocketHandlerAdapter { public JettyWebSocketHandlerAdapter(WebSocketHandler webSocketHandler, JettyWebSocketSession wsSession) { - Assert.notNull(webSocketHandler, "webSocketHandler must not be null"); - Assert.notNull(wsSession, "wsSession must not be null"); + Assert.notNull(webSocketHandler, "WebSocketHandler must not be null"); + Assert.notNull(wsSession, "WebSocketSession must not be null"); this.webSocketHandler = webSocketHandler; this.wsSession = wsSession; } @@ -72,8 +71,8 @@ public class JettyWebSocketHandlerAdapter { this.wsSession.initializeNativeSession(session); this.webSocketHandler.afterConnectionEstablished(this.wsSession); } - catch (Throwable t) { - ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + catch (Throwable ex) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } @@ -83,8 +82,8 @@ public class JettyWebSocketHandlerAdapter { try { this.webSocketHandler.handleMessage(this.wsSession, message); } - catch (Throwable t) { - ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + catch (Throwable ex) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } @@ -94,8 +93,8 @@ public class JettyWebSocketHandlerAdapter { try { this.webSocketHandler.handleMessage(this.wsSession, message); } - catch (Throwable t) { - ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + catch (Throwable ex) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } @@ -107,8 +106,8 @@ public class JettyWebSocketHandlerAdapter { try { this.webSocketHandler.handleMessage(this.wsSession, message); } - catch (Throwable t) { - ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + catch (Throwable ex) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } } @@ -119,8 +118,10 @@ public class JettyWebSocketHandlerAdapter { try { this.webSocketHandler.afterConnectionClosed(this.wsSession, closeStatus); } - catch (Throwable t) { - logger.error("Unhandled error for " + this.wsSession, t); + catch (Throwable ex) { + if (logger.isErrorEnabled()) { + logger.error("Unhandled error for " + this.wsSession, ex); + } } } @@ -129,8 +130,8 @@ public class JettyWebSocketHandlerAdapter { try { this.webSocketHandler.handleTransportError(this.wsSession, cause); } - catch (Throwable t) { - ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + catch (Throwable ex) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/adapter/standard/StandardWebSocketHandlerAdapter.java b/spring-websocket/src/main/java/org/springframework/web/socket/adapter/standard/StandardWebSocketHandlerAdapter.java index dfbc58575e..2a22c40f02 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/adapter/standard/StandardWebSocketHandlerAdapter.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/adapter/standard/StandardWebSocketHandlerAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -49,8 +49,8 @@ public class StandardWebSocketHandlerAdapter extends Endpoint { public StandardWebSocketHandlerAdapter(WebSocketHandler handler, StandardWebSocketSession wsSession) { - Assert.notNull(handler, "handler must not be null"); - Assert.notNull(wsSession, "wsSession must not be null"); + Assert.notNull(handler, "WebSocketHandler must not be null"); + Assert.notNull(wsSession, "WebSocketSession must not be null"); this.handler = handler; this.wsSession = wsSession; } @@ -58,7 +58,6 @@ public class StandardWebSocketHandlerAdapter extends Endpoint { @Override public void onOpen(final javax.websocket.Session session, EndpointConfig config) { - this.wsSession.initializeNativeSession(session); if (this.handler.supportsPartialMessages()) { @@ -100,9 +99,8 @@ public class StandardWebSocketHandlerAdapter extends Endpoint { try { this.handler.afterConnectionEstablished(this.wsSession); } - catch (Throwable t) { - ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); - return; + catch (Throwable ex) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } @@ -111,8 +109,8 @@ public class StandardWebSocketHandlerAdapter extends Endpoint { try { this.handler.handleMessage(this.wsSession, textMessage); } - catch (Throwable t) { - ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + catch (Throwable ex) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } @@ -121,8 +119,8 @@ public class StandardWebSocketHandlerAdapter extends Endpoint { try { this.handler.handleMessage(this.wsSession, binaryMessage); } - catch (Throwable t) { - ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + catch (Throwable ex) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } @@ -131,8 +129,8 @@ public class StandardWebSocketHandlerAdapter extends Endpoint { try { this.handler.handleMessage(this.wsSession, pongMessage); } - catch (Throwable t) { - ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + catch (Throwable ex) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } @@ -142,8 +140,10 @@ public class StandardWebSocketHandlerAdapter extends Endpoint { try { this.handler.afterConnectionClosed(this.wsSession, closeStatus); } - catch (Throwable t) { - logger.error("Unhandled error for " + this.wsSession, t); + catch (Throwable ex) { + if (logger.isErrorEnabled()) { + logger.error("Unhandled error for " + this.wsSession, ex); + } } } @@ -152,8 +152,8 @@ public class StandardWebSocketHandlerAdapter extends Endpoint { try { this.handler.handleTransportError(this.wsSession, exception); } - catch (Throwable t) { - ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + catch (Throwable ex) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/handler/ExceptionWebSocketHandlerDecorator.java b/spring-websocket/src/main/java/org/springframework/web/socket/handler/ExceptionWebSocketHandlerDecorator.java index 916ab025ff..728635620f 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/handler/ExceptionWebSocketHandlerDecorator.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/handler/ExceptionWebSocketHandlerDecorator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,16 +25,16 @@ import org.springframework.web.socket.WebSocketMessage; import org.springframework.web.socket.WebSocketSession; /** - * An exception handling {@link WebSocketHandlerDecorator}. Traps all {@link Throwable} - * instances that escape from the decorated handler and closes the session with - * {@link CloseStatus#SERVER_ERROR}. + * An exception handling {@link WebSocketHandlerDecorator}. + * Traps all {@link Throwable} instances that escape from the decorated + * handler and closes the session with {@link CloseStatus#SERVER_ERROR}. * * @author Rossen Stoyanchev * @since 4.0 */ public class ExceptionWebSocketHandlerDecorator extends WebSocketHandlerDecorator { - private final Log logger = LogFactory.getLog(ExceptionWebSocketHandlerDecorator.class); + private static final Log logger = LogFactory.getLog(ExceptionWebSocketHandlerDecorator.class); public ExceptionWebSocketHandlerDecorator(WebSocketHandler delegate) { @@ -52,20 +52,6 @@ public class ExceptionWebSocketHandlerDecorator extends WebSocketHandlerDecorato } } - public static void tryCloseWithError(WebSocketSession session, Throwable exception, Log logger) { - if (logger.isDebugEnabled()) { - logger.debug("Closing due to exception for " + session, exception); - } - if (session.isOpen()) { - try { - session.close(CloseStatus.SERVER_ERROR); - } - catch (Throwable t) { - // ignore - } - } - } - @Override public void handleMessage(WebSocketSession session, WebSocketMessage message) { try { @@ -91,8 +77,25 @@ public class ExceptionWebSocketHandlerDecorator extends WebSocketHandlerDecorato try { getDelegate().afterConnectionClosed(session, closeStatus); } - catch (Throwable t) { - logger.error("Unhandled error for " + this, t); + catch (Throwable ex) { + if (logger.isErrorEnabled()) { + logger.error("Unhandled error for " + this, ex); + } + } + } + + + public static void tryCloseWithError(WebSocketSession session, Throwable exception, Log logger) { + if (logger.isDebugEnabled()) { + logger.debug("Closing due to exception for " + session, exception); + } + if (session.isOpen()) { + try { + session.close(CloseStatus.SERVER_ERROR); + } + catch (Throwable ex) { + // ignore + } } } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/handler/PerConnectionWebSocketHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/handler/PerConnectionWebSocketHandler.java index b55cd1d24f..c6194649fd 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/handler/PerConnectionWebSocketHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/handler/PerConnectionWebSocketHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,10 +22,8 @@ import java.util.concurrent.ConcurrentHashMap; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; -import org.springframework.util.Assert; import org.springframework.web.socket.CloseStatus; import org.springframework.web.socket.WebSocketHandler; import org.springframework.web.socket.WebSocketMessage; @@ -69,11 +67,13 @@ public class PerConnectionWebSocketHandler implements WebSocketHandler, BeanFact this.supportsPartialMessages = supportsPartialMessages; } + @Override - public void setBeanFactory(BeanFactory beanFactory) throws BeansException { + public void setBeanFactory(BeanFactory beanFactory) { this.provider.setBeanFactory(beanFactory); } + @Override public void afterConnectionEstablished(WebSocketSession session) throws Exception { WebSocketHandler handler = this.provider.getHandler(); @@ -86,12 +86,6 @@ public class PerConnectionWebSocketHandler implements WebSocketHandler, BeanFact getHandler(session).handleMessage(session, message); } - private WebSocketHandler getHandler(WebSocketSession session) { - WebSocketHandler handler = this.handlers.get(session); - Assert.isTrue(handler != null, "WebSocketHandler not found for " + session); - return handler; - } - @Override public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception { getHandler(session).handleTransportError(session, exception); @@ -103,19 +97,7 @@ public class PerConnectionWebSocketHandler implements WebSocketHandler, BeanFact getHandler(session).afterConnectionClosed(session, closeStatus); } finally { - destroy(session); - } - } - - private void destroy(WebSocketSession session) { - WebSocketHandler handler = this.handlers.remove(session); - try { - if (handler != null) { - this.provider.destroy(handler); - } - } - catch (Throwable t) { - logger.warn("Error while destroying " + handler, t); + destroyHandler(session); } } @@ -124,6 +106,30 @@ public class PerConnectionWebSocketHandler implements WebSocketHandler, BeanFact return this.supportsPartialMessages; } + + private WebSocketHandler getHandler(WebSocketSession session) { + WebSocketHandler handler = this.handlers.get(session); + if (handler == null) { + throw new IllegalStateException("WebSocketHandler not found for " + session); + } + return handler; + } + + private void destroyHandler(WebSocketSession session) { + WebSocketHandler handler = this.handlers.remove(session); + try { + if (handler != null) { + this.provider.destroy(handler); + } + } + catch (Throwable ex) { + if (logger.isWarnEnabled()) { + logger.warn("Error while destroying " + handler, ex); + } + } + } + + @Override public String toString() { return "PerConnectionWebSocketHandlerProxy[handlerType=" + this.provider.getHandlerType() + "]"; diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/messaging/SubProtocolWebSocketHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/messaging/SubProtocolWebSocketHandler.java index 51c7fb899a..cd82f68954 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/messaging/SubProtocolWebSocketHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/messaging/SubProtocolWebSocketHandler.java @@ -18,8 +18,8 @@ package org.springframework.web.socket.messaging; import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashSet; +import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -86,7 +86,7 @@ public class SubProtocolWebSocketHandler private final Map protocolHandlerLookup = new TreeMap(String.CASE_INSENSITIVE_ORDER); - private final Set protocolHandlers = new HashSet(); + private final Set protocolHandlers = new LinkedHashSet(); private SubProtocolHandler defaultProtocolHandler; @@ -143,12 +143,14 @@ public class SubProtocolWebSocketHandler public void addProtocolHandler(SubProtocolHandler handler) { List protocols = handler.getSupportedProtocols(); if (CollectionUtils.isEmpty(protocols)) { - logger.error("No sub-protocols for " + handler + "."); + if (logger.isErrorEnabled()) { + logger.error("No sub-protocols for " + handler); + } return; } - for (String protocol: protocols) { + for (String protocol : protocols) { SubProtocolHandler replaced = this.protocolHandlerLookup.put(protocol, handler); - if ((replaced != null) && (replaced != handler) ) { + if (replaced != null && replaced != handler) { throw new IllegalStateException("Can't map " + handler + " to protocol '" + protocol + "'. Already mapped to " + replaced + "."); } @@ -171,7 +173,7 @@ public class SubProtocolWebSocketHandler public void setDefaultProtocolHandler(SubProtocolHandler defaultProtocolHandler) { this.defaultProtocolHandler = defaultProtocolHandler; if (this.protocolHandlerLookup.isEmpty()) { - setProtocolHandlers(Arrays.asList(defaultProtocolHandler)); + setProtocolHandlers(Collections.singletonList(defaultProtocolHandler)); } } @@ -262,8 +264,10 @@ public class SubProtocolWebSocketHandler try { holder.getSession().close(CloseStatus.GOING_AWAY); } - catch (Throwable t) { - logger.error("Failed to close '" + holder.getSession() + "': " + t.getMessage()); + catch (Throwable ex) { + if (logger.isErrorEnabled()) { + logger.error("Failed to close '" + holder.getSession() + "': " + ex); + } } } } @@ -286,36 +290,6 @@ public class SubProtocolWebSocketHandler findProtocolHandler(session).afterSessionStarted(session, this.clientInboundChannel); } - protected final SubProtocolHandler findProtocolHandler(WebSocketSession session) { - String protocol = null; - try { - protocol = session.getAcceptedProtocol(); - } - catch (Exception ex) { - // Shouldn't happen - logger.error("Failed to obtain session.getAcceptedProtocol(). Will use the " + - "default protocol handler (if configured).", ex); - } - SubProtocolHandler handler; - if (!StringUtils.isEmpty(protocol)) { - handler = this.protocolHandlerLookup.get(protocol); - Assert.state(handler != null, "No handler for '" + protocol + "' among " + this.protocolHandlerLookup); - } - else { - if (this.defaultProtocolHandler != null) { - handler = this.defaultProtocolHandler; - } - else if (this.protocolHandlers.size() == 1) { - handler = this.protocolHandlers.iterator().next(); - } - else { - throw new IllegalStateException("Multiple protocol handlers configured and " + - "no protocol was negotiated. Consider configuring a default SubProtocolHandler."); - } - } - return handler; - } - /** * Handle an inbound message from a WebSocket client. */ @@ -340,9 +314,12 @@ public class SubProtocolWebSocketHandler public void handleMessage(Message message) throws MessagingException { String sessionId = resolveSessionId(message); if (sessionId == null) { - logger.error("Couldn't find sessionId in " + message); + if (logger.isErrorEnabled()) { + logger.error("Couldn't find session id in " + message); + } return; } + WebSocketSessionHolder holder = this.sessions.get(sessionId); if (holder == null) { if (logger.isDebugEnabled()) { @@ -351,6 +328,7 @@ public class SubProtocolWebSocketHandler } return; } + WebSocketSession session = holder.getSession(); try { findProtocolHandler(session).handleMessageToClient(session, message); @@ -368,12 +346,64 @@ public class SubProtocolWebSocketHandler logger.debug("Failure while closing session " + sessionId + ".", secondException); } } - catch (Exception e) { + catch (Exception ex) { // Could be part of normal workflow (e.g. browser tab closed) - logger.debug("Failed to send message to client in " + session + ": " + message, e); + if (logger.isDebugEnabled()) { + logger.debug("Failed to send message to client in " + session + ": " + message, ex); + } } } + @Override + public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception { + this.stats.incrementTransportError(); + } + + @Override + public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) throws Exception { + clearSession(session, closeStatus); + } + + @Override + public boolean supportsPartialMessages() { + return false; + } + + + protected final SubProtocolHandler findProtocolHandler(WebSocketSession session) { + String protocol = null; + try { + protocol = session.getAcceptedProtocol(); + } + catch (Exception ex) { + // Shouldn't happen + logger.error("Failed to obtain session.getAcceptedProtocol(). " + + "Will use the default protocol handler (if configured).", ex); + } + + SubProtocolHandler handler; + if (!StringUtils.isEmpty(protocol)) { + handler = this.protocolHandlerLookup.get(protocol); + if (handler == null) { + throw new IllegalStateException( + "No handler for '" + protocol + "' among " + this.protocolHandlerLookup); + } + } + else { + if (this.defaultProtocolHandler != null) { + handler = this.defaultProtocolHandler; + } + else if (this.protocolHandlers.size() == 1) { + handler = this.protocolHandlers.iterator().next(); + } + else { + throw new IllegalStateException("Multiple protocol handlers configured and " + + "no protocol was negotiated. Consider configuring a default SubProtocolHandler."); + } + } + return handler; + } + private String resolveSessionId(Message message) { for (SubProtocolHandler handler : this.protocolHandlerLookup.values()) { String sessionId = handler.resolveSessionId(message); @@ -404,6 +434,7 @@ public class SubProtocolWebSocketHandler if (!isRunning() || (currentTime - this.lastSessionCheckTime < TIME_TO_FIRST_MESSAGE)) { return; } + if (this.sessionCheckLock.tryLock()) { try { for (WebSocketSessionHolder holder : this.sessions.values()) { @@ -423,8 +454,10 @@ public class SubProtocolWebSocketHandler this.stats.incrementNoMessagesReceivedCount(); session.close(CloseStatus.SESSION_NOT_RELIABLE); } - catch (Throwable t) { - logger.error("Failure while closing " + session, t); + catch (Throwable ex) { + if (logger.isErrorEnabled()) { + logger.error("Failure while closing " + session, ex); + } } } } @@ -435,16 +468,6 @@ public class SubProtocolWebSocketHandler } } - @Override - public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception { - this.stats.incrementTransportError(); - } - - @Override - public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) throws Exception { - clearSession(session, closeStatus); - } - private void clearSession(WebSocketSession session, CloseStatus closeStatus) throws Exception { if (logger.isDebugEnabled()) { logger.debug("Clearing session " + session.getId()); @@ -455,16 +478,13 @@ public class SubProtocolWebSocketHandler findProtocolHandler(session).afterSessionEnded(session, closeStatus, this.clientInboundChannel); } - @Override - public boolean supportsPartialMessages() { - return false; - } @Override public String toString() { - return "SubProtocolWebSocketHandler" + getProtocolHandlers(); + return "SubProtocolWebSocketHandler" + this.protocolHandlers; } + private static class WebSocketSessionHolder { private final WebSocketSession session; @@ -473,7 +493,6 @@ public class SubProtocolWebSocketHandler private volatile boolean handledMessages; - private WebSocketSessionHolder(WebSocketSession session) { this.session = session; } @@ -496,11 +515,12 @@ public class SubProtocolWebSocketHandler @Override public String toString() { - return "WebSocketSessionHolder[=session=" + this.session + ", createTime=" + + return "WebSocketSessionHolder[session=" + this.session + ", createTime=" + this.createTime + ", hasHandledMessages=" + this.handledMessages + "]"; } } + private class Stats { private final AtomicInteger total = new AtomicInteger(); @@ -517,7 +537,6 @@ public class SubProtocolWebSocketHandler private final AtomicInteger transportError = new AtomicInteger(); - public void incrementSessionCount(WebSocketSession session) { getCountFor(session).incrementAndGet(); this.total.incrementAndGet();