From 1996071794dddc87c67d99a676f901c725870043 Mon Sep 17 00:00:00 2001 From: Gauthier Roebroeck Date: Wed, 10 Mar 2021 14:30:19 +0800 Subject: [PATCH] fix(api): some metadata fields would not unset if set to null --- .../validation/NullOrBlankOrISBN.kt | 21 +++++++ .../komga/interfaces/rest/BookController.kt | 4 +- .../rest/dto/BookMetadataUpdateDto.kt | 20 ++++--- .../interfaces/rest/BookControllerTest.kt | 55 ++++++++++++++++++- 4 files changed, 88 insertions(+), 12 deletions(-) create mode 100644 komga/src/main/kotlin/org/gotson/komga/infrastructure/validation/NullOrBlankOrISBN.kt diff --git a/komga/src/main/kotlin/org/gotson/komga/infrastructure/validation/NullOrBlankOrISBN.kt b/komga/src/main/kotlin/org/gotson/komga/infrastructure/validation/NullOrBlankOrISBN.kt new file mode 100644 index 000000000..9a19f0078 --- /dev/null +++ b/komga/src/main/kotlin/org/gotson/komga/infrastructure/validation/NullOrBlankOrISBN.kt @@ -0,0 +1,21 @@ +package org.gotson.komga.infrastructure.validation + +import org.hibernate.validator.constraints.CompositionType +import org.hibernate.validator.constraints.ConstraintComposition +import org.hibernate.validator.constraints.ISBN +import javax.validation.Constraint +import javax.validation.constraints.Null +import kotlin.reflect.KClass + +@ConstraintComposition(CompositionType.OR) +@Constraint(validatedBy = []) +@Null +@Blank +@ISBN +@Target(AnnotationTarget.VALUE_PARAMETER, AnnotationTarget.FIELD, AnnotationTarget.PROPERTY_GETTER) +@Retention(AnnotationRetention.RUNTIME) +annotation class NullOrBlankOrISBN( + val message: String = "Must be null or blank or valid ISBN-13", + val groups: Array> = [], + val payload: Array> = [] +) diff --git a/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/BookController.kt b/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/BookController.kt index 5b7a65d58..484e586a0 100644 --- a/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/BookController.kt +++ b/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/BookController.kt @@ -428,7 +428,7 @@ class BookController( existing.copy( title = title ?: existing.title, titleLock = titleLock ?: existing.titleLock, - summary = summary ?: existing.summary, + summary = if (isSet("summary")) summary ?: "" else existing.summary, summaryLock = summaryLock ?: existing.summaryLock, number = number ?: existing.number, numberLock = numberLock ?: existing.numberLock, @@ -444,7 +444,7 @@ class BookController( if (tags != null) tags!! else emptySet() } else existing.tags, tagsLock = tagsLock ?: existing.tagsLock, - isbn = isbn?.filter { it.isDigit() } ?: existing.isbn, + isbn = if (isSet("isbn")) isbn?.filter { it.isDigit() } ?: "" else existing.isbn, isbnLock = isbnLock ?: existing.isbnLock ) } diff --git a/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/dto/BookMetadataUpdateDto.kt b/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/dto/BookMetadataUpdateDto.kt index 36fd2c4ce..30f9d6a18 100644 --- a/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/dto/BookMetadataUpdateDto.kt +++ b/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/dto/BookMetadataUpdateDto.kt @@ -1,7 +1,7 @@ package org.gotson.komga.interfaces.rest.dto +import org.gotson.komga.infrastructure.validation.NullOrBlankOrISBN import org.gotson.komga.infrastructure.validation.NullOrNotBlank -import org.hibernate.validator.constraints.ISBN import java.time.LocalDate import javax.validation.Valid import javax.validation.constraints.NotBlank @@ -16,7 +16,10 @@ class BookMetadataUpdateDto { var titleLock: Boolean? = null - var summary: String? = null + var summary: String? + by Delegates.observable(null) { prop, _, _ -> + isSet[prop.name] = true + } var summaryLock: Boolean? = null @@ -30,7 +33,7 @@ class BookMetadataUpdateDto { var numberSortLock: Boolean? = null var releaseDate: LocalDate? - by Delegates.observable(null) { prop, _, _ -> + by Delegates.observable(null) { prop, _, _ -> isSet[prop.name] = true } @@ -38,21 +41,24 @@ class BookMetadataUpdateDto { @get:Valid var authors: List? - by Delegates.observable?>(null) { prop, _, _ -> + by Delegates.observable(null) { prop, _, _ -> isSet[prop.name] = true } var authorsLock: Boolean? = null var tags: Set? - by Delegates.observable?>(null) { prop, _, _ -> + by Delegates.observable(null) { prop, _, _ -> isSet[prop.name] = true } var tagsLock: Boolean? = null - @get:ISBN - var isbn: String? = null + @get:NullOrBlankOrISBN + var isbn: String? + by Delegates.observable(null) { prop, _, _ -> + isSet[prop.name] = true + } var isbnLock: Boolean? = null } diff --git a/komga/src/test/kotlin/org/gotson/komga/interfaces/rest/BookControllerTest.kt b/komga/src/test/kotlin/org/gotson/komga/interfaces/rest/BookControllerTest.kt index cffc92e67..92d168dd4 100644 --- a/komga/src/test/kotlin/org/gotson/komga/interfaces/rest/BookControllerTest.kt +++ b/komga/src/test/kotlin/org/gotson/komga/interfaces/rest/BookControllerTest.kt @@ -675,6 +675,47 @@ class BookControllerTest( } } + @Test + @WithMockCustomUser(roles = [ROLE_ADMIN]) + fun `given json with blank fields when updating metadata then fields with blanks are unset`() { + makeSeries(name = "series", libraryId = library.id).let { series -> + seriesLifecycle.createSeries(series).also { created -> + val books = listOf(makeBook("1.cbr", libraryId = library.id)) + seriesLifecycle.addBooks(created, books) + } + } + + val bookId = bookRepository.findAll().first().id + bookMetadataRepository.findById(bookId).let { metadata -> + val updated = metadata.copy( + summary = "summary", + isbn = "9781617290459", + ) + + bookMetadataRepository.update(updated) + } + + val jsonString = """ + { + "summary":"", + "isbn":"" + } + """.trimIndent() + + mockMvc.patch("/api/v1/books/$bookId/metadata") { + contentType = MediaType.APPLICATION_JSON + content = jsonString + }.andExpect { + status { isNoContent() } + } + + val updatedMetadata = bookMetadataRepository.findById(bookId) + with(updatedMetadata) { + assertThat(summary).isBlank + assertThat(isbn).isBlank + } + } + @Test @WithMockCustomUser(roles = [ROLE_ADMIN]) fun `given json with null fields when updating metadata then fields with null are unset`() { @@ -692,7 +733,9 @@ class BookControllerTest( val updated = metadata.copy( authors = metadata.authors.toMutableList().also { it.add(Author("Author", "role")) }, releaseDate = testDate, - tags = setOf("tag") + tags = setOf("tag"), + summary = "summary", + isbn = "9781617290459", ) bookMetadataRepository.update(updated) @@ -708,7 +751,9 @@ class BookControllerTest( { "authors":null, "releaseDate":null, - "tags":null + "tags":null, + "summary":null, + "isbn":null } """.trimIndent() @@ -724,6 +769,8 @@ class BookControllerTest( assertThat(authors).isEmpty() assertThat(releaseDate).isNull() assertThat(tags).isEmpty() + assertThat(summary).isBlank + assertThat(isbn).isBlank } } @@ -749,7 +796,8 @@ class BookControllerTest( numberLock = true, numberSort = 2F, numberSortLock = true, - title = "title" + title = "title", + isbn = "9781617290459", ) bookMetadataRepository.update(updated) @@ -775,6 +823,7 @@ class BookControllerTest( assertThat(number).isEqualTo("number") assertThat(numberSort).isEqualTo(2F) assertThat(title).isEqualTo("title") + assertThat(isbn).isEqualTo("9781617290459") } } }