Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/get profile by username #186

Merged
merged 20 commits into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
6a95b59
Expand the Users.sq to return a object that matches the Profile contract
MarinJuricev Oct 8, 2023
437682c
Connect the UserPersistence with the changes from the User.sq
MarinJuricev Oct 8, 2023
0d90fea
Add ProfileService
MarinJuricev Oct 8, 2023
b38fd78
Add profile routes with the dependency setup
MarinJuricev Oct 8, 2023
0ead863
Add test coverage for the profileRoute
MarinJuricev Oct 10, 2023
9a5bfdd
Add profilesRoutes to the new routes extension
MarinJuricev Oct 10, 2023
610ccfd
Migrate to the resource route definition
MarinJuricev Oct 11, 2023
7e66235
Rename the path extraction from parameters -> parameter since paramet…
MarinJuricev Oct 11, 2023
95e523b
Address lint issues
MarinJuricev Oct 11, 2023
3a14800
Rename misleading parameter name
MarinJuricev Oct 11, 2023
5bf96cc
Remove the ProfileService as it's just essentially a wrapper around U…
MarinJuricev Oct 19, 2023
56bcca7
Add MissingParameter in DomainErrors and resolve it in error.kt
MarinJuricev Oct 21, 2023
e820d09
Remove unnecessary GetProfile
MarinJuricev Oct 21, 2023
8c1113f
Use the parameter from the provided route receiver
MarinJuricev Oct 30, 2023
87e0341
Get rid of duplicate Profile
MarinJuricev Oct 31, 2023
6064c0b
Cleanup duplicate userRepo in Dependencies
MarinJuricev Oct 31, 2023
76bea8b
Remove the `parameter` implementation, we have access to the route ob…
MarinJuricev Oct 31, 2023
85eb393
Make the username optional in an attempt to trigger /api/profile
MarinJuricev Oct 31, 2023
64dbe4d
Add test that covers the missing username path
MarinJuricev Oct 31, 2023
665b4ef
Merge branch 'main' into feature/GET-profile-by-username
nomisRev Nov 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/main/kotlin/io/github/nomisrev/DomainError.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ data class IncorrectInput(val errors: NonEmptyList<InvalidField>) : ValidationEr
constructor(head: InvalidField) : this(nonEmptyListOf(head))
}

data class MissingParameter(val name: String) : ValidationError

sealed interface UserError : DomainError

data class UserNotFound(val property: String) : UserError
Expand Down
13 changes: 7 additions & 6 deletions src/main/kotlin/io/github/nomisrev/env/Dependencies.kt
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,12 @@ suspend fun ResourceScope.dependencies(env: Env): Dependencies {
}

