JavaFX HTTP Download task im Hintergrund innerhalb GUI

[ralfb1105]

Hallo zusammen,

ich versuche mich gerade an einem HTTP Download Manager. Den Code ohne JavaFX habe ich am laufen und nun versuche ich das Ganze in eine kleine JavaFX basierte GUI zu "gießen". Ich versuche hierbei das MVC Pattern zu benutzen, so wie ich es hier im Forum gelernt habe. Als Basis hatte ich dazu mal mit Hilfe von dzim eine kleine Muster Applikation geschrieben. Siehe Github MVC-Example

Ich habe verschieden Eingabefelder in meinem MainController:



Wenn alle erfoderlichen Felder ausgefüllt sind wird der Start Button aktiv. Beim click auf den Start Button wird dann folgende Methode im MainController ausgeführt:

public void startDownloadButtonTapped() {
        // Clear messages TextArea/Labels before starting ...
        messages.setText("");
        message1Label.setText("");
        message2Label.setText("");

        toggleStartStopDownloadButton.setFadeTransitionShowButtonOne(false);
        pBar.progressProperty().bind(serviceWorkerTask1.progressProperty());
        downloadFolderTextField.setDisable(true);
        sourceUrlTextField.setDisable(true);
        proxyServerTextField.setDisable(true);
        proxyServerTextField.setOpacity(0.25);
        proxyPortTextField.setDisable(true);
        proxyPortTextField.setOpacity(0.25);
        model.setDownloadFinished(false);
        if (!serviceWorkerTask1.isRunning()) {
            LOGGER.info("Start downloading service task ...");
            serviceWorkerTask1.reset();
            serviceWorkerTask1.start();
        }
    }

Die Arbeit wird dann im ServiceWorkerTask1 gemacht - so meine Idee. Hier habe ich bis jetzt folgendes ausgeführt, was auch so funktioniert:

