From fc406430331b1c681193b087998e8336e4a61445 Mon Sep 17 00:00:00 2001
From: Rossen Stoyanchev <rstoyanchev@pivotal.io>
Date: Tue, 31 May 2016 14:55:50 -0400
Subject: [PATCH] Optimize Consumes/ProducesRequestCondition

Before this change Consumes/ProducesRequestCondition shared a common
match method in the package private AbstractMediaTypeExpression. The
benefit, two lines of code, was negligible but was forcing each
condition into parsing the content type of the request body or
evaluating the content type for the response respectively.

This change removes the shared match method and brings it down into
each sub-class resulting in a performance improvement as well as in
simpler code including exception handling.

Issue: SPR-14299
---
 .../AbstractMediaTypeExpression.java          | 17 ----------
 .../condition/AbstractRequestCondition.java   |  2 +-
 .../condition/ConsumesRequestCondition.java   | 29 ++++++++---------
 .../condition/ProducesRequestCondition.java   | 31 ++++++++++++-------
 4 files changed, 36 insertions(+), 43 deletions(-)

diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractMediaTypeExpression.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractMediaTypeExpression.java
index e6708f279d..eb1257aa56 100644
--- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractMediaTypeExpression.java
+++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractMediaTypeExpression.java
@@ -16,13 +16,10 @@
 
 package org.springframework.web.servlet.mvc.condition;
 
-import javax.servlet.http.HttpServletRequest;
-
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.http.MediaType;
-import org.springframework.web.HttpMediaTypeException;
 import org.springframework.web.bind.annotation.RequestMapping;
 
 /**
@@ -69,20 +66,6 @@ abstract class AbstractMediaTypeExpression implements Comparable<AbstractMediaTy
 		return this.isNegated;
 	}
 
-
-	public final boolean match(HttpServletRequest request) {
-		try {
-			boolean match = matchMediaType(request);
-			return (!this.isNegated ? match : !match);
-		}
-		catch (HttpMediaTypeException ex) {
-			return false;
-		}
-	}
-
-	protected abstract boolean matchMediaType(HttpServletRequest request) throws HttpMediaTypeException;
-
-
 	@Override
 	public int compareTo(AbstractMediaTypeExpression other) {
 		return MediaType.SPECIFICITY_COMPARATOR.compare(this.getMediaType(), other.getMediaType());
diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractRequestCondition.java
index 92dd4e87cd..26fb5641cf 100644
--- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractRequestCondition.java
+++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractRequestCondition.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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.
diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestCondition.java
index 579ba1af35..e7a6af41e5 100644
--- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestCondition.java
+++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestCondition.java
@@ -28,6 +28,7 @@ import javax.servlet.http.HttpServletRequest;
 import org.springframework.http.InvalidMediaTypeException;
 import org.springframework.http.MediaType;
 import org.springframework.util.StringUtils;
+import org.springframework.web.HttpMediaTypeException;
 import org.springframework.web.HttpMediaTypeNotSupportedException;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.cors.CorsUtils;
@@ -170,10 +171,19 @@ public final class ConsumesRequestCondition extends AbstractRequestCondition<Con
 		if (isEmpty()) {
 			return this;
 		}
-		Set<ConsumeMediaTypeExpression> result = new LinkedHashSet<ConsumeMediaTypeExpression>(expressions);
+		MediaType contentType;
+		try {
+			contentType = StringUtils.hasLength(request.getContentType()) ?
+					MediaType.parseMediaType(request.getContentType()) :
+					MediaType.APPLICATION_OCTET_STREAM;
+		}
+		catch (InvalidMediaTypeException ex) {
+			return null;
+		}
+		Set<ConsumeMediaTypeExpression> result = new LinkedHashSet<ConsumeMediaTypeExpression>(this.expressions);
 		for (Iterator<ConsumeMediaTypeExpression> iterator = result.iterator(); iterator.hasNext();) {
 			ConsumeMediaTypeExpression expression = iterator.next();
-			if (!expression.match(request)) {
+			if (!expression.match(contentType)) {
 				iterator.remove();
 			}
 		}
@@ -221,18 +231,9 @@ public final class ConsumesRequestCondition extends AbstractRequestCondition<Con
 			super(mediaType, negated);
 		}
 
-		@Override
-		protected boolean matchMediaType(HttpServletRequest request) throws HttpMediaTypeNotSupportedException {
-			try {
-				MediaType contentType = StringUtils.hasLength(request.getContentType()) ?
-						MediaType.parseMediaType(request.getContentType()) :
-						MediaType.APPLICATION_OCTET_STREAM;
-						return getMediaType().includes(contentType);
-			}
-			catch (InvalidMediaTypeException ex) {
-				throw new HttpMediaTypeNotSupportedException(
-						"Can't parse Content-Type [" + request.getContentType() + "]: " + ex.getMessage());
-			}
+		public final boolean match(MediaType contentType) {
+			boolean match = getMediaType().includes(contentType);
+			return (!isNegated() ? match : !match);
 		}
 	}
 
diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java
index b8bdc5f5f5..55931d4bfd 100644
--- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java
+++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java
@@ -49,6 +49,8 @@ public final class ProducesRequestCondition extends AbstractRequestCondition<Pro
 
 	private final static ProducesRequestCondition PRE_FLIGHT_MATCH = new ProducesRequestCondition();
 
+	private static final ProducesRequestCondition EMPTY_CONDITION = new ProducesRequestCondition();
+
 
 	private final List<ProduceMediaTypeExpression> MEDIA_TYPE_ALL_LIST =
 			Collections.singletonList(new ProduceMediaTypeExpression("*/*"));
