#143 #44 #42 #22: Fixed some download problems with invalid directories or filenames. Added user Feedback. Different settings for audio and video download dir.

app/src/main/java/org/schabi/newpipe/DownloadDialog.java

@@ -2,6 +2,7 @@ package org.schabi.newpipe;
 
 import android.app.Dialog;
 import android.app.DownloadManager;
+import android.app.Notification;
 import android.content.Context;
 import android.content.DialogInterface;
 import android.content.SharedPreferences;
@@ -13,8 +14,11 @@ import android.support.annotation.NonNull;
 import android.support.v4.app.DialogFragment;
 import android.support.v7.app.AlertDialog;
 import android.util.Log;
+import android.widget.Toast;
 
 import java.io.File;
+import java.util.ArrayList;
+import java.util.List;
 
 /**
  * Created by Christian Schabesberger on 21.09.15.
@@ -61,53 +65,83 @@ public class DownloadDialog extends DialogFragment {
                         String suffix = "";
                         String title = arguments.getString(TITLE);
                         String url = "";
-                        String downloadFolder = Environment.DIRECTORY_DOWNLOADS;
+                        File downloadDir = NewPipeSettings.getDownloadFolder();
                         switch(which) {
                             case 0:     // Video
                                 suffix = arguments.getString(FILE_SUFFIX_VIDEO);
                                 url = arguments.getString(VIDEO_URL);
-                                downloadFolder = Environment.DIRECTORY_MOVIES;
+                                downloadDir = NewPipeSettings.getVideoDownloadFolder(context);
                                 break;
                             case 1:
                                 suffix = arguments.getString(FILE_SUFFIX_AUDIO);
                                 url = arguments.getString(AUDIO_URL);
-                                downloadFolder = Environment.DIRECTORY_MUSIC;
+                                downloadDir = NewPipeSettings.getAudioDownloadFolder(context);
                                 break;
                             default:
                                 Log.d(TAG, "lolz");
                         }
-                        //to avoid hard-coded string like "/storage/emulated/0/Movies"
+                        if(!downloadDir.exists()) {
-                        String downloadPath = prefs.getString(getString(R.string.download_path_key),
-                                Environment.getExternalStorageDirectory().getAbsolutePath() + "/" + downloadFolder);
-                        final File dir = new File(downloadPath);
-                        if(!dir.exists()) {
                             //attempt to create directory
-                            boolean mkdir = dir.mkdir();
+                            boolean mkdir = downloadDir.mkdirs();
-                            if(!mkdir && !dir.isDirectory()) {
+                            if(!mkdir && !downloadDir.isDirectory()) {
-                                Log.e(TAG, "Cant' create directory named " + dir.toString());
+                                String message = context.getString(R.string.err_dir_create,downloadDir.toString());
-                                //TODO notify user "download directory should be changed" ?
+                                Log.e(TAG, message);
+                                Toast.makeText(context,message , Toast.LENGTH_LONG).show();
+
+                                return;
                             }
+                            String message = context.getString(R.string.info_dir_created,downloadDir.toString());
+                            Log.e(TAG, message);
+                            Toast.makeText(context,message , Toast.LENGTH_LONG).show();
                         }
 
-                        String saveFilePath = dir + "/" + title + suffix;
+                        File saveFilePath = new File(downloadDir,createFileName(title) + suffix);
+
+                        long id = 0;
                         if (App.isUsingTor()) {
                             // if using Tor, do not use DownloadManager because the proxy cannot be set
-                            Downloader.downloadFile(getContext(), url, saveFilePath);
+                            Downloader.downloadFile(getContext(), url, saveFilePath, title);
                         } else {
                             DownloadManager dm = (DownloadManager) context.getSystemService(Context.DOWNLOAD_SERVICE);
                             DownloadManager.Request request = new DownloadManager.Request(
                                     Uri.parse(url));
-                            request.setDestinationUri(Uri.fromFile(new File(saveFilePath)));
+                            request.setDestinationUri(Uri.fromFile(saveFilePath));
                             request.setNotificationVisibility(DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED);
+
+                            request.setTitle(title);
+                            request.setDescription("'" + url +
+                                    "' => '" + saveFilePath + "'");
+                            request.allowScanningByMediaScanner();
+
                             try {
-                                dm.enqueue(request);
+                                id = dm.enqueue(request);
                             } catch (Exception e) {
                                 e.printStackTrace();
                             }
                         }
+
+                        Log.i(TAG,"Started downloading '" + url +
+                                "' => '" + saveFilePath + "' #" + id);
                     }
                 });
         return builder.create();
     }
 
+    /**
+     * #143 #44 #42 #22: make shure that the filename does not contain illegal chars.
+     * This should fix some of the "cannot download" problems.
+     * */
+    private String createFileName(String fName) {
+        // from http://eng-przemelek.blogspot.de/2009/07/how-to-create-valid-file-name.html
+
+        List<String> forbiddenCharsPatterns = new ArrayList<String> ();
+        forbiddenCharsPatterns.add("[:]+"); // Mac OS, but it looks that also Windows XP
+        forbiddenCharsPatterns.add("[\\*\"/\\\\\\[\\]\\:\\;\\|\\=\\,]+");  // Windows
+        forbiddenCharsPatterns.add("[^\\w\\d\\.]+");  // last chance... only latin letters and digits
+        String nameToTest = fName;
+        for (String pattern : forbiddenCharsPatterns) {
+            nameToTest = nameToTest.replaceAll(pattern, "_");
+        }
+        return nameToTest;
+    }
 }