Service serviceWorkerTask1 = new Service() {
        int indexValue;

        @Override
        protected Task createTask() {
            return new Task() {
                @Override
                protected Void call() throws Exception {
                    // Start entering execution code here ...

                    // Check, and if necessary create DownloadFolder ...
                    dao.checkAndCreateDownloadFolder(downloadFolderTextField.getText());

                    // Get file size
                    model.setFileSize(dao.getFileSize(sourceUrlTextField.getText()));
                    if (model.getFileSize() != -1) {
                        Platform.runLater(() -> message2Label.setText("File size: " + model.getFileSize() + " Bytes"));
                    } else {
                        serviceWorkerTask1.onFailedProperty();
                    }
                  

//                    final int max = 10;
//                    int counter;
//                    for (counter = 1; counter <= max; counter++) {
//                        indexValue = model.getArrayIndex(counter, max);
//                        updateMessage("Value at index " + counter + ": " + indexValue);
//                        updateProgress(counter, max);
//                        Thread.sleep(1000);
//                    }

                    // End entering execution code here ...
                    return null;
                }

                @Override
                protected void succeeded() {
                    super.succeeded();
                    messages.appendText("Download Manager Task Succeeded!");
                    LOGGER.info("Download Manager Task Succeeded!");
                    updateProgress(1.0, 1.0);
                    toggleStartStopDownloadButton.setFadeTransitionShowButtonOne(true);
                    downloadFolderTextField.setDisable(false);
                    sourceUrlTextField.setDisable(false);
                    proxyServerTextField.setDisable(false);
                    proxyServerTextField.setOpacity(1.0);
                    proxyPortTextField.setDisable(false);
                    proxyPortTextField.setOpacity(1.0);
                }

                @Override
                protected void cancelled() {
                    super.cancelled();
                    messages.appendText("Download Manager Task Cancelled!");
                    LOGGER.info("Download Manager Task Cancelled!");
                    toggleStartStopDownloadButton.setFadeTransitionShowButtonOne(true);
                    downloadFolderTextField.setDisable(false);
                    sourceUrlTextField.setDisable(false);
                    proxyServerTextField.setDisable(false);
                    proxyServerTextField.setOpacity(1.0);
                    proxyPortTextField.setDisable(false);
                    proxyPortTextField.setOpacity(1.0);
                    updateProgress(0, 0);
                }

                @Override
                protected void failed() {
                    super.failed();
                    messages.appendText("Download Manager Task Failed!");
                    LOGGER.info("Download Manager Task Failed!");
                    toggleStartStopDownloadButton.setFadeTransitionShowButtonOne(true);
                    downloadFolderTextField.setDisable(false);
                    sourceUrlTextField.setDisable(false);
                    proxyServerTextField.setDisable(false);
                    proxyServerTextField.setOpacity(1.0);
                    proxyPortTextField.setDisable(false);
                    proxyPortTextField.setOpacity(1.0);
                    updateProgress(0, 0);
                }
            };
        }

1. Ich erstelle den DownloadeFolder falls dieser nicht existiert. Das ganze mache ich in einem DAOService:

public void checkAndCreateDownloadFolder(String downloadFolder) {
        try {
            downloadFolderFile = new File(downloadFolder);
            if (!downloadFolderFile.exists()) {
                downloadFolderFile.mkdirs();
            }
        } catch (Exception ex) {
            fireException(ex);
        }
    }

2. Dann ermiietel ich die Größe in Byte des zu ladenden Files:

public int getFileSize(String link) {
        URLConnection conn = null;
        try {
            URL url = new URL(link);
            Proxy proxy = new Proxy(Proxy.Type.HTTP, new InetSocketAddress("proxyserver", 8080));
            conn = url.openConnection(proxy);
            if (conn instanceof HttpURLConnection) {
                ((HttpURLConnection) conn).setRequestMethod("HEAD");
            }
            conn.getInputStream();
            return conn.getContentLength();
        } catch (Exception ex) {
            fireException(ex);
            return -1;
        } finally {
            if (conn instanceof HttpURLConnection) {
                ((HttpURLConnection) conn).disconnect();
            }
        }

    }

Auch das funktioniert soweit.

Info: Der Code für den Proxy Server (CheckBox) ist noch nicht implementiert, aktuell wird immer proxy verwendet.

Nun fehlt mir noch folgendes:
- Start Download des Files
- In der ProgressBar soll angezeigt werden wieviel Bytes von Gesamt Bytes geladen sind.
- Am Ende soll noch noch der Durchsatz angezeigt werden, z.B.: 2 MByte/sec.

Der Code aus dem Programm *OHNE* JavaFX sieht dann so aus:

public void run() {
        try {
            URL url = new URL(link);
            HttpURLConnection hConnection = (HttpURLConnection) url.openConnection();

            // Get size of file (int Bytes) to be downloaded ...
            int fileSize = getFileSize(url);

            // Inputstream arbeitet immer mit Byte
            BufferedInputStream bufferedInputStream = new BufferedInputStream(hConnection.getInputStream());

            // Datei schreiben / erstellen
            outputFile = new File(defaultDownloadFolder, "DownloadFile.data");
            OutputStream outputStream = new FileOutputStream(outputFile);
            BufferedOutputStream bOutputStream = new BufferedOutputStream(outputStream, 1024);

            byte[] buffer = new byte[1024];
            int downloaded = 0;
            int readByte = 0;

            while ((readByte = bufferedInputStream.read(buffer, 0, 1024)) >= 0) {
                bOutputStream.write(buffer, 0, readByte);
                downloaded += readByte;

                System.out.println("Bereits " + downloaded + " Byte " + " von " + fileSize + " Bytes geladen");
            }

            bOutputStream.close();
            bufferedInputStream.close();
            System.out.println("Download erfolgreich");

        } catch (Exception e) {
            e.printStackTrace();
        }

    }

An dieser Stelle fehlt mir nun die Idee wie ich diesen Download und die Anzeige bzw. Update der ProgressBar in mein Programm, sprich in den ServiceTask am Besten integrieren kann. Folgendes ist mir im Detail nicht klar:
1. Wie kann ich den Download im Hintergrund laufen lassen. Meine Idee wäre hier den Download Code in den DAOService zu integrieren.
2. Wenn der Download im DAOService läuft habe ich dort keinen Zugriff auf das Model um z.B. die geladenen Bytes zu speichern um diese dann im MainController anzuzeigen und damit auch die ProgressBar zu bedienen.
3. Ich müsste dann ja auch noch einen Merker, bzw. Trigger (BooleanProperty im Model !!??) haben der mir zeigt das der Download nun fertig ist.

Ich hoffe ich habe mein Problem anschaulich erklärt und Ihr habe eine Idee wie ich das programmieren könnte.

Gruß

Ralf

 

[mihe7]

mit Hilfe von dzim

Du meinst @dzim (kleiner Tipp: wenn Du ein @ davorstellst, erhöhst Du die Wahrscheinlichkeit, dass er Deinen Beitrag liest *evil-grin*)

 

[ralfb1105]

Hallo mihe7,

Danke für den Tipp werde ich beim nächsten Mal beherzigen ... hoffe das hierzu dennoch jemand etwas sagen kann ...

 

[mihe7]

Der Code aus dem Programm *OHNE* JavaFX sieht dann so aus:

Dort würde ich anfangen: ohne JavaFX.

Nehmen wir mal an, wir hätten eine Klasse Download. Die API könnte in etwa so aussehen:

class Download implements Runnable {
    public Download(URL url) { ... }
    public void addDownloadListener(DownloadListener l) { ... }
    public void removeDownloadListener(DownloadListener l) { ... }
    public void run() { ... }
}

Die run-Methode macht das, was Du oben gezeigt hast - mit Ausnahme der Ausgaben. Die ersetzt Du durch Benachrichtigungen der Listener.

Damit hast Du eine Download-Klasse, die unabhängig vom Framework funktioniert und über das Observer-Pattern entkoppelt ist.

Wie kann ich den Download im Hintergrund laufen lassen. Meine Idee wäre hier den Download Code in den DAOService zu integrieren.

Du würdest die Download-Klasse in einem Task verwenden.

Wenn der Download im DAOService läuft habe ich dort keinen Zugriff auf das Model um z.B. die geladenen Bytes zu speichern um diese dann im MainController anzuzeigen und damit auch die ProgressBar zu bedienen.

Hier sehe ich im Wesentlichen vier Möglichkeiten:
1. Du gibst das Model dem Task mit
2. Du gibst einen DownloadListener dem Task mit
3. Dein MainController registriert sich als Listener der Download-Klasse
4. Dein Task registriert sich als Listener der Download-Klasse und der MainController lauscht auf Änderungen von Properties, die der Service anbietet. Du könntest z. B. als Value-Typ die DownloadEvent-Klasse verwenden und dann die value-Property beim Eintreffen eines DownloadEvents aktualisieren.

Ich müsste dann ja auch noch einen Merker, bzw. Trigger (BooleanProperty im Model !!??) haben der mir zeigt das der Download nun fertig ist.

Wäre damit erschlagen, oder?

 

[ralfb1105]

Hallo mihe7,

erst einmal vielen Dank für Deine Ideen und Ausführungen.

Das mit dem DAOService habe ich aus meinen Projekten mit MVC und Datenbanken entnommen, mit dem Hintergrund das der Controller nicht wissen muss/sollte wie er an die Daten kommt - spricht einen Entkoppelung um dann gf. relativ einfach eine andere Download Art, wie. z.B. von HTTP auf FTP, umstellen zu können.

Nach Deinen Ausführungen und bei genauerer Betrachtung würde ich diese "Logik" in die Download Klasse implementieren, welches dann ja quasi die Entkoppelung von Controller zu DAO ist.

Soweit habe ich das verstanden und ich würde jetzt mal probieren eine Download Klasse zu kodieren. An Deiner Ausführung ist mir die wichtige Sache mit

public void addDownloadListener(DownloadListener l) { ... } public void removeDownloadListener(DownloadListener l) { ... }

absolut nicht klar. Wenn Du dazu noch etwas im Detail schreiben könntest wäre das sehr hilfreich.

Eine weitere Problematik ist mir auch noch nicht ganz klar.
Ich starte innerhalb des Service Task den Download, in etwa stelle ich mir das so vor ...

new Thread(new Download(downloadLink)).start();

An der Stelle muss der Service Task ja auf die Beendigung des Download warten und z.B. die ProgressBar aktualisieren und z.B. die bereits geladenen Bytes anzeigen/aktualisieren.

Wie würde ich das denn realisieren? Ich stelle mir vor das ich z.B. so lange in einer while-loop bleibe bis ein Property (im Model??) gesetzt ist.
Oder kommt hier der Listener in's Spiel den ich wie oben geschrieben noch nicht verstanden habe

Zu dem Punkt mit dem Austausch der Daten würde ich der Download Klasse das Model mitgeben - da wüsste ich wie das funktioniert Deine Vorschläge 2-4 sind vermutlich eleganter, aber da haben wir wieder die Listener ...

Gruß

Ralf

 

[Robat]

Habs jetzt nur überflogen, aber bringt die Task-Klasse den ganzen Spaß nicht schon mit sich?

public class HttpDownloadTask extends Task<File> {

    private String link;

    public HttpDownloadTask( String link ) {
        this.link = link;
    }

    @Override
    protected File call() throws Exception {
        File outputFile = null;
        try {
            URL url = new URL(link);
            HttpURLConnection hConnection = ( HttpURLConnection ) url.openConnection();

            updateMessage("Ermittle Dateigröße...");
            int fileSize = getFileSize();

            updateProgress(0, fileSize);
            Platform.runLater(() -> updateMessage(String.format("Download: %d / %d Byte ", 0, fileSize)));
            BufferedInputStream bufferedInputStream = new BufferedInputStream(hConnection.getInputStream());


            outputFile = new File("DownloadFile.data");
            OutputStream outputStream = new FileOutputStream(outputFile);
            BufferedOutputStream bOutputStream = new BufferedOutputStream(outputStream, 1024);

            byte[] buffer = new byte[ 1024 ];
            int downloaded = 0;
            int readByte = 0;

            while ( ( readByte = bufferedInputStream.read(buffer, 0, 1024) ) >= 0 ) {
                bOutputStream.write(buffer, 0, readByte);
                downloaded += readByte;

                final int progress = downloaded;
                updateProgress(downloaded, fileSize);
                Platform.runLater(() -> updateMessage(String.format("Download: %d / %d Byte ", progress, fileSize)));
            }

            bOutputStream.close();
            bufferedInputStream.close();
        } catch ( Exception e ) {
            Platform.runLater(() -> updateMessage("Beim Download ist ein Fehler aufgetreten"));
            failed();
        }
        return outputFile;
    }

    private int getFileSize() {
        URLConnection conn = null;
        try {
            URL url = new URL(link);
            conn = url.openConnection();
            if ( conn instanceof HttpURLConnection ) {
                ( ( HttpURLConnection ) conn ).setRequestMethod("HEAD");
            }
            conn.getInputStream();
            return conn.getContentLength();
        } catch ( Exception e ) {
            Platform.runLater(() -> updateMessage("Fehler bei der Ermittlung der Dateigröße!"));
            failed();
            return -1;
        } finally {
            if ( conn instanceof HttpURLConnection ) {
                ( ( HttpURLConnection ) conn ).disconnect();
            }
        }
    }
}

public class DownloadController implements Initializable {

    @FXML
    private ProgressIndicator downloadIndicator;
    @FXML
    private Button downloadStartButton;
    @FXML
    private Label downloadStatusLabel;

    private Service<File> downloadService;

    @FXML
    public void onDownloadStartPressed() {
        downloadStartButton.setDisable(true);
        downloadService.start();
    }

    @Override
    public void initialize( URL location, ResourceBundle resources ) {
        downloadService = new Service<>() {
            @Override
            protected Task<File> createTask() {
                HttpDownloadTask downloadTask = new HttpDownloadTask("https://speed.hetzner.de/100MB.bin");

                downloadStatusLabel.textProperty().bind(downloadTask.messageProperty());
                downloadIndicator.progressProperty().bind(downloadTask.progressProperty());

                downloadTask.exceptionProperty().addListener((obs, oldVal, newVal) -> downloadStatusLabel.setText(newVal.getMessage()));

                downloadTask.setOnSucceeded(event -> downloadStartButton.setDisable(false));
                downloadTask.setOnCancelled(event -> downloadStartButton.setDisable(false));
                downloadTask.setOnFailed(event -> downloadStartButton.setDisable(false));

                return downloadTask;
            }
        };
    }
}

 

[ralfb1105]

Hallo Robat,

ich habe das jetzt auch nur überflogen, wollte Dir aber schon mal DANKE sagen, das sieht in der Tat so aus wie das was ich vor habe - nur wesentlich übersichtlicher. Ich schaue mir das Morgen in Ruhe an, versuche es zu verstehen und bei mir zu implementieren.

Ich melde mich dann hier im Forum mit dem Ergebnis.

Was ich auch noch nicht kannte ist https://speed.hetzner.de da kann ich mir meine Test Files auf meinem Webspace ja sparen - Super - vielen Dank.

Gruß

Ralf

 

[Robat]

Was ich auch noch nicht kannte ist https://speed.hetzner.de da kann ich mir meine Test Files auf meinem

Das kannte ich vorher auch nicht. Hatte ich nur eben ergoogelt

 

[mihe7]

Habs jetzt nur überflogen, aber bringt die Task-Klasse den ganzen Spaß nicht schon mit sich?

Natürlich bringt die das mit, ist aber eine JavaFX-Klasse, womit die Logik nun fix an FX gebunden ist. Die Trennung davon wäre dann Punkt 4

absolut nicht klar. Wenn Du dazu noch etwas im Detail schreiben könntest wäre das sehr hilfreich.

Naja, ich habe einen etwas anderen Ansatz als @Robat gewählt, um den Kern des Downloads unabhängig von JavaFX zu halten. Stell Dir vor, Du baust eine Download-Bibliothek. Die kannst Du dann für FX, Swing, ggf. Android, ... verwenden.

Im Wesentlichen besteht der Download aus der Methode, die Du geschrieben hast. Lediglich die Statusmeldungen werden nicht am Bildschirm ausgegeben sondern an Beobachter (das wäre dann die registrierten DownloadListener) verteilt.

Diese Entkopplung erkaufst Du Dir mit erhöhter Komplexität gegenüber der Lösung, den Code direkt in den FX-Task einzubauen.

Wenn Du dazu Code sehen willst, kann ich Dir kurzfristig was zusammenschustern.

Ich starte innerhalb des Service Task den Download, in etwa stelle ich mir das so vor ...

Fast. Du brauchst keinen neuen Thread erzeugen, da der FX-Task bereits in einem separaten Thread ausgeführt wird. Sprich: einfach direkt die run()-Methode aufrufen.

 

[ralfb1105]

Hallo mihe7,

Wenn Du dazu Code sehen willst, kann ich Dir kurzfristig was zusammenschustern.

da ich das Ganze als Übung für mich sehe um den Umgang mit Files, JavaFX und grundsätzliche Java Themen mache, wäre es echt super wenn Du dazu etwas schreiben könntest was mir auf die Sprünge hilft um diesen Weg zu kodieren.

Ich werde auch den reinen JavaFX Weg von @Robat probieren, dann habe ich mehrere Wege und Beispiele . Wie von Dir geschrieben hatte ich, bevor ich @Robat Kommentar gelesen habe, die Download Klasse erstellt. Hier mal der Code als Referenz, wie gesagt noch vollkommen ohne Listener, etc.

package de.ralfb_web.services;

import java.io.BufferedInputStream;
import java.io.BufferedOutputStream;
import java.io.File;
import java.io.FileOutputStream;
import java.io.OutputStream;
import java.net.HttpURLConnection;
import java.net.InetSocketAddress;
import java.net.Proxy;
import java.net.URL;
import java.net.URLConnection;

import de.ralfb_web.model.Model;
import de.ralfb_web.utils.ExceptionListener;

public class DownloadWorker implements Runnable {

    private String link;
    private File downloadFolderFile;
    private File outputFile;
    private String saveFileName;
    private Model model;
    private Boolean useProxy;
    private String proxyServer;
    private int proxyPort;
    private ExceptionListener exceptionListener;
    private Proxy proxy;
    private HttpURLConnection hConnection;

    public DownloadWorker(String link, File downloadFolderFile, Model model, Boolean useProxy, String proxyServer,
            int proxyPort) {
        super();
        this.link = link;
        this.downloadFolderFile = downloadFolderFile;
        this.model = model;
        this.useProxy = useProxy;
        this.proxyServer = proxyServer;
        this.proxyPort = proxyPort;
        this.saveFileName = link.substring(link.lastIndexOf('/') + 1);

        if (!downloadFolderFile.exists()) {
            downloadFolderFile.mkdirs();
        }

        this.proxy = new Proxy(Proxy.Type.HTTP, new InetSocketAddress(proxyServer, proxyPort));

    }

    public DownloadWorker(String link, File downloadFolderFile, String saveFileName, Model model, Boolean useProxy) {
        super();
        this.link = link;
        this.downloadFolderFile = downloadFolderFile;
        this.saveFileName = saveFileName;
        this.model = model;
        this.useProxy = useProxy;
        this.saveFileName = link.substring(link.lastIndexOf('/') + 1);

        if (!downloadFolderFile.exists()) {
            downloadFolderFile.mkdirs();
        }
    }

    /**
     * Method to register an ExceptionListener
     * 
     * @param l ExceptionListener
     */
    public void registerExceptionListener(ExceptionListener l) {
        exceptionListener = l;
    }

    /**
     * Method to remove (set to null) an exception listener
     */
    public void removeExceptionListener() {
        exceptionListener = null;
    }

    /**
     * Method to fire an exception using exceptionOccured() method
     * 
     * @param th Throwable
     */
    private void fireException(Throwable th) {
        if (exceptionListener != null) {
            exceptionListener.exceptionOccurred(th);
        }
    }

    @Override
    public void run() {
        try {
            URL url = new URL(link);
            if (useProxy) {
                hConnection = (HttpURLConnection) url.openConnection(proxy);
            } else {
                hConnection = (HttpURLConnection) url.openConnection();
            }

            // Get size of file (int Bytes) to be downloaded ...
            int fileSize = getFileSize(url);

            // Inputstream always works with Byte
            BufferedInputStream bufferedInputStream = new BufferedInputStream(hConnection.getInputStream());

            // Write File
            outputFile = new File(downloadFolderFile, saveFileName);
            OutputStream outputStream = new FileOutputStream(outputFile);
            BufferedOutputStream bOutputStream = new BufferedOutputStream(outputStream, 1024);

            byte[] buffer = new byte[1024];
            // How many bytes are already downloaded
            int downloaded = 0;
            // Actual read bytes
            int readByte = 0;

            while ((readByte = bufferedInputStream.read(buffer, 0, 1024)) >= 0) {
                bOutputStream.write(buffer, 0, readByte);
                downloaded += readByte;

                System.out.println("Bereits " + downloaded + " Byte " + " von " + fileSize + " Bytes geladen");
            }

            bOutputStream.close();
            bufferedInputStream.close();
            System.out.println("Download erfolgreich");

        } catch (Exception ex) {
            fireException(ex);
        }
    }

    public int getFileSize(URL url) {
        URLConnection conn = null;
        try {
            if (useProxy) {
                conn = url.openConnection(proxy);
            } else {
                conn = url.openConnection();
            }

            if (conn instanceof HttpURLConnection) {
                ((HttpURLConnection) conn).setRequestMethod("HEAD");
            }
            conn.getInputStream();
            return conn.getContentLength();
        } catch (Exception ex) {
            fireException(ex);
            return -1;
        } finally {
            if (conn instanceof HttpURLConnection) {
                ((HttpURLConnection) conn).disconnect();
            }
        }

    }

}

Aufgerufen habe ich das dann testweise mal in dem ServiceWorkerTask:

DownloadWorker dw = new DownloadWorker(sourceUrlTextField.getText(), downloadFolderFile, model, httpProxyCheckBox.isSelected(), proxyServerTextField.getText(), Integer.valueOf(proxyPortTextField.getText()));
dw.run();

Info: Die Ausgaben habe ich in der DownloadWorker Klasse zur Kontrolle auf sysout belassen.

Gruß,

Ralf

 

[mihe7]

wie gesagt noch vollkommen ohne Listener, etc.

Du hast ja schon einen ExceptionListener wenn Du das noch als Liste implementierst, dann kannst Du mehrere registrieren (ist aber nicht unbedingt notwendig).

public interface DownloadListener {
    void stateChanged(DownloadEvent e);
}

Spoiler: DownloadEvent.java

import java.net.URL;
import java.util.Optional;

public class DownloadEvent {
    public enum Type {START,RECEIVED,FINISHED,FAILED};

    private final Type type;
    private final URL url;
    private final long total;
    private final long received;
    private final long duration;
    private final Exception cause;

    public DownloadEvent(Type type, URL url, long total, long received, long duration,
                         Exception cause) {
        this.type = type;
        this.url = url;
        this.total = total;
        this.received = received;
        this.duration = duration;
        this.cause = cause;
    }

    public DownloadEvent(URL url, Exception cause) {
        this(Type.FAILED, url, -1L, -1L, -1L, cause);
    }

  
    public DownloadEvent(Type type, URL url, long total, long received, long duration) {
        this(type, url, total, received, duration, null);
    }

    public DownloadEvent(URL url, long total, long received, long duration) {
        this(Type.RECEIVED, url, total, received, duration, null);
    }

    public Type getType() { return type; }
    public URL getURL() { return url; }
    public long getTotal() { return total; }
    public long getReceived() { return received; }
    public long getDuration() { return duration; }
    public Optional<Exception> getCause() { return Optional.ofNullable(cause); }
}

Spoiler: Download.java - Fake

import java.net.URL;
import java.util.ArrayList;
import java.util.List;
import java.util.Random;

public class Download implements Runnable {
    private final List<DownloadListener> listeners = new ArrayList<>();
    private final URL url;

    public Download(URL url) {
        this.url = url;
    }

    public void addDownloadListener(DownloadListener l) {
        listeners.add(l);
    }

    public void removeDownloadListener(DownloadListener l) {
        listeners.remove(l);
    }

    private void fire(DownloadEvent e) {
        listeners.forEach(l -> l.stateChanged(e));
    }

    @Override
    public void run() {
        Random rand = new Random();
        long total = rand.nextInt(10_000_000 - 1_000_000) + 1_000_000;
        long recvd = 0L;
        long start = System.currentTimeMillis();
        long duration = 0L;
        try {
            while (recvd < total) {
                Thread.sleep(500);
                long bytes = rand.nextInt(1_000_000);
                duration = System.currentTimeMillis() - start;
                recvd = Math.min(recvd + bytes, total);
                fire(new DownloadEvent(url, total, recvd, duration));
            }
            fire(new DownloadEvent(DownloadEvent.Type.FINISHED, url, total, recvd, duration));
        } catch (Exception ex) {
            fire(new DownloadEvent(url, ex));
        }
    }
}

Anbindung an die "Konsole":

import java.util.concurrent.Executors;
import java.util.concurrent.ExecutorService;

public class TestDownload {
     public static void main(String[] args) throws Exception {
         Download download = new Download(new java.net.URL("http://www.google.de"));
         download.addDownloadListener(e -> {
             long total = e.getTotal();
             long received = e.getReceived();
             long duration = e.getDuration();

             switch(e.getType()) {
                 case FINISHED:
                     System.out.printf("Durchsatz: %d KB/s\n", total / duration);
                     break;
                 case RECEIVED:
                     System.out.printf("%d %% erledigt, %d von %d Bytes empfangen\n",
                                       received*100/total, received, total);
                     break;
                 case FAILED:
                     System.out.printf("Fehlgeschlagen");
                     e.getCause().ifPresent(Exception::printStackTrace);
                     break;
                 default:
                     System.out.println("Ereignis ignoriert");
                     break;
             }
        });

        // Start in eigenem Thread
        ExecutorService service = Executors.newSingleThreadExecutor();
        service.submit(download);
        service.shutdown();

        // Alternativ: start in laufendem Thread
        // download.run();
     }
}

Das wäre eine Möglichkeit. Schöner (weil man ohne switch auskommt und trotzdem ein funktionales Interfaces hat) wäre es, wenn man die Registrierung abhängig vom Event-Type macht... Dazu könnte man eine Map<DownloadEvent.Type, List<DownloadListener>> statt einer popligen Liste verwenden.

Der Einbau in Deinen FX-Task funktioniert genauso.

    @Override
    protected File call() throws Exception {
        Download download = new Download(link);
        download.addDownloadListener(this::handleDownloadEvent);
        download.run(); // läuft im FX-Hintergrundthread
    }

    private void handleDownloadEvent(DownloadEvent e) {
         long total = e.getTotal();
         long received = e.getReceived();
         long duration = e.getDuration();

         switch(e.getType()) {
             case FINISHED:
                 Platform.runLater(() -> updateMessage(String.format("Download beendet. Durchsatz %d KB/s", total / duration)));
                 break;
             case RECEIVED:
                 Platform.runLater(() -> updateMessage(String.format("Download: %d / %d Byte ", received, total)));
                 break;
             case FAILED:
                 Platform.runLater(() -> updateMessage("Beim Download ist ein Fehler aufgetreten"));
                 e.getCause().ifPresent(Exception::printStackTrace);
                 break;
             default:
                 break;
         }
    }

 

[Robat]

Natürlich bringt die das mit, ist aber eine JavaFX-Klasse, womit die Logik nun fix an FX gebunden ist

Das stimmt wohl. Hätte ich den Thread nicht nur überflogen, wüsste ich auch auf was du hinaus wolltest

EDIT: @mihe7 war wohl schneller Code daher entfernt

 

[mihe7]

EDIT: @mihe7 war wohl schneller

Dafür kommt Deine Lösung ohne switch aus - das würde bei mir mit der genannten Änderung zwar auch gehen, dann wäre ich aber langsamer gewesen als Du

Spoiler

Gut, ich gebs zu: das war nicht der Grund, ich war einfach zu faul

 

[Robat]

Jetzt hast du meine Lösung angesprochen .. da muss ich sie ja doch wieder posten. Menno!

Spoiler

public interface Download {
    void addDownloadListener(DownloadListener listener);
    void removeDownloadListener(DownloadListener listener);
    File execute();
}
public interface DownloadListener {
    void onUpdateProgress(int progress, int total);
    void onUpdateMessage(String message);
    void onSucceed();
    void doCancel();
    void onFail(Exception e);
}

Spoiler

public class HttpDownload implements Download {

    private List<DownloadListener> listeners;
    private String link;

    public HttpDownload( String link ) {
        this.listeners = new ArrayList<>();
        this.link = link;
    }

    public File execute() {
        File outputFile = null;
        try {
            URL url = new URL(link);
            HttpURLConnection hConnection = ( HttpURLConnection ) url.openConnection();

            listeners.forEach(l -> l.onUpdateMessage("Ermittle Dateigröße..."));
            int fileSize = getFileSize();

            listeners.forEach(l -> {
                l.onUpdateProgress(0, fileSize);
                l.onUpdateMessage(String.format("Download: %d / %d Byte ", 0, fileSize));
            });
            BufferedInputStream bufferedInputStream = new BufferedInputStream(hConnection.getInputStream());


            outputFile = new File("DownloadFile.data");
            OutputStream outputStream = new FileOutputStream(outputFile);
            BufferedOutputStream bOutputStream = new BufferedOutputStream(outputStream, 1024);

            byte[] buffer = new byte[ 1024 ];
            int downloaded = 0;
            int readByte = 0;

            while ( ( readByte = bufferedInputStream.read(buffer, 0, 1024) ) >= 0 ) {
                bOutputStream.write(buffer, 0, readByte);
                downloaded += readByte;

                final int progress = downloaded;
                listeners.forEach(l -> {
                    l.onUpdateProgress( progress, fileSize);
                    l.onUpdateMessage(String.format("Download: %d / %d Byte ", progress, fileSize));
                });
            }

            bOutputStream.close();
            bufferedInputStream.close();
        } catch ( Exception e ) {
            listeners.forEach(l -> {
                l.onUpdateMessage("Beim Download ist ein Fehler aufgetreten");
                l.onFail(e);
            });
        } return outputFile;
    }


    private int getFileSize() {
        URLConnection conn = null;
        try {
            URL url = new URL(link);
            conn = url.openConnection();

            if ( conn instanceof HttpURLConnection ) {
                ( ( HttpURLConnection ) conn ).setRequestMethod("HEAD");
            }

            conn.getInputStream();

            return conn.getContentLength();
        } catch ( Exception e ) {
            listeners.forEach(l -> {
                l.onUpdateMessage("Fehler bei der Ermittlung der Dateigröße!");
                l.onFail(e);
            });
            return -1;
        } finally {
            if ( conn instanceof HttpURLConnection ) {
                ( ( HttpURLConnection ) conn ).disconnect();
            }
        }
    }

    @Override
    public void addDownloadListener( DownloadListener listener ) {
        this.listeners.add(listener);
    }

    @Override
    public void removeDownloadListener( DownloadListener listener ) {
        this.listeners.remove(listener);
    }
}

Spoiler

public class HttpDownloadTask extends Task<File> implements DownloadListener {

    private Download downloadImpl;

    public HttpDownloadTask( Download downloadImpl)  {
        this.downloadImpl = downloadImpl;
    }

    @Override
    protected File call() throws Exception {
        downloadImpl.addDownloadListener(this);
        return downloadImpl.execute();
    }

    @Override
    public void onUpdateProgress( int progress, int total ) {
        updateProgress(progress, total);
    }

    @Override
    public void onUpdateMessage( String message ) {
        updateMessage(message);
    }

    @Override
    public void onSucceed() {
        succeeded();
    }

    @Override
    public void doCancel() {
        cancel();
    }

    @Override
    public void onFail( Exception e ) {
        updateMessage(e.getMessage());
        failed();
    }
}

Und benutzen dann eben so

HttpDownloadTask downloadTask = new HttpDownloadTask(new HttpDownload("https://speed.hetzner.de/100MB.bin"));

 

[mihe7]

Das wird ja immer schöner

 

[Robat]

Quatsch das war doch nur hingeklatscht .. da geht bestimmt noch mehr!
Aber immerhin bin ich dann heute mal nicht Schuld! *_*

 

[mihe7]

Aber immerhin bin ich dann heute mal nicht Schuld! *_*

Die einen sagen so, die anderen so:

Hätte ich den Thread nicht nur überflogen, wüsste ich auch auf was du hinaus wolltest

Außerdem: der Tag ist noch lang...

 

[Robat]

Glaubst du etwa nicht an mich? Jetzt bin ich enttäuscht

 

[mihe7]

Glaubst du etwa nicht an mich?

Doch, doch - gerade deswegen

 

[ralfb1105]

Ich muss zugeben ich bin wie immer "platt" oder besser "überwältigt" was den o.g. Code angeht allerdings im absolut positiven Sinn, auch wenn ich jetzt Tage benötige um mir das im detail anzusehen, zu verstehen(!!) und dann bei mir zu implemntieren. Doch diese Zeit investiere ich sehr gerne denn es steckt so viel Know-How dort drin und das versuche ich gerne aufzunehmen - Vielen Dank Euch beiden für die Ausführungen.

Etwas Off-Topic, Ihr verwendet oben diese Spoiler um "lange" Code Passage einzuklappen, was mir sehr gut gefällt. Geht das über:
1. Einfügen Spoiler
2. Innerhalb der Spoiler Tags einfügen Code ?

Gruß

Ralf

 

[Robat]

Ihr verwendet oben diese Spoiler um "lange" Code Passage einzuklappen,[...]. Geht das über:

Ja genau darüber geht das. Falls du den Code über die Code Tags ([code=Java]...[/code] ) einfügen solltest kannst du es auch über die Spoiler-Tags machen .. das würde dann so aussehen: [SPOILER][code=Java]...[/code][/SPOILER].

Ansonsten:
Falls es Fragen geben sollte, frag einfach.

 

[mihe7]

Etwas Off-Topic, Ihr verwendet oben diese Spoiler um "lange" Code Passage einzuklappen, was mir sehr gut gefällt. Geht das über:

Ich mach das so, dass ich im Editor einfach folgendes eintippe:
[spoiler=Name des Spoilers]
[code=Java]
public class X {}
[/code]
[/spoiler]


EDIT: Mist, @Robat hat ja längst geantwortet - hatte die zweite Seite des Threads nicht gesehen....

 

[ralfb1105]

Ich mach das so, dass ich im Editor einfach folgendes eintippe:

Danke - auch wenn Robat etwas schneller war, das mit dem Namen des Spoiler kannte ich auch noch nicht somit, gut das Du geantwortet hast!

Gruß

Ralf

PS: Werde jetzt mal anfangen Euren Code im Detail zu lesen .. mal sehen ob ich ohne weitere Fragen druch komme

 

[ralfb1105]

Hallo @Robat
ich habe a) versucht Deine Lösung zu verstehen und b) diese zu implementieren.
Zu a) Ich habe wieder gemerkt das mir noch einiges an Verständnis zu Interfaces, Lambda Expression und deren Zusammenspiel fehlt. Es ist schwierig das hier als Frage nieder zu schreiben ... dazu müsste man sich vermutlich eher verbal austauschen. Daher bin ich, auch ohne alles im Detail zu verstehen zu b) übergegangen.

