MutablePropertySources will not find or remove proxied sources

[philwebb]

MutablePropertySources has get and remove methods that look like this:

@Override
@Nullable
public PropertySource<?> get(String name) {
	int index = this.propertySourceList.indexOf(PropertySource.named(name));
	return (index != -1 ? this.propertySourceList.get(index) : null);
}

These attempt to find the index of a property source index by checking for the name. The PropertySource.named method returns a ComparisonPropertySource which depends on the inherited PropertySource equals and hashCode implementations.

The equals method looks like this:

	@Override
	public boolean equals(@Nullable Object other) {
		return (this == other || (other instanceof PropertySource &&
				ObjectUtils.nullSafeEquals(this.name, ((PropertySource<?>) other).name)));
	}

If you are using a library such as jasypt, it's possible that your property source will be a proxy instance and won't actually contain a populated name field. The equals method could use ((PropertySource<?>) other).getName())) which would solve this issue.

[philwebb]

Another, potentially better way to solve this would be to rewrite the MutablePropertySources get method entirely. I believe that it currently could suffer from a race condition where another thread could change the index of the element before this.propertySourceList.get(index) is called.

Since propertySourceList is a CopyOnWriteArrayList array, I suspect it would be just as efficient to use list.toArray() and then search the array directly. Another option might be to use an array directly and replicate what CopyOnWriteArrayList currently does.

[philwebb]

/cc @smaldini

[philwebb]

I have a branch here that can be reviewed and cherry-picked if suitable.

[jhoeller]

After some back and forth, in addition to delegating to the getName() method in PropertySource itself (addressing the main point of this issue), I went with a revision that preserves the CopyOnWriteArrayList, performing manual iteration (relying on the stable COW iterator) in the MutablePropertySources.get and contains implementation... and manually synchronizing on the list in the mutator methods. This seems straightforward enough that it's also worth backporting to 5.1.x. To be committed ASAP.