javanna commented on code in PR #14275:
URL: https://github.com/apache/lucene/pull/14275#discussion_r1971187231
##########
lucene/core/src/java/org/apache/lucene/util/NamedSPILoader.java:
##########
@@ -64,18 +64,24 @@ public NamedSPILoader(Class<S> clazz, ClassLoader
classloader) {
*/
public void reload(ClassLoader classloader) {
Objects.requireNonNull(classloader, "classloader");
- final LinkedHashMap<String, S> services = new
LinkedHashMap<>(this.services);
+ final Map<String, S> services = this.services;
+ final LinkedHashMap<String, S> newServices = new LinkedHashMap<>();
for (final S service : ServiceLoader.load(clazz, classloader)) {
final String name = service.getName();
// only add the first one for each name, later services will be ignored
- // this allows to place services before others in classpath to make
- // them used instead of others
- if (!services.containsKey(name)) {
- checkServiceName(name);
- services.put(name, service);
+ // unless the later-found service allows to replace the previous.
+ var prevNew = newServices.get(name);
Review Comment:
what is the current behaviour is the same service is registered multiple
times? I guess only one will win, but it's not deterministic which one?
I was wondering if the replace default behaviour introduces a potential
breaking behaviour for existing users. I guess though that is granted if we
want to make things deterministic: there are things that may have worked almost
by accident before this change, that now require overriding replace explicitly?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
