From 8c095a3d3e6c76a0333a2163abaad15243e86d66 Mon Sep 17 00:00:00 2001 From: xfarrow Date: Mon, 24 Jul 2023 11:43:05 +0200 Subject: [PATCH] Fix #2 --- .../Implementations/JFrameFactory.java | 39 +--- Guify/src/controllers/QueueController.java | 182 +++++------------- Guify/src/views/Queue.java | 45 ++++- Guify/src/views/interfaces/IQueueFrame.java | 5 +- 4 files changed, 101 insertions(+), 170 deletions(-) diff --git a/Guify/src/code/GuiAbstractions/Implementations/JFrameFactory.java b/Guify/src/code/GuiAbstractions/Implementations/JFrameFactory.java index d51eb5f..b79de58 100644 --- a/Guify/src/code/GuiAbstractions/Implementations/JFrameFactory.java +++ b/Guify/src/code/GuiAbstractions/Implementations/JFrameFactory.java @@ -8,44 +8,8 @@ import views.*; */ public class JFrameFactory implements IFrameFactory { - public static Object createJFrame(int frameType) throws Exception { - - switch(frameType) { - - case LOGIN: - throw new Exception("Frame Login requires additional parameters. " - + "Call createJFrame(int frameType, Object additionalParameters) instead"); - - case DESKTOP: - throw new Exception("Frame Desktop requires additional parameters. " - + "Call createJFrame(int frameType, Object additionalParameters) instead"); - - case QUEUE: - return new Queue(); - - case NOTEPAD: - throw new Exception("Frame Notepad requires additional parameters. " - + "Call createJFrame(int frameType, Object additionalParameters) instead"); - - case FIND_AND_REPLACE: - throw new Exception("Frame FindAndReplace requires additional parameters. " - + "Call createJFrame(int frameType, Object additionalParameters) instead"); - - default: - throw new Exception("Invalid frame name"); - } - } - public static Object createJFrame(int frameType, Object controller) throws Exception{ - if( frameType != NOTEPAD - && frameType != FIND_AND_REPLACE - && frameType != LOGIN - && frameType != DESKTOP) { - System.err.println("additionalParams ignored for this frame"); - return createJFrame(frameType); - } - switch(frameType) { case LOGIN: @@ -59,6 +23,9 @@ public class JFrameFactory implements IFrameFactory { case FIND_AND_REPLACE: return new FindAndReplace(controller); + + case QUEUE: + return new Queue(controller); default: throw new Exception("Invalid frame name"); diff --git a/Guify/src/controllers/QueueController.java b/Guify/src/controllers/QueueController.java index 14aafe2..202ec02 100644 --- a/Guify/src/controllers/QueueController.java +++ b/Guify/src/controllers/QueueController.java @@ -2,37 +2,32 @@ package controllers; import java.util.Observable; import java.util.Observer; +import javax.swing.SwingUtilities; import code.QueueEventManager; import code.TransferProgress; import code.GuiAbstractions.Implementations.JFrameFactory; import code.GuiAbstractions.Interfaces.IFrameFactory; import views.interfaces.IQueueFrame; +import java.util.HashMap; -import java.util.concurrent.ConcurrentHashMap; -import javax.swing.SwingUtilities; - -import com.jcraft.jsch.SftpProgressMonitor; - -@SuppressWarnings("deprecation") // Observer/Observable objects are okay here -public class QueueController implements Observer { +@SuppressWarnings("deprecation") +public class QueueController implements Observer{ + + /* + * ========== BEGIN Attributes ========== + */ private IQueueFrame frame; - // A HashMap containing the Transfer Progress entry and the index of said entry - // in the table. - // We need a ConcurrentHashMap instead of a simple HashMap because it - // is accessed by multiple threads. In particular the threads executing update() - // and the Event Dispatch Thread. - // Alternatively, the HashMap could've been managed by the view itself without the need - // to concern over threading/concurrent problems. - // TODO Make the HashMap to be handled by the view itself (EDT thread) rather than concurrent threads - // to enhance readability and understanding - private ConcurrentHashMap indexAssociationMap = new ConcurrentHashMap<>(); - private final static int HASHMAP_DUMMY_VALUE = -1; + private HashMap indexHashMap = new HashMap<>(); + + /* + * ========== END Attributes ========== + */ // Executed by the EDT public QueueController() { try { - frame = (IQueueFrame) JFrameFactory.createJFrame(IFrameFactory.QUEUE); + frame = (IQueueFrame) JFrameFactory.createJFrame(IFrameFactory.QUEUE, this); } catch (Exception e) {} @@ -43,126 +38,55 @@ public class QueueController implements Observer { // transfers could go lost TransferProgress[] queued = QueueEventManager.getInstance().getQueue(); for(TransferProgress transferProgress : queued) { - // It is possible that while iterating on this for, the element - // has already been inserted into the indexAssociationMap from - // another thread executing update(), hence, we check if the key is contained - // already. - if (indexAssociationMap.putIfAbsent(transferProgress, HASHMAP_DUMMY_VALUE) == null) { - int percentage = (int) Math.floor( ((transferProgress.getTransferredBytes() * 100) / transferProgress.getTotalBytes()) ); - int rowIndex = frame.addRow(transferProgress.getSource(), transferProgress.getDestination(), transferProgress.getOperation() == SftpProgressMonitor.GET? "Download" : "Upload", percentage); - indexAssociationMap.put(transferProgress, rowIndex); - } + SwingUtilities.invokeLater(new Runnable() { + @Override + public void run() { + frame.manageTransferProgress(transferProgress); + } + }); } } - // Updated by QueueEventManager. Can run simultaneously - // on multiple threads. + // Executed by different threads @Override public void update(Observable o, Object arg) { - TransferProgress transferProgressObj = (TransferProgress)arg; - if(transferProgressObj.getTransferStatus() == TransferProgress.INIT) { - // Since the Runnable in SwingUtilities.invokeLater contained in this if - // might run *after* a subsequent execution of - // update() having a TransferProgress whose status is UPDATING, said subsequent update() - // must know that this specific transferProgressObj was already inserted in the HashMap, - // otherwise the if (indexAssociationMap.putIfAbsent(transferProgressObj, HASHMAP_DUMMY_VALUE) == null) - // (contained in the if checking whether the status is UPDATING) - // would return true and add it again to the table, because indexAssociationMap.put(transferProgressObj, rowIndex); - // has not been completed yet, being in the Runnable. - // - // We call putIfAbsent() instead of just put() because it is possible that this transferProgressObj is also - // in the initial for present in Queue() if this transfer was initiated in a time x where - // t1 < x < t2, where t1 is the time of completion of QueueEventManager.getInstance().addObserver(this); - // and t2 is the time of completion of QueueEventManager.getInstance().getQueue(); - // - // To sum it up: - // 1) update() receives transferProgressObj whose status is "INIT". Puts "DUMMY_VALUE" - // and schedules the EDT for a table insertion; - // 2) update() receives the same transferProgressObj with the status "UPDATING". - // It successfully sees that indexAssociationMap contains this specific transferProgressObj, - // whose value is DUMMY_VALUE, so it does not schedule an insert, but rather an update. While - // updating it will see that the value is not valid (if(rowIndex != HASHMAP_DUMMY_VALUE)) and will - // not perform an update as well. - // 3) The EDT runs and puts the correct index in the HashMap. - // - // Remember that if an update() receives a transferProgress whose status is INIT, that update() will always run - // before an update() on the same transferProgress whose status is UPDATING, as the thread handling - // each transferProgress is one and one only. - if(indexAssociationMap.putIfAbsent(transferProgressObj, HASHMAP_DUMMY_VALUE) == null) { - // We need SwingUtilities.invokeLater because we - // are not on the Event Dispatch Thread (the thread - // responsible for GUI management), but rather on the - // thread created in SshEngine - SwingUtilities.invokeLater(new Runnable() { - @Override - public void run() { - int rowIndex = frame.addRow(transferProgressObj.getSource(), transferProgressObj.getDestination(), transferProgressObj.getOperation() == SftpProgressMonitor.GET? "Download" : "Upload", 0); - indexAssociationMap.put(transferProgressObj, rowIndex); - } - }); - } + TransferProgress transferProgress = (TransferProgress)arg; + SwingUtilities.invokeLater(new Runnable() { + @Override + public void run() { + frame.manageTransferProgress(transferProgress); + } + }); + } + + public int computePercentage(TransferProgress transferProgress) { + // Avoid division by zero + if(transferProgress.getTotalBytes() == 0) { + // The percentage is 100% if ∀ byte in the file, byte was transferred. + // If there are no bytes in the file, this logic proposition holds true (vacuous truth) + return 100; } - else if(transferProgressObj.getTransferStatus() == TransferProgress.UPDATING) { - int percentage; - // Avoid division by zero - if(transferProgressObj.getTotalBytes() == 0) { - // The percentage is 100% if ∀ byte in the file, byte was transferred. - // If there are no bytes in the file, this logic proposition holds true (vacuous truth) - percentage = 100; - } - else { - percentage = (int) Math.floor( ((transferProgressObj.getTransferredBytes() * 100) / transferProgressObj.getTotalBytes()) ); - } - // It is possible to receive TransferProgress.UPDATING without receiving - // a TransferProgress.INIT (when this controller gets created when the transferring - // was already occurring) and before "for(TransferProgress transferProgress : queued)" - // gets executed on this element - if (indexAssociationMap.putIfAbsent(transferProgressObj, HASHMAP_DUMMY_VALUE) == null) { - // We need SwingUtilities.invokeLater because we - // are not on the Event Dispatch Thread (the thread - // responsible for GUI management), but rather on the - // thread created in SshEngine - SwingUtilities.invokeLater(new Runnable() { - @Override - public void run() { - int rowIndex = frame.addRow(transferProgressObj.getSource(), transferProgressObj.getDestination(), transferProgressObj.getOperation() == SftpProgressMonitor.GET? "Download" : "Upload", percentage); - indexAssociationMap.put(transferProgressObj, rowIndex); - } - }); - } - else { - int rowIndex = indexAssociationMap.get(transferProgressObj); - // It is possible that rowIndex is a DUMMY_VALUE if the insertion - // into the table has been scheduled with SwingUtilities.invokeLater - // but has not ran yet - if(rowIndex != HASHMAP_DUMMY_VALUE) { - // We need SwingUtilities.invokeLater because we - // are not on the Event Dispatch Thread (the thread - // responsible for GUI management), but rather on the - // thread created in SshEngine - SwingUtilities.invokeLater(new Runnable() { - @Override - public void run() { - frame.updateRow(rowIndex, percentage); - } - }); - } - else { - // TODO If the file is small enough, in all the (few) - // updates, rowIndex could always be HASHMAP_DUMMY_VALUE - // because when SwingUtilities tries to insert the row the first time, - // the EDT task will not execute soon enough before the termination - // of the transfer, so we will see that the percentage it's fixed - // to a specific value. - } - } + + // Avoid "stuck at 99%" due to precision issues + else if(transferProgress.getTotalBytes() == transferProgress.getTransferredBytes()) { + return 100; } - else if(transferProgressObj.getTransferStatus() == TransferProgress.END) { - // We choose not to remove the element from the table once it has finished + + else { + return (int) Math.floor( ((transferProgress.getTransferredBytes() * 100) / transferProgress.getTotalBytes()) ); } } + public Integer getTableIndex(TransferProgress transferProgress) { + return indexHashMap.get(transferProgress); + } + + public void putTableIndex(TransferProgress transferProgress, Integer index) { + indexHashMap.put(transferProgress, index); + } + public void showFrame(boolean visible) { - frame.setVisible(true); + frame.setVisible(visible); } + } diff --git a/Guify/src/views/Queue.java b/Guify/src/views/Queue.java index d795fa2..814a0c0 100644 --- a/Guify/src/views/Queue.java +++ b/Guify/src/views/Queue.java @@ -11,19 +11,24 @@ import javax.swing.table.DefaultTableCellRenderer; import javax.swing.table.DefaultTableModel; import javax.swing.table.TableColumn; +import com.jcraft.jsch.SftpProgressMonitor; + import code.Constants; +import code.TransferProgress; +import controllers.QueueController; import views.interfaces.IQueueFrame; public class Queue extends JFrame implements IQueueFrame { private static final long serialVersionUID = 1L; - + private QueueController controller; + /** * * Custom cell renderer in order to be able to display * a progress bar in the JTable * */ - public static class ProgressBarTableCellRenderer extends DefaultTableCellRenderer { + public static class ProgressBarTableCellRenderer extends DefaultTableCellRenderer { private static final long serialVersionUID = 1L; private JProgressBar progressBar; @@ -52,7 +57,8 @@ public class Queue extends JFrame implements IQueueFrame { public DefaultTableModel tableModel; - public Queue() { + public Queue(Object controller) { + this.controller = (QueueController) controller; setTitle("Queue"); String[] columnNames = {"Source", "Destination", "Operation", "Percentage"}; tableModel = new DefaultTableModel(columnNames, 0); @@ -81,4 +87,37 @@ public class Queue extends JFrame implements IQueueFrame { tableModel.setValueAt(percentage, rowIndex, 3); } } + + @Override + public void manageTransferProgress(TransferProgress transferProgress) { + + if(transferProgress.getTransferStatus() == TransferProgress.INIT) { + if(controller.getTableIndex(transferProgress) == null) { + controller.putTableIndex(transferProgress, + addRow(transferProgress.getSource(), + transferProgress.getDestination(), + transferProgress.getOperation() == SftpProgressMonitor.GET? "Download" : "Upload", + 0)); + } + } + + else if(transferProgress.getTransferStatus() == TransferProgress.UPDATING) { + Integer tableIndex = controller.getTableIndex(transferProgress); + if(tableIndex == null) { + controller.putTableIndex(transferProgress, + addRow(transferProgress.getSource(), + transferProgress.getDestination(), + transferProgress.getOperation() == SftpProgressMonitor.GET? "Download" : "Upload", + controller.computePercentage(transferProgress))); + } + else { + updateRow(tableIndex, controller.computePercentage(transferProgress)); + } + } + + else if(transferProgress.getTransferStatus() == TransferProgress.END) { + // Do nothing + } + + } } diff --git a/Guify/src/views/interfaces/IQueueFrame.java b/Guify/src/views/interfaces/IQueueFrame.java index 4ba8144..e141e3b 100644 --- a/Guify/src/views/interfaces/IQueueFrame.java +++ b/Guify/src/views/interfaces/IQueueFrame.java @@ -1,7 +1,8 @@ package views.interfaces; +import code.TransferProgress; + public interface IQueueFrame { public void setVisible(boolean visible); - int addRow(String source, String destination, String operation, int percentage); - void updateRow(int rowIndex, int percentage); + public void manageTransferProgress(TransferProgress transferProgress); }