Portlet mapping predicate compareTo implementations are transitive now

Also removed unused validateHandler code with dead cachedMappings HashMap.

Issue: SPR-9874

org.springframework.web.portlet/src/main/java/org/springframework/web/portlet/handler/AbstractHandlerMapping.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2008 the original author or authors.
+ * Copyright 2002-2012 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.
@@ -19,7 +19,6 @@ package org.springframework.web.portlet.handler;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
-
 import javax.portlet.PortletRequest;
 
 import org.springframework.beans.BeansException;
@@ -42,8 +41,7 @@ import org.springframework.web.portlet.HandlerMapping;
  * @see #setInterceptors
  * @see org.springframework.web.portlet.HandlerInterceptor
  */
-public abstract class AbstractHandlerMapping extends ApplicationObjectSupport
+public abstract class AbstractHandlerMapping extends ApplicationObjectSupport implements HandlerMapping, Ordered {
-	implements HandlerMapping, Ordered {
 
 	private int order = Integer.MAX_VALUE;  // default: same as non-Ordered
 
@@ -244,7 +242,7 @@ public abstract class AbstractHandlerMapping extends ApplicationObjectSupport
 	 * <p>For simply adding an interceptor, consider calling <code>super.getHandlerExecutionChain</code>
 	 * and invoking {@link HandlerExecutionChain#addInterceptor} on the returned chain object.
 	 * @param handler the resolved handler instance (never <code>null</code>)
-	 * @param request current HTTP request
+	 * @param request current portlet request
 	 * @return the HandlerExecutionChain (never <code>null</code>)
 	 * @see #getAdaptedInterceptors()
 	 */

org.springframework.web.portlet/src/main/java/org/springframework/web/portlet/mvc/annotation/DefaultAnnotationHandlerMapping.java

@@ -92,9 +92,6 @@ import org.springframework.web.portlet.handler.PortletRequestMethodNotSupportedE
  */
 public class DefaultAnnotationHandlerMapping extends AbstractMapBasedHandlerMapping<PortletMode> {
 
-	private final Map<Class, RequestMapping> cachedMappings = new HashMap<Class, RequestMapping>();
-
-
 	/**
 	 * Calls the <code>registerHandlers</code> method in addition
 	 * to the superclass's initialization.
@@ -118,7 +115,6 @@ public class DefaultAnnotationHandlerMapping extends AbstractMapBasedHandlerMapp
 			RequestMapping mapping = context.findAnnotationOnBean(beanName, RequestMapping.class);
 			if (mapping != null) {
 				// @RequestMapping found at type level
-				this.cachedMappings.put(handlerType, mapping);
 				String[] modeKeys = mapping.value();
 				String[] params = mapping.params();
 				boolean registerHandlerType = true;
@@ -222,49 +218,6 @@ public class DefaultAnnotationHandlerMapping extends AbstractMapBasedHandlerMapp
 		return request.getPortletMode();
 	}
 
-	/**
-	 * Validate the given annotated handler against the current request.
-	 * @see #validateMapping
-	 */
-	protected void validateHandler(Object handler, PortletRequest request) throws Exception {
-		RequestMapping mapping = this.cachedMappings.get(handler.getClass());
-		if (mapping == null) {
-			mapping = AnnotationUtils.findAnnotation(handler.getClass(), RequestMapping.class);
-		}
-		if (mapping != null) {
-			validateMapping(mapping, request);
-		}
-	}
-
-	/**
-	 * Validate the given type-level mapping metadata against the current request,
-	 * checking request method and parameter conditions.
-	 * @param mapping the mapping metadata to validate
-	 * @param request current portlet request
-	 * @throws Exception if validation failed
-	 */
-	protected void validateMapping(RequestMapping mapping, PortletRequest request) throws Exception {
-		RequestMethod[] mappedMethods = mapping.method();
-		if (!PortletAnnotationMappingUtils.checkRequestMethod(mappedMethods, request)) {
-			String[] supportedMethods = new String[mappedMethods.length];
-			for (int i = 0; i < mappedMethods.length; i++) {
-				supportedMethods[i] = mappedMethods[i].name();
-			}
-			if (request instanceof ClientDataRequest) {
-				throw new PortletRequestMethodNotSupportedException(((ClientDataRequest) request).getMethod(), supportedMethods);
-			}
-			else {
-				throw new PortletRequestMethodNotSupportedException(supportedMethods);
-			}
-		}
-		String[] mappedHeaders = mapping.headers();
-		if (!PortletAnnotationMappingUtils.checkHeaders(mappedHeaders, request)) {
-			throw new PortletRequestBindingException("Header conditions \"" +
-					StringUtils.arrayToDelimitedString(mappedHeaders, ", ") +
-					"\" not met for actual request");
-		}
-	}
-
 
 	private interface SpecialRequestTypePredicate {
 	}
@@ -383,7 +336,10 @@ public class DefaultAnnotationHandlerMapping extends AbstractMapBasedHandlerMapp
 					return compareParams(otherAction);
 				}
 			}
-			return (other instanceof SpecialRequestTypePredicate ? compareParams(other) : -1);
+			if (other instanceof SpecialRequestTypePredicate) {
+				return this.getClass().getName().compareTo(other.getClass().getName());
+			}
+			return -1;
 		}
 	}
 