app/src/main/java/org/schabi/newpipe/Downloader.java

@@ -13,6 +13,7 @@ import android.util.Log;
 
 import java.io.BufferedInputStream;
 import java.io.BufferedReader;
+import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -45,10 +46,32 @@ import info.guardianproject.netcipher.NetCipher;
  * along with NewPipe.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-public class Downloader {
+public class Downloader extends AsyncTask<Void, Integer, Void> {
     public static final String TAG = "Downloader";
     private static final String USER_AGENT = "Mozilla/5.0";
 
+    private NotificationManager nm;
+    private NotificationCompat.Builder builder;
+    private int notifyId = 0x1234;
+    private int fileSize = 0xffffffff;
+
+    private final Context context;
+    private final String fileURL;
+    private final File saveFilePath;
+    private final String title;
+
+    private final String debugContext;
+
+    public Downloader(Context context, String fileURL, File saveFilePath, String title) {
+        this.context = context;
+        this.fileURL = fileURL;
+        this.saveFilePath = saveFilePath;
+        this.title = title;
+
+        this.debugContext = "'" + fileURL +
+                "' => '" + saveFilePath + "'";
+    }
+
     /**Download the text file at the supplied URL as in download(String),
      * but set the HTTP header field "Accept-Language" to the supplied string.
      * @param siteUrl the URL of the text file to return the contents of
@@ -118,86 +141,96 @@ public class Downloader {
      *
      * @param fileURL      HTTP URL of the file to be downloaded
      * @param saveFilePath path of the directory to save the file
+     * @param title
      * @throws IOException
      */
-    public static void downloadFile(final Context context, final String fileURL, final String saveFilePath) {
+    public static void downloadFile(final Context context, final String fileURL, final File saveFilePath, String title) {
-        new AsyncTask<Void, Integer, Void>() {
+        new Downloader(context, fileURL, saveFilePath, title).execute();
+    }
 
-            private NotificationManager nm;
+    /** AsyncTask impl: executed in gui thread */
-            private NotificationCompat.Builder builder;
+    @Override
-            private int notifyId = 0x1234;
+    protected void onPreExecute() {
-            private int fileSize = 0xffffffff;
+        super.onPreExecute();
+        nm = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);
+        Drawable icon = context.getResources().getDrawable(R.mipmap.ic_launcher);
+        builder = new NotificationCompat.Builder(context)
+                .setSmallIcon(android.R.drawable.stat_sys_download)
+                .setLargeIcon(((BitmapDrawable) icon).getBitmap())
+                .setContentTitle(saveFilePath.getName())
+                .setContentText(saveFilePath.getAbsolutePath())
+                .setProgress(fileSize, 0, false);
+        nm.notify(notifyId, builder.build());
+    }
 
-            @Override
+    /** AsyncTask impl: executed in background thread does the download */
-            protected void onPreExecute() {
+    @Override
-                super.onPreExecute();
+    protected Void doInBackground(Void... voids) {
-                nm = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);
+        HttpsURLConnection con = null;
-                Drawable icon = context.getResources().getDrawable(R.mipmap.ic_launcher);
+        InputStream inputStream = null;
-                builder = new NotificationCompat.Builder(context)
+        FileOutputStream outputStream = null;
-                        .setSmallIcon(android.R.drawable.stat_sys_download)
+        try {
-                        .setLargeIcon(((BitmapDrawable) icon).getBitmap())
+            con = NetCipher.getHttpsURLConnection(fileURL);
-                        .setContentTitle(saveFilePath.substring(saveFilePath.lastIndexOf('/') + 1))
+            int responseCode = con.getResponseCode();
-                        .setContentText(saveFilePath)
-                        .setProgress(fileSize, 0, false);
-                nm.notify(notifyId, builder.build());
-            }
 
-            @Override
+            // always check HTTP response code first
-            protected Void doInBackground(Void... voids) {
+            if (responseCode == HttpURLConnection.HTTP_OK) {
-                HttpsURLConnection con = null;
+                fileSize = con.getContentLength();
-                try {
+                inputStream = new BufferedInputStream(con.getInputStream());
-                    con = NetCipher.getHttpsURLConnection(fileURL);
+                outputStream = new FileOutputStream(saveFilePath);
-                    int responseCode = con.getResponseCode();
 
-                    // always check HTTP response code first
+                int bufferSize = 8192;
-                    if (responseCode == HttpURLConnection.HTTP_OK) {
+                int downloaded = 0;
-                        fileSize = con.getContentLength();
-                        InputStream inputStream = new BufferedInputStream(con.getInputStream());
-                        FileOutputStream outputStream = new FileOutputStream(saveFilePath);
 
-                        int bufferSize = 8192;
+                int bytesRead = -1;
-                        int downloaded = 0;
+                byte[] buffer = new byte[bufferSize];
-
+                while ((bytesRead = inputStream.read(buffer)) != -1) {
-                        int bytesRead = -1;
+                    outputStream.write(buffer, 0, bytesRead);
-                        byte[] buffer = new byte[bufferSize];
+                    downloaded += bytesRead;
-                        while ((bytesRead = inputStream.read(buffer)) != -1) {
+                    if (downloaded % 50000 < bufferSize) {
-                            outputStream.write(buffer, 0, bytesRead);
+                        publishProgress(downloaded);
-                            downloaded += bytesRead;
-                            if (downloaded % 50000 < bufferSize) {
-                                publishProgress(downloaded);
-                            }
-                        }
-
-                        outputStream.close();
-                        inputStream.close();
-                        publishProgress(bufferSize);
-
-                    } else {
-                        Log.i(TAG, "No file to download. Server replied HTTP code: " + responseCode);
-                    }
-                } catch (IOException e) {
-                    e.printStackTrace();
-                } finally {
-                    if (con != null) {
-                        con.disconnect();
-                        con = null;
                     }
                 }
-                return null;
-            }
 
-            @Override
+                publishProgress(bufferSize);
-            protected void onProgressUpdate(Integer... progress) {
-                builder.setProgress(fileSize, progress[0], false);
-                nm.notify(notifyId, builder.build());
-            }
 
-            @Override
+            } else {
-            protected void onPostExecute(Void aVoid) {
+                Log.i(TAG, "No file to download. Server replied HTTP code: " + responseCode);
-                super.onPostExecute(aVoid);
-                nm.cancel(notifyId);
             }
-        }.execute();
+        } catch (IOException e) {
+            Log.e(TAG, "No file to download. Server replied HTTP code: ", e);
+            e.printStackTrace();
+        } finally {
+            try {
+                if (outputStream != null) {
+                    outputStream.close();
+                    outputStream = null;
+                }
+                if (inputStream != null) {
+                    inputStream.close();
+                    inputStream = null;
+                }
+            } catch (IOException e) {
+                e.printStackTrace();
+            }
+            if (con != null) {
+                con.disconnect();
+                con = null;
+            }
+        }
+        return null;
+    }
+
+    @Override
+    protected void onProgressUpdate(Integer... progress) {
+        builder.setProgress(fileSize, progress[0], false);
+        nm.notify(notifyId, builder.build());
+    }
+
+    @Override
+    protected void onPostExecute(Void aVoid) {
+        super.onPostExecute(aVoid);
+        nm.cancel(notifyId);
     }
 
 }

app/src/main/java/org/schabi/newpipe/NewPipeSettings.java

@@ -0,0 +1,72 @@
+/**
+ * Created by k3b on 07.01.2016.
+ *
+ * Copyright (C) Christian Schabesberger 2015 <chris.schabesberger@mailbox.org>
+ * NewPipeSettings.java is part of NewPipe.
+ *
+ * NewPipe is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * NewPipe is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with NewPipe.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package org.schabi.newpipe;
+
+import android.content.Context;
+import android.content.SharedPreferences;
+import android.os.Environment;
+import android.preference.PreferenceManager;
+import android.support.annotation.NonNull;
+import android.util.Log;
+
+import java.io.File;
+
+/**
+ * Helper for global settings
+ */
+public class NewPipeSettings {
+    public static void initSettings(Context context) {
+        PreferenceManager.setDefaultValues(context, R.xml.settings, false);
+        getVideoDownloadFolder(context);
+        getAudioDownloadFolder(context);
+    }
+
+    public static File getDownloadFolder() {
+        return getFolder(Environment.DIRECTORY_DOWNLOADS);
+    }
+
+    public static File getVideoDownloadFolder(Context context) {
+        return getFolder(context, R.string.download_path_key, Environment.DIRECTORY_MOVIES);
+    }
+
+    public static File getAudioDownloadFolder(Context context) {
+        return getFolder(context, R.string.download_path_audio_key, Environment.DIRECTORY_MUSIC);
+    }
+
+    private static File getFolder(Context context, int keyID, String defaultDirectoryName) {
+        SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context);
+        final String key = context.getString(keyID);
+        String downloadPath = prefs.getString(key, null);
+        if ((downloadPath != null) && (!downloadPath.isEmpty())) return new File(downloadPath.trim());
+
+        final File folder = getFolder(defaultDirectoryName);
+        SharedPreferences.Editor spEditor = prefs.edit();
+        spEditor.putString(key
+                , new File(folder,"NewPipe").getAbsolutePath());
+        spEditor.apply();
+        return folder;
+    }
+
+    @NonNull
+    private static File getFolder(String defaultDirectoryName) {
+        return new File(Environment.getExternalStorageDirectory(),defaultDirectoryName);
+    }
+}

app/src/main/java/org/schabi/newpipe/SettingsActivity.java

@@ -71,12 +71,14 @@ public class SettingsActivity extends PreferenceActivity  {
         String DEFAULT_AUDIO_FORMAT_PREFERENCE;
         String SEARCH_LANGUAGE_PREFERENCE;
         String DOWNLOAD_PATH_PREFERENCE;
+        String DOWNLOAD_PATH_AUDIO_PREFERENCE;
         String USE_TOR_KEY;
 
         private ListPreference defaultResolutionPreference;
         private ListPreference defaultAudioFormatPreference;
         private ListPreference searchLanguagePreference;
         private EditTextPreference downloadPathPreference;
+        private EditTextPreference downloadPathAudioPreference;
         private CheckBoxPreference useTorCheckBox;
         private SharedPreferences defaultPreferences;
 
@@ -95,6 +97,7 @@ public class SettingsActivity extends PreferenceActivity  {
             DEFAULT_AUDIO_FORMAT_PREFERENCE =getString(R.string.default_audio_format_key);
             SEARCH_LANGUAGE_PREFERENCE =getString(R.string.search_language_key);
             DOWNLOAD_PATH_PREFERENCE = getString(R.string.download_path_key);
+            DOWNLOAD_PATH_AUDIO_PREFERENCE = getString(R.string.download_path_audio_key);
             USE_TOR_KEY = getString(R.string.use_tor_key);
 
             // get pref objects
@@ -106,6 +109,8 @@ public class SettingsActivity extends PreferenceActivity  {
                     (ListPreference) findPreference(SEARCH_LANGUAGE_PREFERENCE);
             downloadPathPreference =
                     (EditTextPreference) findPreference(DOWNLOAD_PATH_PREFERENCE);
+            downloadPathAudioPreference =
+                    (EditTextPreference) findPreference(DOWNLOAD_PATH_AUDIO_PREFERENCE);
             useTorCheckBox = (CheckBoxPreference) findPreference(USE_TOR_KEY);
 
             prefListener = new SharedPreferences.OnSharedPreferenceChangeListener() {
@@ -147,6 +152,9 @@ public class SettingsActivity extends PreferenceActivity  {
             downloadPathPreference.setSummary(
                     defaultPreferences.getString(DOWNLOAD_PATH_PREFERENCE,
                             getString(R.string.download_path_summary)));
+            downloadPathAudioPreference.setSummary(
+                    defaultPreferences.getString(DOWNLOAD_PATH_AUDIO_PREFERENCE,
+                            getString(R.string.download_path_audio_summary)));
         }
     }
 
@@ -246,15 +254,6 @@ public class SettingsActivity extends PreferenceActivity  {
     }
 
     public static void initSettings(Context context) {
-        PreferenceManager.setDefaultValues(context, R.xml.settings, false);
+        NewPipeSettings.initSettings(context);
-        SharedPreferences sp = PreferenceManager.getDefaultSharedPreferences(context);
-        if(sp.getString(context.getString(R.string.download_path_key), "").isEmpty()){
-            SharedPreferences.Editor spEditor = sp.edit();
-            String newPipeDownloadStorage =
-                    Environment.getExternalStorageDirectory().getAbsolutePath() + "/NewPipe";
-            spEditor.putString(context.getString(R.string.download_path_key)
-                    , newPipeDownloadStorage);
-            spEditor.apply();
-        }
     }
 }

app/src/main/res/values/settings_keys.xml

@@ -6,6 +6,7 @@
     <string name="settings_category_other">settings_category_other</string>
     <!-- Key values -->
     <string name="download_path_key">download_path</string>
+    <string name="download_path_audio_key">download_path_audio</string>
     <string name="use_external_video_player_key">use_external_video_player</string>
     <string name="use_external_audio_player_key">use_external_audio_player</string>
     <string name="autoplay_through_intent_key">autoplay_through_intent</string>

app/src/main/res/values/strings.xml

@@ -23,9 +23,15 @@
     <string name="settings_activity_title">Settings</string>
     <string name="use_external_video_player_title">Use external video player</string>
     <string name="use_external_audio_player_title">Use external audio player</string>
-    <string name="download_path_title">Download path</string>
+
+    <string name="download_path_title">Download path video</string>
     <string name="download_path_summary">Path to store downloaded videos in</string>
-    <string name="download_path_dialog_title">Enter download path</string>
+    <string name="download_path_dialog_title">Enter download path for videos</string>
+
+    <string name="download_path_audio_title">Download path audio</string>
+    <string name="download_path_audio_summary">Path to store downloaded audio in</string>
+    <string name="download_path_audio_dialog_title">Enter download path for audios</string>
+
     <string name="autoplay_through_intent_title">Autoplay through Intent</string>
     <string name="autoplay_through_intent_summary">Automatically play a video when it\'s called from another app</string>
     <string name="default_resolution_title">Default Resolution</string>
@@ -69,4 +75,7 @@
     <string name="detail_dislikes_img_view_description">Dislikes</string>
     <string name="use_tor_title">Use Tor</string>
     <string name="use_tor_summary">Force download traffic through Tor for increased privacy (streaming videos not yet supported)</string>
+
+    <string name="err_dir_create">Cannot create download directory \'%1$s\'</string>
+    <string name="info_dir_created">Created download directory \'%1$s\'</string>
 </resources>

app/src/main/res/xml/settings.xml

@@ -81,6 +81,12 @@
             android:summary="@string/download_path_summary"
             android:dialogTitle="@string/download_path_dialog_title" />
 
+        <EditTextPreference
+            android:key="@string/download_path_audio_key"
+            android:title="@string/download_path_audio_title"
+            android:summary="@string/download_path_audio_summary"
+            android:dialogTitle="@string/download_path_audio_dialog_title" />
+
         <CheckBoxPreference
             android:key="@string/autoplay_through_intent_key"
             android:title="@string/autoplay_through_intent_title"