bug: EtcdServiceRegistry.getService() returns arbitrary element from unordered list

[sfloess]

Bug Description

The getService() method returns the first element from getAllServices(), but CopyOnWriteArrayList doesn't guarantee any particular order, especially after concurrent modifications. This makes the behavior unpredictable and inconsistent.

Location

jplatform-registry-etcd/src/main/java/org/flossware/jplatform/registry/etcd/EtcdServiceRegistry.java:129-133

Problematic Code

@Override
public <T> Optional<T> getService(Class<T> serviceInterface) {
    List<T> services = getAllServices(serviceInterface);
    return services.isEmpty() ? Optional.empty() : Optional.of(services.get(0));
    // Returns "first" element but order is undefined!
}

@Override
@SuppressWarnings("unchecked")
public <T> List<T> getAllServices(Class<T> serviceInterface) {
    List<Object> services = localServices.get(serviceInterface);
    if (services == null) {
        return Collections.emptyList();
    }
    return (List<T>) new ArrayList<>(services);  // CopyOnWriteArrayList order not guaranteed
}

Impact

• Unpredictable behavior: Different service returned each time
• Load balancing issues: Can't implement consistent routing
• Testing difficulties: Non-deterministic results
• Service preference ignored: No way to select specific implementation
• Race conditions: Concurrent registration affects which service is returned

Example

EtcdServiceRegistry registry = new EtcdServiceRegistry(config);
registry.start();

// Register multiple implementations
MyService impl1 = new MyServiceImpl1();
MyService impl2 = new MyServiceImpl2();
MyService impl3 = new MyServiceImpl3();

registry.registerService(MyService.class, impl1);
registry.registerService(MyService.class, impl2);
registry.registerService(MyService.class, impl3);

// Get single service
Optional<MyService> service = registry.getService(MyService.class);
// Returns... impl1? impl2? impl3? Undefined!

// Call again
service = registry.getService(MyService.class);
// Returns same? Different? Also undefined!

// After concurrent modifications:
Thread 1: registry.unregisterService(MyService.class, impl2);
Thread 2: Optional<MyService> s = registry.getService(MyService.class);

// Thread 2 might get:
// - impl1 (if unregister completed first)
// - impl2 (if getService read before unregister)
// - impl3 (if list was rearranged somehow)

// User wants round-robin load balancing:
for (int i = 0; i < 100; i++) {
    MyService s = registry.getService(MyService.class).get();
    s.doWork();
}
// All 100 calls go to same service!
// Others never used
// Not actually load balancing

Proposed Fix

Option 1 - Document the undefined behavior:

/**
 * Returns a service implementation for the given interface.
 * <p><b>Note:</b> When multiple implementations are registered, this method
 * returns an arbitrary implementation. The selection is non-deterministic
 * and may change between calls. For load balancing or specific selection,
 * use {@link #getAllServices(Class)} and implement your own selection logic.
 *
 * @param serviceInterface the service interface class
 * @param <T> the service type
 * @return an Optional containing a service implementation, or empty if none registered
 */
@Override
public <T> Optional<T> getService(Class<T> serviceInterface) {
    List<T> services = getAllServices(serviceInterface);
    return services.isEmpty() ? Optional.empty() : Optional.of(services.get(0));
}

Option 2 - Implement round-robin selection:

private final Map<Class<?>, AtomicInteger> roundRobinCounters = new ConcurrentHashMap<>();

@Override
public <T> Optional<T> getService(Class<T> serviceInterface) {
    List<T> services = getAllServices(serviceInterface);
    if (services.isEmpty()) {
        return Optional.empty();
    }
    
    if (services.size() == 1) {
        return Optional.of(services.get(0));
    }
    
    // Round-robin selection
    AtomicInteger counter = roundRobinCounters.computeIfAbsent(
        serviceInterface,
        k -> new AtomicInteger(0)
    );
    
    int index = Math.abs(counter.getAndIncrement() % services.size());
    return Optional.of(services.get(index));
}

Option 3 - Implement random selection (simpler than round-robin):

private static final Random RANDOM = new Random();

@Override
public <T> Optional<T> getService(Class<T> serviceInterface) {
    List<T> services = getAllServices(serviceInterface);
    if (services.isEmpty()) {
        return Optional.empty();
    }
    
    if (services.size() == 1) {
        return Optional.of(services.get(0));
    }
    
    // Random selection for simple load distribution
    int index = RANDOM.nextInt(services.size());
    return Optional.of(services.get(index));
}

Option 4 - Always return first registered (maintain insertion order):

// Document that first registered service is returned
/**
 * Returns the first registered service implementation for the given interface.
 * When multiple implementations are registered, this returns the first one
 * that was registered (insertion order).
 *
 * @param serviceInterface the service interface class
 * @param <T> the service type
 * @return an Optional containing the first registered service, or empty if none
 */
@Override
public <T> Optional<T> getService(Class<T> serviceInterface) {
    List<T> services = getAllServices(serviceInterface);
    return services.isEmpty() ? Optional.empty() : Optional.of(services.get(0));
}
// CopyOnWriteArrayList maintains insertion order, so this is actually deterministic

Recommendation: Option 4 - CopyOnWriteArrayList actually DOES maintain insertion order, so the current implementation is deterministic. Just need to document it clearly.