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.
Comment From: 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.
Comment From: philwebb
/cc @smaldini
Comment From: philwebb
I have a branch here that can be reviewed and cherry-picked if suitable.
Comment From: 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.