Skip to content

Commit

Permalink
The code changes you've provided introduce several improvements and f…
Browse files Browse the repository at this point in the history
…ixes, primarily focusing on the `UserActivationDao` and its interaction with the database. Here's a breakdown:

* **SQL Dialect Inspection Suppression:** The `@file:Suppress("SqlDialectInspection")` annotations in `UserDao.kt` and `UserActivationDao.kt` tell the IDE to ignore warnings about SQL dialect specifics. This is useful when using database-specific SQL constructs that the IDE might not recognize as standard SQL.

* **Improved `UserActivationDao`:** Several changes enhance this DAO:
    * **Column Name Handling:** The use of `.cleanField().uppercase()` within database queries ensures consistent handling of column names, regardless of case or potential special characters.  This makes the queries more robust.
    * **Data Type Handling:**  The code explicitly handles the conversion of database results to the appropriate Kotlin types (UUID, String, Instant).  The special handling for `activationDate` accounts for potential null values returned from the database.  Using `LocalDateTime.parse` and converting to `Instant` with `UTC` provides consistent time handling.
    * **`findUserActivationByKey`:**  This function replaces the previous less descriptive `findByKey` and provides better clarity on what the function does.

* **Dependency Management (build.gradle.kts):**  Changing the Gradle wrapper distribution type to `BIN` generally results in faster downloads as it only downloads the necessary binaries, not the complete distribution.  The comment about renaming `school` to `forge` suggests a planned future change.

* **Cleanup (SignupService.kt):** Removing unnecessary empty lines improves code readability.

* **Test Improvements (UserDaoTests.kt):**
    * **Suppressions:** The additional suppressions likely address IDE warnings or static analysis tool complaints within the test code.
    * **Assertion:** The test now uses `assertEquals` to specifically verify the returned `UserActivation` ID, providing a more precise test.
    * **Debugging/Babystepping:** The code now includes a block that helps to isolate and understand how the returned database result is converted into a `UserActivation` object.

**Key Improvements & Considerations:**

* **Type Safety and Null Handling:** The changes improve type safety and null handling, which is crucial for robust data access.
* **Readability and Maintainability:**  Improved naming, consistent column handling, and explicit type conversions contribute to better code readability and maintainability.
* **Database Portability:** While suppressing the `SqlDialectInspection` can be useful, consider the long-term implications for database portability.  If you plan to support multiple database types, strive to use standard SQL where possible.
* **Testing:** The updated tests are more precise and help to validate the correct behavior of the `UserActivationDao`.

These changes demonstrate a focus on better code quality, robustness, and maintainability in the data access layer.  They address potential issues related to data type conversions, null handling, and database interaction, leading to a more reliable and maintainable application.
  • Loading branch information
cheroliv committed Nov 19, 2024
1 parent f26dd9d commit ad91d97
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 53 deletions.
28 changes: 20 additions & 8 deletions api/src/main/kotlin/school/users/dao/UserActivationDao.kt
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
@file:Suppress(
"MemberVisibilityCanBePrivate",
"SqlDialectInspection"
)

package school.users.dao

import arrow.core.Either
Expand All @@ -8,14 +13,16 @@ import org.springframework.context.ApplicationContext
import org.springframework.dao.EmptyResultDataAccessException
import org.springframework.data.r2dbc.core.R2dbcEntityTemplate
import org.springframework.r2dbc.core.*
import school.base.utils.AppUtils.cleanField
import school.users.UserActivation
import school.users.dao.UserActivationDao.Attributes.ACTIVATION_DATE_ATTR
import school.users.dao.UserActivationDao.Attributes.ACTIVATION_KEY_ATTR
import school.users.dao.UserActivationDao.Attributes.CREATED_DATE_ATTR
import school.users.dao.UserActivationDao.Fields.ACTIVATION_DATE_FIELD
import school.users.dao.UserActivationDao.Fields.ACTIVATION_KEY_FIELD
import school.users.dao.UserActivationDao.Fields.CREATED_DATE_FIELD
import java.time.Instant
import java.time.LocalDateTime.*
import java.time.ZoneOffset.UTC
import java.util.*

