Start of destination support and some refactoring

.vscode/settings.json

@@ -1,9 +1,10 @@
 {
     "cSpell.words": [
         "Cleanup",
+        "gropple",
+        "succ",
         "tmpl",
-        "vars",
+        "vars"
-        "gropple"
     ],
     "cSpell.language": "en-GB"
 }

CHANGELOG.md

@@ -8,9 +8,10 @@ All notable changes to this project will be documented in this file.
 
 - When downloading from a playlist, show the total number of videos and how many have been downloaded
 - Show version in web UI
-- Fixes and improvements to capturing output info and showing it in the UI
 - Improve index page (show URL of queued downloads instead of nothing)
 - Add docker support
+- Fixes and improvements to capturing output info and showing it in the UI
+- Fixes to handling of queued downloads
 
 ## [v0.5.5] - 2022-04-09
 

config/config.go

@@ -104,6 +104,15 @@ func (c *Config) ProfileCalled(name string) *DownloadProfile {
 	return nil
 }
 
+func (c *Config) DestinationCalled(name string) *Destination {
+	for _, p := range c.Destinations {
+		if p.Name == name {
+			return &p
+		}
+	}
+	return nil
+}
+
 func (c *Config) UpdateFromJSON(j []byte) error {
 	newConfig := Config{}
 	err := json.Unmarshal(j, &newConfig)

download/download.go

@@ -25,6 +25,7 @@ type Download struct {
 	ExitCode        int                    `json:"exit_code"`
 	State           string                 `json:"state"`
 	DownloadProfile config.DownloadProfile `json:"download_profile"`
+	Destination     *config.Destination    `json:"destination"`
 	Finished        bool                   `json:"finished"`
 	FinishedTS      time.Time              `json:"finished_ts"`
 	Files           []string               `json:"files"`
@@ -34,8 +35,10 @@ type Download struct {
 	Percent         float32                `json:"percent"`
 	Log             []string               `json:"log"`
 	Config          *config.Config
+	Lock            sync.Mutex
 }
 
+// The Manager holds and is responsible for all Download objects.
 type Manager struct {
 	Downloads    []*Download
 	MaxPerDomain int
@@ -48,11 +51,10 @@ var downloadId int32 = 0
 
 func (m *Manager) ManageQueue() {
 	for {
-
 		m.Lock.Lock()
 
 		m.startQueued(m.MaxPerDomain)
-		m.cleanup()
+		// m.cleanup()
 		m.Lock.Unlock()
 
 		time.Sleep(time.Second)
@@ -62,33 +64,44 @@ func (m *Manager) ManageQueue() {
 // startQueued starts any downloads that have been queued, we would not exceed
 // maxRunning. If maxRunning is 0, there is no limit.
 func (m *Manager) startQueued(maxRunning int) {
+
 	active := make(map[string]int)
 
 	for _, dl := range m.Downloads {
+		dl.Lock.Lock()
 
-		if dl.State == "downloading" {
+		if dl.State == "downloading" || dl.State == "preparing to start" {
 			active[dl.domain()]++
 		}
+		dl.Lock.Unlock()
 
 	}
 
 	for _, dl := range m.Downloads {
 
+		dl.Lock.Lock()
+
 		if dl.State == "queued" && (maxRunning == 0 || active[dl.domain()] < maxRunning) {
-			dl.State = "downloading"
+			dl.State = "preparing to start"
 			active[dl.domain()]++
 			log.Printf("Starting download for id:%d (%s)", dl.Id, dl.Url)
-			go func() {
+
-				m.Begin(dl.Id)
+			dl.Lock.Unlock()
-			}()
+
+			go func(sdl *Download) {
+				sdl.Begin()
+			}(dl)
+		} else {
+			dl.Lock.Unlock()
 		}
+
 	}
 
 }
 
 // cleanup removes old downloads from the list. Hardcoded to remove them one hour
 // completion.
-func (m *Manager) cleanup() {
+func (m *Manager) XXXcleanup() {
 	newDLs := []*Download{}
 	for _, dl := range m.Downloads {
 
@@ -102,59 +115,68 @@ func (m *Manager) cleanup() {
 	m.Downloads = newDLs
 }
 
-func (m *Manager) DlById(id int) *Download {
+// GetDlById returns one of the downloads in our current list.
+func (m *Manager) GetDlById(id int) (*Download, error) {
+	m.Lock.Lock()
+	defer m.Lock.Unlock()
 	for _, dl := range m.Downloads {
 		if dl.Id == id {
-			return dl
+			return dl, nil
 		}
 	}
-	return nil
+	return nil, fmt.Errorf("no download with id %d", id)
 }
 
 // Queue queues a download
-func (m *Manager) Queue(id int) {
+func (m *Manager) Queue(dl *Download) {
-
+	dl.Lock.Lock()
-	dl := m.DlById(id)
+	defer dl.Lock.Unlock()
 	dl.State = "queued"
-
 }
 
-func (m *Manager) NewDownload(conf *config.Config, url string) int {
+func NewDownload(url string, conf *config.Config) *Download {
 	atomic.AddInt32(&downloadId, 1)
 	dl := Download{
-		Config: conf,
-
 		Id:       int(downloadId),
 		Url:      url,
 		PopupUrl: fmt.Sprintf("/fetch/%d", int(downloadId)),
 		State:    "choose profile",
-		Finished: false,
+		Files:    []string{},
-		Eta:      "?",
+		Log:      []string{},
-		Percent:  0.0,
+		Config:   conf,
-		Log:      make([]string, 0, 1000),
+		Lock:     sync.Mutex{},
 	}
-	m.Downloads = append(m.Downloads, &dl)
+	return &dl
-	return int(downloadId)
 }
 
-func (m *Manager) AppendLog(id int, text string) {
+func (m *Manager) AddDownload(dl *Download) {
-	dl := m.DlById(id)
+	m.Lock.Lock()
-	dl.Log = append(dl.Log, text)
+	defer m.Lock.Unlock()
+	m.Downloads = append(m.Downloads, dl)
+	return
 }
 
+// func (dl *Download) AppendLog(text string) {
+// 	dl.Lock.Lock()
+// 	defer dl.Lock.Unlock()
+// 	dl.Log = append(dl.Log, text)
+// }
+
 // Stop the download.
-func (m *Manager) Stop(id int) {
+func (dl *Download) Stop() {
 	if !CanStopDownload {
 		log.Print("attempted to stop download on a platform that it is not currently supported on - please report this as a bug")
 		os.Exit(1)
 	}
-	dl := m.DlById(id)
 
 	log.Printf("stopping the download")
+	dl.Lock.Lock()
+	defer dl.Lock.Unlock()
 	dl.Log = append(dl.Log, "aborted by user")
 	dl.Process.Kill()
 }
 
+// domain returns a domain for this Download. Download should be locked.
 func (dl *Download) domain() string {
 
 	url, err := url.Parse(dl.Url)
@@ -169,10 +191,8 @@ func (dl *Download) domain() string {
 
 // Begin starts a download, by starting the command specified in the DownloadProfile.
 // It blocks until the download is complete.
-func (m *Manager) Begin(id int) {
+func (dl *Download) Begin() {
-	m.Lock.Lock()
+	dl.Lock.Lock()
-
-	dl := m.DlById(id)
 
 	dl.State = "downloading"
 	cmdSlice := []string{}
@@ -192,7 +212,7 @@ func (m *Manager) Begin(id int) {
 		dl.Finished = true
 		dl.FinishedTS = time.Now()
 		dl.Log = append(dl.Log, fmt.Sprintf("error setting up stdout pipe: %v", err))
-		m.Lock.Unlock()
+		dl.Lock.Unlock()
 
 		return
 	}
@@ -203,7 +223,7 @@ func (m *Manager) Begin(id int) {
 		dl.Finished = true
 		dl.FinishedTS = time.Now()
 		dl.Log = append(dl.Log, fmt.Sprintf("error setting up stderr pipe: %v", err))
-		m.Lock.Unlock()
+		dl.Lock.Unlock()
 
 		return
 	}
@@ -215,7 +235,7 @@ func (m *Manager) Begin(id int) {
 		dl.Finished = true
 		dl.FinishedTS = time.Now()
 		dl.Log = append(dl.Log, fmt.Sprintf("error starting command '%s': %v", dl.DownloadProfile.Command, err))
-		m.Lock.Unlock()
+		dl.Lock.Unlock()
 
 		return
 	}
@@ -225,24 +245,24 @@ func (m *Manager) Begin(id int) {
 
 	wg.Add(2)
 
-	m.Lock.Unlock()
+	dl.Lock.Unlock()
 
 	go func() {
 		defer wg.Done()
-		m.updateDownload(dl, stdout)
+		dl.updateDownload(stdout)
 	}()
 
 	go func() {
 		defer wg.Done()
-		m.updateDownload(dl, stderr)
+		dl.updateDownload(stderr)
 	}()
 
 	wg.Wait()
 	cmd.Wait()
 
-	log.Printf("Process finished for id: %d (%v)", dl.Id, cmd)
+	dl.Lock.Lock()
 
-	m.Lock.Lock()
+	log.Printf("Process finished for id: %d (%v)", dl.Id, cmd)
 
 	dl.State = "complete"
 	dl.Finished = true
@@ -252,12 +272,14 @@ func (m *Manager) Begin(id int) {
 	if dl.ExitCode != 0 {
 		dl.State = "failed"
 	}
-
+	dl.Lock.Unlock()
-	m.Lock.Unlock()
 
 }
 
-func (m *Manager) updateDownload(dl *Download, r io.Reader) {
+// updateDownload updates the download based on data from the reader. Expects the
+// Download to be unlocked.
+func (dl *Download) updateDownload(r io.Reader) {
+
 	// XXX not sure if we might get a partial line?
 	buf := make([]byte, 1024)
 	for {
@@ -272,15 +294,12 @@ func (m *Manager) updateDownload(dl *Download, r io.Reader) {
 					continue
 				}
 
-				m.Lock.Lock()
-
 				// append the raw log
+				dl.Lock.Lock()
 				dl.Log = append(dl.Log, l)
-
 				// look for the percent and eta and other metadata
 				dl.updateMetadata(l)
-
+				dl.Lock.Unlock()
-				m.Lock.Unlock()
 
 			}
 		}
@@ -290,6 +309,7 @@ func (m *Manager) updateDownload(dl *Download, r io.Reader) {
 	}
 }
 
+// updateMetadata parses some metadata and updates the Download. Download must be locked.
 func (dl *Download) updateMetadata(s string) {
 
 	// [download]  49.7% of ~15.72MiB at  5.83MiB/s ETA 00:07

main.go

@@ -111,6 +111,25 @@ func main() {
 	// old entries
 	go dm.ManageQueue()
 
+	urls := []string{
+		"https://www.youtube.com/watch?v=qG_rRkuGBW8",
+		"https://www.youtube.com/watch?v=ZUzhZpQAU40",
+		// "https://www.youtube.com/watch?v=kVxM3eRWGak",
+		// "https://www.youtube.com/watch?v=pl-y9869y0w",
+		// "https://www.youtube.com/watch?v=Uw4NEPE4l3A",
+		// "https://www.youtube.com/watch?v=6tIsT57_nS0",
+		// "https://www.youtube.com/watch?v=2RF0lcTuuYE",
+		// "https://www.youtube.com/watch?v=lymwNQY0dus",
+		// "https://www.youtube.com/watch?v=NTc-I4Z_duc",
+		// "https://www.youtube.com/watch?v=wNSm1TJ84Ac",
+	}
+	for _, u := range urls {
+		d := download.NewDownload(u, configService.Config)
+		d.DownloadProfile = *configService.Config.ProfileCalled("standard video")
+		dm.AddDownload(d)
+		dm.Queue(d)
+	}
+
 	log.Printf("Visit %s for details on installing the bookmarklet and to check status", configService.Config.Server.Address)
 	log.Fatal(srv.ListenAndServe())
 
@@ -138,22 +157,21 @@ func homeHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	type Info struct {
-		Downloads      []*download.Download
+		Manager        *download.Manager
 		BookmarkletURL template.URL
 		Config         *config.Config
 		Version        version.Info
 	}
 
-	dm.Lock.Lock()
-	defer dm.Lock.Unlock()
-
 	info := Info{
-		Downloads:      dm.Downloads,
+		Manager:        dm,
 		BookmarkletURL: template.URL(bookmarkletURL),
 		Config:         configService.Config,
 		Version:        versionInfo.GetInfo(),
 	}
 
+	dm.Lock.Lock()
+	defer dm.Lock.Unlock()
 	err = t.ExecuteTemplate(w, "layout", info)
 	if err != nil {
 		panic(err)
@@ -229,20 +247,21 @@ func fetchInfoOneRESTHandler(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		dm.Lock.Lock()
+		thisDownload, err := dm.GetDlById(id)
-		defer dm.Lock.Unlock()
+		if err != nil {
-
-		thisDownload := dm.DlById(id)
-		if thisDownload == nil {
 			http.NotFound(w, r)
 			return
 		}
+		if thisDownload == nil {
+			panic("should not happen")
+		}
 
 		if r.Method == "POST" {
 
 			type updateRequest struct {
-				Action  string `json:"action"`
+				Action      string `json:"action"`
-				Profile string `json:"profile"`
+				Profile     string `json:"profile"`
+				Destination string `json:"destination"`
 			}
 
 			thisReq := updateRequest{}
@@ -268,8 +287,11 @@ func fetchInfoOneRESTHandler(w http.ResponseWriter, r *http.Request) {
 					panic("bad profile name?")
 				}
 				// set the profile
+				thisDownload.Lock.Lock()
 				thisDownload.DownloadProfile = *profile
-				dm.Queue(thisDownload.Id)
+				thisDownload.Lock.Unlock()
+
+				dm.Queue(thisDownload)
 
 				succRes := successResponse{Success: true, Message: "download started"}
 				succResB, _ := json.Marshal(succRes)
@@ -277,8 +299,27 @@ func fetchInfoOneRESTHandler(w http.ResponseWriter, r *http.Request) {
 				return
 			}
 
+			if thisReq.Action == "change_destination" {
+
+				// nil means (probably) that they chose "don't move" - which is fine,
+				// and maps to nil on the Download (the default state).
+				destination := configService.Config.DestinationCalled(thisReq.Destination)
+
+				thisDownload.Lock.Lock()
+				thisDownload.Destination = destination
+				thisDownload.Lock.Unlock()
+
+				log.Printf("%#v", thisDownload)
+
+				succRes := successResponse{Success: true, Message: "destination changed"}
+				succResB, _ := json.Marshal(succRes)
+				w.Write(succResB)
+				return
+			}
+
 			if thisReq.Action == "stop" {
-				dm.Stop(thisDownload.Id)
+
+				thisDownload.Stop()
 				succRes := successResponse{Success: true, Message: "download stopped"}
 				succResB, _ := json.Marshal(succRes)
 				w.Write(succResB)
@@ -310,14 +351,18 @@ func fetchHandler(w http.ResponseWriter, r *http.Request) {
 	vars := mux.Vars(r)
 	idString := vars["id"]
 
-	dm.Lock.Lock()
-	defer dm.Lock.Unlock()
-
 	idInt, err := strconv.ParseInt(idString, 10, 32)
 
 	// existing, load it up
 	if err == nil && idInt > 0 {
-		dl := dm.DlById(int(idInt))
+
+		dl, err := dm.GetDlById(int(idInt))
+		if err != nil {
+			log.Printf("not found")
+			w.WriteHeader(404)
+			return
+		}
+
 		t, err := template.ParseFS(webFS, "web/layout.tmpl", "web/popup.html")
 		if err != nil {
 			panic(err)
@@ -341,6 +386,7 @@ func fetchHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	} else {
 
+		log.Printf("popup for %s", url)
 		// check the URL for a sudden but inevitable betrayal
 		if strings.Contains(url[0], configService.Config.Server.Address) {
 			w.WriteHeader(400)
@@ -348,21 +394,28 @@ func fetchHandler(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		// create the record
+		// create the new download
-
+		log.Print("creating")
-		newDownloadId := dm.NewDownload(configService.Config, url[0])
+		newDL := download.NewDownload(url[0], configService.Config)
-		dm.AppendLog(newDownloadId, "start of log...")
+		log.Print("adding")
+		dm.AddDownload(newDL)
+		log.Print("done")
 
 		t, err := template.ParseFS(webFS, "web/layout.tmpl", "web/popup.html")
 		if err != nil {
 			panic(err)
 		}
 
-		templateData := map[string]interface{}{"Version": versionInfo.GetInfo(), "dl": dm.DlById(newDownloadId), "config": configService.Config, "canStop": download.CanStopDownload}
+		log.Print("lock dl")
+		newDL.Lock.Lock()
+		defer newDL.Lock.Unlock()
+
+		templateData := map[string]interface{}{"Version": versionInfo.GetInfo(), "dl": newDL, "config": configService.Config, "canStop": download.CanStopDownload}
 
 		err = t.ExecuteTemplate(w, "layout", templateData)
 		if err != nil {
 			panic(err)
 		}
+		log.Print("unlock dl because rendered")
 	}
 }

web/index.html

@@ -54,8 +54,6 @@
             </template>
               
 
-            {{ range $k, $v := .Downloads }}
-            {{ end }}
         </tbody>  
     </table>
 </div>
@@ -65,7 +63,7 @@
 <script>
     function index() {
         return { 
-            items: [], version: {},
+            items: [], version: {}, popups: {},
             fetch_version() {
                 fetch('/rest/version')
                 .then(response => response.json())
@@ -90,9 +88,11 @@
                 })
             },
             show_popup(item) {
-			    window.open(item.popup_url, item.id, "width={{ .Config.UI.PopupWidth }},height={{ .Config.UI.PopupHeight }}");
+                // allegedly you can use the reference to pop the window to the front on subsequent
-	    },
+                // clicks, but I can't seem to find a reliable way to do so.
-        }
+                this.popups[item.id] = window.open(item.popup_url, item.id, "width={{ .Config.UI.PopupWidth }},height={{ .Config.UI.PopupHeight }}");
+            },
+	    }
     }
 </script>
 {{ end }}

web/popup.html

@@ -15,6 +15,17 @@
                 </td>
             </tr>
             <tr><th>current filename</th><td x-text="filename"></td></tr>
+            <tr>
+                <th>destination</th>
+                <td>
+                    <select  x-on:change="update_destination()" class="pure-input-1-2" x-model="destination_chosen">
+                        <option value="-">leave in {{ .config.Server.DownloadPath }}</option>
+                    {{ range $i := .config.Destinations }}
+                        <option>{{ $i.Name }}</option>
+                    {{ end }}
+                    </select>
+                </td>
+            </tr>
             <tr><th>state</th><td x-text="state"></td></tr>
             <tr x-show="playlist_total > 0"><th>playlist progress</th><td x-text="playlist_current + '/' + playlist_total"></td></tr>
             <tr><th>progress</th><td x-text="percent"></td></tr>
@@ -40,6 +51,7 @@
             eta: '', percent: 0.0, state: '??', filename: '', finished: false, log :'',
             playlist_current: 0, playlist_total: 0,
             profile_chosen: null,
+            destination_chosen: null,
             watch_profile() {
                 this.$watch('profile_chosen', value => this.profile_chosen(value))
             },
@@ -56,6 +68,18 @@
                     console.log(info)
                 })
             },
+            update_destination(name) {
+                let op = {
+                   method: 'POST',
+                   body: JSON.stringify({action: 'change_destination', destination: this.destination_chosen}),
+                   headers: { 'Content-Type': 'application/json' }
+                };
+                fetch('/rest/fetch/{{ .dl.Id }}', op)
+                .then(response => response.json())
+                .then(info => {
+                    console.log(info)
+                })
+            },
             stop() {
                 let op = {
                    method: 'POST',