Wenn ich jetzt das Programm starte, die Felder ausfülle, wird der Download im Hintergrund ausgeführt, es werden allerdings keine Ausgaben gemacht und die ProgressBar wird auch nicht bedient. Dazu ist mir dann eingefallen das ich in meinem MainController vermutlich die Verbindung zu dem Listener schaffen muss, ansonsten kann da ja vermutlich nichst passieren Hier fehlt mir aber auf Grund von o.g. a) das Know-How.

In meinem MainController habe ich folgendes implementiert:

1. messageProperty().addListener für den ServiceTask registriert im Konstruktor:

serviceWorkerTask1.messageProperty().addListener((obs, oldMsg, newMsg) -> messages.setText(newMsg));

Info: messsages ist hier ein TextField, vermutlich würde ich das aber lieber in einem Label anzeigen.

2. Im serviceWorkerTask1 habe ich dann den neuen HttpDownloadTask gestartet:

@SuppressWarnings("rawtypes")
    Service serviceWorkerTask1 = new Service() {

        @Override
        protected Task createTask() {
            return new Task() {
                @Override
                protected Void call() throws Exception {
                    // Start entering execution code here ...
                   
                    File downloadFolderFile = new File(downloadFolderTextField.getText());           
                    // Implementation from @robat
                    HttpDownloadTask downloadTask = new HttpDownloadTask(new HttpDownloadWorker(sourceUrlTextField.getText(), downloadFolderFile, httpProxyCheckBox.isSelected(), proxyServerTextField.getText(), Integer.valueOf(proxyPortTextField.getText())));
                    downloadTask.run();

Wo und wie müsste ich mich denn jetzt an den DownloadListener "anstecken"?

Gruß

Ralf

 

[Robat]

Hallo Ralf,

was fehlt dir dann an Verständnis zu Lambdas / Interfaces? Ein Lambda-Ausdruck ist erstmal nichts anderes als "syntactic sugar" für eine anonyme innere Klasse. Sollte man eine anonyme innere Klasse eines Interfaces mit genau einer abstrakten Methode erstellen, so kann diese Schreibweise durch einen Lambda-Ausdruck verkürzt werden. Dabei muss lediglich die Parameterliste und der Methodenkörper der Methode angegeben werden. zB

public interface ActionListener {
    void actionPerformed(ActionEvent e);
}

JButton button = new JButton("Do something");
button.addActionListener(new ActionListener() {
    @Override
    public void actionPerformed(ActionEvent e) {
         System.out.println("Clicked");
    }
});

// oder mit Lambda-Ausdruck
button.addActionListener(e -> {
    System.out.println("Clicked");
});
// Da der Methodenkörper nur aus einer Zeile besteht, können die {} auch weggelassen werden
button.addActionListener(e -> System.out.println("Clicked"));

Dazu ist mir dann eingefallen das ich in meinem MainController vermutlich die Verbindung zu dem Listener schaffen muss, ansosnetn kann da ja vermutlich nichst passieren

Das ist vollkommen richtig. Diese "Verbindung" fehlt bei dir gerade noch. Wenn du dir mal das erste Beispiel anschaust, siehst du wie man diese Verbindung herstellst. Der Task besitzt intern Properties. Da ist zum einen die messageProperty und die progressProperty. Beides Properties werden im Task fleißig geupdatet. Jetzt kannst du deine GUI einfach an diese Properties binden.

    @Override
    public void initialize( URL location, ResourceBundle resources ) {
        downloadService = new Service<>() {
            @Override
            protected Task<File> createTask() {
                // ...
                downloadStatusLabel.textProperty().bind(downloadTask.messageProperty());
                downloadIndicator.progressProperty().bind(downloadTask.progressProperty());

                downloadTask.exceptionProperty().addListener((obs, oldVal, newVal) -> downloadStatusLabel.setText(newVal.getMessage()));

                downloadTask.setOnSucceeded(event -> downloadStartButton.setDisable(false));
                downloadTask.setOnCancelled(event -> downloadStartButton.setDisable(false));
                downloadTask.setOnFailed(event -> downloadStartButton.setDisable(false));

                return downloadTask;
            }
        };
    }

PS: Wie im Beispiel zu sehen gibt es auch noch eine exceptionProperty. Auf dieser wird ein Listener registriert und sobald sich an ihr etwas ändert wird die Nachricht der Exception ausgegeben. Ich glaub aber dass ich gerade nirgends im Code die exceptionProperty anpassen - von daher sind eigentlich nur die ersten beiden Zeilen und ggf die onSucceeded, onCacelled und onFailed Methoden wichtig für dich

Gruß Robert

 

[ralfb1105]

Hallo Robert,

Sorry ... das hatte ich gestern gelesen aber heute nach dem vielen neuen Code nicht mehr auf dem Schirm sollte nicht passieren.

Das mit dem

was fehlt dir dann an Verständnis zu Lambdas / Interfaces?

ist nicht so einfach zu beschreiben - vermutlich einfach die Erfahrung und die Übungen. Ich werde mal in Ruhe versuchen meine Fragen an Hand dieses Beispiels zu beschreiben, nur dazu brauche ich etwas Zeit damit es (hoffentlich) verständlich wird.

Nun habe ich, so wie von Dir aufgezeigt in der Initialize() Methode den downloadService erstellt und diesen dann in der Methode startDownloadButtonTapped() aufgerufen:

public void startDownloadButtonTapped() {
        // Clear messages TextArea/Labels before starting ...
        messages.setText("");
        message1Label.setText("");
        message2Label.setText("");
        downloadFolderTextField.setDisable(true);
        sourceUrlTextField.setDisable(true);
        if (httpProxyCheckBox.isSelected()) {
            proxyServerTextField.setDisable(true);
            proxyServerTextField.setOpacity(0.25);
            proxyPortTextField.setDisable(true);
            proxyPortTextField.setOpacity(0.25);
        }

        toggleStartStopDownloadButton.setFadeTransitionShowButtonOne(false);
        downloadService.start();
}

Wenn ich das Programm starte, die Felder fülle und auf Strt drücke wird der Download gestartet, und die Ausgabe Bytes von Bytes erscheint und die ProgressBar wird auch gefüllt.

Frage:

Was müsste denn jetzt in die Methode stopDownloadButtonTapped() um den Download während des laufens zu stoppen?
Ich habe da folgendes rein geschrieben, aber das funktioniert nicht:

public void stopDownloadButtonTapped() {
        downloadService.cancel();
        // serviceWorkerTask1.cancel();
        LOGGER.info("Stop button pressed. Stop downloading service task now!");
    }

Was auch nicht funktioniert und zu einer Exception führt, wenn der Download beendet wurde und ich erneut, ohne etwas zu verändern auf Start drücke:

Caused by: java.lang.RuntimeException: A bound value cannot be set.
    at javafx.scene.control.TextInputControl$TextProperty.set(TextInputControl.java:1321)
    at javafx.scene.control.TextInputControl.setText(TextInputControl.java:349)
    at de.ralfb_web.ui.MainController.startDownloadButtonTapped(MainController.java:293)

Frage am Rande: Wie kann ich ich denn in der Lambda Expression hinter dem "->" mehr als eine Codezeile ausführen?

Ich werde mir das Morgen dann auch noch einmal in Ruhe anschauen ... vielleicht sehe ich ja meinen "Bock"

Gruß

Ralf

 

[ralfb1105]

Frage am Rande: Wie kann ich ich denn in der Lambda Expression hinter dem "->" mehr als eine Codezeile ausführen?

Bitte vergiss die Frage, hast Du mir ja schon oben beantwortet, ich bin heute zu schnell mit den Fragen ... nochmal Sorry ...

button.addActionListener(e -> { System.out.println("Clicked");

Gruß

Ralf

 

[Robat]

Was müsste denn jetzt in die Methode stopDownloadButtonTapped() um den Download während des laufens zu stoppen?
Ich habe da folgendes rein geschrieben, aber das funktioniert nicht:

Dafür fügen wir dem Interface Download eine void cancel() Methode hinzu. In HttpDownload implementieren wir diese Methode und setzen dort einfach einen boolean auf true. Beim Download selber ändern wir die Abbruchbedingung der Schleife wie folgt und informieren am Ende - bei Abbruch - die Listener

while ( ( readByte = bufferedInputStream.read(buffer, 0, 1024) ) >= 0 && !cancel ) {
   ... 
}
...
if(cancel) {
   listeners.forEach(DownloadListener::doCancel);
}

Ein Task hat eine Methode cancelled() welche aufgerufen wird, wenn ein Task über Service#cancel gestoppt wird. Wenn wir jetzt in HttpDownloadTask diese Methode überschreiben, können wir dort mittels Download#cancel den Download stoppen.

/* Service Methode => Aufruf wenn service.cancel()*/
@Override
protected void cancelled() {
    downloadImpl.cancel();
}

/* DownloadListener Methode => Aufruf wenn Listener über DownloadListener::doCancel informiert wird */
@Override
public void doCancel() {
    // do smth
}

 

[Robat]

Was auch nicht funktioniert und zu einer Exception führt, wenn der Download beendet wurde und ich erneut, ohne etwas zu verändern auf Start drücke

Sobald eine Property gebunden ist darfst du nicht mehr die setText() Methode aufrufen .. der Wert soll ja von einer anderen Property kommen.
Setz einfach den Wert der messageProperty des Service auf einen leeren String.

 

[Robat]

Bitte vergiss die Frage, hast Du mir ja schon oben beantwortet, ich bin heute zu schnell mit den Fragen ... nochmal Sorry ...

Mach dir kein Stress deswegen. Der Tag ist schon lang und Fehler passieren

 

[ralfb1105]

Sobald eine Property gebunden ist darfst du nicht mehr die setText() Methode aufrufen .. der Wert soll ja von einer anderen Property kommen.
Setz einfach den Wert der messageProperty des Service auf einen leeren String.

Ok, verstehe - probiere ich Morgen gleich aus. Wieder was dazu gelernt

 

[ralfb1105]

Bei dem Cancel habe ich wohl zu einfach gedacht ... rein vom lesen habe ich es soweit verstanden und werde versuchen es Morgen zu implementieren. Melde mich mit dem Ergebnis.

Schönen Abend noch.

Gruß

Ralf

 

[Robat]

Wir haben ja momentan 3 Schichten

Gui <----> Service <----> Download

Das cancel welches du bereits aufgerufen hast stellt nur die Verbindung zwischen GUI und Service her. Dieser Befehl muss aber noch bis an den Download ran kommen. Daher die von mir vorgeschlagene Veränderung

Meld dich wenns Probleme gibt

 

[ralfb1105]

Ah, ich habe den HttpDownloadTask beendet, nicht aber den HTTP Download, der ja vom HttpDownloadTask gestartet wurde. Jetzt ist es noch etwas klarer - Danke.

 

[ralfb1105]

Sobald eine Property gebunden ist darfst du nicht mehr die setText() Methode aufrufen .. der Wert soll ja von einer anderen Property kommen.
Setz einfach den Wert der messageProperty des Service auf einen leeren String.

Hallo Robert,

ich bin gerade dabei die vielen wertvollen Infos zu verstehen und zu verarbeiten. Den Grund für die Excpetion hast Du richtig erkannt, nachdem ich "messages.setText("")" nicht mehr ausführe gibt es beim erneuten Start auch keine Exception mehr.

Im Moment bin ich daher bei der Frage wo genau ich denn den Wert der messageproperty des Service setzen kann/muss?

Ich habe das mal im HttpDownload in der "File execute()" Methode als erstes gemacht:

listeners.forEach(l -> l.onUpdateMessage(""));

Das bringt zumindest keine Excpetion, ich bin mir aber nicht sicher ob das die richtige Stelle ist.

Das bringt mich auch zur 2. Frage:

Mit dem Aufruf

messages.textProperty().bind(downloadTask.messageProperty());

stelle ich ja die Verbindung vom GUI zum HttpDownload her, der die messages über den Listener (l.onUpdateMessage()) beschreibt.
Wenn ich nun z.B. bei Fehler mehr als eine Zeile habe, sehe ich immer nur die letzte. Wie könnte ich denn bei Fehlern alle Zeilen im TextField anzeigen?

Ich habe mal folgende Änderung im HttpDownloadTask gemacht:

@Override
    public void onFail(Exception ex) {
        updateMessage(ex.getMessage() + "\n");
        failed();
    }

Das bringt aber nichts, es sieht so aus als wenn kein "append" gemacht wird !??

Gruß

Ralf

 

[ralfb1105]

Hallo Robat,

nach einigen Versuchen habe ich jetzt, so glaube ich zumindest, auch die "Stop" Funktion sauber implemntiert.


Die größte Herausforderung für mich waren, wie bereits geschrieben, die verschiedenen Schichten, sprich wer was an wen weitergibt und wo was implementiert ist/werden muss. Aber, diese Aufgabe übt und bringt etwas mehr Verständnis für die, nein besser, DeineLösung

Gruß

Ralf

 

[Robat]

Grüß dich Ralf,

schön, dass du so fleißig dabei bist

Das bringt zumindest keine Excpetion, ich bin mir aber nicht sicher ob das die richtige Stelle ist.

Die Stelle ist zumindest erstmal nicht falsch.

Wenn ich nun z.B. bei Fehler mehr als eine Zeile habe, sehe ich immer nur die letzte. Wie könnte ich denn bei Fehlern alle Zeilen im TextField anzeigen?

Kannst du das etwas genauer beschreiben? Wenn ein Fehler auftritt sollten theoretisch keine Fehler mehr passieren, weil der Download ja dann beendet wird (so sollte es zumindest sein). Oder meinst du, wenn es mehrere Statusänderungen gibt, die angezeigt werden sollen. Sprich sowas wie:

Download gestartet..
Ermittle Dateigröße..
Fehler beim Download .. die angegebene URL konnte nicht gefunden werden

 

[ralfb1105]

Es geht mir hier um die Situation wenn ein Fehler aufgetreten ist. Dann würde ich eine Meldung UND die Exception ausgeben oder falls ggf. eine Exception mehrere Zeilen hat. Hatte gerade das Verhalten wenn ich ein File in der URL angebe das es nicht gibt das ich dann kurz eine Meldung sehe und dann den die URL die es nicht gibt ... ??

 

[ralfb1105]

Hallo Robert,

habe es jetzt noch einmal probiert um es etwas genauer zu beschreiben. Folgender Testfall:


Die URL hat einen ungültigen, sprich nicht vorhandenen Filenamen -> 100Mb.binary anstatt 100MB.bin.
Wenn ich das jetzt starte kommt die Meldung "Determine FileSize" und dann wie Du oben siehst die URL.

Ich habe mir das mal auf der Console ausgeben lassen, und da kommt folgendes an Ausgaben:

Error occured during download!
java.io.FileNotFoundException: https://speed.hetzner.de/100MB.binary
at sun.net.www.protocol.http.HttpURLConnection.getInputStream0(Unknown Source)
at sun.net.www.protocol.http.HttpURLConnection.getInputStream(Unknown Source)
...
...

Nun würde ich gerne folgende Meldungen, die mir wichtig erscheine, im TextField sehen:

Error occured during download!
java.io.FileNotFoundException:

https://speed.hetzner.de/100MB.binary

Ich hoffe jetzt ist es etwas klarer ... war eben mit dem Handy unterwegs, konnte daher nicht so viel schreiben.

Gruß

Ralf

 

[Robat]

Hallo Ralf, sorry für die späte Antwort. Hatte gestern noch viel zu tun

Jetzt ist soweit klar was du haben wolltest und das lässt sich auch recht einfach umsetzen. Du hast schon vollkommen richtig erkannt, dass momentan der Text im Label immer überschrieben wird. Das Anfügen können wir aber erreichen, indem wir uns vorher einfach den alten Text im Label holen. Folgendes Beispiel:
Du hast folgenden catch-Block in der (zB) getFileSize() Methode.

} catch ( Exception e ) {
    listeners.forEach(l -> {
        l.onUpdateMessage("Ermitteln der Dategröße fehlgeschlagen");
        l.onFail(e);
    });
    return -1;
}

Die onFail() Methode müsste jetzt wie oben beschrieben abgeändert werden. Im ersten Part wird die erste Zeile aus dem Stacktrace extrahiert. Im zweiten Block wird die Nachricht an die alte angefügt.

@Override
public void onFail( Exception e ) {
    StringWriter writer = new StringWriter();
    e.printStackTrace(new PrintWriter(writer));
    String cause = writer.toString().split("\n")[0];

    Platform.runLater(() -> updateMessage(getMessage() + "\n" + cause));
    failed();
}

An der Stelle vielleicht ein kleiner Hinweis. Mir sind 2 kleinere Fehler in meinen Beispielen aufgefallen:
- ein Task sollte generell nur aus dem FXThread aktualisiert werden... so solltest du updateMessage() und getMessage() immer nur in Platform.runLater() aufrufen. Bei updateProgress ist das nicht zwingend notwendig, da diese Prüfung intern stattfindet und im Notfall das ganze in den FX-Thread ausgelagert wird.

- Nachdem du die getFileSize() Methode aufgerufen hast sollte natürlich geprüft werden, ob eine gültige Dateigröße zurückgegeben wurde. Wenn die Methode -1 zurückgibt sollte dein Code natürlich entsprechend abbrechen (mit einem return).

 

[ralfb1105]

Hallo Robert,

ich habe dann mal probiert Deine Vorschläge zu verstehen und zu implementieren. Hier die Ergebnisse, und am Ende auch noch eine Frage

Spoiler: HttpDownloadTask

@Override
    public void onFail(Exception ex) {
        // Make sure we will not overwrite messages that are useful in case of trouble.
        StringWriter writer = new StringWriter();
        ex.printStackTrace(new PrintWriter(writer));
        String cause = writer.toString().split("\n")[0];
        Platform.runLater(() -> updateMessage(getMessage() + "\n" + cause));
        failed();
    }

Das hat auch super funktioniert, wie man an dem Screenshot sehen kann:



... dachte ich zumindest im ersten Moment, aber wenn man genau hinsieht ist die Meldung die Falsche, denn ich sollte ja schon beim ermitteln der Filesize stolpern. Darum, wie von Dir richtig agemerkt, muss ich noch den Code so umbauen das ich darauf reagiere wenn bei getFileSize() ein -1 zurückgeliefert wird.

Das habe ich nun wie folgt ausgeführt:

Spoiler: HttpDownload

public File execute() {
        File outputFile = null;
        String saveFileName = null;
        try {
            URL url = new URL(link);
            if (useProxy) {
                hConnection = (HttpURLConnection) url.openConnection(proxy);
            } else {
                hConnection = (HttpURLConnection) url.openConnection();
            }
            listeners.forEach(l -> l.onUpdateMessage(""));

            // Try to determine the filesize
            listeners.forEach(l -> l.onUpdateMessage("Determine Filesize ..."));

            int fileSize = getFileSize();
            if (fileSize != -1) {
                listeners.forEach(l -> {
                    l.onUpdateProgress(0, fileSize);
                    l.onUpdateMessage(String.format("Download: %d / %d Byte ", 0, fileSize));
                });
                BufferedInputStream bufferedInputStream = new BufferedInputStream(hConnection.getInputStream());

                saveFileName = link.substring(link.lastIndexOf('/') + 1);
                outputFile = new File(downloadFolderFile, saveFileName);
                OutputStream outputStream = new FileOutputStream(outputFile);
                BufferedOutputStream bOutputStream = new BufferedOutputStream(outputStream, 1024);

                byte[] buffer = new byte[1024];
                int downloaded = 0;
                int readByte = 0;

                while ((readByte = bufferedInputStream.read(buffer, 0, 1024)) >= 0 && !isCanceled) {
                    bOutputStream.write(buffer, 0, readByte);
                    downloaded += readByte;

                    final int progress = downloaded;
                    listeners.forEach(l -> {
                        l.onUpdateProgress(progress, fileSize);
                        l.onUpdateMessage(String.format("Download: %d / %d Byte ", progress, fileSize));
                    });
                }

                bOutputStream.close();
                bufferedInputStream.close();

                if (isCanceled) {
                    listeners.forEach(l -> l.onUpdateMessage("Download cancelled by User request."));
                    listeners.forEach(DownloadListener::doCancel);
                } 
            } else {
                listeners.forEach(DownloadListener::doCancel);
            }

        } catch (Exception ex) {
            listeners.forEach(l -> {
                l.onUpdateMessage("Error occured during download!");
                l.onUpdateProgress(0, 0);
                l.onFail(ex);
            });
        }
        return outputFile;
    }
private int getFileSize() {
        URLConnection conn = null;
        try {
            URL url = new URL(link);
            if (useProxy) {
                conn = url.openConnection(proxy);
            } else {
                conn = url.openConnection();
            }

            if (conn instanceof HttpURLConnection) {
                ((HttpURLConnection) conn).setRequestMethod("HEAD");
            }

            conn.getInputStream();

            return conn.getContentLength();
        } catch (Exception ex) {
            listeners.forEach(l -> {
                l.onUpdateMessage("Error during determing filesize!");
                l.onUpdateProgress(0, 0);
                l.onFail(ex);
            });
            return -1;
        } finally {
            if (conn instanceof HttpURLConnection) {
                ((HttpURLConnection) conn).disconnect();
            }
        }
    }

Das funktioniert auch im Prinzip:


Wenn ich im Pronzip sagen weißt Du bestimmt was jetzt kommt ... aber ..
Frage:

Ich habe noch meiner Meinung die Problematik das für den MainController die Ausführung "succeeded" ist, das sehe ich im Log, welches ich ja im MainController schreibe:

[Mi Jul 17 14:22:26 MESZ 2019 de.ralfb_web.ui.MainController$6 lambda$0]
INFORMATION: Download Manager Task Succeeded!

So wie ich das sehe liegt das daran das im HttpDownload ja keine Exception im File execute() geschmissen wird wenn getFilsize() mit -1 zurück kommt. Ich habe dann, wie Du oben auch schon siehst, im else-Zweig folgendes geschrieben:

if (fileSize != -1) {
.... 
....
} else {
                listeners.forEach(DownloadListener::doCancel);
}

Das führt dan im Log zu einem

[Mi Jul 17 14:49:46 MESZ 2019 de.ralfb_web.ui.MainController$6 lambda$1]
INFORMATION: Download Manager Task Cancelled!

Richtig wäre natürlich ein Failed. Jetzt muss ich aber mal wieder zugeben das ich nicht ganz genau weiß wie ich das bewerkstelligen könnte
Im getFileSize() geht er in den catch-Zweig und dort wird ja auch der Listener über das problem informiert - so weit verstanden und soweit auch gut. Ich gehe mit einer -1 raus, so das ich im execute() darauf reagieren kann. Hier würde ich jetzt dann noch

listeners.forEach(l ->l.onFail(ex));

ausführen, aber mir fehlt hier die Exception

Mache ich hier generell einen Gedankenfehlr und bin auf dem Holzweg was die Schichten angeht, oder bin ich prinzipiell richtig unterwegs und mir fehlt nur noch die Benachrichtigung des Task das ein Fehler aufgetreten ist?

Gruß

Ralf

 

[Robat]

Hier die Ergebnisse, und am Ende auch noch eine Frage

Meinst du mit "Frage" den Teil am Ende wo es um die Prüfung des Rückgabewertes von getFileSize() geht? Ist dort alles klar?

 

[looparda]

Ich habe mir @Robat's Ansatz angeschaut und ausprobiert. Mir ist folgendes aufgefallen:
Ich würde die View an die Properties des Service (anstatt des Tasks) binden. Die Properties werden je nach State des Service sinnvoll intern aktualisiert. Damit sparst du dir Arbeit die Properties eigenhändig mit dem State nachzuziehen. Oder wurde das bewusst gemacht?

Sorry, falls ich forenweit mit dem ViewModel-Konzept nerve. Aber auch hier sehe Ich Potential für bessere Trennung, falls du noch weiter separieren möchtest: Ich würde im Controller ausschließlich Bindings herstellen sowie Actions delegieren und keinerlei Logik einbauen. Auch der Service gehört nicht in den Controller. Wenn du diese Trennung machst kannst du deine komplette Logik ohne Start der GUI testen.
Der Controller wird sehr einfach:

public class DownloadController implements Initializable {

    @FXML
    private Text downloadStatusLabel;
    @FXML
    private ProgressIndicator downloadIndicator;
    @FXML
    private Button downloadStartButton;
    @FXML
    private Button downloadStopButton;

    private DownloadViewModel viewModel;

    @Override
    public void initialize(URL location, ResourceBundle resources) {
        viewModel = new DownloadViewModel();

        downloadIndicator.progressProperty().bind(viewModel.progressProperty());
        downloadStatusLabel.textProperty().bind(viewModel.downloadStatusLabelProperty());
        downloadStartButton.disableProperty().bind(viewModel.buttonsDisabledProperty());
        downloadStopButton.disableProperty().bind(viewModel.buttonsDisabledProperty());
    }

    @FXML
    public void startDownloadButtonClicked() {
        viewModel.getStartCommand().execute();
    }

    @FXML
    public void stopDownloadButtonClicked() {
        viewModel.getStopCommand().execute();
    }
}

Das ViewModell konzentriert die Logik:

interface StartCommand {
    void execute();
}

interface StopCommand {
    void execute();
}

class DownloadViewModel {

    private final StringProperty downloadStatusLabel = new SimpleStringProperty();
    private final DoubleProperty progressProperty = new SimpleDoubleProperty();
    private final BooleanProperty successProperty = new SimpleBooleanProperty();
    private final BooleanProperty cancelProperty = new SimpleBooleanProperty();
    private final BooleanProperty failedProperty = new SimpleBooleanProperty();

    private final Service<File> downloadService;

    DownloadViewModel() {
        this.downloadService = new Service<File>() {
            @Override
            protected Task<File> createTask() {
                final HttpDownloadTask downloadTask = new HttpDownloadTask(new HttpDownload("https://speed.hetzner.de/100MB.bin"));
                downloadTask.exceptionProperty().addListener((obs, oldVal, newVal) -> downloadStatusLabel.set(newVal.getMessage()));
                return downloadTask;
            }
        };

        downloadStatusLabel.bind(downloadService.messageProperty());
        progressProperty.bind(downloadService.progressProperty());

        downloadService.setOnSucceeded(event -> successProperty.set(true));
        downloadService.setOnCancelled(event -> cancelProperty.set(true));
        downloadService.setOnFailed(event -> failedProperty.set(true));
    }

    StartCommand getStartCommand() {
        return () -> downloadService.start();
    }

    StopCommand getStopCommand() {
        return () -> {
            downloadService.cancel();
            downloadService.reset();
        };
    }

    ReadOnlyStringProperty downloadStatusLabelProperty() {
        return downloadStatusLabel;
    }

    ReadOnlyDoubleProperty progressProperty() {
        return progressProperty;
    }

    BooleanBinding buttonsDisabledProperty() {
        return Bindings.and(successProperty, Bindings.not(failedProperty)).or(cancelProperty);
    }
}

Das Konstrukt mit den Commands ist nicht ganz ausgereift aber es soll verdeutlichen, dass die Logik der Actions im ViewModel sitzt und nur angestoßen wird.
Ich weiß nicht ob ich das gesamte Verhalten korrekt ins ViewModel extrahiert habe, aber die Logik hinter der buttonsDisabledProperty ist murks und soll nur die Flexibilität/Reaktivität des ViewModells verdeutlichen.

 

[ralfb1105]

Meinst du mit "Frage" den Teil am Ende wo es um die Prüfung des Rückgabewertes von getFileSize() geht? Ist dort alles klar?

Hallo Robert,

ich hatte zwischendurch einmal gespeichert weil ich Probleme mit dem Interface beim einfügen von ScreenShots hatte, aus diesem Grunde könnte es sein das Du zwischenzeitlich nicht meinen ganzen Post gesehen hast ...

Ich glaube, oder hoffe zumindest, das die getFileSize() Methode mit dem -1 so OK ist. Die Problemtik habe ich m.E. damit das ich ja auch aus der execute() Methode mit einem Fehler raus muss wenn die darin aufgerufene getFileSize() Methode mit -1 zurück kommt. An dieser Stelle fällt mir nur

listeners.forEach(l ->l.onFail(ex));

ein, aber wie oben geschrieben habe ich ja dort keine Exception mehr .. ?? Wie gesagt, vielleicht bin ich ja auch voll daneben ...

Gruß

Ralf

 

[Robat]

Du hattest Recht. Zu dem Zeitpunkt, wo ich deinen Beitrag gelesen hatte, war tatsächlich noch nicht der gesamte Beitrag da. Daher die vielleicht etwas verwirrende Nachfrage.
Das mit den Exceptions und dem fail-Status ist bei dem Task etwas tricky wie ich festgestellt habe (vielleicht habe ich auch einfach noch den richtigen Weg gefunden?)
Du kannst den Status des Task auf "FAILED" setzen, indem du in der onFail(Exception) Methode deines HttpDownloadTasks einfach die Exception noch mal als RuntimeException wirfst. Im Code würde das heißen:

@Override
public void onFail( Exception e ) {
    StringWriter writer = new StringWriter();
    e.printStackTrace(new PrintWriter(writer));
    String cause = writer.toString().split("\n")[0];

    Platform.runLater(() -> updateMessage(getMessage() + "\n" + cause));
    throw new RuntimeException(e);
}

Damit würde im Controller der Task nicht mehr als erfolgreich sondern eben als fehlgeschlagen deklariert werden.
Das mit dem fileSize würde ich wie folgt ändern:

int fileSize = ...
if(fileSize == -1) {
    return null;
}
...

 

[ralfb1105]

Hallo Robert,

Das mit dem fileSize würde ich wie folgt ändern:

Danke für den Tipp, das sind die kleinen Dinge die ich noch lernen muss habe ich schon mal gelesen aber dann fällt es einem an der Stelle nicht ein .. üben, üben und nochmal üben!

Das mit

throw new RuntimeException(ex);

hat genau den gewünschten Effekt:

INFORMATION: Start downloading service task ... 
[Mi Jul 17 17:18:43 MESZ 2019 de.ralfb_web.ui.MainController$5 lambda$2]
INFORMATION: Download Manager Task Failed!

Jetzt werde ich mich mal an das letzte Thema: Throughput machen. Die Daten dafür sollte ich ja im HttpDownload vorliegen haben.

Danke nochmal für Deine unermütliche Unterstützung.

Gruß

Ralf

 

[Robat]

Du solltest den Kommentar von @looparda nicht vernachlässigen. Schau dir das ViewModel-Konzept bei Gelegenheit auf jeden Fall mal an. Ich persönlich finde es hier nicht notwendig, weil nur das Ziel war, die Download-Logik möglichst unabhängig zu gestallten. Aber da gibt es unterschiedliche Meinungen zu. Generell gibt es , gerade bei FX, einige Stellen wo das ViewModel-Konzept sehr gut und nützlich ist. Stichwort hier wäre zB die De-/Serialisierung von Modellen .. wenn man die Modelle direkt mit Properties vollpackt dann wird das etwas umständlich.

Wenn du diese Trennung machst kannst du deine komplette Logik ohne Start der GUI testen.

Hast du das mal ausprobiert? Würde mich mal interessieren ob das geht. Hintergrund der Frage ist, dass zB die updateMessage() Methode eines Tasks nur in einem FX-Thread aufgerufen werden sollte.

 

[ralfb1105]

Du solltest den Kommentar von @looparda nicht vernachlässigen.

Nein, auf keinen Fall - halte jeden Kommentar für sehr hilfreich. Ich hatte bisher noch nicht geantwortet um mich auf die Lösung der o.g. Punkte zu konzentrieren. Ich kenne mich - bin ein Mann und in meinem Alter ist Multitasking echt nicht mehr hilfreich

Ich habe mir auf jeden Fall vorgenommen das mal in Ruhe anzuschauen und auch mal zu testen, im Moment ist die Zeit leider etwas knapp und sow ie ich das beim überfliegen gesehen habe werde ich da schon einiges reinstecken müssen um es zu verstehen.

Gruß

Ralf

 

[ralfb1105]

Du solltest den Kommentar von @looparda nicht vernachlässigen.

Nein, auf keinen Fall - halte jeden Kommentar für sehr hilfreich. Ich hatte bisher noch nicht geantwortet um mich auf die Lösung der o.g. Punkte zu konzentrieren. Ich kenne mich - bin ein Mann und in meinem Alter ist Multitasking echt nicht mehr hilfreich

Ich habe mir auf jeden Fall vorgenommen das mal in Ruhe anzuschauen und auch mal zu testen, im Moment ist die Zeit leider etwas knapp und sow ie ich das beim überfliegen gesehen habe werde ich da schon einiges reinstecken müssen um es zu verstehen.

Gruß

Ralf

 

[ralfb1105]

@looparda - vielen Dank für Deinen Kommentar und den neuen Ansatz wie man so ein Thema angehen könnte. Wie Du vielleicht dem Thread entnommen hast stehe ich bei Java und JavaFX noch am Anfang und benötige immer etwas mehr Zeit um das zu verstehen und dann ggf. noch zu testen/umzusetzen. Wie geschrieben werde ich es mir aber noch anschauen und hoffentlich die Zeit finden es einmal zu testen. Generell bin ich ein großer Fan von Trennungen und habe hier im Forum dazu auch schon einiges gelernt und gelesen - wie es scheint gibt es da noch großes Potential.

Gruß

Ralf