@@ -422,7 +378,10 @@ public class DefaultAnnotationHandlerMapping extends AbstractMapBasedHandlerMapp
 					return compareParams(otherRender);
 				}
 			}
-			return (other instanceof SpecialRequestTypePredicate ? compareParams(other) : -1);
+			if (other instanceof SpecialRequestTypePredicate) {
+				return this.getClass().getName().compareTo(other.getClass().getName());
+			}
+            return -1;
 		}
 	}
 
@@ -451,7 +410,10 @@ public class DefaultAnnotationHandlerMapping extends AbstractMapBasedHandlerMapp
 					return (hasResourceId ? -1 : 1);
 				}
 			}
-			return (other instanceof SpecialRequestTypePredicate ? 0 : -1);
+			if (other instanceof SpecialRequestTypePredicate) {
+				return this.getClass().getName().compareTo(other.getClass().getName());
+			}
+            return -1;
 		}
 	}
 
@@ -486,7 +448,10 @@ public class DefaultAnnotationHandlerMapping extends AbstractMapBasedHandlerMapp
 					return (hasEventName ? -1 : 1);
 				}
 			}
-			return (other instanceof SpecialRequestTypePredicate ? 0 : -1);
+			if (other instanceof SpecialRequestTypePredicate) {
+				return this.getClass().getName().compareTo(other.getClass().getName());
+			}
+			return -1;
 		}
 	}
 

org.springframework.web.portlet/src/test/java/org/springframework/web/portlet/mvc/annotation/Portlet20AnnotationControllerTests.java

@@ -33,7 +33,6 @@ import javax.portlet.PortletRequest;
 import javax.portlet.PortletSession;
 import javax.portlet.RenderRequest;
 import javax.portlet.RenderResponse;
-import javax.portlet.ResourceRequest;
 import javax.portlet.ResourceResponse;
 import javax.portlet.StateAwareResponse;
 import javax.portlet.WindowState;
