Skip to content

Commit 9f9f36c

Browse files
committed
conditional clean on extract project
1 parent b0bc770 commit 9f9f36c

File tree

5 files changed

+164
-53
lines changed

5 files changed

+164
-53
lines changed

plugins/python3/src/plugin.rs

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,7 @@ mod test {
392392

393393
#[test]
394394
fn finds_project_dir_in_zip() {
395+
init();
395396
let file = File::open("tests/data/PythonProject.zip").unwrap();
396397
let mut zip = ZipArchive::new(file).unwrap();
397398
let dir = Python3Plugin::find_project_dir_in_zip(&mut zip).unwrap();
@@ -400,9 +401,95 @@ mod test {
400401

401402
#[test]
402403
fn doesnt_find_project_dir_in_zip() {
404+
init();
403405
let file = File::open("tests/data/PythonWithoutSrc.zip").unwrap();
404406
let mut zip = ZipArchive::new(file).unwrap();
405407
let dir = Python3Plugin::find_project_dir_in_zip(&mut zip);
406408
assert!(dir.is_err());
407409
}
410+
411+
#[test]
412+
fn extracts_project() {
413+
init();
414+
let plugin = Python3Plugin::new();
415+
let archive = Path::new("tests/data/student_exercise.zip");
416+
let temp = tempfile::tempdir().unwrap();
417+
assert!(!temp.path().join("src/source.py").exists());
418+
plugin.extract_project(archive, temp.path(), false).unwrap();
419+
assert!(temp.path().join("src/source.py").exists());
420+
assert!(temp.path().join("test/test.py").exists());
421+
assert!(temp.path().join("tmc/tmc").exists());
422+
}
423+
424+
#[test]
425+
fn extracts_project_over_existing() {
426+
init();
427+
let plugin = Python3Plugin::new();
428+
let archive = Path::new("tests/data/student_exercise.zip");
429+
let temp = copy_test("tests/data/student_exercise");
430+
assert_eq!(
431+
fs::read_to_string(temp.path().join("src/source.py")).unwrap(),
432+
"NEW"
433+
);
434+
assert_eq!(
435+
fs::read_to_string(temp.path().join("test/test.py")).unwrap(),
436+
"NEW"
437+
);
438+
assert_eq!(
439+
fs::read_to_string(temp.path().join("tmc/tmc")).unwrap(),
440+
"NEW"
441+
);
442+
plugin.extract_project(archive, temp.path(), false).unwrap();
443+
assert_eq!(
444+
fs::read_to_string(temp.path().join("src/source.py")).unwrap(),
445+
"NEW"
446+
);
447+
assert_eq!(
448+
fs::read_to_string(temp.path().join("test/test.py")).unwrap(),
449+
"OLD"
450+
);
451+
assert_eq!(
452+
fs::read_to_string(temp.path().join("tmc/tmc")).unwrap(),
453+
"OLD"
454+
);
455+
assert!(temp.path().join("src/new.py").exists());
456+
assert!(temp.path().join("test/new.py").exists());
457+
assert!(temp.path().join("tmc/new").exists());
458+
}
459+
460+
#[test]
461+
fn extracts_project_over_existing_clean() {
462+
init();
463+
let plugin = Python3Plugin::new();
464+
let archive = Path::new("tests/data/student_exercise.zip");
465+
let temp = copy_test("tests/data/student_exercise");
466+
assert_eq!(
467+
fs::read_to_string(temp.path().join("src/source.py")).unwrap(),
468+
"NEW"
469+
);
470+
assert_eq!(
471+
fs::read_to_string(temp.path().join("test/test.py")).unwrap(),
472+
"NEW"
473+
);
474+
assert_eq!(
475+
fs::read_to_string(temp.path().join("tmc/tmc")).unwrap(),
476+
"NEW"
477+
);
478+
plugin.extract_project(archive, temp.path(), true).unwrap();
479+
assert_eq!(
480+
fs::read_to_string(temp.path().join("src/source.py")).unwrap(),
481+
"NEW"
482+
);
483+
assert_eq!(
484+
fs::read_to_string(temp.path().join("test/test.py")).unwrap(),
485+
"OLD"
486+
);
487+
assert_eq!(
488+
fs::read_to_string(temp.path().join("tmc/tmc")).unwrap(),
489+
"OLD"
490+
);
491+
assert!(temp.path().join("src/new.py").exists());
492+
assert!(!temp.path().join("test/new.py").exists());
493+
assert!(!temp.path().join("tmc/new").exists());
494+
}
408495
}

tmc-langs-cli/src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ fn run() -> Result<()> {
141141
let output_path = matches.value_of("output-path").unwrap();
142142
let output_path = Path::new(output_path);
143143

144-
task_executor::extract_project(archive_path, output_path).with_context(|| {
144+
task_executor::extract_project(archive_path, output_path, true).with_context(|| {
145145
format!("Failed to extract project at {}", output_path.display())
146146
})?;
147147

tmc-langs-core/src/tmc_core.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ impl TmcCore {
290290
progress += step;
291291

292292
self.report_progress("Extracting exercise...", StatusType::Extracting, progress);
293-
task_executor::extract_project(zip_file.path(), target)?;
293+
task_executor::extract_project(zip_file.path(), target, true)?;
294294
progress += step;
295295
}
296296
self.report_complete("Finished downloading and extracting exercises.");
@@ -698,7 +698,7 @@ impl TmcCore {
698698
pub fn download_model_solution(&self, solution_download_url: Url, target: &Path) -> Result<()> {
699699
let zip_file = NamedTempFile::new().map_err(CoreError::TempFile)?;
700700
self.download_from(solution_download_url, zip_file.path())?;
701-
task_executor::extract_project(zip_file.path(), target)?;
701+
task_executor::extract_project(zip_file.path(), target, false)?;
702702
Ok(())
703703
}
704704

tmc-langs-framework/src/plugin.rs

Lines changed: 52 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -158,22 +158,30 @@ pub trait LanguagePlugin {
158158
///
159159
/// This will overwrite any existing files as long as they are not specified as student files
160160
/// by the language dependent student file policy.
161-
fn extract_project(&self, compressed_project: &Path, target_location: &Path) -> Result<()> {
161+
fn extract_project(
162+
&self,
163+
compressed_project: &Path,
164+
target_location: &Path,
165+
clean: bool,
166+
) -> Result<()> {
162167
let policy = Self::get_student_file_policy(target_location);
163-
let zip = compressed_project;
164-
let target = target_location;
165168

166-
log::debug!("Unzipping {} to {}", zip.display(), target.display());
169+
log::debug!(
170+
"Unzipping {} to {}",
171+
compressed_project.display(),
172+
target_location.display()
173+
);
167174

168-
let file = File::open(zip).map_err(|e| TmcError::OpenFile(zip.to_path_buf(), e))?;
175+
let file = File::open(compressed_project)
176+
.map_err(|e| TmcError::OpenFile(compressed_project.to_path_buf(), e))?;
169177
let mut zip_archive = ZipArchive::new(file)?;
170178

179+
// find the exercise root directory inside the archive
171180
let project_dir = Self::find_project_dir_in_zip(&mut zip_archive)?;
172181
log::debug!("Project dir in zip: {}", project_dir.display());
173182

174183
let tmc_project_yml = policy.get_tmc_project_yml()?;
175184

176-
// for each file in the zip, contains its path if unzipped
177185
// used to clean non-student files not in the zip later
178186
let mut unzip_paths = HashSet::new();
179187

@@ -187,18 +195,14 @@ pub trait LanguagePlugin {
187195
continue;
188196
}
189197
};
190-
let path_in_target = target.join(&relative);
198+
let path_in_target = target_location.join(&relative);
191199
log::trace!("processing {:?} -> {:?}", file_path, path_in_target);
200+
unzip_paths.insert(path_in_target.clone());
192201

193202
if file.is_dir() {
194203
log::trace!("creating {:?}", path_in_target);
195204
fs::create_dir_all(&path_in_target)
196205
.map_err(|e| TmcError::CreateDir(path_in_target.clone(), e))?;
197-
unzip_paths.insert(
198-
path_in_target
199-
.canonicalize()
200-
.map_err(|e| TmcError::Canonicalize(path_in_target.clone(), e))?,
201-
);
202206
} else {
203207
let mut write = true;
204208
let mut file_contents = vec![];
@@ -218,8 +222,11 @@ pub trait LanguagePlugin {
218222
.read_to_end(&mut target_file_contents)
219223
.map_err(|e| TmcError::FileRead(path_in_target.clone(), e))?;
220224
if file_contents == target_file_contents
221-
|| (policy.is_student_file(&path_in_target, &target, &tmc_project_yml)?
222-
&& !policy.is_updating_forced(&relative, &tmc_project_yml)?)
225+
|| (policy.is_student_file(
226+
&path_in_target,
227+
&target_location,
228+
&tmc_project_yml,
229+
)? && !policy.is_updating_forced(&relative, &tmc_project_yml)?)
223230
{
224231
write = false;
225232
}
@@ -237,37 +244,41 @@ pub trait LanguagePlugin {
237244
.map_err(|e| TmcError::Write(path_in_target.clone(), e))?;
238245
}
239246
}
240-
unzip_paths.insert(
241-
path_in_target
242-
.canonicalize()
243-
.map_err(|e| TmcError::Canonicalize(path_in_target.clone(), e))?,
244-
);
245247
}
246248

247-
// delete non-student files that were not in zip
248-
log::debug!("deleting non-student files not in zip");
249-
log::debug!("{:?}", unzip_paths);
250-
for entry in WalkDir::new(target).into_iter().filter_map(|e| e.ok()) {
251-
if !unzip_paths.contains(
252-
&entry
253-
.path()
254-
.canonicalize()
255-
.map_err(|e| TmcError::Canonicalize(entry.path().to_path_buf(), e))?,
256-
) && (policy.is_updating_forced(entry.path(), &tmc_project_yml)?
257-
|| !policy.is_student_file(entry.path(), &target, &tmc_project_yml)?)
249+
if clean {
250+
// delete non-student files that were not in zip
251+
log::debug!("deleting non-student files not in zip");
252+
log::debug!("{:?}", unzip_paths);
253+
for entry in WalkDir::new(target_location)
254+
.into_iter()
255+
.filter_map(|e| e.ok())
258256
{
259-
log::debug!("rm {} {}", entry.path().display(), target.display());
260-
if entry.path().is_dir() {
261-
// delete if empty
262-
if WalkDir::new(entry.path()).max_depth(1).into_iter().count() == 1 {
263-
log::debug!("deleting empty directory {}", entry.path().display());
264-
fs::remove_dir(entry.path())
265-
.map_err(|e| TmcError::RemoveDir(entry.path().to_path_buf(), e))?;
257+
if !unzip_paths.contains(entry.path())
258+
&& (policy.is_updating_forced(entry.path(), &tmc_project_yml)?
259+
|| !policy.is_student_file(
260+
entry.path(),
261+
&target_location,
262+
&tmc_project_yml,
263+
)?)
264+
{
265+
log::debug!(
266+
"rm {} {}",
267+
entry.path().display(),
268+
target_location.display()
269+
);
270+
if entry.path().is_dir() {
271+
// delete if empty
272+
if WalkDir::new(entry.path()).max_depth(1).into_iter().count() == 1 {
273+
log::debug!("deleting empty directory {}", entry.path().display());
274+
fs::remove_dir(entry.path())
275+
.map_err(|e| TmcError::RemoveDir(entry.path().to_path_buf(), e))?;
276+
}
277+
} else {
278+
log::debug!("removing file {}", entry.path().display());
279+
fs::remove_file(entry.path())
280+
.map_err(|e| TmcError::RemoveFile(entry.path().to_path_buf(), e))?;
266281
}
267-
} else {
268-
log::debug!("removing file {}", entry.path().display());
269-
fs::remove_file(entry.path())
270-
.map_err(|e| TmcError::RemoveFile(entry.path().to_path_buf(), e))?;
271282
}
272283
}
273284
}

tmc-langs-util/src/task_executor.rs

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,13 @@ pub fn is_exercise_root_directory(path: &Path) -> bool {
9191

9292
/// Finds the correct language plug-in for the given exercise path and calls `LanguagePlugin::extract_project`,
9393
/// If no language plugin matches, see `extract_project_overwrite`.
94-
pub fn extract_project(compressed_project: &Path, target_location: &Path) -> Result<(), TmcError> {
94+
pub fn extract_project(
95+
compressed_project: &Path,
96+
target_location: &Path,
97+
clean: bool,
98+
) -> Result<(), TmcError> {
9599
if let Ok(plugin) = get_language_plugin(target_location) {
96-
plugin.extract_project(compressed_project, target_location)?;
100+
plugin.extract_project(compressed_project, target_location, clean)?;
97101
} else {
98102
log::debug!(
99103
"no matching language plugin found for {}, overwriting",
@@ -203,15 +207,24 @@ impl Plugin {
203207
&self,
204208
cmpressed_project: &Path,
205209
target_location: &Path,
210+
clean: bool,
206211
) -> Result<(), TmcError> {
207212
match self {
208-
Self::CSharp(plugin) => plugin.extract_project(cmpressed_project, target_location),
209-
Self::Make(plugin) => plugin.extract_project(cmpressed_project, target_location),
210-
Self::Maven(plugin) => plugin.extract_project(cmpressed_project, target_location),
211-
Self::NoTests(plugin) => plugin.extract_project(cmpressed_project, target_location),
212-
Self::Python3(plugin) => plugin.extract_project(cmpressed_project, target_location),
213-
Self::R(plugin) => plugin.extract_project(cmpressed_project, target_location),
214-
Self::Ant(plugin) => plugin.extract_project(cmpressed_project, target_location),
213+
Self::CSharp(plugin) => {
214+
plugin.extract_project(cmpressed_project, target_location, clean)
215+
}
216+
Self::Make(plugin) => plugin.extract_project(cmpressed_project, target_location, clean),
217+
Self::Maven(plugin) => {
218+
plugin.extract_project(cmpressed_project, target_location, clean)
219+
}
220+
Self::NoTests(plugin) => {
221+
plugin.extract_project(cmpressed_project, target_location, clean)
222+
}
223+
Self::Python3(plugin) => {
224+
plugin.extract_project(cmpressed_project, target_location, clean)
225+
}
226+
Self::R(plugin) => plugin.extract_project(cmpressed_project, target_location, clean),
227+
Self::Ant(plugin) => plugin.extract_project(cmpressed_project, target_location, clean),
215228
}
216229
}
217230

0 commit comments

Comments
 (0)