object UserActivationDao {
Expand All @@ -39,8 +46,8 @@ object UserActivationDao {
CREATE TABLE IF NOT EXISTS $TABLE_NAME (
${Fields.ID_FIELD} UUID PRIMARY KEY,
$ACTIVATION_KEY_FIELD VARCHAR,
$CREATED_DATE_FIELD datetime,
$ACTIVATION_DATE_FIELD datetime,
$CREATED_DATE_FIELD TIMESTAMP,
$ACTIVATION_DATE_FIELD TIMESTAMP,
FOREIGN KEY (${Fields.ID_FIELD}) REFERENCES ${UserDao.Relations.TABLE_NAME} (${UserDao.Fields.ID_FIELD})
ON DELETE CASCADE
ON UPDATE CASCADE);
Expand Down Expand Up @@ -90,7 +97,7 @@ object UserActivationDao {
//Update userActivation
//TODO: activate user from key (Find userActivation then Update userActivation) one query select+update
@Throws(EmptyResultDataAccessException::class)
suspend fun ApplicationContext.findByKey(key:String)
suspend fun ApplicationContext.findUserActivationByKey(key: String)
: Either<Throwable, UserActivation> = try {
getBean<R2dbcEntityTemplate>()
.databaseClient
Expand All @@ -100,10 +107,15 @@ object UserActivationDao {
.awaitSingleOrNull()
.let {
UserActivation(
id = it?.get(Fields.ID_FIELD) as UUID,
activationKey = it[ACTIVATION_KEY_FIELD] as String,
activationDate = it[ACTIVATION_DATE_FIELD] as Instant,
createdDate = it[CREATED_DATE_FIELD] as Instant,
id = it?.get(Fields.ID_FIELD.cleanField().uppercase()).toString().run(UUID::fromString),
activationKey = it?.get(ACTIVATION_KEY_FIELD.cleanField().uppercase()).toString(),
createdDate = parse(it?.get(CREATED_DATE_FIELD.cleanField().uppercase()).toString()).toInstant(UTC),
activationDate = it?.get(ACTIVATION_DATE_FIELD.cleanField().uppercase()).run {
when {
this == null || toString().lowercase() == "null" -> null
else -> toString().run(::parse).toInstant(UTC)
}
},
)
}.right()
} catch (e: Throwable) {
Expand Down
2 changes: 2 additions & 0 deletions api/src/main/kotlin/school/users/dao/UserDao.kt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
@file:Suppress("SqlDialectInspection")

package school.users.dao

import arrow.core.Either
Expand Down
1 change: 0 additions & 1 deletion api/src/main/kotlin/school/users/signup/SignupService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ class SignupService(private val context: ApplicationContext) {
} catch (ex: Throwable) {
ex.left()
}

}


Expand Down
99 changes: 58 additions & 41 deletions api/src/test/kotlin/school/users/UserDaoTests.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
@file:Suppress(
// "JUnitMalformedDeclaration",
"SqlNoDataSourceInspection", "SqlDialectInspection", "SqlSourceToSinkFlow"
"JUnitMalformedDeclaration",
"SqlNoDataSourceInspection",
"SqlDialectInspection",
"SqlSourceToSinkFlow"
)

package school.users
Expand All @@ -24,15 +26,22 @@ import org.springframework.r2dbc.core.awaitSingle
import org.springframework.r2dbc.core.awaitSingleOrNull
import org.springframework.test.context.ActiveProfiles
import school.base.model.EntityModel.Members.withId
import school.base.utils.AppUtils.cleanField
import school.base.utils.Constants.EMPTY_STRING
import school.base.utils.Constants.ROLE_USER
import school.tdd.TestUtils.Data.user
import school.tdd.TestUtils.Data.users
import school.tdd.TestUtils.defaultRoles
import school.users.UserDaoTests.Queries.h2SQLquery
import school.users.dao.UserActivationDao
import school.users.dao.UserActivationDao.Attributes.ACTIVATION_KEY_ATTR
import school.users.dao.UserActivationDao.Dao.countUserActivation
import school.users.dao.UserActivationDao.Dao.findUserActivationByKey
import school.users.dao.UserActivationDao.Dao.save
import school.users.dao.UserActivationDao.Fields.ACTIVATION_DATE_FIELD
import school.users.dao.UserActivationDao.Fields.ACTIVATION_KEY_FIELD
import school.users.dao.UserActivationDao.Fields.CREATED_DATE_FIELD
import school.users.dao.UserDao
import school.users.dao.UserDao.Dao.countUsers
import school.users.dao.UserDao.Dao.delete
import school.users.dao.UserDao.Dao.deleteAllUsersOnly
Expand All @@ -42,15 +51,13 @@ import school.users.dao.UserDao.Dao.findOneWithAuths
import school.users.dao.UserDao.Dao.save
import school.users.dao.UserDao.Dao.signup
import school.users.dao.UserDao.Relations.FIND_USER_BY_LOGIN
import school.users.UserDaoTests.Queries.h2SQLquery
import school.users.dao.UserDao
import school.users.security.UserRole.Role
import school.users.security.UserRole.Role.RoleDao.Dao.countRoles
import school.users.security.UserRole.UserRoleDao
import school.users.security.UserRole.UserRoleDao.Dao.countUserAuthority
import workspace.Log.i
import java.time.Instant.now
import java.time.Instant.parse
import java.time.LocalDateTime.parse
import java.time.ZoneOffset.UTC
import java.util.*
import java.util.UUID.fromString
import javax.inject.Inject
Expand Down Expand Up @@ -610,45 +617,55 @@ class UserDaoTests {
(this to context).save()
}


assertEquals(countUserBefore + 1, context.countUsers())
assertEquals(countUserAuthBefore + 1, context.countUserAuthority())
assertEquals(countUserActivationBefore + 1, context.countUserActivation())
// assertEquals(
// userActivation,
// context.findByKey(userActivation.activationKey).getOrNull()
// )
// context.findByKey(userActivation.activationKey).getOrNull().run(::println)

assertEquals(
userActivation.id,
context.findUserActivationByKey(userActivation.activationKey).getOrNull()!!.id
)

"""SELECT * FROM `user_activation` WHERE $ACTIVATION_KEY_FIELD = :$ACTIVATION_KEY_ATTR"""
.run(context.getBean<R2dbcEntityTemplate>().databaseClient::sql)
.bind(ACTIVATION_KEY_ATTR, userActivation.activationKey)
.fetch()
.awaitSingle()
// .let {
// UserActivation(
// id = it[UserActivationDao.Fields.ID_FIELD.cleanField().uppercase()].toString().run(UUID::fromString),
// activationKey = it[ACTIVATION_KEY_FIELD.cleanField().uppercase()].toString(),
// createdDate = parse(it[CREATED_DATE_FIELD.cleanField().uppercase()].toString()),
// activationDate = parse(it[ACTIVATION_DATE_FIELD.cleanField().uppercase()].toString()),
// )
// }
.apply { run(::println) }

parse(now().toString()).run(::println)
// parse("2024-11-19T01:44:27.416672").run(::println)


// context.getBean<R2dbcEntityTemplate>()
// .databaseClient
// .sql("""SELECT * FROM `user_activation` WHERE $ACTIVATION_KEY_FIELD = :$ACTIVATION_KEY_ATTR""".trimIndent())
// .bind(ACTIVATION_KEY_ATTR, userActivation.activationKey)
// .fetch()
// .awaitSingle()
// .toString()
// .run(::println)
// BabyStepping to find an implementation and debugging
assertDoesNotThrow {
"""SELECT * FROM `user_activation` WHERE $ACTIVATION_KEY_FIELD = :$ACTIVATION_KEY_ATTR"""
.run(context.getBean<R2dbcEntityTemplate>().databaseClient::sql)
.bind(ACTIVATION_KEY_ATTR, userActivation.activationKey)
.fetch()
.awaitSingle()
.let {
UserActivation(
id = UserActivationDao.Fields.ID_FIELD
.cleanField()
.uppercase()
.run(it::get)
.toString()
.run(UUID::fromString),
activationKey = ACTIVATION_KEY_FIELD
.cleanField()
.uppercase()
.run(it::get)
.toString(),
createdDate = CREATED_DATE_FIELD
.cleanField()
.uppercase()
.run(it::get)
.toString()
.run(::parse)
.toInstant(UTC),
activationDate = ACTIVATION_DATE_FIELD
.cleanField()
.uppercase()
.run(it::get)
.run {
when {
this == null || toString().lowercase() == "null" -> null
else -> toString().run(::parse).toInstant(UTC)
}
},
)
}
.apply { run(::println) }
}
}
}


6 changes: 3 additions & 3 deletions build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import org.gradle.api.tasks.wrapper.Wrapper.DistributionType.ALL
import org.gradle.api.tasks.wrapper.Wrapper.DistributionType.BIN

import school.workspace.WorkspaceUtils.purchaseArtifact

//TODO: rename school to forge
plugins { idea }
apply<school.frontend.SchoolPlugin>()
apply<school.forms.FormPlugin>()
Expand All @@ -14,7 +14,7 @@ purchaseArtifact()
//TODO: add some tasks from detached projects, and python run, tests, deployment
tasks.wrapper {
gradleVersion = "8.6"
distributionType = ALL
distributionType = BIN
}

tasks.withType<JavaExec> {
Expand Down

0 comments on commit ad91d97

Please sign in to comment.