Refactoring PortResolverImpl - simpler code and remove InitializingBean implementation.

core/src/main/java/org/springframework/security/util/PortResolverImpl.java

@@ -15,65 +15,58 @@
 
 package org.springframework.security.util;
 
-import org.springframework.beans.factory.InitializingBean;
-
 import org.springframework.util.Assert;
 
 import javax.servlet.ServletRequest;
 
 
 /**
- * Concrete implementation of {@link PortResolver} that obtains the port from
+ * Concrete implementation of {@link PortResolver} that obtains the port from <tt>ServletRequest.getServerPort()</tt>.
- * <code>ServletRequest.getServerPort()</code>.<P>This class is capable of handling the IE bug which results in an
+ * <p>
+ * This class is capable of handling the IE bug which results in an
  * incorrect URL being presented in the header subsequent to a redirect to a different scheme and port where the port
  * is not a well-known number (ie 80 or 443). Handling involves detecting an incorrect response from
  * <code>ServletRequest.getServerPort()</code> for the scheme (eg a HTTP request on 8443) and then determining the
- * real server port (eg HTTP request is really on 8080). The map of valid ports is obtained from the configured {@link
+ * real server port (eg HTTP request is really on 8080). The map of valid ports is obtained from the configured
- * PortMapper}.</p>
+ * {@link PortMapper}.
  *
  * @author Ben Alex
  * @version $Id$
  */
-public class PortResolverImpl implements InitializingBean, PortResolver {
+public class PortResolverImpl implements PortResolver {
     //~ Instance fields ================================================================================================
 
     private PortMapper portMapper = new PortMapperImpl();
 
     //~ Methods ========================================================================================================
 
-    public void afterPropertiesSet() throws Exception {
-        Assert.notNull(portMapper, "portMapper required");
-    }
-
     public PortMapper getPortMapper() {
         return portMapper;
     }
 
     public int getServerPort(ServletRequest request) {
-        int result = request.getServerPort();
+        int serverPort = request.getServerPort();
+        Integer portLookup = null;
 
-        if ("http".equals(request.getScheme().toLowerCase())) {
+        String scheme = request.getScheme().toLowerCase();
-            Integer http = portMapper.lookupHttpPort(new Integer(result));
 
-            if (http != null) {
+        if ("http".equals(scheme)) {
-                // IE 6 bug
+            portLookup = portMapper.lookupHttpPort(Integer.valueOf(serverPort));
-                result = http.intValue();
+
-            }
+        } else if ("https".equals(scheme)) {
+            portLookup = portMapper.lookupHttpsPort(Integer.valueOf(serverPort));
         }
 
-        if ("https".equals(request.getScheme().toLowerCase())) {
+        if (portLookup != null) {
-            Integer https = portMapper.lookupHttpsPort(new Integer(result));
+            // IE 6 bug
-
+            serverPort = portLookup.intValue();
-            if (https != null) {
-                // IE 6 bug
-                result = https.intValue();
-            }
         }
 
-        return result;
+        return serverPort;
     }
 
     public void setPortMapper(PortMapper portMapper) {
+        Assert.notNull(portMapper, "portMapper cannot be null");
         this.portMapper = portMapper;
     }
 }

core/src/test/java/org/springframework/security/util/PortResolverImplTests.java

@@ -39,17 +39,12 @@ public class PortResolverImplTests extends TestCase {
 
     //~ Methods ========================================================================================================
 
-    public static void main(String[] args) {
-        junit.textui.TestRunner.run(PortResolverImplTests.class);
-    }
-
     public final void setUp() throws Exception {
         super.setUp();
     }
 
     public void testDetectsBuggyIeHttpRequest() throws Exception {
         PortResolverImpl pr = new PortResolverImpl();
-        pr.afterPropertiesSet();
 
         MockHttpServletRequest request = new MockHttpServletRequest();
         request.setServerPort(8443);
@@ -59,7 +54,6 @@ public class PortResolverImplTests extends TestCase {
 
     public void testDetectsBuggyIeHttpsRequest() throws Exception {
         PortResolverImpl pr = new PortResolverImpl();
-        pr.afterPropertiesSet();
 
         MockHttpServletRequest request = new MockHttpServletRequest();
         request.setServerPort(8080);
@@ -69,10 +63,9 @@ public class PortResolverImplTests extends TestCase {
 
     public void testDetectsEmptyPortMapper() throws Exception {
         PortResolverImpl pr = new PortResolverImpl();
-        pr.setPortMapper(null);
 
         try {
-            pr.afterPropertiesSet();
+            pr.setPortMapper(null);
             fail("Should have thrown IllegalArgumentException");
         } catch (IllegalArgumentException expected) {
             assertTrue(true);
@@ -88,7 +81,6 @@ public class PortResolverImplTests extends TestCase {
 
     public void testNormalOperation() throws Exception {
         PortResolverImpl pr = new PortResolverImpl();
-        pr.afterPropertiesSet();
 
         MockHttpServletRequest request = new MockHttpServletRequest();
         request.setScheme("http");