Feature: Fresh transactions to request (#1316)

src/main/java/com/iota/iri/network/TransactionRequester.java

@@ -71,14 +71,31 @@ public void requestTransaction(Hash hash, boolean milestone) throws Exception {
                     transactionsToRequest.remove(hash);
                     milestoneTransactionsToRequest.add(hash);
                 } else {
-                    if(!milestoneTransactionsToRequest.contains(hash) && !transactionsToRequestIsFull()) {
+                    if(!milestoneTransactionsToRequest.contains(hash)) {
+                        if (transactionsToRequestIsFull()) {
+                            popEldestTransactionToRequest();
+                        }
                         transactionsToRequest.add(hash);
                     }
                 }
             }
         }
     }
 
+    /**
+     * This method removes the oldest transaction in the transactionsToRequest Set.
+     *
+     * It used when the queue capacity is reached, and new transactions would be dropped as a result.
+     */
+    // @VisibleForTesting
+    void popEldestTransactionToRequest() {
+        Iterator<Hash> iterator = transactionsToRequest.iterator();
+        if (iterator.hasNext()) {
+            iterator.next();
+            iterator.remove();
+        }
+    }
+
     /**
      * This method allows to check if a transaction was requested by the TransactionRequester.
      *
@@ -111,13 +128,14 @@ public Hash transactionToRequest(boolean milestone) throws Exception {
         synchronized (syncObj) {
             // repeat while we have transactions that shall be requested
             while (requestSet.size() != 0) {
-                // remove the first item in our set for further examination
+                // get the first item in our set for further examination
                 Iterator<Hash> iterator = requestSet.iterator();
                 hash = iterator.next();
-                iterator.remove();
 
                 // if we have received the transaction in the mean time ....
                 if (TransactionViewModel.exists(tangle, hash)) {
+                    // we remove the transaction from the queue since we got it
+                    iterator.remove();
                     // ... dump a log message ...
                     log.info("Removed existing tx from request list: " + hash);
                     messageQ.publish("rtl %s", hash);
@@ -126,11 +144,6 @@ public Hash transactionToRequest(boolean milestone) throws Exception {
                     continue;
                 }
 
-                // ... otherwise -> re-add it at the end of the set ...
-                //
-                // Note: we always have enough space since we removed the element before
-                requestSet.add(hash);
-
                 // ... and abort our loop to continue processing with the element we found
                 break;
             }

src/test/java/com/iota/iri/controllers/TransactionRequesterTest.java → src/test/java/com/iota/iri/network/TransactionRequesterTest.java

@@ -1,8 +1,8 @@
-package com.iota.iri.controllers;
+package com.iota.iri.network;
 
 import com.iota.iri.conf.MainnetConfig;
+import com.iota.iri.controllers.TransactionViewModelTest;
 import com.iota.iri.model.Hash;
-import com.iota.iri.network.TransactionRequester;
 import com.iota.iri.service.snapshot.SnapshotProvider;
 import com.iota.iri.service.snapshot.impl.SnapshotProviderImpl;
 import com.iota.iri.storage.Tangle;
@@ -11,6 +11,10 @@
 import org.junit.Before;
 import org.junit.Test;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
 import static org.junit.Assert.*;
 
 /**
@@ -76,6 +80,48 @@ public void instance() throws Exception {
 
     }
 
+    @Test
+    public void popEldestTransactionToRequest() throws Exception {
+        TransactionRequester txReq = new TransactionRequester(tangle, snapshotProvider, mq);
+        // Add some Txs to the pool and see if the method pops the eldest one
+        Hash eldest = TransactionViewModelTest.getRandomTransactionHash();
+        txReq.requestTransaction(eldest, false);
+        txReq.requestTransaction(TransactionViewModelTest.getRandomTransactionHash(), false);
+        txReq.requestTransaction(TransactionViewModelTest.getRandomTransactionHash(), false);
+        txReq.requestTransaction(TransactionViewModelTest.getRandomTransactionHash(), false);
+
+        txReq.popEldestTransactionToRequest();
+        // Check that the transaction is there no more
+        assertFalse(txReq.isTransactionRequested(eldest, false));
+    }
+
+    @Test
+    public void transactionRequestedFreshness() throws Exception {
+        // Add some Txs to the pool and see if the method pops the eldest one
+        List<Hash> eldest = new ArrayList<Hash>(Arrays.asList(
+                TransactionViewModelTest.getRandomTransactionHash(),
+                TransactionViewModelTest.getRandomTransactionHash(),
+                TransactionViewModelTest.getRandomTransactionHash()
+        ));
+        TransactionRequester txReq = new TransactionRequester(tangle, snapshotProvider, mq);
+        int capacity = TransactionRequester.MAX_TX_REQ_QUEUE_SIZE;
+        //fill tips list
+        for (int i = 0; i < 3; i++) {
+            txReq.requestTransaction(eldest.get(i), false);
+        }
+        for (int i = 0; i < capacity; i++) {
+            Hash hash = TransactionViewModelTest.getRandomTransactionHash();
+            txReq.requestTransaction(hash,false);
+        }
+
+        //check that limit wasn't breached
+        assertEquals("Queue capacity breached!!", capacity, txReq.numberOfTransactionsToRequest());
+        // None of the eldest transactions should be in the pool
+        for (int i = 0; i < 3; i++) {
+            assertFalse("Old transaction has been requested", txReq.isTransactionRequested(eldest.get(i), false));
+        }
+    }
+
     @Test
     public void nonMilestoneCapacityLimited() throws Exception {
         TransactionRequester txReq = new TransactionRequester(tangle, snapshotProvider, mq);