diff --git a/Guify/src/code/QueueEventManager.java b/Guify/src/code/QueueEventManager.java index fbef1cb..06ec5b1 100644 --- a/Guify/src/code/QueueEventManager.java +++ b/Guify/src/code/QueueEventManager.java @@ -11,8 +11,6 @@ public class QueueEventManager extends Observable { private QueueEventManager() {} // We need this object in order to retrieve old transfers which are not being transferred - // TODO Prove mathematical correctness - // See documentation ConcurrentLinkedQueue queue = new ConcurrentLinkedQueue(); public static synchronized QueueEventManager getInstance() { diff --git a/Guify/src/controllers/QueueController.java b/Guify/src/controllers/QueueController.java index 2f3c53b..5523c6c 100644 --- a/Guify/src/controllers/QueueController.java +++ b/Guify/src/controllers/QueueController.java @@ -10,7 +10,7 @@ import code.GuiAbstractions.Interfaces.IFrameFactory; import views.interfaces.IQueueFrame; import java.util.HashMap; -@SuppressWarnings("deprecation") +@SuppressWarnings("deprecation") // Observer - Observable are okay here public class QueueController implements Observer{ /* @@ -22,6 +22,7 @@ public class QueueController implements Observer{ private HashMap indexHashMap = new HashMap<>(); private long lastGuiExecutionTime = 0; private static final long THROTTLE_TIME_MS = 10; // 10ms = 1/100th of second + /* * ========== END Attributes ========== */ @@ -37,7 +38,32 @@ public class QueueController implements Observer{ QueueEventManager.getInstance().addObserver(this); // Get previous enqueued elements. Do not place before addObserver(this) or some - // transfers could go lost + // transfers could go lost. + // + // A necessary but not sufficient condition in order to claim that the Queue + // works well, is that when it gets opened at time x, all the non-finished + // transfers initiated in a time t such that t < x must be shown + // in the table. + // More specifically, x represents the time of completion of the instruction + // QueueEventManager.getInstance().addObserver(this); as + // any subsequent transfer initiated after this point will be guaranteed to + // be shown in the table (handled by the "update()" method). + // + // Having understood this, we may now suppose t1 to be the time of completion of the + // instruction QueueEventManager.getInstance().getQueue() and t2 to be + // the time of completion of the instruction + // QueueEventManager.getInstance().addObserver(this) where t1 < t2. + // This would've meant that any transfer initiated in a time t such that + // t1 < t < t2 would not have been adequately processed as "getQueue()" was + // already executed, and "addObserver(this)" would not have been performed yet, + // making the transfer not visible in the queue when the Queue frame would've + // been opened. + // + // One could argue that when any chunk of data is transferred at any time t + // where t > t2, the update() method will be called, showing the transfer in + // the queue. It's not guaranteed that this happens (as it may encounter an + // error before t2 and after t1 or simply in may complete in this time frame) + TransferProgress[] queued = QueueEventManager.getInstance().getQueue(); for(TransferProgress transferProgress : queued) { SwingUtilities.invokeLater(new Runnable() { @@ -49,14 +75,53 @@ public class QueueController implements Observer{ } } - // Executed on different threads + // Executed on different threads. Called by notifyObservers in + // QueueEventManager in turn called + // by the threads created in SshEngine. + // + // Keep in mind that: + // For all distinct pairs of calls on the method update(o, arg) + // denoted as (update(o, arg)_1 , update(o, arg)_2) having the same + // object "arg", it holds that + // ((TransferProgress)arg).getTransferredBytes() evaluated during + // update(o, arg)_1 is less or equal than ((TransferProgress)arg).getTransferredBytes() + // evaluated during update(o, arg)_2. + // In simple words, if update() is called at time t1 and at time t2, + // where arg is the same TransferProgress object, then the second time + // the value of + // transferProgress.getTransferredBytes() will be greater or equal + // than the value of + // transferProgress.getTransferredBytes() evaluated the first time. + // This happens as the transfer of a single object is an operation executed + // on a single thread, hence any update will have a getTransferredBytes() greater + // or equal than the previous one. + // + // Keep also in mind that SwingUtilities.invokeLater() called at time t1 + // will always run before SwingUtilities.invokeLater() called at time t2 + // where t2 > t1. In other words, the order of execution is kept. + // + // This observation may be sufficient to prove that at any + // time t1, the value of the percentage of a specific TransferProgress + // will always be equal or greater than the value it was at any time t + // where t < t1. In other words it's impossible that the progress bar + // will go down (e.g. from 50% to 49%), but this would be true even if + // the concept described above would be false. This happens because + // the TransferProgress + // handled by manageTransferProgress will always have the latest updated values + // for that specific object, as it is shared with the thread + // which continuously updates it. + // We do not need any locks as no race conditions + // can happen because in the EDT thread we only read said object, nor any + // inconsistencies can arise. + // For more information you can view the comments at Queue@manageTransferProgress @Override public void update(Observable o, Object arg) { TransferProgress transferProgress = (TransferProgress)arg; // Do not invoke the Event Dispatch Thread too frequently as // it may impact the performance of low-end computer systems. - if(System.currentTimeMillis() - lastGuiExecutionTime >= THROTTLE_TIME_MS || transferProgress.getTransferStatus() == TransferProgress.END) { + if(System.currentTimeMillis() - lastGuiExecutionTime >= THROTTLE_TIME_MS || + transferProgress.getTransferStatus() == TransferProgress.END) { SwingUtilities.invokeLater(new Runnable() { @Override public void run() { @@ -67,6 +132,12 @@ public class QueueController implements Observer{ } } + /** + * Computes the transfer completion percentage + * @param transferProgress A transferProgress object + * @return A value in the range [0, 100] indicating + * the transfer completion percentage + */ public int computePercentage(TransferProgress transferProgress) { // Avoid division by zero if(transferProgress.getTotalBytes() == 0) { @@ -74,31 +145,40 @@ public class QueueController implements Observer{ // If there are no bytes in the file, this logic proposition holds true (vacuous truth) return 100; } - - // Avoid "stuck at 99%" due to precision issues - else if(transferProgress.getTotalBytes() == transferProgress.getTransferredBytes()) { - return 100; - } - else { - return (int) Math.floor( ((transferProgress.getTransferredBytes() * 100) / transferProgress.getTotalBytes()) ); + return (int) Math.round( ((transferProgress.getTransferredBytes() * 100F) / transferProgress.getTotalBytes()) ); } } + /** + * Given a TransferProgress object, retrieve its index + * @param transferProgress + * @return Its index or null if not present + */ public Integer getTableIndex(TransferProgress transferProgress) { return indexHashMap.get(transferProgress); } + /** + * Put index in the HashMap + * @param transferProgress TransferProgress object + * @param index An integer value representing the index + */ public void putTableIndex(TransferProgress transferProgress, Integer index) { indexHashMap.put(transferProgress, index); } - public void showFrame(boolean visible) { - frame.setVisible(visible); - } - + /** + * Checks if a specific TransferProgress is contained in + * the HashMap + * @param transferProgress A TransferProgress object + * @return true if present, false otherwise + */ public boolean isTransferProgressInHashMap(TransferProgress transferProgress) { return getTableIndex(transferProgress) != null; } + public void showFrame(boolean visible) { + frame.setVisible(visible); + } } diff --git a/Guify/src/views/Queue.java b/Guify/src/views/Queue.java index 0e93d8d..bc35d96 100644 --- a/Guify/src/views/Queue.java +++ b/Guify/src/views/Queue.java @@ -80,6 +80,13 @@ public class Queue extends JFrame implements IQueueFrame { return tableModel.getRowCount() - 1; } + /** + * Given a value (representing the transfer completion percentage) + * and an index representing the 0-based index of the row to update, + * update that row with that value + * @param rowIndex 0-base index of the row to update + * @param percentage The transfer completion percentage to set + */ public void updateRow(int rowIndex, int percentage) { if(rowIndex < tableModel.getRowCount()) { tableModel.setValueAt(percentage, rowIndex, 3); @@ -92,33 +99,47 @@ public class Queue extends JFrame implements IQueueFrame { // and we'll prove its correctness in all the cases: // // 1. Init: This method can receive a transferProgress whose status is INIT and - // a. Not present in the HashMap: this can happen both when QueueController receives - // an update() and when it iterates in the constructor over the previously enqueued + // a. Not present in the HashMap: this can happen either when QueueController receives + // an update() or when it iterates in the constructor over the previously enqueued // elements. In both cases, the transfer gets correctly put in the table. // b. Present in the HashMap: despite it's counterintuitive, this can happen when // a transfer gets initialized after QueueEventManager.getInstance().addObserver(this); // and before QueueEventManager.getInstance().getQueue(). // This would lead either update() // or the QueueController's constructor to call this method over a transferProgress - // already in the HashMap but whose status is INIT. In this case, updateRow() will + // already in the HashMap (because inserted by the other one) + // but whose status is INIT. In this case, updateRow() will // be called, but without any side effects as the percentage would be zero regardless. // // 2. Updating: // a. Not present in the HashMap: This can happen when the Queue UI is opened - // while an element is already being transferred. If it's not present in the HashMap - // it's added. - // b. Present in the HashMap: it will be updated. + // while an element is already being transferred. This happens because when + // the transfer had a "INIT" status, this object did not exist yet. + // If it's not present in the HashMap + // then it will be added. + // b. Present in the HashMap: then it will be correctly updated. // // 3. End: Same case for Updating // - // If any update gets triggered after QueueEventManager.getInstance().addObserver(this); - // and before QueueEventManager.getInstance().getQueue(), it is possible that [...] - // but this would not create any problems + // It's important to note that this method will always operate over the + // last version of the same TransferProgress object as it is updated + // from another thread when any chunk of data is transmitted. + // This will not create any inconsistencies, because the only + // attribute that can be different is getTransferredBytes which will + // be read just once per call, nor race conditions + // because this method only performs reads, without modifying. + // + // This ensures that at any + // time t1, the value of the percentage of a specific TransferProgress + // will always be equal or greater than the value it was at any time t + // where t < t1. In other words it's impossible that the progress bar + // will go down (e.g. from 50% to 49%). @Override public void manageTransferProgress(TransferProgress transferProgress) { // Remember that when QueueController calls frame.manageTransferProgress(transferProgress), - // here transferProgress might have a different status (as it's updated by a different thread). + // here transferProgress might have different attributes than it was originally called on + // (as it's updated by a different thread). // We do not need a lock as we do not edit it, but just keep it in mind. if(!controller.isTransferProgressInHashMap(transferProgress)) {