diff --git a/geode-core/src/integrationTest/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java index 01ad836099fc..3db16de9e9fd 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java @@ -166,41 +166,6 @@ private void issueStopRequest(final int port) .stop(InetAddress.getLocalHost(), port); } - /** - * This test validates the AutoConnectionSourceImpl.updateLocatorInLocatorList method. That method - * takes InetSocketAddres of locator which unable to connect to locator. And update that - * InetSocketAddres with hostaddress of locator in locatorlist. - * - * In this test we validate this using identityHashCode. - */ - @Test - public void testLocatorIpChange() { - int port = 11011; - List locators = new ArrayList<>(); - InetSocketAddress floc1 = new InetSocketAddress("fakeLocalHost1", port); - InetSocketAddress floc2 = new InetSocketAddress("fakeLocalHost2", port); - - locators.add(floc1); - locators.add(floc2); - - List la = new ArrayList<>(); - la.add(new LocatorAddress(floc1, floc1.getHostName())); - la.add(new LocatorAddress(floc2, floc2.getHostName())); - - AutoConnectionSourceImpl src = new AutoConnectionSourceImpl(la, "", 60 * 1000); - - // This method will create a new InetSocketAddress of floc1 - src.updateLocatorInLocatorList(new LocatorAddress(floc1, floc1.getHostName())); - - List cLocList = src.getCurrentLocators(); - - Assert.assertEquals(2, cLocList.size()); - - for (InetSocketAddress t : cLocList) { - Assert.assertNotSame("Should have replaced floc1 intsance", t, floc1); - } - } - /** * This test validates the AutoConnectionSourceImpl.addbadLocators method. That method adds * badLocator from badLocator list to new Locator list. And it make sure that new locator list diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java index 9b45bca21fc7..1714af33790d 100644 --- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java +++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java @@ -93,6 +93,7 @@ public class AutoConnectionSourceImpl implements ConnectionSource { } }; private final List initialLocators; + private final String serverGroup; private AtomicReference locators = new AtomicReference<>(); private AtomicReference onlineLocators = new AtomicReference<>(); @@ -206,12 +207,12 @@ ServerLocationResponse queryOneLocatorUsingConnection(LocatorAddress locator, Object returnObj = null; try { pool.getStats().incLocatorRequests(); - returnObj = locatorConnection.requestToServer(locator.getSocketInetAddressNoLookup(), request, + returnObj = locatorConnection.requestToServer(locator.getSocketInetAddress(), request, connectionTimeout, true); ServerLocationResponse response = (ServerLocationResponse) returnObj; pool.getStats().incLocatorResponses(); if (response != null) { - reportLiveLocator(locator.getSocketInetAddressNoLookup()); + reportLiveLocator(locator.getSocketInetAddress()); } return response; } catch (IOException | ToDataException ioe) { @@ -219,8 +220,7 @@ ServerLocationResponse queryOneLocatorUsingConnection(LocatorAddress locator, logger.warn("Encountered ToDataException when communicating with a locator. " + "This is expected if the locator is shutting down.", ioe); } - reportDeadLocator(locator.getSocketInetAddressNoLookup(), ioe); - updateLocatorInLocatorList(locator); + reportDeadLocator(locator.getSocketInetAddress(), ioe); return null; } catch (ClassNotFoundException e) { logger.warn("Received exception from locator {}", locator, e); @@ -229,45 +229,11 @@ ServerLocationResponse queryOneLocatorUsingConnection(LocatorAddress locator, if (logger.isDebugEnabled()) { logger.debug("Received odd response object from the locator: {}", returnObj); } - reportDeadLocator(locator.getSocketInetAddressNoLookup(), e); + reportDeadLocator(locator.getSocketInetAddress(), e); return null; } } - /** - * If connecting to the locator fails with an IOException, this may be because the locator's IP - * has changed. Add the locator back to the list of locators using host address rather than IP. - * This will cause another DNS lookup, hopefully finding the locator. - * - */ - protected void updateLocatorInLocatorList(LocatorAddress locator) { - if (locator.getSocketInetAddressNoLookup().getHostName() != null && !locator.isIpString()) { - LocatorList locatorList = locators.get(); - List newLocatorsList = new ArrayList<>(); - - for (LocatorAddress tloc : locatorList.getLocatorAddresses()) { - if (tloc.equals(locator)) { - InetSocketAddress changeLoc = new InetSocketAddress(locator.getHostName(), - locator.getSocketInetAddressNoLookup().getPort()); - LocatorAddress hostAddress = new LocatorAddress(changeLoc, locator.getHostName()); - newLocatorsList.add(hostAddress); - } else { - newLocatorsList.add(tloc); - } - } - - logger.debug("updateLocatorInLocatorList locator list from: {} to {}", - locatorList.getLocators(), newLocatorsList); - - LocatorList newLocatorList = new LocatorList(newLocatorsList); - locators.set(newLocatorList); - } - } - - protected List getCurrentLocators() { - return locators.get().getLocators(); - } - private ServerLocationResponse queryLocators(ServerLocationRequest request) { Iterator controllerItr = locators.get().iterator(); ServerLocationResponse response; @@ -297,6 +263,7 @@ private void updateLocatorList(LocatorListResponse response) { List newOnlineLocators = new ArrayList<>(locatorResponse.size()); Set badLocators = new HashSet<>(initialLocators); + for (ServerLocation locator : locatorResponse) { InetSocketAddress address = new InetSocketAddress(locator.getHostName(), locator.getPort()); LocatorAddress hostAddress = new LocatorAddress(address, locator.getHostName()); diff --git a/geode-tcp-server/build.gradle b/geode-tcp-server/build.gradle index 7b2a3d659d1d..8af8b0aa5bc8 100644 --- a/geode-tcp-server/build.gradle +++ b/geode-tcp-server/build.gradle @@ -27,7 +27,10 @@ dependencies { //Commons validator is used to validate inet addresses implementation('commons-validator:commons-validator') - + testCompile(project(':geode-junit')) { + exclude module: 'geode-core' + } + testCompile('org.assertj:assertj-core') testImplementation('com.tngtech.archunit:archunit-junit4') diff --git a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/LocatorAddress.java b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/LocatorAddress.java index cf073580dc75..0097b50d4ad3 100644 --- a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/LocatorAddress.java +++ b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/LocatorAddress.java @@ -15,96 +15,77 @@ package org.apache.geode.distributed.internal.tcpserver; import java.net.InetSocketAddress; +import java.util.Objects; import org.apache.commons.validator.routines.InetAddressValidator; public class LocatorAddress { private final InetSocketAddress socketInetAddress; - private final String hostname; - private final int port; - private final boolean isIpString; public LocatorAddress(InetSocketAddress loc, String locStr) { - this.socketInetAddress = loc; - this.hostname = locStr; - this.port = loc.getPort(); - this.isIpString = InetAddressValidator.getInstance().isValid(locStr); + if (InetAddressValidator.getInstance().isValid(locStr)) { + socketInetAddress = new InetSocketAddress(locStr, loc.getPort()); + } else { + socketInetAddress = cloneUnresolved(loc); + } } + /** + * @deprecated Users should not care if literal IP or hostname is used. + */ + @Deprecated public boolean isIpString() { - return isIpString; + return !socketInetAddress.isUnresolved(); } /** - * if host is ipString then it will return the cached InetSocketAddress Otherwise it will create - * the new instance of InetSocketAddress + * If location is not litteral IP address a new resolved {@link InetSocketAddress} is returned. + * + * @return resolved {@link InetSocketAddress}, otherwise stored {@link InetSocketAddress} if + * literal IP address is used. */ public InetSocketAddress getSocketInetAddress() { - if (this.isIpString) { - return this.socketInetAddress; + if (socketInetAddress.isUnresolved()) { + return new InetSocketAddress(socketInetAddress.getHostString(), socketInetAddress.getPort()); } else { - return new InetSocketAddress(hostname, this.socketInetAddress.getPort()); + return this.socketInetAddress; } } public String getHostName() { - return hostname; + return socketInetAddress.getHostString(); } public int getPort() { - return port; - } - - /** - * If component has retry logic then use this method to get the InetSocketAddress address - * AutoConnectionSourceImpl for client has retry logic; This way client will not make DNS query - * each time - * - */ - public InetSocketAddress getSocketInetAddressNoLookup() { - return this.socketInetAddress; + return socketInetAddress.getPort(); } @Override public int hashCode() { - int prime = 31; - int result = 1; - result = prime * result + (isIpString ? 1231 : 1237); - result = prime * result + (socketInetAddress == null ? 0 : socketInetAddress.hashCode()); - result = prime * result + (hostname == null ? 0 : hostname.hashCode()); - return result; + return socketInetAddress.hashCode(); } @Override - public boolean equals(Object obj) { - if (this == obj) + public boolean equals(Object o) { + if (this == o) { return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - LocatorAddress other = (LocatorAddress) obj; - if (isIpString != other.isIpString) - return false; - if (socketInetAddress == null) { - if (other.socketInetAddress != null) - return false; - } else if (!socketInetAddress.equals(other.socketInetAddress)) - return false; - if (hostname == null) { - if (other.hostname != null) - return false; - } else if (!hostname.equals(other.hostname)) + } + if (o == null || getClass() != o.getClass()) { return false; - return true; + } + LocatorAddress that = (LocatorAddress) o; + return Objects.equals(socketInetAddress, that.socketInetAddress); } @Override public String toString() { - return getClass().getSimpleName() - + " [socketInetAddress=" + socketInetAddress + ", hostname=" + hostname - + ", isIpString=" + isIpString + "]"; + return getClass().getSimpleName() + " [socketInetAddress=" + socketInetAddress + "]"; + } + + private InetSocketAddress cloneUnresolved(final InetSocketAddress inetSocketAddress) { + return InetSocketAddress.createUnresolved(inetSocketAddress.getHostString(), + inetSocketAddress.getPort()); } } diff --git a/geode-tcp-server/src/test/java/org/apache/geode/distributed/internal/tcpserver/LocatorAddressTest.java b/geode-tcp-server/src/test/java/org/apache/geode/distributed/internal/tcpserver/LocatorAddressTest.java new file mode 100644 index 000000000000..ad2ed0e192ad --- /dev/null +++ b/geode-tcp-server/src/test/java/org/apache/geode/distributed/internal/tcpserver/LocatorAddressTest.java @@ -0,0 +1,109 @@ +/* + * 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.geode.distributed.internal.tcpserver; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.InetSocketAddress; + +import org.junit.Test; + +public class LocatorAddressTest { + + /** + * Test that getSocketInentAddress returns resolved InetSocketAddress + */ + @Test + public void Test_getSocketInentAddress_returns_resolved_SocketAddress() { + InetSocketAddress host1address = new InetSocketAddress(8080); + LocatorAddress locator1 = new LocatorAddress(host1address, "localhost"); + + InetSocketAddress actual = locator1.getSocketInetAddress(); + + assertThat(actual.isUnresolved()).isFalse(); + } + + /** + * Test that getSocketInentAddress returns unresolved InetSocketAddress + */ + @Test + public void Test_getSocketInentAddress_returns_unresolved_SocketAddress() { + InetSocketAddress host1address = InetSocketAddress.createUnresolved("fakelocalhost", 8090); + LocatorAddress locator1 = new LocatorAddress(host1address, "fakelocalhost"); + + InetSocketAddress actual = locator1.getSocketInetAddress(); + + assertThat(actual.isUnresolved()).isTrue(); + } + + /** + * Test whether LocatorAddress are equal, when created from resolved and unresolved + * InetSocketAddress + */ + @Test + public void Test_equals_LocatorAddress_from_resolved_and_unresolved_SocketAddress() { + InetSocketAddress host1address = InetSocketAddress.createUnresolved("localhost", 8090); + LocatorAddress locator1 = new LocatorAddress(host1address, "localhost"); + + InetSocketAddress host2address = locator1.getSocketInetAddress(); + LocatorAddress locator2 = new LocatorAddress(host2address, "localhost"); + + assertThat(host1address.isUnresolved()).isTrue(); + assertThat(host2address.isUnresolved()).isFalse(); + assertThat(locator1.equals(locator2)).isTrue(); + } + + @Test + public void Test_getPort_returns_port() { + InetSocketAddress host1address = InetSocketAddress.createUnresolved("localhost", 8090); + LocatorAddress locator1 = new LocatorAddress(host1address, "localhost"); + assertThat(locator1.getPort()).isEqualTo(8090); + } + + @Test + public void Test_getHostName_returns_hostname() { + InetSocketAddress host1address = InetSocketAddress.createUnresolved("fakelocalhost", 8091); + LocatorAddress locator1 = new LocatorAddress(host1address, "fakelocalhost"); + assertThat(locator1.getHostName()).isEqualTo("fakelocalhost"); + } + + @Test + public void Test_hashCode_of_SocketAddress() { + InetSocketAddress host1address = InetSocketAddress.createUnresolved("fakelocalhost", 8091); + LocatorAddress locator1 = new LocatorAddress(host1address, "fakelocalhost"); + assertThat(locator1.hashCode()).isEqualTo(host1address.hashCode()); + } + + @Test + public void Test_toString_LocatorAddress() { + InetSocketAddress host1address = InetSocketAddress.createUnresolved("fakelocalhost", 8091); + LocatorAddress locator1 = new LocatorAddress(host1address, "fakelocalhost"); + assertThat(locator1.toString()).contains("socketInetAddress"); + } + + @Test + public void Test_isIpString_for_LocatorAddress_constructed_from_IPstring() { + InetSocketAddress host1address = new InetSocketAddress(8080); + LocatorAddress locator1 = new LocatorAddress(host1address, "127.0.0.1"); + assertThat(locator1.isIpString()).isTrue(); + } + + @Test + public void Test_isIpString_for_LocatorAddress_constructed_from_hostname() { + InetSocketAddress host1address = new InetSocketAddress(8080); + LocatorAddress locator1 = new LocatorAddress(host1address, "localhost"); + assertThat(locator1.isIpString()).isFalse(); + } +}