This is an automated email from the ASF dual-hosted git repository.
edcoleman pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push:
new 4f51fb9475 clean up AddressUtil.parseAddress usage (#4185)
4f51fb9475 is described below
commit 4f51fb9475eddaf8f70e1bf3bda16c2ed1cf2cae
Author: EdColeman <d...@etcoleman.com>
AuthorDate: Tue Jan 23 18:58:44 2024 -0500
clean up AddressUtil.parseAddress usage (#4185)
* clean up AddressUtil.parseAddress usage
* address port normalization, added test
---
.../core/clientImpl/InstanceOperationsImpl.java | 3 +--
.../apache/accumulo/core/lock/ServiceLockData.java | 2 +-
.../accumulo/core/metadata/TServerInstance.java | 4 ++--
.../org/apache/accumulo/core/util/AddressUtil.java | 18 +++++++++------
.../apache/accumulo/core/util/AddressUtilTest.java | 26 ++++++++++++++++++++++
.../accumulo/server/manager/LiveTServerSet.java | 4 ++--
.../manager/state/TabletStateChangeIterator.java | 2 +-
.../monitor/rest/manager/ManagerResource.java | 2 +-
.../rest/tservers/TabletServerResource.java | 2 +-
9 files changed, 46 insertions(+), 17 deletions(-)
diff --git
a/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
b/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
index c71b79c39a..ea9ff145ec 100644
---
a/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
+++
b/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
@@ -352,8 +352,7 @@ public class InstanceOperationsImpl implements
InstanceOperations {
@Override
public void ping(String tserver) throws AccumuloException {
- try (
- TTransport transport =
createTransport(AddressUtil.parseAddress(tserver, false), context)) {
+ try (TTransport transport =
createTransport(AddressUtil.parseAddress(tserver), context)) {
Client client = createClient(ThriftClientTypes.TABLET_SERVER, transport);
client.getTabletServerStatus(TraceUtil.traceInfo(), context.rpcCreds());
} catch (TException e) {
diff --git
a/core/src/main/java/org/apache/accumulo/core/lock/ServiceLockData.java
b/core/src/main/java/org/apache/accumulo/core/lock/ServiceLockData.java
index 865c0a9f4c..c550d64c16 100644
--- a/core/src/main/java/org/apache/accumulo/core/lock/ServiceLockData.java
+++ b/core/src/main/java/org/apache/accumulo/core/lock/ServiceLockData.java
@@ -170,7 +170,7 @@ public class ServiceLockData implements
Comparable<ServiceLockData> {
public HostAndPort getAddress(ThriftService service) {
String s = getAddressString(service);
- return s == null ? null : AddressUtil.parseAddress(s, false);
+ return s == null ? null : AddressUtil.parseAddress(s);
}
public String getGroup(ThriftService service) {
diff --git
a/core/src/main/java/org/apache/accumulo/core/metadata/TServerInstance.java
b/core/src/main/java/org/apache/accumulo/core/metadata/TServerInstance.java
index 56aaa06a8a..9e26102525 100644
--- a/core/src/main/java/org/apache/accumulo/core/metadata/TServerInstance.java
+++ b/core/src/main/java/org/apache/accumulo/core/metadata/TServerInstance.java
@@ -62,11 +62,11 @@ public class TServerInstance implements
Comparable<TServerInstance> {
}
public TServerInstance(String address, long session) {
- this(AddressUtil.parseAddress(address, false), Long.toHexString(session));
+ this(AddressUtil.parseAddress(address), Long.toHexString(session));
}
public TServerInstance(Value address, Text session) {
- this(AddressUtil.parseAddress(new String(address.get(), UTF_8), false),
session.toString());
+ this(AddressUtil.parseAddress(new String(address.get(), UTF_8)),
session.toString());
}
@Override
diff --git a/core/src/main/java/org/apache/accumulo/core/util/AddressUtil.java
b/core/src/main/java/org/apache/accumulo/core/util/AddressUtil.java
index 880944c5f8..b6bb6f9534 100644
--- a/core/src/main/java/org/apache/accumulo/core/util/AddressUtil.java
+++ b/core/src/main/java/org/apache/accumulo/core/util/AddressUtil.java
@@ -31,11 +31,10 @@ public class AddressUtil {
private static final Logger log = LoggerFactory.getLogger(AddressUtil.class);
- public static HostAndPort parseAddress(String address, boolean
ignoreMissingPort)
- throws NumberFormatException {
- address = address.replace('+', ':');
- HostAndPort hap = HostAndPort.fromString(address);
- if (!ignoreMissingPort && !hap.hasPort()) {
+ public static HostAndPort parseAddress(final String address) throws
NumberFormatException {
+ String normalized = normalizePortSeparator(address);
+ HostAndPort hap = HostAndPort.fromString(normalized);
+ if (!hap.hasPort()) {
throw new IllegalArgumentException(
"Address was expected to contain port. address=" + address);
}
@@ -43,8 +42,13 @@ public class AddressUtil {
return hap;
}
- public static HostAndPort parseAddress(String address, int defaultPort) {
- return parseAddress(address, true).withDefaultPort(defaultPort);
+ public static HostAndPort parseAddress(final String address, final int
defaultPort) {
+ String normalized = normalizePortSeparator(address);
+ return HostAndPort.fromString(normalized).withDefaultPort(defaultPort);
+ }
+
+ private static String normalizePortSeparator(final String address) {
+ return address.replace('+', ':');
}
/**
diff --git
a/core/src/test/java/org/apache/accumulo/core/util/AddressUtilTest.java
b/core/src/test/java/org/apache/accumulo/core/util/AddressUtilTest.java
index e81b72d142..4c1303aa08 100644
--- a/core/src/test/java/org/apache/accumulo/core/util/AddressUtilTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/util/AddressUtilTest.java
@@ -28,6 +28,8 @@ import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.net.HostAndPort;
+
/**
* Test the AddressUtil class.
*/
@@ -102,4 +104,28 @@ public class AddressUtilTest {
"The JVM Security settings cache DNS failures forever, this should
cause an exception.");
}
+
+ @Test
+ public void normalizeAddressRequirePortTest() {
+ HostAndPort hostAndPort = AddressUtil.parseAddress("127.0.1.2+8080");
+ assertEquals("127.0.1.2", hostAndPort.getHost());
+ assertEquals(8080, hostAndPort.getPort());
+
+ HostAndPort hostAndPort2 = AddressUtil.parseAddress("127.0.1.2:9123");
+ assertEquals("127.0.1.2", hostAndPort2.getHost());
+ assertEquals(9123, hostAndPort2.getPort());
+
+ assertThrows(IllegalArgumentException.class, () ->
AddressUtil.parseAddress("127.0.1.2"));
+ }
+
+ @Test
+ public void normalizeAddressWithDefaultTest() {
+ HostAndPort hostAndPort = AddressUtil.parseAddress("127.0.1.2+8080", 9123);
+ assertEquals("127.0.1.2", hostAndPort.getHost());
+ assertEquals(8080, hostAndPort.getPort());
+
+ HostAndPort hostAndPort2 = AddressUtil.parseAddress("127.0.1.2", 9123);
+ assertEquals("127.0.1.2", hostAndPort2.getHost());
+ assertEquals(9123, hostAndPort2.getPort());
+ }
}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/manager/LiveTServerSet.java
b/server/base/src/main/java/org/apache/accumulo/server/manager/LiveTServerSet.java
index e932d5c903..b41c381de1 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/manager/LiveTServerSet.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/manager/LiveTServerSet.java
@@ -398,11 +398,11 @@ public class LiveTServerSet implements Watcher {
if (index == -1) {
throw new IllegalArgumentException("Could not parse tabletserver '" +
tabletServer + "'");
}
- addr = AddressUtil.parseAddress(tabletServer.substring(0, index), false);
+ addr = AddressUtil.parseAddress(tabletServer.substring(0, index));
// Strip off the last bracket
sessionId = tabletServer.substring(index + 1, tabletServer.length() - 1);
} else {
- addr = AddressUtil.parseAddress(tabletServer, false);
+ addr = AddressUtil.parseAddress(tabletServer);
}
for (Entry<String,TServerInfo> entry : servers.entrySet()) {
if (entry.getValue().instance.getHostAndPort().equals(addr)) {
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletStateChangeIterator.java
b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletStateChangeIterator.java
index d07528e319..8f5962965b 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletStateChangeIterator.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletStateChangeIterator.java
@@ -135,7 +135,7 @@ public class TabletStateChangeIterator extends
SkippingIterator {
if (instance != null && instance.endsWith("]")) {
instance = instance.substring(0, instance.length() - 1);
}
- result.add(new TServerInstance(AddressUtil.parseAddress(hostport,
false), instance));
+ result.add(new TServerInstance(AddressUtil.parseAddress(hostport),
instance));
}
}
return result;
diff --git
a/server/monitor/src/main/java/org/apache/accumulo/monitor/rest/manager/ManagerResource.java
b/server/monitor/src/main/java/org/apache/accumulo/monitor/rest/manager/ManagerResource.java
index a9d62059b4..c2c20cc3e3 100644
---
a/server/monitor/src/main/java/org/apache/accumulo/monitor/rest/manager/ManagerResource.java
+++
b/server/monitor/src/main/java/org/apache/accumulo/monitor/rest/manager/ManagerResource.java
@@ -102,7 +102,7 @@ public class ManagerResource {
List<String> managers = monitor.getContext().getManagerLocations();
String manager =
- managers.isEmpty() ? "Down" :
AddressUtil.parseAddress(managers.get(0), false).getHost();
+ managers.isEmpty() ? "Down" :
AddressUtil.parseAddress(managers.get(0)).getHost();
int onlineTabletServers = mmi.tServerInfo.size();
int totalTabletServers = tservers.size();
int tablets = monitor.getTotalTabletCount();
diff --git
a/server/monitor/src/main/java/org/apache/accumulo/monitor/rest/tservers/TabletServerResource.java
b/server/monitor/src/main/java/org/apache/accumulo/monitor/rest/tservers/TabletServerResource.java
index f38cf3b4eb..c91fa6a998 100644
---
a/server/monitor/src/main/java/org/apache/accumulo/monitor/rest/tservers/TabletServerResource.java
+++
b/server/monitor/src/main/java/org/apache/accumulo/monitor/rest/tservers/TabletServerResource.java
@@ -125,7 +125,7 @@ public class TabletServerResource {
for (TabletServerStatus server : mmi.tServerInfo) {
if (server.logSorts != null) {
for (RecoveryStatus recovery : server.logSorts) {
- String serv = AddressUtil.parseAddress(server.name, false).getHost();
+ String serv = AddressUtil.parseAddress(server.name).getHost();
String log = recovery.name;
int time = recovery.runtime;
double progress = recovery.progress;
