From b7d96d2ea4bd770500a6b5fed198f4262e554bbf Mon Sep 17 00:00:00 2001 From: Vladislav Kopnin Date: Sun, 27 Sep 2026 21:54:00 +0300 Subject: [PATCH] Refactor: reduce detekt findings (unused params, throws, long methods, config) --- config/detekt/detekt.yml | 13 ++ .../ru/ysu/lms/markdown/MarkdownRenderer.kt | 60 ++++---- .../ysu/lms/academic/lab/LabImportParser.kt | 140 ++++++++++-------- .../ru/ysu/lms/progress/ProgressController.kt | 5 +- .../ru/ysu/lms/progress/SubmissionService.kt | 64 +++++--- .../lms/progress/domain/GradePublication.kt | 6 +- .../ru/ysu/lms/progress/domain/LabGrade.kt | 6 +- .../ysu/lms/progress/SubmissionServiceTest.kt | 1 - .../lms/web/admin/ContentAdminController.kt | 2 +- .../kotlin/ru/ysu/lms/web/api/ApiClient.kt | 4 +- .../ru/ysu/lms/web/auth/AuthPageController.kt | 2 +- .../ru/ysu/lms/web/learn/LearnController.kt | 2 +- .../ysu/lms/web/student/StudentController.kt | 2 +- 13 files changed, 188 insertions(+), 119 deletions(-) diff --git a/config/detekt/detekt.yml b/config/detekt/detekt.yml index b1c9f9e..a00c5c0 100644 --- a/config/detekt/detekt.yml +++ b/config/detekt/detekt.yml @@ -37,6 +37,16 @@ style: active: true LoopWithTooManyJumpStatements: maxJumpCount: 2 + # MVC-контроллеры возвращают имя шаблона — это не «магическая константа». + FunctionOnlyReturningConstant: + ignoreAnnotated: + - 'GetMapping' + - 'PostMapping' + +performance: + # runApplication(*args) — идиоматичная точка входа Spring Boot. + SpreadOperator: + active: false naming: FunctionNaming: @@ -44,6 +54,9 @@ naming: - 'Test' MatchingDeclarationName: active: false + # Enum-значения хранятся в БД и API строками в lowerCamelCase. + EnumNaming: + active: false empty-blocks: EmptyCatchBlock: diff --git a/libs/markdown/src/main/kotlin/ru/ysu/lms/markdown/MarkdownRenderer.kt b/libs/markdown/src/main/kotlin/ru/ysu/lms/markdown/MarkdownRenderer.kt index fea3b8b..bcc3d7a 100644 --- a/libs/markdown/src/main/kotlin/ru/ysu/lms/markdown/MarkdownRenderer.kt +++ b/libs/markdown/src/main/kotlin/ru/ysu/lms/markdown/MarkdownRenderer.kt @@ -134,7 +134,35 @@ class MarkdownRenderer( private fun buildSanitizer(): PolicyFactory = HtmlPolicyBuilder() - .allowElements( + .allowElements(*ALLOWED_ELEMENTS) + .allowAttributes("href") + .onElements("a") + .allowAttributes("title") + .onElements("a", "img") + .allowAttributes("src", "alt") + .onElements("img") + .allowAttributes("src", "poster", "controls", "preload", "width", "height", "loop", "muted", "playsinline") + .onElements("video") + .allowAttributes("src", "type") + .onElements("source") + .allowAttributes("src", "kind", "label", "srclang", "default") + .onElements("track") + .allowAttributes("class") + .onElements(*CLASS_ATTR_ELEMENTS) + .allowAttributes("id") + .onElements("h1", "h2", "h3", "h4", "h5", "h6") + .allowAttributes("colspan", "rowspan") + .onElements("td", "th") + .allowUrlProtocols("http", "https", "mailto") + .requireRelNofollowOnLinks() + .toFactory() + + private companion object { + const val MAX_DEPTH = 5 + + /** Теги, разрешённые в отрендеренном HTML (allow-list санитайзера). */ + val ALLOWED_ELEMENTS = + arrayOf( "p", "br", "hr", @@ -177,20 +205,11 @@ class MarkdownRenderer( "source", "track", "audio", - ).allowAttributes("href") - .onElements("a") - .allowAttributes("title") - .onElements("a", "img") - .allowAttributes("src", "alt") - .onElements("img") - .allowAttributes("src", "poster", "controls", "preload", "width", "height", "loop", "muted", "playsinline") - .onElements("video") - .allowAttributes("src", "type") - .onElements("source") - .allowAttributes("src", "kind", "label", "srclang", "default") - .onElements("track") - .allowAttributes("class") - .onElements( + ) + + /** Элементы, которым разрешён атрибут class (подсветка кода, фигуры и т.п.). */ + val CLASS_ATTR_ELEMENTS = + arrayOf( "code", "pre", "div", @@ -204,16 +223,7 @@ class MarkdownRenderer( "figcaption", "video", "img", - ).allowAttributes("id") - .onElements("h1", "h2", "h3", "h4", "h5", "h6") - .allowAttributes("colspan", "rowspan") - .onElements("td", "th") - .allowUrlProtocols("http", "https", "mailto") - .requireRelNofollowOnLinks() - .toFactory() - - private companion object { - const val MAX_DEPTH = 5 + ) val MEDIA = Regex( pattern = "(?m)^[ \\t]*:::[ \\t]*(figure|video)[ \\t]+(\\S+)[ \\t]*(?:\\|[ \\t]*(.*?))?[ \\t]*$", diff --git a/services/academic-service/src/main/kotlin/ru/ysu/lms/academic/lab/LabImportParser.kt b/services/academic-service/src/main/kotlin/ru/ysu/lms/academic/lab/LabImportParser.kt index 45de22b..1bb3056 100644 --- a/services/academic-service/src/main/kotlin/ru/ysu/lms/academic/lab/LabImportParser.kt +++ b/services/academic-service/src/main/kotlin/ru/ysu/lms/academic/lab/LabImportParser.kt @@ -33,92 +33,112 @@ class LabImportParser { } private fun parseZip(bytes: ByteArray): LabImportBundle { + val entries = readEntries(bytes) + val meta = readMeta(entries) + val readmeMd = readReadme(entries) + + val variants = mutableListOf() + val assets = mutableMapOf() + for ((path, data) in entries) { + val normalized = path.replace('\\', '/') + when { + isInDir(normalized, "variants") -> + parseVariantFile(normalized, String(data, Charsets.UTF_8), meta.levels)?.let(variants::add) + isInDir(normalized, "assets") -> assets[normalized] = data + } + } + if (variants.isEmpty()) { + throw BadRequestException("В архиве не найдены варианты (папка variants/)") + } + return LabImportBundle(meta, readmeMd, variants, assets) + } + + private fun readEntries(bytes: ByteArray): Map { val entries = mutableMapOf() ZipInputStream(ByteArrayInputStream(bytes)).use { zis -> var entry = zis.nextEntry while (entry != null) { - if (!entry.isDirectory) { - entries[entry.name] = zis.readAllBytes() - } + if (!entry.isDirectory) entries[entry.name] = zis.readAllBytes() entry = zis.nextEntry } } + return entries + } + private fun readMeta(entries: Map): LabMeta { val metaEntry = entries.entries.firstOrNull { it.key.endsWith("lab.yaml") || it.key.endsWith("lab.yml") } ?: throw BadRequestException("В архиве не найден lab.yaml") - val meta = yaml.readValue(metaEntry.value, LabMeta::class.java) - - val readmeEntry = entries.entries.firstOrNull { it.key.endsWith("/README.md") || it.key == "README.md" } - val readmeMd = readmeEntry?.let { String(it.value, Charsets.UTF_8) } ?: "" - - val variants = mutableListOf() - val assets = mutableMapOf() - - for ((path, data) in entries) { - val p = path.replace('\\', '/') - when { - p.contains("/variants/") || p.startsWith("variants/") -> { - val content = String(data, Charsets.UTF_8) - val variant = parseVariantFile(p, content, meta.levels) - if (variant != null) variants.add(variant) - } - p.contains("/assets/") || p.startsWith("assets/") -> { - assets[p] = data - } - } - } - - if (variants.isEmpty()) { - throw BadRequestException("В архиве не найдены варианты (папка variants/)") - } - - return LabImportBundle(meta, readmeMd, variants, assets) + return yaml.readValue(metaEntry.value, LabMeta::class.java) } + private fun readReadme(entries: Map): String = + entries.entries + .firstOrNull { it.key.endsWith("/README.md") || it.key == "README.md" } + ?.let { String(it.value, Charsets.UTF_8) } + ?: "" + + private fun isInDir( + path: String, + dir: String, + ): Boolean = path.contains("/$dir/") || path.startsWith("$dir/") + private fun parseVariantFile( path: String, content: String, levels: List, ): VariantMeta? { - val levelMap = levels.associateBy { it.code } val lines = content.lines() - val firstSep = lines.indexOfFirst { it.trim() == "---" } - if (firstSep != 0) { - return VariantMeta( - level = "basic", - number = path.substringAfterLast("/").removeSuffix(".md"), - title = "", - statement = content.trim(), - ) - } - val secondSep = lines.withIndex().firstOrNull { it.index != firstSep && it.value.trim() == "---" }?.index ?: -1 - if (secondSep < 0) return null - val fmLines = lines.subList(1, secondSep) - val fm = mutableMapOf() - for (line in fmLines) { - val parts = line.split(":", limit = 2) - if (parts.size == 2) fm[parts[0].trim()] = unquote(parts[1].trim()) - } - val levelCode = normalizeLevel(fm["level"] ?: "basic") - if (levelMap[levelCode] == null) return null - val body = lines.drop(secondSep + 1).joinToString("\n").trim() + val frontMatter = + readFrontMatter(lines) + ?: return VariantMeta( + level = "basic", + number = path.substringAfterLast("/").removeSuffix(".md"), + title = "", + statement = content.trim(), + ) + val levelCode = normalizeLevel(frontMatter.values["level"] ?: "basic") + val number = frontMatter.values["number"]?.takeIf { it.isNotBlank() } + if (levels.none { it.code == levelCode } || number == null) return null + val body = lines.drop(frontMatter.bodyStart).joinToString("\n").trim() return VariantMeta( level = levelCode, - number = fm["number"]?.takeIf { it.isNotBlank() } ?: return null, - title = fm["title"] ?: "", + number = number, + title = frontMatter.values["title"] ?: "", statement = body.ifBlank { "—" }, ) } - private fun unquote(value: String): String = - if (value.length >= 2 && - ((value.first() == '"' && value.last() == '"') || (value.first() == '\'' && value.last() == '\'')) - ) { - value.substring(1, value.length - 1) - } else { - value + private data class FrontMatter( + val values: Map, + val bodyStart: Int, + ) + + /** Читает YAML-front-matter (между двумя `---`), иначе возвращает null. */ + private fun readFrontMatter(lines: List): FrontMatter? { + if (lines.firstOrNull()?.trim() != "---") return null + val end = + lines + .withIndex() + .drop(1) + .firstOrNull { it.value.trim() == "---" } + ?.index ?: return null + val values = mutableMapOf() + for (line in lines.subList(1, end)) { + val parts = line.split(":", limit = 2) + if (parts.size == 2) values[parts[0].trim()] = unquote(parts[1].trim()) } + return FrontMatter(values, end + 1) + } + + private fun unquote(value: String): String = if (isQuoted(value)) value.substring(1, value.length - 1) else value + + private fun isQuoted(value: String): Boolean { + if (value.length < 2) return false + val first = value.first() + val last = value.last() + return (first == '"' && last == '"') || (first == '\'' && last == '\'') + } private fun normalizeLevel(s: String): String = when (s.lowercase().trim()) { diff --git a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/ProgressController.kt b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/ProgressController.kt index 9895df4..cfc5e8f 100644 --- a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/ProgressController.kt +++ b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/ProgressController.kt @@ -26,7 +26,6 @@ class ProgressController( private val service: ProgressService, private val publications: GradePublicationService, private val submissions: SubmissionService, - private val academic: AcademicClient, ) { @GetMapping("/me/assignments") fun meAssignments( @@ -39,9 +38,7 @@ class ProgressController( } @GetMapping("/me/labs") - fun meLabs( - @AuthenticationPrincipal jwt: Jwt, - ): List> = listOf() + fun meLabs(): List> = listOf() @PostMapping("/levels") @PreAuthorize("hasAuthority('ROLE_ADMIN')") diff --git a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/SubmissionService.kt b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/SubmissionService.kt index c271d53..4bff03e 100644 --- a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/SubmissionService.kt +++ b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/SubmissionService.kt @@ -25,32 +25,54 @@ class SubmissionService( files: List, token: String, ): SubmissionDto { - if (files.isEmpty() || files.all { it.isEmpty }) { - throw BadRequestException("Приложите файлы или архив") - } - val lab = - runCatching { academic.getLabSummary(labId.toString(), token) }.getOrNull() - ?: throw NotFoundException("Лабораторная не найдена") - val deadline = lab.deadlineAt?.let { runCatching { Instant.parse(it) }.getOrNull() } - if (deadline != null && Instant.now().isAfter(deadline)) { - throw BadRequestException("Срок сдачи истёк") - } - val last = submissions.findFirstByStudentIdAndLabIdOrderBySeqDesc(studentId, labId) - val seq = (last?.seq ?: 0) + 1 - val payload = files.mapIndexed { i, f -> (f.originalFilename ?: "file$i") to f.bytes } - payload.forEach { (name, bytes) -> - if (looksLikeProjectFile(name, bytes)) { - throw BadRequestException( - "Файл «$name» похож на файл проекта (XML), а не на исходник. " + - "Приложите файл .c/.h или zip с проектом.", - ) - } - } + val payload = validatedPayload(files) + ensureDeadline(requireLab(labId, token)) + val seq = nextSeq(studentId, labId) val stored = storage.store(studentId, labId, seq, payload) val entity = submissions.save(Submission(studentId = studentId, labId = labId, seq = seq)) return toDto(entity, stored.files) } + private fun validatedPayload(files: List): List> { + if (files.isEmpty() || files.all { it.isEmpty }) { + throw BadRequestException("Приложите файлы или архив") + } + return files + .mapIndexed { i, f -> (f.originalFilename ?: "file$i") to f.bytes } + .onEach { (name, bytes) -> rejectProjectFile(name, bytes) } + } + + private fun rejectProjectFile( + name: String, + bytes: ByteArray, + ) { + if (looksLikeProjectFile(name, bytes)) { + throw BadRequestException( + "Файл «$name» похож на файл проекта (XML), а не на исходник. " + + "Приложите файл .c/.h или zip с проектом.", + ) + } + } + + private fun requireLab( + labId: UUID, + token: String, + ): LabInfo = + runCatching { academic.getLabSummary(labId.toString(), token) }.getOrNull() + ?: throw NotFoundException("Лабораторная не найдена") + + private fun ensureDeadline(lab: LabInfo) { + val deadline = lab.deadlineAt?.let { runCatching { Instant.parse(it) }.getOrNull() } + if (deadline != null && Instant.now().isAfter(deadline)) { + throw BadRequestException("Срок сдачи истёк") + } + } + + private fun nextSeq( + studentId: UUID, + labId: UUID, + ): Int = (submissions.findFirstByStudentIdAndLabIdOrderBySeqDesc(studentId, labId)?.seq ?: 0) + 1 + @Transactional(readOnly = true) fun mySubmissions( studentId: UUID, diff --git a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/domain/GradePublication.kt b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/domain/GradePublication.kt index 0d5b807..d1865be 100644 --- a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/domain/GradePublication.kt +++ b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/domain/GradePublication.kt @@ -21,4 +21,8 @@ class GradePublication( data class GradePublicationKey( val groupId: UUID = UUID(0, 0), val labId: UUID = UUID(0, 0), -) : Serializable +) : Serializable { + companion object { + private const val serialVersionUID: Long = 1L + } +} diff --git a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/domain/LabGrade.kt b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/domain/LabGrade.kt index db0b4d8..d993801 100644 --- a/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/domain/LabGrade.kt +++ b/services/progress-service/src/main/kotlin/ru/ysu/lms/progress/domain/LabGrade.kt @@ -23,4 +23,8 @@ class LabGrade( data class LabGradeKey( val studentId: UUID = UUID(0, 0), val labId: UUID = UUID(0, 0), -) : Serializable +) : Serializable { + companion object { + private const val serialVersionUID: Long = 1L + } +} diff --git a/services/progress-service/src/test/kotlin/ru/ysu/lms/progress/SubmissionServiceTest.kt b/services/progress-service/src/test/kotlin/ru/ysu/lms/progress/SubmissionServiceTest.kt index 0559901..1feadb5 100644 --- a/services/progress-service/src/test/kotlin/ru/ysu/lms/progress/SubmissionServiceTest.kt +++ b/services/progress-service/src/test/kotlin/ru/ysu/lms/progress/SubmissionServiceTest.kt @@ -10,7 +10,6 @@ import org.springframework.mock.web.MockMultipartFile import ru.ysu.lms.common.BadRequestException import ru.ysu.lms.progress.domain.Submission import ru.ysu.lms.progress.repo.SubmissionRepository -import java.nio.file.Files import java.nio.file.Path import java.util.UUID import java.util.zip.ZipInputStream diff --git a/web/src/main/kotlin/ru/ysu/lms/web/admin/ContentAdminController.kt b/web/src/main/kotlin/ru/ysu/lms/web/admin/ContentAdminController.kt index cff1028..75dd731 100644 --- a/web/src/main/kotlin/ru/ysu/lms/web/admin/ContentAdminController.kt +++ b/web/src/main/kotlin/ru/ysu/lms/web/admin/ContentAdminController.kt @@ -24,7 +24,7 @@ class ContentAdminController( private val api: ApiClient, ) { @GetMapping - fun index(session: HttpSession): String = "redirect:/learn" + fun index(): String = "redirect:/learn" @PostMapping("/import") fun import( diff --git a/web/src/main/kotlin/ru/ysu/lms/web/api/ApiClient.kt b/web/src/main/kotlin/ru/ysu/lms/web/api/ApiClient.kt index f50e759..a7ce4a2 100644 --- a/web/src/main/kotlin/ru/ysu/lms/web/api/ApiClient.kt +++ b/web/src/main/kotlin/ru/ysu/lms/web/api/ApiClient.kt @@ -228,7 +228,7 @@ class ApiClient( private fun callOrNull(block: () -> T?): T? = try { block() - } catch (ex: RestClientResponseException) { + } catch (ignore: RestClientResponseException) { null } @@ -239,7 +239,7 @@ class ApiClient( throw ex } catch (ex: RestClientResponseException) { throw ApiException(ex.statusCode.value(), detail(ex)) - } catch (ex: RestClientException) { + } catch (ignore: RestClientException) { throw ApiException(502, "Сервис недоступен. Проверьте, что gateway запущен.") } diff --git a/web/src/main/kotlin/ru/ysu/lms/web/auth/AuthPageController.kt b/web/src/main/kotlin/ru/ysu/lms/web/auth/AuthPageController.kt index 7a5ea14..6fdd110 100644 --- a/web/src/main/kotlin/ru/ysu/lms/web/auth/AuthPageController.kt +++ b/web/src/main/kotlin/ru/ysu/lms/web/auth/AuthPageController.kt @@ -55,7 +55,7 @@ class AuthPageController( } @GetMapping("/setup") - fun setupPage(model: Model): String { + fun setupPage(): String { if (api.adminExists()) { return "redirect:/login" } diff --git a/web/src/main/kotlin/ru/ysu/lms/web/learn/LearnController.kt b/web/src/main/kotlin/ru/ysu/lms/web/learn/LearnController.kt index 5a4fb9c..545f87d 100644 --- a/web/src/main/kotlin/ru/ysu/lms/web/learn/LearnController.kt +++ b/web/src/main/kotlin/ru/ysu/lms/web/learn/LearnController.kt @@ -110,7 +110,7 @@ class LearnController( val a: ContentArticleDto = try { api.contentArticle(token(session), chapter, section, article, all = admin) - } catch (ex: ApiException) { + } catch (ignore: ApiException) { return "redirect:/learn" } diff --git a/web/src/main/kotlin/ru/ysu/lms/web/student/StudentController.kt b/web/src/main/kotlin/ru/ysu/lms/web/student/StudentController.kt index b2c36a7..b97fc96 100644 --- a/web/src/main/kotlin/ru/ysu/lms/web/student/StudentController.kt +++ b/web/src/main/kotlin/ru/ysu/lms/web/student/StudentController.kt @@ -113,7 +113,7 @@ class StudentController( val submittedOrDefending = defense != null && (defense.status == "waiting" || defense.assignmentMd.isNotBlank()) model.addAttribute("canSubmit", canSubmit(lab.deadlineAt) && !submittedOrDefending) "student/lab" - } catch (ex: ApiException) { + } catch (ignore: ApiException) { "redirect:/student/labs" } }