From ad91d97cdac67de8bf4577c74fdca606a6921b16 Mon Sep 17 00:00:00 2001 From: cheroliv Date: Tue, 19 Nov 2024 15:45:47 +0100 Subject: [PATCH] The code changes you've provided introduce several improvements and fixes, 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. --- .../school/users/dao/UserActivationDao.kt | 28 ++++-- .../main/kotlin/school/users/dao/UserDao.kt | 2 + .../school/users/signup/SignupService.kt | 1 - .../test/kotlin/school/users/UserDaoTests.kt | 99 +++++++++++-------- build.gradle.kts | 6 +- 5 files changed, 83 insertions(+), 53 deletions(-) diff --git a/api/src/main/kotlin/school/users/dao/UserActivationDao.kt b/api/src/main/kotlin/school/users/dao/UserActivationDao.kt index f20f67b..2522c8c 100644 --- a/api/src/main/kotlin/school/users/dao/UserActivationDao.kt +++ b/api/src/main/kotlin/school/users/dao/UserActivationDao.kt @@ -1,3 +1,8 @@ +@file:Suppress( + "MemberVisibilityCanBePrivate", + "SqlDialectInspection" +) + package school.users.dao import arrow.core.Either @@ -8,6 +13,7 @@ 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 @@ -15,7 +21,8 @@ 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 { @@ -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); @@ -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 = try { getBean() .databaseClient @@ -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) { diff --git a/api/src/main/kotlin/school/users/dao/UserDao.kt b/api/src/main/kotlin/school/users/dao/UserDao.kt index aaee178..fecfab3 100644 --- a/api/src/main/kotlin/school/users/dao/UserDao.kt +++ b/api/src/main/kotlin/school/users/dao/UserDao.kt @@ -1,3 +1,5 @@ +@file:Suppress("SqlDialectInspection") + package school.users.dao import arrow.core.Either diff --git a/api/src/main/kotlin/school/users/signup/SignupService.kt b/api/src/main/kotlin/school/users/signup/SignupService.kt index 3f79eb0..9e0f0db 100755 --- a/api/src/main/kotlin/school/users/signup/SignupService.kt +++ b/api/src/main/kotlin/school/users/signup/SignupService.kt @@ -122,7 +122,6 @@ class SignupService(private val context: ApplicationContext) { } catch (ex: Throwable) { ex.left() } - } diff --git a/api/src/test/kotlin/school/users/UserDaoTests.kt b/api/src/test/kotlin/school/users/UserDaoTests.kt index 0b1439e..04f1956 100755 --- a/api/src/test/kotlin/school/users/UserDaoTests.kt +++ b/api/src/test/kotlin/school/users/UserDaoTests.kt @@ -1,6 +1,8 @@ @file:Suppress( -// "JUnitMalformedDeclaration", - "SqlNoDataSourceInspection", "SqlDialectInspection", "SqlSourceToSinkFlow" + "JUnitMalformedDeclaration", + "SqlNoDataSourceInspection", + "SqlDialectInspection", + "SqlSourceToSinkFlow" ) package school.users @@ -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 @@ -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 @@ -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().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() -// .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().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) } + } } } - diff --git a/build.gradle.kts b/build.gradle.kts index bf2f3ac..51a9ce8 100755 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -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() apply() @@ -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 {