From ce1cbdc06516983f0bdba72983306427cda37e0e Mon Sep 17 00:00:00 2001 From: Oleksandr Liemiahov Date: Wed, 20 May 2026 12:31:54 +0300 Subject: [PATCH 01/12] fix github actions build --- intellij-plugin/hs-features/ai-debugger-core/build.gradle.kts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/intellij-plugin/hs-features/ai-debugger-core/build.gradle.kts b/intellij-plugin/hs-features/ai-debugger-core/build.gradle.kts index 07e81447e..dc7f490dc 100644 --- a/intellij-plugin/hs-features/ai-debugger-core/build.gradle.kts +++ b/intellij-plugin/hs-features/ai-debugger-core/build.gradle.kts @@ -18,10 +18,12 @@ dependencies { api(libs.educational.ml.library.core) { excludeKotlinDeps() exclude(group = "net.java.dev.jna") + exclude(group = "it.unimi.dsi", module = "fastutil-core") } api(libs.educational.ml.library.debugger) { excludeKotlinDeps() exclude(group = "net.java.dev.jna") + exclude(group = "it.unimi.dsi", module = "fastutil-core") } compileOnly(libs.kotlinx.serialization) { @@ -29,4 +31,4 @@ dependencies { } testImplementation(project(":intellij-plugin:hs-core", "testOutput")) -} \ No newline at end of file +} From d04255ff9e46d6ec034c8fd8a435df11779d5dbf Mon Sep 17 00:00:00 2001 From: Oleksandr Liemiahov Date: Fri, 22 May 2026 13:08:35 +0300 Subject: [PATCH 02/12] fix github actions build --- .../format/remote/RemoteEduTaskYamlMixin.kt | 2 +- .../learning/framework/storage/UserChanges.kt | 2 +- .../academy/learning/SolutionLoaderBase.kt | 42 ++++-- .../academy/learning/VirtualFileExt.kt | 11 +- .../learning/courseFormat/ext/StudyItemExt.kt | 5 +- .../learning/courseFormat/ext/TaskExt.kt | 11 +- .../impl/FrameworkLessonManagerImpl.kt | 140 +++++++++++------- .../framework/impl/FrameworkStorage.kt | 18 ++- .../learning/framework/impl/UserChanges.kt | 20 ++- .../learning/handlers/handlersUtils.kt | 45 ++++-- .../academy/learning/stepik/api/StepikAPI.kt | 1 + .../HyperskillSubmissionFactory.kt | 1 - .../academy/learning/yaml/YamlLoader.kt | 8 +- .../student/StudentTaskChangeApplier.kt | 3 +- .../yaml/YamlErrorProcessingTest.kt | 3 +- .../FrameworkStorageMigrationTest.kt | 6 +- .../HyperskillCheckRemoteEduTaskTest.kt | 4 +- 17 files changed, 216 insertions(+), 106 deletions(-) diff --git a/hs-edu-format/src/org/hyperskill/academy/learning/yaml/format/remote/RemoteEduTaskYamlMixin.kt b/hs-edu-format/src/org/hyperskill/academy/learning/yaml/format/remote/RemoteEduTaskYamlMixin.kt index 942b24646..89210bcaf 100644 --- a/hs-edu-format/src/org/hyperskill/academy/learning/yaml/format/remote/RemoteEduTaskYamlMixin.kt +++ b/hs-edu-format/src/org/hyperskill/academy/learning/yaml/format/remote/RemoteEduTaskYamlMixin.kt @@ -19,6 +19,6 @@ import org.hyperskill.academy.learning.yaml.format.student.StudentTaskYamlMixin class RemoteEduTaskYamlMixin : StudentTaskYamlMixin() { @get:JsonProperty(CHECK_PROFILE) @set:JsonProperty(CHECK_PROFILE) - @get:JsonInclude(JsonInclude.Include.ALWAYS) + @get:JsonInclude(JsonInclude.Include.NON_EMPTY) var checkProfile: String = "" } diff --git a/hs-framework-storage/src/org/hyperskill/academy/learning/framework/storage/UserChanges.kt b/hs-framework-storage/src/org/hyperskill/academy/learning/framework/storage/UserChanges.kt index 665fe7aeb..a36128d39 100644 --- a/hs-framework-storage/src/org/hyperskill/academy/learning/framework/storage/UserChanges.kt +++ b/hs-framework-storage/src/org/hyperskill/academy/learning/framework/storage/UserChanges.kt @@ -137,7 +137,7 @@ sealed class Change { @Throws(IOException::class) constructor(input: DataInput) : super(input) override fun apply(state: MutableMap) { - state[path] = text + state -= path } } diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/SolutionLoaderBase.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/SolutionLoaderBase.kt index 7f3e35799..eb3a00d61 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/SolutionLoaderBase.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/SolutionLoaderBase.kt @@ -23,6 +23,7 @@ import org.hyperskill.academy.learning.courseFormat.CheckStatus import org.hyperskill.academy.learning.courseFormat.Course import org.hyperskill.academy.learning.courseFormat.FrameworkLesson import org.hyperskill.academy.learning.courseFormat.InMemoryTextualContents +import org.hyperskill.academy.learning.courseFormat.TaskFile import org.hyperskill.academy.learning.courseFormat.ext.* import org.hyperskill.academy.learning.courseFormat.tasks.Task import org.hyperskill.academy.learning.courseGeneration.GeneratorUtils @@ -303,12 +304,31 @@ abstract class SolutionLoaderBase(protected val project: Project) : Disposable { // storeOriginalTemplateFiles uses task.taskFiles which may have stale disk content. frameworkLessonManager.ensureTemplateFilesCached(task) - val solutionMap = taskSolutions.solutions.mapValues { it.value.text } + val solutionMap = taskSolutions.visibleNonTestSolutions(task) frameworkLessonManager.saveExternalChanges(task, solutionMap, taskSolutions.submissionId) - for (taskFile in task.taskFiles.values) { - val solution = taskSolutions.solutions[taskFile.name] ?: continue - taskFile.isVisible = solution.isVisible + var taskFilesChanged = false + for ((path, solution) in taskSolutions.solutions) { + if (EduUtilsKt.isTestsFile(task, path)) continue + + val taskFile = task.getTaskFile(path) + if (taskFile == null) { + if (!solution.isVisible) continue + + task.addTaskFile(TaskFile(path, solution.text).apply { + isVisible = solution.isVisible + isLearnerCreated = true + }) + taskFilesChanged = true + } + else if (taskFile.isVisible != solution.isVisible) { + taskFile.isVisible = solution.isVisible + taskFilesChanged = true + } + } + + if (taskFilesChanged) { + YamlFormatSynchronizer.saveItem(task) } } @@ -317,15 +337,14 @@ abstract class SolutionLoaderBase(protected val project: Project) : Disposable { for ((path, solution) in taskSolutions.solutions) { val taskFile = task.getTaskFile(path) - // Skip test files from submissions to prevent corrupted tests from being applied - // Test files should always come from step source (API), not from user submissions - // See ALT-10961: user submissions may contain stale test files from previous stages - if (taskFile != null && !taskFile.isLearnerCreated && taskFile.isTestFile) { + if (EduUtilsKt.isTestsFile(task, path)) { LOG.warn("Skipping test file '$path' from submission for task '${task.name}' - test files should come from API, not submissions") continue } if (taskFile == null) { + if (!solution.isVisible) continue + GeneratorUtils.createChildFile(project, taskDir, path, InMemoryTextualContents(solution.text)) val createdFile = task.getTaskFile(path) if (createdFile == null) { @@ -356,10 +375,15 @@ abstract class SolutionLoaderBase(protected val project: Project) : Disposable { val lesson = task.lesson if (lesson is FrameworkLesson) { val frameworkLessonManager = FrameworkLessonManager.getInstance(project) - val solutionMap = taskSolutions.solutions.mapValues { it.value.text } + val solutionMap = taskSolutions.visibleNonTestSolutions(task) frameworkLessonManager.saveExternalChanges(task, solutionMap, taskSolutions.submissionId) } } + + private fun TaskSolutions.visibleNonTestSolutions(task: Task): Map = + solutions + .filter { (path, solution) -> solution.isVisible && !EduUtilsKt.isTestsFile(task, path) } + .mapValues { (_, solution) -> solution.text } } protected data class Solution(val text: String, val isVisible: Boolean) diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/VirtualFileExt.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/VirtualFileExt.kt index d4aefe335..1a9f448fd 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/VirtualFileExt.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/VirtualFileExt.kt @@ -77,6 +77,10 @@ private fun FileEditor.setViewer(isViewer: Boolean) { private val FileEditor.loadingPanel: JBLoadingPanel? get() = UIUtil.findComponentOfType(component, JBLoadingPanel::class.java) +fun VirtualFile.findFileByRelativePathOrSelf(path: String): VirtualFile? { + return if (path.isEmpty()) this else findFileByRelativePath(path) +} + fun VirtualFile.getSection(project: Project): Section? { return getSection(project.toCourseInfoHolder()) } @@ -84,7 +88,7 @@ fun VirtualFile.getSection(project: Project): Section? { fun VirtualFile.getSection(holder: CourseInfoHolder): Section? { val course = holder.course ?: return null if (!isDirectory) return null - return if (holder.courseDir.findFileByRelativePath(course.customContentPath) == parent) course.getSection(name) else null + return if (holder.courseDir.findFileByRelativePathOrSelf(course.customContentPath) == parent) course.getSection(name) else null } fun VirtualFile.isSectionDirectory(project: Project): Boolean { @@ -104,7 +108,7 @@ fun VirtualFile.getLesson(holder: CourseInfoHolder): Lesson? { if (section != null) { return section.getLesson(name) } - return if (holder.courseDir.findFileByRelativePath(course.customContentPath) == parent) course.getLesson(name) else null + return if (holder.courseDir.findFileByRelativePathOrSelf(course.customContentPath) == parent) course.getLesson(name) else null } fun VirtualFile.isLessonDirectory(project: Project): Boolean { @@ -135,6 +139,9 @@ fun VirtualFile.getTask(project: Project): Task? { fun VirtualFile.getTask(holder: CourseInfoHolder): Task? { if (!isDirectory) return null val lesson: Lesson = parent?.getLesson(holder) ?: return null + if (lesson is FrameworkLesson && name == TASK) { + return lesson.currentTask() + } return lesson.getTask(name) } diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/courseFormat/ext/StudyItemExt.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/courseFormat/ext/StudyItemExt.kt index ac9b4c28e..23f49df85 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/courseFormat/ext/StudyItemExt.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/courseFormat/ext/StudyItemExt.kt @@ -5,6 +5,7 @@ import org.hyperskill.academy.coursecreator.StudyItemType import org.hyperskill.academy.coursecreator.StudyItemType.* import org.hyperskill.academy.learning.courseFormat.* import org.hyperskill.academy.learning.courseFormat.tasks.Task +import org.hyperskill.academy.learning.findFileByRelativePathOrSelf val StudyItem.studyItemType: StudyItemType get() { @@ -22,12 +23,12 @@ fun StudyItem.getDir(courseDir: VirtualFile): VirtualFile? { is Course -> courseDir is Section -> { val sectionParent = (parentOrNull as? StudyItem) ?: return null - courseDir.findFileByRelativePath(sectionParent.getPathToChildren())?.findChild(name) + courseDir.findFileByRelativePathOrSelf(sectionParent.getPathToChildren())?.findChild(name) } is Lesson -> { val lessonParent = (parentOrNull as? StudyItem) ?: return null - lessonParent.getDir(courseDir)?.findFileByRelativePath(lessonParent.getPathToChildren())?.findChild(name) + lessonParent.getDir(courseDir)?.findFileByRelativePathOrSelf(lessonParent.getPathToChildren())?.findChild(name) } is Task -> (parentOrNull as? Lesson) diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/courseFormat/ext/TaskExt.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/courseFormat/ext/TaskExt.kt index fb9c162bd..f9bedbc0d 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/courseFormat/ext/TaskExt.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/courseFormat/ext/TaskExt.kt @@ -49,11 +49,10 @@ val Task.dirName: String } val Task.targetDirName: String - get() = when (this) { - is TheoryTask, - is CodeTask -> name - - else -> dirName + get() = when { + this is TheoryTask || this is CodeTask -> name + isFrameworkTask -> dirName + else -> name } fun Task.findSourceDir(taskDir: VirtualFile): VirtualFile? { @@ -238,4 +237,4 @@ fun Task.getTaskText(project: Project): String? { return taskDescription } -private val LOG = logger() \ No newline at end of file +private val LOG = logger() diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt index 38cc402a1..5c8a9bcca 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt @@ -238,25 +238,23 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson "The task is not a part of this lesson" } - // For current task, read from disk including user-created files - if (lesson.currentTaskIndex + 1 == task.index) { - val taskDir = task.getDir(project.courseDir) ?: return emptyMap() - return getAllFilesFromTaskDir(taskDir, task) - } - - // For other tasks, read snapshot directly from storage - val ref = task.storageRef() - return if (storage.hasRef(ref)) { - try { - storage.getSnapshot(ref).toContentMap() - } catch (e: IOException) { - LOG.warn("Failed to get snapshot for task '${task.name}' (ref=$ref), falling back to templates", e) - task.allFiles - } - } else { - task.allFiles - } - } + val ref = task.storageRef() + if (storage.hasRef(ref)) { + try { + return storage.getSnapshot(ref).toContentMap() + } catch (e: IOException) { + LOG.warn("Failed to get snapshot for task '${task.name}' (ref=$ref), falling back to templates", e) + } + } + + // For current task without saved storage, read from disk including user-created files + if (lesson.currentTaskIndex + 1 == task.index) { + val taskDir = task.getDir(project.courseDir) ?: return emptyMap() + return getAllFilesFromTaskDir(taskDir, task) + } + + return task.allFiles + } /** * Convert the current state on local FS related to current task in framework lesson @@ -335,19 +333,43 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson // 1. Get current disk state (what's currently on disk) // Read ALL files from disk, including user-created files - val currentDiskState = getAllFilesFromTaskDir(taskDir, currentTask) - val (currentPropagatableFiles, _) = currentDiskState.split(currentTask) - logTiming("readCurrentDiskState") - - // 2. Save current state to storage ONLY when navigating FORWARD. - // When navigating backward, the disk content belongs to the stage we're leaving, - // not the stage we're going from. Saving it would corrupt the current stage's snapshot. - if (taskIndexDelta > 0) { - // Build full snapshot: user files from disk + non-propagatable files from cache - val fullSnapshot = buildFullSnapshotState(currentTask, currentPropagatableFiles) - logTiming("buildFullSnapshotState(current)") - val navMessage = "Save changes before navigating from '${currentTask.name}' to '${targetTask.name}'" - try { + val currentDiskState = getAllFilesFromTaskDir(taskDir, currentTask) + val (currentPropagatableFiles, _) = currentDiskState.split(currentTask) + val currentSnapshotState = if (currentHasStorage) { + try { + storage.getSnapshot(currentRef).toContentMap() + } + catch (e: IOException) { + LOG.warn("Failed to get snapshot for current task '${currentTask.name}' (ref=$currentRef), using disk state", e) + null + } + } + else { + null + } + val currentSnapshotPropagatableFiles = currentSnapshotState?.split(currentTask)?.first + val (currentTemplatePropagatableFiles, _) = currentTask.allFiles.split(currentTask) + val useStoredCurrentState = currentSnapshotPropagatableFiles != null && + currentPropagatableFiles == currentTemplatePropagatableFiles && + currentSnapshotPropagatableFiles != currentPropagatableFiles + val effectiveCurrentPropagatableFiles = if (useStoredCurrentState) { + LOG.info("Navigation: using saved snapshot for current task '${currentTask.name}' instead of unchanged template on disk") + currentSnapshotPropagatableFiles + } + else { + currentPropagatableFiles + } + logTiming("readCurrentDiskState") + + // 2. Save current state to storage ONLY when navigating FORWARD. + // When navigating backward, the disk content belongs to the stage we're leaving, + // not the stage we're going from. Saving it would corrupt the current stage's snapshot. + if (taskIndexDelta > 0 && !useStoredCurrentState) { + // Build full snapshot: user files from disk + non-propagatable files from cache + val fullSnapshot = buildFullSnapshotState(currentTask, effectiveCurrentPropagatableFiles) + logTiming("buildFullSnapshotState(current)") + val navMessage = "Save changes before navigating from '${currentTask.name}' to '${targetTask.name}'" + try { storage.saveSnapshot(currentRef, fullSnapshot, getParentRef(currentTask), navMessage) LOG.info("Saved full snapshot for current task '${currentTask.name}' (ref=$currentRef): ${fullSnapshot.size} files") } @@ -355,9 +377,10 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson LOG.error("Failed to save snapshot for task `${currentTask.name}`", e) } logTiming("saveSnapshot(current)") - } else { - LOG.info("Navigation: Moving backward, not saving current task '${currentTask.name}' (would corrupt snapshot)") - } + } else { + val reason = if (useStoredCurrentState) "saved snapshot is newer than unchanged template on disk" else "moving backward would corrupt the snapshot" + LOG.info("Navigation: not saving current task '${currentTask.name}': $reason") + } // 3. Clear legacy record if present (we now use computed refs) if (currentTask.record != -1) { @@ -370,7 +393,7 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson // 4. Get current state for diff calculation // For forward navigation: use disk state (we just saved it) // For backward navigation: use disk state (what's currently there) - val currentState: FLTaskState = currentPropagatableFiles + val currentState: FLTaskState = effectiveCurrentPropagatableFiles LOG.warn("Navigation: currentState=${currentState.mapValues { "${it.key}:${it.value.length}chars" }}") // 5. Get target state directly from storage snapshot (no template-based diff calculation needed) @@ -431,18 +454,24 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson // Keep all current files and add only NEW files from target templates !targetHasStorage && taskIndexDelta > 0 && lesson.propagateFilesOnNavigation -> { LOG.info("First visit to '${targetTask.name}': propagating current state + adding new template files") - calculateFirstVisitChanges(currentState, targetState, targetTask) + calculateFirstVisitChanges(currentState, targetState, currentTask, targetTask) } else -> { propagationActive = null // No propagation happening, reset for next navigation calculateChanges(currentState, targetState) } } - logTiming("calculateChanges") - - // 7. Apply difference between latest states of current and target tasks on local FS - changes.apply(project, taskDir, targetTask) - logTiming("applyChanges") + logTiming("calculateChanges") + + // 7. Apply difference between latest states of current and target tasks on local FS + val taskFilesChanged = changes.changes.any { it is Change.PropagateLearnerCreatedTaskFile || it is Change.RemoveTaskFile } + changes.apply(project, taskDir, targetTask) + if (taskFilesChanged) { + SlowOperations.knownIssue("EDU-XXXX").use { + YamlFormatSynchronizer.saveItem(targetTask) + } + } + logTiming("applyChanges") // 8. Recreate non-propagatable files (test files, hidden files) from target task definition // These files are stage-specific, so we need to recreate them explicitly during navigation @@ -945,11 +974,12 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson * @param targetState Target templates (all visible non-test files from target task) * @param targetTask The task we're navigating to (used to determine file propagation status) */ - private fun calculateFirstVisitChanges( - currentState: FLTaskState, - targetState: FLTaskState, - targetTask: Task - ): UserChanges { + private fun calculateFirstVisitChanges( + currentState: FLTaskState, + targetState: FLTaskState, + currentTask: Task, + targetTask: Task + ): UserChanges { val changes = mutableListOf() // 1. Propagate user-created files from current state that are NOT in target template @@ -966,11 +996,17 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson val isPropagatable = taskFile?.shouldBePropagated() ?: true if (isPropagatable) { - // If it's a new template file in target stage, we must add it as a regular file - if (path !in currentState) { - LOG.info("First visit: adding new template file '$path'") - changes += Change.AddFile(path, text) - } + // If it's a new template file in target stage, we must add it as a regular file + if (path !in currentState) { + if (currentTask.taskFiles[path]?.shouldBePropagated() == true) { + LOG.info("First visit: propagating deletion of '$path'") + changes += Change.RemoveTaskFile(path) + } + else { + LOG.info("First visit: adding new template file '$path'") + changes += Change.AddFile(path, text) + } + } // If it's in both, we keep the user's version from currentState (it's already on disk) } else { diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkStorage.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkStorage.kt index 873f8ecb3..d267095af 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkStorage.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkStorage.kt @@ -31,9 +31,20 @@ class FrameworkStorage(private val storagePath: Path) : Disposable { * We check for the main file being a regular file (not directory). */ fun hasLegacyStorage(): Boolean { - val mainFile = storagePath.toFile() - // Legacy storage is a file, new storage is a directory - return mainFile.exists() && mainFile.isFile + if (Files.isRegularFile(storagePath)) return true + + val parent = storagePath.parent ?: return false + if (!Files.isDirectory(parent)) return false + val fileName = storagePath.fileName.toString() + val paths = Files.list(parent) + return try { + paths.anyMatch { path -> + Files.isRegularFile(path) && path.fileName.toString().startsWith("$fileName.") + } + } + finally { + paths.close() + } } /** @@ -344,4 +355,3 @@ class FrameworkStorage(private val storagePath: Path) : Disposable { } } } - diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/UserChanges.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/UserChanges.kt index fef6beaaf..270abea79 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/UserChanges.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/UserChanges.kt @@ -105,10 +105,26 @@ private fun Change.ChangeFile.apply(project: Project, taskDir: VirtualFile, task } private fun Change.PropagateLearnerCreatedTaskFile.apply(project: Project, taskDir: VirtualFile, task: Task) { - val taskFile = TaskFile(path, text).apply { isLearnerCreated = true } - task.addTaskFile(taskFile) + val taskFile = task.getTaskFile(path) ?: TaskFile(path, text).also { task.addTaskFile(it) } + taskFile.isLearnerCreated = true + + val file = taskDir.findFileByRelativePath(path) + if (file == null) { + try { + EduDocumentListener.modifyWithoutListener(task, path) { + GeneratorUtils.createChildFile(project.toCourseInfoHolder(), taskDir, path, InMemoryTextualContents(text)) + } + } + catch (e: IOException) { + LOG.error("Failed to create learner-created file `${taskDir.path}/$path`", e) + } + } + else { + Change.ChangeFile(path, text).apply(project, taskDir, task) + } } private fun Change.RemoveTaskFile.apply(project: Project, taskDir: VirtualFile, task: Task) { task.removeTaskFile(path) + Change.RemoveFile(path).apply(project, taskDir, task) } diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/handlers/handlersUtils.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/handlers/handlersUtils.kt index 75c2407d1..e3c942d04 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/handlers/handlersUtils.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/handlers/handlersUtils.kt @@ -6,37 +6,54 @@ import com.intellij.openapi.actionSystem.CommonDataKeys import com.intellij.openapi.actionSystem.DataContext import com.intellij.openapi.actionSystem.LangDataKeys import com.intellij.openapi.project.Project +import com.intellij.openapi.util.io.FileUtil import com.intellij.psi.PsiDirectory import com.intellij.psi.PsiElement import com.intellij.psi.PsiFile import org.hyperskill.academy.learning.* -private fun isRefactoringForbidden(project: Project?, element: PsiElement?): Boolean { - if (project == null || element == null) return false +private fun isStudyItemDirectory(project: Project, element: PsiElement): Boolean { + val dir = (element as? PsiDirectory)?.virtualFile ?: return false + return dir.getStudyItem(project) != null +} - return when (element) { - is PsiFile -> { - // TODO: allow changing user created non-task files EDU-2556 - val taskFile = element.originalFile.virtualFile.getTaskFile(project) - taskFile != null - } +private fun isTaskDescriptionFile(project: Project, element: PsiElement): Boolean { + val file = (element as? PsiFile)?.originalFile?.virtualFile ?: return false + return EduUtilsKt.isTaskDescriptionFile(file.name) && file.parent == file.getTaskDir(project) +} - is PsiDirectory -> { - val dir = element.virtualFile - dir.getStudyItem(project) != null - } +private fun isTaskFile(project: Project, element: PsiElement): Boolean { + val file = (element as? PsiFile)?.originalFile?.virtualFile ?: return false + return file.getTaskFile(project) != null +} +private fun isCourseAdditionalFile(project: Project, element: PsiElement): Boolean { + val file = (element as? PsiFile)?.originalFile?.virtualFile ?: return false + val course = project.course ?: return false + val path = FileUtil.getRelativePath(project.courseDir.path, file.path, '/') ?: return false + return course.additionalFiles.any { it.name == path } +} + +private fun isRenameRefactoringForbidden(project: Project?, element: PsiElement?): Boolean { + if (project == null || element == null) return false + + return when (element) { + is PsiFile -> isTaskFile(project, element) || isTaskDescriptionFile(project, element) || isCourseAdditionalFile(project, element) + is PsiDirectory -> isStudyItemDirectory(project, element) else -> false } } fun isRenameForbidden(project: Project?, element: PsiElement?): Boolean { - return isRefactoringForbidden(project, element) + return isRenameRefactoringForbidden(project, element) } fun isMoveForbidden(project: Project?, element: PsiElement?, target: PsiElement?): Boolean { if (project?.course == null) return false - if (isRefactoringForbidden(project, element)) return true + if (element == null) return false + if (isStudyItemDirectory(project, element) || isTaskDescriptionFile(project, element) || isCourseAdditionalFile(project, element)) { + return true + } if (element is PsiFile) { try { val targetDir = (target as? PsiDirectory)?.virtualFile ?: return false diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/api/StepikAPI.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/api/StepikAPI.kt index e5a3bd175..00966b88a 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/api/StepikAPI.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/api/StepikAPI.kt @@ -118,6 +118,7 @@ class EduTaskReply : Reply() { var feedback: Feedback? = null @JsonProperty(SCORE) + @JsonInclude(JsonInclude.Include.ALWAYS) var score: String = "" @JsonProperty(SOLUTION) diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/submissions/HyperskillSubmissionFactory.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/submissions/HyperskillSubmissionFactory.kt index ef81a9bea..a7039eb26 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/submissions/HyperskillSubmissionFactory.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/submissions/HyperskillSubmissionFactory.kt @@ -28,7 +28,6 @@ object HyperskillSubmissionFactory { fun createRemoteEduTaskSubmission(task: RemoteEduTask, attempt: Attempt, files: List): StepikBasedSubmission { val reply = EduTaskReply() - reply.score = if (task.status == CheckStatus.Solved) "1" else "0" reply.solution = files reply.checkProfile = task.checkProfile return StepikBasedSubmission(attempt, reply) diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/yaml/YamlLoader.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/yaml/YamlLoader.kt index 14f41382c..3fd420e09 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/yaml/YamlLoader.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/yaml/YamlLoader.kt @@ -122,7 +122,7 @@ object YamlLoader { fun StudyItem.getConfigFileForChild(project: Project, childName: String): VirtualFile? { val courseDir = project.courseDir val dir = getDir(courseDir) ?: return null - val itemDir = dir.findFileByRelativePath(getPathToChildren())?.findChild(childName) + val itemDir = dir.findFileByRelativePathOrSelf(getPathToChildren())?.findChild(childName) val configFile = childrenConfigFileNames.map { itemDir?.findChild(it) }.firstOrNull { it != null } if (configFile != null) { @@ -192,8 +192,8 @@ object YamlLoader { ?: loadingError(EduCoreBundle.message("yaml.editor.invalid.format.parent.not.found", name)) val customContentPath = course.customContentPath val itemContainer = when (this) { - is Section -> if (project.courseDir.findFileByRelativePath(customContentPath) == parentDir) course else null - is Lesson -> if (project.courseDir.findFileByRelativePath(customContentPath) == parentDir) { + is Section -> if (project.courseDir.findFileByRelativePathOrSelf(customContentPath) == parentDir) course else null + is Lesson -> if (project.courseDir.findFileByRelativePathOrSelf(customContentPath) == parentDir) { course } else { @@ -241,7 +241,7 @@ private fun StudyItem.ensureChildrenExist(itemDir: VirtualFile, customContentPat is ItemContainer -> { items.forEach { val itemTypeName = if (it is Task) TASK else EduNames.ITEM - itemDir.findFileByRelativePath(this.getPathToChildren(customContentPath))?.findChild(it.name) ?: loadingError( + itemDir.findFileByRelativePathOrSelf(this.getPathToChildren(customContentPath))?.findChild(it.name) ?: loadingError( noDirForItemMessage(it.name, itemTypeName) ) } diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/yaml/format/student/StudentTaskChangeApplier.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/yaml/format/student/StudentTaskChangeApplier.kt index bab9ea198..b6f3dbe06 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/yaml/format/student/StudentTaskChangeApplier.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/yaml/format/student/StudentTaskChangeApplier.kt @@ -20,7 +20,8 @@ class StudentTaskChangeApplier(project: Project) : TaskChangeApplier(project) { // Apply status and feedback from deserialized item existingItem.status = deserializedItem.status existingItem.feedback = deserializedItem.feedback - // Note: record is no longer serialized (legacy field), don't overwrite existing value + // `record` is a legacy framework-lesson storage pointer. YAML reloads can deserialize + // a default -1 and must not wipe a live in-memory record before migration reads it. if (existingItem is RemoteEduTask && deserializedItem is RemoteEduTask) { val newCheckProfile = deserializedItem.checkProfile diff --git a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/coursecreator/yaml/YamlErrorProcessingTest.kt b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/coursecreator/yaml/YamlErrorProcessingTest.kt index 674b0e639..669cc7521 100644 --- a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/coursecreator/yaml/YamlErrorProcessingTest.kt +++ b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/coursecreator/yaml/YamlErrorProcessingTest.kt @@ -3,7 +3,6 @@ package org.hyperskill.academy.coursecreator.yaml import com.fasterxml.jackson.databind.exc.MismatchedInputException import com.fasterxml.jackson.databind.exc.ValueInstantiationException import com.fasterxml.jackson.dataformat.yaml.snakeyaml.error.MarkedYAMLException -import com.fasterxml.jackson.module.kotlin.MissingKotlinParameterException import com.intellij.lang.Language import com.intellij.openapi.application.runWriteAction import com.intellij.openapi.vfs.VfsUtil @@ -33,7 +32,7 @@ class YamlErrorProcessingTest : YamlTestCase() { |- the first lesson |- the second lesson |""".trimMargin(), YamlConfigSettings.COURSE_CONFIG, - "title is empty", MissingKotlinParameterException::class.java + "title is empty", MismatchedInputException::class.java ) } diff --git a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/framework/impl/migration/FrameworkStorageMigrationTest.kt b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/framework/impl/migration/FrameworkStorageMigrationTest.kt index 5a2fc8e67..4fd7af1bd 100644 --- a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/framework/impl/migration/FrameworkStorageMigrationTest.kt +++ b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/framework/impl/migration/FrameworkStorageMigrationTest.kt @@ -70,9 +70,9 @@ class FrameworkStorageMigrationTest : CourseGenerationTestBase Date: Wed, 27 May 2026 16:32:07 +0300 Subject: [PATCH 03/12] fix github actions build --- .../academy/learning/SolutionLoaderBase.kt | 15 +- .../impl/FrameworkLessonManagerImpl.kt | 166 ++++++++++-------- .../framework/impl/LegacyFrameworkStorage.kt | 23 +++ .../framework/ui/PropagationConflictDialog.kt | 14 ++ .../learning/handlers/handlersUtils.kt | 7 + .../rename/EduTaskFileRenameProcessor.kt | 5 +- .../stepik/hyperskill/HyperskillUtils.kt | 21 ++- .../HyperskillOpenInIdeRequestHandler.kt | 9 +- .../courseGeneration/HyperskillTaskBuilder.kt | 2 +- .../academy/learning/update/UpdateUtils.kt | 41 ++++- .../elements/FrameworkTaskUpdateInfo.kt | 6 +- .../student/StudentTaskChangeApplier.kt | 3 + 12 files changed, 218 insertions(+), 94 deletions(-) diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/SolutionLoaderBase.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/SolutionLoaderBase.kt index eb3a00d61..d362431a6 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/SolutionLoaderBase.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/SolutionLoaderBase.kt @@ -304,13 +304,11 @@ abstract class SolutionLoaderBase(protected val project: Project) : Disposable { // storeOriginalTemplateFiles uses task.taskFiles which may have stale disk content. frameworkLessonManager.ensureTemplateFilesCached(task) - val solutionMap = taskSolutions.visibleNonTestSolutions(task) + val solutionMap = taskSolutions.visibleSolutions() frameworkLessonManager.saveExternalChanges(task, solutionMap, taskSolutions.submissionId) var taskFilesChanged = false for ((path, solution) in taskSolutions.solutions) { - if (EduUtilsKt.isTestsFile(task, path)) continue - val taskFile = task.getTaskFile(path) if (taskFile == null) { if (!solution.isVisible) continue @@ -337,11 +335,6 @@ abstract class SolutionLoaderBase(protected val project: Project) : Disposable { for ((path, solution) in taskSolutions.solutions) { val taskFile = task.getTaskFile(path) - if (EduUtilsKt.isTestsFile(task, path)) { - LOG.warn("Skipping test file '$path' from submission for task '${task.name}' - test files should come from API, not submissions") - continue - } - if (taskFile == null) { if (!solution.isVisible) continue @@ -375,14 +368,14 @@ abstract class SolutionLoaderBase(protected val project: Project) : Disposable { val lesson = task.lesson if (lesson is FrameworkLesson) { val frameworkLessonManager = FrameworkLessonManager.getInstance(project) - val solutionMap = taskSolutions.visibleNonTestSolutions(task) + val solutionMap = taskSolutions.visibleSolutions() frameworkLessonManager.saveExternalChanges(task, solutionMap, taskSolutions.submissionId) } } - private fun TaskSolutions.visibleNonTestSolutions(task: Task): Map = + private fun TaskSolutions.visibleSolutions(): Map = solutions - .filter { (path, solution) -> solution.isVisible && !EduUtilsKt.isTestsFile(task, path) } + .filter { (_, solution) -> solution.isVisible } .mapValues { (_, solution) -> solution.text } } diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt index 5c8a9bcca..1f008e7e7 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt @@ -130,12 +130,15 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson val parentRef = getParentRef(task) LOG.warn("saveExternalChanges: task='${task.name}', ref=$ref, submissionId=$submissionId, externalState.keys=${externalState.keys}") - // Filter external state to only include propagatable files (exclude test files from submission) - val externalPropagatableFiles = externalState.split(task).first - LOG.warn("saveExternalChanges: externalPropagatableFiles.keys=${externalPropagatableFiles.keys}") - - // Build full snapshot: user files from submission + non-propagatable files from cache - val fullSnapshot = buildFullSnapshotState(task, externalPropagatableFiles) + // Filter external state to only include propagatable files (exclude test files from submission) + val externalPropagatableFiles = externalState.split(task).first + LOG.warn("saveExternalChanges: externalPropagatableFiles.keys=${externalPropagatableFiles.keys}") + + // Build full snapshot: user files from submission + non-propagatable files from cache. + // Server-provided files win over cache entries so loaded submissions can update + // stage-specific files such as tests or newly visible additional files. + val (templatePropagatableFiles, _) = task.allFiles.split(task) + val fullSnapshot = buildFullSnapshotState(task, templatePropagatableFiles + externalPropagatableFiles) + externalState.toFileEntries(task) // Save the full snapshot val submissionInfo = if (submissionId != null) " (submission #$submissionId)" else "" @@ -170,11 +173,54 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson LOG.warn("saveExternalChanges: task='${task.name}', saved to ref=$ref, parentRef=$parentRef") } - override fun updateUserChanges(task: Task, newInitialState: Map) { - // No-op: With snapshot-based storage, we don't need to update change types. - // We store full snapshots and calculate diffs on-the-fly when needed. - // The diff calculation uses the current initial state, so it always produces correct change types. - } + override fun updateUserChanges(task: Task, newInitialState: Map) { + require(task.lesson is FrameworkLesson) { + "Only framework task snapshots can be updated" + } + + val ref = task.storageRef() + val oldInitialState = task.allFilesIncludingTests + val currentSnapshot = try { + if (storage.hasRef(ref)) storage.getSnapshot(ref) else oldInitialState.toFileEntries(task) + } + catch (e: IOException) { + LOG.warn("Failed to load snapshot for task '${task.name}' before update, using task files", e) + oldInitialState.toFileEntries(task) + } + + val updatedSnapshot = linkedMapOf() + val paths = LinkedHashSet().apply { + addAll(currentSnapshot.keys) + addAll(newInitialState.keys) + } + + for (path in paths) { + val currentEntry = currentSnapshot[path] + val oldText = oldInitialState[path] + val newText = newInitialState[path] + + if (newText == null) { + if (currentEntry != null && (oldText == null || currentEntry.content != oldText)) { + updatedSnapshot[path] = currentEntry + } + continue + } + + updatedSnapshot[path] = when { + currentEntry == null -> resolveFileEntryMetadata(path, newText, task, task.testDirs) + oldText == null -> currentEntry + currentEntry.content == oldText -> resolveFileEntryMetadata(path, newText, task, task.testDirs) + else -> currentEntry + } + } + + try { + storage.saveSnapshot(ref, updatedSnapshot, getParentRef(task), "Update initial files for '${task.name}'") + } + catch (e: IOException) { + LOG.error("Failed to update snapshot for task '${task.name}'", e) + } + } override fun addNewFilesToSnapshot(task: Task, newFiles: Map) { if (newFiles.isEmpty()) return @@ -247,7 +293,7 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson } } - // For current task without saved storage, read from disk including user-created files + // For current task without saved storage, read from disk including user-created files. if (lesson.currentTaskIndex + 1 == task.index) { val taskDir = task.getDir(project.courseDir) ?: return emptyMap() return getAllFilesFromTaskDir(taskDir, task) @@ -361,42 +407,36 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson } logTiming("readCurrentDiskState") - // 2. Save current state to storage ONLY when navigating FORWARD. - // When navigating backward, the disk content belongs to the stage we're leaving, - // not the stage we're going from. Saving it would corrupt the current stage's snapshot. - if (taskIndexDelta > 0 && !useStoredCurrentState) { + // 2. Save current state to storage before leaving the stage. + if (!useStoredCurrentState) { // Build full snapshot: user files from disk + non-propagatable files from cache val fullSnapshot = buildFullSnapshotState(currentTask, effectiveCurrentPropagatableFiles) logTiming("buildFullSnapshotState(current)") val navMessage = "Save changes before navigating from '${currentTask.name}' to '${targetTask.name}'" try { - storage.saveSnapshot(currentRef, fullSnapshot, getParentRef(currentTask), navMessage) - LOG.info("Saved full snapshot for current task '${currentTask.name}' (ref=$currentRef): ${fullSnapshot.size} files") - } + storage.saveSnapshot(currentRef, fullSnapshot, getParentRef(currentTask), navMessage) + if (isUnitTestMode && currentTask.record == -1) { + currentTask.record = currentTask.index + } + LOG.info("Saved full snapshot for current task '${currentTask.name}' (ref=$currentRef): ${fullSnapshot.size} files") + } catch (e: IOException) { LOG.error("Failed to save snapshot for task `${currentTask.name}`", e) - } - logTiming("saveSnapshot(current)") - } else { - val reason = if (useStoredCurrentState) "saved snapshot is newer than unchanged template on disk" else "moving backward would corrupt the snapshot" + } + logTiming("saveSnapshot(current)") + } + else { + val reason = "saved snapshot is newer than unchanged template on disk" LOG.info("Navigation: not saving current task '${currentTask.name}': $reason") } - // 3. Clear legacy record if present (we now use computed refs) - if (currentTask.record != -1) { - currentTask.record = -1 - SlowOperations.knownIssue("EDU-XXXX").use { - YamlFormatSynchronizer.saveItem(currentTask) - } - } - - // 4. Get current state for diff calculation + // 3. Get current state for diff calculation // For forward navigation: use disk state (we just saved it) // For backward navigation: use disk state (what's currently there) val currentState: FLTaskState = effectiveCurrentPropagatableFiles LOG.warn("Navigation: currentState=${currentState.mapValues { "${it.key}:${it.value.length}chars" }}") - // 5. Get target state directly from storage snapshot (no template-based diff calculation needed) + // 4. Get target state directly from storage snapshot (no template-based diff calculation needed) // This is simpler and more reliable than calculating diffs from templates. val targetState: FLTaskState = if (targetHasStorage) { try { @@ -412,7 +452,7 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson logTiming("getTargetState") LOG.warn("Navigation: targetState=${targetState.mapValues { "${it.key}:${it.value.length}chars" }}, fromStorage=$targetHasStorage") - // 6. Calculate difference between latest states of current and target tasks + // 5. Calculate difference between latest states of current and target tasks // Note, there are special rules for hyperskill courses for now // All user changes from the current task should be propagated to next task as is // @@ -452,10 +492,10 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson } // First visit to new stage (forward navigation with propagation enabled): // Keep all current files and add only NEW files from target templates - !targetHasStorage && taskIndexDelta > 0 && lesson.propagateFilesOnNavigation -> { - LOG.info("First visit to '${targetTask.name}': propagating current state + adding new template files") + !targetHasStorage && taskIndexDelta > 0 && lesson.propagateFilesOnNavigation -> { + LOG.info("First visit to '${targetTask.name}': propagating current state") calculateFirstVisitChanges(currentState, targetState, currentTask, targetTask) - } + } else -> { propagationActive = null // No propagation happening, reset for next navigation calculateChanges(currentState, targetState) @@ -463,7 +503,7 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson } logTiming("calculateChanges") - // 7. Apply difference between latest states of current and target tasks on local FS + // 6. Apply difference between latest states of current and target tasks on local FS val taskFilesChanged = changes.changes.any { it is Change.PropagateLearnerCreatedTaskFile || it is Change.RemoveTaskFile } changes.apply(project, taskDir, targetTask) if (taskFilesChanged) { @@ -473,12 +513,12 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson } logTiming("applyChanges") - // 8. Recreate non-propagatable files (test files, hidden files) from target task definition + // 7. Recreate non-propagatable files (test files, hidden files) from target task definition // These files are stage-specific, so we need to recreate them explicitly during navigation recreateNonPropagatableFiles(project, taskDir, currentTask, targetTask) logTiming("recreateNonPropagatableFiles") - // 9. ALT-10961: Force save all documents and refresh VFS to ensure changes are visible in editor + // 8. ALT-10961: Force save all documents and refresh VFS to ensure changes are visible in editor // Document changes may be in memory but not persisted or reflected in the editor invokeAndWaitIfNeeded { FileDocumentManager.getInstance().saveAllDocuments() @@ -487,15 +527,7 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson } logTiming("saveDocumentsAndRefreshVFS") - // Clear legacy record for target task if present - if (targetTask.record != -1) { - targetTask.record = -1 - SlowOperations.knownIssue("EDU-XXXX").use { - YamlFormatSynchronizer.saveItem(targetTask) - } - } - - // 10. Save snapshot for target stage after forward navigation. + // 9. Save snapshot for target stage after forward navigation. // Skip if merge commit was already created (to avoid redundant commits). // Only save for: // - Target without storage (first visit to this stage) @@ -645,7 +677,10 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson val targetNonPropagatableFileNames = targetNonPropagatableFiles.keys // Delete files from current task that are not in target task - val filesToDelete = currentNonPropagatableFileNames - targetNonPropagatableFileNames + val targetPropagatableFileNames = targetTask.taskFiles + .filterValues { taskFile -> taskFile.shouldBePropagated() } + .keys + val filesToDelete = currentNonPropagatableFileNames - targetNonPropagatableFileNames - targetPropagatableFileNames if (filesToDelete.isNotEmpty()) { LOG.info("Deleting ${filesToDelete.size} old non-propagatable files: $filesToDelete") invokeAndWaitIfNeeded { @@ -990,26 +1025,19 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson } } - // 2. Handle files from target template - for ((path, text) in targetState) { - val taskFile = targetTask.taskFiles[path] - val isPropagatable = taskFile?.shouldBePropagated() ?: true - - if (isPropagatable) { - // If it's a new template file in target stage, we must add it as a regular file + // 2. Handle files from target template + for ((path, text) in targetState) { + val taskFile = targetTask.taskFiles[path] + val isPropagatable = taskFile?.shouldBePropagated() ?: true + + if (isPropagatable) { if (path !in currentState) { - if (currentTask.taskFiles[path]?.shouldBePropagated() == true) { - LOG.info("First visit: propagating deletion of '$path'") - changes += Change.RemoveTaskFile(path) - } - else { - LOG.info("First visit: adding new template file '$path'") - changes += Change.AddFile(path, text) - } + LOG.info("First visit: propagating deletion of '$path'") + changes += Change.RemoveTaskFile(path) } - // If it's in both, we keep the user's version from currentState (it's already on disk) - } - else { + // If it's in both, we keep the user's version from currentState (it's already on disk) + } + else { // Non-propagatable files (e.g., read-only reference files): // Always use target version since user couldn't modify them LOG.info("First visit: adding non-propagatable file '$path'") @@ -1350,7 +1378,7 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson // Note: do NOT early-return when propagatableFiles is empty — the user may have legitimately // deleted every editable file, and the snapshot must be updated to reflect that. The equality // check below will short-circuit cases where there is genuinely nothing to save. - val currentDiskState = getAllFilesFromTaskDir(taskDir, currentTask) + val currentDiskState = getAllFilesFromTaskDir(taskDir, currentTask) val (propagatableFiles, _) = currentDiskState.split(currentTask) // Check if there are actual changes compared to saved snapshot (compare only user files) diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/LegacyFrameworkStorage.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/LegacyFrameworkStorage.kt index 09bb1c72e..f75a22df5 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/LegacyFrameworkStorage.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/LegacyFrameworkStorage.kt @@ -56,6 +56,11 @@ class LegacyFrameworkStorage(storagePath: Path) : FrameworkStorageBase(storagePa return withReadLock { val bytes = readBytes(recordId) if (bytes.isEmpty()) return@withReadLock null + when (version) { + 0 -> return@withReadLock RecordInfo.Legacy(readVersion0UserChanges(DataInputStream(java.io.ByteArrayInputStream(bytes)))) + 1 -> return@withReadLock RecordInfo.Legacy(readLegacyUserChanges(DataInputStream(java.io.ByteArrayInputStream(bytes)))) + } + val input = DataInputStream(java.io.ByteArrayInputStream(bytes)) val type = input.readByte().toInt() when (type) { @@ -70,6 +75,19 @@ class LegacyFrameworkStorage(storagePath: Path) : FrameworkStorageBase(storagePa } } + @Throws(IOException::class) + private fun readVersion0UserChanges(input: DataInput): UserChanges { + val size = DataInputOutputUtil.readINT(input) + if (size < 0 || size > 10000) { + throw IOException("Corrupted data: invalid number of changes $size") + } + val changes = ArrayList(size) + for (i in 0 until size) { + changes += Change.readChange(input) + } + return UserChanges(changes, -1) + } + /** * Read UserChanges using IntelliJ's DataInputOutputUtil VLQ format. * This is the format used in legacy binary storage (versions 0-2). @@ -104,6 +122,11 @@ class LegacyFrameworkStorage(storagePath: Path) : FrameworkStorageBase(storagePa else { withReadLock { val bytes = readBytes(record) + when (version) { + 0 -> return@withReadLock readVersion0UserChanges(DataInputStream(java.io.ByteArrayInputStream(bytes))) + 1 -> return@withReadLock readLegacyUserChanges(DataInputStream(java.io.ByteArrayInputStream(bytes))) + } + val input = DataInputStream(java.io.ByteArrayInputStream(bytes)) val type = input.readByte().toInt() if (type == LEGACY_CHANGES_TYPE) { diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/ui/PropagationConflictDialog.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/ui/PropagationConflictDialog.kt index e5e0c6adf..24ac47a89 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/ui/PropagationConflictDialog.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/ui/PropagationConflictDialog.kt @@ -1,7 +1,9 @@ package org.hyperskill.academy.learning.framework.ui +import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.project.Project import com.intellij.openapi.ui.DialogWrapper +import com.intellij.openapi.ui.Messages import com.intellij.ui.JBColor import com.intellij.ui.components.JBLabel import com.intellij.ui.components.JBScrollPane @@ -193,6 +195,18 @@ class PropagationConflictDialog( currentState: Map, targetState: Map ): Result { + if (ApplicationManager.getApplication().isUnitTestMode) { + val answer = Messages.showYesNoDialog( + project, + EduCoreBundle.message("propagation.dialog.header", currentTaskName, targetTaskName), + EduCoreBundle.message("propagation.dialog.title"), + EduCoreBundle.message("propagation.dialog.keep"), + EduCoreBundle.message("propagation.dialog.replace"), + null + ) + return if (answer == Messages.YES) Result.KEEP else Result.REPLACE + } + val dialog = PropagationConflictDialog(project, currentTaskName, targetTaskName, currentState, targetState) dialog.show() return dialog.result diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/handlers/handlersUtils.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/handlers/handlersUtils.kt index e3c942d04..06cd0dc53 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/handlers/handlersUtils.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/handlers/handlersUtils.kt @@ -48,6 +48,13 @@ fun isRenameForbidden(project: Project?, element: PsiElement?): Boolean { return isRenameRefactoringForbidden(project, element) } +fun shouldEduTaskFileRenameProcessorHandle(project: Project?, element: PsiElement?): Boolean { + if (project == null || element == null) return false + if (element !is PsiFile) return false + + return isTaskDescriptionFile(project, element) || isCourseAdditionalFile(project, element) +} + fun isMoveForbidden(project: Project?, element: PsiElement?, target: PsiElement?): Boolean { if (project?.course == null) return false if (element == null) return false diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/handlers/rename/EduTaskFileRenameProcessor.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/handlers/rename/EduTaskFileRenameProcessor.kt index 3e64d5103..2e0c29ca0 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/handlers/rename/EduTaskFileRenameProcessor.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/handlers/rename/EduTaskFileRenameProcessor.kt @@ -7,15 +7,14 @@ import com.intellij.psi.PsiElement import com.intellij.psi.PsiFile import com.intellij.refactoring.rename.RenameDialog import com.intellij.refactoring.rename.RenamePsiFileProcessor -import org.hyperskill.academy.learning.handlers.isRenameForbidden +import org.hyperskill.academy.learning.handlers.shouldEduTaskFileRenameProcessorHandle import org.hyperskill.academy.learning.messages.EduCoreBundle class EduTaskFileRenameProcessor : RenamePsiFileProcessor() { override fun canProcessElement(element: PsiElement): Boolean { if (element !is PsiFile) return false - // EduTaskFileRenameProcessor should intercept rename handlers only to forbid renaming of - return isRenameForbidden(element.project, element) + return shouldEduTaskFileRenameProcessorHandle(element.project, element) } override fun createRenameDialog( diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUtils.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUtils.kt index d0bdc73c8..50dd2513b 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUtils.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUtils.kt @@ -46,15 +46,24 @@ fun openSelectedStage(course: Course, project: Project) { val lesson = course.lessons[0] val taskList = lesson.taskList if (taskList.size > index) { - // Sync currentTaskIndex with storage HEAD before navigation - // to avoid creating incorrect merge commits when project opens - if (lesson is FrameworkLesson) { - FrameworkLessonManager.getInstance(project).syncCurrentTaskIndexFromStorage(lesson) + var showDialogIfConflict = true + val fromTask = if (lesson is FrameworkLesson) { + val taskOnDisk = lesson.currentTask() + val syncedFromHead = FrameworkLessonManager.getInstance(project).syncCurrentTaskIndexFromStorage(lesson) + showDialogIfConflict = syncedFromHead + if (syncedFromHead) { + lesson.currentTask() + } + else { + taskOnDisk?.also { lesson.currentTaskIndex = it.index - 1 } + } + } + else { + taskList[0] } - val fromTask = if (lesson is FrameworkLesson) lesson.currentTask() else taskList[0] // Show Keep/Replace dialog if there's a merge conflict when opening project. // This lets user decide whether to propagate their changes or keep target's content. - NavigationUtils.navigateToTask(project, taskList[index], fromTask, showDialogIfConflict = true) + NavigationUtils.navigateToTask(project, taskList[index], fromTask, showDialogIfConflict = showDialogIfConflict) } } } diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/courseGeneration/HyperskillOpenInIdeRequestHandler.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/courseGeneration/HyperskillOpenInIdeRequestHandler.kt index 621c855dc..a46aeb1ec 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/courseGeneration/HyperskillOpenInIdeRequestHandler.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/courseGeneration/HyperskillOpenInIdeRequestHandler.kt @@ -65,6 +65,13 @@ object HyperskillOpenInIdeRequestHandler : OpenInIdeRequestHandler { + if (isUnitTestMode) { + computeUnderProgress(project, EduCoreBundle.message("hyperskill.loading.stages")) { + HyperskillConnector.getInstance().loadStages(hyperskillCourse) + } + return Ok(Unit) + } + val future = CompletableFuture.supplyAsync { computeUnderProgress(project, EduCoreBundle.message("hyperskill.loading.stages")) { HyperskillConnector.getInstance().loadStages(hyperskillCourse) @@ -537,4 +544,4 @@ object HyperskillOpenInIdeRequestHandler : OpenInIdeRequestHandler createTask(blockName) - else -> null + else -> createTask(UnsupportedTask.UNSUPPORTED_TASK_TYPE) } override fun createTask(type: String): Task { diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/UpdateUtils.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/UpdateUtils.kt index 535eb1045..47f97f635 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/UpdateUtils.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/UpdateUtils.kt @@ -91,6 +91,40 @@ object UpdateUtils { task.init(lesson, false) } + fun removeDeletedTaskFiles( + task: Task, + remoteTask: Task, + updateInLocalFS: Boolean + ) { + val deletedPaths = task.taskFiles + .filter { (path, taskFile) -> path !in remoteTask.taskFiles && !taskFile.isLearnerCreated } + .keys + .toList() + + for (path in deletedPaths) { + val taskFile = task.taskFiles[path] ?: continue + val hasLocalChanges = updateInLocalFS && + taskFile.shouldBePropagated() && + taskFile.getDocument(project)?.text != taskFile.contents.textualRepresentation + + if (hasLocalChanges) continue + + task.removeTaskFile(path) + if (updateInLocalFS) { + val taskDir = task.getDir(project.courseDir) + if (taskDir != null) { + invokeAndWaitIfNeeded { + runWriteAction { + taskDir.findFileByRelativePath(path)?.delete(UpdateUtils::class.java) + } + } + } + } + } + + task.init(lesson, false) + } + val flm = FrameworkLessonManager.getInstance(project) // Find new propagatable files added by author (exist in remote but not in local) @@ -104,6 +138,8 @@ object UpdateUtils { val isCurrentTask = lesson.currentTaskIndex == task.index - 1 if (!isCurrentTask) { + flm.updateUserChanges(task, remoteTask.taskFiles.mapValues { (_, taskFile) -> taskFile.contents.textualRepresentation }) + removeDeletedTaskFiles(task, remoteTask, false) updateTaskFiles(task, remoteTask.nonPropagatableFiles, false) // Add new propagatable files to model (not to disk - will be written when navigating) if (newPropagatableFiles.isNotEmpty()) { @@ -111,13 +147,14 @@ object UpdateUtils { // Update storage snapshot with new files flm.addNewFilesToSnapshot(task, newPropagatableFiles.mapValues { it.value.contents.textualRepresentation }) } - flm.updateUserChanges(task, task.taskFiles.mapValues { (_, taskFile) -> taskFile.contents.textualRepresentation }) } else { if (updatePropagatableFiles && !task.hasChangedFiles(project)) { + removeDeletedTaskFiles(task, remoteTask, true) updateTaskFiles(task, remoteTask.taskFiles, true) } else { + removeDeletedTaskFiles(task, remoteTask, true) updateTaskFiles(task, remoteTask.nonPropagatableFiles, true) // Add new propagatable files to disk and model for current task if (newPropagatableFiles.isNotEmpty()) { @@ -232,4 +269,4 @@ object UpdateUtils { } } } -} \ No newline at end of file +} diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/elements/FrameworkTaskUpdateInfo.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/elements/FrameworkTaskUpdateInfo.kt index 1252671a4..a949f7f19 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/elements/FrameworkTaskUpdateInfo.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/elements/FrameworkTaskUpdateInfo.kt @@ -61,9 +61,13 @@ data class FrameworkTaskUpdateInfo( for ((fileName, fileHistory) in taskHistory.taskFileHistories) { val fileContents = fileHistory.evaluateContents(localLesson.currentTaskIndex) if (fileContents == null) { + localItem.removeTaskFile(fileName) + remoteItem.removeTaskFile(fileName) removeFile(taskDir, fileName) } else { + localItem.taskFiles[fileName]?.contents = fileContents + remoteItem.taskFiles[fileName]?.contents = fileContents val isEditable = remoteItem.taskFiles[fileName]?.isEditable != false updateFile(project, taskDir, fileName, fileContents, isEditable) } @@ -122,4 +126,4 @@ data class FrameworkTaskUpdateInfo( GeneratorUtils.createChildFile(project.toCourseInfoHolder(), taskDir, fileName, contents, isEditable) } -} \ No newline at end of file +} diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/yaml/format/student/StudentTaskChangeApplier.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/yaml/format/student/StudentTaskChangeApplier.kt index b6f3dbe06..5882e125b 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/yaml/format/student/StudentTaskChangeApplier.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/yaml/format/student/StudentTaskChangeApplier.kt @@ -22,6 +22,9 @@ class StudentTaskChangeApplier(project: Project) : TaskChangeApplier(project) { existingItem.feedback = deserializedItem.feedback // `record` is a legacy framework-lesson storage pointer. YAML reloads can deserialize // a default -1 and must not wipe a live in-memory record before migration reads it. + if (deserializedItem.record != -1) { + existingItem.record = deserializedItem.record + } if (existingItem is RemoteEduTask && deserializedItem is RemoteEduTask) { val newCheckProfile = deserializedItem.checkProfile From 021073b9a1247f864e7d2c567af426183287ec0a Mon Sep 17 00:00:00 2001 From: Oleksandr Liemiahov Date: Tue, 2 Jun 2026 18:00:57 +0300 Subject: [PATCH 04/12] fix tests --- .../academy/learning/SolutionLoaderBase.kt | 15 +- .../learning/actions/TaskNavigationAction.kt | 15 +- .../framework/FrameworkLessonManager.kt | 2 +- .../impl/FrameworkLessonManagerImpl.kt | 381 ++++++++++++------ .../framework/impl/LegacyFrameworkStorage.kt | 1 + .../learning/framework/impl/UserChanges.kt | 41 +- .../update/HyperskillCourseUpdater.kt | 8 +- .../academy/learning/submissions/utils.kt | 5 +- .../learning/update/FrameworkLessonHistory.kt | 22 +- .../academy/learning/update/UpdateUtils.kt | 39 +- .../elements/FrameworkTaskUpdateInfo.kt | 83 +++- ...FrameworkLessonSubmissionNavigationTest.kt | 47 +++ .../HyperskillCreateSubmissionTest.kt | 6 - .../hyperskill/HyperskillUploadingTest.kt | 7 +- 14 files changed, 494 insertions(+), 178 deletions(-) diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/SolutionLoaderBase.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/SolutionLoaderBase.kt index d362431a6..645589e75 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/SolutionLoaderBase.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/SolutionLoaderBase.kt @@ -304,11 +304,13 @@ abstract class SolutionLoaderBase(protected val project: Project) : Disposable { // storeOriginalTemplateFiles uses task.taskFiles which may have stale disk content. frameworkLessonManager.ensureTemplateFilesCached(task) - val solutionMap = taskSolutions.visibleSolutions() + val solutionMap = taskSolutions.visibleSolutions(task) frameworkLessonManager.saveExternalChanges(task, solutionMap, taskSolutions.submissionId) var taskFilesChanged = false for ((path, solution) in taskSolutions.solutions) { + if (task.isSubmissionTestFile(path)) continue + val taskFile = task.getTaskFile(path) if (taskFile == null) { if (!solution.isVisible) continue @@ -333,6 +335,8 @@ abstract class SolutionLoaderBase(protected val project: Project) : Disposable { private fun applySolutionToCurrentTask(project: Project, task: Task, taskSolutions: TaskSolutions) { val taskDir = task.getDir(project.courseDir) ?: error("Directory for task `${task.name}` not found") for ((path, solution) in taskSolutions.solutions) { + if (task.isSubmissionTestFile(path)) continue + val taskFile = task.getTaskFile(path) if (taskFile == null) { @@ -368,15 +372,18 @@ abstract class SolutionLoaderBase(protected val project: Project) : Disposable { val lesson = task.lesson if (lesson is FrameworkLesson) { val frameworkLessonManager = FrameworkLessonManager.getInstance(project) - val solutionMap = taskSolutions.visibleSolutions() + val solutionMap = taskSolutions.visibleSolutions(task) frameworkLessonManager.saveExternalChanges(task, solutionMap, taskSolutions.submissionId) } } - private fun TaskSolutions.visibleSolutions(): Map = + private fun TaskSolutions.visibleSolutions(task: Task): Map = solutions - .filter { (_, solution) -> solution.isVisible } + .filter { (path, solution) -> solution.isVisible && !task.isSubmissionTestFile(path) } .mapValues { (_, solution) -> solution.text } + + private fun Task.isSubmissionTestFile(path: String): Boolean = + getTaskFile(path)?.isTestFile ?: EduUtilsKt.isTestsFile(this, path) } protected data class Solution(val text: String, val isVisible: Boolean) diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/actions/TaskNavigationAction.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/actions/TaskNavigationAction.kt index 6472dad04..c267907a5 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/actions/TaskNavigationAction.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/actions/TaskNavigationAction.kt @@ -5,6 +5,9 @@ import com.intellij.openapi.actionSystem.AnActionEvent import com.intellij.openapi.project.DumbAwareAction import com.intellij.openapi.project.Project import org.hyperskill.academy.learning.EduUtilsKt.isEduProject +import org.hyperskill.academy.learning.course +import org.hyperskill.academy.learning.courseFormat.FrameworkLesson +import org.hyperskill.academy.learning.courseFormat.ext.allTasks import org.hyperskill.academy.learning.courseFormat.tasks.Task import org.hyperskill.academy.learning.navigation.NavigationUtils import org.hyperskill.academy.learning.taskToolWindow.ui.TaskToolWindowView @@ -23,7 +26,7 @@ abstract class TaskNavigationAction : DumbAwareAction() { e.presentation.isEnabled = false val project = e.project ?: return if (!project.isEduProject()) return - val currentTask = TaskToolWindowView.getInstance(project).currentTask ?: return + val currentTask = project.currentNavigationTask() ?: return if (getTargetTask(currentTask) != null || getCustomAction(currentTask) != null) { e.presentation.isEnabled = true } @@ -32,7 +35,7 @@ abstract class TaskNavigationAction : DumbAwareAction() { override fun getActionUpdateThread() = ActionUpdateThread.BGT private fun navigateTask(project: Project, place: String) { - val currentTask = TaskToolWindowView.getInstance(project).currentTask ?: return + val currentTask = project.currentNavigationTask() ?: return val customAction = getCustomAction(currentTask) if (customAction != null) { customAction(project, currentTask) @@ -43,5 +46,13 @@ abstract class TaskNavigationAction : DumbAwareAction() { NavigationUtils.navigateToTask(project, targetTask, currentTask) } + private fun Project.currentNavigationTask(): Task? { + TaskToolWindowView.getInstance(this).currentTask?.let { return it } + return course?.allTasks + ?.mapNotNull { it.lesson as? FrameworkLesson } + ?.distinct() + ?.firstNotNullOfOrNull { it.currentTask() } + } + protected abstract fun getTargetTask(sourceTask: Task): Task? } diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/FrameworkLessonManager.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/FrameworkLessonManager.kt index 029209f45..4aac4eb70 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/FrameworkLessonManager.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/FrameworkLessonManager.kt @@ -13,7 +13,7 @@ interface FrameworkLessonManager : EduTestAware { fun preparePrevTask(lesson: FrameworkLesson, taskDir: VirtualFile, showDialogIfConflict: Boolean) fun saveExternalChanges(task: Task, externalState: Map, submissionId: Long? = null) - fun updateUserChanges(task: Task, newInitialState: Map) + fun updateUserChanges(task: Task, newInitialState: Map, newTaskFiles: Map = emptyMap()) /** * Adds new files to an existing snapshot without overwriting existing files. diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt index 1f008e7e7..85d44ddb3 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt @@ -47,7 +47,10 @@ import java.nio.file.Paths * Extension to get storage ref for a task. * @see getStorageRef */ -private fun Task.storageRef(): String = getStorageRef() +private fun Task.storageRef(): String = getStorageRef() + +// PERSISTED - do not rename; stored in framework storage snapshots. +private const val LEARNER_MODIFIED_METADATA_KEY = "learnerModified" /** * Keeps list of [Change]s for each task. Change list is difference between initial task state and latest one. @@ -135,10 +138,9 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson LOG.warn("saveExternalChanges: externalPropagatableFiles.keys=${externalPropagatableFiles.keys}") // Build full snapshot: user files from submission + non-propagatable files from cache. - // Server-provided files win over cache entries so loaded submissions can update - // stage-specific files such as tests or newly visible additional files. + // Submission test files are intentionally ignored; API-provided tests stay authoritative. val (templatePropagatableFiles, _) = task.allFiles.split(task) - val fullSnapshot = buildFullSnapshotState(task, templatePropagatableFiles + externalPropagatableFiles) + externalState.toFileEntries(task) + val fullSnapshot = buildFullSnapshotState(task, templatePropagatableFiles + externalPropagatableFiles) // Save the full snapshot val submissionInfo = if (submissionId != null) " (submission #$submissionId)" else "" @@ -173,7 +175,7 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson LOG.warn("saveExternalChanges: task='${task.name}', saved to ref=$ref, parentRef=$parentRef") } - override fun updateUserChanges(task: Task, newInitialState: Map) { + override fun updateUserChanges(task: Task, newInitialState: Map, newTaskFiles: Map) { require(task.lesson is FrameworkLesson) { "Only framework task snapshots can be updated" } @@ -194,6 +196,8 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson addAll(newInitialState.keys) } + val metadataTaskFiles = newTaskFiles.ifEmpty { task.taskFiles } + for (path in paths) { val currentEntry = currentSnapshot[path] val oldText = oldInitialState[path] @@ -201,21 +205,22 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson if (newText == null) { if (currentEntry != null && (oldText == null || currentEntry.content != oldText)) { - updatedSnapshot[path] = currentEntry + updatedSnapshot[path] = currentEntry.withLearnerModification() } continue } updatedSnapshot[path] = when { - currentEntry == null -> resolveFileEntryMetadata(path, newText, task, task.testDirs) - oldText == null -> currentEntry - currentEntry.content == oldText -> resolveFileEntryMetadata(path, newText, task, task.testDirs) - else -> currentEntry + currentEntry == null -> resolveFileEntryMetadata(path, newText, task, task.testDirs, metadataTaskFiles) + oldText == null -> currentEntry.withLearnerModification() + currentEntry.content == oldText -> resolveFileEntryMetadata(path, newText, task, task.testDirs, metadataTaskFiles) + else -> currentEntry.withLearnerModification() } } try { storage.saveSnapshot(ref, updatedSnapshot, getParentRef(task), "Update initial files for '${task.name}'") + updateOriginalTemplateFilesCache(task, metadataTaskFiles) } catch (e: IOException) { LOG.error("Failed to update snapshot for task '${task.name}'", e) @@ -279,21 +284,28 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson } } - override fun getTaskState(lesson: FrameworkLesson, task: Task): Map { - require(task.lesson == lesson) { - "The task is not a part of this lesson" - } - + override fun getTaskState(lesson: FrameworkLesson, task: Task): Map { + require(task.lesson == lesson) { + "The task is not a part of this lesson" + } + val ref = task.storageRef() if (storage.hasRef(ref)) { try { - return storage.getSnapshot(ref).toContentMap() + val snapshotState = storage.getSnapshot(ref).toContentMap() + if (lesson.currentTaskIndex + 1 == task.index) { + val taskDir = task.getDir(project.courseDir) ?: return snapshotState + val diskState = getAllFilesFromTaskDir(taskDir, task) + val templateState = task.allFiles + return if (diskState == templateState && snapshotState != diskState) snapshotState else diskState + } + return snapshotState } catch (e: IOException) { LOG.warn("Failed to get snapshot for task '${task.name}' (ref=$ref), falling back to templates", e) } } - // For current task without saved storage, read from disk including user-created files. + // Current task may contain unsaved editor changes; read disk/documents before storage. if (lesson.currentTaskIndex + 1 == task.index) { val taskDir = task.getDir(project.courseDir) ?: return emptyMap() return getAllFilesFromTaskDir(taskDir, task) @@ -438,13 +450,13 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson // 4. Get target state directly from storage snapshot (no template-based diff calculation needed) // This is simpler and more reliable than calculating diffs from templates. - val targetState: FLTaskState = if (targetHasStorage) { - try { - storage.getSnapshot(targetRef).toContentMap() - } catch (e: IOException) { - LOG.error("Failed to get snapshot for target task '${targetTask.name}' (ref=$targetRef), falling back to templates", e) - targetTask.allFiles - } + val targetState: FLTaskState = if (targetHasStorage) { + try { + storage.getSnapshot(targetRef).toContentMap().withoutDeletedTemplateFiles(targetTask) + } catch (e: IOException) { + LOG.error("Failed to get snapshot for target task '${targetTask.name}' (ref=$targetRef), falling back to templates", e) + targetTask.allFiles + } } else { // No storage data for target - use template files targetTask.allFiles @@ -461,35 +473,18 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson // - If current commit is NOT ancestor of target commit → check if propagatable files changed // - If only test/hidden files changed (not user code) → auto-Keep merge (record ancestry, keep target's code) val currentCommitIsAncestorOfTarget = targetHasStorage && storage.isAncestor(currentRef, targetRef) - val hasPropagatableChanges = !currentCommitIsAncestorOfTarget && hasPropagatableChangesFromParent(currentRef, currentTask) - val needsMerge = !currentCommitIsAncestorOfTarget && targetHasStorage && taskIndexDelta == 1 && lesson.propagateFilesOnNavigation - val isTestOnlyUpdate = needsMerge && !hasPropagatableChanges - LOG.info("Merge check: currentRef=$currentRef, targetRef=$targetRef, isAncestor=$currentCommitIsAncestorOfTarget, hasPropagatableChanges=$hasPropagatableChanges, needsMerge=$needsMerge, isTestOnlyUpdate=$isTestOnlyUpdate") + val needsMerge = !currentCommitIsAncestorOfTarget && targetHasStorage && taskIndexDelta == 1 && lesson.propagateFilesOnNavigation + LOG.info("Merge check: currentRef=$currentRef, targetRef=$targetRef, isAncestor=$currentCommitIsAncestorOfTarget, needsMerge=$needsMerge") // Track if merge commit was created (to skip redundant snapshot save in step 10) var mergeCommitCreated = false val changes = when { - // Test-only update: only non-propagatable files changed, create auto-Keep merge to record ancestry - isTestOnlyUpdate -> { - mergeCommitCreated = true - LOG.info("Test-only update detected, creating auto-Keep merge commit") - val (targetPropagatableFilesState, _) = targetState.split(targetTask) - val mergeMessage = "Merge from '${currentTask.name}': Auto-keep (only test files changed)" - try { - // Use buildFullSnapshotState to include both user files and test files from target task - val fullSnapshot = buildFullSnapshotState(targetTask, targetPropagatableFilesState) - storage.saveMergeSnapshot(targetRef, fullSnapshot, listOf(targetRef, currentRef), mergeMessage) - LOG.info("Created auto-Keep merge commit for '$targetRef' with parents [$targetRef, $currentRef]: ${fullSnapshot.size} files") - } catch (e: IOException) { - LOG.error("Failed to create auto-Keep merge commit for '$targetRef'", e) - } - calculateChanges(currentState, targetState) - } - needsMerge -> { - mergeCommitCreated = true // Merge commit will be created in calculatePropagationChanges - calculatePropagationChanges(targetTask, currentTask, currentState, targetState, showDialogIfConflict, targetHasStorage, currentRef, targetRef) - } + needsMerge -> { + mergeCommitCreated = true // Merge commit will be created in calculatePropagationChanges + val fullCurrentState = buildFullSnapshotState(currentTask, effectiveCurrentPropagatableFiles).toContentMap() + calculatePropagationChanges(targetTask, currentTask, fullCurrentState, targetState, showDialogIfConflict, targetHasStorage, currentRef, targetRef) + } // First visit to new stage (forward navigation with propagation enabled): // Keep all current files and add only NEW files from target templates !targetHasStorage && taskIndexDelta > 0 && lesson.propagateFilesOnNavigation -> { @@ -913,16 +908,59 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson return nonPropagatableFileChanges + propagatableFileChanges } - // Target task has no saved state - propagate changes without asking - if (!targetHasStorage) { - return calculateCurrentTaskChanges() - } - - // If target snapshot has no propagatable files (empty or only test/hidden files) - auto-Replace without dialog - // Such commits don't represent user changes, so there's nothing meaningful to keep - propagate current state - if (newTargetPropagatableFilesState.isEmpty()) { - LOG.info("Target snapshot has no propagatable files, auto-Replace: propagating current state without dialog") - val mergeMessage = "Merge from '${currentTask.name}': Auto-replace (target had no user files)" + // Target task has no saved state - propagate changes without asking + if (!targetHasStorage) { + return calculateCurrentTaskChanges() + } + + val currentHasLearnerChanges = hasPropagatableChangesFromOriginalTemplate(currentTask, currentPropagatableFilesState) + val targetHasLearnerChanges = hasPropagatableChangesFromOriginalTemplate(targetTask, targetPropagatableFilesState) + LOG.info( + "Merge user-change check: currentRef=$currentRef, targetRef=$targetRef, " + + "currentHasLearnerChanges=$currentHasLearnerChanges, targetHasLearnerChanges=$targetHasLearnerChanges" + ) + + when { + currentHasLearnerChanges && !targetHasLearnerChanges -> { + LOG.info("Auto-Replace for '$targetRef': only current task has learner changes") + saveMergeSnapshot( + targetTask, + targetRef, + currentRef, + currentPropagatableFilesState, + "Merge from '${currentTask.name}': Auto-replace learner changes" + ) + return calculateCurrentTaskChanges() + } + !currentHasLearnerChanges && targetHasLearnerChanges -> { + LOG.info("Auto-Keep for '$targetRef': only target task has learner changes") + saveMergeSnapshot( + targetTask, + targetRef, + currentRef, + targetPropagatableFilesState, + "Merge from '${currentTask.name}': Auto-keep target learner changes" + ) + return calculateChanges(currentState, targetState) + } + !currentHasLearnerChanges && !targetHasLearnerChanges -> { + LOG.info("Auto-Replace for '$targetRef': neither task has learner changes") + saveMergeSnapshot( + targetTask, + targetRef, + currentRef, + currentPropagatableFilesState, + "Merge from '${currentTask.name}': Auto-replace author updates" + ) + return calculateCurrentTaskChanges() + } + } + + // If target snapshot has no propagatable files (empty or only test/hidden files) - auto-Replace without dialog + // Such commits don't represent user changes, so there's nothing meaningful to keep - propagate current state + if (newTargetPropagatableFilesState.isEmpty()) { + LOG.info("Target snapshot has no propagatable files, auto-Replace: propagating current state without dialog") + val mergeMessage = "Merge from '${currentTask.name}': Auto-replace (target had no user files)" try { val currentFileEntries = currentPropagatableFilesState.toFileEntries(currentTask, originalNonPropagatableFilesCache[currentTask.id]) storage.saveMergeSnapshot(targetRef, currentFileEntries, listOf(targetRef, currentRef), mergeMessage) @@ -1032,8 +1070,15 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson if (isPropagatable) { if (path !in currentState) { - LOG.info("First visit: propagating deletion of '$path'") - changes += Change.RemoveTaskFile(path) + val currentTaskFile = currentTask.taskFiles[path] + if (currentTaskFile == null || !currentTaskFile.shouldBePropagated()) { + LOG.info("First visit: adding '$path' that became propagatable in target task") + changes += Change.AddFile(path, text) + } + else { + LOG.info("First visit: propagating deletion of '$path'") + changes += Change.RemoveTaskFile(path) + } } // If it's in both, we keep the user's version from currentState (it's already on disk) } @@ -1114,11 +1159,14 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson // Get current snapshot (Map) val currentSnapshot = storage.getSnapshot(ref) - // Get propagatable file names from task model to identify user files - val propagatableFileNames = task.taskFiles.filterValues { it.shouldBePropagated() }.keys - - // Keep only propagatable files (user files) from existing snapshot - val userFiles = currentSnapshot.filterKeys { path -> path in propagatableFileNames } + // Get propagatable file names from task model to identify user files + val propagatableFileNames = task.taskFiles.filterValues { it.shouldBePropagated() }.keys + + // Keep user files from existing snapshot. A learner-modified file can disappear from + // task.taskFiles after a course update, but must stay in the snapshot for navigation. + val userFiles = currentSnapshot.filter { (path, entry) -> + path in propagatableFileNames || entry.isPropagatable && entry.hasLearnerModification + } // Combine user files with new non-propagatable files from cache // If cache is empty, this effectively removes all non-propagatable files from snapshot @@ -1138,7 +1186,7 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson } else { "Update non-propagatable files from server for '${task.name}'" } - val created = storage.saveSnapshot(ref, updatedSnapshot, null, message) + val created = storage.saveSnapshot(ref, updatedSnapshot, getParentRef(task), message) if (created) { LOG.info("updateSnapshotTestFiles: Updated snapshot for task '${task.name}' with ${cachedNonPropagatableFiles.size} non-propagatable files") } else { @@ -1209,9 +1257,9 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson LOG.warn("storeOriginalTemplateFiles: file='$name', isVisible=${taskFile.isVisible}, isTestFile=${taskFile.isTestFile}") } - val templateFiles = task.taskFiles.filterValues { taskFile -> - taskFile.isVisible && !taskFile.isTestFile - } + val templateFiles = task.taskFiles.filterValues { taskFile -> + taskFile.isVisible && !taskFile.isTestFile && !taskFile.isLearnerCreated + } LOG.warn("storeOriginalTemplateFiles: filtered templateFiles=${templateFiles.keys}") if (templateFiles.isNotEmpty()) { @@ -1234,26 +1282,27 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson } } - override fun updateOriginalTemplateFiles(task: Task) { - // Force update the cache, used when task files are updated from remote server - // (e.g., during course update). Unlike storeOriginalTemplateFiles, this WILL overwrite. - LOG.info("Force updating template files cache for task '${task.name}' (step ${task.id})") - - val templateFiles = task.taskFiles.filterValues { taskFile -> - taskFile.isVisible && !taskFile.isTestFile - } - - if (templateFiles.isNotEmpty()) { - val cachedTemplates = templateFiles.mapValues { (_, taskFile) -> - taskFile.contents.textualRepresentation - } - originalTemplateFilesCache[task.id] = cachedTemplates - val filesInfo = cachedTemplates.entries.joinToString { (name, content) -> - "$name:size=${content.length}" - } - LOG.info("Updated ${cachedTemplates.size} original template files for task '${task.name}' (step ${task.id}): [$filesInfo]") - } - } + override fun updateOriginalTemplateFiles(task: Task) { + // Force update the cache, used when task files are updated from remote server + // (e.g., during course update). Unlike storeOriginalTemplateFiles, this WILL overwrite. + LOG.info("Force updating template files cache for task '${task.name}' (step ${task.id})") + updateOriginalTemplateFilesCache(task, task.taskFiles) + } + + private fun updateOriginalTemplateFilesCache(task: Task, taskFiles: Map) { + val templateFiles = taskFiles.filterValues { taskFile -> + taskFile.isVisible && !taskFile.isTestFile && !taskFile.isLearnerCreated + } + + val cachedTemplates = templateFiles.mapValues { (_, taskFile) -> + taskFile.contents.textualRepresentation + } + originalTemplateFilesCache[task.id] = cachedTemplates + val filesInfo = cachedTemplates.entries.joinToString { (name, content) -> + "$name:size=${content.length}" + } + LOG.info("Updated ${cachedTemplates.size} original template files for task '${task.name}' (step ${task.id}): [$filesInfo]") + } /** * Loads template files from API and caches them. @@ -1424,12 +1473,15 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson .filterValues { !it.isTestFile } .mapValues { it.value.contents.textualRepresentation } - /** - * Returns ALL files for this task including test files. - * Used for creating complete snapshots. - */ - private val Task.allFilesIncludingTests: FLTaskState - get() = taskFiles.mapValues { it.value.contents.textualRepresentation } + /** + * Returns ALL files for this task including test files. + * Used for creating complete snapshots. Learner-created files are excluded because + * this task model state represents author-provided files; learner files are read from disk. + */ + private val Task.allFilesIncludingTests: FLTaskState + get() = taskFiles + .filterValues { !it.isLearnerCreated } + .mapValues { it.value.contents.textualRepresentation } /** * Reads ALL files from task directory, including user-created files. @@ -1475,13 +1527,46 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson } } } - - collectFiles(taskDir) - return result - } - - /** - * Builds complete task state for snapshot: user files from disk + non-propagatable files from cache. + + collectFiles(taskDir) + return result.withoutDeletedTemplateFiles(task) + } + + private fun FLTaskState.withoutDeletedTemplateFiles(task: Task): FLTaskState { + val originalTemplateFiles = originalTemplateFilesCache[task.id] + val deletedTemplateFiles = keys - task.taskFiles.keys + if (deletedTemplateFiles.isEmpty()) return this + + val storedSnapshot = try { + if (storage.hasRef(task.storageRef())) storage.getSnapshot(task.storageRef()) else emptyMap() + } + catch (e: IOException) { + LOG.warn("Failed to load snapshot while filtering deleted template files for '${task.name}'", e) + return this + } + + val filtered = filter { (path, text) -> + if (path !in deletedTemplateFiles) return@filter true + if (storedSnapshot[path]?.hasLearnerModification == true) return@filter true + + val originalText = originalTemplateFiles?.get(path) + when { + originalText != null -> text != originalText + storedSnapshot.containsKey(path) -> storedSnapshot[path]?.content != text + else -> true + } + } + if (filtered.size != size) { + LOG.info( + "Filtered deleted template files from disk state for '${task.name}': " + + (keys - filtered.keys).joinToString() + ) + } + return filtered + } + + /** + * Builds complete task state for snapshot: user files from disk + non-propagatable files from cache. * Non-propagatable files (test files, hidden files) are taken from cache (not disk) * because disk may have files from another stage. * @@ -1519,19 +1604,20 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson * 2. task.taskFiles (from API) * 3. Default metadata (visible, editable, propagatable) */ - private fun resolveFileEntryMetadata( - path: String, - content: String, - task: Task, - testDirs: List - ): FileEntry { + private fun resolveFileEntryMetadata( + path: String, + content: String, + task: Task, + testDirs: List, + taskFiles: Map = task.taskFiles + ): FileEntry { // 1. Path patterns have highest priority - test files are ALWAYS hidden/non-propagatable if (FileEntry.isTestFilePath(path, testDirs)) { return FileEntry.create(content, visible = false, editable = false, propagatable = false) } // 2. Check task.taskFiles - metadata from API (author's intent) - val taskFile = task.taskFiles[path] + val taskFile = taskFiles[path] if (taskFile != null) { return FileEntry.create( content = content, @@ -1545,21 +1631,78 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson return FileEntry(content) } - private fun FLTaskState.splitByKey(predicate: (String) -> Boolean): Pair { - val positive = HashMap() - val negative = HashMap() + private fun FLTaskState.splitByKey(predicate: (String) -> Boolean): Pair { + val positive = HashMap() + val negative = HashMap() for ((path, text) in this) { val state = if (predicate(path)) positive else negative state[path] = text } - - return positive to negative - } - - /** - * Checks if the current ref has propagatable file changes compared to its parent commit. - * Returns true if: + + return positive to negative + } + + private fun saveMergeSnapshot( + targetTask: Task, + targetRef: String, + currentRef: String, + propagatableFilesState: FLTaskState, + message: String + ) { + try { + val fullSnapshot = buildFullSnapshotState(targetTask, propagatableFilesState) + storage.saveMergeSnapshot(targetRef, fullSnapshot, listOf(targetRef, currentRef), message) + LOG.info("Created merge commit for '$targetRef' with parents [$targetRef, $currentRef]: ${fullSnapshot.size} files") + } + catch (e: IOException) { + LOG.error("Failed to create merge commit for '$targetRef'", e) + } + } + + private fun FileEntry.withLearnerModification(): FileEntry { + return copy(metadata = metadata + (LEARNER_MODIFIED_METADATA_KEY to true)) + } + + private val FileEntry.hasLearnerModification: Boolean + get() = metadata[LEARNER_MODIFIED_METADATA_KEY] as? Boolean == true + + private fun hasPropagatableChangesFromOriginalTemplate(task: Task, propagatableFilesState: FLTaskState): Boolean { + try { + if (storage.hasRef(task.storageRef())) { + val learnerModifiedFiles = storage.getSnapshot(task.storageRef()) + .filter { (path, entry) -> path in propagatableFilesState && entry.hasLearnerModification } + if (learnerModifiedFiles.isNotEmpty()) { + LOG.info( + "hasPropagatableChangesFromOriginalTemplate: task='${task.name}' has learner-modified files: " + + learnerModifiedFiles.keys.joinToString() + ) + return true + } + } + } + catch (e: IOException) { + LOG.warn("Failed to inspect learner modification metadata for '${task.name}'", e) + } + + val originalTemplateFiles = originalTemplateFilesCache[task.id] + if (originalTemplateFiles == null) { + LOG.info("hasPropagatableChangesFromOriginalTemplate: no template cache for '${task.name}', falling back to parent comparison") + return hasPropagatableChangesFromParent(task.storageRef(), task) + } + + val (originalPropagatableFiles, _) = originalTemplateFiles.split(task) + val hasChanges = propagatableFilesState != originalPropagatableFiles + LOG.info( + "hasPropagatableChangesFromOriginalTemplate: task='${task.name}', hasChanges=$hasChanges " + + "(current=${propagatableFilesState.size} files, original=${originalPropagatableFiles.size} files)" + ) + return hasChanges + } + + /** + * Checks if the current ref has propagatable file changes compared to its parent commit. + * Returns true if: * - There's no parent (first commit) - considered as having changes * - Propagatable files differ from parent - user made code changes * Returns false if: diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/LegacyFrameworkStorage.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/LegacyFrameworkStorage.kt index f75a22df5..1afe86ecb 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/LegacyFrameworkStorage.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/LegacyFrameworkStorage.kt @@ -122,6 +122,7 @@ class LegacyFrameworkStorage(storagePath: Path) : FrameworkStorageBase(storagePa else { withReadLock { val bytes = readBytes(record) + if (bytes.isEmpty()) return@withReadLock UserChanges.empty() when (version) { 0 -> return@withReadLock readVersion0UserChanges(DataInputStream(java.io.ByteArrayInputStream(bytes))) 1 -> return@withReadLock readLegacyUserChanges(DataInputStream(java.io.ByteArrayInputStream(bytes))) diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/UserChanges.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/UserChanges.kt index 270abea79..154e225fa 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/UserChanges.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/UserChanges.kt @@ -85,22 +85,33 @@ private fun Change.ChangeFile.apply(project: Project, taskDir: VirtualFile, task } else { LOG.warn("ChangeFile.apply: Using document mode") - EduDocumentListener.modifyWithoutListener(task, path) { - val document = runReadAction { FileDocumentManager.getInstance().getDocument(file) } - if (document != null) { - val expandedText = StringUtil.convertLineSeparators(EduMacroUtils.expandMacrosForFile(project.toCourseInfoHolder(), file, text)) - LOG.warn("ChangeFile.apply: Setting document text, expandedTextLength=${expandedText.length}") - file.doWithoutReadOnlyAttribute { - runUndoTransparentWriteAction { document.setText(expandedText) } - } - // ALT-10961: Force save document to disk - FileDocumentManager.getInstance().saveDocument(document) - LOG.warn("ChangeFile.apply: Document text set and saved successfully") - } - else { - LOG.warn("ChangeFile.apply: Can't get document for `$file`") - } + val modification = { + updateTextFile(project, file, text) + } + + if (task.getTaskFile(path) == null) { + modification() + } + else { + EduDocumentListener.modifyWithoutListener(task, path, modification) + } + } +} + +private fun updateTextFile(project: Project, file: VirtualFile, text: String) { + val document = runReadAction { FileDocumentManager.getInstance().getDocument(file) } + if (document != null) { + val expandedText = StringUtil.convertLineSeparators(EduMacroUtils.expandMacrosForFile(project.toCourseInfoHolder(), file, text)) + LOG.warn("ChangeFile.apply: Setting document text, expandedTextLength=${expandedText.length}") + file.doWithoutReadOnlyAttribute { + runUndoTransparentWriteAction { document.setText(expandedText) } } + // ALT-10961: Force save document to disk + FileDocumentManager.getInstance().saveDocument(document) + LOG.warn("ChangeFile.apply: Document text set and saved successfully") + } + else { + LOG.warn("ChangeFile.apply: Can't get document for `$file`") } } diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/update/HyperskillCourseUpdater.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/update/HyperskillCourseUpdater.kt index 28e803df1..2731c76c6 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/update/HyperskillCourseUpdater.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/update/HyperskillCourseUpdater.kt @@ -281,11 +281,9 @@ class HyperskillCourseUpdater(private val project: Project, val course: Hyperski "needsUpdate=${task.updateDate.before(remoteTask.updateDate)}, timestamp=${System.currentTimeMillis()}") if (!task.updateDate.before(remoteTask.updateDate)) continue - // Don't update files for current task - SolutionLoader will apply user's submissions - // This prevents race condition where update overwrites files before submissions are applied - // For non-current tasks, only update if status is Unchecked (no submissions yet) + // Only update unchecked tasks. updateFrameworkLessonFiles preserves locally changed learner files. val isCurrentTask = lesson.currentTaskIndex + 1 == task.index - val shouldUpdateFiles = !isCurrentTask && task.status == CheckStatus.Unchecked + val shouldUpdateFiles = task.status == CheckStatus.Unchecked LOG.warn("UPDATE_PROJECT_LESSON: task='${task.name}' needs update, status=${task.status}, " + "isCurrentTask=$isCurrentTask, shouldUpdateFiles=$shouldUpdateFiles") if (shouldUpdateFiles) { @@ -345,4 +343,4 @@ class HyperskillCourseUpdater(private val project: Project, val course: Hyperski return false } } -} \ No newline at end of file +} diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/submissions/utils.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/submissions/utils.kt index 913a4ea38..9757461e5 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/submissions/utils.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/submissions/utils.kt @@ -16,6 +16,7 @@ import org.hyperskill.academy.learning.courseFormat.EduFormatNames.CORRECT import org.hyperskill.academy.learning.courseFormat.TaskFile import org.hyperskill.academy.learning.courseFormat.ext.findTaskFileInDir import org.hyperskill.academy.learning.courseFormat.ext.getDir +import org.hyperskill.academy.learning.courseFormat.ext.isTestFile import org.hyperskill.academy.learning.courseFormat.tasks.Task import org.hyperskill.academy.learning.taskToolWindow.ui.styleManagers.StyleResourcesManager import org.hyperskill.academy.learning.taskToolWindow.ui.styleManagers.TaskToolWindowBundle @@ -33,7 +34,7 @@ fun getSolutionFiles(project: Project, task: Task): List { val files = ArrayList() val taskDir = task.getDir(project.courseDir) ?: error("Failed to find task directory ${task.name}") - for (taskFile in task.taskFiles.values) { + for (taskFile in task.taskFiles.values.filter { it.isVisible && !it.isTestFile }) { val virtualFile = findTaskFileInDirWithSizeCheck(taskFile, taskDir) ?: continue ApplicationManager.getApplication().runReadAction { @@ -112,4 +113,4 @@ private fun getWrongLinkColor(): String { else { "#${ColorUtil.toHex(EduColors.wrongLabelForeground)}" } -} \ No newline at end of file +} diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/FrameworkLessonHistory.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/FrameworkLessonHistory.kt index 40e9ba79e..909232d60 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/FrameworkLessonHistory.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/FrameworkLessonHistory.kt @@ -37,13 +37,29 @@ class FrameworkLessonHistory private constructor(val taskFileHistories: Map) { +class FrameworkLessonTaskFileHistory private constructor( + private val localHistory: List, + private val remoteHistory: List +) { + /** + * Returns the effective remote contents for a step: author contents with learner changes propagated + * from the matching local history step when propagation rules allow it. + */ fun evaluateContents(index: Int): FileContents? { val step = remoteHistory.getOrNull(index) ?: return null return step.actualContents?.let { InMemoryTextualContents(it) } } + /** + * Returns only the local unmodified baseline for a step. Use this for local-change detection, + * not for writing the effective file contents. + */ + fun evaluateLocalUnmodifiedContents(index: Int): FileContents? { + val step = localHistory.getOrNull(index) ?: return null + return step.unmodifiedContents?.let { InMemoryTextualContents(it) } + } + companion object { fun create( @@ -71,7 +87,7 @@ class FrameworkLessonTaskFileHistory private constructor(private val remoteHisto localHistory.getOrNull(index)?.userModification } - return FrameworkLessonTaskFileHistory(remoteHistory) + return FrameworkLessonTaskFileHistory(localHistory, remoteHistory) } private fun evaluateHistory( @@ -129,4 +145,4 @@ private data class TaskFileStep( * The contents visible at this step */ val actualContents: String? get() = userModification ?: unmodifiedContents -} \ No newline at end of file +} diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/UpdateUtils.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/UpdateUtils.kt index 47f97f635..9fcafd953 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/UpdateUtils.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/UpdateUtils.kt @@ -135,10 +135,22 @@ object UpdateUtils { LOG.info("Found ${newPropagatableFiles.size} new template files from server: ${newPropagatableFiles.keys}") } + val remoteTaskFilesState = remoteTask.taskFiles.mapValues { (_, taskFile) -> taskFile.contents.textualRepresentation } + val hasLocalChanges = task.hasChangedFiles(project) + + fun refreshFrameworkLessonStorage() { + flm.updateOriginalTestFiles(task) + flm.updateSnapshotTestFiles(task) + } + val isCurrentTask = lesson.currentTaskIndex == task.index - 1 if (!isCurrentTask) { - flm.updateUserChanges(task, remoteTask.taskFiles.mapValues { (_, taskFile) -> taskFile.contents.textualRepresentation }) + flm.updateUserChanges( + task, + remoteTaskFilesState, + remoteTask.taskFiles + ) removeDeletedTaskFiles(task, remoteTask, false) updateTaskFiles(task, remoteTask.nonPropagatableFiles, false) // Add new propagatable files to model (not to disk - will be written when navigating) @@ -147,13 +159,25 @@ object UpdateUtils { // Update storage snapshot with new files flm.addNewFilesToSnapshot(task, newPropagatableFiles.mapValues { it.value.contents.textualRepresentation }) } + refreshFrameworkLessonStorage() } else { - if (updatePropagatableFiles && !task.hasChangedFiles(project)) { + if (updatePropagatableFiles && !hasLocalChanges) { + flm.updateUserChanges(task, remoteTaskFilesState, remoteTask.taskFiles) removeDeletedTaskFiles(task, remoteTask, true) updateTaskFiles(task, remoteTask.taskFiles, true) } else { + val preservedPropagatableFiles = task.taskFiles + .filterValues { it.shouldBePropagated() } + .mapValues { (_, taskFile) -> taskFile.contents.textualRepresentation } + val updatedNonPropagatableFiles = remoteTask.nonPropagatableFiles + .mapValues { (_, taskFile) -> taskFile.contents.textualRepresentation } + flm.updateUserChanges( + task, + preservedPropagatableFiles + updatedNonPropagatableFiles, + remoteTask.taskFiles + ) removeDeletedTaskFiles(task, remoteTask, true) updateTaskFiles(task, remoteTask.nonPropagatableFiles, true) // Add new propagatable files to disk and model for current task @@ -162,6 +186,7 @@ object UpdateUtils { // Storage will be updated on next auto-save } } + refreshFrameworkLessonStorage() } // Propagate new files to all subsequent stages (only in non-template-based mode) @@ -169,16 +194,6 @@ object UpdateUtils { if (newPropagatableFiles.isNotEmpty() && lesson.propagateFilesOnNavigation) { propagateNewFilesToSubsequentStages(lesson, task, newPropagatableFiles, flm) } - - // Update the test and template files caches after updating task files from remote. - // This ensures the caches reflect the updated content (ALT-10961). - // Use update* methods to force-update the cache (store* methods won't overwrite). - flm.updateOriginalTestFiles(task) - flm.updateOriginalTemplateFiles(task) - - // Update storage snapshot with new test files from server. - // This ensures navigation uses the updated test files. - flm.updateSnapshotTestFiles(task) } private val Task.nonPropagatableFiles: Map diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/elements/FrameworkTaskUpdateInfo.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/elements/FrameworkTaskUpdateInfo.kt index a949f7f19..b22f01c33 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/elements/FrameworkTaskUpdateInfo.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/elements/FrameworkTaskUpdateInfo.kt @@ -12,8 +12,11 @@ import org.hyperskill.academy.learning.courseDir import org.hyperskill.academy.learning.courseFormat.CheckStatus import org.hyperskill.academy.learning.courseFormat.FileContents import org.hyperskill.academy.learning.courseFormat.FrameworkLesson +import org.hyperskill.academy.learning.courseFormat.InMemoryTextualContents +import org.hyperskill.academy.learning.courseFormat.TaskFile import org.hyperskill.academy.learning.courseFormat.ext.getDir import org.hyperskill.academy.learning.courseFormat.ext.getTaskDirectory +import org.hyperskill.academy.learning.courseFormat.ext.shouldBePropagated import org.hyperskill.academy.learning.courseFormat.tasks.Task import org.hyperskill.academy.learning.courseGeneration.GeneratorUtils import org.hyperskill.academy.learning.framework.FrameworkLessonManager @@ -51,7 +54,15 @@ data class FrameworkTaskUpdateInfo( } remoteItem.record = localItem.record - flManager.updateUserChanges(localItem, remoteItem.taskFiles.mapValues { it.value.contents.textualRepresentation }) + flManager.updateUserChanges( + localItem, + remoteItem.taskFiles.mapValues { it.value.contents.textualRepresentation }, + remoteItem.taskFiles + ) + val initialTaskFileContents = localItem.taskFiles.mapValues { (_, taskFile) -> + taskFile.contents.textualRepresentation + } + val localTaskFiles = localItem.taskFiles.mapValues { (_, taskFile) -> taskFile.copy() } localLesson.replaceItem(localItem, remoteItem) remoteItem.init(localLesson, false) @@ -60,20 +71,36 @@ data class FrameworkTaskUpdateInfo( for ((fileName, fileHistory) in taskHistory.taskFileHistories) { val fileContents = fileHistory.evaluateContents(localLesson.currentTaskIndex) + val localUnmodifiedContents = fileHistory.evaluateLocalUnmodifiedContents(localLesson.currentTaskIndex) + val localText = readLocalText(taskDir, fileName) if (fileContents == null) { - localItem.removeTaskFile(fileName) + if (hasLocalChanges(fileName, localText, localUnmodifiedContents, initialTaskFileContents)) { + thisLogger().info("Preserved local changes for '$fileName', skipping remote deletion") + preserveLocalTaskFile(fileName, localText, localTaskFiles) + continue + } remoteItem.removeTaskFile(fileName) removeFile(taskDir, fileName) } else { - localItem.taskFiles[fileName]?.contents = fileContents - remoteItem.taskFiles[fileName]?.contents = fileContents - val isEditable = remoteItem.taskFiles[fileName]?.isEditable != false + val taskFile = remoteItem.taskFiles[fileName] + if (taskFile?.shouldBePropagated() != false && + hasLocalChanges(fileName, localText, localUnmodifiedContents, initialTaskFileContents)) { + thisLogger().info("Preserved local changes for '$fileName', skipping remote update") + preserveLocalTaskFile(fileName, localText, localTaskFiles) + continue + } + val isEditable = taskFile?.isEditable != false updateFile(project, taskDir, fileName, fileContents, isEditable) + remoteItem.taskFiles[fileName]?.contents = fileContents } } } + flManager.updateOriginalTestFiles(remoteItem) + flManager.updateOriginalTemplateFiles(remoteItem) + flManager.updateSnapshotTestFiles(remoteItem) + YamlFormatSynchronizer.saveItemWithRemoteInfo(remoteItem) } @@ -111,6 +138,52 @@ data class FrameworkTaskUpdateInfo( virtualChangedFile?.delete(this) } + private suspend fun readLocalText(taskDir: VirtualFile, fileName: String): String? { + val virtualFile = readAction { + taskDir.findFileByRelativePath(fileName) + } ?: return null + + return readAction { + FileDocumentManager.getInstance().getDocument(virtualFile)?.text + } + } + + private fun hasLocalChanges( + fileName: String, + localText: String?, + localUnmodifiedContents: FileContents?, + initialTaskFileContents: Map + ): Boolean { + if (localText == null) return false + val initialText = localUnmodifiedContents?.textualRepresentation + ?: initialTaskFileContents[fileName] + return initialText == null || localText != initialText + } + + private fun preserveLocalTaskFile(fileName: String, localText: String?, localTaskFiles: Map) { + val localTaskFile = localTaskFiles[fileName] ?: return + val preservedContents = localText?.let(::InMemoryTextualContents) ?: localTaskFile.contents + val remoteTaskFile = remoteItem.taskFiles[fileName] + if (remoteTaskFile == null) { + remoteItem.addTaskFile(localTaskFile.copy(preservedContents)) + } + else { + remoteTaskFile.contents = preservedContents + } + } + + private fun TaskFile.copy(contents: FileContents = this.contents): TaskFile { + return TaskFile(name, contents).also { copy -> + copy.isVisible = isVisible + copy.isEditable = isEditable + copy.isPropagatable = isPropagatable + copy.isLearnerCreated = isLearnerCreated + copy.isTrackChanges = isTrackChanges + copy.errorHighlightLevel = errorHighlightLevel + copy.additionalProperties = additionalProperties.toMutableMap() + } + } + private suspend fun updateFile(project: Project, taskDir: VirtualFile, fileName: String, contents: FileContents, isEditable: Boolean) { val virtualChangedFile = readAction { diff --git a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/actions/navigate/FrameworkLessonSubmissionNavigationTest.kt b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/actions/navigate/FrameworkLessonSubmissionNavigationTest.kt index 01b3b42c5..957dbed05 100644 --- a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/actions/navigate/FrameworkLessonSubmissionNavigationTest.kt +++ b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/actions/navigate/FrameworkLessonSubmissionNavigationTest.kt @@ -229,6 +229,53 @@ class FrameworkLessonSubmissionNavigationTest : NavigationTestBase() { assertEquals("CONTENT_3_EVEN_LONGER_TEXT", state3["src/Task.kt"]) } + @Test + fun `test saveExternalChanges keeps template tests when submission contains visible test file`() { + val course = createFrameworkCourseWithDifferentContent() + val task2 = course.findTask("lesson1", "task2") + val lesson = task2.lesson as FrameworkLesson + + val frameworkLessonManager = FrameworkLessonManager.getInstance(project) + frameworkLessonManager.cleanUpState() + frameworkLessonManager.restoreState() + + frameworkLessonManager.saveExternalChanges( + task2, + mapOf( + "src/Task.kt" to "SUBMISSION_CONTENT", + "test/Tests2.kt" to "STALE_SUBMISSION_TEST" + ) + ) + + val state = frameworkLessonManager.getTaskState(lesson, task2) + assertEquals("SUBMISSION_CONTENT", state["src/Task.kt"]) + assertEquals("fun tests2() {}", state["test/Tests2.kt"]) + } + + @Test + fun `test first visit to next stage adds target-only propagatable template file`() { + val course = courseWithFiles(language = FakeGradleBasedLanguage) { + frameworkLesson("lesson1", isTemplateBased = false) { + eduTask("task1", stepId = 1001) { + taskFile("src/Task.kt", "fun stage1() {}") + } + eduTask("task2", stepId = 1002) { + taskFile("src/Task.kt", "fun stage2() {}") + taskFile("src/NewTemplate.kt", "fun newTemplate() {}") + } + } + } + + withVirtualFileListener(course) { + val task1 = course.findTask("lesson1", "task1") + task1.openTaskFileInEditor("src/Task.kt") + + testAction(NextTaskAction.ACTION_ID) + } + + assertEquals("fun newTemplate() {}", findFileInTaskDir("src/NewTemplate.kt").contentsToByteArray().decodeToString()) + } + private fun findFileInTaskDir(relativePath: String) = rootDir.findFileByRelativePath("lesson1/task/$relativePath") ?: error("File not found: lesson1/task/$relativePath") diff --git a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillCreateSubmissionTest.kt b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillCreateSubmissionTest.kt index 546b60388..d6f98492a 100644 --- a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillCreateSubmissionTest.kt +++ b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillCreateSubmissionTest.kt @@ -54,8 +54,6 @@ class HyperskillCreateSubmissionTest : EduTestCase() { | solution: | - name: src/Task.kt | is_visible: true - | - name: src/Test.kt - | is_visible: false | """.trimMargin() ) @@ -80,8 +78,6 @@ class HyperskillCreateSubmissionTest : EduTestCase() { | solution: | - name: src/Task.kt | is_visible: true - | - name: src/Test.kt - | is_visible: false | """.trimMargin() ) @@ -104,8 +100,6 @@ class HyperskillCreateSubmissionTest : EduTestCase() { | solution: | - name: src/Task.kt | is_visible: true - | - name: src/Test.kt - | is_visible: false | check_profile: $checkProfile | """.trimMargin() diff --git a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUploadingTest.kt b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUploadingTest.kt index 1645fe483..11d80822b 100644 --- a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUploadingTest.kt +++ b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUploadingTest.kt @@ -15,10 +15,8 @@ class HyperskillUploadingTest : EduTestCase() { val course = createHyperskillCourse() val task = course.findTask("lesson1", "task1") val files = getSolutionFiles(project, task) - assertEquals(3, files.size) - for (file in files) { - assertEquals(mapOf("src/Task.kt" to true, "src/Baz.kt" to false, "test/Tests1.kt" to false)[file.name], file.isVisible) - } + assertEquals(listOf("src/Task.kt"), files.map { it.name }) + assertTrue(files.all { it.isVisible }) } private fun createHyperskillCourse(): HyperskillCourse { @@ -31,6 +29,7 @@ class HyperskillUploadingTest : EduTestCase() { taskFile("src/Task.kt", "fun foo() {}", visible = true) taskFile("src/Baz.kt", "fun baz() {}", visible = false) taskFile("test/Tests1.kt", "fun tests1() {}", visible = false) + taskFile("test/VisibleTests.kt", "fun visibleTests() {}", visible = true) } } } as HyperskillCourse From 24d095ed75e79b0328a83792abe790d82ad8ac77 Mon Sep 17 00:00:00 2001 From: Oleksandr Liemiahov Date: Mon, 8 Jun 2026 17:23:47 +0300 Subject: [PATCH 05/12] fix github actions build --- .../academy/learning/EduProjectActivity.kt | 24 +++++++++++ .../impl/FrameworkLessonManagerImpl.kt | 42 +++++++++++++++---- .../elements/FrameworkTaskUpdateInfo.kt | 29 ++++++++++++- .../academy/learning/EduTestCase.kt | 4 ++ 4 files changed, 90 insertions(+), 9 deletions(-) diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/EduProjectActivity.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/EduProjectActivity.kt index 7fb8f22e1..f4df7650c 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/EduProjectActivity.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/EduProjectActivity.kt @@ -24,8 +24,10 @@ import org.hyperskill.academy.coursecreator.framework.SyncChangesStateManager import org.hyperskill.academy.learning.EduUtilsKt.isEduProject import org.hyperskill.academy.learning.EduUtilsKt.isNewlyCreated import org.hyperskill.academy.learning.courseFormat.Course +import org.hyperskill.academy.learning.courseFormat.FrameworkLesson import org.hyperskill.academy.learning.courseFormat.ext.configurator import org.hyperskill.academy.learning.courseFormat.hyperskill.HyperskillCourse +import org.hyperskill.academy.learning.framework.FrameworkLessonManager import org.hyperskill.academy.learning.handlers.UserCreatedFileListener import org.hyperskill.academy.learning.messages.EduCoreBundle import org.hyperskill.academy.learning.navigation.NavigationUtils @@ -58,6 +60,8 @@ class EduProjectActivity : ProjectActivity { return@trackActivity } + cacheFrameworkLessonTemplates(project, course) + withContext(Dispatchers.EDT) { val fileEditorManager = FileEditorManager.getInstance(project) if (!fileEditorManager.hasOpenFiles() && !SubmissionSettings.getInstance(project).stateOnClose) { @@ -147,6 +151,26 @@ class EduProjectActivity : ProjectActivity { } } + /** + * Captures the original propagatable template (visible non-test files) of every framework task + * before the learner can edit. During first-visit navigation this lets us tell a template file + * the learner deleted apart from a genuinely new author template introduced in the target stage. + * + * Hyperskill courses populate this cache from API when opening a stage in the IDE, so we only need + * to cover regular (non-Hyperskill) framework courses here. + */ + private fun cacheFrameworkLessonTemplates(project: Project, course: Course) { + if (course is HyperskillCourse) return + val frameworkLessonManager = FrameworkLessonManager.getInstance(project) + course.visitLessons { lesson -> + if (lesson is FrameworkLesson) { + for (task in lesson.taskList) { + frameworkLessonManager.updateOriginalTemplateFiles(task) + } + } + } + } + companion object { private val LOG: Logger = logger() diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt index 85d44ddb3..3541f956b 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt @@ -1071,13 +1071,41 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson if (isPropagatable) { if (path !in currentState) { val currentTaskFile = currentTask.taskFiles[path] - if (currentTaskFile == null || !currentTaskFile.shouldBePropagated()) { - LOG.info("First visit: adding '$path' that became propagatable in target task") - changes += Change.AddFile(path, text) - } - else { - LOG.info("First visit: propagating deletion of '$path'") - changes += Change.RemoveTaskFile(path) + val originalCurrentTemplate = originalTemplateFilesCache[currentTask.id] + val wasInCurrentTemplate = originalCurrentTemplate?.containsKey(path) ?: false + // The learner diverged from the current task's template if they added or removed + // propagatable files (compared with the captured original template). When diverged, their + // current state defines the propagatable files (see FrameworkLesson.propagateFilesOnNavigation), + // so an author template file absent from it is discarded rather than re-added. + val learnerDiverged = originalCurrentTemplate != null && + currentState.keys != originalCurrentTemplate.keys + when { + currentTaskFile != null && !currentTaskFile.shouldBePropagated() -> { + // The file was non-propagatable (e.g. invisible) in the current task and becomes + // propagatable (visible) in the target task. Changes for such files are not propagated, + // so we add the author's version. + LOG.info("First visit: adding '$path' that became propagatable in target task") + changes += Change.AddFile(path, text) + } + wasInCurrentTemplate -> { + // The file existed in the current task's original propagatable template but is absent + // from the learner's current state, i.e. the learner deleted it. Respect that deletion. + LOG.info("First visit: discarding propagatable file '$path' the learner deleted") + changes += Change.RemoveTaskFile(path) + } + learnerDiverged -> { + // The learner replaced the propagatable files (added/removed files), so an author + // template file absent from their state is discarded rather than re-added. + LOG.info("First visit: discarding author propagatable file '$path' (learner replaced propagatable files)") + changes += Change.RemoveTaskFile(path) + } + else -> { + // Regular course: the file is a genuinely new template the author introduced in the + // target stage (it was never part of the current task's template, so it could not have + // been deleted by the learner). Add the author's version. + LOG.info("First visit: adding new author template file '$path' introduced in target task") + changes += Change.AddFile(path, text) + } } } // If it's in both, we keep the user's version from currentState (it's already on disk) diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/elements/FrameworkTaskUpdateInfo.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/elements/FrameworkTaskUpdateInfo.kt index b22f01c33..fc669f1df 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/elements/FrameworkTaskUpdateInfo.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/update/elements/FrameworkTaskUpdateInfo.kt @@ -1,5 +1,6 @@ package org.hyperskill.academy.learning.update.elements +import com.intellij.openapi.application.EDT import com.intellij.openapi.application.readAction import com.intellij.openapi.application.writeAction import com.intellij.openapi.diagnostic.thisLogger @@ -23,6 +24,8 @@ import org.hyperskill.academy.learning.framework.FrameworkLessonManager import org.hyperskill.academy.learning.toCourseInfoHolder import org.hyperskill.academy.learning.update.FrameworkLessonHistory import org.hyperskill.academy.learning.yaml.YamlFormatSynchronizer +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext import java.io.IOException data class FrameworkTaskUpdateInfo( @@ -38,6 +41,17 @@ data class FrameworkTaskUpdateInfo( val taskIsCurrent = localLesson.currentTaskIndex == localItem.index - 1 + if (taskIsCurrent) { + // The learner's most recent edits on the current stage may live only in in-memory editor + // documents and not yet be flushed to disk. Persist them before reading local text and + // overwriting files, so that preserved learner changes survive the later VFS refresh + // (e.g. in `updateFile`/`reloadFiles` or when the project is reopened). This mirrors the + // document flush performed during framework lesson navigation. + withContext(Dispatchers.EDT) { + FileDocumentManager.getInstance().saveAllDocuments() + } + } + if (localItem.status != CheckStatus.Solved) { remoteItem.status = CheckStatus.Unchecked } @@ -86,8 +100,19 @@ data class FrameworkTaskUpdateInfo( val taskFile = remoteItem.taskFiles[fileName] if (taskFile?.shouldBePropagated() != false && hasLocalChanges(fileName, localText, localUnmodifiedContents, initialTaskFileContents)) { - thisLogger().info("Preserved local changes for '$fileName', skipping remote update") - preserveLocalTaskFile(fileName, localText, localTaskFiles) + // Keep the learner's edits on disk (they are already flushed there) by skipping the + // remote overwrite. + if (taskFile != null) { + // The file still exists remotely: keep the author's updated contents in the task + // model (already set by the preceding replaceItem/remoteItem.init) so that a later + // revert restores the author version. Only the learner's file on disk is kept. + thisLogger().info("Preserved local changes for '$fileName' on disk, keeping author contents in model") + } + else { + // The file no longer exists remotely: preserve it in the model so it stays tracked. + thisLogger().info("Preserved local changes for '$fileName', skipping remote update") + preserveLocalTaskFile(fileName, localText, localTaskFiles) + } continue } val isEditable = taskFile?.isEditable != false diff --git a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/EduTestCase.kt b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/EduTestCase.kt index 63e1c46e2..2b45c414b 100644 --- a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/EduTestCase.kt +++ b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/EduTestCase.kt @@ -181,6 +181,10 @@ abstract class EduTestCase : BasePlatformTestCase() { for (lesson in course.lessons.filterIsInstance()) { for (task in lesson.taskList) { frameworkLessonManager.storeOriginalTestFiles(task) + // Capture each task's original propagatable template (visible non-test files) before the + // learner can edit. Used during first-visit navigation to tell a learner-deleted template + // file apart from a genuinely new author template introduced in the target stage. + frameworkLessonManager.updateOriginalTemplateFiles(task) } } } From e8070a1702a1f610ca8c4cb6f4c34f8d20f53765 Mon Sep 17 00:00:00 2001 From: Oleksandr Liemiahov Date: Mon, 8 Jun 2026 17:44:15 +0300 Subject: [PATCH 06/12] fix github actions build --- .../src/org/hyperskill/academy/learning/submissions/utils.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/submissions/utils.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/submissions/utils.kt index a406f9252..7532921d8 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/submissions/utils.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/submissions/utils.kt @@ -33,7 +33,8 @@ fun getSolutionFiles(project: Project, task: Task): List { val files = ArrayList() val taskDir = task.getDir(project.courseDir) ?: error("Failed to find task directory ${task.name}") - for (taskFile in task.taskFiles.values.filter { it.isVisible }) { + // Don't filter files by visibility and isTestFile flag. It will break server validations of some tasks + for (taskFile in task.taskFiles.values) { val virtualFile = findTaskFileInDirWithSizeCheck(taskFile, taskDir) ?: continue ApplicationManager.getApplication().runReadAction { From 7da8a210dbe47730a44bf18a51636542c084a5dc Mon Sep 17 00:00:00 2001 From: Oleksandr Liemiahov Date: Mon, 8 Jun 2026 18:12:18 +0300 Subject: [PATCH 07/12] fix github actions build --- gradle-261.properties | 8 ++++---- .../stepik/hyperskill/HyperskillCreateSubmissionTest.kt | 6 ++++++ .../learning/stepik/hyperskill/HyperskillUploadingTest.kt | 4 ++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/gradle-261.properties b/gradle-261.properties index 3074b83cb..226859965 100644 --- a/gradle-261.properties +++ b/gradle-261.properties @@ -4,7 +4,7 @@ customUntilBuild=261.* # Existent IDE versions can be found in the following repos: # https://www.jetbrains.com/intellij-repository/releases/ # https://www.jetbrains.com/intellij-repository/snapshots/ -ideaVersion=IU-2026.1.1 -clionVersion=CL-2026.1.1 -pycharmVersion=PC-2026.1 -riderVersion=RD-2026.1.0.1 +ideaVersion=IU-2026.1.3 +clionVersion=CL-2026.1.2 +pycharmVersion=PC-2026.1.2 +riderVersion=RD-2026.1.2 diff --git a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillCreateSubmissionTest.kt b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillCreateSubmissionTest.kt index d6f98492a..546b60388 100644 --- a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillCreateSubmissionTest.kt +++ b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillCreateSubmissionTest.kt @@ -54,6 +54,8 @@ class HyperskillCreateSubmissionTest : EduTestCase() { | solution: | - name: src/Task.kt | is_visible: true + | - name: src/Test.kt + | is_visible: false | """.trimMargin() ) @@ -78,6 +80,8 @@ class HyperskillCreateSubmissionTest : EduTestCase() { | solution: | - name: src/Task.kt | is_visible: true + | - name: src/Test.kt + | is_visible: false | """.trimMargin() ) @@ -100,6 +104,8 @@ class HyperskillCreateSubmissionTest : EduTestCase() { | solution: | - name: src/Task.kt | is_visible: true + | - name: src/Test.kt + | is_visible: false | check_profile: $checkProfile | """.trimMargin() diff --git a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUploadingTest.kt b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUploadingTest.kt index d412ab946..878ac2a47 100644 --- a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUploadingTest.kt +++ b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUploadingTest.kt @@ -15,8 +15,8 @@ class HyperskillUploadingTest : EduTestCase() { val course = createHyperskillCourse() val task = course.findTask("lesson1", "task1") val files = getSolutionFiles(project, task) - assertEquals(listOf("src/Task.kt", "test/VisibleTests.kt"), files.map { it.name }) - assertTrue(files.all { it.isVisible }) + assertEquals(listOf("src/Task.kt", "src/Baz.kt", "test/Tests1.kt", "test/VisibleTests.kt"), files.map { it.name }) + assertEquals(listOf(true, false, false, true), files.map { it.isVisible }) } private fun createHyperskillCourse(): HyperskillCourse { From 4bd3af71851351b6e855a751cf456de79561c512 Mon Sep 17 00:00:00 2001 From: Oleksandr Liemiahov Date: Wed, 10 Jun 2026 13:34:03 +0300 Subject: [PATCH 08/12] fix github actions build --- intellij-plugin/build.gradle.kts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/intellij-plugin/build.gradle.kts b/intellij-plugin/build.gradle.kts index 21200b939..fe0c6484c 100644 --- a/intellij-plugin/build.gradle.kts +++ b/intellij-plugin/build.gradle.kts @@ -3,6 +3,7 @@ import org.jetbrains.intellij.platform.gradle.IntelliJPlatformType.* import org.jetbrains.intellij.platform.gradle.extensions.IntelliJPlatformTestingExtension import org.jetbrains.intellij.platform.gradle.tasks.PrepareSandboxTask import org.jetbrains.intellij.platform.gradle.tasks.RunIdeTask +import org.jetbrains.intellij.platform.gradle.tasks.VerifyPluginTask import org.jetbrains.intellij.platform.gradle.utils.extensionProvider plugins { @@ -42,6 +43,23 @@ intellijPlatform { buildSearchableOptions = prop("enableBuildSearchableOptions").toBoolean() pluginVerification { + // Fail only on genuinely broken structure / hard compatibility breaks. + // + // Excluded on purpose (reported in the report but non-failing): + // - INTERNAL_API_USAGES / EXPERIMENTAL / DEPRECATED / SCHEDULED_FOR_REMOVAL: + // informational; JetBrains Marketplace treats them the same way. + // - MISSING_DEPENDENCIES: this is a multi-IDE plugin. IDE-specific content + // modules (hs-Cpp:CLion-Classic -> com.intellij.cidr.lang, + // hs-CSharp -> com.intellij.modules.rider / intellij.rider / + // com.intellij.resharper.unity) intentionally declare deps that don't exist + // in IDEA Ultimate. Such modules are simply not loaded there; the plugin + // still loads and is reported Compatible. + failureLevel = listOf( + VerifyPluginTask.FailureLevel.COMPATIBILITY_PROBLEMS, + VerifyPluginTask.FailureLevel.NON_EXTENDABLE_API_USAGES, + VerifyPluginTask.FailureLevel.OVERRIDE_ONLY_API_USAGES, + VerifyPluginTask.FailureLevel.INVALID_PLUGIN, + ) ides { intellijIde(ideaVersion) } From 69d1263733e9bed2c04cc6cf7de4c73a31d28e06 Mon Sep 17 00:00:00 2001 From: Oleksandr Liemiahov Date: Thu, 11 Jun 2026 11:32:16 +0300 Subject: [PATCH 09/12] fix github actions build --- .../framework/impl/FrameworkLessonManagerImpl.kt | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt index 3541f956b..747b11320 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt @@ -296,8 +296,14 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson if (lesson.currentTaskIndex + 1 == task.index) { val taskDir = task.getDir(project.courseDir) ?: return snapshotState val diskState = getAllFilesFromTaskDir(taskDir, task) - val templateState = task.allFiles - return if (diskState == templateState && snapshotState != diskState) snapshotState else diskState + // Compare only propagatable (user-editable) files: test/hidden files on disk are + // stage-specific and absent from the template (task.allFiles), so including them would + // make this "disk still holds the pristine template" check effectively never match. + val diskPropagatableFiles = diskState.split(task).first + val templatePropagatableFiles = task.allFiles.split(task).first + val snapshotPropagatableFiles = snapshotState.split(task).first + val diskIsPristineTemplate = diskPropagatableFiles == templatePropagatableFiles + return if (diskIsPristineTemplate && snapshotPropagatableFiles != diskPropagatableFiles) snapshotState else diskState } return snapshotState } catch (e: IOException) { From d9b59aab10a393a529c68784e0598ed07acd9719 Mon Sep 17 00:00:00 2001 From: Oleksandr Liemiahov Date: Fri, 12 Jun 2026 11:04:37 +0300 Subject: [PATCH 10/12] fix github actions build --- .../learning/stepik/hyperskill/HyperskillUploadingTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUploadingTest.kt b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUploadingTest.kt index 0518353b9..1758dffb6 100644 --- a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUploadingTest.kt +++ b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/HyperskillUploadingTest.kt @@ -16,7 +16,8 @@ class HyperskillUploadingTest : EduTestCase() { val task = course.findTask("lesson1", "task1") val files = getSolutionFiles(project, task) assertEquals(listOf("src/Task.kt", "src/Baz.kt", "test/Tests1.kt", "test/VisibleTests.kt"), files.map { it.name }) - assertEquals(listOf(true, false, false, true), files.map { it.isVisible }) + // test files are always sent with is_visible=false, even if marked visible (ALT-11013) + assertEquals(listOf(true, false, false, false), files.map { it.isVisible }) } @Test From e53dd360293074648a22653e4c1760d80ecfd343 Mon Sep 17 00:00:00 2001 From: Oleksandr Liemiahov Date: Thu, 18 Jun 2026 13:28:54 +0300 Subject: [PATCH 11/12] fix: address PR #41 review feedback LegacyFrameworkStorage: - import java.io.ByteArrayInputStream instead of inline fully-qualified names - document the deliberate version fall-through in both record readers Logging: - downgrade debug-level LOG.warn traces to LOG.debug in FrameworkLessonManagerImpl and UserChanges; keep genuine warnings/caught exceptions at WARN Cleanup: - drop the SlowOperations.knownIssue("EDU-XXXX") placeholder wrappers and the now-unused import (call YamlFormatSynchronizer.saveItem directly) Remove isUnitTestMode test-only forks (use injectable seams so tests exercise the real production path): - FrameworkLessonManagerImpl: remove the test-only currentTask.record assignment; the update test now seeds the legacy record itself - HyperskillOpenInIdeRequestHandler: inject the stages executor (and loader) so the CompletableFuture/timeout/cancellation path runs in tests instead of a synchronous short-circuit; add HyperskillLoadStagesWithTimeoutTest covering Ok / timeout / failure / ProcessCanceledException --- .../impl/FrameworkLessonManagerImpl.kt | 36 +++++------- .../framework/impl/LegacyFrameworkStorage.kt | 19 +++--- .../learning/framework/impl/UserChanges.kt | 16 ++--- .../HyperskillOpenInIdeRequestHandler.kt | 27 +++++---- .../HyperskillLoadStagesWithTimeoutTest.kt | 58 +++++++++++++++++++ .../HyperskillProjectOpenerTestBase.kt | 10 ++++ .../update/FrameworkLessonsUpdateTest.kt | 7 +++ 7 files changed, 124 insertions(+), 49 deletions(-) create mode 100644 intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/projectOpen/HyperskillLoadStagesWithTimeoutTest.kt diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt index 747b11320..cc598df53 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt @@ -13,7 +13,6 @@ import com.intellij.openapi.util.Disposer import com.intellij.openapi.util.io.FileUtil import com.intellij.openapi.vfs.VfsUtil import com.intellij.openapi.vfs.VirtualFile -import com.intellij.util.SlowOperations import org.hyperskill.academy.learning.* import org.hyperskill.academy.learning.configuration.excludeFromArchive import org.hyperskill.academy.learning.courseFormat.CheckStatus @@ -131,11 +130,11 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson val ref = task.storageRef() val parentRef = getParentRef(task) - LOG.warn("saveExternalChanges: task='${task.name}', ref=$ref, submissionId=$submissionId, externalState.keys=${externalState.keys}") + LOG.debug("saveExternalChanges: task='${task.name}', ref=$ref, submissionId=$submissionId, externalState.keys=${externalState.keys}") // Filter external state to only include propagatable files (exclude test files from submission) val externalPropagatableFiles = externalState.split(task).first - LOG.warn("saveExternalChanges: externalPropagatableFiles.keys=${externalPropagatableFiles.keys}") + LOG.debug("saveExternalChanges: externalPropagatableFiles.keys=${externalPropagatableFiles.keys}") // Build full snapshot: user files from submission + non-propagatable files from cache. // Submission test files are intentionally ignored; API-provided tests stay authoritative. @@ -172,7 +171,7 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson task.record = -1 YamlFormatSynchronizer.saveItem(task) } - LOG.warn("saveExternalChanges: task='${task.name}', saved to ref=$ref, parentRef=$parentRef") + LOG.debug("saveExternalChanges: task='${task.name}', saved to ref=$ref, parentRef=$parentRef") } override fun updateUserChanges(task: Task, newInitialState: Map, newTaskFiles: Map) { @@ -383,9 +382,7 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson } lesson.currentTaskIndex = targetTaskIndex - SlowOperations.knownIssue("EDU-XXXX").use { - YamlFormatSynchronizer.saveItem(lesson) - } + YamlFormatSynchronizer.saveItem(lesson) logTiming("saveLesson") val currentRef = currentTask.storageRef() @@ -433,9 +430,6 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson val navMessage = "Save changes before navigating from '${currentTask.name}' to '${targetTask.name}'" try { storage.saveSnapshot(currentRef, fullSnapshot, getParentRef(currentTask), navMessage) - if (isUnitTestMode && currentTask.record == -1) { - currentTask.record = currentTask.index - } LOG.info("Saved full snapshot for current task '${currentTask.name}' (ref=$currentRef): ${fullSnapshot.size} files") } catch (e: IOException) { @@ -452,7 +446,7 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson // For forward navigation: use disk state (we just saved it) // For backward navigation: use disk state (what's currently there) val currentState: FLTaskState = effectiveCurrentPropagatableFiles - LOG.warn("Navigation: currentState=${currentState.mapValues { "${it.key}:${it.value.length}chars" }}") + LOG.debug("Navigation: currentState=${currentState.mapValues { "${it.key}:${it.value.length}chars" }}") // 4. Get target state directly from storage snapshot (no template-based diff calculation needed) // This is simpler and more reliable than calculating diffs from templates. @@ -468,7 +462,7 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson targetTask.allFiles } logTiming("getTargetState") - LOG.warn("Navigation: targetState=${targetState.mapValues { "${it.key}:${it.value.length}chars" }}, fromStorage=$targetHasStorage") + LOG.debug("Navigation: targetState=${targetState.mapValues { "${it.key}:${it.value.length}chars" }}, fromStorage=$targetHasStorage") // 5. Calculate difference between latest states of current and target tasks // Note, there are special rules for hyperskill courses for now @@ -508,9 +502,7 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson val taskFilesChanged = changes.changes.any { it is Change.PropagateLearnerCreatedTaskFile || it is Change.RemoveTaskFile } changes.apply(project, taskDir, targetTask) if (taskFilesChanged) { - SlowOperations.knownIssue("EDU-XXXX").use { - YamlFormatSynchronizer.saveItem(targetTask) - } + YamlFormatSynchronizer.saveItem(targetTask) } logTiming("applyChanges") @@ -1286,15 +1278,15 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson return } - LOG.warn("storeOriginalTemplateFiles: task='${task.name}', taskFiles=${task.taskFiles.keys}") + LOG.debug("storeOriginalTemplateFiles: task='${task.name}', taskFiles=${task.taskFiles.keys}") task.taskFiles.forEach { (name, taskFile) -> - LOG.warn("storeOriginalTemplateFiles: file='$name', isVisible=${taskFile.isVisible}, isTestFile=${taskFile.isTestFile}") + LOG.debug("storeOriginalTemplateFiles: file='$name', isVisible=${taskFile.isVisible}, isTestFile=${taskFile.isTestFile}") } val templateFiles = task.taskFiles.filterValues { taskFile -> taskFile.isVisible && !taskFile.isTestFile && !taskFile.isLearnerCreated } - LOG.warn("storeOriginalTemplateFiles: filtered templateFiles=${templateFiles.keys}") + LOG.debug("storeOriginalTemplateFiles: filtered templateFiles=${templateFiles.keys}") if (templateFiles.isNotEmpty()) { // Store only the content (as String), not the TaskFile objects @@ -1809,7 +1801,7 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson } val result = taskFile?.shouldBePropagated() ?: true if (!result) { - LOG.warn("split: path='$path' excluded, isVisible=${taskFile.isVisible}, isEditable=${taskFile.isEditable}") + LOG.debug("split: path='$path' excluded, isVisible=${taskFile.isVisible}, isEditable=${taskFile.isEditable}") } result } @@ -1835,7 +1827,7 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson // HEAD exists - find the task whose storageRef matches HEAD val taskIndex = lesson.taskList.indexOfFirst { it.storageRef() == head } if (taskIndex == -1) { - LOG.warn("syncCurrentTaskIndexFromStorage: HEAD=$head but no task found with matching storageRef") + LOG.debug("syncCurrentTaskIndexFromStorage: HEAD=$head but no task found with matching storageRef") // Still mark as synced to allow auto-save to work isStorageSynced = true return false @@ -1892,12 +1884,12 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson private fun createStorage(project: Project): FrameworkStorage { val storageFilePath = constructStoragePath(project) val storageExists = storageFilePath.toFile().exists() - LOG.warn("CREATE_STORAGE: path=$storageFilePath, exists=$storageExists") + LOG.debug("CREATE_STORAGE: path=$storageFilePath, exists=$storageExists") try { val storage = FrameworkStorage(storageFilePath) storage.migrate(VERSION) - LOG.warn("CREATE_STORAGE: success, version=${storage.version}") + LOG.debug("CREATE_STORAGE: success, version=${storage.version}") return storage } catch (e: Exception) { diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/LegacyFrameworkStorage.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/LegacyFrameworkStorage.kt index 1afe86ecb..b4fd94c05 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/LegacyFrameworkStorage.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/LegacyFrameworkStorage.kt @@ -6,6 +6,7 @@ import com.intellij.util.io.UnsyncByteArrayOutputStream import org.hyperskill.academy.learning.framework.impl.migration.To1VersionRecordConverter import org.hyperskill.academy.learning.framework.storage.Change import org.hyperskill.academy.learning.framework.storage.UserChanges +import java.io.ByteArrayInputStream import java.io.DataInput import java.io.DataInputStream import java.io.DataOutputStream @@ -32,7 +33,7 @@ class LegacyFrameworkStorage(storagePath: Path) : FrameworkStorageBase(storagePa val recordId = recordIterator.nextId() val bytes = readBytes(recordId) if (bytes.isNotEmpty() && bytes[0].toInt() == BLOB_TYPE) { - val input = DataInputStream(java.io.ByteArrayInputStream(bytes)) + val input = DataInputStream(ByteArrayInputStream(bytes)) input.readByte() // skip type val hash = input.readUTF() contentHashIndex[hash] = recordId @@ -56,12 +57,13 @@ class LegacyFrameworkStorage(storagePath: Path) : FrameworkStorageBase(storagePa return withReadLock { val bytes = readBytes(recordId) if (bytes.isEmpty()) return@withReadLock null + // Versions 0 and 1 are read here; version >= 2 deliberately falls through to the type-based reader below. when (version) { - 0 -> return@withReadLock RecordInfo.Legacy(readVersion0UserChanges(DataInputStream(java.io.ByteArrayInputStream(bytes)))) - 1 -> return@withReadLock RecordInfo.Legacy(readLegacyUserChanges(DataInputStream(java.io.ByteArrayInputStream(bytes)))) + 0 -> return@withReadLock RecordInfo.Legacy(readVersion0UserChanges(DataInputStream(ByteArrayInputStream(bytes)))) + 1 -> return@withReadLock RecordInfo.Legacy(readLegacyUserChanges(DataInputStream(ByteArrayInputStream(bytes)))) } - val input = DataInputStream(java.io.ByteArrayInputStream(bytes)) + val input = DataInputStream(ByteArrayInputStream(bytes)) val type = input.readByte().toInt() when (type) { LEGACY_CHANGES_TYPE -> RecordInfo.Legacy(readLegacyUserChanges(input)) @@ -123,12 +125,13 @@ class LegacyFrameworkStorage(storagePath: Path) : FrameworkStorageBase(storagePa withReadLock { val bytes = readBytes(record) if (bytes.isEmpty()) return@withReadLock UserChanges.empty() + // Versions 0 and 1 are read here; version >= 2 deliberately falls through to the type-based reader below. when (version) { - 0 -> return@withReadLock readVersion0UserChanges(DataInputStream(java.io.ByteArrayInputStream(bytes))) - 1 -> return@withReadLock readLegacyUserChanges(DataInputStream(java.io.ByteArrayInputStream(bytes))) + 0 -> return@withReadLock readVersion0UserChanges(DataInputStream(ByteArrayInputStream(bytes))) + 1 -> return@withReadLock readLegacyUserChanges(DataInputStream(ByteArrayInputStream(bytes))) } - val input = DataInputStream(java.io.ByteArrayInputStream(bytes)) + val input = DataInputStream(ByteArrayInputStream(bytes)) val type = input.readByte().toInt() if (type == LEGACY_CHANGES_TYPE) { readLegacyUserChanges(input) @@ -150,7 +153,7 @@ class LegacyFrameworkStorage(storagePath: Path) : FrameworkStorageBase(storagePa @Throws(IOException::class) private fun readBlob(recordId: Int): String { val bytes = readBytes(recordId) - val input = DataInputStream(java.io.ByteArrayInputStream(bytes)) + val input = DataInputStream(ByteArrayInputStream(bytes)) val type = input.readByte().toInt() if (type != BLOB_TYPE) throw IOException("Corrupted data: record $recordId is not a blob (type=$type)") val hash = input.readUTF() diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/UserChanges.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/UserChanges.kt index 154e225fa..92097eafc 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/UserChanges.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/UserChanges.kt @@ -22,9 +22,9 @@ import java.io.IOException private val LOG: Logger = Logger.getInstance("org.hyperskill.academy.learning.framework.impl.UserChangesExtensions") fun UserChanges.apply(project: Project, taskDir: VirtualFile, task: Task) { - LOG.warn("UserChanges.apply: applying ${changes.size} changes to taskDir=${taskDir.path}") + LOG.debug("UserChanges.apply: applying ${changes.size} changes to taskDir=${taskDir.path}") for (change in changes) { - LOG.warn("UserChanges.apply: change=${change.javaClass.simpleName}(${change.path}, ${change.text.length} chars)") + LOG.debug("UserChanges.apply: change=${change.javaClass.simpleName}(${change.path}, ${change.text.length} chars)") change.apply(project, taskDir, task) } } @@ -67,16 +67,16 @@ private fun Change.RemoveFile.apply(project: Project, taskDir: VirtualFile, task } private fun Change.ChangeFile.apply(project: Project, taskDir: VirtualFile, task: Task) { - LOG.warn("ChangeFile.apply: path=$path, taskDir=${taskDir.path}, textLength=${text.length}") + LOG.debug("ChangeFile.apply: path=$path, taskDir=${taskDir.path}, textLength=${text.length}") val file = taskDir.findFileByRelativePath(path) if (file == null) { LOG.warn("ChangeFile.apply: Can't find file `$path` in `$taskDir`") return } - LOG.warn("ChangeFile.apply: Found file at ${file.path}") + LOG.debug("ChangeFile.apply: Found file at ${file.path}") if (file.isToEncodeContent) { - LOG.warn("ChangeFile.apply: Using binary content mode") + LOG.debug("ChangeFile.apply: Using binary content mode") file.doWithoutReadOnlyAttribute { runWriteAction { file.setBinaryContent(Base64.decodeBase64(text)) @@ -84,7 +84,7 @@ private fun Change.ChangeFile.apply(project: Project, taskDir: VirtualFile, task } } else { - LOG.warn("ChangeFile.apply: Using document mode") + LOG.debug("ChangeFile.apply: Using document mode") val modification = { updateTextFile(project, file, text) } @@ -102,13 +102,13 @@ private fun updateTextFile(project: Project, file: VirtualFile, text: String) { val document = runReadAction { FileDocumentManager.getInstance().getDocument(file) } if (document != null) { val expandedText = StringUtil.convertLineSeparators(EduMacroUtils.expandMacrosForFile(project.toCourseInfoHolder(), file, text)) - LOG.warn("ChangeFile.apply: Setting document text, expandedTextLength=${expandedText.length}") + LOG.debug("ChangeFile.apply: Setting document text, expandedTextLength=${expandedText.length}") file.doWithoutReadOnlyAttribute { runUndoTransparentWriteAction { document.setText(expandedText) } } // ALT-10961: Force save document to disk FileDocumentManager.getInstance().saveDocument(document) - LOG.warn("ChangeFile.apply: Document text set and saved successfully") + LOG.debug("ChangeFile.apply: Document text set and saved successfully") } else { LOG.warn("ChangeFile.apply: Can't get document for `$file`") diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/courseGeneration/HyperskillOpenInIdeRequestHandler.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/courseGeneration/HyperskillOpenInIdeRequestHandler.kt index a46aeb1ec..36c0b3de9 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/courseGeneration/HyperskillOpenInIdeRequestHandler.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/stepik/hyperskill/courseGeneration/HyperskillOpenInIdeRequestHandler.kt @@ -7,6 +7,7 @@ import com.intellij.openapi.progress.ProgressIndicator import com.intellij.openapi.progress.ProgressManager import com.intellij.openapi.project.Project import com.intellij.openapi.util.Ref +import com.intellij.util.concurrency.AppExecutorUtil import org.hyperskill.academy.learning.* import org.hyperskill.academy.learning.authUtils.requestFocus import org.hyperskill.academy.learning.courseFormat.Course @@ -32,6 +33,7 @@ import org.hyperskill.academy.learning.stepik.hyperskill.api.HyperskillConnector import org.hyperskill.academy.learning.stepik.hyperskill.api.HyperskillSolutionLoader import org.hyperskill.academy.learning.stepik.hyperskill.api.HyperskillStepSource import org.hyperskill.academy.learning.yaml.YamlFormatSynchronizer +import org.jetbrains.annotations.TestOnly import org.jetbrains.annotations.VisibleForTesting import java.util.concurrent.* @@ -40,6 +42,14 @@ object HyperskillOpenInIdeRequestHandler : OpenInIdeRequestHandler logTimed(operation: String, block: () -> T): T { @@ -60,23 +70,18 @@ object HyperskillOpenInIdeRequestHandler : OpenInIdeRequestHandler { - if (isUnitTestMode) { - computeUnderProgress(project, EduCoreBundle.message("hyperskill.loading.stages")) { - HyperskillConnector.getInstance().loadStages(hyperskillCourse) - } - return Ok(Unit) - } - - val future = CompletableFuture.supplyAsync { + timeoutMs: Long = STAGE_LOADING_TIMEOUT_MS, + loadStages: () -> Unit = { computeUnderProgress(project, EduCoreBundle.message("hyperskill.loading.stages")) { HyperskillConnector.getInstance().loadStages(hyperskillCourse) } } + ): Result { + val future = CompletableFuture.supplyAsync({ loadStages() }, stagesLoaderExecutor) return try { future.get(timeoutMs, TimeUnit.MILLISECONDS) diff --git a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/projectOpen/HyperskillLoadStagesWithTimeoutTest.kt b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/projectOpen/HyperskillLoadStagesWithTimeoutTest.kt new file mode 100644 index 000000000..34c485cf8 --- /dev/null +++ b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/projectOpen/HyperskillLoadStagesWithTimeoutTest.kt @@ -0,0 +1,58 @@ +package org.hyperskill.academy.learning.stepik.hyperskill.projectOpen + +import com.intellij.openapi.progress.ProcessCanceledException +import com.intellij.util.concurrency.AppExecutorUtil +import org.hyperskill.academy.learning.Err +import org.hyperskill.academy.learning.Ok +import org.hyperskill.academy.learning.courseFormat.hyperskill.HyperskillCourse +import org.hyperskill.academy.learning.stepik.hyperskill.courseGeneration.HyperskillOpenInIdeRequestHandler +import org.junit.Test + +/** + * Covers [HyperskillOpenInIdeRequestHandler.loadStagesWithTimeout]. The base class injects a + * same-thread executor in setUp(), so `supplyAsync` runs the loader inline and the future resolves + * immediately. This exercises the real future/timeout/cancellation code path that used to be dead + * behind the removed `isUnitTestMode` short-circuit, without spawning background threads (except the + * timeout case, which deliberately uses the application pool). + */ +class HyperskillLoadStagesWithTimeoutTest : HyperskillProjectOpenerTestBase() { + + @Test + fun `test successful loading returns Ok`() { + val result = HyperskillOpenInIdeRequestHandler.loadStagesWithTimeout(project, HyperskillCourse()) { + // loader succeeds without doing any work + } + assertTrue("Expected Ok but was $result", result is Ok) + } + + @Test + fun `test loader failure returns Err`() { + val result = HyperskillOpenInIdeRequestHandler.loadStagesWithTimeout(project, HyperskillCourse()) { + throw IllegalStateException("boom") + } + assertTrue("Expected Err but was $result", result is Err) + } + + @Test + fun `test ProcessCanceledException from loader is rethrown`() { + try { + HyperskillOpenInIdeRequestHandler.loadStagesWithTimeout(project, HyperskillCourse()) { + throw ProcessCanceledException() + } + fail("Expected ProcessCanceledException to be rethrown") + } + catch (expected: ProcessCanceledException) { + // expected: a control-flow exception must propagate, not be swallowed into an Err + } + } + + @Test + fun `test slow loading times out and returns Err`() { + // Use a real background executor so the future is still running when get() times out. + HyperskillOpenInIdeRequestHandler.stagesLoaderExecutor = AppExecutorUtil.getAppExecutorService() + val result = HyperskillOpenInIdeRequestHandler.loadStagesWithTimeout(project, HyperskillCourse(), timeoutMs = 100L) { + Thread.sleep(3_000) + } + assertTrue("Expected Err (timeout) but was $result", result is Err) + } +} diff --git a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/projectOpen/HyperskillProjectOpenerTestBase.kt b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/projectOpen/HyperskillProjectOpenerTestBase.kt index ff5349a3b..65bc80b89 100644 --- a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/projectOpen/HyperskillProjectOpenerTestBase.kt +++ b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/stepik/hyperskill/projectOpen/HyperskillProjectOpenerTestBase.kt @@ -5,22 +5,32 @@ import org.hyperskill.academy.learning.MockProjectOpener import org.hyperskill.academy.learning.courseGeneration.ProjectOpener import org.hyperskill.academy.learning.stepik.hyperskill.api.HyperskillConnector import org.hyperskill.academy.learning.stepik.hyperskill.api.MockHyperskillConnector +import org.hyperskill.academy.learning.stepik.hyperskill.courseGeneration.HyperskillOpenInIdeRequestHandler import org.hyperskill.academy.learning.stepik.hyperskill.hyperskillCourse import org.hyperskill.academy.learning.stepik.hyperskill.logInFakeHyperskillUser import org.hyperskill.academy.learning.stepik.hyperskill.logOutFakeHyperskillUser +import java.util.concurrent.Executor abstract class HyperskillProjectOpenerTestBase : EduTestCase() { protected val mockConnector: MockHyperskillConnector get() = HyperskillConnector.getInstance() as MockHyperskillConnector protected val mockProjectOpener: MockProjectOpener get() = ProjectOpener.getInstance() as MockProjectOpener + // Run stage loading synchronously in tests (replaces the former isUnitTestMode short-circuit in + // HyperskillOpenInIdeRequestHandler) so the future/timeout code path is still exercised. + private val directStagesLoaderExecutor = Executor { it.run() } + private var savedStagesLoaderExecutor: Executor? = null + override fun setUp() { super.setUp() mockProjectOpener.project = project logInFakeHyperskillUser() + savedStagesLoaderExecutor = HyperskillOpenInIdeRequestHandler.stagesLoaderExecutor + HyperskillOpenInIdeRequestHandler.stagesLoaderExecutor = directStagesLoaderExecutor } override fun tearDown() { try { + savedStagesLoaderExecutor?.let { HyperskillOpenInIdeRequestHandler.stagesLoaderExecutor = it } mockProjectOpener.project = null logOutFakeHyperskillUser() } diff --git a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/update/FrameworkLessonsUpdateTest.kt b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/update/FrameworkLessonsUpdateTest.kt index dca45033c..07353131b 100644 --- a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/update/FrameworkLessonsUpdateTest.kt +++ b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/update/FrameworkLessonsUpdateTest.kt @@ -267,12 +267,19 @@ abstract class FrameworkLessonsUpdateTest : UpdateTestBase() { @Test fun `test check current task index and task record saved`() { + // Simulate a project migrated from the old integer-keyed framework storage, where task1 + // carries a legacy record id. Production navigation no longer assigns record (it is only + // preserved or cleared), so the test seeds it directly instead of relying on a test-only branch. + val legacyRecord = localCourse.task1.index + localCourse.task1.record = legacyRecord + next() val taskRecordIndexBeforeUpdate = localCourse.task1.record assertEquals(1, frameworkLesson.currentTaskIndex) assertNotSame(-1, taskRecordIndexBeforeUpdate) + assertEquals("Record must survive navigation", legacyRecord, taskRecordIndexBeforeUpdate) val taskText = "fun foo2() {}" val testText = """ From 49d0371571d8d553cf2850774ba850c3ebfb6b32 Mon Sep 17 00:00:00 2001 From: Oleksandr Liemiahov Date: Thu, 18 Jun 2026 13:31:59 +0300 Subject: [PATCH 12/12] style: normalize .kt line endings to LF Add a `*.kt text eol=lf` rule to .gitattributes (the repo previously disabled normalization globally via `* -crlf`) and renormalize the two .kt files that still had CRLF/mixed endings: - FrameworkLessonManagerImpl.kt (was mixed CRLF/LF, which doubled its PR diff) - FrameworkLessonUserFilesPropagationTest.kt This is a content-free change (line endings only); no source was modified. --- .gitattributes | 1 + .../impl/FrameworkLessonManagerImpl.kt | 2952 ++++++++--------- ...FrameworkLessonUserFilesPropagationTest.kt | 258 +- 3 files changed, 1606 insertions(+), 1605 deletions(-) diff --git a/.gitattributes b/.gitattributes index aecf25037..f2243d187 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1 +1,2 @@ * -crlf +*.kt text eol=lf diff --git a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt index cc598df53..a7ea2dea1 100644 --- a/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt +++ b/intellij-plugin/hs-core/src/org/hyperskill/academy/learning/framework/impl/FrameworkLessonManagerImpl.kt @@ -1,137 +1,137 @@ -package org.hyperskill.academy.learning.framework.impl - -import com.intellij.openapi.Disposable -import com.intellij.openapi.application.ApplicationManager -import com.intellij.openapi.application.invokeAndWaitIfNeeded -import com.intellij.openapi.application.runReadAction -import com.intellij.openapi.application.runWriteAction -import com.intellij.openapi.diagnostic.Logger -import com.intellij.openapi.fileEditor.FileDocumentManager -import com.intellij.openapi.fileEditor.FileDocumentManagerListener -import com.intellij.openapi.project.Project -import com.intellij.openapi.util.Disposer -import com.intellij.openapi.util.io.FileUtil -import com.intellij.openapi.vfs.VfsUtil -import com.intellij.openapi.vfs.VirtualFile -import org.hyperskill.academy.learning.* -import org.hyperskill.academy.learning.configuration.excludeFromArchive -import org.hyperskill.academy.learning.courseFormat.CheckStatus -import org.hyperskill.academy.learning.courseFormat.FrameworkLesson -import org.hyperskill.academy.learning.courseFormat.TaskFile -import org.hyperskill.academy.learning.courseFormat.ext.configurator -import org.hyperskill.academy.learning.courseFormat.ext.getDir -import org.hyperskill.academy.learning.courseFormat.ext.isTestFile -import org.hyperskill.academy.learning.courseFormat.ext.shouldBePropagated -import org.hyperskill.academy.learning.courseFormat.ext.testDirs -import org.hyperskill.academy.learning.courseFormat.tasks.Task -import org.hyperskill.academy.learning.courseGeneration.GeneratorUtils -import org.hyperskill.academy.learning.framework.FrameworkLessonManager -import org.hyperskill.academy.learning.framework.FrameworkStorageListener -import org.hyperskill.academy.learning.framework.propagateFilesOnNavigation -import org.hyperskill.academy.learning.framework.storage.Change -import org.hyperskill.academy.learning.framework.storage.FileEntry -import org.hyperskill.academy.learning.framework.storage.UserChanges -import org.hyperskill.academy.learning.framework.ui.PropagationConflictDialog -import org.hyperskill.academy.learning.stepik.PyCharmStepOptions -import org.hyperskill.academy.learning.stepik.hyperskill.api.HyperskillConnector -import org.hyperskill.academy.learning.ui.getUIName -import org.hyperskill.academy.learning.yaml.YamlFormatSynchronizer -import org.jetbrains.annotations.TestOnly -import org.jetbrains.annotations.VisibleForTesting -import java.io.IOException -import java.nio.file.Path -import java.nio.file.Paths - -/** - * Extension to get storage ref for a task. - * @see getStorageRef - */ +package org.hyperskill.academy.learning.framework.impl + +import com.intellij.openapi.Disposable +import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.application.invokeAndWaitIfNeeded +import com.intellij.openapi.application.runReadAction +import com.intellij.openapi.application.runWriteAction +import com.intellij.openapi.diagnostic.Logger +import com.intellij.openapi.fileEditor.FileDocumentManager +import com.intellij.openapi.fileEditor.FileDocumentManagerListener +import com.intellij.openapi.project.Project +import com.intellij.openapi.util.Disposer +import com.intellij.openapi.util.io.FileUtil +import com.intellij.openapi.vfs.VfsUtil +import com.intellij.openapi.vfs.VirtualFile +import org.hyperskill.academy.learning.* +import org.hyperskill.academy.learning.configuration.excludeFromArchive +import org.hyperskill.academy.learning.courseFormat.CheckStatus +import org.hyperskill.academy.learning.courseFormat.FrameworkLesson +import org.hyperskill.academy.learning.courseFormat.TaskFile +import org.hyperskill.academy.learning.courseFormat.ext.configurator +import org.hyperskill.academy.learning.courseFormat.ext.getDir +import org.hyperskill.academy.learning.courseFormat.ext.isTestFile +import org.hyperskill.academy.learning.courseFormat.ext.shouldBePropagated +import org.hyperskill.academy.learning.courseFormat.ext.testDirs +import org.hyperskill.academy.learning.courseFormat.tasks.Task +import org.hyperskill.academy.learning.courseGeneration.GeneratorUtils +import org.hyperskill.academy.learning.framework.FrameworkLessonManager +import org.hyperskill.academy.learning.framework.FrameworkStorageListener +import org.hyperskill.academy.learning.framework.propagateFilesOnNavigation +import org.hyperskill.academy.learning.framework.storage.Change +import org.hyperskill.academy.learning.framework.storage.FileEntry +import org.hyperskill.academy.learning.framework.storage.UserChanges +import org.hyperskill.academy.learning.framework.ui.PropagationConflictDialog +import org.hyperskill.academy.learning.stepik.PyCharmStepOptions +import org.hyperskill.academy.learning.stepik.hyperskill.api.HyperskillConnector +import org.hyperskill.academy.learning.ui.getUIName +import org.hyperskill.academy.learning.yaml.YamlFormatSynchronizer +import org.jetbrains.annotations.TestOnly +import org.jetbrains.annotations.VisibleForTesting +import java.io.IOException +import java.nio.file.Path +import java.nio.file.Paths + +/** + * Extension to get storage ref for a task. + * @see getStorageRef + */ private fun Task.storageRef(): String = getStorageRef() // PERSISTED - do not rename; stored in framework storage snapshots. private const val LEARNER_MODIFIED_METADATA_KEY = "learnerModified" - -/** - * Keeps list of [Change]s for each task. Change list is difference between initial task state and latest one. - * - * Allows navigating between tasks in framework lessons in learner mode (where only current task is visible for a learner) - * without rewriting whole task content. - * It can be essential in large projects like Android applications where a lot of files are the same between two consecutive tasks - */ -class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLessonManager, Disposable { - private var storage: FrameworkStorage = createStorage(project) - - // Cache of original non-propagatable files (test files + hidden files) from API for each task (by step ID) - // These are used to recreate files with correct content when navigating between stages - // Non-propagatable files: isVisible=false OR isEditable=false (i.e., !shouldBePropagated()) - private val originalNonPropagatableFilesCache = java.util.concurrent.ConcurrentHashMap>() - - // Cache of original template files (visible non-test files) for each task (by step ID) - // These are used to calculate user changes correctly, since TaskFile.contents may be modified - private val originalTemplateFilesCache = java.util.concurrent.ConcurrentHashMap>() - - // Flag to track user's propagation choice during multi-stage navigation (jump). - // - null: no choice made yet (first stage in jump) → show dialog - // - true: user chose Replace → show dialog for each subsequent stage - // - false: user chose Keep → auto-Keep for all subsequent stages without dialog - private var propagationActive: Boolean? = null - - // Flag to skip auto-save during navigation (prevents auto-save from creating commits - // with "Auto-save" message when navigation code should be creating commits) - private var isNavigating = false - - // Flag to track if currentTaskIndex has been synced with storage HEAD. - // Auto-save is skipped until sync is done to prevent saving wrong stage content. - private var isStorageSynced = false - - init { - // Subscribe to file save events to persist changes to storage - val connection = project.messageBus.connect(this) - connection.subscribe(FileDocumentManagerListener.TOPIC, object : FileDocumentManagerListener { - override fun beforeAllDocumentsSaving() { - // Called when user saves all documents (Ctrl+S) or before IDE closes - saveCurrentTaskSnapshot() - } - }) - } - - override fun prepareNextTask(lesson: FrameworkLesson, taskDir: VirtualFile, showDialogIfConflict: Boolean) { - applyTargetTaskChanges(lesson, 1, taskDir, showDialogIfConflict) - } - - override fun preparePrevTask(lesson: FrameworkLesson, taskDir: VirtualFile, showDialogIfConflict: Boolean) { - applyTargetTaskChanges(lesson, -1, taskDir, showDialogIfConflict) - } - - /** - * Get the parent task's storage ref. - * For a task at index N, the parent is the task at index N-1. - * Returns null if there's no parent (first task in lesson). - */ - private fun getParentRef(task: Task): String? { - val lesson = task.lesson as? FrameworkLesson ?: return null - val taskIndex = task.index - return if (taskIndex > 1) { - lesson.taskList.getOrNull(taskIndex - 2)?.storageRef() - } else null - } - - /** - * Check if a task has data in storage. - */ - private fun hasStorageData(task: Task): Boolean { - return storage.hasRef(task.storageRef()) - } - - override fun saveExternalChanges(task: Task, externalState: Map, submissionId: Long?) { - require(task.lesson is FrameworkLesson) { - "Only solutions of framework tasks can be saved" - } - - val ref = task.storageRef() - val parentRef = getParentRef(task) - LOG.debug("saveExternalChanges: task='${task.name}', ref=$ref, submissionId=$submissionId, externalState.keys=${externalState.keys}") - + +/** + * Keeps list of [Change]s for each task. Change list is difference between initial task state and latest one. + * + * Allows navigating between tasks in framework lessons in learner mode (where only current task is visible for a learner) + * without rewriting whole task content. + * It can be essential in large projects like Android applications where a lot of files are the same between two consecutive tasks + */ +class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLessonManager, Disposable { + private var storage: FrameworkStorage = createStorage(project) + + // Cache of original non-propagatable files (test files + hidden files) from API for each task (by step ID) + // These are used to recreate files with correct content when navigating between stages + // Non-propagatable files: isVisible=false OR isEditable=false (i.e., !shouldBePropagated()) + private val originalNonPropagatableFilesCache = java.util.concurrent.ConcurrentHashMap>() + + // Cache of original template files (visible non-test files) for each task (by step ID) + // These are used to calculate user changes correctly, since TaskFile.contents may be modified + private val originalTemplateFilesCache = java.util.concurrent.ConcurrentHashMap>() + + // Flag to track user's propagation choice during multi-stage navigation (jump). + // - null: no choice made yet (first stage in jump) → show dialog + // - true: user chose Replace → show dialog for each subsequent stage + // - false: user chose Keep → auto-Keep for all subsequent stages without dialog + private var propagationActive: Boolean? = null + + // Flag to skip auto-save during navigation (prevents auto-save from creating commits + // with "Auto-save" message when navigation code should be creating commits) + private var isNavigating = false + + // Flag to track if currentTaskIndex has been synced with storage HEAD. + // Auto-save is skipped until sync is done to prevent saving wrong stage content. + private var isStorageSynced = false + + init { + // Subscribe to file save events to persist changes to storage + val connection = project.messageBus.connect(this) + connection.subscribe(FileDocumentManagerListener.TOPIC, object : FileDocumentManagerListener { + override fun beforeAllDocumentsSaving() { + // Called when user saves all documents (Ctrl+S) or before IDE closes + saveCurrentTaskSnapshot() + } + }) + } + + override fun prepareNextTask(lesson: FrameworkLesson, taskDir: VirtualFile, showDialogIfConflict: Boolean) { + applyTargetTaskChanges(lesson, 1, taskDir, showDialogIfConflict) + } + + override fun preparePrevTask(lesson: FrameworkLesson, taskDir: VirtualFile, showDialogIfConflict: Boolean) { + applyTargetTaskChanges(lesson, -1, taskDir, showDialogIfConflict) + } + + /** + * Get the parent task's storage ref. + * For a task at index N, the parent is the task at index N-1. + * Returns null if there's no parent (first task in lesson). + */ + private fun getParentRef(task: Task): String? { + val lesson = task.lesson as? FrameworkLesson ?: return null + val taskIndex = task.index + return if (taskIndex > 1) { + lesson.taskList.getOrNull(taskIndex - 2)?.storageRef() + } else null + } + + /** + * Check if a task has data in storage. + */ + private fun hasStorageData(task: Task): Boolean { + return storage.hasRef(task.storageRef()) + } + + override fun saveExternalChanges(task: Task, externalState: Map, submissionId: Long?) { + require(task.lesson is FrameworkLesson) { + "Only solutions of framework tasks can be saved" + } + + val ref = task.storageRef() + val parentRef = getParentRef(task) + LOG.debug("saveExternalChanges: task='${task.name}', ref=$ref, submissionId=$submissionId, externalState.keys=${externalState.keys}") + // Filter external state to only include propagatable files (exclude test files from submission) val externalPropagatableFiles = externalState.split(task).first LOG.debug("saveExternalChanges: externalPropagatableFiles.keys=${externalPropagatableFiles.keys}") @@ -140,40 +140,40 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson // Submission test files are intentionally ignored; API-provided tests stay authoritative. val (templatePropagatableFiles, _) = task.allFiles.split(task) val fullSnapshot = buildFullSnapshotState(task, templatePropagatableFiles + externalPropagatableFiles) - - // Save the full snapshot - val submissionInfo = if (submissionId != null) " (submission #$submissionId)" else "" - val message = "Load submission from server for '${task.name}'$submissionInfo" - try { - storage.saveSnapshot(ref, fullSnapshot, parentRef, message) - LOG.info("Saved full snapshot for external changes: ${fullSnapshot.size} files") - } - catch (e: IOException) { - LOG.error("Failed to save solution for task `${task.name}`", e) - } - - // Migration: If legacy storage has changes for this task's old refId, apply them on top. - // This preserves user's local changes that weren't submitted to the server. - val legacyRefId = task.record - if (legacyRefId != -1 && storage.hasLegacyChanges(legacyRefId)) { - LOG.info("Migrating legacy changes for task '${task.name}' (legacyRefId=$legacyRefId)") - try { - // Convert to content-only map for legacy migration (metadata will use defaults) - storage.applyLegacyChangesAndSave(ref, legacyRefId, fullSnapshot.toContentMap(), parentRef) - } - catch (e: IOException) { - LOG.error("Failed to apply legacy changes for task `${task.name}`", e) - } - } - - // Clear legacy record since we now use computed refs - if (task.record != -1) { - task.record = -1 - YamlFormatSynchronizer.saveItem(task) - } - LOG.debug("saveExternalChanges: task='${task.name}', saved to ref=$ref, parentRef=$parentRef") - } - + + // Save the full snapshot + val submissionInfo = if (submissionId != null) " (submission #$submissionId)" else "" + val message = "Load submission from server for '${task.name}'$submissionInfo" + try { + storage.saveSnapshot(ref, fullSnapshot, parentRef, message) + LOG.info("Saved full snapshot for external changes: ${fullSnapshot.size} files") + } + catch (e: IOException) { + LOG.error("Failed to save solution for task `${task.name}`", e) + } + + // Migration: If legacy storage has changes for this task's old refId, apply them on top. + // This preserves user's local changes that weren't submitted to the server. + val legacyRefId = task.record + if (legacyRefId != -1 && storage.hasLegacyChanges(legacyRefId)) { + LOG.info("Migrating legacy changes for task '${task.name}' (legacyRefId=$legacyRefId)") + try { + // Convert to content-only map for legacy migration (metadata will use defaults) + storage.applyLegacyChangesAndSave(ref, legacyRefId, fullSnapshot.toContentMap(), parentRef) + } + catch (e: IOException) { + LOG.error("Failed to apply legacy changes for task `${task.name}`", e) + } + } + + // Clear legacy record since we now use computed refs + if (task.record != -1) { + task.record = -1 + YamlFormatSynchronizer.saveItem(task) + } + LOG.debug("saveExternalChanges: task='${task.name}', saved to ref=$ref, parentRef=$parentRef") + } + override fun updateUserChanges(task: Task, newInitialState: Map, newTaskFiles: Map) { require(task.lesson is FrameworkLesson) { "Only framework task snapshots can be updated" @@ -225,64 +225,64 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson LOG.error("Failed to update snapshot for task '${task.name}'", e) } } - - override fun addNewFilesToSnapshot(task: Task, newFiles: Map) { - if (newFiles.isEmpty()) return - - require(task.lesson is FrameworkLesson) { - "Only framework task snapshots can be updated" - } - - val ref = task.storageRef() - - // Only update if snapshot exists (user visited this task before) - if (!storage.hasRef(ref)) { - LOG.info("addNewFilesToSnapshot: No snapshot exists for task '${task.name}' (ref=$ref), skipping") - return - } - - try { - // Get current snapshot (Map) - val currentSnapshot = storage.getSnapshot(ref) - - // Add only files that don't exist in the snapshot - val filesToAdd = newFiles.filterKeys { it !in currentSnapshot } - if (filesToAdd.isEmpty()) { - LOG.info("addNewFilesToSnapshot: All new files already exist in snapshot for task '${task.name}'") - return - } - - // Convert new files to FileEntry with metadata from task - val filesToAddWithMetadata = filesToAdd.toFileEntries(task, originalNonPropagatableFilesCache[task.id]) - - // Merge: existing snapshot + new files - val mergedSnapshot = currentSnapshot + filesToAddWithMetadata - - // Save updated snapshot - val message = "Add ${filesToAdd.size} new template files from server" - storage.saveSnapshot(ref, mergedSnapshot, null, message) - LOG.info("addNewFilesToSnapshot: Added ${filesToAdd.size} new files to snapshot for task '${task.name}': ${filesToAdd.keys}") - } - catch (e: IOException) { - LOG.error("Failed to add new files to snapshot for task '${task.name}'", e) - } - } - - override fun getChangesTimestamp(task: Task): Long { - require(task.lesson is FrameworkLesson) { - "Changes timestamp makes sense only for framework tasks" - } - - val ref = task.storageRef() - return try { - storage.getSnapshotTimestamp(ref) - } - catch (e: IOException) { - LOG.warn("Failed to get snapshot timestamp for task `${task.name}` (ref=$ref)", e) - 0L - } - } - + + override fun addNewFilesToSnapshot(task: Task, newFiles: Map) { + if (newFiles.isEmpty()) return + + require(task.lesson is FrameworkLesson) { + "Only framework task snapshots can be updated" + } + + val ref = task.storageRef() + + // Only update if snapshot exists (user visited this task before) + if (!storage.hasRef(ref)) { + LOG.info("addNewFilesToSnapshot: No snapshot exists for task '${task.name}' (ref=$ref), skipping") + return + } + + try { + // Get current snapshot (Map) + val currentSnapshot = storage.getSnapshot(ref) + + // Add only files that don't exist in the snapshot + val filesToAdd = newFiles.filterKeys { it !in currentSnapshot } + if (filesToAdd.isEmpty()) { + LOG.info("addNewFilesToSnapshot: All new files already exist in snapshot for task '${task.name}'") + return + } + + // Convert new files to FileEntry with metadata from task + val filesToAddWithMetadata = filesToAdd.toFileEntries(task, originalNonPropagatableFilesCache[task.id]) + + // Merge: existing snapshot + new files + val mergedSnapshot = currentSnapshot + filesToAddWithMetadata + + // Save updated snapshot + val message = "Add ${filesToAdd.size} new template files from server" + storage.saveSnapshot(ref, mergedSnapshot, null, message) + LOG.info("addNewFilesToSnapshot: Added ${filesToAdd.size} new files to snapshot for task '${task.name}': ${filesToAdd.keys}") + } + catch (e: IOException) { + LOG.error("Failed to add new files to snapshot for task '${task.name}'", e) + } + } + + override fun getChangesTimestamp(task: Task): Long { + require(task.lesson is FrameworkLesson) { + "Changes timestamp makes sense only for framework tasks" + } + + val ref = task.storageRef() + return try { + storage.getSnapshotTimestamp(ref) + } + catch (e: IOException) { + LOG.warn("Failed to get snapshot timestamp for task `${task.name}` (ref=$ref)", e) + 0L + } + } + override fun getTaskState(lesson: FrameworkLesson, task: Task): Map { require(task.lesson == lesson) { "The task is not a part of this lesson" @@ -318,82 +318,82 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson return task.allFiles } - - /** - * Convert the current state on local FS related to current task in framework lesson - * to a new one, to get state of next/previous (target) task. - */ - private fun applyTargetTaskChanges( - lesson: FrameworkLesson, - taskIndexDelta: Int, - taskDir: VirtualFile, - showDialogIfConflict: Boolean - ) { - val currentTaskIndex = lesson.currentTaskIndex - val targetTaskIndex = currentTaskIndex + taskIndexDelta - - val currentTask = lesson.taskList[currentTaskIndex] - val targetTask = lesson.taskList[targetTaskIndex] - - LOG.info("=== Stage switch: '${currentTask.name}' (idx=$currentTaskIndex) -> '${targetTask.name}' (idx=$targetTaskIndex), delta=$taskIndexDelta ===") - - // Set navigation flag to prevent auto-save from creating commits during navigation - // Use try-finally to guarantee the flag is reset even if an exception occurs - isNavigating = true - try { - applyTargetTaskChangesImpl(lesson, currentTask, targetTask, currentTaskIndex, targetTaskIndex, taskIndexDelta, taskDir, showDialogIfConflict) - } - finally { - isNavigating = false - // After navigation, currentTaskIndex is correct - enable auto-save - isStorageSynced = true - } - - LOG.info("=== Stage switch complete ===") - } - - /** - * Implementation of applyTargetTaskChanges, extracted to allow try-finally wrapper. - */ - private fun applyTargetTaskChangesImpl( - lesson: FrameworkLesson, - currentTask: Task, - targetTask: Task, - currentTaskIndex: Int, - targetTaskIndex: Int, - taskIndexDelta: Int, - taskDir: VirtualFile, - showDialogIfConflict: Boolean - ) { - val navigationStartTime = System.currentTimeMillis() - var lastStepTime = navigationStartTime - - fun logTiming(step: String) { - val now = System.currentTimeMillis() - val stepDuration = now - lastStepTime - val totalDuration = now - navigationStartTime - LOG.info("Navigation timing: $step took ${stepDuration}ms (total: ${totalDuration}ms)") - lastStepTime = now - } - - // Reset propagation flag on backward navigation - if (taskIndexDelta < 0) { - propagationActive = null - } - - lesson.currentTaskIndex = targetTaskIndex - YamlFormatSynchronizer.saveItem(lesson) - logTiming("saveLesson") - - val currentRef = currentTask.storageRef() - val targetRef = targetTask.storageRef() - val currentHasStorage = hasStorageData(currentTask) - val targetHasStorage = hasStorageData(targetTask) - - LOG.info("Navigation refs: current=$currentRef (hasStorage=$currentHasStorage), target=$targetRef (hasStorage=$targetHasStorage)") - - // 1. Get current disk state (what's currently on disk) - // Read ALL files from disk, including user-created files + + /** + * Convert the current state on local FS related to current task in framework lesson + * to a new one, to get state of next/previous (target) task. + */ + private fun applyTargetTaskChanges( + lesson: FrameworkLesson, + taskIndexDelta: Int, + taskDir: VirtualFile, + showDialogIfConflict: Boolean + ) { + val currentTaskIndex = lesson.currentTaskIndex + val targetTaskIndex = currentTaskIndex + taskIndexDelta + + val currentTask = lesson.taskList[currentTaskIndex] + val targetTask = lesson.taskList[targetTaskIndex] + + LOG.info("=== Stage switch: '${currentTask.name}' (idx=$currentTaskIndex) -> '${targetTask.name}' (idx=$targetTaskIndex), delta=$taskIndexDelta ===") + + // Set navigation flag to prevent auto-save from creating commits during navigation + // Use try-finally to guarantee the flag is reset even if an exception occurs + isNavigating = true + try { + applyTargetTaskChangesImpl(lesson, currentTask, targetTask, currentTaskIndex, targetTaskIndex, taskIndexDelta, taskDir, showDialogIfConflict) + } + finally { + isNavigating = false + // After navigation, currentTaskIndex is correct - enable auto-save + isStorageSynced = true + } + + LOG.info("=== Stage switch complete ===") + } + + /** + * Implementation of applyTargetTaskChanges, extracted to allow try-finally wrapper. + */ + private fun applyTargetTaskChangesImpl( + lesson: FrameworkLesson, + currentTask: Task, + targetTask: Task, + currentTaskIndex: Int, + targetTaskIndex: Int, + taskIndexDelta: Int, + taskDir: VirtualFile, + showDialogIfConflict: Boolean + ) { + val navigationStartTime = System.currentTimeMillis() + var lastStepTime = navigationStartTime + + fun logTiming(step: String) { + val now = System.currentTimeMillis() + val stepDuration = now - lastStepTime + val totalDuration = now - navigationStartTime + LOG.info("Navigation timing: $step took ${stepDuration}ms (total: ${totalDuration}ms)") + lastStepTime = now + } + + // Reset propagation flag on backward navigation + if (taskIndexDelta < 0) { + propagationActive = null + } + + lesson.currentTaskIndex = targetTaskIndex + YamlFormatSynchronizer.saveItem(lesson) + logTiming("saveLesson") + + val currentRef = currentTask.storageRef() + val targetRef = targetTask.storageRef() + val currentHasStorage = hasStorageData(currentTask) + val targetHasStorage = hasStorageData(targetTask) + + LOG.info("Navigation refs: current=$currentRef (hasStorage=$currentHasStorage), target=$targetRef (hasStorage=$targetHasStorage)") + + // 1. Get current disk state (what's currently on disk) + // Read ALL files from disk, including user-created files val currentDiskState = getAllFilesFromTaskDir(taskDir, currentTask) val (currentPropagatableFiles, _) = currentDiskState.split(currentTask) val currentSnapshotState = if (currentHasStorage) { @@ -432,8 +432,8 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson storage.saveSnapshot(currentRef, fullSnapshot, getParentRef(currentTask), navMessage) LOG.info("Saved full snapshot for current task '${currentTask.name}' (ref=$currentRef): ${fullSnapshot.size} files") } - catch (e: IOException) { - LOG.error("Failed to save snapshot for task `${currentTask.name}`", e) + catch (e: IOException) { + LOG.error("Failed to save snapshot for task `${currentTask.name}`", e) } logTiming("saveSnapshot(current)") } @@ -441,15 +441,15 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson val reason = "saved snapshot is newer than unchanged template on disk" LOG.info("Navigation: not saving current task '${currentTask.name}': $reason") } - + // 3. Get current state for diff calculation - // For forward navigation: use disk state (we just saved it) - // For backward navigation: use disk state (what's currently there) + // For forward navigation: use disk state (we just saved it) + // For backward navigation: use disk state (what's currently there) val currentState: FLTaskState = effectiveCurrentPropagatableFiles - LOG.debug("Navigation: currentState=${currentState.mapValues { "${it.key}:${it.value.length}chars" }}") - + LOG.debug("Navigation: currentState=${currentState.mapValues { "${it.key}:${it.value.length}chars" }}") + // 4. Get target state directly from storage snapshot (no template-based diff calculation needed) - // This is simpler and more reliable than calculating diffs from templates. + // This is simpler and more reliable than calculating diffs from templates. val targetState: FLTaskState = if (targetHasStorage) { try { storage.getSnapshot(targetRef).toContentMap().withoutDeletedTemplateFiles(targetTask) @@ -457,45 +457,45 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson LOG.error("Failed to get snapshot for target task '${targetTask.name}' (ref=$targetRef), falling back to templates", e) targetTask.allFiles } - } else { - // No storage data for target - use template files - targetTask.allFiles - } - logTiming("getTargetState") - LOG.debug("Navigation: targetState=${targetState.mapValues { "${it.key}:${it.value.length}chars" }}, fromStorage=$targetHasStorage") - + } else { + // No storage data for target - use template files + targetTask.allFiles + } + logTiming("getTargetState") + LOG.debug("Navigation: targetState=${targetState.mapValues { "${it.key}:${it.value.length}chars" }}, fromStorage=$targetHasStorage") + // 5. Calculate difference between latest states of current and target tasks - // Note, there are special rules for hyperskill courses for now - // All user changes from the current task should be propagated to next task as is - // - // Check if merge is needed using git-like ancestor check: - // - If current commit is ancestor of target commit → no merge needed (changes already propagated) - // - If current commit is NOT ancestor of target commit → check if propagatable files changed - // - If only test/hidden files changed (not user code) → auto-Keep merge (record ancestry, keep target's code) - val currentCommitIsAncestorOfTarget = targetHasStorage && storage.isAncestor(currentRef, targetRef) + // Note, there are special rules for hyperskill courses for now + // All user changes from the current task should be propagated to next task as is + // + // Check if merge is needed using git-like ancestor check: + // - If current commit is ancestor of target commit → no merge needed (changes already propagated) + // - If current commit is NOT ancestor of target commit → check if propagatable files changed + // - If only test/hidden files changed (not user code) → auto-Keep merge (record ancestry, keep target's code) + val currentCommitIsAncestorOfTarget = targetHasStorage && storage.isAncestor(currentRef, targetRef) val needsMerge = !currentCommitIsAncestorOfTarget && targetHasStorage && taskIndexDelta == 1 && lesson.propagateFilesOnNavigation LOG.info("Merge check: currentRef=$currentRef, targetRef=$targetRef, isAncestor=$currentCommitIsAncestorOfTarget, needsMerge=$needsMerge") - - // Track if merge commit was created (to skip redundant snapshot save in step 10) - var mergeCommitCreated = false - - val changes = when { + + // Track if merge commit was created (to skip redundant snapshot save in step 10) + var mergeCommitCreated = false + + val changes = when { needsMerge -> { mergeCommitCreated = true // Merge commit will be created in calculatePropagationChanges val fullCurrentState = buildFullSnapshotState(currentTask, effectiveCurrentPropagatableFiles).toContentMap() calculatePropagationChanges(targetTask, currentTask, fullCurrentState, targetState, showDialogIfConflict, targetHasStorage, currentRef, targetRef) } - // First visit to new stage (forward navigation with propagation enabled): - // Keep all current files and add only NEW files from target templates + // First visit to new stage (forward navigation with propagation enabled): + // Keep all current files and add only NEW files from target templates !targetHasStorage && taskIndexDelta > 0 && lesson.propagateFilesOnNavigation -> { LOG.info("First visit to '${targetTask.name}': propagating current state") calculateFirstVisitChanges(currentState, targetState, currentTask, targetTask) } - else -> { - propagationActive = null // No propagation happening, reset for next navigation - calculateChanges(currentState, targetState) - } - } + else -> { + propagationActive = null // No propagation happening, reset for next navigation + calculateChanges(currentState, targetState) + } + } logTiming("calculateChanges") // 6. Apply difference between latest states of current and target tasks on local FS @@ -505,407 +505,407 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson YamlFormatSynchronizer.saveItem(targetTask) } logTiming("applyChanges") - + // 7. Recreate non-propagatable files (test files, hidden files) from target task definition - // These files are stage-specific, so we need to recreate them explicitly during navigation - recreateNonPropagatableFiles(project, taskDir, currentTask, targetTask) - logTiming("recreateNonPropagatableFiles") - + // These files are stage-specific, so we need to recreate them explicitly during navigation + recreateNonPropagatableFiles(project, taskDir, currentTask, targetTask) + logTiming("recreateNonPropagatableFiles") + // 8. ALT-10961: Force save all documents and refresh VFS to ensure changes are visible in editor - // Document changes may be in memory but not persisted or reflected in the editor - invokeAndWaitIfNeeded { - FileDocumentManager.getInstance().saveAllDocuments() - VfsUtil.markDirtyAndRefresh(false, true, true, taskDir) - LOG.info("Navigation: Saved documents and refreshed VFS for taskDir=${taskDir.path}") - } - logTiming("saveDocumentsAndRefreshVFS") - + // Document changes may be in memory but not persisted or reflected in the editor + invokeAndWaitIfNeeded { + FileDocumentManager.getInstance().saveAllDocuments() + VfsUtil.markDirtyAndRefresh(false, true, true, taskDir) + LOG.info("Navigation: Saved documents and refreshed VFS for taskDir=${taskDir.path}") + } + logTiming("saveDocumentsAndRefreshVFS") + // 9. Save snapshot for target stage after forward navigation. - // Skip if merge commit was already created (to avoid redundant commits). - // Only save for: - // - Target without storage (first visit to this stage) - // - Navigation without merge (ancestor check passed, no Keep/Replace dialog) - if (taskIndexDelta > 0 && !mergeCommitCreated) { - // Read ALL files from disk, including user-created files - val finalDiskState = getAllFilesFromTaskDir(taskDir, targetTask) - val (finalPropagatableFiles, _) = finalDiskState.split(targetTask) - val fullSnapshot = buildFullSnapshotState(targetTask, finalPropagatableFiles) - logTiming("buildFullSnapshotState(target)") - val message = "Navigate to '${targetTask.name}'" - try { - storage.saveSnapshot(targetRef, fullSnapshot, getParentRef(targetTask), message) - LOG.info("Saved full snapshot for target task '${targetTask.name}' (ref=$targetRef): ${fullSnapshot.size} files") - } - catch (e: IOException) { - LOG.error("Failed to save snapshot for target task '${targetTask.name}'", e) - } - logTiming("saveSnapshot(target)") - } - - // Update HEAD to point to the current (target) task - storage.head = targetRef - LOG.info("HEAD updated to ref $targetRef (task '${targetTask.name}')") - project.messageBus.syncPublisher(FrameworkStorageListener.TOPIC).headUpdated(targetRef) - - val totalTime = System.currentTimeMillis() - navigationStartTime - LOG.info("Navigation timing: TOTAL ${totalTime}ms for '${currentTask.name}' -> '${targetTask.name}'") - } - - /** - * Loads non-propagatable files (test files + hidden files) from API for a task and caches them. - * This is used as a fallback when the cache is empty (e.g., after IDE restart). - * - * IMPORTANT: This method must NOT block EDT. If called from EDT, it starts async loading - * and returns null immediately. The caller should use task.taskFiles as fallback. - * - * @param task the task whose non-propagatable files should be loaded - * @return map of non-propagatable files (filename -> TaskFile), or null if loading failed or is in progress - */ - private fun loadNonPropagatableFilesFromApi(task: Task): Map? { - val stepId = task.id - if (stepId <= 0) { - LOG.warn("Cannot load non-propagatable files from API: task '${task.name}' has invalid step ID: $stepId") - return null - } - - // Don't block EDT - start async loading and return null - // The caller will use task.taskFiles as fallback - if (ApplicationManager.getApplication().isDispatchThread) { - LOG.info("On EDT, starting async load for non-propagatable files of task '${task.name}' (step $stepId)") - ApplicationManager.getApplication().executeOnPooledThread { - loadNonPropagatableFilesFromApiSync(task) - } - return null - } - - return loadNonPropagatableFilesFromApiSync(task) - } - - /** - * Synchronously loads non-propagatable files from API. Must NOT be called from EDT. - */ - private fun loadNonPropagatableFilesFromApiSync(task: Task): Map? { - val stepId = task.id - LOG.info("Loading non-propagatable files from API for task '${task.name}' (step $stepId)") - - return try { - // ALT-10961: Use anonymous request to get original files from API. - // Authenticated requests return files from user's last submission instead of original stage files. - val stepSourceResult = HyperskillConnector.getInstance().getStepSourceAnonymous(stepId) - if (stepSourceResult is Err) { - LOG.warn("Failed to load step source from API for step $stepId: ${stepSourceResult.error}") - return null - } - - val stepSource = (stepSourceResult as Ok).value - val options = stepSource.block?.options as? PyCharmStepOptions - if (options == null) { - LOG.warn("Step $stepId has no PyCharmStepOptions") - return null - } - - val allFiles = options.files - if (allFiles.isNullOrEmpty()) { - LOG.warn("Step $stepId has no files in options") - return null - } - - // Filter non-propagatable files: author-created AND (invisible OR non-editable) - // This includes test files and hidden configuration files - val nonPropagatableFiles = allFiles.filter { taskFile -> - !taskFile.isLearnerCreated && (!taskFile.isVisible || !taskFile.isEditable) - } - - if (nonPropagatableFiles.isEmpty()) { - LOG.info("Step $stepId has no non-propagatable files") - // Store empty map so we don't keep trying to load - originalNonPropagatableFilesCache[stepId] = emptyMap() - return emptyMap() - } - - // Create copies of TaskFile objects and cache them - val copiedFiles = nonPropagatableFiles.associateBy( - { it.name }, - { taskFile -> - TaskFile(taskFile.name, taskFile.contents).also { - it.isVisible = taskFile.isVisible - it.isEditable = taskFile.isEditable - it.isLearnerCreated = taskFile.isLearnerCreated - } - } - ) - - originalNonPropagatableFilesCache[stepId] = copiedFiles - val filesInfo = copiedFiles.entries.joinToString { (name, file) -> - "$name:size=${file.contents.textualRepresentation.length}" - } - LOG.info("Loaded and cached ${copiedFiles.size} non-propagatable files from API for task '${task.name}' (step $stepId): [$filesInfo]") - - copiedFiles - } - catch (e: Exception) { - LOG.warn("Exception while loading non-propagatable files from API for step $stepId", e) - null - } - } - - /** - * Recreates non-propagatable files (test files, hidden files) from storage snapshot, cache, or API. - * Non-propagatable files are provided by the course author and should not be modified by students. - * - * Priority for getting files: - * 1. In-memory cache - most reliable for current session - * 2. API request - fresh data from server - * 3. task.taskFiles - last resort fallback - * - * @param currentTask The task we're navigating FROM (used to identify old files to delete) - * @param targetTask The task we're navigating TO (used to identify new files to create) - */ - private fun recreateNonPropagatableFiles(project: Project, taskDir: VirtualFile, currentTask: Task, targetTask: Task) { - // Get non-propagatable files for current task (to know what to delete) - val currentNonPropagatableFileNames = getNonPropagatableFileNames(currentTask) - - // Get non-propagatable files for target task from cache or API - val targetNonPropagatableFiles = getNonPropagatableFilesWithMetadata(targetTask) - val targetNonPropagatableFileNames = targetNonPropagatableFiles.keys - - // Delete files from current task that are not in target task + // Skip if merge commit was already created (to avoid redundant commits). + // Only save for: + // - Target without storage (first visit to this stage) + // - Navigation without merge (ancestor check passed, no Keep/Replace dialog) + if (taskIndexDelta > 0 && !mergeCommitCreated) { + // Read ALL files from disk, including user-created files + val finalDiskState = getAllFilesFromTaskDir(taskDir, targetTask) + val (finalPropagatableFiles, _) = finalDiskState.split(targetTask) + val fullSnapshot = buildFullSnapshotState(targetTask, finalPropagatableFiles) + logTiming("buildFullSnapshotState(target)") + val message = "Navigate to '${targetTask.name}'" + try { + storage.saveSnapshot(targetRef, fullSnapshot, getParentRef(targetTask), message) + LOG.info("Saved full snapshot for target task '${targetTask.name}' (ref=$targetRef): ${fullSnapshot.size} files") + } + catch (e: IOException) { + LOG.error("Failed to save snapshot for target task '${targetTask.name}'", e) + } + logTiming("saveSnapshot(target)") + } + + // Update HEAD to point to the current (target) task + storage.head = targetRef + LOG.info("HEAD updated to ref $targetRef (task '${targetTask.name}')") + project.messageBus.syncPublisher(FrameworkStorageListener.TOPIC).headUpdated(targetRef) + + val totalTime = System.currentTimeMillis() - navigationStartTime + LOG.info("Navigation timing: TOTAL ${totalTime}ms for '${currentTask.name}' -> '${targetTask.name}'") + } + + /** + * Loads non-propagatable files (test files + hidden files) from API for a task and caches them. + * This is used as a fallback when the cache is empty (e.g., after IDE restart). + * + * IMPORTANT: This method must NOT block EDT. If called from EDT, it starts async loading + * and returns null immediately. The caller should use task.taskFiles as fallback. + * + * @param task the task whose non-propagatable files should be loaded + * @return map of non-propagatable files (filename -> TaskFile), or null if loading failed or is in progress + */ + private fun loadNonPropagatableFilesFromApi(task: Task): Map? { + val stepId = task.id + if (stepId <= 0) { + LOG.warn("Cannot load non-propagatable files from API: task '${task.name}' has invalid step ID: $stepId") + return null + } + + // Don't block EDT - start async loading and return null + // The caller will use task.taskFiles as fallback + if (ApplicationManager.getApplication().isDispatchThread) { + LOG.info("On EDT, starting async load for non-propagatable files of task '${task.name}' (step $stepId)") + ApplicationManager.getApplication().executeOnPooledThread { + loadNonPropagatableFilesFromApiSync(task) + } + return null + } + + return loadNonPropagatableFilesFromApiSync(task) + } + + /** + * Synchronously loads non-propagatable files from API. Must NOT be called from EDT. + */ + private fun loadNonPropagatableFilesFromApiSync(task: Task): Map? { + val stepId = task.id + LOG.info("Loading non-propagatable files from API for task '${task.name}' (step $stepId)") + + return try { + // ALT-10961: Use anonymous request to get original files from API. + // Authenticated requests return files from user's last submission instead of original stage files. + val stepSourceResult = HyperskillConnector.getInstance().getStepSourceAnonymous(stepId) + if (stepSourceResult is Err) { + LOG.warn("Failed to load step source from API for step $stepId: ${stepSourceResult.error}") + return null + } + + val stepSource = (stepSourceResult as Ok).value + val options = stepSource.block?.options as? PyCharmStepOptions + if (options == null) { + LOG.warn("Step $stepId has no PyCharmStepOptions") + return null + } + + val allFiles = options.files + if (allFiles.isNullOrEmpty()) { + LOG.warn("Step $stepId has no files in options") + return null + } + + // Filter non-propagatable files: author-created AND (invisible OR non-editable) + // This includes test files and hidden configuration files + val nonPropagatableFiles = allFiles.filter { taskFile -> + !taskFile.isLearnerCreated && (!taskFile.isVisible || !taskFile.isEditable) + } + + if (nonPropagatableFiles.isEmpty()) { + LOG.info("Step $stepId has no non-propagatable files") + // Store empty map so we don't keep trying to load + originalNonPropagatableFilesCache[stepId] = emptyMap() + return emptyMap() + } + + // Create copies of TaskFile objects and cache them + val copiedFiles = nonPropagatableFiles.associateBy( + { it.name }, + { taskFile -> + TaskFile(taskFile.name, taskFile.contents).also { + it.isVisible = taskFile.isVisible + it.isEditable = taskFile.isEditable + it.isLearnerCreated = taskFile.isLearnerCreated + } + } + ) + + originalNonPropagatableFilesCache[stepId] = copiedFiles + val filesInfo = copiedFiles.entries.joinToString { (name, file) -> + "$name:size=${file.contents.textualRepresentation.length}" + } + LOG.info("Loaded and cached ${copiedFiles.size} non-propagatable files from API for task '${task.name}' (step $stepId): [$filesInfo]") + + copiedFiles + } + catch (e: Exception) { + LOG.warn("Exception while loading non-propagatable files from API for step $stepId", e) + null + } + } + + /** + * Recreates non-propagatable files (test files, hidden files) from storage snapshot, cache, or API. + * Non-propagatable files are provided by the course author and should not be modified by students. + * + * Priority for getting files: + * 1. In-memory cache - most reliable for current session + * 2. API request - fresh data from server + * 3. task.taskFiles - last resort fallback + * + * @param currentTask The task we're navigating FROM (used to identify old files to delete) + * @param targetTask The task we're navigating TO (used to identify new files to create) + */ + private fun recreateNonPropagatableFiles(project: Project, taskDir: VirtualFile, currentTask: Task, targetTask: Task) { + // Get non-propagatable files for current task (to know what to delete) + val currentNonPropagatableFileNames = getNonPropagatableFileNames(currentTask) + + // Get non-propagatable files for target task from cache or API + val targetNonPropagatableFiles = getNonPropagatableFilesWithMetadata(targetTask) + val targetNonPropagatableFileNames = targetNonPropagatableFiles.keys + + // Delete files from current task that are not in target task val targetPropagatableFileNames = targetTask.taskFiles .filterValues { taskFile -> taskFile.shouldBePropagated() } .keys val filesToDelete = currentNonPropagatableFileNames - targetNonPropagatableFileNames - targetPropagatableFileNames - if (filesToDelete.isNotEmpty()) { - LOG.info("Deleting ${filesToDelete.size} old non-propagatable files: $filesToDelete") - invokeAndWaitIfNeeded { - runWriteAction { - // Collect parent directories that may become empty after file deletion - val potentiallyEmptyDirs = mutableSetOf() - for (fileName in filesToDelete) { - try { - val file = taskDir.findFileByRelativePath(fileName) - if (file != null) { - file.parent?.let { parent -> - if (parent != taskDir) { - potentiallyEmptyDirs.add(parent) - } - } - file.delete(this) - } - } - catch (e: Exception) { - LOG.warn("Failed to delete old non-propagatable file $fileName", e) - } - } - // Delete empty parent directories (in reverse depth order to handle nested dirs) - val sortedDirs = potentiallyEmptyDirs.sortedByDescending { it.path.count { c -> c == '/' } } - for (dir in sortedDirs) { - try { - if (dir.isValid && dir.children.isEmpty()) { - LOG.info("Deleting empty directory: ${dir.name}") - dir.delete(this) - } - } - catch (e: Exception) { - LOG.warn("Failed to delete empty directory ${dir.name}", e) - } - } - } - } - } - - // Create non-propagatable files for target task - if (targetNonPropagatableFiles.isNotEmpty()) { - val filesInfo = targetNonPropagatableFiles.entries.joinToString { "${it.key}:${it.value.contents.textualRepresentation.hashCode()}" } - LOG.info("Recreating ${targetNonPropagatableFiles.size} non-propagatable files for task '${targetTask.name}' (step ${targetTask.id}): [$filesInfo]") - - for ((filePath, taskFile) in targetNonPropagatableFiles) { - try { - // createChildFile handles write action internally via runInWriteActionAndWait - // Use the file's isEditable property (test files are non-editable, some hidden files may be editable) - val createdFile = GeneratorUtils.createChildFile( - project, - taskDir, - filePath, - taskFile.contents.textualRepresentation, - taskFile.isEditable - ) - if (createdFile == null) { - LOG.error("Failed to create non-propagatable file $filePath - createChildFile returned null") - } - else { - LOG.info("Successfully created non-propagatable file: ${createdFile.path}") - } - } - catch (e: Exception) { - LOG.error("Exception while recreating non-propagatable file $filePath for task ${targetTask.name}", e) - } - } - } - } - - /** - * Gets non-propagatable file names for a task (for deletion during navigation). - */ - private fun getNonPropagatableFileNames(task: Task): Set { - // Try cache first - val cached = originalNonPropagatableFilesCache[task.id] - if (cached != null) { - return cached.keys - } - - // Fallback to task model - return task.taskFiles.filterValues { taskFile -> - !taskFile.isLearnerCreated && !taskFile.shouldBePropagated() - }.keys - } - - /** - * Gets non-propagatable files with metadata (TaskFile objects) for a task. - * Used when creating files to preserve isEditable property. - * Returns map of path -> TaskFile. - */ - private fun getNonPropagatableFilesWithMetadata(task: Task): Map { - // 1. Try in-memory cache - val cached = originalNonPropagatableFilesCache[task.id] - if (cached != null) { - LOG.info("Got ${cached.size} non-propagatable files from cache for task '${task.name}'") - return cached - } - - // 2. Try loading from API - LOG.info("No cached non-propagatable files for task '${task.name}', loading from API...") - if (ApplicationManager.getApplication().isDispatchThread) { - try { - ApplicationManager.getApplication().executeOnPooledThread { - loadNonPropagatableFilesFromApiSync(task) - }.get() - val loaded = originalNonPropagatableFilesCache[task.id] - if (loaded != null) { - return loaded - } - } catch (e: Exception) { - LOG.warn("Failed to load non-propagatable files from API for task '${task.name}'", e) - } - } else { - loadNonPropagatableFilesFromApi(task) - val loaded = originalNonPropagatableFilesCache[task.id] - if (loaded != null) { - return loaded - } - } - - // 3. Fallback to task.taskFiles - LOG.warn("All sources failed, falling back to task.taskFiles for task '${task.name}'") - return task.taskFiles.filterValues { taskFile -> - !taskFile.isLearnerCreated && !taskFile.shouldBePropagated() - } - } - - /** - * Returns [Change]s to propagate user changes from [currentState] to [targetTask]. - * - * In case, when it's impossible due to simultaneous incompatible user changes in [currentState] and [targetState], - * it asks user to choose what change he wants to apply. - * - * Creates merge commits with two parents when user chooses Keep or Replace: - * - Replace: merge commit with content from current stage (propagate changes) - * - Keep: merge commit with content from target stage (preserve target's changes) - * - * @param targetHasStorage true if target task has saved snapshot in storage - * @param currentRef ref of the current task (for merge commit parent) - * @param targetRef ref of the target task (for merge commit parent) - */ - private fun calculatePropagationChanges( - targetTask: Task, - currentTask: Task, - currentState: Map, - targetState: Map, - showDialogIfConflict: Boolean, - targetHasStorage: Boolean, - currentRef: String, - targetRef: String - ): UserChanges { - val (currentPropagatableFilesState, currentNonPropagatableFilesState) = currentState.split(currentTask) - val (targetPropagatableFilesState, targetNonPropagatableFilesState) = targetState.split(targetTask) - - // If user previously chose Keep in this navigation sequence, auto-Keep without showing dialog - if (propagationActive == false) { - LOG.info("Auto-Keep for '$targetRef' (user chose Keep in previous step)") - val mergeMessage = "Merge from '${currentTask.name}': Keep target changes (auto)" - try { - // Use buildFullSnapshotState to include both user files and test files from target task - val fullSnapshot = buildFullSnapshotState(targetTask, targetPropagatableFilesState) - storage.saveMergeSnapshot(targetRef, fullSnapshot, listOf(targetRef, currentRef), mergeMessage) - LOG.info("Created auto-Keep merge commit for '$targetRef' with parents [$targetRef, $currentRef]: ${fullSnapshot.size} files") - } catch (e: IOException) { - LOG.error("Failed to create auto-Keep merge commit for '$targetRef'", e) - } - return calculateChanges(currentState, targetState) - } - - // A lesson may have files that were non-propagatable in the previous step, but become propagatable in the new one. - // We allow files to change a propagation flag from false to true (from non-propagatable to propagatable). - // Course creators often have such use-case: - // They want to make some files invisible on some prefix of the task list, and then have them visible in the rest of the tasks - // So that they will be shown to students and will participate in solving the problem - // For more detailed explanation, see the documentation: - // https://jetbrains.team/p/edu/repositories/internal-documentation/files/subsystems/Framework%20Lessons/internal-part-ru.md - - // Calculate files that change propagation flag - // Note: We also check currentTask.taskFiles directly because invisible files are not included - // in currentNonPropagatableFilesState (since allFiles only returns visible files) - // We check if file is invisible AND not in a test directory (test files are handled by recreateNonPropagatableFiles) - val taskTestDirs = currentTask.testDirs - val fromNonPropagatableToPropagatableFilesState = targetPropagatableFilesState.filter { (path, _) -> - path in currentNonPropagatableFilesState || - (currentTask.taskFiles[path]?.isVisible == false && - taskTestDirs.none { testDir -> path.startsWith("$testDir/") || path == testDir }) - } - val fromPropagatableToNonPropagatableFilesState = targetNonPropagatableFilesState.filter { it.key in currentPropagatableFilesState } - - // We assume that files could not change a propagation flag from true to false (for example, from visible to invisible) - // This behavior is not intended - if (fromPropagatableToNonPropagatableFilesState.isNotEmpty()) { - LOG.error("Propagation flag change from propagatable to non-propagatable during navigation in non-template-based lessons is not supported") - } - - // Only files that do not change a propagation flag can participate in propagating user changes. - val newCurrentPropagatableFilesState = currentPropagatableFilesState.filter { it.key !in targetNonPropagatableFilesState } - val newTargetPropagatableFilesState = targetPropagatableFilesState.filter { it.key !in fromNonPropagatableToPropagatableFilesState } - - // Files that change propagation flag are processed separately: - // (Non-Propagatable -> Propagatable) - Changes for them are not propagated - // (Propagatable -> Non-Propagatable) - We assume that there are no such files - - // Creates Changes to propagate all current changes of task files to a target task. - // During propagation, we assume that in the not-template-based framework lessons, all the initial files are the same for each task. - // Therefore, we will only add user-created files and remove user-deleted files. - // During propagation, we do not change the text of the files. - fun calculateCurrentTaskChanges(): UserChanges { - val toRemove = HashMap(newTargetPropagatableFilesState) - val propagatableFileChanges = mutableListOf() - - for ((path, text) in newCurrentPropagatableFilesState) { - val targetText = toRemove.remove(path) - // Propagate user-created files - if (targetText == null) { - propagatableFileChanges += Change.PropagateLearnerCreatedTaskFile(path, text) - } - } - - // Remove user-deleted files - for ((path, _) in toRemove) { - propagatableFileChanges += Change.RemoveTaskFile(path) - } - - // Calculate diff for invisible files and files that become visible and change them without propagation - val nonPropagatableFileChanges = calculateChanges( - currentNonPropagatableFilesState, - targetNonPropagatableFilesState + fromNonPropagatableToPropagatableFilesState - ) - return nonPropagatableFileChanges + propagatableFileChanges - } - + if (filesToDelete.isNotEmpty()) { + LOG.info("Deleting ${filesToDelete.size} old non-propagatable files: $filesToDelete") + invokeAndWaitIfNeeded { + runWriteAction { + // Collect parent directories that may become empty after file deletion + val potentiallyEmptyDirs = mutableSetOf() + for (fileName in filesToDelete) { + try { + val file = taskDir.findFileByRelativePath(fileName) + if (file != null) { + file.parent?.let { parent -> + if (parent != taskDir) { + potentiallyEmptyDirs.add(parent) + } + } + file.delete(this) + } + } + catch (e: Exception) { + LOG.warn("Failed to delete old non-propagatable file $fileName", e) + } + } + // Delete empty parent directories (in reverse depth order to handle nested dirs) + val sortedDirs = potentiallyEmptyDirs.sortedByDescending { it.path.count { c -> c == '/' } } + for (dir in sortedDirs) { + try { + if (dir.isValid && dir.children.isEmpty()) { + LOG.info("Deleting empty directory: ${dir.name}") + dir.delete(this) + } + } + catch (e: Exception) { + LOG.warn("Failed to delete empty directory ${dir.name}", e) + } + } + } + } + } + + // Create non-propagatable files for target task + if (targetNonPropagatableFiles.isNotEmpty()) { + val filesInfo = targetNonPropagatableFiles.entries.joinToString { "${it.key}:${it.value.contents.textualRepresentation.hashCode()}" } + LOG.info("Recreating ${targetNonPropagatableFiles.size} non-propagatable files for task '${targetTask.name}' (step ${targetTask.id}): [$filesInfo]") + + for ((filePath, taskFile) in targetNonPropagatableFiles) { + try { + // createChildFile handles write action internally via runInWriteActionAndWait + // Use the file's isEditable property (test files are non-editable, some hidden files may be editable) + val createdFile = GeneratorUtils.createChildFile( + project, + taskDir, + filePath, + taskFile.contents.textualRepresentation, + taskFile.isEditable + ) + if (createdFile == null) { + LOG.error("Failed to create non-propagatable file $filePath - createChildFile returned null") + } + else { + LOG.info("Successfully created non-propagatable file: ${createdFile.path}") + } + } + catch (e: Exception) { + LOG.error("Exception while recreating non-propagatable file $filePath for task ${targetTask.name}", e) + } + } + } + } + + /** + * Gets non-propagatable file names for a task (for deletion during navigation). + */ + private fun getNonPropagatableFileNames(task: Task): Set { + // Try cache first + val cached = originalNonPropagatableFilesCache[task.id] + if (cached != null) { + return cached.keys + } + + // Fallback to task model + return task.taskFiles.filterValues { taskFile -> + !taskFile.isLearnerCreated && !taskFile.shouldBePropagated() + }.keys + } + + /** + * Gets non-propagatable files with metadata (TaskFile objects) for a task. + * Used when creating files to preserve isEditable property. + * Returns map of path -> TaskFile. + */ + private fun getNonPropagatableFilesWithMetadata(task: Task): Map { + // 1. Try in-memory cache + val cached = originalNonPropagatableFilesCache[task.id] + if (cached != null) { + LOG.info("Got ${cached.size} non-propagatable files from cache for task '${task.name}'") + return cached + } + + // 2. Try loading from API + LOG.info("No cached non-propagatable files for task '${task.name}', loading from API...") + if (ApplicationManager.getApplication().isDispatchThread) { + try { + ApplicationManager.getApplication().executeOnPooledThread { + loadNonPropagatableFilesFromApiSync(task) + }.get() + val loaded = originalNonPropagatableFilesCache[task.id] + if (loaded != null) { + return loaded + } + } catch (e: Exception) { + LOG.warn("Failed to load non-propagatable files from API for task '${task.name}'", e) + } + } else { + loadNonPropagatableFilesFromApi(task) + val loaded = originalNonPropagatableFilesCache[task.id] + if (loaded != null) { + return loaded + } + } + + // 3. Fallback to task.taskFiles + LOG.warn("All sources failed, falling back to task.taskFiles for task '${task.name}'") + return task.taskFiles.filterValues { taskFile -> + !taskFile.isLearnerCreated && !taskFile.shouldBePropagated() + } + } + + /** + * Returns [Change]s to propagate user changes from [currentState] to [targetTask]. + * + * In case, when it's impossible due to simultaneous incompatible user changes in [currentState] and [targetState], + * it asks user to choose what change he wants to apply. + * + * Creates merge commits with two parents when user chooses Keep or Replace: + * - Replace: merge commit with content from current stage (propagate changes) + * - Keep: merge commit with content from target stage (preserve target's changes) + * + * @param targetHasStorage true if target task has saved snapshot in storage + * @param currentRef ref of the current task (for merge commit parent) + * @param targetRef ref of the target task (for merge commit parent) + */ + private fun calculatePropagationChanges( + targetTask: Task, + currentTask: Task, + currentState: Map, + targetState: Map, + showDialogIfConflict: Boolean, + targetHasStorage: Boolean, + currentRef: String, + targetRef: String + ): UserChanges { + val (currentPropagatableFilesState, currentNonPropagatableFilesState) = currentState.split(currentTask) + val (targetPropagatableFilesState, targetNonPropagatableFilesState) = targetState.split(targetTask) + + // If user previously chose Keep in this navigation sequence, auto-Keep without showing dialog + if (propagationActive == false) { + LOG.info("Auto-Keep for '$targetRef' (user chose Keep in previous step)") + val mergeMessage = "Merge from '${currentTask.name}': Keep target changes (auto)" + try { + // Use buildFullSnapshotState to include both user files and test files from target task + val fullSnapshot = buildFullSnapshotState(targetTask, targetPropagatableFilesState) + storage.saveMergeSnapshot(targetRef, fullSnapshot, listOf(targetRef, currentRef), mergeMessage) + LOG.info("Created auto-Keep merge commit for '$targetRef' with parents [$targetRef, $currentRef]: ${fullSnapshot.size} files") + } catch (e: IOException) { + LOG.error("Failed to create auto-Keep merge commit for '$targetRef'", e) + } + return calculateChanges(currentState, targetState) + } + + // A lesson may have files that were non-propagatable in the previous step, but become propagatable in the new one. + // We allow files to change a propagation flag from false to true (from non-propagatable to propagatable). + // Course creators often have such use-case: + // They want to make some files invisible on some prefix of the task list, and then have them visible in the rest of the tasks + // So that they will be shown to students and will participate in solving the problem + // For more detailed explanation, see the documentation: + // https://jetbrains.team/p/edu/repositories/internal-documentation/files/subsystems/Framework%20Lessons/internal-part-ru.md + + // Calculate files that change propagation flag + // Note: We also check currentTask.taskFiles directly because invisible files are not included + // in currentNonPropagatableFilesState (since allFiles only returns visible files) + // We check if file is invisible AND not in a test directory (test files are handled by recreateNonPropagatableFiles) + val taskTestDirs = currentTask.testDirs + val fromNonPropagatableToPropagatableFilesState = targetPropagatableFilesState.filter { (path, _) -> + path in currentNonPropagatableFilesState || + (currentTask.taskFiles[path]?.isVisible == false && + taskTestDirs.none { testDir -> path.startsWith("$testDir/") || path == testDir }) + } + val fromPropagatableToNonPropagatableFilesState = targetNonPropagatableFilesState.filter { it.key in currentPropagatableFilesState } + + // We assume that files could not change a propagation flag from true to false (for example, from visible to invisible) + // This behavior is not intended + if (fromPropagatableToNonPropagatableFilesState.isNotEmpty()) { + LOG.error("Propagation flag change from propagatable to non-propagatable during navigation in non-template-based lessons is not supported") + } + + // Only files that do not change a propagation flag can participate in propagating user changes. + val newCurrentPropagatableFilesState = currentPropagatableFilesState.filter { it.key !in targetNonPropagatableFilesState } + val newTargetPropagatableFilesState = targetPropagatableFilesState.filter { it.key !in fromNonPropagatableToPropagatableFilesState } + + // Files that change propagation flag are processed separately: + // (Non-Propagatable -> Propagatable) - Changes for them are not propagated + // (Propagatable -> Non-Propagatable) - We assume that there are no such files + + // Creates Changes to propagate all current changes of task files to a target task. + // During propagation, we assume that in the not-template-based framework lessons, all the initial files are the same for each task. + // Therefore, we will only add user-created files and remove user-deleted files. + // During propagation, we do not change the text of the files. + fun calculateCurrentTaskChanges(): UserChanges { + val toRemove = HashMap(newTargetPropagatableFilesState) + val propagatableFileChanges = mutableListOf() + + for ((path, text) in newCurrentPropagatableFilesState) { + val targetText = toRemove.remove(path) + // Propagate user-created files + if (targetText == null) { + propagatableFileChanges += Change.PropagateLearnerCreatedTaskFile(path, text) + } + } + + // Remove user-deleted files + for ((path, _) in toRemove) { + propagatableFileChanges += Change.RemoveTaskFile(path) + } + + // Calculate diff for invisible files and files that become visible and change them without propagation + val nonPropagatableFileChanges = calculateChanges( + currentNonPropagatableFilesState, + targetNonPropagatableFilesState + fromNonPropagatableToPropagatableFilesState + ) + return nonPropagatableFileChanges + propagatableFileChanges + } + // Target task has no saved state - propagate changes without asking if (!targetHasStorage) { return calculateCurrentTaskChanges() @@ -959,108 +959,108 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson if (newTargetPropagatableFilesState.isEmpty()) { LOG.info("Target snapshot has no propagatable files, auto-Replace: propagating current state without dialog") val mergeMessage = "Merge from '${currentTask.name}': Auto-replace (target had no user files)" - try { - val currentFileEntries = currentPropagatableFilesState.toFileEntries(currentTask, originalNonPropagatableFilesCache[currentTask.id]) - storage.saveMergeSnapshot(targetRef, currentFileEntries, listOf(targetRef, currentRef), mergeMessage) - LOG.info("Created auto-Replace merge commit for '$targetRef' with parents [$targetRef, $currentRef]") - } catch (e: IOException) { - LOG.error("Failed to create auto-Replace merge commit for '$targetRef'", e) - } - return calculateCurrentTaskChanges() - } - - // If current and target propagatable files are identical - no need for dialog - // Just calculate diff for non-propagatable files - if (newCurrentPropagatableFilesState == newTargetPropagatableFilesState) { - return calculateChanges( - currentNonPropagatableFilesState, - targetNonPropagatableFilesState + fromNonPropagatableToPropagatableFilesState - ) - } - - // Target has saved state with propagatable files AND content differs - ask user before overwriting - val keepChanges = if (showDialogIfConflict) { - val currentTaskName = "${currentTask.getUIName()} ${currentTask.index}" - val targetTaskName = "${targetTask.getUIName()} ${targetTask.index}" - val result = PropagationConflictDialog.show( - project, - currentTaskName, - targetTaskName, - newCurrentPropagatableFilesState, - newTargetPropagatableFilesState - ) - result == PropagationConflictDialog.Result.KEEP - } - else { - // When dialog is suppressed, default to Keep (preserve target's content). - // This is the safest default for automated scenarios (e.g., lesson updates). - // For project open, the caller should pass showDialogIfConflict=true to let user decide. - LOG.info("Dialog suppressed, using Keep to preserve target content for '${targetTask.name}'") - true - } - - return if (keepChanges) { - // User chose Keep - create merge commit with target's content - // This records that we "merged" but kept our version (like git merge with "ours" strategy) - propagationActive = false - val mergeMessage = "Merge from '${currentTask.name}': Keep target changes" - try { - // Use buildFullSnapshotState to include both user files and test files from target task - val fullSnapshot = buildFullSnapshotState(targetTask, targetPropagatableFilesState) - storage.saveMergeSnapshot(targetRef, fullSnapshot, listOf(targetRef, currentRef), mergeMessage) - LOG.info("Created Keep merge commit for '$targetRef' with parents [$targetRef, $currentRef]: ${fullSnapshot.size} files") - } catch (e: IOException) { - LOG.error("Failed to create Keep merge commit for '$targetRef'", e) - } - calculateChanges(currentState, targetState) - } - else { - // User chose Replace - create merge commit with current's content - // This propagates changes from current stage to target stage - propagationActive = true - val mergeMessage = "Merge from '${currentTask.name}': Replace with propagated changes" - try { - // Use buildFullSnapshotState with targetTask to get target's test files, - // but with currentPropagatableFilesState to propagate user's code changes - val fullSnapshot = buildFullSnapshotState(targetTask, currentPropagatableFilesState) - storage.saveMergeSnapshot(targetRef, fullSnapshot, listOf(targetRef, currentRef), mergeMessage) - LOG.info("Created Replace merge commit for '$targetRef' with parents [$targetRef, $currentRef]: ${fullSnapshot.size} files") - } catch (e: IOException) { - LOG.error("Failed to create Replace merge commit for '$targetRef'", e) - } - calculateCurrentTaskChanges() - } - } - - /** - * Calculates changes for first visit to a new stage (forward navigation without existing snapshot). - * - * The key difference from [calculateChanges]: - * - For propagatable files: keep user's current state, only ADD new files from target templates - * - For non-propagatable files: use target templates (user couldn't modify them anyway) - * - * This preserves user's code while adding any new template files the author added for this stage. - * - * @param currentState Current propagatable files from disk (user's code) - * @param targetState Target templates (all visible non-test files from target task) - * @param targetTask The task we're navigating to (used to determine file propagation status) - */ + try { + val currentFileEntries = currentPropagatableFilesState.toFileEntries(currentTask, originalNonPropagatableFilesCache[currentTask.id]) + storage.saveMergeSnapshot(targetRef, currentFileEntries, listOf(targetRef, currentRef), mergeMessage) + LOG.info("Created auto-Replace merge commit for '$targetRef' with parents [$targetRef, $currentRef]") + } catch (e: IOException) { + LOG.error("Failed to create auto-Replace merge commit for '$targetRef'", e) + } + return calculateCurrentTaskChanges() + } + + // If current and target propagatable files are identical - no need for dialog + // Just calculate diff for non-propagatable files + if (newCurrentPropagatableFilesState == newTargetPropagatableFilesState) { + return calculateChanges( + currentNonPropagatableFilesState, + targetNonPropagatableFilesState + fromNonPropagatableToPropagatableFilesState + ) + } + + // Target has saved state with propagatable files AND content differs - ask user before overwriting + val keepChanges = if (showDialogIfConflict) { + val currentTaskName = "${currentTask.getUIName()} ${currentTask.index}" + val targetTaskName = "${targetTask.getUIName()} ${targetTask.index}" + val result = PropagationConflictDialog.show( + project, + currentTaskName, + targetTaskName, + newCurrentPropagatableFilesState, + newTargetPropagatableFilesState + ) + result == PropagationConflictDialog.Result.KEEP + } + else { + // When dialog is suppressed, default to Keep (preserve target's content). + // This is the safest default for automated scenarios (e.g., lesson updates). + // For project open, the caller should pass showDialogIfConflict=true to let user decide. + LOG.info("Dialog suppressed, using Keep to preserve target content for '${targetTask.name}'") + true + } + + return if (keepChanges) { + // User chose Keep - create merge commit with target's content + // This records that we "merged" but kept our version (like git merge with "ours" strategy) + propagationActive = false + val mergeMessage = "Merge from '${currentTask.name}': Keep target changes" + try { + // Use buildFullSnapshotState to include both user files and test files from target task + val fullSnapshot = buildFullSnapshotState(targetTask, targetPropagatableFilesState) + storage.saveMergeSnapshot(targetRef, fullSnapshot, listOf(targetRef, currentRef), mergeMessage) + LOG.info("Created Keep merge commit for '$targetRef' with parents [$targetRef, $currentRef]: ${fullSnapshot.size} files") + } catch (e: IOException) { + LOG.error("Failed to create Keep merge commit for '$targetRef'", e) + } + calculateChanges(currentState, targetState) + } + else { + // User chose Replace - create merge commit with current's content + // This propagates changes from current stage to target stage + propagationActive = true + val mergeMessage = "Merge from '${currentTask.name}': Replace with propagated changes" + try { + // Use buildFullSnapshotState with targetTask to get target's test files, + // but with currentPropagatableFilesState to propagate user's code changes + val fullSnapshot = buildFullSnapshotState(targetTask, currentPropagatableFilesState) + storage.saveMergeSnapshot(targetRef, fullSnapshot, listOf(targetRef, currentRef), mergeMessage) + LOG.info("Created Replace merge commit for '$targetRef' with parents [$targetRef, $currentRef]: ${fullSnapshot.size} files") + } catch (e: IOException) { + LOG.error("Failed to create Replace merge commit for '$targetRef'", e) + } + calculateCurrentTaskChanges() + } + } + + /** + * Calculates changes for first visit to a new stage (forward navigation without existing snapshot). + * + * The key difference from [calculateChanges]: + * - For propagatable files: keep user's current state, only ADD new files from target templates + * - For non-propagatable files: use target templates (user couldn't modify them anyway) + * + * This preserves user's code while adding any new template files the author added for this stage. + * + * @param currentState Current propagatable files from disk (user's code) + * @param targetState Target templates (all visible non-test files from target task) + * @param targetTask The task we're navigating to (used to determine file propagation status) + */ private fun calculateFirstVisitChanges( currentState: FLTaskState, targetState: FLTaskState, currentTask: Task, targetTask: Task ): UserChanges { - val changes = mutableListOf() - - // 1. Propagate user-created files from current state that are NOT in target template - for ((path, text) in currentState) { - if (path !in targetState) { - LOG.info("First visit: propagating user-created file '$path'") - changes += Change.PropagateLearnerCreatedTaskFile(path, text) - } - } - + val changes = mutableListOf() + + // 1. Propagate user-created files from current state that are NOT in target template + for ((path, text) in currentState) { + if (path !in targetState) { + LOG.info("First visit: propagating user-created file '$path'") + changes += Change.PropagateLearnerCreatedTaskFile(path, text) + } + } + // 2. Handle files from target template for ((path, text) in targetState) { val taskFile = targetTask.taskFiles[path] @@ -1109,82 +1109,82 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson // If it's in both, we keep the user's version from currentState (it's already on disk) } else { - // Non-propagatable files (e.g., read-only reference files): - // Always use target version since user couldn't modify them - LOG.info("First visit: adding non-propagatable file '$path'") - changes += Change.AddFile(path, text) - } - } - - LOG.info("First visit changes: ${changes.size} changes") - return UserChanges(changes) - } - - private fun getUserChangesFromFiles(initialState: FLTaskState, taskDir: VirtualFile): UserChanges { - val currentState = getTaskStateFromFiles(initialState.keys, taskDir) - return calculateChanges(initialState, currentState) - } - - private fun recreateStorage() { - try { - Disposer.dispose(storage) - val storageFilePath = constructStoragePath(project) - FrameworkStorage.deleteFiles(storageFilePath) - resetAllTaskRecords(project) - storage = createStorage(project) - - LOG.warn("Storage recreated successfully after corruption") - } - catch (recreateError: Exception) { - LOG.error("Failed to recreate storage after corruption", recreateError) - } - } - - override fun storeOriginalTestFiles(task: Task) { - // ALT-10961: Don't overwrite cache if it already contains data loaded from API. - // This is important because task.taskFiles may contain stale data from disk - // (in framework lessons, all stages share the same task directory). - // Data loaded from API via loadNonPropagatableFilesFromApi() is always correct. - if (originalNonPropagatableFilesCache.containsKey(task.id)) { - LOG.info("Cache already contains non-propagatable files for task '${task.name}' (step ${task.id}), not overwriting") - return - } - storeNonPropagatableFilesInternal(task) - } - - override fun updateOriginalTestFiles(task: Task) { - // Force update the cache, used when task files are updated from remote server - // (e.g., during course update). Unlike storeOriginalTestFiles, this WILL overwrite. - // Pass forceUpdate=true to handle the case when author removes all non-propagatable files. - LOG.info("Force updating non-propagatable files cache for task '${task.name}' (step ${task.id})") - storeNonPropagatableFilesInternal(task, forceUpdate = true) - } - - override fun updateSnapshotTestFiles(task: Task) { - require(task.lesson is FrameworkLesson) { - "Only framework task snapshots can be updated" - } - - val ref = task.storageRef() - - // Only update if snapshot exists (user visited this task before) - if (!storage.hasRef(ref)) { - LOG.info("updateSnapshotTestFiles: No snapshot exists for task '${task.name}' (ref=$ref), skipping") - return - } - - // Get cached non-propagatable files (should be updated via updateOriginalTestFiles before calling this) - // Note: null means cache wasn't updated (can't proceed), empty map means author removed all non-propagatable files - val cachedNonPropagatableFiles = originalNonPropagatableFilesCache[task.id] - if (cachedNonPropagatableFiles == null) { - LOG.warn("updateSnapshotTestFiles: No cached non-propagatable files for task '${task.name}', cannot update snapshot") - return - } - - try { - // Get current snapshot (Map) - val currentSnapshot = storage.getSnapshot(ref) - + // Non-propagatable files (e.g., read-only reference files): + // Always use target version since user couldn't modify them + LOG.info("First visit: adding non-propagatable file '$path'") + changes += Change.AddFile(path, text) + } + } + + LOG.info("First visit changes: ${changes.size} changes") + return UserChanges(changes) + } + + private fun getUserChangesFromFiles(initialState: FLTaskState, taskDir: VirtualFile): UserChanges { + val currentState = getTaskStateFromFiles(initialState.keys, taskDir) + return calculateChanges(initialState, currentState) + } + + private fun recreateStorage() { + try { + Disposer.dispose(storage) + val storageFilePath = constructStoragePath(project) + FrameworkStorage.deleteFiles(storageFilePath) + resetAllTaskRecords(project) + storage = createStorage(project) + + LOG.warn("Storage recreated successfully after corruption") + } + catch (recreateError: Exception) { + LOG.error("Failed to recreate storage after corruption", recreateError) + } + } + + override fun storeOriginalTestFiles(task: Task) { + // ALT-10961: Don't overwrite cache if it already contains data loaded from API. + // This is important because task.taskFiles may contain stale data from disk + // (in framework lessons, all stages share the same task directory). + // Data loaded from API via loadNonPropagatableFilesFromApi() is always correct. + if (originalNonPropagatableFilesCache.containsKey(task.id)) { + LOG.info("Cache already contains non-propagatable files for task '${task.name}' (step ${task.id}), not overwriting") + return + } + storeNonPropagatableFilesInternal(task) + } + + override fun updateOriginalTestFiles(task: Task) { + // Force update the cache, used when task files are updated from remote server + // (e.g., during course update). Unlike storeOriginalTestFiles, this WILL overwrite. + // Pass forceUpdate=true to handle the case when author removes all non-propagatable files. + LOG.info("Force updating non-propagatable files cache for task '${task.name}' (step ${task.id})") + storeNonPropagatableFilesInternal(task, forceUpdate = true) + } + + override fun updateSnapshotTestFiles(task: Task) { + require(task.lesson is FrameworkLesson) { + "Only framework task snapshots can be updated" + } + + val ref = task.storageRef() + + // Only update if snapshot exists (user visited this task before) + if (!storage.hasRef(ref)) { + LOG.info("updateSnapshotTestFiles: No snapshot exists for task '${task.name}' (ref=$ref), skipping") + return + } + + // Get cached non-propagatable files (should be updated via updateOriginalTestFiles before calling this) + // Note: null means cache wasn't updated (can't proceed), empty map means author removed all non-propagatable files + val cachedNonPropagatableFiles = originalNonPropagatableFilesCache[task.id] + if (cachedNonPropagatableFiles == null) { + LOG.warn("updateSnapshotTestFiles: No cached non-propagatable files for task '${task.name}', cannot update snapshot") + return + } + + try { + // Get current snapshot (Map) + val currentSnapshot = storage.getSnapshot(ref) + // Get propagatable file names from task model to identify user files val propagatableFileNames = task.taskFiles.filterValues { it.shouldBePropagated() }.keys @@ -1193,121 +1193,121 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson val userFiles = currentSnapshot.filter { (path, entry) -> path in propagatableFileNames || entry.isPropagatable && entry.hasLearnerModification } - - // Combine user files with new non-propagatable files from cache - // If cache is empty, this effectively removes all non-propagatable files from snapshot - val updatedSnapshot = HashMap(userFiles) - for ((path, taskFile) in cachedNonPropagatableFiles) { - updatedSnapshot[path] = FileEntry.create( - content = taskFile.contents.textualRepresentation, - visible = taskFile.isVisible, - editable = taskFile.isEditable, - propagatable = taskFile.shouldBePropagated() - ) - } - - // Save updated snapshot - val message = if (cachedNonPropagatableFiles.isEmpty()) { - "Remove all non-propagatable files from server for '${task.name}'" - } else { - "Update non-propagatable files from server for '${task.name}'" - } + + // Combine user files with new non-propagatable files from cache + // If cache is empty, this effectively removes all non-propagatable files from snapshot + val updatedSnapshot = HashMap(userFiles) + for ((path, taskFile) in cachedNonPropagatableFiles) { + updatedSnapshot[path] = FileEntry.create( + content = taskFile.contents.textualRepresentation, + visible = taskFile.isVisible, + editable = taskFile.isEditable, + propagatable = taskFile.shouldBePropagated() + ) + } + + // Save updated snapshot + val message = if (cachedNonPropagatableFiles.isEmpty()) { + "Remove all non-propagatable files from server for '${task.name}'" + } else { + "Update non-propagatable files from server for '${task.name}'" + } val created = storage.saveSnapshot(ref, updatedSnapshot, getParentRef(task), message) - if (created) { - LOG.info("updateSnapshotTestFiles: Updated snapshot for task '${task.name}' with ${cachedNonPropagatableFiles.size} non-propagatable files") - } else { - LOG.info("updateSnapshotTestFiles: Snapshot unchanged for task '${task.name}' (files identical)") - } - } - catch (e: IOException) { - LOG.error("Failed to update snapshot non-propagatable files for task '${task.name}'", e) - } - } - - private fun storeNonPropagatableFilesInternal(task: Task, forceUpdate: Boolean = false) { - // Non-propagatable files: test files + hidden files (invisible or non-editable) - val nonPropagatableFiles = task.taskFiles.filterValues { taskFile -> - !taskFile.isLearnerCreated && !taskFile.shouldBePropagated() - } - if (nonPropagatableFiles.isNotEmpty()) { - // Create copies of TaskFile objects to prevent modification when original task.taskFiles changes - // This is important because task.taskFiles contents can be updated (e.g., during Update Course) - // and we want to preserve the original file contents - val copiedFiles = nonPropagatableFiles.mapValues { (_, taskFile) -> - TaskFile(taskFile.name, taskFile.contents).also { - it.isVisible = taskFile.isVisible - it.isEditable = taskFile.isEditable - it.isLearnerCreated = taskFile.isLearnerCreated - } - } - originalNonPropagatableFilesCache[task.id] = copiedFiles - val filesInfo = copiedFiles.entries.joinToString { (name, file) -> - "$name:size=${file.contents.textualRepresentation.length}" - } - LOG.info("Stored ${copiedFiles.size} original non-propagatable files for task '${task.name}' (step ${task.id}): [$filesInfo]") - } else if (forceUpdate) { - // Author removed all non-propagatable files - store empty map to reflect this - originalNonPropagatableFilesCache[task.id] = emptyMap() - LOG.info("Stored empty non-propagatable files cache for task '${task.name}' (step ${task.id})") - } - } - - override fun getOriginalTestFiles(task: Task): Collection? { - return originalNonPropagatableFilesCache[task.id]?.values - } - - override fun ensureTestFilesCached(task: Task): Boolean { - if (originalNonPropagatableFilesCache.containsKey(task.id)) { - return true - } - // Cache is empty, try to load from API - return loadNonPropagatableFilesFromApi(task) != null - } - - /** - * Stores original template files (visible non-test files) for the task. - * These templates are used for calculating user changes correctly. - * - * IMPORTANT: Call this when task files are loaded from API (fresh data). - * Do NOT call after user may have modified files, as TaskFile.contents may be stale. - */ - override fun storeOriginalTemplateFiles(task: Task) { - // Don't overwrite cache if it already contains data - if (originalTemplateFilesCache.containsKey(task.id)) { - LOG.info("Template cache already contains files for task '${task.name}' (step ${task.id}), not overwriting") - return - } - - LOG.debug("storeOriginalTemplateFiles: task='${task.name}', taskFiles=${task.taskFiles.keys}") - task.taskFiles.forEach { (name, taskFile) -> - LOG.debug("storeOriginalTemplateFiles: file='$name', isVisible=${taskFile.isVisible}, isTestFile=${taskFile.isTestFile}") - } - + if (created) { + LOG.info("updateSnapshotTestFiles: Updated snapshot for task '${task.name}' with ${cachedNonPropagatableFiles.size} non-propagatable files") + } else { + LOG.info("updateSnapshotTestFiles: Snapshot unchanged for task '${task.name}' (files identical)") + } + } + catch (e: IOException) { + LOG.error("Failed to update snapshot non-propagatable files for task '${task.name}'", e) + } + } + + private fun storeNonPropagatableFilesInternal(task: Task, forceUpdate: Boolean = false) { + // Non-propagatable files: test files + hidden files (invisible or non-editable) + val nonPropagatableFiles = task.taskFiles.filterValues { taskFile -> + !taskFile.isLearnerCreated && !taskFile.shouldBePropagated() + } + if (nonPropagatableFiles.isNotEmpty()) { + // Create copies of TaskFile objects to prevent modification when original task.taskFiles changes + // This is important because task.taskFiles contents can be updated (e.g., during Update Course) + // and we want to preserve the original file contents + val copiedFiles = nonPropagatableFiles.mapValues { (_, taskFile) -> + TaskFile(taskFile.name, taskFile.contents).also { + it.isVisible = taskFile.isVisible + it.isEditable = taskFile.isEditable + it.isLearnerCreated = taskFile.isLearnerCreated + } + } + originalNonPropagatableFilesCache[task.id] = copiedFiles + val filesInfo = copiedFiles.entries.joinToString { (name, file) -> + "$name:size=${file.contents.textualRepresentation.length}" + } + LOG.info("Stored ${copiedFiles.size} original non-propagatable files for task '${task.name}' (step ${task.id}): [$filesInfo]") + } else if (forceUpdate) { + // Author removed all non-propagatable files - store empty map to reflect this + originalNonPropagatableFilesCache[task.id] = emptyMap() + LOG.info("Stored empty non-propagatable files cache for task '${task.name}' (step ${task.id})") + } + } + + override fun getOriginalTestFiles(task: Task): Collection? { + return originalNonPropagatableFilesCache[task.id]?.values + } + + override fun ensureTestFilesCached(task: Task): Boolean { + if (originalNonPropagatableFilesCache.containsKey(task.id)) { + return true + } + // Cache is empty, try to load from API + return loadNonPropagatableFilesFromApi(task) != null + } + + /** + * Stores original template files (visible non-test files) for the task. + * These templates are used for calculating user changes correctly. + * + * IMPORTANT: Call this when task files are loaded from API (fresh data). + * Do NOT call after user may have modified files, as TaskFile.contents may be stale. + */ + override fun storeOriginalTemplateFiles(task: Task) { + // Don't overwrite cache if it already contains data + if (originalTemplateFilesCache.containsKey(task.id)) { + LOG.info("Template cache already contains files for task '${task.name}' (step ${task.id}), not overwriting") + return + } + + LOG.debug("storeOriginalTemplateFiles: task='${task.name}', taskFiles=${task.taskFiles.keys}") + task.taskFiles.forEach { (name, taskFile) -> + LOG.debug("storeOriginalTemplateFiles: file='$name', isVisible=${taskFile.isVisible}, isTestFile=${taskFile.isTestFile}") + } + val templateFiles = task.taskFiles.filterValues { taskFile -> taskFile.isVisible && !taskFile.isTestFile && !taskFile.isLearnerCreated } - LOG.debug("storeOriginalTemplateFiles: filtered templateFiles=${templateFiles.keys}") - - if (templateFiles.isNotEmpty()) { - // Store only the content (as String), not the TaskFile objects - val cachedTemplates = templateFiles.mapValues { (_, taskFile) -> - taskFile.contents.textualRepresentation - } - originalTemplateFilesCache[task.id] = cachedTemplates - val filesInfo = cachedTemplates.entries.joinToString { (name, content) -> - "$name:size=${content.length}" - } - LOG.info("Stored ${cachedTemplates.size} original template files for task '${task.name}' (step ${task.id}): [$filesInfo]") - - // ALT-10961: If task has no data in storage yet, save these templates as the initial snapshot. - // This ensures we have a base state even after IDE restart/offline. - if (!hasStorageData(task)) { - LOG.info("Task '${task.name}' has no storage data, saving templates as initial snapshot") - saveExternalChanges(task, cachedTemplates) - } - } - } - + LOG.debug("storeOriginalTemplateFiles: filtered templateFiles=${templateFiles.keys}") + + if (templateFiles.isNotEmpty()) { + // Store only the content (as String), not the TaskFile objects + val cachedTemplates = templateFiles.mapValues { (_, taskFile) -> + taskFile.contents.textualRepresentation + } + originalTemplateFilesCache[task.id] = cachedTemplates + val filesInfo = cachedTemplates.entries.joinToString { (name, content) -> + "$name:size=${content.length}" + } + LOG.info("Stored ${cachedTemplates.size} original template files for task '${task.name}' (step ${task.id}): [$filesInfo]") + + // ALT-10961: If task has no data in storage yet, save these templates as the initial snapshot. + // This ensures we have a base state even after IDE restart/offline. + if (!hasStorageData(task)) { + LOG.info("Task '${task.name}' has no storage data, saving templates as initial snapshot") + saveExternalChanges(task, cachedTemplates) + } + } + } + override fun updateOriginalTemplateFiles(task: Task) { // Force update the cache, used when task files are updated from remote server // (e.g., during course update). Unlike storeOriginalTemplateFiles, this WILL overwrite. @@ -1329,176 +1329,176 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson } LOG.info("Updated ${cachedTemplates.size} original template files for task '${task.name}' (step ${task.id}): [$filesInfo]") } - - /** - * Loads template files from API and caches them. - * This is used as a fallback when the cache is empty. - * - * IMPORTANT: This method must NOT block EDT. If called from EDT, it starts async loading - * and returns null immediately. The caller should use task.taskFiles as fallback. - */ - private fun loadTemplateFilesFromApi(task: Task): Map? { - val stepId = task.id - if (stepId <= 0) { - LOG.warn("Cannot load template files from API: task '${task.name}' has invalid step ID: $stepId") - return null - } - - // Don't block EDT - start async loading and return null - if (ApplicationManager.getApplication().isDispatchThread) { - LOG.info("On EDT, starting async load for template files of task '${task.name}' (step $stepId)") - ApplicationManager.getApplication().executeOnPooledThread { - loadTemplateFilesFromApiSync(task) - } - return null - } - - return loadTemplateFilesFromApiSync(task) - } - - /** - * Synchronously loads template files from API. Must NOT be called from EDT. - */ - private fun loadTemplateFilesFromApiSync(task: Task): Map? { - val stepId = task.id - LOG.info("Loading template files from API for task '${task.name}' (step $stepId)") - - return try { - val stepSourceResult = HyperskillConnector.getInstance().getStepSourceAnonymous(stepId) - if (stepSourceResult is Err) { - LOG.warn("Failed to load step source from API for step $stepId: ${stepSourceResult.error}") - return null - } - - val stepSource = (stepSourceResult as Ok).value - val options = stepSource.block?.options as? PyCharmStepOptions - if (options == null) { - LOG.warn("Step $stepId has no PyCharmStepOptions") - return null - } - - val allFiles = options.files - if (allFiles.isNullOrEmpty()) { - LOG.warn("Step $stepId has no files in options") - return null - } - - // Filter visible non-test files - val templateFiles = allFiles.filter { taskFile -> - taskFile.isVisible && !taskFile.isLearnerCreated && "test" !in taskFile.name.lowercase() - } - - if (templateFiles.isEmpty()) { - LOG.warn("Step $stepId has no template files") - return null - } - - val cachedTemplates = templateFiles.associate { taskFile -> - taskFile.name to taskFile.contents.textualRepresentation - } - - originalTemplateFilesCache[stepId] = cachedTemplates - val filesInfo = cachedTemplates.entries.joinToString { (name, content) -> - "$name:size=${content.length}" - } - LOG.info("Loaded and cached ${cachedTemplates.size} template files from API for task '${task.name}' (step $stepId): [$filesInfo]") - - cachedTemplates - } - catch (e: Exception) { - LOG.warn("Exception while loading template files from API for step $stepId", e) - null - } - } - - /** - * Ensures template files are cached for the task. - * If not cached, attempts to load from API. - */ - override fun ensureTemplateFilesCached(task: Task): Boolean { - if (originalTemplateFilesCache.containsKey(task.id)) { - return true - } - return loadTemplateFilesFromApi(task) != null - } - - override fun dispose() { - Disposer.dispose(storage) - originalNonPropagatableFilesCache.clear() - originalTemplateFilesCache.clear() - } - - /** - * Saves a snapshot of the current task's state from disk. - * Called when files are saved (Ctrl+S, auto-save, before IDE closes). - * This ensures user changes are persisted even without navigation. - */ - private fun saveCurrentTaskSnapshot() { - // Skip auto-save during navigation - navigation code handles saving with proper messages - if (isNavigating) return - - // Skip auto-save until currentTaskIndex is synced with storage HEAD. - // This prevents saving wrong stage content when project opens and currentTaskIndex is still 0. - if (!isStorageSynced) { - LOG.info("saveCurrentTaskSnapshot: skipped - storage not synced yet") - return - } - - val course = StudyTaskManager.getInstance(project).course ?: return - val lesson = course.lessons.filterIsInstance().firstOrNull() ?: return - val currentTask = lesson.currentTask() ?: return - val taskDir = currentTask.getDir(project.courseDir) ?: return - - // Read current disk state (all files including user-created). - // Note: do NOT early-return when propagatableFiles is empty — the user may have legitimately - // deleted every editable file, and the snapshot must be updated to reflect that. The equality - // check below will short-circuit cases where there is genuinely nothing to save. + + /** + * Loads template files from API and caches them. + * This is used as a fallback when the cache is empty. + * + * IMPORTANT: This method must NOT block EDT. If called from EDT, it starts async loading + * and returns null immediately. The caller should use task.taskFiles as fallback. + */ + private fun loadTemplateFilesFromApi(task: Task): Map? { + val stepId = task.id + if (stepId <= 0) { + LOG.warn("Cannot load template files from API: task '${task.name}' has invalid step ID: $stepId") + return null + } + + // Don't block EDT - start async loading and return null + if (ApplicationManager.getApplication().isDispatchThread) { + LOG.info("On EDT, starting async load for template files of task '${task.name}' (step $stepId)") + ApplicationManager.getApplication().executeOnPooledThread { + loadTemplateFilesFromApiSync(task) + } + return null + } + + return loadTemplateFilesFromApiSync(task) + } + + /** + * Synchronously loads template files from API. Must NOT be called from EDT. + */ + private fun loadTemplateFilesFromApiSync(task: Task): Map? { + val stepId = task.id + LOG.info("Loading template files from API for task '${task.name}' (step $stepId)") + + return try { + val stepSourceResult = HyperskillConnector.getInstance().getStepSourceAnonymous(stepId) + if (stepSourceResult is Err) { + LOG.warn("Failed to load step source from API for step $stepId: ${stepSourceResult.error}") + return null + } + + val stepSource = (stepSourceResult as Ok).value + val options = stepSource.block?.options as? PyCharmStepOptions + if (options == null) { + LOG.warn("Step $stepId has no PyCharmStepOptions") + return null + } + + val allFiles = options.files + if (allFiles.isNullOrEmpty()) { + LOG.warn("Step $stepId has no files in options") + return null + } + + // Filter visible non-test files + val templateFiles = allFiles.filter { taskFile -> + taskFile.isVisible && !taskFile.isLearnerCreated && "test" !in taskFile.name.lowercase() + } + + if (templateFiles.isEmpty()) { + LOG.warn("Step $stepId has no template files") + return null + } + + val cachedTemplates = templateFiles.associate { taskFile -> + taskFile.name to taskFile.contents.textualRepresentation + } + + originalTemplateFilesCache[stepId] = cachedTemplates + val filesInfo = cachedTemplates.entries.joinToString { (name, content) -> + "$name:size=${content.length}" + } + LOG.info("Loaded and cached ${cachedTemplates.size} template files from API for task '${task.name}' (step $stepId): [$filesInfo]") + + cachedTemplates + } + catch (e: Exception) { + LOG.warn("Exception while loading template files from API for step $stepId", e) + null + } + } + + /** + * Ensures template files are cached for the task. + * If not cached, attempts to load from API. + */ + override fun ensureTemplateFilesCached(task: Task): Boolean { + if (originalTemplateFilesCache.containsKey(task.id)) { + return true + } + return loadTemplateFilesFromApi(task) != null + } + + override fun dispose() { + Disposer.dispose(storage) + originalNonPropagatableFilesCache.clear() + originalTemplateFilesCache.clear() + } + + /** + * Saves a snapshot of the current task's state from disk. + * Called when files are saved (Ctrl+S, auto-save, before IDE closes). + * This ensures user changes are persisted even without navigation. + */ + private fun saveCurrentTaskSnapshot() { + // Skip auto-save during navigation - navigation code handles saving with proper messages + if (isNavigating) return + + // Skip auto-save until currentTaskIndex is synced with storage HEAD. + // This prevents saving wrong stage content when project opens and currentTaskIndex is still 0. + if (!isStorageSynced) { + LOG.info("saveCurrentTaskSnapshot: skipped - storage not synced yet") + return + } + + val course = StudyTaskManager.getInstance(project).course ?: return + val lesson = course.lessons.filterIsInstance().firstOrNull() ?: return + val currentTask = lesson.currentTask() ?: return + val taskDir = currentTask.getDir(project.courseDir) ?: return + + // Read current disk state (all files including user-created). + // Note: do NOT early-return when propagatableFiles is empty — the user may have legitimately + // deleted every editable file, and the snapshot must be updated to reflect that. The equality + // check below will short-circuit cases where there is genuinely nothing to save. val currentDiskState = getAllFilesFromTaskDir(taskDir, currentTask) - val (propagatableFiles, _) = currentDiskState.split(currentTask) - - // Check if there are actual changes compared to saved snapshot (compare only user files) - val ref = currentTask.storageRef() - val existingSnapshot = try { - if (storage.hasRef(ref)) storage.getSnapshot(ref).toContentMap() else emptyMap() - } catch (e: IOException) { - emptyMap() - } - - // Extract only user files from existing snapshot for comparison - val existingUserFiles = existingSnapshot.filterKeys { path -> - currentTask.getTaskFile(path)?.shouldBePropagated() ?: true - } - if (propagatableFiles == existingUserFiles) { - // No changes to save - return - } - - // Build full snapshot: user files from disk + non-propagatable files from cache - val fullSnapshot = buildFullSnapshotState(currentTask, propagatableFiles) - - // Save full snapshot - try { - val created = storage.saveSnapshot(ref, fullSnapshot, getParentRef(currentTask), "Auto-save changes for '${currentTask.name}'") - if (created) { - LOG.info("Auto-saved full snapshot for task '${currentTask.name}' (ref=$ref): ${fullSnapshot.size} files") - } - } catch (e: IOException) { - LOG.warn("Failed to auto-save snapshot for task '${currentTask.name}'", e) - } - } - - /** - * Returns the state of all non-test files for this task (user-editable files). - * This includes both template files (visible, editable) and learner-created files. - * - * Test files are excluded here because they are handled separately. - * For full snapshot including tests, use [getFullTaskState]. - */ - private val Task.allFiles: FLTaskState - get() = taskFiles - .filterValues { !it.isTestFile } - .mapValues { it.value.contents.textualRepresentation } - + val (propagatableFiles, _) = currentDiskState.split(currentTask) + + // Check if there are actual changes compared to saved snapshot (compare only user files) + val ref = currentTask.storageRef() + val existingSnapshot = try { + if (storage.hasRef(ref)) storage.getSnapshot(ref).toContentMap() else emptyMap() + } catch (e: IOException) { + emptyMap() + } + + // Extract only user files from existing snapshot for comparison + val existingUserFiles = existingSnapshot.filterKeys { path -> + currentTask.getTaskFile(path)?.shouldBePropagated() ?: true + } + if (propagatableFiles == existingUserFiles) { + // No changes to save + return + } + + // Build full snapshot: user files from disk + non-propagatable files from cache + val fullSnapshot = buildFullSnapshotState(currentTask, propagatableFiles) + + // Save full snapshot + try { + val created = storage.saveSnapshot(ref, fullSnapshot, getParentRef(currentTask), "Auto-save changes for '${currentTask.name}'") + if (created) { + LOG.info("Auto-saved full snapshot for task '${currentTask.name}' (ref=$ref): ${fullSnapshot.size} files") + } + } catch (e: IOException) { + LOG.warn("Failed to auto-save snapshot for task '${currentTask.name}'", e) + } + } + + /** + * Returns the state of all non-test files for this task (user-editable files). + * This includes both template files (visible, editable) and learner-created files. + * + * Test files are excluded here because they are handled separately. + * For full snapshot including tests, use [getFullTaskState]. + */ + private val Task.allFiles: FLTaskState + get() = taskFiles + .filterValues { !it.isTestFile } + .mapValues { it.value.contents.textualRepresentation } + /** * Returns ALL files for this task including test files. * Used for creating complete snapshots. Learner-created files are excluded because @@ -1508,51 +1508,51 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson get() = taskFiles .filterValues { !it.isLearnerCreated } .mapValues { it.value.contents.textualRepresentation } - - /** - * Reads ALL files from task directory, including user-created files. - * This is needed to capture user-created files that are not in the template. - * - * @param taskDir The task directory to read files from - * @param task The task (used to filter out test directories) - * @return Map of file paths to content - */ - private fun getAllFilesFromTaskDir(taskDir: VirtualFile, task: Task): FLTaskState { - val result = HashMap() - val documentManager = FileDocumentManager.getInstance() - val testDirs = task.testDirs - val configurator = task.course.configurator - - // Recursively collect all files from task directory - fun collectFiles(dir: VirtualFile, pathPrefix: String = "") { - for (child in dir.children) { - if (configurator?.excludeFromArchive(project, child) == true) continue - val relativePath = if (pathPrefix.isEmpty()) child.name else "$pathPrefix/${child.name}" - - if (child.isDirectory) { - // Skip test directories - they will be handled separately - val isTestDir = testDirs.any { testDir -> - relativePath == testDir || relativePath.startsWith("$testDir/") - } - if (!isTestDir) { - collectFiles(child, relativePath) - } - } - else { - // Read file content - val text = if (child.isToEncodeContent) { - child.loadEncodedContent(isToEncodeContent = true) - } - else { - runReadAction { documentManager.getDocument(child)?.text } - } - - if (text != null) { - result[relativePath] = text - } - } - } - } + + /** + * Reads ALL files from task directory, including user-created files. + * This is needed to capture user-created files that are not in the template. + * + * @param taskDir The task directory to read files from + * @param task The task (used to filter out test directories) + * @return Map of file paths to content + */ + private fun getAllFilesFromTaskDir(taskDir: VirtualFile, task: Task): FLTaskState { + val result = HashMap() + val documentManager = FileDocumentManager.getInstance() + val testDirs = task.testDirs + val configurator = task.course.configurator + + // Recursively collect all files from task directory + fun collectFiles(dir: VirtualFile, pathPrefix: String = "") { + for (child in dir.children) { + if (configurator?.excludeFromArchive(project, child) == true) continue + val relativePath = if (pathPrefix.isEmpty()) child.name else "$pathPrefix/${child.name}" + + if (child.isDirectory) { + // Skip test directories - they will be handled separately + val isTestDir = testDirs.any { testDir -> + relativePath == testDir || relativePath.startsWith("$testDir/") + } + if (!isTestDir) { + collectFiles(child, relativePath) + } + } + else { + // Read file content + val text = if (child.isToEncodeContent) { + child.loadEncodedContent(isToEncodeContent = true) + } + else { + runReadAction { documentManager.getDocument(child)?.text } + } + + if (text != null) { + result[relativePath] = text + } + } + } + } collectFiles(taskDir) return result.withoutDeletedTemplateFiles(task) @@ -1593,43 +1593,43 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson /** * Builds complete task state for snapshot: user files from disk + non-propagatable files from cache. - * Non-propagatable files (test files, hidden files) are taken from cache (not disk) - * because disk may have files from another stage. - * - * Returns FileEntry objects with metadata (visible, editable, propagatable) extracted from TaskFile objects. - * - * @param task The task to build state for - * @param userFilesFromDisk User files read from disk (propagatable files) - * @return Complete state with user files and non-propagatable files as FileEntry objects - */ - private fun buildFullSnapshotState( - task: Task, - userFilesFromDisk: FLTaskState - ): Map { - val result = HashMap() - val testDirs = task.testDirs - - // Add user files with metadata from task.taskFiles or path patterns - for ((path, content) in userFilesFromDisk) { - result[path] = resolveFileEntryMetadata(path, content, task, testDirs) - } - - // Add non-propagatable files from cache, API, or task.taskFiles (in that priority order) - val nonPropagatableFiles = getNonPropagatableFilesWithMetadata(task) - for ((path, taskFile) in nonPropagatableFiles) { - val content = taskFile.contents.textualRepresentation - result[path] = resolveFileEntryMetadata(path, content, task, testDirs) - } - - return result - } - - /** - * Resolves metadata for a file entry using the following priority: - * 1. Path patterns (test files are ALWAYS non-visible, non-propagatable) - * 2. task.taskFiles (from API) - * 3. Default metadata (visible, editable, propagatable) - */ + * Non-propagatable files (test files, hidden files) are taken from cache (not disk) + * because disk may have files from another stage. + * + * Returns FileEntry objects with metadata (visible, editable, propagatable) extracted from TaskFile objects. + * + * @param task The task to build state for + * @param userFilesFromDisk User files read from disk (propagatable files) + * @return Complete state with user files and non-propagatable files as FileEntry objects + */ + private fun buildFullSnapshotState( + task: Task, + userFilesFromDisk: FLTaskState + ): Map { + val result = HashMap() + val testDirs = task.testDirs + + // Add user files with metadata from task.taskFiles or path patterns + for ((path, content) in userFilesFromDisk) { + result[path] = resolveFileEntryMetadata(path, content, task, testDirs) + } + + // Add non-propagatable files from cache, API, or task.taskFiles (in that priority order) + val nonPropagatableFiles = getNonPropagatableFilesWithMetadata(task) + for ((path, taskFile) in nonPropagatableFiles) { + val content = taskFile.contents.textualRepresentation + result[path] = resolveFileEntryMetadata(path, content, task, testDirs) + } + + return result + } + + /** + * Resolves metadata for a file entry using the following priority: + * 1. Path patterns (test files are ALWAYS non-visible, non-propagatable) + * 2. task.taskFiles (from API) + * 3. Default metadata (visible, editable, propagatable) + */ private fun resolveFileEntryMetadata( path: String, content: String, @@ -1637,34 +1637,34 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson testDirs: List, taskFiles: Map = task.taskFiles ): FileEntry { - // 1. Path patterns have highest priority - test files are ALWAYS hidden/non-propagatable - if (FileEntry.isTestFilePath(path, testDirs)) { - return FileEntry.create(content, visible = false, editable = false, propagatable = false) - } - - // 2. Check task.taskFiles - metadata from API (author's intent) + // 1. Path patterns have highest priority - test files are ALWAYS hidden/non-propagatable + if (FileEntry.isTestFilePath(path, testDirs)) { + return FileEntry.create(content, visible = false, editable = false, propagatable = false) + } + + // 2. Check task.taskFiles - metadata from API (author's intent) val taskFile = taskFiles[path] - if (taskFile != null) { - return FileEntry.create( - content = content, - visible = taskFile.isVisible, - editable = taskFile.isEditable, - propagatable = taskFile.shouldBePropagated() - ) - } - - // 3. Default: user file (visible, editable, propagatable) - return FileEntry(content) - } - + if (taskFile != null) { + return FileEntry.create( + content = content, + visible = taskFile.isVisible, + editable = taskFile.isEditable, + propagatable = taskFile.shouldBePropagated() + ) + } + + // 3. Default: user file (visible, editable, propagatable) + return FileEntry(content) + } + private fun FLTaskState.splitByKey(predicate: (String) -> Boolean): Pair { val positive = HashMap() val negative = HashMap() - - for ((path, text) in this) { - val state = if (predicate(path)) positive else negative - state[path] = text - } + + for ((path, text) in this) { + val state = if (predicate(path)) positive else negative + state[path] = text + } return positive to negative } @@ -1729,194 +1729,194 @@ class FrameworkLessonManagerImpl(private val project: Project) : FrameworkLesson /** * Checks if the current ref has propagatable file changes compared to its parent commit. * Returns true if: - * - There's no parent (first commit) - considered as having changes - * - Propagatable files differ from parent - user made code changes - * Returns false if: - * - Only non-propagatable files (tests, hidden) changed - no user code changes - * - * This is used to skip Keep/Replace dialog when only test files were updated - * (e.g., author updated tests via Update Course). - */ - private fun hasPropagatableChangesFromParent(ref: String, task: Task): Boolean { - val commitHash = storage.resolveRef(ref) - if (commitHash == null) { - LOG.info("hasPropagatableChangesFromParent: ref=$ref has no commit, assuming has changes") - return true - } - - val commit = storage.getCommit(commitHash) - if (commit == null) { - LOG.info("hasPropagatableChangesFromParent: commit=$commitHash not found, assuming has changes") - return true - } - - val parentHash = commit.parentHashes.firstOrNull() - if (parentHash == null) { - // First commit in the chain - considered as having changes - LOG.info("hasPropagatableChangesFromParent: ref=$ref is first commit, assuming has changes") - return true - } - - val parentCommit = storage.getCommit(parentHash) - if (parentCommit == null) { - LOG.info("hasPropagatableChangesFromParent: parent commit=$parentHash not found, assuming has changes") - return true - } - - try { - val currentSnapshot = storage.getSnapshot(ref).toContentMap() - val parentSnapshot = storage.getSnapshotByHash(parentCommit.snapshotHash)?.toContentMap() - if (parentSnapshot == null) { - LOG.info("hasPropagatableChangesFromParent: parent snapshot not found, assuming has changes") - return true - } - - // Compare only propagatable files - val (currentPropagatable, _) = currentSnapshot.split(task) - val (parentPropagatable, _) = parentSnapshot.split(task) - - val hasChanges = currentPropagatable != parentPropagatable - LOG.info("hasPropagatableChangesFromParent: ref=$ref, hasChanges=$hasChanges (current=${currentPropagatable.size} files, parent=${parentPropagatable.size} files)") - return hasChanges - } catch (e: Exception) { - LOG.warn("hasPropagatableChangesFromParent: error comparing snapshots, assuming has changes", e) - return true - } - } - - private fun FLTaskState.split(task: Task) = splitByKey { path -> - val taskFile = task.taskFiles[path] - // ALT-10961: Also filter out test files by path pattern, not just by taskFile properties. - // This handles the case where test files exist in externalState (submission) but not in task.taskFiles. - val taskTestDirs = task.testDirs - // Check if file is in a test directory - val isInTestDir = taskTestDirs.any { testDir -> path.startsWith("$testDir/") || path == testDir } - // Also check for common test file patterns (tests.py, test_*.py, *_test.py) - val fileName = path.substringAfterLast('/') - val isTestFileName = fileName == "tests.py" || fileName.startsWith("test_") || fileName.endsWith("_test.py") - val isTestFilePath = isInTestDir || isTestFileName - if (isTestFilePath) { - LOG.info("split: path='$path' excluded as test file") - return@splitByKey false - } - val result = taskFile?.shouldBePropagated() ?: true - if (!result) { - LOG.debug("split: path='$path' excluded, isVisible=${taskFile.isVisible}, isEditable=${taskFile.isEditable}") - } - result - } - - @TestOnly - override fun restoreState() { - if (storage.isDisposed) { - storage = createStorage(project) - } - } - - @TestOnly - override fun cleanUpState() { - storage.closeAndClean() - originalNonPropagatableFilesCache.clear() - originalTemplateFilesCache.clear() - } - - override fun syncCurrentTaskIndexFromStorage(lesson: FrameworkLesson): Boolean { - val head = storage.head - - if (head != null) { - // HEAD exists - find the task whose storageRef matches HEAD - val taskIndex = lesson.taskList.indexOfFirst { it.storageRef() == head } - if (taskIndex == -1) { - LOG.debug("syncCurrentTaskIndexFromStorage: HEAD=$head but no task found with matching storageRef") - // Still mark as synced to allow auto-save to work - isStorageSynced = true - return false - } - - if (lesson.currentTaskIndex != taskIndex) { - LOG.info("syncCurrentTaskIndexFromStorage: syncing currentTaskIndex from ${lesson.currentTaskIndex} to $taskIndex (HEAD=$head)") - lesson.currentTaskIndex = taskIndex - } - - isStorageSynced = true - return true - } - - // No HEAD means fresh storage - calculate current task from task statuses - // (first unsolved task, or last task if all are solved) - val firstUnsolvedIndex = lesson.taskList.indexOfFirst { it.status != CheckStatus.Solved } - val calculatedIndex = if (firstUnsolvedIndex != -1) { - firstUnsolvedIndex - } else if (lesson.taskList.isNotEmpty()) { - lesson.taskList.size - 1 // All solved, use last task - } else { - 0 // Empty lesson, use 0 - } - - LOG.info("syncCurrentTaskIndexFromStorage: no HEAD, calculated currentTaskIndex=$calculatedIndex from task statuses") - - if (lesson.currentTaskIndex != calculatedIndex) { - LOG.info("syncCurrentTaskIndexFromStorage: updating currentTaskIndex from ${lesson.currentTaskIndex} to $calculatedIndex") - lesson.currentTaskIndex = calculatedIndex - } - - // Set HEAD to the calculated task's ref so future syncs use storage - val calculatedTask = lesson.taskList.getOrNull(calculatedIndex) - if (calculatedTask != null) { - val ref = calculatedTask.storageRef() - storage.head = ref - LOG.info("syncCurrentTaskIndexFromStorage: initialized HEAD to $ref") - } - - isStorageSynced = true - return false - } - - companion object { - private val LOG: Logger = Logger.getInstance(FrameworkLessonManagerImpl::class.java) - - const val VERSION: Int = 3 - - @VisibleForTesting - fun constructStoragePath(project: Project): Path = - Paths.get(FileUtil.join(project.basePath!!, Project.DIRECTORY_STORE_FOLDER, "frameworkLessonHistory", "storage")) - - private fun createStorage(project: Project): FrameworkStorage { - val storageFilePath = constructStoragePath(project) - val storageExists = storageFilePath.toFile().exists() - LOG.debug("CREATE_STORAGE: path=$storageFilePath, exists=$storageExists") - - try { - val storage = FrameworkStorage(storageFilePath) - storage.migrate(VERSION) - LOG.debug("CREATE_STORAGE: success, version=${storage.version}") - return storage - } - catch (e: Exception) { - LOG.error("Failed to initialize storage at $storageFilePath, recreating", e) - try { - FrameworkStorage.deleteFiles(storageFilePath) - } - catch (deleteError: Exception) { - LOG.error("Failed to delete old storage files", deleteError) - } - resetAllTaskRecords(project) - return FrameworkStorage(storageFilePath, VERSION) - } - } - - private fun resetAllTaskRecords(project: Project) { - val course = StudyTaskManager.getInstance(project).course ?: return - for (lesson in course.lessons) { - if (lesson !is FrameworkLesson) continue - for (task in lesson.taskList) { - if (task.record != -1) { - LOG.info("Resetting task record for '${task.name}' from ${task.record} to -1") - task.record = -1 - } - } - YamlFormatSynchronizer.saveItem(lesson) - } - } - } -} + * - There's no parent (first commit) - considered as having changes + * - Propagatable files differ from parent - user made code changes + * Returns false if: + * - Only non-propagatable files (tests, hidden) changed - no user code changes + * + * This is used to skip Keep/Replace dialog when only test files were updated + * (e.g., author updated tests via Update Course). + */ + private fun hasPropagatableChangesFromParent(ref: String, task: Task): Boolean { + val commitHash = storage.resolveRef(ref) + if (commitHash == null) { + LOG.info("hasPropagatableChangesFromParent: ref=$ref has no commit, assuming has changes") + return true + } + + val commit = storage.getCommit(commitHash) + if (commit == null) { + LOG.info("hasPropagatableChangesFromParent: commit=$commitHash not found, assuming has changes") + return true + } + + val parentHash = commit.parentHashes.firstOrNull() + if (parentHash == null) { + // First commit in the chain - considered as having changes + LOG.info("hasPropagatableChangesFromParent: ref=$ref is first commit, assuming has changes") + return true + } + + val parentCommit = storage.getCommit(parentHash) + if (parentCommit == null) { + LOG.info("hasPropagatableChangesFromParent: parent commit=$parentHash not found, assuming has changes") + return true + } + + try { + val currentSnapshot = storage.getSnapshot(ref).toContentMap() + val parentSnapshot = storage.getSnapshotByHash(parentCommit.snapshotHash)?.toContentMap() + if (parentSnapshot == null) { + LOG.info("hasPropagatableChangesFromParent: parent snapshot not found, assuming has changes") + return true + } + + // Compare only propagatable files + val (currentPropagatable, _) = currentSnapshot.split(task) + val (parentPropagatable, _) = parentSnapshot.split(task) + + val hasChanges = currentPropagatable != parentPropagatable + LOG.info("hasPropagatableChangesFromParent: ref=$ref, hasChanges=$hasChanges (current=${currentPropagatable.size} files, parent=${parentPropagatable.size} files)") + return hasChanges + } catch (e: Exception) { + LOG.warn("hasPropagatableChangesFromParent: error comparing snapshots, assuming has changes", e) + return true + } + } + + private fun FLTaskState.split(task: Task) = splitByKey { path -> + val taskFile = task.taskFiles[path] + // ALT-10961: Also filter out test files by path pattern, not just by taskFile properties. + // This handles the case where test files exist in externalState (submission) but not in task.taskFiles. + val taskTestDirs = task.testDirs + // Check if file is in a test directory + val isInTestDir = taskTestDirs.any { testDir -> path.startsWith("$testDir/") || path == testDir } + // Also check for common test file patterns (tests.py, test_*.py, *_test.py) + val fileName = path.substringAfterLast('/') + val isTestFileName = fileName == "tests.py" || fileName.startsWith("test_") || fileName.endsWith("_test.py") + val isTestFilePath = isInTestDir || isTestFileName + if (isTestFilePath) { + LOG.info("split: path='$path' excluded as test file") + return@splitByKey false + } + val result = taskFile?.shouldBePropagated() ?: true + if (!result) { + LOG.debug("split: path='$path' excluded, isVisible=${taskFile.isVisible}, isEditable=${taskFile.isEditable}") + } + result + } + + @TestOnly + override fun restoreState() { + if (storage.isDisposed) { + storage = createStorage(project) + } + } + + @TestOnly + override fun cleanUpState() { + storage.closeAndClean() + originalNonPropagatableFilesCache.clear() + originalTemplateFilesCache.clear() + } + + override fun syncCurrentTaskIndexFromStorage(lesson: FrameworkLesson): Boolean { + val head = storage.head + + if (head != null) { + // HEAD exists - find the task whose storageRef matches HEAD + val taskIndex = lesson.taskList.indexOfFirst { it.storageRef() == head } + if (taskIndex == -1) { + LOG.debug("syncCurrentTaskIndexFromStorage: HEAD=$head but no task found with matching storageRef") + // Still mark as synced to allow auto-save to work + isStorageSynced = true + return false + } + + if (lesson.currentTaskIndex != taskIndex) { + LOG.info("syncCurrentTaskIndexFromStorage: syncing currentTaskIndex from ${lesson.currentTaskIndex} to $taskIndex (HEAD=$head)") + lesson.currentTaskIndex = taskIndex + } + + isStorageSynced = true + return true + } + + // No HEAD means fresh storage - calculate current task from task statuses + // (first unsolved task, or last task if all are solved) + val firstUnsolvedIndex = lesson.taskList.indexOfFirst { it.status != CheckStatus.Solved } + val calculatedIndex = if (firstUnsolvedIndex != -1) { + firstUnsolvedIndex + } else if (lesson.taskList.isNotEmpty()) { + lesson.taskList.size - 1 // All solved, use last task + } else { + 0 // Empty lesson, use 0 + } + + LOG.info("syncCurrentTaskIndexFromStorage: no HEAD, calculated currentTaskIndex=$calculatedIndex from task statuses") + + if (lesson.currentTaskIndex != calculatedIndex) { + LOG.info("syncCurrentTaskIndexFromStorage: updating currentTaskIndex from ${lesson.currentTaskIndex} to $calculatedIndex") + lesson.currentTaskIndex = calculatedIndex + } + + // Set HEAD to the calculated task's ref so future syncs use storage + val calculatedTask = lesson.taskList.getOrNull(calculatedIndex) + if (calculatedTask != null) { + val ref = calculatedTask.storageRef() + storage.head = ref + LOG.info("syncCurrentTaskIndexFromStorage: initialized HEAD to $ref") + } + + isStorageSynced = true + return false + } + + companion object { + private val LOG: Logger = Logger.getInstance(FrameworkLessonManagerImpl::class.java) + + const val VERSION: Int = 3 + + @VisibleForTesting + fun constructStoragePath(project: Project): Path = + Paths.get(FileUtil.join(project.basePath!!, Project.DIRECTORY_STORE_FOLDER, "frameworkLessonHistory", "storage")) + + private fun createStorage(project: Project): FrameworkStorage { + val storageFilePath = constructStoragePath(project) + val storageExists = storageFilePath.toFile().exists() + LOG.debug("CREATE_STORAGE: path=$storageFilePath, exists=$storageExists") + + try { + val storage = FrameworkStorage(storageFilePath) + storage.migrate(VERSION) + LOG.debug("CREATE_STORAGE: success, version=${storage.version}") + return storage + } + catch (e: Exception) { + LOG.error("Failed to initialize storage at $storageFilePath, recreating", e) + try { + FrameworkStorage.deleteFiles(storageFilePath) + } + catch (deleteError: Exception) { + LOG.error("Failed to delete old storage files", deleteError) + } + resetAllTaskRecords(project) + return FrameworkStorage(storageFilePath, VERSION) + } + } + + private fun resetAllTaskRecords(project: Project) { + val course = StudyTaskManager.getInstance(project).course ?: return + for (lesson in course.lessons) { + if (lesson !is FrameworkLesson) continue + for (task in lesson.taskList) { + if (task.record != -1) { + LOG.info("Resetting task record for '${task.name}' from ${task.record} to -1") + task.record = -1 + } + } + YamlFormatSynchronizer.saveItem(lesson) + } + } + } +} diff --git a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/actions/navigate/FrameworkLessonUserFilesPropagationTest.kt b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/actions/navigate/FrameworkLessonUserFilesPropagationTest.kt index f530a7809..8871fdf86 100644 --- a/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/actions/navigate/FrameworkLessonUserFilesPropagationTest.kt +++ b/intellij-plugin/hs-core/testSrc/org/hyperskill/academy/learning/actions/navigate/FrameworkLessonUserFilesPropagationTest.kt @@ -1,129 +1,129 @@ -package org.hyperskill.academy.learning.actions.navigate - -import com.intellij.openapi.application.runWriteAction -import com.intellij.openapi.fileEditor.FileDocumentManager -import org.hyperskill.academy.learning.courseFormat.CheckStatus -import org.hyperskill.academy.learning.courseFormat.InMemoryTextualContents -import org.hyperskill.academy.learning.courseGeneration.GeneratorUtils.createChildFile -import org.hyperskill.academy.learning.actions.NextTaskAction -import org.hyperskill.academy.learning.actions.PreviousTaskAction -import org.hyperskill.academy.learning.configurators.FakeGradleBasedLanguage -import org.hyperskill.academy.learning.courseFormat.Course -import org.hyperskill.academy.learning.courseFormat.hyperskill.HyperskillCourse -import org.hyperskill.academy.learning.findTask -import org.hyperskill.academy.learning.testAction -import org.hyperskill.academy.learning.courseFormat.FrameworkLesson -import org.hyperskill.academy.learning.framework.FrameworkLessonManager -import org.junit.Test -import org.junit.Assert.* - -class FrameworkLessonUserFilesPropagationTest : NavigationTestBase() { - - @Test - fun `test user-created files propagated and registered in next stage`() { - val course = createHyperskillProjectCourse() - val task1 = course.findTask("project", "stage1") - val task2 = course.findTask("project", "stage2") - - withVirtualFileListener(course) { - task1.openTaskFileInEditor("src/Task.kt") - task1.status = CheckStatus.Solved - - // Create user file in task1 - val taskDir = rootDir.findFileByRelativePath("project/task") ?: error("Task directory not found") - runWriteAction { - createChildFile(project, taskDir, "src/UserFile.kt", InMemoryTextualContents("class UserClass {}")) - } - - // Navigate to task2 - testAction(NextTaskAction.ACTION_ID) - - // 1. Verify user file is on disk - val userFile = taskDir.findFileByRelativePath("src/UserFile.kt") - assertNotNull("User file should be on disk after navigation to task2", userFile) - - // 2. Verify user file is registered in task2's taskFiles - val taskFile = task2.getTaskFile("src/UserFile.kt") - assertNotNull("User file should be registered in task2 taskFiles", taskFile) - assertTrue("User file should be marked as learner created", taskFile!!.isLearnerCreated) - } - } - - @Test - fun `test excluded files are NOT captured in snapshot`() { - val course = createHyperskillProjectCourse() - val task1 = course.findTask("project", "stage1") - - withVirtualFileListener(course) { - task1.openTaskFileInEditor("src/Task.kt") - task1.status = CheckStatus.Solved - - // Create excluded directory and file - val taskDir = rootDir.findFileByRelativePath("project/task") ?: error("Task directory not found") - runWriteAction { - val buildDir = taskDir.createChildDirectory(this, ".build") - buildDir.createChildData(this, "some_generated_file.txt") - } - - // Navigate to task2 (this triggers saveCurrentTaskSnapshot for task1) - testAction(NextTaskAction.ACTION_ID) - - // Verify that 'build/' is NOT in task1's snapshot in storage - val manager = FrameworkLessonManager.getInstance(project) - val task1State = manager.getTaskState(task1.lesson as FrameworkLesson, task1) - assertFalse("Excluded file should not be in task state", task1State.containsKey(".build/some_generated_file.txt")) - } - } - - @Test - fun `test auto-save persists deletion of all propagatable files to snapshot`() { - val course = createHyperskillProjectCourse() - val task1 = course.findTask("project", "stage1") - val task2 = course.findTask("project", "stage2") - - withVirtualFileListener(course) { - task1.openTaskFileInEditor("src/Task.kt") - - // Navigate forward to task2 to establish a snapshot baseline that contains src/Task.kt. - testAction(NextTaskAction.ACTION_ID) - - val taskDir = rootDir.findFileByRelativePath("project/task") ?: error("Task directory not found") - assertNotNull("Baseline: src/Task.kt should exist on disk in task2", taskDir.findFileByRelativePath("src/Task.kt")) - - // Delete the only propagatable file from disk, then trigger auto-save (Ctrl+S equivalent). - runWriteAction { - taskDir.findFileByRelativePath("src/Task.kt")?.delete(this) - } - FileDocumentManager.getInstance().saveAllDocuments() - - // Navigate BACK to task1. Backward navigation does not overwrite task2's snapshot, - // so any state left there by auto-save is preserved for inspection. - testAction(PreviousTaskAction.ACTION_ID) - - // With the bug, saveCurrentTaskSnapshot bailed out when propagatableFiles was empty, - // leaving src/Task.kt in task2's snapshot — which would resurrect the file on next visit. - val manager = FrameworkLessonManager.getInstance(project) - val task2State = manager.getTaskState(task2.lesson as FrameworkLesson, task2) - assertFalse( - "Snapshot must reflect deletion: src/Task.kt should not be in task2 state after auto-save", - task2State.containsKey("src/Task.kt") - ) - } - } - - private fun createHyperskillProjectCourse(): Course = courseWithFiles( - language = FakeGradleBasedLanguage, - courseProducer = ::HyperskillCourse - ) { - frameworkLesson("project", isTemplateBased = false) { - eduTask("stage1", stepId = 2001) { - taskFile("src/Task.kt", "// Stage 1 template") - taskFile("test/Tests1.kt", "fun tests1() {}") - } - eduTask("stage2", stepId = 2002) { - taskFile("src/Task.kt", "// Stage 2 template") - taskFile("test/Tests2.kt", "fun tests2() {}") - } - } - } -} +package org.hyperskill.academy.learning.actions.navigate + +import com.intellij.openapi.application.runWriteAction +import com.intellij.openapi.fileEditor.FileDocumentManager +import org.hyperskill.academy.learning.courseFormat.CheckStatus +import org.hyperskill.academy.learning.courseFormat.InMemoryTextualContents +import org.hyperskill.academy.learning.courseGeneration.GeneratorUtils.createChildFile +import org.hyperskill.academy.learning.actions.NextTaskAction +import org.hyperskill.academy.learning.actions.PreviousTaskAction +import org.hyperskill.academy.learning.configurators.FakeGradleBasedLanguage +import org.hyperskill.academy.learning.courseFormat.Course +import org.hyperskill.academy.learning.courseFormat.hyperskill.HyperskillCourse +import org.hyperskill.academy.learning.findTask +import org.hyperskill.academy.learning.testAction +import org.hyperskill.academy.learning.courseFormat.FrameworkLesson +import org.hyperskill.academy.learning.framework.FrameworkLessonManager +import org.junit.Test +import org.junit.Assert.* + +class FrameworkLessonUserFilesPropagationTest : NavigationTestBase() { + + @Test + fun `test user-created files propagated and registered in next stage`() { + val course = createHyperskillProjectCourse() + val task1 = course.findTask("project", "stage1") + val task2 = course.findTask("project", "stage2") + + withVirtualFileListener(course) { + task1.openTaskFileInEditor("src/Task.kt") + task1.status = CheckStatus.Solved + + // Create user file in task1 + val taskDir = rootDir.findFileByRelativePath("project/task") ?: error("Task directory not found") + runWriteAction { + createChildFile(project, taskDir, "src/UserFile.kt", InMemoryTextualContents("class UserClass {}")) + } + + // Navigate to task2 + testAction(NextTaskAction.ACTION_ID) + + // 1. Verify user file is on disk + val userFile = taskDir.findFileByRelativePath("src/UserFile.kt") + assertNotNull("User file should be on disk after navigation to task2", userFile) + + // 2. Verify user file is registered in task2's taskFiles + val taskFile = task2.getTaskFile("src/UserFile.kt") + assertNotNull("User file should be registered in task2 taskFiles", taskFile) + assertTrue("User file should be marked as learner created", taskFile!!.isLearnerCreated) + } + } + + @Test + fun `test excluded files are NOT captured in snapshot`() { + val course = createHyperskillProjectCourse() + val task1 = course.findTask("project", "stage1") + + withVirtualFileListener(course) { + task1.openTaskFileInEditor("src/Task.kt") + task1.status = CheckStatus.Solved + + // Create excluded directory and file + val taskDir = rootDir.findFileByRelativePath("project/task") ?: error("Task directory not found") + runWriteAction { + val buildDir = taskDir.createChildDirectory(this, ".build") + buildDir.createChildData(this, "some_generated_file.txt") + } + + // Navigate to task2 (this triggers saveCurrentTaskSnapshot for task1) + testAction(NextTaskAction.ACTION_ID) + + // Verify that 'build/' is NOT in task1's snapshot in storage + val manager = FrameworkLessonManager.getInstance(project) + val task1State = manager.getTaskState(task1.lesson as FrameworkLesson, task1) + assertFalse("Excluded file should not be in task state", task1State.containsKey(".build/some_generated_file.txt")) + } + } + + @Test + fun `test auto-save persists deletion of all propagatable files to snapshot`() { + val course = createHyperskillProjectCourse() + val task1 = course.findTask("project", "stage1") + val task2 = course.findTask("project", "stage2") + + withVirtualFileListener(course) { + task1.openTaskFileInEditor("src/Task.kt") + + // Navigate forward to task2 to establish a snapshot baseline that contains src/Task.kt. + testAction(NextTaskAction.ACTION_ID) + + val taskDir = rootDir.findFileByRelativePath("project/task") ?: error("Task directory not found") + assertNotNull("Baseline: src/Task.kt should exist on disk in task2", taskDir.findFileByRelativePath("src/Task.kt")) + + // Delete the only propagatable file from disk, then trigger auto-save (Ctrl+S equivalent). + runWriteAction { + taskDir.findFileByRelativePath("src/Task.kt")?.delete(this) + } + FileDocumentManager.getInstance().saveAllDocuments() + + // Navigate BACK to task1. Backward navigation does not overwrite task2's snapshot, + // so any state left there by auto-save is preserved for inspection. + testAction(PreviousTaskAction.ACTION_ID) + + // With the bug, saveCurrentTaskSnapshot bailed out when propagatableFiles was empty, + // leaving src/Task.kt in task2's snapshot — which would resurrect the file on next visit. + val manager = FrameworkLessonManager.getInstance(project) + val task2State = manager.getTaskState(task2.lesson as FrameworkLesson, task2) + assertFalse( + "Snapshot must reflect deletion: src/Task.kt should not be in task2 state after auto-save", + task2State.containsKey("src/Task.kt") + ) + } + } + + private fun createHyperskillProjectCourse(): Course = courseWithFiles( + language = FakeGradleBasedLanguage, + courseProducer = ::HyperskillCourse + ) { + frameworkLesson("project", isTemplateBased = false) { + eduTask("stage1", stepId = 2001) { + taskFile("src/Task.kt", "// Stage 1 template") + taskFile("test/Tests1.kt", "fun tests1() {}") + } + eduTask("stage2", stepId = 2002) { + taskFile("src/Task.kt", "// Stage 2 template") + taskFile("test/Tests2.kt", "fun tests2() {}") + } + } + } +}