Skip to content

Commit

Permalink
fix: prevent update or replacement of resources in the services if at…
Browse files Browse the repository at this point in the history
…tributes already used by other resources (#11)

* fix: prevent update or replacement of resources in the services if attributes already used by other resources

Signed-off-by: ablandel <[email protected]>

* chore: update changelog

Signed-off-by: ablandel <[email protected]>

---------

Signed-off-by: ablandel <[email protected]>
  • Loading branch information
ablandel committed Jun 15, 2024
1 parent c6352bb commit e8e2379
Show file tree
Hide file tree
Showing 8 changed files with 386 additions and 5 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ Add the changes here.

- Update of the dev tools configuration - removing of the broken bootRunDev `gradle` task
- Add `kaomojis` mappedBy association in the `Tag` table (to `Kaomoji` table)
- Add `@Transaction` annotation on `Tag` and `Kaomoji` services CREATE, DELETE and UPDATE/REPLACE methods

### Fix

- Fix N+1 request problem during `Kaomoji` entity fetch
- Prevent update or replacement of resources in the services if attributes already used by other resources

## [1.1.0] - 2024-06-07

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import github.ablandel.anotherkaomoji.entity.Kaomoji
import org.springframework.data.jpa.repository.Query
import org.springframework.data.repository.CrudRepository
import org.springframework.stereotype.Repository
import java.util.*

@Repository
interface KaomojiRepository : CrudRepository<Kaomoji, Long> {
Expand All @@ -15,4 +16,7 @@ interface KaomojiRepository : CrudRepository<Kaomoji, Long> {

@Query("SELECT k FROM kaomoji k WHERE LOWER(k.key) = LOWER(:key) OR k.emoticon = :emoticon")
fun findByKeyOrEmoticon(key: String, emoticon: String): List<Kaomoji>

fun findByKeyIgnoreCase(key: String): Optional<Kaomoji>
fun findByEmoticon(emoticon: String): Optional<Kaomoji>
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import github.ablandel.anotherkaomoji.exception.InconsistentParameterException
import github.ablandel.anotherkaomoji.exception.InvalidParameterException
import github.ablandel.anotherkaomoji.repository.KaomojiRepository
import org.springframework.stereotype.Service
import org.springframework.transaction.annotation.Transactional
import java.util.*

@Service
class KaomojiService(val kaomojiRepository: KaomojiRepository, val tagService: TagService) {
Expand Down Expand Up @@ -40,11 +42,13 @@ class KaomojiService(val kaomojiRepository: KaomojiRepository, val tagService: T
return KaomojiDto.toDomain(findKaomojiById(id))
}

@Transactional
fun deleteById(id: Long) {
findKaomojiById(id) // Used to check if the ID exists
kaomojiRepository.deleteById(id)
}

@Transactional
fun create(kaomojiDto: KaomojiDto): KaomojiDto {
if (kaomojiDto.key.isNullOrBlank()) {
throw InvalidParameterException("Invalid parameters: [`key`]")
Expand All @@ -61,7 +65,7 @@ class KaomojiService(val kaomojiRepository: KaomojiRepository, val tagService: T
kaomojiRepository.save(
KaomojiDto.fromDomain(
kaomojiDto,
tagService.prepare(kaomojiDto.tags.filter { it -> it.label != null }.map { it -> it.label!! })
tagService.prepare(kaomojiDto.tags.filter { it.label != null }.map { it.label!! })
.toList()
)
)
Expand All @@ -71,6 +75,27 @@ class KaomojiService(val kaomojiRepository: KaomojiRepository, val tagService: T
}
}

private fun checkIfUniqueOrThrow(kaomojiDto: KaomojiDto, kaomoji: Kaomoji) {
if (kaomojiDto.key != null && kaomojiDto.emoticon != null) {
val otherKaomoji = kaomojiRepository.findByKeyOrEmoticon(kaomojiDto.key, kaomojiDto.emoticon)
if ((otherKaomoji.isNotEmpty() && kaomoji.id != otherKaomoji[0].id) || otherKaomoji.size > 1) {
throw DataAlreadyExistException("Kaomoji cannot be created with key `${kaomojiDto.key}` and emoticon `${kaomojiDto.emoticon}` - key or emoticon already used for another kaomoji(s)")
}
} else {
var otherKaomoji: Optional<Kaomoji> = Optional.empty()
if (kaomojiDto.key != null) {
otherKaomoji = kaomojiRepository.findByKeyIgnoreCase(kaomojiDto.key)
}
if (otherKaomoji.isEmpty && kaomojiDto.emoticon != null) {
otherKaomoji = kaomojiRepository.findByEmoticon(kaomojiDto.emoticon)
}
if (otherKaomoji.isPresent && kaomoji.id != otherKaomoji.get().id) {
throw DataAlreadyExistException("Kaomoji cannot be created with key `${kaomojiDto.key}` and emoticon `${kaomojiDto.emoticon}` - key or emoticon already used for another kaomoji(s)")
}
}
}

@Transactional
private fun replaceOrUpdate(
id: Long,
kaomojiDto: KaomojiDto,
Expand All @@ -85,9 +110,11 @@ class KaomojiService(val kaomojiRepository: KaomojiRepository, val tagService: T
if (kaomojiDto.emoticon != null && kaomojiDto.emoticon.isBlank()) {
throw InvalidParameterException("Invalid parameters: [`emoticon`]")
}
// Before replacing or updating, check if the key or the emoticon are already used by another kaomoji.
val kaomoji = findKaomojiById(id)
val tags = if (kaomojiDto.tags != null) tagService.prepare(kaomojiDto.tags.filter { it -> it.label != null }
.map { it -> it.label!! }).toList()
checkIfUniqueOrThrow(kaomojiDto, kaomoji)
val tags = if (kaomojiDto.tags != null) tagService.prepare(kaomojiDto.tags.filter { it.label != null }
.map { it.label!! }).toList()
else kaomoji.tags
return KaomojiDto.toDomain(
kaomojiRepository.save(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import github.ablandel.anotherkaomoji.exception.InconsistentParameterException
import github.ablandel.anotherkaomoji.exception.InvalidParameterException
import github.ablandel.anotherkaomoji.repository.TagRepository
import org.springframework.stereotype.Service
import org.springframework.transaction.annotation.Transactional

@Service
class TagService(val tagRepository: TagRepository) {
Expand Down Expand Up @@ -40,11 +41,13 @@ class TagService(val tagRepository: TagRepository) {
return TagDto.toDomain(tag)
}

@Transactional
fun deleteById(id: Long) {
findTagById(id) // Used to check if the ID exists
tagRepository.deleteById(id)
}

@Transactional
fun create(tagDto: TagDto): TagDto {
if (tagDto.label.isNullOrBlank()) {
throw InvalidParameterException("Invalid parameters: [`label`]")
Expand Down Expand Up @@ -72,6 +75,16 @@ class TagService(val tagRepository: TagRepository) {
return tags
}

private fun checkIfUniqueOrThrow(tagDto: TagDto, tag: Tag) {
if (tagDto.label != null) {
val otherTag = tagRepository.findByLabelIgnoreCase(tagDto.label)
if (otherTag.isPresent && tag.id != otherTag.get().id) {
throw DataAlreadyExistException("label", tagDto.label)
}
}
}

@Transactional
private fun replaceOrUpdate(
id: Long,
tagDto: TagDto,
Expand All @@ -83,7 +96,10 @@ class TagService(val tagRepository: TagRepository) {
if (tagDto.label != null && tagDto.label.isBlank()) {
throw InvalidParameterException("Invalid parameters: [`label`]")
}
return TagDto.toDomain(tagRepository.save(mapping(findTagById(id), tagDto)))
// Before replacing or updating, check if the label is already used by another tag.
val tag = findTagById(id)
checkIfUniqueOrThrow(tagDto, tag)
return TagDto.toDomain(tagRepository.save(mapping(tag, tagDto)))
}

fun replace(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,54 @@ class KaomojiRepositoryTest {
assertTrue(kaomojis.all { kaomoji -> kaomoji.updatedAt != null })
assertTrue(kaomojis.map { kaomoji: Kaomoji -> kaomoji.key }.toList().containsAll(listOf("key2", "key3")))
}

@Test
@Transactional
@Rollback
fun `test findByLabelIgnoreCase`() {
kaomojiRepository.save(
Kaomoji(
key = "key1",
emoticon = "emoticon1",
tags = listOf()
)
)
val kaomoji = kaomojiRepository.findByKeyIgnoreCase("key1")
assertTrue(kaomoji.isPresent)
assertTrue(kaomoji.get().id != null)
assertTrue(kaomoji.get().createdAt != null)
assertTrue(kaomoji.get().updatedAt != null)
assertTrue(kaomoji.get().key == "key1")
assertTrue(kaomoji.get().emoticon == "emoticon1")
assertTrue(kaomoji.get().tags.isEmpty())
val kaomojiOtherCase = kaomojiRepository.findByKeyIgnoreCase("Key1")
assertTrue(kaomojiOtherCase.isPresent)
assertTrue(kaomojiOtherCase.get().id != null)
assertTrue(kaomojiOtherCase.get().createdAt != null)
assertTrue(kaomojiOtherCase.get().updatedAt != null)
assertTrue(kaomojiOtherCase.get().key == "key1")
assertTrue(kaomojiOtherCase.get().emoticon == "emoticon1")
assertTrue(kaomojiOtherCase.get().tags.isEmpty())
}

@Test
@Transactional
@Rollback
fun `test findByEmoticon`() {
kaomojiRepository.save(
Kaomoji(
key = "key1",
emoticon = "emoticon1",
tags = listOf()
)
)
val kaomoji = kaomojiRepository.findByEmoticon("emoticon1")
assertTrue(kaomoji.isPresent)
assertTrue(kaomoji.get().id != null)
assertTrue(kaomoji.get().createdAt != null)
assertTrue(kaomoji.get().updatedAt != null)
assertTrue(kaomoji.get().key == "key1")
assertTrue(kaomoji.get().emoticon == "emoticon1")
assertTrue(kaomoji.get().tags.isEmpty())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ class TagRepositoryTest {
tagRepository.save(Tag(label = "label1"))
val tag = tagRepository.findByLabelIgnoreCase("label1")
assertTrue(tag.isPresent)
assertTrue(tag.get().id != null)
assertTrue(tag.get().createdAt != null)
assertTrue(tag.get().updatedAt != null)
assertTrue(tag.get().label == "label1")
val tagOtherCase = tagRepository.findByLabelIgnoreCase("Label1")
assertTrue(tagOtherCase.isPresent)
Expand Down
Loading

0 comments on commit e8e2379

Please sign in to comment.