fix: address PR review comments for OAuth2 implementation #32

Merged
skettiks merged 25 commits from feat/oauth2-user-service-26 into develop 2026-05-16 19:30:55 +00:00
7 changed files with 11 additions and 63 deletions
Showing only changes of commit a4f99bbe8a - Show all commits
-5
View File
3
@@ -31,7 +31,6 @@ java {
dependencies { dependencies {
devitq commented 2026-05-15 19:38:51 +00:00 (Migrated from github.com)
Review

do not remove any deps, this removal does not make any sense to your PR

do not remove any deps, this removal does not make any sense to your PR
implementation(platform(libs.sentry.bom)) implementation(platform(libs.sentry.bom))
implementation(platform(libs.spring.grpc.bom))
implementation(libs.spring.boot.starter.web) implementation(libs.spring.boot.starter.web)
implementation(libs.spring.boot.starter.actuator) implementation(libs.spring.boot.starter.actuator)
@@ -47,9 +46,6 @@ dependencies {
implementation(libs.opentelemetry.exporter.otlp) implementation(libs.opentelemetry.exporter.otlp)
implementation(libs.sentry.spring.boot.starter) implementation(libs.sentry.spring.boot.starter)
implementation(libs.spring.grpc.starter)
implementation(libs.grpc.services)
implementation(libs.spring.boot.starter.oauth2.client) implementation(libs.spring.boot.starter.oauth2.client)
runtimeOnly(libs.micrometer.registry.prometheus) runtimeOnly(libs.micrometer.registry.prometheus)
@@ -60,7 +56,6 @@ dependencies {
testImplementation(libs.spring.boot.starter.test) testImplementation(libs.spring.boot.starter.test)
testImplementation(libs.kotlin.test.junit5) testImplementation(libs.kotlin.test.junit5)
testImplementation(libs.spring.grpc.test)
testImplementation(libs.mockk) testImplementation(libs.mockk)
testRuntimeOnly(libs.junit.platform.launcher) testRuntimeOnly(libs.junit.platform.launcher)
devitq commented 2026-05-15 19:38:46 +00:00 (Migrated from github.com)
Review

do not remove any deps, this removal does not make any sense to your PR

do not remove any deps, this removal does not make any sense to your PR
} }
4
@@ -9,7 +9,6 @@ data class UserEntity(
val id: UUID, val id: UUID,
val name: String, val name: String,
val email: String, val email: String,
val password: String?,
val provider: String?, val provider: String?,
val providerId: String?, val providerId: String?,
val createdAt: LocalDateTime, val createdAt: LocalDateTime,
@@ -32,7 +31,6 @@ fun User.toEntity(
id = id, id = id,
name = name, name = name,
email = email, email = email,
password = password,
provider = provider?.name, provider = provider?.name,
providerId = providerId, providerId = providerId,
createdAt = createdAt, createdAt = createdAt,
1
@@ -20,7 +20,6 @@ class UserRepository(
id = UUID.fromString(rs.getString("id")), id = UUID.fromString(rs.getString("id")),
name = rs.getString("name"), name = rs.getString("name"),
email = rs.getString("email"), email = rs.getString("email"),
password = rs.getString("password"),
provider = rs.getString("provider"), provider = rs.getString("provider"),
providerId = rs.getString("provider_id"), providerId = rs.getString("provider_id"),
createdAt = rs.getTimestamp("created_at").toLocalDateTime(), createdAt = rs.getTimestamp("created_at").toLocalDateTime(),
2
@@ -46,12 +45,11 @@ class UserRepository(
jdbc.update( jdbc.update(
""" """
UPDATE users UPDATE users
SET name = ?, email = ?, password = ?, provider = ?, provider_id = ? SET name = ?, email = ?, provider = ?, provider_id = ?
WHERE id = ? WHERE id = ?
""".trimIndent(), """.trimIndent(),
entity.name, entity.name,
entity.email, entity.email,
user.password,
entity.provider, entity.provider,
entity.providerId, entity.providerId,
entity.id, entity.id,
@@ -60,13 +58,12 @@ class UserRepository(
if (updatedRows == 0) { if (updatedRows == 0) {
jdbc.update( jdbc.update(
""" """
INSERT INTO users (id, name, email, password, provider, provider_id, created_at) INSERT INTO users (id, name, email, provider, provider_id, created_at)
VALUES (?, ?, ?, ?, ?, ?, ?) VALUES (?, ?, ?, ?, ?, ?)
""".trimIndent(), """.trimIndent(),
entity.id, entity.id,
entity.name, entity.name,
entity.email, entity.email,
user.password,
entity.provider, entity.provider,
entity.providerId, entity.providerId,
entity.createdAt, entity.createdAt,
@@ -78,7 +75,7 @@ class UserRepository(
override fun findById(id: UUID): User? { override fun findById(id: UUID): User? {
val entities = val entities =
jdbc.query( jdbc.query(
"SELECT id, name, email, password, provider, provider_id, created_at FROM users WHERE id = ?", "SELECT id, name, email, provider, provider_id, created_at FROM users WHERE id = ?",
userEntityRowMapper, userEntityRowMapper,
id, id,
) )
@@ -88,7 +85,7 @@ class UserRepository(
override fun findByEmail(email: String): User? { override fun findByEmail(email: String): User? {
val entities = val entities =
jdbc.query( jdbc.query(
"SELECT id, name, email, password, provider, provider_id, created_at FROM users WHERE email = ?", "SELECT id, name, email, provider, provider_id, created_at FROM users WHERE email = ?",
userEntityRowMapper, userEntityRowMapper,
email, email,
) )
@@ -98,7 +95,7 @@ class UserRepository(
override fun findAll(): List<User> = override fun findAll(): List<User> =
jdbc jdbc
.query( .query(
"SELECT id, name, email, password, provider, provider_id, created_at FROM users", "SELECT id, name, email, provider, provider_id, created_at FROM users",
userEntityRowMapper, userEntityRowMapper,
).map { it.toDomain() } ).map { it.toDomain() }
@@ -106,47 +103,6 @@ class UserRepository(
jdbc.update("DELETE FROM users WHERE id = ?", id) jdbc.update("DELETE FROM users WHERE id = ?", id)
} }
override fun saveWithOAuth2(user: User, provider: String, providerId: String): User {
val updatedRows = jdbc.update(
"""
UPDATE users
SET name = ?, email = ?, password = ?, provider = ?, provider_id = ?
WHERE id = ?
""".trimIndent(),
user.name,
user.email,
user.password,
provider,
providerId,
user.id,
)
if (updatedRows == 0) {
jdbc.update(
"""
INSERT INTO users (id, name, email, password, provider, provider_id)
VALUES (?, ?, ?, ?, ?, ?)
""".trimIndent(),
user.id,
user.name,
user.email,
user.password,
provider,
providerId,
)
}
return user
}
override fun findByProviderAndProviderId(provider: String, providerId: String): User? {
val entities = jdbc.query(
"SELECT id, name, email, password, provider, provider_id, created_at FROM users WHERE provider = ? AND provider_id = ?",
userEntityRowMapper,
provider,
providerId,
)
return entities.firstOrNull()?.toDomain()
}
override fun findByProviderAndProviderId( override fun findByProviderAndProviderId(
provider: AuthProvider, provider: AuthProvider,
providerId: String, providerId: String,
@@ -154,7 +110,7 @@ class UserRepository(
val entities = val entities =
jdbc.query( jdbc.query(
""" """
SELECT id, name, email, password, provider, provider_id, created_at SELECT id, name, email, provider, provider_id, created_at
FROM users FROM users
WHERE provider = ? AND provider_id = ? WHERE provider = ? AND provider_id = ?
""".trimIndent(), """.trimIndent(),
@@ -1,3 +1,2 @@
CREATE UNIQUE INDEX IF NOT EXISTS idx_users_provider_provider_id CREATE UNIQUE INDEX IF NOT EXISTS idx_users_provider_provider_id
ON users(provider, provider_id) ON users(provider, provider_id);
WHERE provider IS NOT NULL AND provider_id IS NOT NULL;
@@ -17,7 +17,7 @@ import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPat
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
@SpringBootTest @SpringBootTest
@AutoConfigureMockMvc @AutoConfigureMockMvc(addFilters = false)
class FilmControllerTest { class FilmControllerTest {
@Autowired @Autowired
private lateinit var mockMvc: MockMvc private lateinit var mockMvc: MockMvc
@@ -17,7 +17,7 @@ import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
import org.springframework.transaction.annotation.Transactional import org.springframework.transaction.annotation.Transactional
@SpringBootTest @SpringBootTest
@AutoConfigureMockMvc @AutoConfigureMockMvc(addFilters = false)
@Transactional @Transactional
class FilmLibraryControllerTest { class FilmLibraryControllerTest {
@Autowired @Autowired
@@ -17,7 +17,7 @@ import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
import org.springframework.transaction.annotation.Transactional import org.springframework.transaction.annotation.Transactional
@SpringBootTest @SpringBootTest
@AutoConfigureMockMvc @AutoConfigureMockMvc(addFilters = false)
@Transactional @Transactional
class UserControllerTest { class UserControllerTest {
@Autowired @Autowired