return Dependencies(
userService,
jwtService,
articleService(slugGenerator, articleRepo, userRepo, tagPersistence, favouritePersistence),
checks,
tagPersistence,
userRepo
userService = userService,
jwtService = jwtService,
articleService =
articleService(slugGenerator, articleRepo, userRepo, tagPersistence, favouritePersistence),
healthCheck = checks,
tagPersistence = tagPersistence,
userPersistence = userRepo,
)
}
11 changes: 11 additions & 0 deletions src/main/kotlin/io/github/nomisrev/repo/UserPersistence.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import io.github.nomisrev.DomainError
import io.github.nomisrev.PasswordNotMatched
import io.github.nomisrev.UserNotFound
import io.github.nomisrev.UsernameAlreadyExists
import io.github.nomisrev.routes.Profile
import io.github.nomisrev.service.UserInfo
import io.github.nomisrev.sqldelight.FollowingQueries
import io.github.nomisrev.sqldelight.UsersQueries
Expand Down Expand Up @@ -35,6 +36,8 @@ interface UserPersistence {
/** Select a User by its username */
suspend fun select(username: String): Either<DomainError, UserInfo>

suspend fun selectProfile(username: String): Either<DomainError, Profile>

@Suppress("LongParameterList")
suspend fun update(
userId: UserId,
Expand Down Expand Up @@ -104,6 +107,14 @@ fun userPersistence(
ensureNotNull(userInfo) { UserNotFound("username=$username") }
}

override suspend fun selectProfile(username: String): Either<DomainError, Profile> = either {
val profileInfo = usersQueries.selectProfile(username, ::toProfile).executeAsOneOrNull()
ensureNotNull(profileInfo) { UserNotFound("username=$username") }
}

fun toProfile(username: String, bio: String, image: String, following: Long): Profile =
Profile(username, bio, image, following > 0)

override suspend fun update(
userId: UserId,
email: String?,
Expand Down
2 changes: 2 additions & 0 deletions src/main/kotlin/io/github/nomisrev/routes/error.kt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import io.github.nomisrev.IncorrectInput
import io.github.nomisrev.IncorrectJson
import io.github.nomisrev.JwtGeneration
import io.github.nomisrev.JwtInvalid
import io.github.nomisrev.MissingParameter
import io.github.nomisrev.PasswordNotMatched
import io.github.nomisrev.UserNotFound
import io.github.nomisrev.UsernameAlreadyExists
Expand Down Expand Up @@ -55,6 +56,7 @@ suspend fun PipelineContext<Unit, ApplicationCall>.respond(error: DomainError):
is JwtInvalid -> unprocessable(error.description)
is CannotGenerateSlug -> unprocessable(error.description)
is ArticleBySlugNotFound -> unprocessable("Article by slug ${error.slug} not found")
is MissingParameter -> unprocessable("Missing ${error.name} parameter in request")
}

private suspend inline fun PipelineContext<Unit, ApplicationCall>.unprocessable(
Expand Down
16 changes: 16 additions & 0 deletions src/main/kotlin/io/github/nomisrev/routes/profile.kt
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
@file:Suppress("MatchingDeclarationName")

package io.github.nomisrev.routes
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to add this for detekt, I'd prefer not to change the filename. Perhaps we should disable this rule in some other issue.

Suggested change
package io.github.nomisrev.routes
@file:Suppress("MatchingDeclarationName")
package io.github.nomisrev.routes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, having to force every routes implementation to add the @file:Suppress for detekt isn't deal, something for a future PR 😄

Addressed in 76bea8b


import arrow.core.raise.either
import arrow.core.raise.ensure
import io.github.nomisrev.MissingParameter
import io.github.nomisrev.auth.jwtAuth
import io.github.nomisrev.repo.UserPersistence
import io.github.nomisrev.service.JwtService
import io.ktor.http.HttpStatusCode
import io.ktor.resources.Resource
import io.ktor.server.resources.delete
import io.ktor.server.resources.get
import io.ktor.server.routing.Route
import kotlinx.serialization.Serializable

Expand All @@ -22,11 +27,22 @@ data class Profile(

@Resource("/profiles")
data class ProfilesResource(val parent: RootResource = RootResource) {
@Resource("/{username?}")
data class Username(val parent: ProfilesResource = ProfilesResource(), val username: String?)

@Resource("/{username}/follow")
data class Follow(val parent: ProfilesResource = ProfilesResource(), val username: String)
}

fun Route.profileRoutes(userPersistence: UserPersistence, jwtService: JwtService) {
get<ProfilesResource.Username> { route ->
either {
ensure(!route.username.isNullOrBlank()) { MissingParameter("username") }
userPersistence.selectProfile(route.username).bind()
}
.respond(HttpStatusCode.OK)
}

delete<ProfilesResource.Follow> { follow ->
jwtAuth(jwtService) { (_, userId) ->
either {
Expand Down
1 change: 0 additions & 1 deletion src/main/kotlin/io/github/nomisrev/routes/root.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ fun Application.routes(deps: Dependencies) = routing {
userRoutes(deps.userService, deps.jwtService)
tagRoutes(deps.tagPersistence)
profileRoutes(deps.userPersistence, deps.jwtService)
articleRoutes(deps.articleService, deps.jwtService)
}

@Resource("/api") data object RootResource
10 changes: 10 additions & 0 deletions src/main/sqldelight/io/github/nomisrev/sqldelight/Users.sq
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@ SELECT email, username, bio, image
FROM users
WHERE username = :username;

selectProfile:
SELECT users.username, users.bio, users.image,
CASE
WHEN following.follower_id IS NULL THEN 0
ELSE 1
END AS following
FROM users
LEFT JOIN following ON CAST(users.id AS BIGINT) = following.followed_id
WHERE users.username = :username;

selectSecurityByEmail:
SELECT id, username, salt, hashed_password, bio, image
FROM users
Expand Down
50 changes: 50 additions & 0 deletions src/test/kotlin/io/github/nomisrev/routes/ProfileRouteSpec.kt
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
package io.github.nomisrev.routes

import io.github.nomisrev.env.Dependencies
import io.github.nomisrev.service.RegisterUser
import io.github.nomisrev.withServer
import io.kotest.assertions.arrow.core.shouldBeRight
import io.kotest.core.spec.style.StringSpec
import io.kotest.matchers.shouldBe
import io.ktor.client.call.body
import io.ktor.client.plugins.resources.delete
import io.ktor.client.plugins.resources.get
import io.ktor.client.request.bearerAuth
import io.ktor.http.ContentType
import io.ktor.http.HttpStatusCode
import io.ktor.http.contentType

class ProfileRouteSpec :
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're missing a test case for when the path parameter is missing, I think that would've resulted MissingRequestParameterException case.

Copy link
Contributor Author

@MarinJuricev MarinJuricev Oct 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that the endpoint that we'd want to hit/test is api/profiles/ and assert that our new validation is triggered

However, I'm having difficulties actually hitting the above endpoint with the current Resource setup, I get 404 not found.

I've read through the docs, and tried with the ? approach, but with no luck.

@Resource("/{$USERNAME?}")
data class Username(val parent: ProfileResource = ProfileResource(), val username: String)

Do you have any suggestions?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange, I made a small change and it's working fine for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm probably missing something obvious, I've tried various things to trigger the api/profiles logic.

The username implementation is marked as optional and a placeholder test that fails on my side is added here

I've tried also to write it without the @Resource implementation so that I just hit /api/profiles, but with no luck 🤷

StringSpec({
Expand Down Expand Up @@ -66,4 +70,50 @@ class ProfileRouteSpec :
response.status shouldBe HttpStatusCode.UnprocessableEntity
}
}

"Get profile with no following" {
withServer { dependencies: Dependencies ->
dependencies.userService
.register(RegisterUser(validUsername, validEmail, validPw))
.shouldBeRight()
val response =
get(ProfilesResource.Username(username = validUsername)) {
contentType(ContentType.Application.Json)
}

response.status shouldBe HttpStatusCode.OK
with(response.body<Profile>()) {
username shouldBe validUsername
bio shouldBe ""
image shouldBe ""
following shouldBe false
}
}
}

"Get profile invalid username" {
withServer {
val response =
get(ProfilesResource.Username(username = validUsername)) {
contentType(ContentType.Application.Json)
}

response.status shouldBe HttpStatusCode.UnprocessableEntity
response.body<GenericErrorModel>().errors.body shouldBe
listOf("User with username=$validUsername not found")
}
}

"Get profile by username missing username" {
withServer {
val response =
get(ProfilesResource.Username(username = "")) {
contentType(ContentType.Application.Json)
}

response.status shouldBe HttpStatusCode.UnprocessableEntity
response.body<GenericErrorModel>().errors.body shouldBe
listOf("Missing username parameter in request")
}
}
})