Objects with multi-threaded access should not lazily populate a hash field

Issue. SPR-11428

spring-core/src/main/java/org/springframework/core/MethodParameter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -64,8 +64,6 @@ public class MethodParameter {
 	/** Map from Integer level to Integer type index */
 	Map<Integer, Integer> typeIndexesPerLevel;
 
-	private int hash = 0;
-
 
 	/**
 	 * Create a new MethodParameter for the given method, with nesting level 1.
@@ -137,7 +135,6 @@ public class MethodParameter {
 		this.parameterName = original.parameterName;
 		this.nestingLevel = original.nestingLevel;
 		this.typeIndexesPerLevel = original.typeIndexesPerLevel;
-		this.hash = original.hash;
 	}
 
 
@@ -450,29 +447,14 @@ public class MethodParameter {
 		}
 		if (obj != null && obj instanceof MethodParameter) {
 			MethodParameter other = (MethodParameter) obj;
-
-			if (this.parameterIndex != other.parameterIndex) {
-				return false;
-			}
-			else if (this.getMember().equals(other.getMember())) {
-				return true;
-			}
-			else {
-				return false;
-			}
+			return (this.parameterIndex == other.parameterIndex && getMember().equals(other.getMember()));
 		}
 		return false;
 	}
 
 	@Override
 	public int hashCode() {
-		int result = this.hash;
-		if (result == 0) {
-			result = getMember().hashCode();
-			result = 31 * result + this.parameterIndex;
-			this.hash = result;
-		}
-		return result;
+		return (getMember().hashCode() * 31 + this.parameterIndex);
 	}
 
 

spring-messaging/src/main/java/org/springframework/messaging/simp/SimpMessageMappingInfo.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -36,8 +36,6 @@ public class SimpMessageMappingInfo implements MessageCondition<SimpMessageMappi
 
 	private final DestinationPatternsMessageCondition destinationConditions;
 
-	private int hash;
-
 
 	public SimpMessageMappingInfo(SimpMessageTypeMessageCondition messageTypeMessageCondition,
 			DestinationPatternsMessageCondition destinationConditions) {
@@ -58,13 +56,10 @@ public class SimpMessageMappingInfo implements MessageCondition<SimpMessageMappi
 
 	@Override
 	public SimpMessageMappingInfo combine(SimpMessageMappingInfo other) {
-
 		SimpMessageTypeMessageCondition typeCond =
 				this.getMessageTypeMessageCondition().combine(other.getMessageTypeMessageCondition());
-
 		DestinationPatternsMessageCondition destCond =
 				this.destinationConditions.combine(other.getDestinationConditions());
-
 		return new SimpMessageMappingInfo(typeCond, destCond);
 	}
 
@@ -102,20 +97,15 @@ public class SimpMessageMappingInfo implements MessageCondition<SimpMessageMappi
 		}
 		if (obj != null && obj instanceof SimpMessageMappingInfo) {
 			SimpMessageMappingInfo other = (SimpMessageMappingInfo) obj;
-			return this.destinationConditions.equals(other.destinationConditions);
+			return (this.destinationConditions.equals(other.destinationConditions) &&
+					this.messageTypeMessageCondition.equals(other.messageTypeMessageCondition));
 		}
 		return false;
 	}
 
 	@Override
 	public int hashCode() {
-		int result = hash;
-		if (result == 0) {
-			result = destinationConditions.hashCode();
-			result = 31 * result + messageTypeMessageCondition.hashCode();
-			hash = result;
-		}
-		return result;
+		return (this.destinationConditions.hashCode() * 31 + this.messageTypeMessageCondition.hashCode());
 	}
 
 	@Override

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -36,7 +36,7 @@ import org.springframework.web.servlet.mvc.condition.RequestMethodsRequestCondit
  * 	<li>{@link HeadersRequestCondition}
  * 	<li>{@link ConsumesRequestCondition}
  * 	<li>{@link ProducesRequestCondition}
- * 	<li>{@code RequestCondition<?>} (optional, custom request condition)
+ * 	<li>{@code RequestCondition} (optional, custom request condition)
  * </ol>
  *
  * @author Arjen Poutsma
@@ -59,24 +59,20 @@ public final class RequestMappingInfo implements RequestCondition<RequestMapping
 
 	private final RequestConditionHolder customConditionHolder;
 
-	private int hash;
 
 	/**
 	 * Creates a new instance with the given request conditions.
 	 */
-	public RequestMappingInfo(PatternsRequestCondition patterns,
-							  RequestMethodsRequestCondition methods,
-							  ParamsRequestCondition params,
-							  HeadersRequestCondition headers,
-							  ConsumesRequestCondition consumes,
-							  ProducesRequestCondition produces,
-							  RequestCondition<?> custom) {
-		this.patternsCondition = patterns != null ? patterns : new PatternsRequestCondition();
-		this.methodsCondition = methods != null ? methods : new RequestMethodsRequestCondition();
-		this.paramsCondition = params != null ? params : new ParamsRequestCondition();
-		this.headersCondition = headers != null ? headers : new HeadersRequestCondition();
-		this.consumesCondition = consumes != null ? consumes : new ConsumesRequestCondition();
-		this.producesCondition = produces != null ? produces : new ProducesRequestCondition();
+	public RequestMappingInfo(PatternsRequestCondition patterns, RequestMethodsRequestCondition methods,
+			ParamsRequestCondition params, HeadersRequestCondition headers, ConsumesRequestCondition consumes,
+			ProducesRequestCondition produces, RequestCondition<?> custom) {
+
+		this.patternsCondition = (patterns != null ? patterns : new PatternsRequestCondition());
+		this.methodsCondition = (methods != null ? methods : new RequestMethodsRequestCondition());
+		this.paramsCondition = (params != null ? params : new ParamsRequestCondition());
+		this.headersCondition = (headers != null ? headers : new HeadersRequestCondition());
+		this.consumesCondition = (consumes != null ? consumes : new ConsumesRequestCondition());
+		this.producesCondition = (produces != null ? produces : new ProducesRequestCondition());
 		this.customConditionHolder = new RequestConditionHolder(custom);
 	}
 
@@ -88,61 +84,63 @@ public final class RequestMappingInfo implements RequestCondition<RequestMapping
 				info.consumesCondition, info.producesCondition, customRequestCondition);
 	}
 
