From 3f5defdb9c77692179e74778650d162bc1f2f2ea Mon Sep 17 00:00:00 2001 From: Marvin Rausch Date: Sat, 17 Jun 2023 16:34:18 +0200 Subject: [PATCH 1/2] HubConnectionManager: fix bug that attempt still was in failed list, even if connection attempt was successful in second try --- .../hub/BasicHubConnectionManager.java | 14 ++++++++++ .../hub/HubConnectionManagerTest.java | 25 ++++++++++++++++++ .../KnownBugsHubConnectionManagerTest.java | 26 ------------------- 3 files changed, 39 insertions(+), 26 deletions(-) diff --git a/src/main/java/net/sharksystem/hub/BasicHubConnectionManager.java b/src/main/java/net/sharksystem/hub/BasicHubConnectionManager.java index a885d70..84dd4cc 100644 --- a/src/main/java/net/sharksystem/hub/BasicHubConnectionManager.java +++ b/src/main/java/net/sharksystem/hub/BasicHubConnectionManager.java @@ -51,6 +51,7 @@ protected void syncLists() { break; // found - go ahead. } } + // remove failed attempt from connected hubs list if(!found) toBeRemoved.add(wishedConnection); } @@ -75,6 +76,19 @@ protected void syncLists() { public long getTimeStamp() { return lastConnectionAttempt; } }); } + + // remove attempts which where successful later on + List connectedNow = new ArrayList<>(); + for(HubConnectionManager.FailedConnectionAttempt failedAttempt: this.failedConnectionAttempts){ + for(HubConnectorDescription runningConnection : this.hcdListHub) { + if(failedAttempt.getHubConnectorDescription().isSame(runningConnection)) { + connectedNow.add(failedAttempt); + } + } + } + for(FailedConnectionAttempt failedAttempt : connectedNow) { + this.failedConnectionAttempts.remove(failedAttempt); + } } private HubConnectorDescription findSameInList(HubConnectorDescription hcd, List hcdList) { diff --git a/src/test/java/net/sharksystem/hub/HubConnectionManagerTest.java b/src/test/java/net/sharksystem/hub/HubConnectionManagerTest.java index 71e8938..46fe8f3 100644 --- a/src/test/java/net/sharksystem/hub/HubConnectionManagerTest.java +++ b/src/test/java/net/sharksystem/hub/HubConnectionManagerTest.java @@ -186,5 +186,30 @@ public void disconnectHubEdgeFailedConnectionAttempt() throws IOException, Shark assertEquals(1, hubConnectionManager.getFailedConnectionAttempts().size()); } + @Test + public void connectHubEdgeSuccessfulAttemptAfterFailedAttempt() throws IOException, SharkException, InterruptedException { + HubConnectorDescription hubDescriptionSecondHub = new TCPHubConnectorDescriptionImpl("localhost", + hubPort + 1, multiChannel); + hubConnectionManager.connectHub(hubDescriptionSecondHub); + // give it some time for connection attempt + Thread.sleep(1000); + + // first attempt should fail, because hub wasn't started yet + assertEquals(0, hubConnectionManager.getConnectedHubs().size()); + assertEquals(1, hubConnectionManager.getFailedConnectionAttempts().size()); + + ASAPTCPHub asapHub2 = ASAPTCPHub.startTCPHubThread(hubPort+1, multiChannel, MAX_IDLE_IN_SECONDS); + hubConnectionManager.connectHub(hubDescriptionSecondHub); + + // give it some time for connection establishment + Thread.sleep(1000); + // connection should be established now + assertEquals(1, hubConnectionManager.getConnectedHubs().size()); + // failed attempt from first try should be removed + assertEquals(0, hubConnectionManager.getFailedConnectionAttempts().size()); + + asapHub2.kill(); + } + } \ No newline at end of file diff --git a/src/test/java/net/sharksystem/hub/KnownBugsHubConnectionManagerTest.java b/src/test/java/net/sharksystem/hub/KnownBugsHubConnectionManagerTest.java index beeaf33..d254a8e 100644 --- a/src/test/java/net/sharksystem/hub/KnownBugsHubConnectionManagerTest.java +++ b/src/test/java/net/sharksystem/hub/KnownBugsHubConnectionManagerTest.java @@ -27,7 +27,6 @@ public class KnownBugsHubConnectionManagerTest { @Before public void setUp() throws Exception { hubPort = TestHelper.getPortNumber(); - HubConnectorDescription localHostHubDescription = new TCPHubConnectorDescriptionImpl("localhost", hubPort, multiChannel); ASAPPeerFS asapPeer = new ASAPTestPeerFS(ALICE_ID, Collections.singletonList(FORMAT)); hubConnectionManager = new HubConnectionManagerImpl(new ASAPEncounterManagerImpl(asapPeer), asapPeer); asapHub = ASAPTCPHub.startTCPHubThread(hubPort, multiChannel, MAX_IDLE_IN_SECONDS); @@ -60,29 +59,4 @@ public void connectHubEdgeTwoAttempts() throws IOException, SharkException, Inte asapHub2.kill(); } - @Test - public void connectHubEdgeSuccessfulAttemptAfterFailedAttempt() throws IOException, SharkException, InterruptedException { - HubConnectorDescription hubDescriptionSecondHub = new TCPHubConnectorDescriptionImpl("localhost", - hubPort + 1, multiChannel); - hubConnectionManager.connectHub(hubDescriptionSecondHub); - // give it some time for connection attempt - Thread.sleep(1000); - - // first attempt should fail, because hub wasn't started yet - assertEquals(0, hubConnectionManager.getConnectedHubs().size()); - assertEquals(1, hubConnectionManager.getFailedConnectionAttempts().size()); - - ASAPTCPHub asapHub2 = ASAPTCPHub.startTCPHubThread(hubPort+1, multiChannel, MAX_IDLE_IN_SECONDS); - hubConnectionManager.connectHub(hubDescriptionSecondHub); - - // give it some time for connection establishment - Thread.sleep(1000); - // connection should be established now - assertEquals(1, hubConnectionManager.getConnectedHubs().size()); - // failed attempt from first try should be removed - assertEquals(0, hubConnectionManager.getFailedConnectionAttempts().size()); - - asapHub2.kill(); - } - } \ No newline at end of file From e26bd7c3de49f8d1cbd6917950530fbf8021ce17 Mon Sep 17 00:00:00 2001 From: Marvin Rausch Date: Thu, 22 Jun 2023 18:56:48 +0200 Subject: [PATCH 2/2] HubConnectionManager: Fix the bug where the failed attempt list is not cleared after the 'disconnect' function is called --- .../hub/BasicHubConnectionManager.java | 62 ++++++++++++------- .../hub/HubConnectionManagerTest.java | 5 +- 2 files changed, 41 insertions(+), 26 deletions(-) diff --git a/src/main/java/net/sharksystem/hub/BasicHubConnectionManager.java b/src/main/java/net/sharksystem/hub/BasicHubConnectionManager.java index 84dd4cc..875961e 100644 --- a/src/main/java/net/sharksystem/hub/BasicHubConnectionManager.java +++ b/src/main/java/net/sharksystem/hub/BasicHubConnectionManager.java @@ -11,9 +11,9 @@ * A streamlined facade meant to be used on service side of an ASAP application that makes use of hubs. * This class assumes the proposed architecture: A connection handler (in most cases subclassed from ASAPPeer) * is wrapped into an encounter manager. - * + *

* Instances of that hub connection manager class are initiated with that encounter manager and an ASAPPeer instance. - * + *

* It provides just a few methods to connect and disconnect hubs. Some optional behaviour is hidden. */ public abstract class BasicHubConnectionManager implements HubConnectionManager { @@ -30,7 +30,7 @@ public abstract class BasicHubConnectionManager implements HubConnectionManager * Hub manager can be asked to connect to or disconnect from hubs. We send a list of hubs which are to be * connected. Connection establishment can take a while, though. It is a wish list on application side for a while. * Connection establishment can fail - not any wish can be fulfilled. - * + *

* We assume that this methode is called due to user interaction. Meaning: Intervals between two calls are long * enough to establish a connection or fail in the attempt. Implication: We assume: hub internal list is always * accurate. We can keep track of failed attempts. And we do. @@ -38,25 +38,25 @@ public abstract class BasicHubConnectionManager implements HubConnectionManager protected void syncLists() { // avoid calls within milliseconds long now = System.currentTimeMillis(); - if(now - this.lastSync <= 100) return; + if (now - this.lastSync <= 100) return; this.lastSync = now; // ask for current list from hub List toBeRemoved = new ArrayList<>(); - for(HubConnectorDescription wishedConnection : this.hcdList) { + for (HubConnectorDescription wishedConnection : this.hcdList) { boolean found = false; - for(HubConnectorDescription runningConnection : this.hcdListHub) { - if(wishedConnection.isSame(runningConnection)) { + for (HubConnectorDescription runningConnection : this.hcdListHub) { + if (wishedConnection.isSame(runningConnection)) { found = true; break; // found - go ahead. } } // remove failed attempt from connected hubs list - if(!found) toBeRemoved.add(wishedConnection); + if (!found) toBeRemoved.add(wishedConnection); } // remove and remember unfulfilled wishes - for(HubConnectorDescription failedConnection : toBeRemoved) { + for (HubConnectorDescription failedConnection : toBeRemoved) { // 1st remove from wish list this.hcdList.remove(failedConnection); @@ -68,32 +68,37 @@ protected void syncLists() { break; } } - if(recordedOldAttempt != null) this.failedConnectionAttempts.remove(recordedOldAttempt); + if (recordedOldAttempt != null) this.failedConnectionAttempts.remove(recordedOldAttempt); this.failedConnectionAttempts.add(new HubConnectionManager.FailedConnectionAttempt() { @Override - public HubConnectorDescription getHubConnectorDescription() { return failedConnection; } + public HubConnectorDescription getHubConnectorDescription() { + return failedConnection; + } + @Override - public long getTimeStamp() { return lastConnectionAttempt; } + public long getTimeStamp() { + return lastConnectionAttempt; + } }); } // remove attempts which where successful later on List connectedNow = new ArrayList<>(); - for(HubConnectionManager.FailedConnectionAttempt failedAttempt: this.failedConnectionAttempts){ - for(HubConnectorDescription runningConnection : this.hcdListHub) { - if(failedAttempt.getHubConnectorDescription().isSame(runningConnection)) { + for (HubConnectionManager.FailedConnectionAttempt failedAttempt : this.failedConnectionAttempts) { + for (HubConnectorDescription runningConnection : this.hcdListHub) { + if (failedAttempt.getHubConnectorDescription().isSame(runningConnection)) { connectedNow.add(failedAttempt); } } } - for(FailedConnectionAttempt failedAttempt : connectedNow) { + for (FailedConnectionAttempt failedAttempt : connectedNow) { this.failedConnectionAttempts.remove(failedAttempt); } } private HubConnectorDescription findSameInList(HubConnectorDescription hcd, List hcdList) { - for(HubConnectorDescription hcdInList : hcdList) { - if(hcd.isSame(hcdInList)) { + for (HubConnectorDescription hcdInList : hcdList) { + if (hcd.isSame(hcdInList)) { return hcdInList; } } @@ -104,7 +109,7 @@ private HubConnectorDescription findSameInList(HubConnectorDescription hcd, List public void connectHub(HubConnectorDescription hcd) throws SharkException, IOException { this.lastConnectionAttempt = System.currentTimeMillis(); // new connection attempt // already connected? - if(this.findSameInList(hcd, this.hcdList) != null) return; // yes, connected: nothing to do here + if (this.findSameInList(hcd, this.hcdList) != null) return; // yes, connected: nothing to do here // else this.hcdList.add(hcd); } @@ -114,10 +119,21 @@ public void connectHub(HubConnectorDescription hcd) throws SharkException, IOExc public void disconnectHub(HubConnectorDescription hcd) throws SharkException, IOException { // *not* in there? HubConnectorDescription disconnectHcd = this.findSameInList(hcd, this.hcdList); - if(disconnectHcd == null) return; // not connected - nothing to do here - // else - this.hcdList.remove(disconnectHcd); - // sync hub connections + if (disconnectHcd != null) { + this.hcdList.remove(disconnectHcd); + } else { + // remove hcd from failed attempts list + FailedConnectionAttempt attemptToRemove = null; + for (FailedConnectionAttempt failedAttempt : this.failedConnectionAttempts) { + if(failedAttempt.getHubConnectorDescription().isSame(hcd)){ + attemptToRemove = failedAttempt; + break; + } + } + if (attemptToRemove != null) this.failedConnectionAttempts.remove(attemptToRemove); + } + + } @Override diff --git a/src/test/java/net/sharksystem/hub/HubConnectionManagerTest.java b/src/test/java/net/sharksystem/hub/HubConnectionManagerTest.java index 46fe8f3..5698f23 100644 --- a/src/test/java/net/sharksystem/hub/HubConnectionManagerTest.java +++ b/src/test/java/net/sharksystem/hub/HubConnectionManagerTest.java @@ -176,14 +176,13 @@ public void disconnectHubEdgeFailedConnectionAttempt() throws IOException, Shark assertEquals(1, hubConnectionManager.getFailedConnectionAttempts().size()); - hubConnectionManager.disconnectHub(localHostHubDescription); + hubConnectionManager.disconnectHub(hubDescriptionWrongPort); // give it some time for disconnecting Thread.sleep(1000); // connected hub list should contain one item assertEquals(0, hubConnectionManager.getConnectedHubs().size()); // still one element left after disconnecting - // TODO clarify whether this is a bug - assertEquals(1, hubConnectionManager.getFailedConnectionAttempts().size()); + assertEquals(0, hubConnectionManager.getFailedConnectionAttempts().size()); } @Test