diff --git a/backend/src/main/kotlin/com/lightswitch/application/service/FeatureFlagService.kt b/backend/src/main/kotlin/com/lightswitch/application/service/FeatureFlagService.kt index eafb848..6ab48bc 100644 --- a/backend/src/main/kotlin/com/lightswitch/application/service/FeatureFlagService.kt +++ b/backend/src/main/kotlin/com/lightswitch/application/service/FeatureFlagService.kt @@ -1,6 +1,5 @@ package com.lightswitch.application.service -import com.lightswitch.infrastructure.database.entity.Condition import com.lightswitch.infrastructure.database.entity.FeatureFlag import com.lightswitch.infrastructure.database.entity.User import com.lightswitch.infrastructure.database.model.Type @@ -8,6 +7,10 @@ import com.lightswitch.infrastructure.database.repository.ConditionRepository import com.lightswitch.infrastructure.database.repository.FeatureFlagRepository import com.lightswitch.presentation.exception.BusinessException import com.lightswitch.presentation.model.flag.CreateFeatureFlagRequest +import com.lightswitch.presentation.model.flag.UpdateFeatureFlagRequest +import com.lightswitch.presentation.model.flag.defaultValueAsPair +import com.lightswitch.presentation.model.flag.variantPairs +import jakarta.persistence.EntityNotFoundException import org.springframework.stereotype.Service import org.springframework.transaction.annotation.Transactional @@ -21,7 +24,7 @@ class FeatureFlagService( } fun getFlagOrThrow(key: String): FeatureFlag { - return featureFlagRepository.findByName(key) ?: throw BusinessException("Feature flag $key does not exist") + return featureFlagRepository.findByName(key) ?: throw EntityNotFoundException("Feature flag $key does not exist") } @Transactional @@ -29,7 +32,7 @@ class FeatureFlagService( user: User, request: CreateFeatureFlagRequest, ): FeatureFlag { - featureFlagRepository.findByName(request.key)?.let { + if (featureFlagRepository.existsByName(request.key)) { throw BusinessException("FeatureFlag with key ${request.key} already exists") } @@ -46,17 +49,37 @@ class FeatureFlagService( ) request.defaultValueAsPair() - .let { (key, value) -> Condition(flag = flag, key = key, value = value) } - .let { conditionRepository.save(it) } - .also { - flag.defaultCondition = it - flag.conditions.add(it) - } + .let { (key, value) -> flag.addDefaultCondition(key = key, value = value) } + request.variantPairs() + ?.map { variant -> flag.addCondition(key = variant.first, value = variant.second) } + + return featureFlagRepository.save(flag) + } + @Transactional + fun update( + user: User, + key: String, + request: UpdateFeatureFlagRequest, + ): FeatureFlag { + if (request.key != key && featureFlagRepository.existsByName(request.key)) { + throw BusinessException("FeatureFlag with key ${request.key} already exists") + } + + val flag = getFlagOrThrow(key) + flag.name = request.key + flag.type = Type.from(request.type) + flag.description = request.description + flag.updatedBy = user + + flag.defaultCondition = null + flag.conditions.clear() + conditionRepository.deleteByFlag(flag) + + request.defaultValueAsPair() + .let { (key, value) -> flag.addDefaultCondition(key = key, value = value) } request.variantPairs() - ?.map { variant -> Condition(flag = flag, key = variant.first, value = variant.second) } - ?.let { conditionRepository.saveAll(it) } - ?.also { flag.conditions.addAll(it) } + ?.map { variant -> flag.addCondition(key = variant.first, value = variant.second) } return featureFlagRepository.save(flag) } diff --git a/backend/src/main/kotlin/com/lightswitch/infrastructure/database/entity/FeatureFlag.kt b/backend/src/main/kotlin/com/lightswitch/infrastructure/database/entity/FeatureFlag.kt index c5f9d94..e764265 100644 --- a/backend/src/main/kotlin/com/lightswitch/infrastructure/database/entity/FeatureFlag.kt +++ b/backend/src/main/kotlin/com/lightswitch/infrastructure/database/entity/FeatureFlag.kt @@ -25,12 +25,12 @@ class FeatureFlag( @GeneratedValue(strategy = GenerationType.IDENTITY) var id: Long? = null, @Column(nullable = false) - val name: String, + var name: String, @Column(nullable = false) - val description: String, + var description: String, @Column(nullable = false) @Enumerated(value = EnumType.STRING) - val type: Type, + var type: Type, @Column(nullable = false) var enabled: Boolean, @OneToOne(fetch = FetchType.LAZY, cascade = [CascadeType.ALL], optional = true) @@ -44,4 +44,21 @@ class FeatureFlag( @LastModifiedBy @ManyToOne(fetch = FetchType.LAZY) var updatedBy: User, -) : BaseEntity() +) : BaseEntity() { + fun addDefaultCondition( + key: String, + value: Any, + ) { + val condition = Condition(flag = this, key = key, value = value) + defaultCondition = condition + conditions.add(condition) + } + + fun addCondition( + key: String, + value: Any, + ) { + val condition = Condition(flag = this, key = key, value = value) + conditions.add(condition) + } +} diff --git a/backend/src/main/kotlin/com/lightswitch/infrastructure/database/repository/ConditionRepository.kt b/backend/src/main/kotlin/com/lightswitch/infrastructure/database/repository/ConditionRepository.kt index 103aea8..7ae8935 100644 --- a/backend/src/main/kotlin/com/lightswitch/infrastructure/database/repository/ConditionRepository.kt +++ b/backend/src/main/kotlin/com/lightswitch/infrastructure/database/repository/ConditionRepository.kt @@ -1,6 +1,13 @@ package com.lightswitch.infrastructure.database.repository import com.lightswitch.infrastructure.database.entity.Condition +import com.lightswitch.infrastructure.database.entity.FeatureFlag import org.springframework.data.jpa.repository.JpaRepository +import org.springframework.data.jpa.repository.Modifying +import org.springframework.data.jpa.repository.Query -interface ConditionRepository : JpaRepository +interface ConditionRepository : JpaRepository { + @Modifying(clearAutomatically = true) + @Query("DELETE FROM Condition c where c.flag = :flag") + fun deleteByFlag(flag: FeatureFlag) +} diff --git a/backend/src/main/kotlin/com/lightswitch/infrastructure/database/repository/FeatureFlagRepository.kt b/backend/src/main/kotlin/com/lightswitch/infrastructure/database/repository/FeatureFlagRepository.kt index 35192ec..30218a5 100644 --- a/backend/src/main/kotlin/com/lightswitch/infrastructure/database/repository/FeatureFlagRepository.kt +++ b/backend/src/main/kotlin/com/lightswitch/infrastructure/database/repository/FeatureFlagRepository.kt @@ -5,6 +5,8 @@ import org.springframework.data.jpa.repository.EntityGraph import org.springframework.data.jpa.repository.JpaRepository interface FeatureFlagRepository : JpaRepository { + fun existsByName(name: String): Boolean + @EntityGraph(attributePaths = ["createdBy", "updatedBy", "defaultCondition", "conditions"]) fun findByName(name: String): FeatureFlag? diff --git a/backend/src/main/kotlin/com/lightswitch/infrastructure/swagger/SwaggerConfig.kt b/backend/src/main/kotlin/com/lightswitch/infrastructure/swagger/SwaggerConfig.kt index 853059e..0028524 100644 --- a/backend/src/main/kotlin/com/lightswitch/infrastructure/swagger/SwaggerConfig.kt +++ b/backend/src/main/kotlin/com/lightswitch/infrastructure/swagger/SwaggerConfig.kt @@ -4,6 +4,7 @@ import io.swagger.v3.oas.annotations.enums.SecuritySchemeType import io.swagger.v3.oas.annotations.security.SecurityScheme import io.swagger.v3.oas.models.OpenAPI import io.swagger.v3.oas.models.info.Info +import io.swagger.v3.oas.models.security.SecurityRequirement import org.springframework.context.annotation.Bean import org.springframework.context.annotation.Configuration @@ -24,4 +25,7 @@ class SwaggerConfig { .description("API for feature flag management.") .version("1.0.0"), ) + .addSecurityItem( + SecurityRequirement().addList("bearerAuth"), + ) } diff --git a/backend/src/main/kotlin/com/lightswitch/presentation/controller/FeatureFlagController.kt b/backend/src/main/kotlin/com/lightswitch/presentation/controller/FeatureFlagController.kt index 487a302..5e64878 100644 --- a/backend/src/main/kotlin/com/lightswitch/presentation/controller/FeatureFlagController.kt +++ b/backend/src/main/kotlin/com/lightswitch/presentation/controller/FeatureFlagController.kt @@ -85,10 +85,16 @@ class FeatureFlagController( @PathVariable key: String, @RequestBody request: UpdateFeatureFlagRequest, ): PayloadResponse { - return PayloadResponse( - status = "status", - message = "message", - data = null, + // TODO: Improve the way finding the authenticated user. + val authentication = SecurityContextHolder.getContext().authentication + val userId = authentication.name + val user = userRepository.findByIdOrNull(userId.toLong()) ?: throw BusinessException("User not found") + + val flag = featureFlagService.update(user, key, request) + + return PayloadResponse.success( + message = "Updated flag successfully", + data = FeatureFlagResponse.from(flag), ) } diff --git a/backend/src/main/kotlin/com/lightswitch/presentation/model/flag/CreateFeatureFlagRequest.kt b/backend/src/main/kotlin/com/lightswitch/presentation/model/flag/CreateFeatureFlagRequest.kt index 6db9d03..c1c5143 100644 --- a/backend/src/main/kotlin/com/lightswitch/presentation/model/flag/CreateFeatureFlagRequest.kt +++ b/backend/src/main/kotlin/com/lightswitch/presentation/model/flag/CreateFeatureFlagRequest.kt @@ -14,12 +14,8 @@ data class CreateFeatureFlagRequest( @field:Pattern(regexp = "(?i)^(number|boolean|string)$", message = "Type must be one of: number, boolean, string") val type: String, @field:NotEmpty(message = "Default value is required.") - val defaultValue: Map, + override val defaultValue: Map, @field:NotBlank(message = "Description is required.") val description: String, - val variants: List>? = null, -) { - fun defaultValueAsPair(): Pair = defaultValue.entries.first().toPair() - - fun variantPairs(): List>? = variants?.map { it.entries.first().toPair() } -} + override val variants: List>? = null, +) : FeatureFlagRequest diff --git a/backend/src/main/kotlin/com/lightswitch/presentation/model/flag/FeatureFlagRequest.kt b/backend/src/main/kotlin/com/lightswitch/presentation/model/flag/FeatureFlagRequest.kt new file mode 100644 index 0000000..1262247 --- /dev/null +++ b/backend/src/main/kotlin/com/lightswitch/presentation/model/flag/FeatureFlagRequest.kt @@ -0,0 +1,10 @@ +package com.lightswitch.presentation.model.flag + +interface FeatureFlagRequest { + val defaultValue: Map + val variants: List>? +} + +fun FeatureFlagRequest.defaultValueAsPair(): Pair = defaultValue.entries.first().toPair() + +fun FeatureFlagRequest.variantPairs(): List>? = variants?.map { it.entries.first().toPair() } diff --git a/backend/src/main/kotlin/com/lightswitch/presentation/model/flag/UpdateFeatureFlagRequest.kt b/backend/src/main/kotlin/com/lightswitch/presentation/model/flag/UpdateFeatureFlagRequest.kt index fa034bf..90b5c47 100644 --- a/backend/src/main/kotlin/com/lightswitch/presentation/model/flag/UpdateFeatureFlagRequest.kt +++ b/backend/src/main/kotlin/com/lightswitch/presentation/model/flag/UpdateFeatureFlagRequest.kt @@ -2,18 +2,17 @@ package com.lightswitch.presentation.model.flag import jakarta.validation.constraints.NotBlank import jakarta.validation.constraints.NotEmpty -import jakarta.validation.constraints.NotNull +import jakarta.validation.constraints.Pattern data class UpdateFeatureFlagRequest( @field:NotBlank(message = "Key is required.") val key: String, - @field:NotNull(message = "Type is required.") + @field:NotBlank(message = "Type is required.") + @field:Pattern(regexp = "(?i)^(number|boolean|string)$", message = "Type must be one of: number, boolean, string") val type: String, @field:NotEmpty(message = "Default value is required.") - val defaultValue: Map, + override val defaultValue: Map, @field:NotBlank(message = "Description is required.") val description: String, - val variants: List>? = null, - @field:NotBlank(message = "UpdatedBy is required.") - val updatedBy: String, -) + override val variants: List>? = null, +) : FeatureFlagRequest diff --git a/backend/src/test/kotlin/com/lightswitch/application/service/FeatureFlagServiceTest.kt b/backend/src/test/kotlin/com/lightswitch/application/service/FeatureFlagServiceTest.kt index f699bc7..aff250d 100644 --- a/backend/src/test/kotlin/com/lightswitch/application/service/FeatureFlagServiceTest.kt +++ b/backend/src/test/kotlin/com/lightswitch/application/service/FeatureFlagServiceTest.kt @@ -10,6 +10,8 @@ import com.lightswitch.infrastructure.database.repository.FeatureFlagRepository import com.lightswitch.infrastructure.database.repository.UserRepository import com.lightswitch.presentation.exception.BusinessException import com.lightswitch.presentation.model.flag.CreateFeatureFlagRequest +import com.lightswitch.presentation.model.flag.UpdateFeatureFlagRequest +import jakarta.persistence.EntityNotFoundException import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatThrownBy import org.assertj.core.api.Assertions.tuple @@ -41,14 +43,7 @@ class FeatureFlagServiceTest : BaseRepositoryTest() { @Test fun `getFlags should return all feature flags`() { - val user = - userRepository.save( - User( - username = "test-user", - passwordHash = "passwordHash", - lastLoginAt = LocalDate.of(2025, 1, 1).atStartOfDay(), - ), - ) + val user = saveTestUser() val flag1 = featureFlagRepository.save( FeatureFlag( @@ -112,14 +107,7 @@ class FeatureFlagServiceTest : BaseRepositoryTest() { @Test fun `getFlags should return empty list when all feature flags are deleted`() { - val user = - userRepository.save( - User( - username = "test-user", - passwordHash = "passwordHash", - lastLoginAt = LocalDate.of(2025, 1, 1).atStartOfDay(), - ), - ) + val user = saveTestUser() val flag = FeatureFlag( name = "user-limit", @@ -138,14 +126,7 @@ class FeatureFlagServiceTest : BaseRepositoryTest() { @Test fun `getFlagOrThrow should return feature flag when key exists`() { - val user = - userRepository.save( - User( - username = "test-user", - passwordHash = "passwordHash", - lastLoginAt = LocalDate.of(2025, 1, 1).atStartOfDay(), - ), - ) + val user = saveTestUser() val savedFlag = featureFlagRepository.save( FeatureFlag( @@ -196,24 +177,17 @@ class FeatureFlagServiceTest : BaseRepositoryTest() { } @Test - fun `getFlagOrThrow should throw BusinessException when key does not exist`() { + fun `getFlagOrThrow should throw EntityNotFoundException when key does not exist`() { val nonExistentKey = "non-existent-key" assertThatThrownBy { featureFlagService.getFlagOrThrow(nonExistentKey) } - .isInstanceOf(BusinessException::class.java) + .isInstanceOf(EntityNotFoundException::class.java) .hasMessageContaining("Feature flag $nonExistentKey does not exist") } @Test fun `getFlagOrThrow should not return when feature flag is deleted`() { - val user = - userRepository.save( - User( - username = "test-user", - passwordHash = "passwordHash", - lastLoginAt = LocalDate.of(2025, 1, 1).atStartOfDay(), - ), - ) + val user = saveTestUser() val flag = FeatureFlag( name = "user-limit", @@ -228,12 +202,13 @@ class FeatureFlagServiceTest : BaseRepositoryTest() { featureFlagRepository.save(flag) assertThatThrownBy { featureFlagService.getFlagOrThrow("user-limit") } - .isInstanceOf(BusinessException::class.java) + .isInstanceOf(EntityNotFoundException::class.java) .hasMessageContaining("Feature flag user-limit does not exist") } @Test fun `create feature flag can save feature flag & conditions`() { + val user = saveTestUser() val request = CreateFeatureFlagRequest( key = "user-limit", @@ -248,14 +223,6 @@ class FeatureFlagServiceTest : BaseRepositoryTest() { mapOf("enterprise" to 1000), ), ) - val user = - userRepository.save( - User( - username = "username", - passwordHash = "passwordHash", - lastLoginAt = LocalDate.of(2025, 1, 1).atStartOfDay(), - ), - ) val flag = featureFlagService.create(user, request) @@ -281,6 +248,7 @@ class FeatureFlagServiceTest : BaseRepositoryTest() { @Test fun `create feature flag with only defaultValue`() { + val user = saveTestUser() val request = CreateFeatureFlagRequest( key = "user-limit", @@ -290,15 +258,6 @@ class FeatureFlagServiceTest : BaseRepositoryTest() { description = "User Limit Flag", variants = null, ) - val user = - userRepository.save( - User( - username = "username", - passwordHash = "passwordHash", - lastLoginAt = LocalDate.of(2025, 1, 1).atStartOfDay(), - ), - ) - val flag = featureFlagService.create(user, request) assertThat(flag.name).isEqualTo("user-limit") @@ -327,14 +286,7 @@ class FeatureFlagServiceTest : BaseRepositoryTest() { description = "Duplicate Key Test", variants = listOf(mapOf("free" to 10)), ) - val user = - userRepository.save( - User( - username = "username", - passwordHash = "passwordHash", - lastLoginAt = LocalDate.of(2025, 1, 1).atStartOfDay(), - ), - ) + val user = saveTestUser() featureFlagService.create(user, request) @@ -345,6 +297,7 @@ class FeatureFlagServiceTest : BaseRepositoryTest() { @Test fun `create feature flag throw IllegalArgumentException when type is not supported`() { + val user = saveTestUser() val request = CreateFeatureFlagRequest( key = "invalid-type", @@ -354,17 +307,159 @@ class FeatureFlagServiceTest : BaseRepositoryTest() { description = "Invalid Type Test", variants = listOf(mapOf("free" to 10)), ) - val user = - userRepository.save( - User( - username = "username", - passwordHash = "passwordHash", - lastLoginAt = LocalDate.of(2025, 1, 1).atStartOfDay(), + + assertThatThrownBy { featureFlagService.create(user, request) } + .isInstanceOf(IllegalArgumentException::class.java) + .hasMessageContaining("Unsupported type: json") + } + + @Test + fun `update feature flag can update feature flag`() { + val user = saveTestUser() + val flag = + featureFlagRepository.save( + FeatureFlag( + name = "user-limit", + description = "description", + type = Type.BOOLEAN, + enabled = true, + createdBy = user, + updatedBy = user, ), + ).apply { + this.defaultCondition = Condition(flag = this, key = "boolean", value = true) + this.conditions.add(this.defaultCondition!!) + this.conditions.add(Condition(flag = this, key = "free", value = true)) + }.also { + featureFlagRepository.save(it) + } + val request = + UpdateFeatureFlagRequest( + key = "user-limit-updated", + type = "number", + description = "Updated description", + defaultValue = mapOf("number" to 123), + variants = + listOf( + mapOf("free" to 10), + mapOf("pro" to 100), + ), ) - assertThatThrownBy { featureFlagService.create(user, request) } + val updated = featureFlagService.update(user, "user-limit", request) + + assertThat(flag.id).isEqualTo(updated.id) + assertThat(updated.name).isEqualTo("user-limit-updated") + assertThat(updated.description).isEqualTo("Updated description") + assertThat(updated.type).isEqualTo(Type.NUMBER) + assertThat(updated.defaultCondition) + .extracting("key", "value") + .containsExactly("number", 123) + assertThat(updated.conditions) + .hasSize(3) + .extracting("key", "value") + .containsExactlyInAnyOrder( + tuple("number", 123), + tuple("free", 10), + tuple("pro", 100), + ) + assertThat(updated.updatedBy.id).isEqualTo(user.id) + } + + @Test + fun `update feature flag throw BusinessException if trying to rename to an existing key`() { + val user = saveTestUser() + featureFlagRepository.save( + FeatureFlag( + name = "flag-one", + description = "Flag One", + type = Type.STRING, + enabled = true, + createdBy = user, + updatedBy = user, + ), + ) + featureFlagRepository.save( + FeatureFlag( + name = "flag-two", + description = "Flag Two", + type = Type.STRING, + enabled = true, + createdBy = user, + updatedBy = user, + ), + ) + val request = + UpdateFeatureFlagRequest( + key = "flag-two", + type = "boolean", + description = "Try rename", + defaultValue = mapOf("boolean" to true), + variants = null, + ) + + assertThatThrownBy { + featureFlagService.update(user, "flag-one", request) + } + .isInstanceOf(BusinessException::class.java) + .hasMessageContaining("FeatureFlag with key flag-two already exists") + } + + @Test + fun `update feature flag throw EntityNotFoundException if trying to update non-existent flag`() { + val user = saveTestUser() + val nonExistentKey = "not-exist" + val request = + UpdateFeatureFlagRequest( + key = "not-exist", + type = "boolean", + description = "Desc", + defaultValue = mapOf("boolean" to false), + variants = null, + ) + + assertThatThrownBy { + featureFlagService.update(user, nonExistentKey, request) + } + .isInstanceOf(EntityNotFoundException::class.java) + .hasMessageContaining("Feature flag $nonExistentKey does not exist") + } + + @Test + fun `update feature flag throw IllegalArgumentException if type is invalid`() { + val user = saveTestUser() + featureFlagRepository.save( + FeatureFlag( + name = "flag-invalid-type", + description = "Test Flag", + type = Type.BOOLEAN, + enabled = true, + createdBy = user, + updatedBy = user, + ), + ) + val request = + UpdateFeatureFlagRequest( + key = "flag-invalid-type", + type = "json", + description = "Invalid Type", + defaultValue = mapOf("json" to "whatever"), + variants = null, + ) + + assertThatThrownBy { + featureFlagService.update(user, "flag-invalid-type", request) + } .isInstanceOf(IllegalArgumentException::class.java) .hasMessageContaining("Unsupported type: json") } + + private fun saveTestUser() = + userRepository.save( + User( + username = "test-user", + passwordHash = "test-pass", + lastLoginAt = LocalDate.of(2025, 1, 1).atStartOfDay(), + ), + ) }