+
 	/**
 	 * Returns the URL patterns of this {@link RequestMappingInfo};
-	 * or instance with 0 patterns, never {@code null}
+	 * or instance with 0 patterns, never {@code null}.
 	 */
 	public PatternsRequestCondition getPatternsCondition() {
-		return patternsCondition;
+		return this.patternsCondition;
 	}
 
 	/**
 	 * Returns the HTTP request methods of this {@link RequestMappingInfo};
-	 * or instance with 0 request methods, never {@code null}
+	 * or instance with 0 request methods, never {@code null}.
 	 */
 	public RequestMethodsRequestCondition getMethodsCondition() {
-		return methodsCondition;
+		return this.methodsCondition;
 	}
 
 	/**
 	 * Returns the "parameters" condition of this {@link RequestMappingInfo};
-	 * or instance with 0 parameter expressions, never {@code null}
+	 * or instance with 0 parameter expressions, never {@code null}.
 	 */
 	public ParamsRequestCondition getParamsCondition() {
-		return paramsCondition;
+		return this.paramsCondition;
 	}
 
 	/**
 	 * Returns the "headers" condition of this {@link RequestMappingInfo};
-	 * or instance with 0 header expressions, never {@code null}
+	 * or instance with 0 header expressions, never {@code null}.
 	 */
 	public HeadersRequestCondition getHeadersCondition() {
-		return headersCondition;
+		return this.headersCondition;
 	}
 
 	/**
 	 * Returns the "consumes" condition of this {@link RequestMappingInfo};
-	 * or instance with 0 consumes expressions, never {@code null}
+	 * or instance with 0 consumes expressions, never {@code null}.
 	 */
 	public ConsumesRequestCondition getConsumesCondition() {
-		return consumesCondition;
+		return this.consumesCondition;
 	}
 
 	/**
 	 * Returns the "produces" condition of this {@link RequestMappingInfo};
-	 * or instance with 0 produces expressions, never {@code null}
+	 * or instance with 0 produces expressions, never {@code null}.
 	 */
 	public ProducesRequestCondition getProducesCondition() {
-		return producesCondition;
+		return this.producesCondition;
 	}
 
 	/**
-	 * Returns the "custom" condition of this {@link RequestMappingInfo}; or {@code null}
+	 * Returns the "custom" condition of this {@link RequestMappingInfo}; or {@code null}.
 	 */
 	public RequestCondition<?> getCustomCondition() {
-		return customConditionHolder.getCondition();
+		return this.customConditionHolder.getCondition();
 	}
 
