From 3164bd6f8d0fc82c5f671be0852cdcd600d0cdef Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Tue, 8 Mar 2016 10:38:59 -0600 Subject: [PATCH] Polish Sorting ObjectPostProcessor * Add Test * Only sort on adding new entry Issue gh-3572 --- .../annotation/SecurityConfigurerAdapter.java | 11 ++-- ...urityConfigurerAdapterClosureTests.groovy} | 20 ++++++- .../ConcereteSecurityConfigurerAdapter.java | 0 .../SecurityConfigurerAdapterTests.java | 58 +++++++++++++++++++ 4 files changed, 83 insertions(+), 6 deletions(-) rename config/src/test/groovy/org/springframework/security/config/annotation/{SecurityConfigurerAdapterTests.groovy => SecurityConfigurerAdapterClosureTests.groovy} (68%) rename config/src/test/{groovy => java}/org/springframework/security/config/annotation/ConcereteSecurityConfigurerAdapter.java (100%) create mode 100644 config/src/test/java/org/springframework/security/config/annotation/SecurityConfigurerAdapterTests.java diff --git a/config/src/main/java/org/springframework/security/config/annotation/SecurityConfigurerAdapter.java b/config/src/main/java/org/springframework/security/config/annotation/SecurityConfigurerAdapter.java index f4bb69175b..b36d406a40 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/SecurityConfigurerAdapter.java +++ b/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> @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> */ private boolean addObjectPostProcessor( ObjectPostProcessor objectPostProcessor) { - return this.postProcessors.add(objectPostProcessor); + boolean result = this.postProcessors.add(objectPostProcessor); + Collections.sort(postProcessors, AnnotationAwareOrderComparator.INSTANCE); + return result; } } } diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/SecurityConfigurerAdapterTests.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/SecurityConfigurerAdapterClosureTests.groovy similarity index 68% rename from config/src/test/groovy/org/springframework/security/config/annotation/SecurityConfigurerAdapterTests.groovy rename to config/src/test/groovy/org/springframework/security/config/annotation/SecurityConfigurerAdapterClosureTests.groovy index 903f2b3e89..9b3d75b5ae 100644 --- a/config/src/test/groovy/org/springframework/security/config/annotation/SecurityConfigurerAdapterTests.groovy +++ b/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> { + private List list = new ArrayList(); + + @Override + public void configure(SecurityBuilder builder) throws Exception { + list = postProcess(list); + } + + public ConcereteSecurityConfigurerAdapter list(List l) { + this.list = l; + return this; + } + } } diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/ConcereteSecurityConfigurerAdapter.java b/config/src/test/java/org/springframework/security/config/annotation/ConcereteSecurityConfigurerAdapter.java similarity index 100% rename from config/src/test/groovy/org/springframework/security/config/annotation/ConcereteSecurityConfigurerAdapter.java rename to config/src/test/java/org/springframework/security/config/annotation/ConcereteSecurityConfigurerAdapter.java diff --git a/config/src/test/java/org/springframework/security/config/annotation/SecurityConfigurerAdapterTests.java b/config/src/test/java/org/springframework/security/config/annotation/SecurityConfigurerAdapterTests.java new file mode 100644 index 0000000000..6376e3facb --- /dev/null +++ b/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, 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; + } + } +}