msansm1/rss-podcast-dl · 812d86c

⑂ 812d86c

Téléchargement atomique : écriture via fichier .part puis renommage

- download_file écrit dans <fichier>.part et ne renomme vers la
  destination finale qu'une fois le téléchargement complet et la
  taille vérifiée : un téléchargement interrompu n'est plus pris
  pour un épisode complet et est retéléchargé à la sync suivante
- le .part est supprimé en cas d'erreur (réseau, taille, écriture)
- suppression de la requête HEAD préalable : le Content-Length vient
  du GET, et un serveur sans Content-Length ne fait plus échouer
  le téléchargement (progression à 0 dans ce cas)
- un mismatch de taille échoue désormais avec ContentLengthMismatch
  au lieu d'être seulement loggé
- tests : .part orphelin ignoré par needs_download, construction
  du chemin .part

msansm1 committed 2026-09-28T22:18:59+02:00 Browse files

812d86c parent: 1c88366

modified src/rss/downloader.rs +100 -44 Viewed

@@ -91,28 +91,9 @@ pub fn download_file(
91	91	.build()
92	92	.map_err(\|e\| DownloadError::NetworkError(e.to_string()))?;
93	93
94		- // Send HEAD request first to get content length
95		- let head_response = match client.head(url).send() {
96		- Ok(resp) => resp,
97		- Err(e) => {
98		- if e.is_timeout() {
99		- return Err(DownloadError::Timeout(60));
100		- }
101		- return Err(DownloadError::NetworkError(e.to_string()));
102		- }
103		- };
104		-
105		- if !head_response.status().is_success() {
106		- return Err(DownloadError::HttpError(format!("HTTP {}: {}", head_response.status(), head_response.status())));
107		- }
108		-
109		- let total_size = head_response
110		- .content_length()
111		- .ok_or(DownloadError::ContentLengthMismatch)?;
112		-
113	94	info!("Downloading {} to {}", url, dest.display());
114	95
115		- // Download the file
	96	+ // Send the GET request; the content length comes from this response
116	97	let response = client.get(url).send()
117	98	.map_err(\|e\| {
118	99	if e.is_timeout() {
@@ -123,37 +104,83 @@ pub fn download_file(
123	104	})?;
124	105
125	106	if !response.status().is_success() {
126		- return Err(DownloadError::HttpError(format!("HTTP {}: {}", response.status(), response.status())));
	107	+ return Err(DownloadError::HttpError(format!("HTTP {}", response.status())));
127	108	}
128	109
129		- // Create the file
130		- let mut file = fs::File::create(dest)
	110	+ let total_size = response.content_length();
	111	+
	112	+ // Download to a temporary .part file. The file is only renamed to its
	113	+ // final destination once complete, so an interrupted download is
	114	+ // detected and retried on the next sync.
	115	+ let part_path = part_file_path(dest);
	116	+
	117	+ let downloaded = match stream_to_file(response, &part_path, total_size, &progress_callback) {
	118	+ Ok(n) => n,
	119	+ Err(e) => {
	120	+ // Clean up the partial file so it is never mistaken for a
	121	+ // complete download
	122	+ let _ = fs::remove_file(&part_path);
	123	+ return Err(e);
	124	+ }
	125	+ };
	126	+
	127	+ // Rename the completed part file to its final destination.
	128	+ // On Windows, rename fails if the destination exists: remove it first
	129	+ // (e.g. a race with a previous incomplete attempt).
	130	+ if dest.exists() {
	131	+ fs::remove_file(dest)
	132	+ .map_err(\|e\| DownloadError::WriteError(e.to_string()))?;
	133	+ }
	134	+ fs::rename(&part_path, dest)
	135	+ .map_err(\|e\| DownloadError::WriteError(e.to_string()))?;
	136	+
	137	+ info!("Successfully downloaded {} bytes to {}", downloaded, dest.display());
	138	+
	139	+ Ok(())
	140	+}
	141	+
	142	+/// Returns the temporary ".part" path for a destination file
	143	+fn part_file_path(dest: &Path) -> PathBuf {
	144	+ let mut name = dest.file_name().map(\|n\| n.to_os_string()).unwrap_or_default();
	145	+ name.push(".part");
	146	+ dest.with_file_name(name)
	147	+}
	148	+
	149	+/// Streams a response body to a file, reporting download progress.
	150	+/// Fails if the downloaded size does not match the expected content length.
	151	+fn stream_to_file(
	152	+ mut response: reqwest::blocking::Response,
	153	+ part_path: &Path,
	154	+ total_size: Option<u64>,
	155	+ progress_callback: &Option<ProgressCallback>,
	156	+) -> Result<u64, DownloadError> {
	157	+ let mut file = fs::File::create(part_path)
131	158	.map_err(\|e\| DownloadError::FileCreationError(e.to_string()))?;
132	159
133	160	let mut downloaded: u64 = 0;
134	161
135	162	// Read response in chunks
136		- let mut reader = response;
137	163	let mut buffer = vec![0u8; 8192];
138	164
139	165	loop {
140		- let bytes_read = match reader.read(&mut buffer) {
	166	+ let bytes_read = match response.read(&mut buffer) {
141	167	Ok(0) => break, // EOF
142	168	Ok(n) => n,
143	169	Err(e) => return Err(DownloadError::NetworkError(e.to_string())),
144	170	};
145		-
	171	+
146	172	file.write_all(&buffer[..bytes_read])
147	173	.map_err(\|e\| DownloadError::WriteError(e.to_string()))?;
148		-
	174	+
149	175	downloaded += bytes_read as u64;
150		-
	176	+
151	177	// Report progress if callback is provided
152		- if let Some(cb) = &progress_callback {
153		- let progress = if total_size > 0 {
154		- (downloaded as f32 / total_size as f32).min(1.0)
155		- } else {
156		- 0.0
	178	+ if let Some(cb) = progress_callback {
	179	+ let progress = match total_size {
	180	+ Some(total) if total > 0 => {
	181	+ (downloaded as f32 / total as f32).min(1.0)
	182	+ }
	183	+ _ => 0.0,
157	184	};
158	185	cb(progress);
159	186	}
@@ -161,15 +188,18 @@ pub fn download_file(
161	188
162	189	file.flush().map_err(\|e\| DownloadError::WriteError(e.to_string()))?;
163	190
164		- // Verify size matches
165		- if total_size > 0 && downloaded != total_size {
166		- warn!("Downloaded size mismatch: expected {}, got {}", total_size, downloaded);
167		- // Don't fail, but log the discrepancy
	191	+ // Verify size matches; an incomplete file must not be kept
	192	+ if let Some(total) = total_size {
	193	+ if downloaded != total {
	194	+ warn!(
	195	+ "Downloaded size mismatch: expected {}, got {}",
	196	+ total, downloaded
	197	+ );
	198	+ return Err(DownloadError::ContentLengthMismatch);
	199	+ }
168	200	}
169	201
170		- info!("Successfully downloaded {} bytes to {}", downloaded, dest.display());
171		-
172		- Ok(())
	202	+ Ok(downloaded)
173	203	}
174	204
175	205	/// Downloads an episode to its designated location
@@ -220,13 +250,13 @@ pub fn download_episodes(
220	250	download_path: &Path,
221	251	) -> Result<Vec<PathBuf>, DownloadError> {
222	252	let mut downloaded_paths = Vec::new();
223		-
	253	+
224	254	for episode in episodes {
225	255	// Use a no-op callback for now
226	256	let path = download_episode(episode, download_path, Box::new(\|_\| {}))?;
227	257	downloaded_paths.push(path);
228	258	}
229		-
	259	+
230	260	Ok(downloaded_paths)
231	261	}
232	262
@@ -253,13 +283,13 @@ pub fn needs_download(episode: &Episode, download_path: &Path) -> bool {
253	283	#[allow(dead_code)]
254	284	pub fn remove_downloaded_episode(episode: &Episode, download_path: &Path) -> Result<(), DownloadError> {
255	285	let save_path = episode.get_save_path(download_path);
256		-
	286	+
257	287	if save_path.exists() {
258	288	fs::remove_file(&save_path)
259	289	.map_err(\|e\| DownloadError::WriteError(e.to_string()))?;
260	290	info!("Removed downloaded episode: {}", episode.title);
261	291	}
262		-
	292	+
263	293	Ok(())
264	294	}
265	295
@@ -305,6 +335,32 @@ mod tests {
305	335	assert!(!needs_download(&episode, download_path));
306	336	}
307	337
	338	+ #[test]
	339	+ fn test_needs_download_part_file_only() {
	340	+ let episode = Episode::new(
	341	+ "ep-123",
	342	+ "Test Episode",
	343	+ "https://example.com/episode.mp3",
	344	+ Utc::now(),
	345	+ );
	346	+ let temp_dir = tempdir().unwrap();
	347	+ let download_path = temp_dir.path();
	348	+
	349	+ // Only an interrupted (.part) download exists: it must be re-downloaded
	350	+ let part_path = part_file_path(&download_path.join(&episode.audio_filename));
	351	+ fs::write(&part_path, "partial content").unwrap();
	352	+
	353	+ assert!(needs_download(&episode, download_path));
	354	+ }
	355	+
	356	+ #[test]
	357	+ fn test_part_file_path() {
	358	+ let dest = Path::new("downloads").join("episode.mp3");
	359	+ let part = part_file_path(&dest);
	360	+ assert_eq!(part, Path::new("downloads").join("episode.mp3.part"));
	361	+ assert_eq!(part.file_name().unwrap().to_string_lossy(), "episode.mp3.part");
	362	+ }
	363	+
308	364	#[test]
309	365	fn test_get_download_dir() {
310	366	let path = PathBuf::from("/downloads/podcasts");

          .build()
          .map_err(|e| DownloadError::NetworkError(e.to_string()))?;
--    // Send HEAD request first to get content length
--    let head_response = match client.head(url).send() {
--        Ok(resp) => resp,
--        Err(e) => {
--            if e.is_timeout() {
--                return Err(DownloadError::Timeout(60));
--            }
--            return Err(DownloadError::NetworkError(e.to_string()));
--        }
--    };
--
--    if !head_response.status().is_success() {
--        return Err(DownloadError::HttpError(format!("HTTP {}: {}", head_response.status(), head_response.status())));
--    }
--
--    let total_size = head_response
--        .content_length()
--        .ok_or(DownloadError::ContentLengthMismatch)?;
--
      info!("Downloading {} to {}", url, dest.display());
--    // Download the file
++    // Send the GET request; the content length comes from this response
      let response = client.get(url).send()
          .map_err(|e| {
              if e.is_timeout() {
          })?;
      if !response.status().is_success() {
--        return Err(DownloadError::HttpError(format!("HTTP {}: {}", response.status(), response.status())));
++        return Err(DownloadError::HttpError(format!("HTTP {}", response.status())));
      }
--    // Create the file
++    let total_size = response.content_length();
--    let mut file = fs::File::create(dest)
++
++    // Download to a temporary .part file. The file is only renamed to its
++    // final destination once complete, so an interrupted download is
++    // detected and retried on the next sync.
++    let part_path = part_file_path(dest);
++
++    let downloaded = match stream_to_file(response, &part_path, total_size, &progress_callback) {
++        Ok(n) => n,
++        Err(e) => {
++            // Clean up the partial file so it is never mistaken for a
++            // complete download
++            let _ = fs::remove_file(&part_path);
++            return Err(e);
++        }
++    };
++
++    // Rename the completed part file to its final destination.
++    // On Windows, rename fails if the destination exists: remove it first
++    // (e.g. a race with a previous incomplete attempt).
++    if dest.exists() {
++        fs::remove_file(dest)
++            .map_err(|e| DownloadError::WriteError(e.to_string()))?;
++    }
++    fs::rename(&part_path, dest)
++        .map_err(|e| DownloadError::WriteError(e.to_string()))?;
++
++    info!("Successfully downloaded {} bytes to {}", downloaded, dest.display());
++
++    Ok(())
++}
++
++/// Returns the temporary ".part" path for a destination file
++fn part_file_path(dest: &Path) -> PathBuf {
++    let mut name = dest.file_name().map(|n| n.to_os_string()).unwrap_or_default();
++    name.push(".part");
++    dest.with_file_name(name)
++}
++
++/// Streams a response body to a file, reporting download progress.
++/// Fails if the downloaded size does not match the expected content length.
++fn stream_to_file(
++    mut response: reqwest::blocking::Response,
++    part_path: &Path,
++    total_size: Option<u64>,
++    progress_callback: &Option<ProgressCallback>,
++) -> Result<u64, DownloadError> {
++    let mut file = fs::File::create(part_path)
          .map_err(|e| DownloadError::FileCreationError(e.to_string()))?;
      let mut downloaded: u64 = 0;
      // Read response in chunks
--    let mut reader = response;
      let mut buffer = vec![0u8; 8192];
      loop {
--        let bytes_read = match reader.read(&mut buffer) {
++        let bytes_read = match response.read(&mut buffer) {
              Ok(0) => break, // EOF
              Ok(n) => n,
              Err(e) => return Err(DownloadError::NetworkError(e.to_string())),
          };
--
++
          file.write_all(&buffer[..bytes_read])
              .map_err(|e| DownloadError::WriteError(e.to_string()))?;
--
++
          downloaded += bytes_read as u64;
--
++
          // Report progress if callback is provided
--        if let Some(cb) = &progress_callback {
++        if let Some(cb) = progress_callback {
--            let progress = if total_size > 0 {
++            let progress = match total_size {
--                (downloaded as f32 / total_size as f32).min(1.0)
++                Some(total) if total > 0 => {
--            } else {
++                    (downloaded as f32 / total as f32).min(1.0)
--                0.0
++                }
++                _ => 0.0,
              };
              cb(progress);
          }
      file.flush().map_err(|e| DownloadError::WriteError(e.to_string()))?;
--    // Verify size matches
++    // Verify size matches; an incomplete file must not be kept
--    if total_size > 0 && downloaded != total_size {
++    if let Some(total) = total_size {
--        warn!("Downloaded size mismatch: expected {}, got {}", total_size, downloaded);
++        if downloaded != total {
--        // Don't fail, but log the discrepancy
++            warn!(
++                "Downloaded size mismatch: expected {}, got {}",
++                total, downloaded
++            );
++            return Err(DownloadError::ContentLengthMismatch);
++        }
      }
--    info!("Successfully downloaded {} bytes to {}", downloaded, dest.display());
++    Ok(downloaded)
--
--    Ok(())
  }
  /// Downloads an episode to its designated location
      download_path: &Path,
  ) -> Result<Vec<PathBuf>, DownloadError> {
      let mut downloaded_paths = Vec::new();
--
++
      for episode in episodes {
          // Use a no-op callback for now
          let path = download_episode(episode, download_path, Box::new(|_| {}))?;
          downloaded_paths.push(path);
      }
--
++
      Ok(downloaded_paths)
  }
  #[allow(dead_code)]
  pub fn remove_downloaded_episode(episode: &Episode, download_path: &Path) -> Result<(), DownloadError> {
      let save_path = episode.get_save_path(download_path);
--
++
      if save_path.exists() {
          fs::remove_file(&save_path)
              .map_err(|e| DownloadError::WriteError(e.to_string()))?;
          info!("Removed downloaded episode: {}", episode.title);
      }
--
++
      Ok(())
  }
          assert!(!needs_download(&episode, download_path));
      }
++    #[test]
++    fn test_needs_download_part_file_only() {
++        let episode = Episode::new(
++            "ep-123",
++            "Test Episode",
++            "https://example.com/episode.mp3",
++            Utc::now(),
++        );
++        let temp_dir = tempdir().unwrap();
++        let download_path = temp_dir.path();
++
++        // Only an interrupted (.part) download exists: it must be re-downloaded
++        let part_path = part_file_path(&download_path.join(&episode.audio_filename));
++        fs::write(&part_path, "partial content").unwrap();
++
++        assert!(needs_download(&episode, download_path));
++    }
++
++    #[test]
++    fn test_part_file_path() {
++        let dest = Path::new("downloads").join("episode.mp3");
++        let part = part_file_path(&dest);
++        assert_eq!(part, Path::new("downloads").join("episode.mp3.part"));
++        assert_eq!(part.file_name().unwrap().to_string_lossy(), "episode.mp3.part");
++    }
++
      #[test]
      fn test_get_download_dir() {
          let path = PathBuf::from("/downloads/podcasts");