even more refactoring

Author: Heliozoa

tmc-client/Cargo.toml

@@ -9,6 +9,7 @@ license = "MIT OR Apache-2.0"
 tmc-langs-plugins = { path = "../tmc-langs-plugins" }
 tmc-langs-util = { path = "../tmc-langs-util" }
 
+chrono = { version = "0.4", features = ["serde"] }
 dirs = "3"
 http = "0.2"
 lazy_static = "1"
@@ -17,7 +18,7 @@ oauth2 = { version = "4.0.0-alpha.3", features = ["reqwest"] }
 percent-encoding = "2"
 regex = "1"
 reqwest = { version = "0.11", default-features = false, features = ["blocking", "json", "rustls-tls", "multipart"] }
-schemars = "0.8"
+schemars = { version = "0.8", features = ["chrono"] }
 serde = { version = "1", features = ["derive"] }
 serde_json = "1"
 tempfile = "3"

tmc-client/src/error.rs

@@ -11,6 +11,7 @@ type TokenError = oauth2::RequestTokenError<
     oauth2::StandardErrorResponse<oauth2::basic::BasicErrorResponseType>,
 >;
 
+/// The main error type for tmc-client.
 #[derive(Debug, Error)]
 pub enum ClientError {
     // Arc

tmc-client/src/response.rs

@@ -1,5 +1,6 @@
 //! Contains types which model the JSON responses from tmc-server
 
+use chrono::{DateTime, FixedOffset};
 use lazy_static::lazy_static;
 use regex::Regex;
 use schemars::JsonSchema;
@@ -39,6 +40,7 @@ pub struct User {
     pub administrator: bool,
 }
 
+/// Organization information.
 #[derive(Debug, Serialize, Deserialize, JsonSchema)]
 pub struct Organization {
     pub name: String,
@@ -48,6 +50,7 @@ pub struct Organization {
     pub pinned: bool,
 }
 
+/// Information for a course.
 #[derive(Debug, Deserialize, Serialize, JsonSchema)]
 pub struct Course {
     pub id: usize,
@@ -61,6 +64,7 @@ pub struct Course {
     pub spyware_urls: Vec<String>,
 }
 
+/// Data for a course.
 #[derive(Debug, Deserialize, Serialize, JsonSchema)]
 pub struct CourseData {
     pub name: String,
@@ -69,7 +73,7 @@ pub struct CourseData {
     pub cache_version: Option<usize>,
     pub spreadsheet_key: Option<String>,
     pub hidden_if_registered_after: Option<String>,
-    pub refreshed_at: Option<String>,
+    pub refreshed_at: Option<DateTime<FixedOffset>>,
     pub locked_exercise_points_visible: bool,
     pub description: Option<String>,
     pub paste_visibility: Option<String>,
@@ -102,6 +106,7 @@ struct CourseDetailsInner {
     pub exercises: Vec<Exercise>,
 }
 
+/// Details for a course.
 #[derive(Debug, Deserialize, Serialize, JsonSchema)]
 #[serde(from = "CourseDetailsWrapper")]
 pub struct CourseDetails {
@@ -149,6 +154,7 @@ pub struct Exercise {
     pub solution_zip_url: Option<String>,
 }
 
+/// Exercise for a course.
 #[derive(Debug, Deserialize, Serialize, JsonSchema)]
 pub struct CourseExercise {
     pub id: usize,
@@ -195,9 +201,10 @@ pub struct AwardedPoint {
     user_id: usize,
     submission_id: usize,
     name: String,
-    created_at: String,
+    created_at: DateTime<FixedOffset>,
 }
 
+/// Details for an exercise.
 #[derive(Debug, Deserialize, Serialize, JsonSchema)]
 pub struct ExerciseDetails {
     pub course_name: String,
@@ -219,22 +226,23 @@ pub struct ExercisesDetails {
     pub checksum: String,
 }
 
+/// Exercise submission.
 #[derive(Debug, Deserialize, Serialize, JsonSchema)]
 pub struct Submission {
     pub id: usize,
     pub user_id: usize,
     pub pretest_error: Option<String>,
-    pub created_at: String,
+    pub created_at: DateTime<FixedOffset>,
     pub exercise_name: String,
     pub course_id: usize,
     pub processed: bool,
     pub all_tests_passed: bool,
     pub points: Option<String>,
-    pub processing_tried_at: Option<String>,
-    pub processing_began_at: Option<String>,
-    pub processing_completed_at: Option<String>,
+    pub processing_tried_at: Option<DateTime<FixedOffset>>,
+    pub processing_began_at: Option<DateTime<FixedOffset>>,
+    pub processing_completed_at: Option<DateTime<FixedOffset>>,
     pub times_sent_to_sandbox: usize,
-    pub processing_attempts_started_at: String,
+    pub processing_attempts_started_at: DateTime<FixedOffset>,
     pub params_json: Option<String>,
     pub requires_review: bool,
     pub requests_review: bool,
@@ -253,7 +261,7 @@ pub struct ExerciseSubmission {
     pub id: usize,
     pub user_id: usize,
     pub course_id: usize,
-    pub created_at: String,
+    pub created_at: DateTime<FixedOffset>,
     pub all_tests_passed: bool,
     pub points: Option<String>,
     pub submitted_zip_url: String,
@@ -263,6 +271,7 @@ pub struct ExerciseSubmission {
     pub requests_review: bool,
 }
 
+/// Exercise submission.
 #[derive(Debug, Deserialize, Serialize, JsonSchema, Clone)]
 pub struct NewSubmission {
     pub show_submission_url: String,
@@ -291,6 +300,7 @@ pub enum SandboxStatus {
     ProcessingOnSandbox,
 }
 
+/// Finished submission.
 #[derive(Debug, Deserialize, Serialize, PartialEq, Eq, JsonSchema)]
 pub struct SubmissionFinished {
     pub api_version: usize,
@@ -328,6 +338,7 @@ pub enum SubmissionStatus {
     Hidden,
 }
 
+/// Response to feedback.
 #[derive(Debug, Deserialize, Serialize, JsonSchema)]
 pub struct SubmissionFeedbackResponse {
     pub api_version: usize,
@@ -419,6 +430,7 @@ impl<'de> Visitor<'de> for SubmissionFeedbackKindVisitor {
     }
 }
 
+/// Code review.
 #[derive(Debug, Deserialize, Serialize, JsonSchema)]
 pub struct Review {
     pub submission_id: String,
@@ -431,10 +443,11 @@ pub struct Review {
     pub points_not_awarded: Vec<String>,
     pub url: String,
     pub update_url: String,
-    pub created_at: String,
+    pub created_at: DateTime<FixedOffset>,
     pub updated_at: String,
 }
 
+/// Updated exercises.
 #[derive(Debug, Deserialize, Serialize, JsonSchema)]
 pub struct UpdateResult {
     pub created: Vec<Exercise>,

tmc-client/src/tmc_client.rs

@@ -14,16 +14,14 @@ use serde::{Deserialize, Serialize};
 use std::collections::HashMap;
 use std::io::Write;
 use std::path::{Path, PathBuf};
-use std::sync::{
-    atomic::{AtomicUsize, Ordering},
-    Arc, Mutex,
-};
+use std::sync::Arc;
 use std::thread;
 use std::time::Duration;
 use tempfile::NamedTempFile;
 use tmc_langs_util::{file_util, progress_reporter, FileError};
 use walkdir::WalkDir;
 
+/// Authentication token.
 pub type Token =
     oauth2::StandardTokenResponse<oauth2::EmptyExtraTokenFields, oauth2::basic::BasicTokenType>;
 
@@ -37,6 +35,7 @@ pub enum ClientUpdateData {
 }
 
 /// A struct for interacting with the TestMyCode service, including authentication.
+#[derive(Clone)]
 pub struct TmcClient(Arc<TmcCore>);
 
 struct TmcCore {
@@ -194,164 +193,6 @@ impl TmcClient {
         self.organization(organization_slug)
     }
 
-    /// Downloads the given exercises. Overwrites existing exercises if they exist.
-    ///
-    /// # Errors
-    /// Returns an error if there's some problem reaching the API, or if the API returns an error.
-    /// The method extracts zip archives, which may fail.
-    ///
-    ///
-    /// # Examples
-    /// ```rust,no_run
-    /// use tmc_client::TmcClient;
-    /// use std::path::PathBuf;
-    ///
-    /// let client = TmcClient::new_in_config("https://tmc.mooc.fi".to_string(), "some_client".to_string(), "some_version".to_string()).unwrap();
-    /// // authenticate
-    /// client.download_or_update_exercises(vec![
-    ///     (1234, PathBuf::from("./exercises/1234")),
-    ///     (2345, PathBuf::from("./exercises/2345")),
-    /// ]);
-    /// ```
-    pub fn download_or_update_exercises(
-        &self,
-        exercises: Vec<(usize, PathBuf)>,
-    ) -> Result<(), ClientError> {
-        // todo: bit of a mess, refactor
-        let exercises_len = exercises.len();
-        start_stage(
-            exercises_len * 2 + 1, // each download progresses at 2 points, plus the final finishing step
-            format!("Downloading {} exercises", exercises_len),
-            None,
-        );
-
-        // for each exercise, check if there's already something on disk
-        // if yes, check if it needs updating
-        // if not, check if there's a previous submission
-        //   if yes, download it
-        //   if not, download the exercise template
-
-        let thread_count = exercises_len.min(4); // max 4 threads
-        let mut handles = vec![];
-        let exercises = Arc::new(Mutex::new(exercises));
-        let starting_download_counter = Arc::new(AtomicUsize::new(1));
-        let downloaded_counter = Arc::new(AtomicUsize::new(1));
-
-        // spawn threads
-        for _thread_id in 0..thread_count {
-            let client = Arc::clone(&self.0);
-            let exercises = Arc::clone(&exercises);
-            let starting_download_counter = Arc::clone(&starting_download_counter);
-            let downloaded_counter = Arc::clone(&downloaded_counter);
-
-            // each thread returns either a list of successful downloads, or a tuple of successful downloads and errors
-            type ThreadErr = (Vec<usize>, Vec<(usize, Box<ClientError>)>);
-            let handle = std::thread::spawn(move || -> Result<Vec<usize>, ThreadErr> {
-                let client_clone = TmcClient(client);
-                let mut downloaded = vec![];
-                let mut errors = vec![];
-
-                // repeat until out of exercises
-                loop {
-                    // acquiring mutex
-                    let mut exercises = exercises.lock().expect("the threads should never panic");
-                    let (exercise_id, target) = if let Some((id, path)) = exercises.pop() {
-                        (id, path)
-                    } else {
-                        // no exercises left, break loop and exit thread
-                        break;
-                    };
-                    drop(exercises);
-                    // dropped mutex
-
-                    let exercise_download_result = || -> Result<(), ClientError> {
-                        // TODO: do in memory without zip_file?
-                        let starting_download_count =
-                            starting_download_counter.fetch_add(1, Ordering::SeqCst);
-                        let zip_file = NamedTempFile::new().map_err(ClientError::TempFile)?;
-
-                        progress_stage(
-                            format!(
-                                "Downloading exercise {} to '{}'. ({} out of {})",
-                                exercise_id,
-                                target.display(),
-                                starting_download_count,
-                                exercises_len
-                            ),
-                            ClientUpdateData::ExerciseDownload {
-                                id: exercise_id,
-                                path: target.clone(),
-                            },
-                        );
-
-                        client_clone.download_exercise(exercise_id, zip_file.path())?;
-                        let downloaded_count = downloaded_counter.fetch_add(1, Ordering::SeqCst);
-                        tmc_langs_plugins::extract_project(zip_file, &target, true)?;
-                        progress_stage(
-                            format!(
-                                "Downloaded exercise {} to '{}'. ({} out of {})",
-                                exercise_id,
-                                target.display(),
-                                downloaded_count,
-                                exercises_len
-                            ),
-                            ClientUpdateData::ExerciseDownload {
-                                id: exercise_id,
-                                path: target,
-                            },
-                        );
-                        Ok(())
-                    }();
-
-                    // return underlying error with exercise id if download failed
-                    match exercise_download_result {
-                        Ok(()) => downloaded.push(exercise_id),
-                        Err(e) => {
-                            log::error!("Failed to download exercise {}", exercise_id);
-                            errors.push((exercise_id, Box::new(e)));
-                        }
-                    }
-                }
-
-                if errors.is_empty() {
-                    Ok(downloaded)
-                } else {
-                    Err((downloaded, errors))
-                }
-            });
-            handles.push(handle);
-        }
-
-        let mut successful = vec![];
-        let mut failed = vec![];
-        for handle in handles {
-            match handle.join().expect("the threads should never panic") {
-                Ok(s) => successful.extend(s),
-                Err((s, f)) => {
-                    successful.extend(s);
-                    failed.extend(f);
-                }
-            }
-        }
-
-        finish_stage(
-            format!(
-                "Successfully downloaded {} out of {} exercises.",
-                successful.len(),
-                exercises_len
-            ),
-            None,
-        );
-        if !failed.is_empty() {
-            Err(ClientError::IncompleteDownloadResult {
-                downloaded: successful,
-                failed,
-            })
-        } else {
-            Ok(())
-        }
-    }
-
     /// Fetches the course's information.
     ///
     /// # Errors
@@ -534,7 +375,7 @@ impl TmcClient {
                 }
             }
         }
-        self.download_or_update_exercises(vec![(exercise_id, exercise_path)])
+        self.download_exercise(exercise_id, &exercise_path)
     }
 
     pub fn download_old_submission(
@@ -862,7 +703,7 @@ mod test {
         let target = temp_dir.path().join("temp");
         assert!(!target.exists());
         let exercises = vec![(1234, target.clone())];
-        client.download_or_update_exercises(exercises).unwrap();
+        //client.download_or_update_exercises(exercises).unwrap();
         assert!(target.join("src/main/java/Hiekkalaatikko.java").exists());
     }
 

tmc-client/src/tmc_client/api.rs

@@ -639,11 +639,7 @@ impl TmcClient {
         todo!("needs admin?");
     }
 
-    pub(super) fn download_exercise(
-        &self,
-        exercise_id: usize,
-        target: &Path,
-    ) -> Result<(), ClientError> {
+    pub fn download_exercise(&self, exercise_id: usize, target: &Path) -> Result<(), ClientError> {
         let url_tail = format!("core/exercises/{}/download", exercise_id);
         self.download(&url_tail, target)
     }