From 491236220b6f2efda75abf540428b0ae979a635b Mon Sep 17 00:00:00 2001 From: Jaakko Nakaza Date: Wed, 30 Oct 2024 16:50:43 +0200 Subject: [PATCH] Don't allow running multiple instances of some services at once and add better error handling for module imports --- .../services/ModuleImportExport.kt | 120 ++++++++++++------ .../services/exercise/SubmitExercise.kt | 11 ++ .../resources/messages/resources.properties | 2 + 3 files changed, 93 insertions(+), 40 deletions(-) diff --git a/src/main/kotlin/fi/aalto/cs/apluscourses/services/ModuleImportExport.kt b/src/main/kotlin/fi/aalto/cs/apluscourses/services/ModuleImportExport.kt index 583954c5d..a031bc13f 100644 --- a/src/main/kotlin/fi/aalto/cs/apluscourses/services/ModuleImportExport.kt +++ b/src/main/kotlin/fi/aalto/cs/apluscourses/services/ModuleImportExport.kt @@ -7,11 +7,13 @@ import com.intellij.openapi.fileChooser.FileChooserDescriptor import com.intellij.openapi.module.ModuleManager import com.intellij.openapi.project.Project import com.intellij.openapi.project.guessModuleDir +import com.intellij.openapi.ui.DialogBuilder import com.intellij.openapi.util.Comparing import com.intellij.openapi.util.io.FileUtil import com.intellij.openapi.vfs.VirtualFile import com.intellij.openapi.vfs.toNioPathOrNull import com.intellij.platform.ide.progress.withModalProgress +import com.intellij.ui.dsl.builder.panel import fi.aalto.cs.apluscourses.MyBundle.message import fi.aalto.cs.apluscourses.api.APlusApi import fi.aalto.cs.apluscourses.model.component.Component @@ -21,12 +23,14 @@ import fi.aalto.cs.apluscourses.model.people.User import fi.aalto.cs.apluscourses.notifications.ModuleExportedNotification import fi.aalto.cs.apluscourses.services.course.CourseManager import fi.aalto.cs.apluscourses.ui.module.ExportModuleDialog +import fi.aalto.cs.apluscourses.utils.CoursesLogger import fi.aalto.cs.apluscourses.utils.Version import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import java.io.FileOutputStream +import java.util.concurrent.atomic.AtomicBoolean import java.util.zip.ZipEntry import java.util.zip.ZipFile import java.util.zip.ZipOutputStream @@ -36,71 +40,107 @@ class ModuleImportExport( val project: Project, val cs: CoroutineScope ) { + private val running = AtomicBoolean(false) + fun importModules() { + if (!running.compareAndSet(false, true)) { + return + } + val modulesWithErrors = mutableListOf() FileChooser.chooseFiles(FileChooserDescriptorImpl(), project, null) { files -> cs.launch { withModalProgress(project, message("ui.ModuleImportExport.import.progress")) { + files.forEach { file -> - val zip = ZipFile(file.toNioPath().toFile()) - val desiredModuleName = file.nameWithoutExtension - - val zipFile = FileUtil.createTempDirectory("apluscourses", "modules") - zip.entries().asSequence().forEach { entry -> - val entryName = if (entry.name.endsWith(".iml")) { - "$desiredModuleName.iml" - } else { - entry.name + try { + val zip = ZipFile(file.toNioPath().toFile()) + val desiredModuleName = file.nameWithoutExtension + + val zipFile = FileUtil.createTempDirectory("apluscourses", "modules") + zip.entries().asSequence().forEach { entry -> + val entryName = if (entry.name.endsWith(".iml")) { + "$desiredModuleName.iml" + } else { + entry.name + } + val entryFile = zipFile.resolve(entryName) + + if (entry.isDirectory) { + entryFile.mkdirs() + } else { + entryFile.parentFile.mkdirs() + entryFile.writeBytes(zip.getInputStream(entry).readBytes()) + + } } - val entryFile = zipFile.resolve(entryName) - if (entry.isDirectory) { - entryFile.mkdirs() - } else { - entryFile.parentFile.mkdirs() - entryFile.writeBytes(zip.getInputStream(entry).readBytes()) + // Create and load module + val module = Module( + name = desiredModuleName, + zipUrl = "", + changelog = null, + latestVersion = Version.DEFAULT, + language = null, + project = project + ) - } - } + val moduleDir = module.fullPath.toFile() + zipFile.copyRecursively(moduleDir, overwrite = true) - // Create and load module - val module = Module( - name = desiredModuleName, - zipUrl = "", - changelog = null, - latestVersion = Version.DEFAULT, - language = null, - project = project - ) - - val moduleDir = module.fullPath.toFile() - zipFile.copyRecursively(moduleDir, overwrite = true) - - withContext(Dispatchers.EDT) { - if (module.updateAndGetStatus() == Component.Status.NOT_LOADED) { - module.loadToProject() + withContext(Dispatchers.EDT) { + if (module.updateAndGetStatus() == Component.Status.NOT_LOADED) { + module.loadToProject() + } } - } - module.downloadAndInstall() - val missingDependencies = CourseManager.getInstance(project).getMissingDependencies(module) - missingDependencies.forEach { it.downloadAndInstall() } + module.downloadAndInstall() + val missingDependencies = CourseManager.getInstance(project).getMissingDependencies(module) + missingDependencies.forEach { it.downloadAndInstall() } - zipFile.deleteRecursively() + zipFile.deleteRecursively() + } catch (e: Exception) { + CoursesLogger.error("Failed to import module", e) + modulesWithErrors.add(file.nameWithoutExtension) + } } + CourseManager.getInstance(project).refreshModuleStatuses() } + if (modulesWithErrors.isNotEmpty()) { + withContext(Dispatchers.EDT) { + DialogBuilder(project) + .title(message("ui.ModuleImportExport.import.error.title")) + .centerPanel( + panel { + row { + text( + message("ui.ModuleImportExport.import.error.content", modulesWithErrors + .map { "
  • ${it}
    • " } + .joinToString("")) + ) + } + } + ) + .show() + } + } } } + running.set(false) } fun exportModule() { cs.launch { + if (!running.compareAndSet(false, true)) { + return@launch + } exportModuleSuspend() + running.set(false) } } - suspend fun exportModuleSuspend() { + private suspend fun exportModuleSuspend() { val (student, modules, groups) = withModalProgress( project, message("ui.ModuleImportExport.export.loading") @@ -198,7 +238,7 @@ class ModuleImportExport( return false } - val extension = file.getExtension() + val extension = file.extension return Comparing.strEqual(extension, "zip") } } diff --git a/src/main/kotlin/fi/aalto/cs/apluscourses/services/exercise/SubmitExercise.kt b/src/main/kotlin/fi/aalto/cs/apluscourses/services/exercise/SubmitExercise.kt index 660a8ff33..67e5a36a4 100644 --- a/src/main/kotlin/fi/aalto/cs/apluscourses/services/exercise/SubmitExercise.kt +++ b/src/main/kotlin/fi/aalto/cs/apluscourses/services/exercise/SubmitExercise.kt @@ -25,6 +25,7 @@ import fi.aalto.cs.apluscourses.utils.FileUtil import kotlinx.coroutines.* import java.io.IOException import java.nio.file.Path +import java.util.concurrent.atomic.AtomicBoolean @Service(Service.Level.PROJECT) class SubmitExercise( @@ -32,9 +33,14 @@ class SubmitExercise( private val cs: CoroutineScope ) { private val submissionInfos: MutableMap = HashMap() + private val submitting = AtomicBoolean(false) fun submit(exercise: Exercise) { cs.launch { + if (!submitting.compareAndSet(false, true)) { + CoursesLogger.warn("Already submitting exercise") + return@launch + } try { CoursesLogger.info("Submitting $exercise") var submissionInfo = submissionInfos[exercise.id] @@ -57,12 +63,14 @@ class SubmitExercise( if (!submittable) { CoursesLogger.warn("$exercise not submittable") notifier.notify(NotSubmittableNotification(), project) + submitting.set(false) return@launch } val course = CourseManager.course(project) if (course == null) { CoursesLogger.error("Course not found") + submitting.set(false) return@launch } @@ -97,6 +105,7 @@ class SubmitExercise( ) }) { CoursesLogger.info("Duplicate submission detected and user chose to cancel") + submitting.set(false) return@launch } @@ -126,6 +135,7 @@ class SubmitExercise( if (withContext(Dispatchers.EDT) { !submissionDialog.showAndGet() }) { + submitting.set(false) return@launch } @@ -147,6 +157,7 @@ class SubmitExercise( notifier.notify(MissingModuleNotification(), project) } CoursesLogger.debug("Finished submitting exercise") + submitting.set(false) } } diff --git a/src/main/resources/messages/resources.properties b/src/main/resources/messages/resources.properties index c8b09dbd2..b53704042 100755 --- a/src/main/resources/messages/resources.properties +++ b/src/main/resources/messages/resources.properties @@ -161,6 +161,8 @@ ui.ExportModuleDialog.error.noFilename=Please enter a file name ui.ModuleImportExport.import.title=Import Module ui.ModuleImportExport.import.description=Choose module zip file(s) to import ui.ModuleImportExport.import.progress=Importing modules +ui.ModuleImportExport.import.error.title=Error Importing Modules +ui.ModuleImportExport.import.error.content=Could not import the following modules:
      {0}

    They might not have been properly exported or are not IntelliJ modules. Try importing them manually. ui.ModuleImportExport.export.loading=Loading… ui.ModuleImportExport.export.submitter=Submitter: {0} ({1}) # News