chore(backend): Add more tests and improve response codes

backend/src/com/grohden/repotagger/Application.kt

@@ -1,8 +1,6 @@
 package com.grohden.repotagger
 
-import com.grohden.repotagger.api.repository
-import com.grohden.repotagger.api.session
-import com.grohden.repotagger.api.userTag
+import com.grohden.repotagger.api.*
 import com.grohden.repotagger.dao.DAOFacade
 import com.grohden.repotagger.dao.DAOFacadeDatabase
 import com.grohden.repotagger.github.api.GithubClient
@@ -203,6 +201,27 @@ fun Application.moduleWithDependencies(dao: DAOFacade) {
         exception<NoSessionException> {
             call.respond(HttpStatusCode.Unauthorized)
         }
+        exception<BadRequest> {
+            // Second call returns null string when message is null
+            if (it.message != null) {
+                call.respond(HttpStatusCode.BadRequest)
+            } else {
+                call.respond(HttpStatusCode.BadRequest) {
+                    it.message
+                }
+            }
+
+        }
+        exception<NotFound> {
+            // Second call returns null string when message is null
+            if (it.message != null) {
+                call.respond(HttpStatusCode.NotFound)
+            } else {
+                call.respond(HttpStatusCode.NotFound) {
+                    it.message
+                }
+            }
+        }
     }
 
     routing {

backend/src/com/grohden/repotagger/api/ApiUtils.kt

@@ -0,0 +1,47 @@
+package com.grohden.repotagger.api
+
+import io.ktor.application.ApplicationCall
+import io.ktor.request.ContentTransformationException
+import io.ktor.request.receive
+
+
+class BadRequest(
+    message: String
+) : RuntimeException(message)
+
+
+class NotFound(
+    message: String
+) : RuntimeException(message)
+
+/**
+ * Requires a parameter if it's
+ * not present throw [BadRequest]
+ */
+fun ApplicationCall.requireParam(key: String): String {
+    return parameters[key] ?: throw BadRequest("Arg $key is invalid")
+}
+
+/**
+ * Requires a parameter to be non null and int, otherwise
+ * throws [BadRequest]
+ */
+fun ApplicationCall.requireIntParam(key: String): Int {
+    return try {
+        requireParam(key).toInt()
+    } catch (error: NumberFormatException) {
+        throw BadRequest("Arg $key is invalid")
+    }
+}
+
+/**
+ * Requires a parameter to be non null and int, otherwise
+ * throws [BadRequest]
+ */
+suspend inline fun <reified T : Any> ApplicationCall.requireReceived(): T {
+    return try {
+        receive()
+    } catch (error: ContentTransformationException) {
+        throw BadRequest("Could not parse body contents")
+    }
+}

backend/src/com/grohden/repotagger/api/Repository.kt

@@ -96,12 +96,11 @@ fun Route.repository(
          */
         get("/{githubId}/tags") {
             val session = call.requireSession()
-            val repoGithubId = call.parameters["githubId"]!!.toInt()
+            val repoGithubId = call.requireIntParam("githubId")
             val taggerRepo = dao.findUserRepositoryByGithubId(
                 userGithubId = session.githubUserId,
                 repoGithubId = repoGithubId
             )
-            // TODO: return error for repo null.
             val tags = taggerRepo?.let {
                 dao.findUserTagsByRepository(
                     userGithubId = session.githubUserId,
@@ -118,7 +117,7 @@ fun Route.repository(
          * returns a list of [DetailedRepository]
          */
         get("/details/{githubId}") {
-            val repoGithubId = call.parameters["githubId"]!!.toInt()
+            val repoGithubId = call.requireIntParam("githubId")
             val session = call.requireSession()
             val taggerRepo = dao.findUserRepositoryByGithubId(
                 userGithubId = session.githubUserId,
@@ -175,22 +174,25 @@ fun Route.repository(
          */
         delete("{githubId}/remove-tag/{userTagId}") {
             val session = call.requireSession()
-            val repoGithubId = call.parameters["githubId"]!!.toInt()
-            val tagId = call.parameters["userTagId"]!!.toInt()
+            val repoGithubId = call.requireIntParam("githubId")
+            val tagId = call.requireIntParam("userTagId")
 
             val tag = dao.findUserTagById(
                 tagId = tagId,
                 userGithubId = session.githubUserId
-            )!!
+            ) ?: throw NotFound("tag not found")
             val repo = dao.findUserRepositoryByGithubId(
                 userGithubId = session.githubUserId,
                 repoGithubId = repoGithubId
-            )!!
+            ) ?: throw NotFound("repository not found")
 
             dao.removeUserTagFromRepository(
                 tagId = tag.tagId,
                 repoId = repo.repoId
             )
+
+            dao.removeIfOrphanTag(tag.tagId)
+
             call.respond(HttpStatusCode.OK)
         }
     }

backend/src/com/grohden/repotagger/api/UserTag.kt

@@ -7,7 +7,6 @@ import com.grohden.repotagger.github.api.GithubClient
 import com.grohden.repotagger.requireSession
 import io.ktor.application.call
 import io.ktor.http.HttpStatusCode
-import io.ktor.request.receiveOrNull
 import io.ktor.response.respond
 import io.ktor.routing.Route
 import io.ktor.routing.get
@@ -41,7 +40,7 @@ fun Route.userTag(
          * Returns OK and a list of [DetailedRepository]
          */
         get("/{tagId}/repositories") {
-            val tagId = call.parameters["tagId"]!!.toInt()
+            val tagId = call.requireIntParam("tagId")
             val session = call.requireSession()
             val repositories = dao.findRepositoriesByUserTag(
                 userGithubId = session.githubUserId,
@@ -52,16 +51,14 @@ fun Route.userTag(
         }
 
         /**
-         * FIXME: should be moved to repository
-         *
          * Registers a user tag on a repository
          *
          * Receives a [CreateTagInput]
          *
          * Returns OK response with the newly created [UserTagDTO]
          */
         post("/add") {
-            val input = call.receiveOrNull<CreateTagInput>()!!
+            val input = call.requireReceived<CreateTagInput>()
             val session = call.requireSession()
 
             var repo = dao.findUserRepositoryByGithubId(
@@ -73,7 +70,7 @@ fun Route.userTag(
                 // We avoid hitting the rate limit
                 // and also spamming a LOT of requests
                 // by saving the repo when it's created
-                // TODO: we will be outsync with github data
+                // TODO: we will be outsync with github data,
                 //  we need to put some "last time updated"
                 //  tolerance into the [SourceRepositoryTable]
                 val githubRepo = githubClient.repositoryById(
@@ -104,7 +101,7 @@ fun Route.userTag(
                 )
             }
 
-            call.respond(HttpStatusCode.OK, tag)
+            call.respond(HttpStatusCode.Created, tag)
         }
     }
 }

backend/src/com/grohden/repotagger/dao/DAOFacade.kt

@@ -86,6 +86,12 @@ interface DAOFacade {
      * Finds all tags that a user has created
      */
     fun findUserTags(userGithubId: Int): List<UserTagDTO>
+
+    /**
+     * If a tag is orphan (no refs to it from any repo) it
+     * is removed
+     */
+    fun removeIfOrphanTag(tagId: Int);
 }
 
 
@@ -248,6 +254,21 @@ class DAOFacadeDatabase(
             .toDTOList()
     }
 
+    override fun removeIfOrphanTag(tagId: Int) = transaction(db) {
+        val refsCount = (SourceRepositoryTable innerJoin SourceRepoUserTagTable)
+            .select {
+                SourceRepoUserTagTable.repository eq SourceRepositoryTable.id and
+                        (SourceRepoUserTagTable.tag eq tagId)
+            }
+            .let { SourceRepositoryDAO.wrapRows(it) }
+            .count()
+
+        if(refsCount == 0L) {
+            UserTagsTable.deleteWhere(limit = 1) {
+                UserTagsTable.id eq tagId
+            }
+        }
+    }
 
     @TestOnly
     fun dropAll() = transaction(db) {

backend/test/com/grohden/repotagger/RepositoryTest.kt

@@ -125,4 +125,42 @@ class RepositoryTest : BaseTest() {
             }
         }
     }
+
+    @Test
+    fun `should delete a orphan tag from db`() = testApp {
+        cookiesSession {
+            login()
+            val tag = createTag(
+                CreateTagInput(
+                    tagName = "dart",
+                    repoGithubId = defaultRepoId
+                )
+            ).let { gson.fromJson<UserTagDTO>(it.response.content!!) }
+
+            deleteRepoTag(defaultRepoId, tag.tagId).apply {
+                requestHandled shouldBe true
+                response.status() shouldBeEqualTo HttpStatusCode.OK
+                response.content shouldBe null
+            }
+
+
+            listAllTags().apply {
+                val tags = gson.fromJson<List<UserTagDTO>>(response.content!!)
+
+                tags.shouldBeEmpty()
+            }
+        }
+    }
+
+    @Test
+    fun `should respond error for a not found tag`() = testApp {
+        cookiesSession {
+            login()
+            deleteRepoTag(defaultRepoId, 42).apply {
+                requestHandled shouldBe true
+                response.status() shouldBeEqualTo HttpStatusCode.NotFound
+                response.content shouldBe null
+            }
+        }
+    }
 }

backend/test/com/grohden/repotagger/UserTagTest.kt

@@ -4,7 +4,7 @@ import com.grohden.repotagger.dao.CreateTagInput
 import com.grohden.repotagger.dao.tables.UserTagDTO
 import com.grohden.repotagger.extensions.fromJson
 import com.grohden.repotagger.utils.createTag
-import com.grohden.repotagger.utils.listAlTags
+import com.grohden.repotagger.utils.listAllTags
 import io.ktor.http.HttpStatusCode
 import io.ktor.server.testing.cookiesSession
 import org.amshove.kluent.shouldBe
@@ -36,14 +36,14 @@ class UserTagTest : BaseTest() {
                 )
             ).apply {
                 requestHandled shouldBe true
-                response.status() shouldBeEqualTo HttpStatusCode.OK
+                response.status() shouldBeEqualTo HttpStatusCode.Created
                 response.content.let {
                     gson.fromJson(it, UserTagDTO::class.java)
                 }.tagName shouldBeEqualTo "jojo"
             }
 
 
-            listAlTags().apply {
+            listAllTags().apply {
                 requestHandled shouldBe true
                 response.status() shouldBeEqualTo HttpStatusCode.OK
                 val tags = response.content!!.let {

backend/test/com/grohden/repotagger/utils/ApiUtils.kt

@@ -20,7 +20,7 @@ fun TestApplicationEngine.createTag(
 }
 
 
-fun TestApplicationEngine.listAlTags(): TestApplicationCall {
+fun TestApplicationEngine.listAllTags(): TestApplicationCall {
     return handleRequest(HttpMethod.Get, "/api/tag/list") {
         withContentType(ContentType.Application.Json)
     }