JavaFX HTTP Download task im Hintergrund innerhalb GUI

Bitte aktiviere JavaScript!
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 ;)

Java:
@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:

12222

... 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:

Java:
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:
12223

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:

Code:
[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:

Java:
if (fileSize != -1) {
.... 
....
} else {
                listeners.forEach(DownloadListener::doCancel);
}
Das führt dan im Log zu einem
Code:
[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 o_O
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

Java:
listeners.forEach(l ->l.onFail(ex));
ausführen, aber mir fehlt hier die Exception :eek:

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
 
Zuletzt bearbeitet:
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:
Java:
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:
Java:
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.
 
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
Java:
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
 
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:
Java:
@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:
Java:
int fileSize = ...
if(fileSize == -1) {
    return null;
}
...
 
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 :p habe ich schon mal gelesen aber dann fällt es einem an der Stelle nicht ein .. üben, üben und nochmal üben!

Das mit

Java:
throw new RuntimeException(ex);
hat genau den gewünschten Effekt:
Code:
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
 
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.
 
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 :p

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
 
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 :p

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
 
@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
 
Zunächst würde mich noch interessieren:
Ich würde die View an die Properties des Service (anstatt des Tasks) binden. [...] Oder wurde das bewusst gemacht?
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.
Wenn ich mein ViewModel teste, dann würde ich den Service mocken und könnte mir solche Probleme fern halten.

Wenn ich den Service testen möchte, dann müsste ich den JavaFX Application Thread haben, weil Service an sich sowhol als auch Platform.runLater diesen voraussetzen. So wie hier beschrieben konnte ich einen Service testen. http://blog.buildpath.de/how-to-test-javafx-services/

Wie Du vielleicht dem Thread entnommen hast stehe ich bei Java und JavaFX noch am Anfang
Man kann auch nicht alles auf einmal machen. Man muss es oft erst "naiv" machen, um an die Grenzen zu stoßen, damit man die Vorteile eines anderen Ansatzes sieht und versteht.
 
Hallo zusammen,

ich habe jetzt noch das letzte Feature "Anzeige Mbps" eingebaut:
Java:
// Variables needed by calculating throughput
            long start = System.nanoTime();
            final double NANOS_PER_SECOND = 1000000000.0;
            final double BYTES_PER_MB = 1024 * 1024;
            double speedInMBps = 0;
            double speedInMbps = 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));
                });
            }
            LOGGER.info(String.format("Download: %d / %d Byte ", downloaded, fileSize));
            speedInMBps = NANOS_PER_SECOND / BYTES_PER_MB * downloaded / (System.nanoTime() - start);
            speedInMbps = speedInMBps * 8;
            NumberFormat nfFormat = NumberFormat.getInstance();
            nfFormat.setMaximumFractionDigits(2);
            model.setDownloadMbps(String.valueOf(nfFormat.format(speedInMbps)));
            LOGGER.info("Throughput Mbps: " + String.valueOf(nfFormat.format(speedInMbps)));
12237

Ich habe ein paar Infos von anderen Beiträgen verwendet, es macht das was es soll, ob es nun 100% richtig ist kann ich nicht genau sagen da ich keine umfangreichen Tests gemacht habe :p

Da mir dieser Threat mit den vielen hilfreichen Beiträgen, Ideen und Diskussionen sehr viel an Neues und vor allem Wissen zu JavaFX, Listener und der Trennung zwischen GUI, Service und Worker gebracht hat, habe ich mir gedacht das es vielleicht ein ganz gutes Beispiel ist was auch anderen helfen könnte. Aus diesem Grunde habe ich das Projekt auf Github gestellt:
Github DownloadManagerFX

Danke an dieser Stelle auch noch einmal an @Robat , @mihe7 und @looparda für die Beiträge und Ideen!

Gruß

Ralf
 
Ich gebe Dir mal einen Tipp damit bei kleiner buf Größe Deine GUI nicht einfriert:
Java:
long count = 0;

//...

try (/*...*/) {
	int b;
	while ((b = in.read()) != -1) {
		out.write(b);
		count++;
		if (count % (1000 * 1000) == 0) {
			int value = (int) (count / (1000 * 1000));
			x.setValue(value);
		}
	}
}

Ist die Größe bekannt, kannst Du x ja einstellen...
Die Durchschnittswerte errechnen sich aus System.currentTimeMillis, nanoTime herzunehmen ist an dieser Stelle natürlich Schwachsinn.
 
Ich stehe ehrlich gesagt gerade etwas auf dem Schlauch o_O ich habe jetzt noch nicht so viel getestet, bin auch gerade nur am Tablet, aber ein GUI einfrieren habe ich bis jetzt noch nicht beobachtet. Weiterhin fehlt mir im Moment noch der Hinweis wo genau ich das einbauen sollte. @Tobias-nrw Könntest Du mir bezogen auf den Code noch ein paar mehr Hinweise geben wo genau diese Verbesserung einzusetzen wäre und wann aus Deiner Sicht das GUI einfrieren könnte?

Gruß

Ralf
 
Zuletzt bearbeitet:
Die Durchschnittswerte errechnen sich aus System.currentTimeMillis, nanoTime herzunehmen ist an dieser Stelle natürlich Schwachsinn.
Ich habe das aus folgendem Thread und war der Meinung das es für meine Zwecke zu gebrauchen wäre, lasse mich natürlich gerne eines besseren belehren, wobei ich den Ausdruck „Schwachsinn“ schon etwas hart finde ...

https://stackoverflow.com/questions/351565/system-currenttimemillis-vs-system-nanotime

Gruß

Ralf
 
Passende Stellenanzeigen aus deiner Region:

Neue Themen

Oben