Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve AdvisedSupport.getAdvisors() #26017

Merged
merged 1 commit into from
Nov 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2020 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.
Expand Down Expand Up @@ -223,4 +223,12 @@ public interface Advised extends TargetClassAware {
*/
String toProxyConfigString();

/**
* Equivalent to {@code getAdvisors().length}
* @return count of advisors of this advised
*/
default int getAdvisorCount() {
return getAdvisors().length;
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
Expand Down Expand Up @@ -95,12 +95,6 @@ public class AdvisedSupport extends ProxyConfig implements Advised {
*/
private List<Advisor> advisors = new ArrayList<>();

/**
* Array updated on changes to the advisors list, which is easier
* to manipulate internally.
*/
private Advisor[] advisorArray = new Advisor[0];


/**
* No-arg constructor for use as a JavaBean.
Expand Down Expand Up @@ -244,7 +238,7 @@ public boolean isInterfaceProxied(Class<?> intf) {

@Override
public final Advisor[] getAdvisors() {
return this.advisorArray;
return this.advisors.toArray(new Advisor[0]);
}

@Override
Expand Down Expand Up @@ -292,7 +286,6 @@ public void removeAdvisor(int index) throws AopConfigException {
}
}

updateAdvisorArray();
adviceChanged();
}

Expand Down Expand Up @@ -339,7 +332,6 @@ public void addAdvisors(Collection<Advisor> advisors) {
Assert.notNull(advisor, "Advisor must not be null");
this.advisors.add(advisor);
}
updateAdvisorArray();
adviceChanged();
}
}
Expand All @@ -363,27 +355,18 @@ private void addAdvisorInternal(int pos, Advisor advisor) throws AopConfigExcept
"Illegal position " + pos + " in advisor list with size " + this.advisors.size());
}
this.advisors.add(pos, advisor);
updateAdvisorArray();
adviceChanged();
}

/**
* Bring the array up to date with the list.
*/
protected final void updateAdvisorArray() {
this.advisorArray = this.advisors.toArray(new Advisor[0]);
}

/**
* Allows uncontrolled access to the {@link List} of {@link Advisor Advisors}.
* <p>Use with care, and remember to {@link #updateAdvisorArray() refresh the advisor array}
* and {@link #adviceChanged() fire advice changed events} when making any modifications.
* <p>Use with care, and remember to {@link #adviceChanged() fire advice changed events}
* when making any modifications.
*/
protected final List<Advisor> getAdvisorsInternal() {
return this.advisors;
}


@Override
public void addAdvice(Advice advice) throws AopConfigException {
int pos = this.advisors.size();
Expand Down Expand Up @@ -521,7 +504,6 @@ protected void copyConfigurationFrom(AdvisedSupport other, TargetSource targetSo
Assert.notNull(advisor, "Advisor must not be null");
this.advisors.add(advisor);
}
updateAdvisorArray();
adviceChanged();
}

Expand All @@ -536,7 +518,6 @@ AdvisedSupport getConfigurationOnlyCopy() {
copy.advisorChainFactory = this.advisorChainFactory;
copy.interfaces = this.interfaces;
copy.advisors = this.advisors;
copy.updateAdvisorArray();
return copy;
}

Expand All @@ -553,6 +534,10 @@ private void readObject(ObjectInputStream ois) throws IOException, ClassNotFound
this.methodCache = new ConcurrentHashMap<>(32);
}

@Override
public int getAdvisorCount() {
return advisors.size();
}

@Override
public String toProxyConfigString() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
Expand Down Expand Up @@ -206,7 +206,7 @@ public static boolean equalsProxiedInterfaces(AdvisedSupport a, AdvisedSupport b
* Check equality of the advisors behind the given AdvisedSupport objects.
*/
public static boolean equalsAdvisors(AdvisedSupport a, AdvisedSupport b) {
return Arrays.equals(a.getAdvisors(), b.getAdvisors());
return a.getAdvisorCount() == b.getAdvisorCount() && Arrays.equals(a.getAdvisors(), b.getAdvisors());
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ class CglibAopProxy implements AopProxy, Serializable {
*/
public CglibAopProxy(AdvisedSupport config) throws AopConfigException {
Assert.notNull(config, "AdvisedSupport must not be null");
if (config.getAdvisors().length == 0 && config.getTargetSource() == AdvisedSupport.EMPTY_TARGET_SOURCE) {
if (config.getAdvisorCount() == 0 && config.getTargetSource() == AdvisedSupport.EMPTY_TARGET_SOURCE) {
throw new AopConfigException("No advisors and no TargetSource specified");
}
this.advised = config;
Expand Down Expand Up @@ -942,11 +942,11 @@ public boolean equals(@Nullable Object other) {
}
// Advice instance identity is unimportant to the proxy class:
// All that matters is type and ordering.
Advisor[] thisAdvisors = this.advised.getAdvisors();
Advisor[] thatAdvisors = otherAdvised.getAdvisors();
if (thisAdvisors.length != thatAdvisors.length) {
if (this.advised.getAdvisorCount() != otherAdvised.getAdvisorCount()) {
return false;
}
Advisor[] thisAdvisors = this.advised.getAdvisors();
Advisor[] thatAdvisors = otherAdvised.getAdvisors();
for (int i = 0; i < thisAdvisors.length; i++) {
Advisor thisAdvisor = thisAdvisors[i];
Advisor thatAdvisor = thatAdvisors[i];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ final class JdkDynamicAopProxy implements AopProxy, InvocationHandler, Serializa
*/
public JdkDynamicAopProxy(AdvisedSupport config) throws AopConfigException {
Assert.notNull(config, "AdvisedSupport must not be null");
if (config.getAdvisors().length == 0 && config.getTargetSource() == AdvisedSupport.EMPTY_TARGET_SOURCE) {
if (config.getAdvisorCount() == 0 && config.getTargetSource() == AdvisedSupport.EMPTY_TARGET_SOURCE) {
throw new AopConfigException("No advisors and no TargetSource specified");
}
this.advised = config;
Expand Down