simplified project dir discovery in zip

Heliozoa · Heliozoa · commit adcaeac8047d · 2020-08-07T12:32:31.000+03:00
diff --git a/plugins/java/src/ant.rs b/plugins/java/src/ant.rs
@@ -109,40 +109,6 @@ impl LanguagePlugin for AntPlugin {
             || path.join("test").exists() && path.join("src").exists()
     }
 
-    /// Tries to find a directory which contains both a src and a test directory.
-    fn find_project_dir_in_zip<R: Read + Seek>(
-        zip_archive: &mut ZipArchive<R>,
-    ) -> Result<PathBuf, TmcError> {
-        let mut dirs_with_src = HashSet::new();
-        let mut dirs_with_test = HashSet::new();
-        for i in 0..zip_archive.len() {
-            // zips don't necessarily contain entries for intermediate directories,
-            // so we need to check every path for src and test
-            let file = zip_archive.by_index(i)?;
-            let file_path = file.sanitized_name();
-            // todo: do in one pass somehow
-            if file_path.components().any(|c| c.as_os_str() == "src") {
-                let path: PathBuf = file_path
-                    .components()
-                    .take_while(|c| c.as_os_str() != "src")
-                    .collect();
-                dirs_with_src.insert(path);
-            }
-            if file_path.components().any(|c| c.as_os_str() == "test") {
-                let path: PathBuf = file_path
-                    .components()
-                    .take_while(|c| c.as_os_str() != "test")
-                    .collect();
-                dirs_with_test.insert(path);
-            }
-        }
-        // return any dir with src and test
-        if let Some(val) = dirs_with_src.intersection(&dirs_with_test).next() {
-            return Ok(val.clone());
-        }
-        Err(TmcError::NoProjectDirInZip)
-    }
-
     fn get_student_file_policy(project_path: &Path) -> Self::StudentFilePolicy {
         AntStudentFilePolicy::new(project_path.to_path_buf())
     }
