README.md

@@ -151,9 +151,8 @@ this API.
 Concurrency is currently not handled by the API : if 2 users try to update the same player's points at the same time,
 the update could return inconsistent data. We could for example use a versioning on player entity to set it up.
 
-- change score computing
-- add checkstyle
-- ...
-- open api
-- Itest
-  regarder les contraintes
\ No newline at end of file
+We also have no pagination on finding players : this could be a good improvement in the case of dealing with large
+amount of players.
+
+Finally, one last improvement we could make is to handle transactions. This is permitted by AWS SDK but would require
+some refactoring inside the Repository layer.
\ No newline at end of file

src/main/kotlin/configuration/DynamoDbConfiguration.kt

 package betclic.test.configuration
 
+import io.ktor.server.config.*
 import org.slf4j.LoggerFactory
+import software.amazon.awssdk.auth.credentials.AwsBasicCredentials
+import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider
 import software.amazon.awssdk.enhanced.dynamodb.DynamoDbEnhancedAsyncClient
+import software.amazon.awssdk.regions.Region
 import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient
 import java.net.URI
-import software.amazon.awssdk.regions.Region
-import software.amazon.awssdk.auth.credentials.AwsBasicCredentials
-import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider
+
+private const val APPLICATION_YAML = "application.yaml"
 
 class DynamoDbConfiguration {
     private val logger = LoggerFactory.getLogger(DynamoDbConfiguration::class.java)
 
     fun createDynamoDbClient(): DynamoDbAsyncClient {
+        val url = ApplicationConfig(APPLICATION_YAML).property("ktor.database.dynamodbUrl").getString()
+        val accessKey = ApplicationConfig(APPLICATION_YAML).property("ktor.database.accessKey").getString()
+        val secretKey = ApplicationConfig(APPLICATION_YAML).property("ktor.database.secretKey").getString()
         return DynamoDbAsyncClient.builder()
-            .endpointOverride(URI("http://localhost:8000"))
+            .endpointOverride(URI(url))
             .region(Region.US_EAST_1)
             .credentialsProvider(
-                StaticCredentialsProvider.create(AwsBasicCredentials.create("dummy", "dummy"))
+                StaticCredentialsProvider.create(AwsBasicCredentials.create(accessKey, secretKey))
             )
             .build()
+
     }
 
     fun createDataSource(dynamoDbAsyncClient: DynamoDbAsyncClient): DynamoDbEnhancedAsyncClient {

src/main/kotlin/configuration/ExceptionConfiguration.kt

@@ -6,26 +6,40 @@ import io.ktor.server.application.*
 import io.ktor.server.plugins.*
 import io.ktor.server.plugins.statuspages.*
 import io.ktor.server.response.*
+import kotlinx.serialization.Serializable
 import software.amazon.awssdk.services.dynamodb.model.DynamoDbException
 
 private const val SOMETHING_WENT_WRONG = "Something went wrong"
+private const val RESOURCE_NOT_FOUND = "Resource not found"
+
+@Serializable
+data class ErrorResponse(
+    val message: String,
+    val statusCode: Int,
+)
 
 fun Application.configureExceptionHandling() {
     install(StatusPages) {
         exception<Throwable> { call, cause ->
-            when (cause) {
-                is NotFoundException -> call.respond(HttpStatusCode.NotFound, cause.message ?: SOMETHING_WENT_WRONG)
-                is DynamoDbException -> call.respond(
-                    HttpStatusCode.InternalServerError,
-                    cause.message ?: "$SOMETHING_WENT_WRONG with DynamoDb"
+            val (errorStatusCode, errorResponse) = when (cause) {
+                is NotFoundException -> HttpStatusCode.NotFound to ErrorResponse(
+                    message = cause.message ?: RESOURCE_NOT_FOUND,
+                    statusCode = HttpStatusCode.NotFound.value
                 )
 
-                is AlreadyExistingPlayerException -> call.respond(
-                    HttpStatusCode.BadRequest,
-                    cause.message ?: SOMETHING_WENT_WRONG
+                is DynamoDbException -> HttpStatusCode.ServiceUnavailable to ErrorResponse(
+                    message = cause.message ?: "$SOMETHING_WENT_WRONG with DynamoDb",
+                    statusCode = HttpStatusCode.ServiceUnavailable.value,
                 )
+
+                is AlreadyExistingPlayerException -> HttpStatusCode.BadRequest to ErrorResponse(
+                    message = cause.message ?: SOMETHING_WENT_WRONG,
+                    statusCode = HttpStatusCode.BadRequest.value,
+                )
+
+                else -> throw cause
             }
-            call.respondText(text = "500: $cause", status = HttpStatusCode.InternalServerError)
+            call.respond(errorStatusCode, errorResponse)
         }
     }
 }
\ No newline at end of file

src/main/kotlin/configuration/SecurityConfiguration.kt

-package betclic.test.configuration
-
-import io.ktor.server.application.*
-
-fun Application.configureSecurity() {
-    //TODO Add comment to say what I would do in term of security
-}

src/main/kotlin/player/exceptions/AlreadyExistingException.kt

 package betclic.test.player.exceptions
 
 class AlreadyExistingPlayerException(val pseudo: String) :
-    Exception("$pseudo already exists. You still can update this player points")
\ No newline at end of file
+    RuntimeException("$pseudo already exists. You still can update this player points")
\ No newline at end of file

src/main/kotlin/player/repositories/PlayerRepository.kt

@@ -4,15 +4,15 @@ import betclic.test.player.entities.Player
 
 interface PlayerRepository {
 
-    suspend fun createNewPlayer(player: Player): Player
+    suspend fun create(player: Player): Player
 
-    suspend fun updatePlayer(player: Player): Player
+    suspend fun update(player: Player): Player
 
-    suspend fun findPlayerByPseudo(pseudo: String): Player?
+    suspend fun findByPseudo(pseudo: String): Player?
 
     suspend fun getRank(player: Player): Int
 
     suspend fun findAll(): List<Player>
 
-    suspend fun deleteAllPlayers()
+    suspend fun deleteAll()
 }
\ No newline at end of file

src/main/kotlin/player/repositories/PlayerRepositoryImpl.kt

@@ -5,11 +5,8 @@ import betclic.test.player.entities.PlayerEntity
 import betclic.test.player.entities.toPlayer
 import betclic.test.player.entities.toPlayerEntity
 import dev.andrewohara.dynamokt.DataClassTableSchema
-import io.ktor.server.application.*
-import kotlinx.coroutines.coroutineScope
 import kotlinx.coroutines.future.await
 import kotlinx.coroutines.reactive.asFlow
-import org.slf4j.LoggerFactory
 import software.amazon.awssdk.enhanced.dynamodb.DynamoDbEnhancedAsyncClient
 import software.amazon.awssdk.enhanced.dynamodb.Expression
 import software.amazon.awssdk.enhanced.dynamodb.Key
@@ -20,20 +17,18 @@ class PlayerRepositoryImpl(dynamoDbEnhancedClient: DynamoDbEnhancedAsyncClient)
     private val tableName = PlayerEntity::class.simpleName
     private val tableSchema = DataClassTableSchema(PlayerEntity::class)
     private val table = dynamoDbEnhancedClient.table(tableName, tableSchema)
-    private val logger = LoggerFactory.getLogger(Application::class.java)
 
-    override suspend fun createNewPlayer(player: Player) = coroutineScope {
+    override suspend fun create(player: Player): Player {
         table.putItem(player.toPlayerEntity()).await()
-        logger.info("Successfully created new player $player")
-        return@coroutineScope player
+        return player
     }
 
-    override suspend fun updatePlayer(player: Player): Player {
+    override suspend fun update(player: Player): Player {
         val updatedPlayer = table.updateItem(player.toPlayerEntity()).await()
         return updatedPlayer.toPlayer()
     }
 
-    override suspend fun findPlayerByPseudo(pseudo: String): Player? {
+    override suspend fun findByPseudo(pseudo: String): Player? {
         val foundPlayer = table.getItem(
             Key.builder().partitionValue(pseudo).build()
         ).await()
@@ -61,7 +56,7 @@ class PlayerRepositoryImpl(dynamoDbEnhancedClient: DynamoDbEnhancedAsyncClient)
     }
 
 
-    override suspend fun deleteAllPlayers() {
+    override suspend fun deleteAll() {
         table.deleteTable()
         table.createTable().await()
 

src/main/kotlin/player/routes/PlayerRoute.kt

@@ -3,7 +3,6 @@ package betclic.test.player.routes
 import betclic.test.player.dtos.PlayerCreationDTO
 import betclic.test.player.dtos.PlayerInfoDTO
 import betclic.test.player.dtos.PlayerUpdateDTO
-import betclic.test.player.entities.Player
 import betclic.test.player.services.PlayerService
 import io.ktor.http.*
 import io.ktor.server.application.*
@@ -12,10 +11,15 @@ import io.ktor.server.response.*
 import io.ktor.server.routing.*
 import org.koin.ktor.ext.inject
 
+private const val PLAYERS = "/players"
+
 fun Routing.playerRoutes() {
     val playerService by inject<PlayerService>()
-    // const
-    route("/players") {
+    route(PLAYERS) {
+        get {
+            val pseudo = call.request.queryParameters["pseudo"] ?: return@get call.respond(HttpStatusCode.BadRequest)
+            call.respond<PlayerInfoDTO>(HttpStatusCode.OK, playerService.getPlayerInfoByPseudo(pseudo))
+        }
         post {
             val request = call.receive<PlayerCreationDTO>()
             call.respond(HttpStatusCode.Created, playerService.createNewPlayer(request))
@@ -26,23 +30,13 @@ fun Routing.playerRoutes() {
             call.respond(HttpStatusCode.OK, playerService.updatePlayer(request))
         }
 
-        get("/{pseudo}") {
-            val pseudo = call.parameters["pseudo"] ?: return@get call.respond(HttpStatusCode.BadRequest)
-            call.respond<Player>(playerService.findPlayerByPseudo(pseudo))
-        }
-
-        get("/{pseudo}/info") {
-            val pseudo = call.parameters["pseudo"] ?: return@get call.respond(HttpStatusCode.BadRequest)
-            call.respond<PlayerInfoDTO>(playerService.getPlayerInfoByPseudo(pseudo))
-        }
-
         get("/ranking") {
-            call.respond(playerService.getPlayersRanked())
+            call.respond(HttpStatusCode.OK, playerService.getPlayersRanked())
         }
 
         delete {
             playerService.deleteAllPlayers()
-            call.respond(HttpStatusCode.NoContent)
+            call.respond(HttpStatusCode.NoContent) // code de retours partout
         }
 
     }

src/main/kotlin/player/services/PlayerService.kt

@@ -10,7 +10,7 @@ interface PlayerService {
 
     suspend fun updatePlayer(playerUpdateDTO: PlayerUpdateDTO): Player
 
-    suspend fun findPlayerByPseudo(pseudo: String): Player
+    suspend fun getPlayerByPseudo(pseudo: String): Player
 
     suspend fun getPlayerInfoByPseudo(pseudo: String): PlayerInfoDTO
 

src/main/kotlin/player/services/PlayerServiceImpl.kt

@@ -8,29 +8,38 @@ import betclic.test.player.dtos.toPlayerInfoDTO
 import betclic.test.player.entities.Player
 import betclic.test.player.exceptions.AlreadyExistingPlayerException
 import betclic.test.player.repositories.PlayerRepository
+import io.ktor.server.application.*
 import io.ktor.server.plugins.*
+import org.slf4j.LoggerFactory
 
 class PlayerServiceImpl(private val playerRepository: PlayerRepository) : PlayerService {
+    private val logger = LoggerFactory.getLogger(Application::class.java)
 
     override suspend fun createNewPlayer(playerCreationDTO: PlayerCreationDTO): Player {
-        if (playerRepository.findPlayerByPseudo(playerCreationDTO.pseudo) != null) {
+        logger.info("Creating new player")
+        if (playerRepository.findByPseudo(playerCreationDTO.pseudo) != null) {
             throw AlreadyExistingPlayerException(playerCreationDTO.pseudo)
         }
-        return playerRepository.createNewPlayer(playerCreationDTO.toPlayer())
+        val created = playerRepository.create(playerCreationDTO.toPlayer())
+        logger.info("Player ${created.pseudo} successfully created")
+        return created
     }
 
     override suspend fun updatePlayer(playerUpdateDTO: PlayerUpdateDTO): Player {
-        findPlayerByPseudo(playerUpdateDTO.pseudo)
-        return playerRepository.updatePlayer(playerUpdateDTO.toPlayer())
+        logger.info("Updating player ${playerUpdateDTO.pseudo}")
+        getPlayerByPseudo(playerUpdateDTO.pseudo)
+        val updated = playerRepository.update(playerUpdateDTO.toPlayer())
+        logger.info("Player ${updated.pseudo} successfully updated")
+        return updated
     }
 
-    override suspend fun findPlayerByPseudo(pseudo: String): Player {
-        val player = playerRepository.findPlayerByPseudo(pseudo) ?: throw NotFoundException("Player $pseudo not found")
-        return player
+    override suspend fun getPlayerByPseudo(pseudo: String): Player {
+        return playerRepository.findByPseudo(pseudo) ?: throw NotFoundException("Player $pseudo not found")
     }
 
     override suspend fun getPlayerInfoByPseudo(pseudo: String): PlayerInfoDTO {
-        val player = findPlayerByPseudo(pseudo)
+        logger.info("Gathering infos for $pseudo")
+        val player = getPlayerByPseudo(pseudo)
         val rank = playerRepository.getRank(player)
         return player.toPlayerInfoDTO(rank)
     }
@@ -39,7 +48,8 @@ class PlayerServiceImpl(private val playerRepository: PlayerRepository) : Player
         val allPlayers = playerRepository.findAll()
         var currentRank = 1
         var previousPoints = 0
-        return allPlayers.sortedByDescending { it.pointsNumber }.mapIndexed { index, player ->
+        logger.info("Sorting all player by their rank")
+        val sorted = allPlayers.sortedByDescending { it.pointsNumber }.mapIndexed { index, player ->
             if (previousPoints != player.pointsNumber) {
                 currentRank = index + 1
             }
@@ -50,10 +60,13 @@ class PlayerServiceImpl(private val playerRepository: PlayerRepository) : Player
                 ranking = currentRank,
             )
         }
+        return sorted
 
     }
 
     override suspend fun deleteAllPlayers() {
-        playerRepository.deleteAllPlayers()
+        logger.info("Deleting all players from database")
+        playerRepository.deleteAll()
+        logger.info("Players successfully deleted")
     }
 }
\ No newline at end of file

src/main/resources/application.yaml

@@ -6,3 +6,5 @@ ktor:
     port: 8080
   database:
     dynamodbUrl: "http://localhost:8000"
+    accessKey: "dummy"
+    secretKey: "dummy"

src/test/kotlin/player/PlayerIntegrationTest.kt

@@ -32,7 +32,7 @@ class PlayerIntegrationTest : BaseIntegrationTest() {
         assertEquals(HttpStatusCode.Created, response.status)
 
         val playerRepository by inject<PlayerRepository>()
-        val player = playerRepository.findPlayerByPseudo(NAME_1)
+        val player = playerRepository.findByPseudo(NAME_1)
         assertThat(player).extracting("pseudo", "pointsNumber")
             .containsExactly(NAME_1, 0)
     }
@@ -40,13 +40,13 @@ class PlayerIntegrationTest : BaseIntegrationTest() {
     @Test
     fun `When calling player update, a player should be updated in DB`() = iTest {
         val playerRepository by inject<PlayerRepository>()
-        playerRepository.createNewPlayer(Player(pseudo = NAME_2, pointsNumber = 0))
+        playerRepository.create(Player(pseudo = NAME_2, pointsNumber = 0))
         val response = client.put("/players") {
             header(HttpHeaders.ContentType, ContentType.Application.Json)
             setBody(Json.encodeToString(PlayerUpdateDTO(pseudo = NAME_2, pointsNumber = 30)))
         }
         assertEquals(HttpStatusCode.OK, response.status)
-        val player = playerRepository.findPlayerByPseudo(NAME_2)
+        val player = playerRepository.findByPseudo(NAME_2)
         assertThat(player).extracting("pseudo", "pointsNumber")
             .containsExactly(NAME_2, 30)
     }

src/test/kotlin/player/PlayerServiceTest.kt

@@ -18,8 +18,6 @@ import org.junit.Assert.assertThrows
 import org.junit.Test
 
 class PlayerServiceTest {
-
-
     private val playerRepository: PlayerRepository = mockk()
 
     private val playerService: PlayerServiceImpl = PlayerServiceImpl(playerRepository)
@@ -31,15 +29,15 @@ class PlayerServiceTest {
 
     @Test
     fun `when creating player, should call the repository once`() {
-        coEvery { playerRepository.createNewPlayer(player1) } returns player1
-        coEvery { playerRepository.findPlayerByPseudo(john) } returns null
+        coEvery { playerRepository.create(player1) } returns player1
+        coEvery { playerRepository.findByPseudo(john) } returns null
         runBlocking { playerService.createNewPlayer(PlayerCreationDTO(pseudo = john)) }
-        coVerify(exactly = 1) { playerRepository.createNewPlayer(player1) }
+        coVerify(exactly = 1) { playerRepository.create(player1) }
     }
 
     @Test
     fun `when creating player that exists, should throw on existing player`() {
-        coEvery { playerRepository.findPlayerByPseudo(john) } returns player1
+        coEvery { playerRepository.findByPseudo(john) } returns player1
         assertThrows(AlreadyExistingPlayerException::class.java) {
             runBlocking { playerService.createNewPlayer(PlayerCreationDTO(pseudo = john)) }
         }
@@ -47,15 +45,15 @@ class PlayerServiceTest {
 
     @Test
     fun `when updating player, should call the repository once`() {
-        coEvery { playerRepository.findPlayerByPseudo(pseudo = john) } returns player1
-        coEvery { playerRepository.updatePlayer(playerToUpdate) } returns playerToUpdate
+        coEvery { playerRepository.findByPseudo(pseudo = john) } returns player1
+        coEvery { playerRepository.update(playerToUpdate) } returns playerToUpdate
         runBlocking { playerService.updatePlayer(PlayerUpdateDTO(pseudo = john, pointsNumber = 10)) }
-        coVerify(exactly = 1) { playerRepository.updatePlayer(playerToUpdate) }
+        coVerify(exactly = 1) { playerRepository.update(playerToUpdate) }
     }
 
     @Test
-    fun `when updating player that doesn't exist, should throw on not found player`() {
-        coEvery { playerRepository.findPlayerByPseudo(pseudo = john) } returns null
+    fun `when updating player that doesn't exist, should throw a not found player`() {
+        coEvery { playerRepository.findByPseudo(pseudo = john) } returns null
         assertThrows(NotFoundException::class.java) {
             runBlocking { playerService.updatePlayer(PlayerUpdateDTO(pseudo = john, pointsNumber = 10)) }
         }
@@ -63,22 +61,22 @@ class PlayerServiceTest {
 
     @Test
     fun `when finding user by pseudo, should call the repository once`() {
-        coEvery { playerRepository.findPlayerByPseudo(pseudo = john) } returns player1
-        runBlocking { playerService.findPlayerByPseudo(pseudo = john) }
-        coVerify(exactly = 1) { playerRepository.findPlayerByPseudo(john) }
+        coEvery { playerRepository.findByPseudo(pseudo = john) } returns player1
+        runBlocking { playerService.getPlayerByPseudo(pseudo = john) }
+        coVerify(exactly = 1) { playerRepository.findByPseudo(john) }
     }
 
     @Test
-    fun `when finding by pseudo, should throw on not found player`() {
-        coEvery { playerRepository.findPlayerByPseudo(pseudo = john) } returns null
+    fun `when finding by pseudo without match, should throw a not found player`() {
+        coEvery { playerRepository.findByPseudo(pseudo = john) } returns null
         assertThrows(NotFoundException::class.java) {
-            runBlocking { playerService.findPlayerByPseudo(pseudo = john) }
+            runBlocking { playerService.getPlayerByPseudo(pseudo = john) }
         }
     }
 
     @Test
     fun `when getting player info, should return player with his rank`() {
-        coEvery { playerRepository.findPlayerByPseudo(pseudo = john) } returns player1
+        coEvery { playerRepository.findByPseudo(pseudo = john) } returns player1
         coEvery { playerRepository.getRank(player1) } returns 2
         val result = runBlocking { playerService.getPlayerInfoByPseudo(pseudo = john) }
         assertThat(result.pseudo).isEqualTo(john)
@@ -86,8 +84,8 @@ class PlayerServiceTest {
     }
 
     @Test
-    fun `when getting player info on non-existing player, should throw on not found player`() {
-        coEvery { playerRepository.findPlayerByPseudo(pseudo = john) } returns null
+    fun `when getting player info on non-existing player, should throw a not found player exception`() {
+        coEvery { playerRepository.findByPseudo(pseudo = john) } returns null
         coEvery { playerRepository.getRank(player1) } returns 2
         assertThrows(NotFoundException::class.java) {
             runBlocking { playerService.getPlayerInfoByPseudo(pseudo = john) }
@@ -114,8 +112,8 @@ class PlayerServiceTest {
 
     @Test
     fun `when deleting all players, should call the repository once`() {
-        coEvery { playerRepository.deleteAllPlayers() } just runs
+        coEvery { playerRepository.deleteAll() } just runs
         runBlocking { playerService.deleteAllPlayers() }
-        coVerify(exactly = 1) { playerRepository.deleteAllPlayers() }
+        coVerify(exactly = 1) { playerRepository.deleteAll() }
     }
 }
\ No newline at end of file