@@ -704,6 +703,7 @@ public class Portlet20AnnotationControllerTests {
 				// the collection with [Render,Action,Render] predicates
 				wac.registerSingleton("firstController", FirstController.class);
 				wac.registerSingleton("secondController", SecondController.class);
+				wac.registerSingleton("thirdController", ThirdController.class);
 				wac.registerSingleton("handlerMapping", DefaultAnnotationHandlerMapping.class);
 				wac.registerSingleton("handlerAdapter", AnnotationMethodHandlerAdapter.class);
 				wac.setPortletContext(new MockPortletContext());
@@ -714,11 +714,44 @@ public class Portlet20AnnotationControllerTests {
 		};
 		portlet.init(new MockPortletConfig());
 
-		// Prepare render request with 'page=baz' parameters
+		// Make sure all 6 annotated methods can be called
+
 		MockRenderRequest request = new MockRenderRequest(PortletMode.VIEW);
 		MockRenderResponse response = new MockRenderResponse();
-		request.addParameter("page", "baz");
+
+		// renderFirst
+		portlet.render(request, response);
+		assertArrayEquals(new String[] { "renderFirst" }, response.getProperties("RESPONSE"));
+
+		// renderSecond
+		request.setWindowState(WindowState.MAXIMIZED);
+		request.setParameter("report", "second");
+		portlet.render(request, response);
+		assertArrayEquals(new String[] { "renderSecond" }, response.getProperties("RESPONSE"));
+
+		// renderThirds
+		request.setWindowState(WindowState.MAXIMIZED);
+		request.setParameter("report", "third");
 		portlet.render(request, response);
+		assertArrayEquals(new String[] { "renderThird" }, response.getProperties("RESPONSE"));
+
+		MockResourceRequest resourceRequest;
+		MockResourceResponse resourceResponse = new MockResourceResponse();
+
+		// resourceFirst
+		resourceRequest = new MockResourceRequest("first");
+		portlet.serveResource(resourceRequest, resourceResponse);
+		assertArrayEquals(new String[] { "resourceFirst" }, resourceResponse.getProperties("RESPONSE"));
+
+		// resourceSecond
+		resourceRequest = new MockResourceRequest("second");
+		portlet.serveResource(resourceRequest, resourceResponse);
+		assertArrayEquals(new String[] { "resourceSecond" }, resourceResponse.getProperties("RESPONSE"));
+
+		// resourceThirds
+		resourceRequest = new MockResourceRequest("third");
+		portlet.serveResource(resourceRequest, resourceResponse);
+		assertArrayEquals(new String[] { "resourceThird" }, resourceResponse.getProperties("RESPONSE"));
 	}
 
 
@@ -1201,13 +1234,15 @@ public class Portlet20AnnotationControllerTests {
 	public static class FirstController {
 
 		@RenderMapping
-		public String renderBar() {
+		public String renderFirst(RenderResponse response) {
-			throw new UnsupportedOperationException("Should not be called");
+			response.setProperty("RESPONSE", "renderFirst");
+			return "renderFirst";
 		}
 
-		@ActionMapping("xyz")
+		@ResourceMapping("first")
-		public void processXyz() {
+		public String resourceFirst(ResourceResponse response) {
-			throw new UnsupportedOperationException("Should not be called");
+			response.setProperty("RESPONSE", "resourceFirst");
+			return "resourceFirst";
 		}
 	}
 
@@ -1215,14 +1250,33 @@ public class Portlet20AnnotationControllerTests {
 	@RequestMapping(value="view")
 	public static class SecondController {
 
-		@ResourceMapping
+		@ResourceMapping("second")
-		public void processResource(ResourceRequest request, ResourceResponse response) {
+		public String processResource(ResourceResponse response) {
-			throw new UnsupportedOperationException("Should not be called");
+			response.setProperty("RESPONSE", "resourceSecond");
+			return "resourceSecond";
+		}
+
+		@RenderMapping(value = "MAXIMIZED", params = "report=second")
+		public String renderSecond(RenderResponse response) {
+			response.setProperty("RESPONSE", "renderSecond");
+			return "renderSecond";
+		}
+	}
+
+
+	@RequestMapping(value="view")
+	public static class ThirdController {
+
+		@ResourceMapping("third")
+		public String processResource(ResourceResponse response) {
+			response.setProperty("RESPONSE", "resourceThird");
+			return "resourceThird";
 		}
 
-		@RenderMapping(params="page=baz")
+		@RenderMapping(value = "MAXIMIZED", params = "report=third")
-		public String renderBaz() {
+		public String renderSecond(RenderResponse response) {
-			return "SUCCESS";
+			response.setProperty("RESPONSE", "renderThird");
+			return "renderThird";
 		}
 	}