@@ -528,7 +494,7 @@ mod test {
 
     #[test]
     fn doesnt_find_project_dir_in_zip() {
-        let file = File::open("tests/data/AntWithoutTest.zip").unwrap();
+        let file = File::open("tests/data/AntWithoutSrc.zip").unwrap();
         let mut zip = ZipArchive::new(file).unwrap();
         let dir = AntPlugin::find_project_dir_in_zip(&mut zip);
         assert!(dir.is_err());
diff --git a/plugins/java/src/maven.rs b/plugins/java/src/maven.rs
@@ -103,22 +103,6 @@ impl LanguagePlugin for MavenPlugin {
         path.join("pom.xml").exists()
     }
 
-    /// Tries to find a directory which contains a pom.xml file.
-    fn find_project_dir_in_zip<R: Read + Seek>(
-        zip_archive: &mut ZipArchive<R>,
-    ) -> Result<PathBuf, TmcError> {
-        for i in 0..zip_archive.len() {
-            let file = zip_archive.by_index(i)?;
-            let file_path = file.sanitized_name();
-            if file_path.file_name() == Some(OsStr::new("pom.xml")) {
-                if let Some(parent) = file_path.parent() {
-                    return Ok(parent.to_path_buf());
-                }
-            }
-        }
-        Err(TmcError::NoProjectDirInZip)
-    }
-
     fn get_student_file_policy(project_path: &Path) -> Self::StudentFilePolicy {
         MavenStudentFilePolicy::new(project_path.to_path_buf())
     }
@@ -452,7 +436,7 @@ mod test {
 
     #[test]
     fn doesnt_find_project_dir_in_zip() {
-        let file = File::open("tests/data/MavenWithoutPom.zip").unwrap();
+        let file = File::open("tests/data/MavenWithoutSrc.zip").unwrap();
         let mut zip = ZipArchive::new(file).unwrap();
         let dir = MavenPlugin::find_project_dir_in_zip(&mut zip);
         assert!(dir.is_err());
diff --git a/plugins/java/tests/data/AntWithoutSrc.zip b/plugins/java/tests/data/AntWithoutSrc.zip
diff --git a/plugins/java/tests/data/MavenWithoutSrc.zip b/plugins/java/tests/data/MavenWithoutSrc.zip
diff --git a/plugins/make/src/plugin.rs b/plugins/make/src/plugin.rs
@@ -256,22 +256,6 @@ impl LanguagePlugin for MakePlugin {
         path.join("Makefile").is_file()
     }
 
-    /// Tries to find a directory with a Makefile in it.
-    fn find_project_dir_in_zip<R: Read + Seek>(
-        zip_archive: &mut ZipArchive<R>,
-    ) -> Result<PathBuf, TmcError> {
-        for i in 0..zip_archive.len() {
-            let file = zip_archive.by_index(i)?;
-            let file_path = file.sanitized_name();
-            if file_path.file_name() == Some(OsStr::new("Makefile")) {
-                if let Some(parent) = file_path.parent() {
-                    return Ok(parent.to_path_buf());
-                }
-            }
-        }
-        Err(TmcError::NoProjectDirInZip)
-    }
-
     // does not check for success
     fn clean(&self, path: &Path) -> Result<(), TmcError> {
         let mut command = TmcCommand::new("make");
@@ -419,7 +403,7 @@ mod test {
 
     #[test]
     fn doesnt_find_project_dir_in_zip() {
-        let file = File::open("tests/data/MakeWithoutMakefile.zip").unwrap();
+        let file = File::open("tests/data/MakeWithoutSrc.zip").unwrap();
         let mut zip = ZipArchive::new(file).unwrap();
         let dir = MakePlugin::find_project_dir_in_zip(&mut zip);
         assert!(dir.is_err());
diff --git a/plugins/make/tests/data/MakeWithoutSrc.zip b/plugins/make/tests/data/MakeWithoutSrc.zip
diff --git a/plugins/python3/src/plugin.rs b/plugins/python3/src/plugin.rs
@@ -109,22 +109,6 @@ impl LanguagePlugin for Python3Plugin {
         setup.exists() || requirements.exists() || test.exists() || tmc.exists()
     }
 
-    // returns the parent of the src directory, if one is found
-    fn find_project_dir_in_zip<R: Read + Seek>(
-        zip_archive: &mut ZipArchive<R>,
-    ) -> Result<PathBuf, TmcError> {
-        for i in 0..zip_archive.len() {
-            let file = zip_archive.by_index(i)?;
-            let file_path = file.sanitized_name();
-            if file_path.file_name() == Some(OsStr::new("src")) {
-                if let Some(parent) = file_path.parent() {
-                    return Ok(parent.to_path_buf());
-                }
-            }
-        }
-        Err(TmcError::NoProjectDirInZip)
-    }
-
     fn clean(&self, exercise_path: &Path) -> Result<(), TmcError> {
         for entry in WalkDir::new(exercise_path)
             .into_iter()
diff --git a/plugins/r/src/plugin.rs b/plugins/r/src/plugin.rs
@@ -131,21 +131,17 @@ impl LanguagePlugin for RPlugin {
         zip_archive: &mut ZipArchive<R>,
     ) -> Result<PathBuf, TmcError> {
         for i in 0..zip_archive.len() {
+            // zips don't necessarily contain entries for intermediate directories,
+            // so we need to check every path for R
             let file = zip_archive.by_index(i)?;
             let file_path = file.sanitized_name();
-            if file_path.file_name() == Some(OsStr::new("R")) {
-                if let Some(parent) = file_path.parent() {
-                    return Ok(parent.to_path_buf());
-                }
-            }
-            if file_path.file_name() == Some(OsStr::new("testthat")) {
-                if let Some(init_parent) = file_path.parent() {
-                    if init_parent.file_name() == Some(OsStr::new("tests")) {
-                        if let Some(test_parent) = init_parent.parent() {
-                            return Ok(test_parent.to_path_buf());
-                        }
-                    }
-                }
+            // todo: do in one pass somehow
+            if file_path.components().any(|c| c.as_os_str() == "R") {
+                let path: PathBuf = file_path
+                    .components()
+                    .take_while(|c| c.as_os_str() != "R")
+                    .collect();
+                return Ok(path);
             }
         }
         Err(TmcError::NoProjectDirInZip)
diff --git a/tmc-langs-framework/src/plugin.rs b/tmc-langs-framework/src/plugin.rs
@@ -360,7 +360,25 @@ pub trait LanguagePlugin {
 
     /// Searches the zip for a valid project directory.
     /// Note that the returned path may not actually have an entry in the zip.
-    fn find_project_dir_in_zip<R: Read + Seek>(zip_archive: &mut ZipArchive<R>) -> Result<PathBuf>;
+    /// The default implementation tries to find a directory that contains a "src" directory,
+    /// which may be sufficient for some languages.
+    fn find_project_dir_in_zip<R: Read + Seek>(zip_archive: &mut ZipArchive<R>) -> Result<PathBuf> {
+        for i in 0..zip_archive.len() {
+            // zips don't necessarily contain entries for intermediate directories,
+            // so we need to check every path for src
+            let file = zip_archive.by_index(i)?;
+            let file_path = file.sanitized_name();
+            // todo: do in one pass somehow
+            if file_path.components().any(|c| c.as_os_str() == "src") {
+                let path: PathBuf = file_path
+                    .components()
+                    .take_while(|c| c.as_os_str() != "src")
+                    .collect();
+                return Ok(path);
+            }
+        }
+        Err(TmcError::NoProjectDirInZip)
+    }
 
     /// Tells if there's a valid exercise in this path.
     fn is_exercise_type_correct(path: &Path) -> bool;