@@ -187,25 +189,29 @@ public final class ProducesRequestCondition extends AbstractRequestCondition<Pro
 		if (isEmpty()) {
 			return this;
 		}
+		List<MediaType> acceptedMediaTypes;
+		try {
+			acceptedMediaTypes = getAcceptedMediaTypes(request);
+		}
+		catch (HttpMediaTypeException ex) {
+			return null;
+		}
 		Set<ProduceMediaTypeExpression> result = new LinkedHashSet<ProduceMediaTypeExpression>(expressions);
 		for (Iterator<ProduceMediaTypeExpression> iterator = result.iterator(); iterator.hasNext();) {
 			ProduceMediaTypeExpression expression = iterator.next();
-			if (!expression.match(request)) {
+			if (!expression.match(acceptedMediaTypes)) {
 				iterator.remove();
 			}
 		}
 		if (!result.isEmpty()) {
 			return new ProducesRequestCondition(result, this.contentNegotiationManager);
 		}
-		try {
-			if (getAcceptedMediaTypes(request).contains(MediaType.ALL)) {
-				return new ProducesRequestCondition();
-			}
+		else if (acceptedMediaTypes.contains(MediaType.ALL)) {
+			return EMPTY_CONDITION;
 		}
-		catch (HttpMediaTypeException ex) {
-			// Ignore
+		else {
+			return null;
 		}
-		return null;
 	}
 
 	/**
@@ -314,9 +320,12 @@ public final class ProducesRequestCondition extends AbstractRequestCondition<Pro
 			super(expression);
 		}
 
-		@Override
-		protected boolean matchMediaType(HttpServletRequest request) throws HttpMediaTypeNotAcceptableException {
-			List<MediaType> acceptedMediaTypes = getAcceptedMediaTypes(request);
+		public final boolean match(List<MediaType> acceptedMediaTypes) {
+			boolean match = matchMediaType(acceptedMediaTypes);
+			return (!isNegated() ? match : !match);
+		}
+
+		private boolean matchMediaType(List<MediaType> acceptedMediaTypes) {
 			for (MediaType acceptedMediaType : acceptedMediaTypes) {
 				if (getMediaType().isCompatibleWith(acceptedMediaType)) {
 					return true;