Skip to content

Commit

Permalink
fix(api): books could disappear for users if read by others
Browse files Browse the repository at this point in the history
  • Loading branch information
gotson committed Jun 5, 2020
1 parent 8517613 commit 3d1f0e0
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,14 @@ class BookDtoDao(
.leftJoin(m).on(b.ID.eq(m.BOOK_ID))
.leftJoin(d).on(b.ID.eq(d.BOOK_ID))
.leftJoin(r).on(b.ID.eq(r.BOOK_ID))
.where(conditions)
.and(readProgressCondition(userId))
.where(conditions)
.fetchOne(0, Long::class.java)

val orderBy = pageable.sort.toOrderBy(sorts)

val dtos = selectBase()
val dtos = selectBase(userId)
.where(conditions)
.and(readProgressCondition(userId))
.orderBy(orderBy)
.limit(pageable.pageSize)
.offset(pageable.offset)
Expand All @@ -78,9 +77,8 @@ class BookDtoDao(
}

override fun findByIdOrNull(bookId: Long, userId: Long): BookDto? =
selectBase()
selectBase(userId)
.where(b.ID.eq(bookId))
.and(readProgressCondition(userId))
.fetchAndMap()
.firstOrNull()

Expand All @@ -99,17 +97,16 @@ class BookDtoDao(
val seriesId = record.get(0, Long::class.java)
val numberSort = record.get(1, Float::class.java)

return selectBase()
return selectBase(userId)
.where(b.SERIES_ID.eq(seriesId))
.and(readProgressCondition(userId))
.orderBy(d.NUMBER_SORT.let { if (next) it.asc() else it.desc() })
.seek(numberSort)
.limit(1)
.fetchAndMap()
.firstOrNull()
}

private fun selectBase() =
private fun selectBase(userId: Long) =
dsl.select(
*b.fields(),
*mediaFields,
Expand All @@ -119,6 +116,7 @@ class BookDtoDao(
.leftJoin(m).on(b.ID.eq(m.BOOK_ID))
.leftJoin(d).on(b.ID.eq(d.BOOK_ID))
.leftJoin(r).on(b.ID.eq(r.BOOK_ID))
.and(readProgressCondition(userId))

private fun ResultQuery<Record>.fetchAndMap() =
fetch()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,8 @@ class SeriesDtoDao(
}

override fun findByIdOrNull(seriesId: Long, userId: Long): SeriesDto? =
selectBase()
selectBase(userId)
.where(s.ID.eq(seriesId))
.and(readProgressCondition(userId))
.groupBy(*groupFields)
.fetchAndMap()
.firstOrNull()
Expand All @@ -89,17 +88,15 @@ class SeriesDtoDao(
.leftJoin(d).on(s.ID.eq(d.SERIES_ID))
.leftJoin(r).on(b.ID.eq(r.BOOK_ID))
.where(conditions)
.and(readProgressCondition(userId))
.groupBy(s.ID)
.having(having)
.fetch()
.size

val orderBy = pageable.sort.toOrderBy(sorts)

val dtos = selectBase()
val dtos = selectBase(userId)
.where(conditions)
.and(readProgressCondition(userId))
.groupBy(*groupFields)
.having(having)
.orderBy(orderBy)
Expand All @@ -114,7 +111,7 @@ class SeriesDtoDao(
)
}

private fun selectBase(): SelectOnConditionStep<Record> =
private fun selectBase(userId: Long): SelectOnConditionStep<Record> =
dsl.select(*groupFields)
.select(DSL.count(b.ID).`as`(BOOKS_COUNT))
.select(countUnread.`as`(BOOKS_UNREAD_COUNT))
Expand All @@ -124,6 +121,7 @@ class SeriesDtoDao(
.leftJoin(b).on(s.ID.eq(b.SERIES_ID))
.leftJoin(d).on(s.ID.eq(d.SERIES_ID))
.leftJoin(r).on(b.ID.eq(r.BOOK_ID))
.and(readProgressCondition(userId))

private fun readProgressCondition(userId: Long): Condition = r.USER_ID.eq(userId).or(r.USER_ID.isNull)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import org.gotson.komga.domain.persistence.SeriesRepository
import org.gotson.komga.domain.service.KomgaUserLifecycle
import org.gotson.komga.domain.service.LibraryLifecycle
import org.gotson.komga.domain.service.SeriesLifecycle
import org.gotson.komga.infrastructure.security.KomgaPrincipal
import org.hamcrest.core.IsNull
import org.junit.jupiter.api.AfterAll
import org.junit.jupiter.api.AfterEach
Expand All @@ -37,12 +38,15 @@ import org.springframework.boot.test.context.SpringBootTest
import org.springframework.http.HttpHeaders
import org.springframework.http.MediaType
import org.springframework.jdbc.core.JdbcTemplate
import org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user
import org.springframework.test.context.junit.jupiter.SpringExtension
import org.springframework.test.web.servlet.MockMvc
import org.springframework.test.web.servlet.MockMvcResultMatchersDsl
import org.springframework.test.web.servlet.delete
import org.springframework.test.web.servlet.get
import org.springframework.test.web.servlet.patch
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath
import java.time.LocalDate
import javax.sql.DataSource
import kotlin.random.Random
Expand Down Expand Up @@ -72,13 +76,16 @@ class BookControllerTest(
}

private var library = makeLibrary()
private lateinit var user2: KomgaUser
private lateinit var user3: KomgaUser

@BeforeAll
fun `setup library`() {
jdbcTemplate.execute("ALTER SEQUENCE hibernate_sequence RESTART WITH 1")

library = libraryRepository.insert(library) // id = 1
userRepository.save(KomgaUser("[email protected]", "", false)) // id = 2
user2 = userRepository.save(KomgaUser("[email protected]", "", false)) // id = 2
user3 = userRepository.save(KomgaUser("[email protected]", "", false)) // id = 3
}

@AfterAll
Expand Down Expand Up @@ -892,4 +899,51 @@ class BookControllerTest(
}
}
}

@Test
fun `given a user with read progress when getting books for the other user then books are returned correctly`() {
makeSeries(name = "series", libraryId = library.id).let { series ->
seriesLifecycle.createSeries(series).also { created ->
val books = listOf(makeBook("1.cbr", libraryId = library.id), makeBook("2.cbr", libraryId = library.id))
seriesLifecycle.addBooks(created, books)
}
}

val book = bookRepository.findAll().first()
mediaRepository.findById(book.id).let {
mediaRepository.update(it.copy(
status = Media.Status.READY,
pages = (1..10).map { BookPage("$it", "image/jpeg") }
))
}

val jsonString = """
{
"completed": true
}
""".trimIndent()

mockMvc.perform(MockMvcRequestBuilders
.patch("/api/v1/books/${book.id}/read-progress")
.with(user(KomgaPrincipal(user2)))
.contentType(MediaType.APPLICATION_JSON)
.content(jsonString)
)

mockMvc.perform(MockMvcRequestBuilders
.get("/api/v1/books")
.with(user(KomgaPrincipal(user2)))
.contentType(MediaType.APPLICATION_JSON)
).andExpect(
jsonPath("$.totalElements").value(2)
)

mockMvc.perform(MockMvcRequestBuilders
.get("/api/v1/books")
.with(user(KomgaPrincipal(user3)))
.contentType(MediaType.APPLICATION_JSON)
).andExpect(
jsonPath("$.totalElements").value(2)
)
}
}

0 comments on commit 3d1f0e0

Please sign in to comment.