From cc9395cfafe7a436330f35b3238bbf7415f416b1 Mon Sep 17 00:00:00 2001 From: Nathaniel Colangelo Date: Sun, 2 Aug 2026 20:20:51 -1000 Subject: [PATCH] Fix AdminApi.setConfiguration(RaftPeer[], RaftPeer[]) dropping the servers array The two-array overload built its Arguments with setListenersInNewConf(serversInNewConf) followed by setListenersInNewConf(listenersInNewConf), so the servers were dropped and the builder's server list stayed null; every call to this overload then failed with a NullPointerException from Preconditions.assertUnique in the Arguments constructor. Route the servers through setServersInNewConf, as the javadoc documents, and add a regression test covering both two-argument overloads. Co-Authored-By: Claude Fable 5 --- .../org/apache/ratis/client/api/AdminApi.java | 2 +- .../org/apache/ratis/client/TestAdminApi.java | 80 +++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 ratis-test/src/test/java/org/apache/ratis/client/TestAdminApi.java diff --git a/ratis-client/src/main/java/org/apache/ratis/client/api/AdminApi.java b/ratis-client/src/main/java/org/apache/ratis/client/api/AdminApi.java index 8ce6a5eb1b..5976fa2054 100644 --- a/ratis-client/src/main/java/org/apache/ratis/client/api/AdminApi.java +++ b/ratis-client/src/main/java/org/apache/ratis/client/api/AdminApi.java @@ -60,7 +60,7 @@ default RaftClientReply setConfiguration(RaftPeer[] serversInNewConf, RaftPeer[] throws IOException { return setConfiguration(SetConfigurationRequest.Arguments .newBuilder() - .setListenersInNewConf(serversInNewConf) + .setServersInNewConf(serversInNewConf) .setListenersInNewConf(listenersInNewConf) .build()); } diff --git a/ratis-test/src/test/java/org/apache/ratis/client/TestAdminApi.java b/ratis-test/src/test/java/org/apache/ratis/client/TestAdminApi.java new file mode 100644 index 0000000000..70696d56c2 --- /dev/null +++ b/ratis-test/src/test/java/org/apache/ratis/client/TestAdminApi.java @@ -0,0 +1,80 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.ratis.client; + +import org.apache.ratis.BaseTest; +import org.apache.ratis.client.api.AdminApi; +import org.apache.ratis.proto.RaftProtos.RaftPeerRole; +import org.apache.ratis.protocol.RaftClientReply; +import org.apache.ratis.protocol.RaftPeer; +import org.apache.ratis.protocol.RaftPeerId; +import org.apache.ratis.protocol.SetConfigurationRequest; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; + +/** Test the default methods of {@link AdminApi}. */ +public class TestAdminApi extends BaseTest { + static AdminApi newCapturingAdminApi(AtomicReference captured) { + return new AdminApi() { + @Override + public RaftClientReply setConfiguration(SetConfigurationRequest.Arguments arguments) { + captured.set(arguments); + return null; + } + + @Override + public RaftClientReply transferLeadership(RaftPeerId newLeader, RaftPeerId leaderId, long timeoutMs) { + throw new UnsupportedOperationException(); + } + }; + } + + static RaftPeer newPeer(String id) { + return RaftPeer.newBuilder().setId(id).build(); + } + + @Test + public void testSetConfigurationWithArrays() throws Exception { + final RaftPeer[] servers = {newPeer("s0"), newPeer("s1"), newPeer("s2")}; + final RaftPeer[] listeners = {newPeer("l0")}; + + final AtomicReference captured = new AtomicReference<>(); + newCapturingAdminApi(captured).setConfiguration(servers, listeners); + + final SetConfigurationRequest.Arguments arguments = captured.get(); + Assertions.assertEquals(Arrays.asList(servers), arguments.getServersInNewConf()); + Assertions.assertEquals(Arrays.asList(listeners), arguments.getPeersInNewConf(RaftPeerRole.LISTENER)); + } + + @Test + public void testSetConfigurationWithLists() throws Exception { + final List servers = Arrays.asList(newPeer("s0"), newPeer("s1"), newPeer("s2")); + final List listeners = Arrays.asList(newPeer("l0")); + + final AtomicReference captured = new AtomicReference<>(); + newCapturingAdminApi(captured).setConfiguration(servers, listeners); + + final SetConfigurationRequest.Arguments arguments = captured.get(); + Assertions.assertEquals(servers, arguments.getServersInNewConf()); + Assertions.assertEquals(listeners, arguments.getPeersInNewConf(RaftPeerRole.LISTENER)); + } +}