+
 	/**
 	 * Combines "this" request mapping info (i.e. the current instance) with another request mapping info instance.
 	 * <p>Example: combine type- and method-level request mappings.
@@ -170,22 +168,22 @@ public final class RequestMappingInfo implements RequestCondition<RequestMapping
 	 */
 	@Override
 	public RequestMappingInfo getMatchingCondition(HttpServletRequest request) {
-		RequestMethodsRequestCondition methods = methodsCondition.getMatchingCondition(request);
-		ParamsRequestCondition params = paramsCondition.getMatchingCondition(request);
-		HeadersRequestCondition headers = headersCondition.getMatchingCondition(request);
-		ConsumesRequestCondition consumes = consumesCondition.getMatchingCondition(request);
-		ProducesRequestCondition produces = producesCondition.getMatchingCondition(request);
+		RequestMethodsRequestCondition methods = this.methodsCondition.getMatchingCondition(request);
+		ParamsRequestCondition params = this.paramsCondition.getMatchingCondition(request);
+		HeadersRequestCondition headers = this.headersCondition.getMatchingCondition(request);
+		ConsumesRequestCondition consumes = this.consumesCondition.getMatchingCondition(request);
+		ProducesRequestCondition produces = this.producesCondition.getMatchingCondition(request);
 
 		if (methods == null || params == null || headers == null || consumes == null || produces == null) {
 			return null;
 		}
 
-		PatternsRequestCondition patterns = patternsCondition.getMatchingCondition(request);
+		PatternsRequestCondition patterns = this.patternsCondition.getMatchingCondition(request);
 		if (patterns == null) {
 			return null;
 		}
 
-		RequestConditionHolder custom = customConditionHolder.getMatchingCondition(request);
+		RequestConditionHolder custom = this.customConditionHolder.getMatchingCondition(request);
 		if (custom == null) {
 			return null;
 		}
@@ -193,39 +191,40 @@ public final class RequestMappingInfo implements RequestCondition<RequestMapping
 		return new RequestMappingInfo(patterns, methods, params, headers, consumes, produces, custom.getCondition());
 	}
 
