Polish Sorting ObjectPostProcessor

* Add Test
* Only sort on adding new entry

Issue gh-3572

config/src/main/java/org/springframework/security/config/annotation/SecurityConfigurerAdapter.java

@@ -15,13 +15,13 @@
  */
 package org.springframework.security.config.annotation;
 
-import org.springframework.core.GenericTypeResolver;
-import org.springframework.core.annotation.AnnotationAwareOrderComparator;
-
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
+import org.springframework.core.GenericTypeResolver;
+import org.springframework.core.annotation.AnnotationAwareOrderComparator;
+
 /**
  * A base class for {@link SecurityConfigurer} that allows subclasses to only implement
  * the methods they are interested in. It also provides a mechanism for using the
@@ -115,7 +115,6 @@ public abstract class SecurityConfigurerAdapter<O, B extends SecurityBuilder<O>>
 
 		@SuppressWarnings({ "rawtypes", "unchecked" })
 		public Object postProcess(Object object) {
-			Collections.sort(postProcessors, AnnotationAwareOrderComparator.INSTANCE);
 			for (ObjectPostProcessor opp : postProcessors) {
 				Class<?> oppClass = opp.getClass();
 				Class<?> oppType = GenericTypeResolver.resolveTypeArgument(oppClass,
@@ -134,7 +133,9 @@ public abstract class SecurityConfigurerAdapter<O, B extends SecurityBuilder<O>>
 		 */
 		private boolean addObjectPostProcessor(
 				ObjectPostProcessor<? extends Object> objectPostProcessor) {
-			return this.postProcessors.add(objectPostProcessor);
+			boolean result = this.postProcessors.add(objectPostProcessor);
+			Collections.sort(postProcessors, AnnotationAwareOrderComparator.INSTANCE);
+			return result;
 		}
 	}
 }

config/src/test/groovy/org/springframework/security/config/annotation/SecurityConfigurerAdapterTests.groovy → config/src/test/groovy/org/springframework/security/config/annotation/SecurityConfigurerAdapterClosureTests.groovy

@@ -15,13 +15,16 @@
  */
 package org.springframework.security.config.annotation
 
+import java.util.ArrayList;
+import java.util.List;
+
 import spock.lang.Specification
 
 /**
  * @author Rob Winch
  *
  */
-class SecurityConfigurerAdapterTests extends Specification {
+class SecurityConfigurerAdapterClosureTests extends Specification {
 	ConcereteSecurityConfigurerAdapter conf = new ConcereteSecurityConfigurerAdapter()
 
 	def "addPostProcessor closure"() {
@@ -37,4 +40,19 @@ class SecurityConfigurerAdapterTests extends Specification {
 		then:
 			conf.list.contains("a")
 	}
+
+	class ConcereteSecurityConfigurerAdapter extends
+			SecurityConfigurerAdapter<Object, SecurityBuilder<Object>> {
+		private List<Object> list = new ArrayList<Object>();
+
+		@Override
+		public void configure(SecurityBuilder<Object> builder) throws Exception {
+			list = postProcess(list);
+		}
+
+		public ConcereteSecurityConfigurerAdapter list(List<Object> l) {
+			this.list = l;
+			return this;
+		}
+	}
 }

config/src/test/java/org/springframework/security/config/annotation/SecurityConfigurerAdapterTests.java

@@ -0,0 +1,58 @@
+/*
+ * 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.security.config.annotation;
+
+import static org.assertj.core.api.Assertions.*;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.core.Ordered;
+
+public class SecurityConfigurerAdapterTests {
+	ConcereteSecurityConfigurerAdapter adapter;
+
+	@Before
+	public void setup() {
+		adapter = new ConcereteSecurityConfigurerAdapter();
+	}
+
+	@Test
+	public void postProcessObjectPostProcessorsAreSorted() {
+		adapter.addObjectPostProcessor(new OrderedObjectPostProcessor(Ordered.LOWEST_PRECEDENCE));
+		adapter.addObjectPostProcessor(new OrderedObjectPostProcessor(Ordered.HIGHEST_PRECEDENCE));
+
+		assertThat(adapter.postProcess("hi"))
+				.isEqualTo("hi " + Ordered.HIGHEST_PRECEDENCE + " " + Ordered.LOWEST_PRECEDENCE);
+	}
+
+	static class OrderedObjectPostProcessor implements ObjectPostProcessor<String>, Ordered {
+		private final int order;
+
+		public OrderedObjectPostProcessor(int order) {
+			super();
+			this.order = order;
+		}
+
+		public int getOrder() {
+			return order;
+		}
+
+		@SuppressWarnings("unchecked")
+		public String postProcess(String object) {
+			return object + " " + order;
+		}
+	}
+}