diff --git a/core/podcasts/podcasts.go b/core/podcasts/podcasts.go index 1cdb6d16b..96a267767 100644 --- a/core/podcasts/podcasts.go +++ b/core/podcasts/podcasts.go @@ -348,13 +348,6 @@ func (s *podcastService) doDownload(ctx context.Context, ep *model.PodcastEpisod } dest := filepath.Join(dir, ep.ID+"."+suffix) - f, err := os.Create(dest) - if err != nil { - s.setEpisodeError(ctx, ep, err) - return - } - defer f.Close() - if err := validateURL(ep.EnclosureURL); err != nil { s.setEpisodeError(ctx, ep, fmt.Errorf("invalid enclosure URL: %w", err)) return @@ -380,6 +373,15 @@ func (s *podcastService) doDownload(ctx context.Context, ep *model.PodcastEpisod return } + // Create the file only once we have a 200 response, so failed requests + // don't leave empty files behind. + f, err := os.Create(dest) + if err != nil { + s.setEpisodeError(ctx, ep, err) + return + } + defer f.Close() + // Use Content-Length as total size when RSS feed didn't provide it if resp.ContentLength > 0 && ep.Size == 0 { ep.Size = resp.ContentLength @@ -407,8 +409,14 @@ func (s *podcastService) doDownload(ctx context.Context, ep *model.PodcastEpisod now := time.Now() tags := model.Tags{} tags.Add("genre", "Podcast") + // Reuse the existing MediaFile on re-download so Put updates it instead of + // leaving a duplicate track behind. + mfID := ep.StreamID + if mfID == "" { + mfID = id.NewRandom() + } mf := &model.MediaFile{ - ID: id.NewRandom(), + ID: mfID, LibraryID: libID, Path: relPath, Title: ep.Title, diff --git a/core/podcasts/podcasts_test.go b/core/podcasts/podcasts_test.go index f64ff08cb..2865cce67 100644 --- a/core/podcasts/podcasts_test.go +++ b/core/podcasts/podcasts_test.go @@ -179,6 +179,21 @@ var _ = Describe("PodcastService", func() { Expect(found).To(BeTrue()) }) + It("reuses the existing StreamID's MediaFile on re-download", func() { + _ = svc.DownloadEpisode(ctx, "ep-1") + Eventually(func() model.PodcastStatus { + return episodeRepo.Data["ep-1"].Status + }, "3s").Should(Equal(model.PodcastStatusCompleted)) + first := episodeRepo.Data["ep-1"].StreamID + Expect(first).ToNot(BeEmpty()) + + _ = svc.DownloadEpisode(ctx, "ep-1") + Eventually(func() model.PodcastStatus { + return episodeRepo.Data["ep-1"].Status + }, "3s").Should(Equal(model.PodcastStatusCompleted)) + Expect(episodeRepo.Data["ep-1"].StreamID).To(Equal(first)) + }) + It("records the file path after download", func() { _ = svc.DownloadEpisode(ctx, "ep-1") expectedPath := filepath.Join(conf.Server.DataFolder.String(), "podcasts", "ch-1", "ep-1.mp3") @@ -236,6 +251,22 @@ var _ = Describe("PodcastService", func() { BeforeEach(func() { channelRepo.Data["ch-1"] = &model.PodcastChannel{ID: "ch-1", Title: "Test Channel"} }) + It("does not leave an empty file behind when the request fails", func() { + episodeRepo.Data["ep-bad"] = &model.PodcastEpisode{ + ID: "ep-bad", + ChannelID: "ch-1", + EnclosureURL: "http://localhost:0/no-such.mp3", + Suffix: "mp3", + Status: model.PodcastStatusNew, + } + _ = svc.DownloadEpisode(ctx, "ep-bad") + Eventually(func() model.PodcastStatus { + return episodeRepo.Data["ep-bad"].Status + }, "3s").Should(Equal(model.PodcastStatusError)) + dest := filepath.Join(conf.Server.DataFolder.String(), "podcasts", "ch-1", "ep-bad.mp3") + _, err := os.Stat(dest) + Expect(os.IsNotExist(err)).To(BeTrue()) + }) It("sets status to error when download fails", func() { episodeRepo.Data["ep-bad"] = &model.PodcastEpisode{ ID: "ep-bad", diff --git a/ui/src/podcast/PodcastList.jsx b/ui/src/podcast/PodcastList.jsx index 7e7dafe6e..421751c12 100644 --- a/ui/src/podcast/PodcastList.jsx +++ b/ui/src/podcast/PodcastList.jsx @@ -179,6 +179,7 @@ const PodcastList = ({ permissions, ...props }) => { > {isXsmall ? ( } primaryText={(r) => r.title} secondaryText={(r) => r.url} diff --git a/ui/src/podcast/PodcastShow.jsx b/ui/src/podcast/PodcastShow.jsx index 3e7c95e4d..5b32fa41d 100644 --- a/ui/src/podcast/PodcastShow.jsx +++ b/ui/src/podcast/PodcastShow.jsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from 'react' +import React, { useEffect, useRef, useState } from 'react' import { Card, CardContent, @@ -110,11 +110,17 @@ const PodcastShow = (props) => { const { record } = useShowController(props) const [episodes, setEpisodes] = useState([]) + // Incremented on every load and on channel change; responses from a + // superseded request are ignored so they can't overwrite the current channel. + const requestGen = useRef(0) + const loadEpisodes = () => { if (!record?.id) return + const gen = ++requestGen.current subsonic .getPodcasts(record.id, true) .then((res) => { + if (gen !== requestGen.current) return const channels = res?.json?.['subsonic-response']?.podcasts?.channel || [] const ch = channels.find((c) => c.id === record.id) setEpisodes(ch?.episode || []) @@ -122,7 +128,13 @@ const PodcastShow = (props) => { .catch(() => {}) } - useEffect(loadEpisodes, [record?.id]) + useEffect(() => { + setEpisodes([]) // don't show the previous channel's episodes + loadEpisodes() + return () => { + requestGen.current++ + } + }, [record?.id]) // Stay subscribed to SSE progress while the view is mounted: a download // started from here may not be flagged 'downloading' yet when we refresh.