+
 	/**
 	 * Compares "this" info (i.e. the current instance) with another info in the context of a request.
-	 * <p>Note: it is assumed both instances have been obtained via
+	 * <p>Note: It is assumed both instances have been obtained via
 	 * {@link #getMatchingCondition(HttpServletRequest)} to ensure they have conditions with
 	 * content relevant to current request.
 	 */
 	@Override
 	public int compareTo(RequestMappingInfo other, HttpServletRequest request) {
-		int result = patternsCondition.compareTo(other.getPatternsCondition(), request);
+		int result = this.patternsCondition.compareTo(other.getPatternsCondition(), request);
 		if (result != 0) {
 			return result;
 		}
-		result = paramsCondition.compareTo(other.getParamsCondition(), request);
+		result = this.paramsCondition.compareTo(other.getParamsCondition(), request);
 		if (result != 0) {
 			return result;
 		}
-		result = headersCondition.compareTo(other.getHeadersCondition(), request);
+		result = this.headersCondition.compareTo(other.getHeadersCondition(), request);
 		if (result != 0) {
 			return result;
 		}
-		result = consumesCondition.compareTo(other.getConsumesCondition(), request);
+		result = this.consumesCondition.compareTo(other.getConsumesCondition(), request);
 		if (result != 0) {
 			return result;
 		}
-		result = producesCondition.compareTo(other.getProducesCondition(), request);
+		result = this.producesCondition.compareTo(other.getProducesCondition(), request);
 		if (result != 0) {
 			return result;
 		}
-		result = methodsCondition.compareTo(other.getMethodsCondition(), request);
+		result = this.methodsCondition.compareTo(other.getMethodsCondition(), request);
 		if (result != 0) {
 			return result;
 		}
-		result = customConditionHolder.compareTo(other.customConditionHolder, request);
+		result = this.customConditionHolder.compareTo(other.customConditionHolder, request);
 		if (result != 0) {
 			return result;
 		}
@@ -252,30 +251,22 @@ public final class RequestMappingInfo implements RequestCondition<RequestMapping
 
 	@Override
 	public int hashCode() {
-		int result = hash;
-		if (result == 0) {
-			result = patternsCondition.hashCode();
-			result = 31 * result + methodsCondition.hashCode();
-			result = 31 * result + paramsCondition.hashCode();
-			result = 31 * result + headersCondition.hashCode();
-			result = 31 * result + consumesCondition.hashCode();
-			result = 31 * result + producesCondition.hashCode();
-			result = 31 * result + customConditionHolder.hashCode();
-			hash = result;
-		}
-		return result;
+		return (this.patternsCondition.hashCode() * 31 +  // primary differentiation
+				this.methodsCondition.hashCode() + this.paramsCondition.hashCode() +
+				this.headersCondition.hashCode() + this.consumesCondition.hashCode() +
+				this.producesCondition.hashCode() + this.customConditionHolder.hashCode());
 	}
 
 	@Override
 	public String toString() {
 		StringBuilder builder = new StringBuilder("{");
-		builder.append(patternsCondition);
-		builder.append(",methods=").append(methodsCondition);
-		builder.append(",params=").append(paramsCondition);
-		builder.append(",headers=").append(headersCondition);
-		builder.append(",consumes=").append(consumesCondition);
-		builder.append(",produces=").append(producesCondition);
-		builder.append(",custom=").append(customConditionHolder);
+		builder.append(this.patternsCondition);
+		builder.append(",methods=").append(this.methodsCondition);
+		builder.append(",params=").append(this.paramsCondition);
+		builder.append(",headers=").append(this.headersCondition);
+		builder.append(",consumes=").append(this.consumesCondition);
+		builder.append(",produces=").append(this.producesCondition);
+		builder.append(",custom=").append(this.customConditionHolder);
 		builder.append('}');
 		return builder.toString();
 	}

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -16,18 +16,12 @@
 
 package org.springframework.web.servlet.mvc.method;
 
-import static java.util.Arrays.asList;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 
 import org.junit.Test;
+
 import org.springframework.mock.web.test.MockHttpServletRequest;
 import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.servlet.mvc.condition.ConsumesRequestCondition;
@@ -37,6 +31,9 @@ import org.springframework.web.servlet.mvc.condition.PatternsRequestCondition;
 import org.springframework.web.servlet.mvc.condition.ProducesRequestCondition;
 import org.springframework.web.servlet.mvc.condition.RequestMethodsRequestCondition;
 
+import static java.util.Arrays.*;
+import static org.junit.Assert.*;
+
 /**
  * Test fixture for {@link RequestMappingInfo} tests.
  *
@@ -212,7 +209,6 @@ public class RequestMappingInfoTests {
 
 	@Test
 	public void equals() {
-
 		RequestMappingInfo info1 = new RequestMappingInfo(
 				new PatternsRequestCondition("/foo"),
 				new RequestMethodsRequestCondition(RequestMethod.GET),
@@ -244,7 +240,7 @@ public class RequestMappingInfoTests {
 				new ParamsRequestCondition("customFoo=customBar"));
 
 		assertFalse(info1.equals(info2));
-		assertTrue(info1.hashCode() != info2.hashCode());
+		assertNotEquals(info1.hashCode(), info2.hashCode());
 
 		info2 = new RequestMappingInfo(
 				new PatternsRequestCondition("/foo"),
@@ -256,7 +252,7 @@ public class RequestMappingInfoTests {
 				new ParamsRequestCondition("customFoo=customBar"));
 
 		assertFalse(info1.equals(info2));
-		assertTrue(info1.hashCode() != info2.hashCode());
+		assertNotEquals(info1.hashCode(), info2.hashCode());
 
 		info2 = new RequestMappingInfo(
 				new PatternsRequestCondition("/foo"),
@@ -268,7 +264,7 @@ public class RequestMappingInfoTests {
 				new ParamsRequestCondition("customFoo=customBar"));
 
 		assertFalse(info1.equals(info2));
-		assertTrue(info1.hashCode() != info2.hashCode());
+		assertNotEquals(info1.hashCode(), info2.hashCode());
 
 		info2 = new RequestMappingInfo(
 				new PatternsRequestCondition("/foo"),
@@ -280,7 +276,7 @@ public class RequestMappingInfoTests {
 				new ParamsRequestCondition("customFoo=customBar"));
 
 		assertFalse(info1.equals(info2));
-		assertTrue(info1.hashCode() != info2.hashCode());
+		assertNotEquals(info1.hashCode(), info2.hashCode());
 
 		info2 = new RequestMappingInfo(
 				new PatternsRequestCondition("/foo"),
@@ -292,7 +288,7 @@ public class RequestMappingInfoTests {
 				new ParamsRequestCondition("customFoo=customBar"));
 
 		assertFalse(info1.equals(info2));
-		assertTrue(info1.hashCode() != info2.hashCode());
+		assertNotEquals(info1.hashCode(), info2.hashCode());
 
 		info2 = new RequestMappingInfo(
 				new PatternsRequestCondition("/foo"),
@@ -304,7 +300,7 @@ public class RequestMappingInfoTests {
 				new ParamsRequestCondition("customFoo=customBar"));
 
 		assertFalse(info1.equals(info2));
-		assertTrue(info1.hashCode() != info2.hashCode());
+		assertNotEquals(info1.hashCode(), info2.hashCode());
 
 		info2 = new RequestMappingInfo(
 				new PatternsRequestCondition("/foo"),
@@ -316,7 +312,7 @@ public class RequestMappingInfoTests {
 				new ParamsRequestCondition("customFoo=NOOOOOO"));
 
 		assertFalse(info1.equals(info2));
-		assertTrue(info1.hashCode() != info2.hashCode());
+		assertNotEquals(info1.hashCode(), info2.hashCode());
 	}
 
-}
+}