diff --git a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingOption.java b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingOption.java index c499b4ac10..d701e8d88d 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingOption.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingOption.java @@ -15,13 +15,13 @@ @JsonSerialize(using = SettingOptionSerializer.class) @JsonDeserialize(using = SettingOptionDeserializer.class) public enum SettingOption { - LOGO_URL("The logo url", Category.THEME.name(), Type.FILE.name()); + LOGO_URL("The logo url", Category.THEME, Type.FILE); private final String description; - private final String category; - private final String type; + private final Category category; + private final Type type; - SettingOption(String description, String category, String type) { + SettingOption(String description, Category category, Type type) { this.description = description; this.category = category; this.type = type; diff --git a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingOptionSerializer.java b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingOptionSerializer.java index 305f7eaa30..689118b66d 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingOptionSerializer.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingOptionSerializer.java @@ -25,7 +25,9 @@ public void serialize( generator.writeFieldName("description"); generator.writeString(settingOption.getDescription()); generator.writeFieldName("category"); - generator.writeString(settingOption.getCategory()); + generator.writeString(settingOption.getCategory().name()); + generator.writeFieldName("type"); + generator.writeString(settingOption.getType().name()); generator.writeEndObject(); } } \ No newline at end of file diff --git a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsController.java b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsController.java index 9cdbeef41d..42dbb9797d 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsController.java @@ -2,8 +2,6 @@ import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.services.permissions.RequiredPermission; -import io.micronaut.core.annotation.Nullable; -import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; import io.micronaut.http.HttpStatus; import io.micronaut.http.MediaType; @@ -16,6 +14,7 @@ import io.micronaut.validation.Validated; import io.swagger.v3.oas.annotations.tags.Tag; import jakarta.validation.Valid; +import jakarta.validation.constraints.NotNull; import java.net.URI; import java.util.List; @@ -38,49 +37,68 @@ public SettingsController(SettingsServices settingsServices) { } /** - * Find setting by its name, or if blank find all settings. + * Find all settings that are currently configured * - * @param name {@link String} name of the setting * @return {@link >} Returned setting */ @ExecuteOn(TaskExecutors.BLOCKING) - @Get("/{?name}") + @Get @RequiredPermission(Permission.CAN_VIEW_SETTINGS) - public List findByName(@Nullable String name) { - return settingsServices.findByName(name); + public List findAllSettings() { + return settingsServices.findAllSettings().stream() + .map(this::fromEntity).toList(); } + /** + * Find setting by its name + * + * @param name {@link String} name of the setting + * @return {@link } Returned setting + */ + @ExecuteOn(TaskExecutors.BLOCKING) + @Get("/{name}") + @RequiredPermission(Permission.CAN_VIEW_SETTINGS) + public SettingsResponseDTO findByName(@PathVariable @NotNull String name) { + return fromEntity(settingsServices.findByName(name)); + } + + /** + * Find all available setting options that can be used to configure a new setting. + * Note: there can only be one setting per unique name + * @return {@link } Returned setting options + */ @Get("/options") @RequiredPermission(Permission.CAN_VIEW_SETTINGS) public List getOptions() { return SettingOption.getOptions(); } + /** * Create and save a new setting. - * - * @param settingDTO, {@link SettingsCreateDTO} + * Note: there can only be one setting unique name + * @param settingDTO, {@link SettingsDTO} * @return {@link HttpResponse} */ @ExecuteOn(TaskExecutors.BLOCKING) @Post @RequiredPermission(Permission.CAN_ADMINISTER_SETTINGS) - public HttpResponse save(@Body @Valid SettingsCreateDTO settingDTO) { + public HttpResponse save(@Body @Valid SettingsDTO settingDTO) { Setting savedSetting = settingsServices.save(fromDTO(settingDTO)); URI location = UriBuilder.of(PATH).path(savedSetting.getId().toString()).build(); return HttpResponse.created(fromEntity(savedSetting), location); } /** - * Update the setting. + * Update only the value field of a setting found by its name. * - * @param settingDTO, {@link SettingsUpdateDTO} + * @param settingsDTO, {@link SettingsDTO} * @return {@link } */ @Put @ExecuteOn(TaskExecutors.BLOCKING) @RequiredPermission(Permission.CAN_ADMINISTER_SETTINGS) - public HttpResponse update(@Body @Valid SettingsUpdateDTO settingDTO) { - Setting savedSetting = settingsServices.update(fromUpdateDTO(settingDTO)); + public HttpResponse update(@Body @Valid SettingsDTO settingsDTO) { + Setting savedSetting = settingsServices.update(settingsDTO.getName(), settingsDTO.getValue()); SettingsResponseDTO settingsResponseDTO = fromEntity(savedSetting); URI location = UriBuilder.of(PATH).path(savedSetting.getId().toString()).build(); return HttpResponse.ok(settingsResponseDTO).headers(headers -> @@ -100,18 +118,18 @@ public HttpStatus delete(UUID id) { return settingsServices.delete(id) ? HttpStatus.OK : HttpStatus.UNPROCESSABLE_ENTITY; } - private Setting fromDTO(SettingsCreateDTO settingsCreateDTO) { - return new Setting(settingsCreateDTO.getName(), settingsCreateDTO.getValue()); - } - - private Setting fromUpdateDTO(SettingsUpdateDTO settingsUpdateDTO) { - return new Setting(settingsUpdateDTO.getId(), settingsUpdateDTO.getName(), settingsUpdateDTO.getValue()); + private Setting fromDTO(SettingsDTO settingsDTO) { + return new Setting(settingsDTO.getName(), settingsDTO.getValue()); } private SettingsResponseDTO fromEntity(Setting entity) { + SettingOption option = SettingOption.fromName(entity.getName()); SettingsResponseDTO dto = new SettingsResponseDTO(); dto.setId(entity.getId()); dto.setName(entity.getName()); + dto.setDescription(option.getDescription()); + dto.setCategory(option.getCategory()); + dto.setType(option.getType()); dto.setValue(entity.getValue()); return dto; } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsCreateDTO.java b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsDTO.java similarity index 81% rename from server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsCreateDTO.java rename to server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsDTO.java index ab9d2c2c2f..c9d4840e1d 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsCreateDTO.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsDTO.java @@ -3,21 +3,19 @@ import io.micronaut.core.annotation.Introspected; import io.swagger.v3.oas.annotations.media.Schema; import jakarta.validation.constraints.NotBlank; -import jakarta.validation.constraints.NotNull; import lombok.Getter; import lombok.Setter; @Setter @Getter @Introspected -public class SettingsCreateDTO { +public class SettingsDTO { - @NotNull @NotBlank @Schema(description = "name of the setting") private String name; - @NotNull + @NotBlank @Schema(description = "value of the setting") private String value; diff --git a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsRepository.java b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsRepository.java index 70cb712fdc..32a8c687a4 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsRepository.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsRepository.java @@ -6,6 +6,7 @@ import io.micronaut.data.repository.CrudRepository; import java.util.List; +import java.util.Optional; import java.util.UUID; @JdbcRepository(dialect = Dialect.POSTGRES) @@ -18,5 +19,5 @@ public interface SettingsRepository extends CrudRepository { @NonNull List findAll(); - @NonNull List findByName(@NonNull String name); + Optional findByName(@NonNull String name); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsResponseDTO.java b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsResponseDTO.java index fce875d998..1668ef9d94 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsResponseDTO.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsResponseDTO.java @@ -18,12 +18,22 @@ public class SettingsResponseDTO { @Schema(description = "id of the setting") private UUID id; - @NotNull @NotBlank @Schema(description = "name of the setting") private String name; - + + @NotBlank + @Schema(description = "description for the setting") + private String description; + @NotNull + @Schema(description = "category of the setting") + private SettingOption.Category category; + + @NotNull + @Schema(description = "type of the setting") + private SettingOption.Type type; + @NotBlank @Schema(description = "value of the setting") private String value; diff --git a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsServices.java b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsServices.java index 176967f935..97f765c36f 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsServices.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsServices.java @@ -1,6 +1,6 @@ package com.objectcomputing.checkins.services.settings; -import io.micrometer.context.Nullable; +import jakarta.validation.constraints.NotNull; import java.util.List; import java.util.UUID; @@ -9,9 +9,11 @@ public interface SettingsServices { Setting save(Setting setting); - Setting update(Setting setting); + Setting update(String name, String value); - List findByName(@Nullable String name); + Setting findByName(@NotNull String name); + + List findAllSettings(); Boolean delete(UUID id); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsServicesImpl.java index 56292e0d50..e4bc67e849 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsServicesImpl.java @@ -2,14 +2,11 @@ import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; -import io.micronaut.core.annotation.Nullable; import jakarta.inject.Singleton; import jakarta.validation.constraints.NotNull; -import java.util.ArrayList; import java.util.List; import java.util.UUID; -import java.util.stream.Collectors; @Singleton public class SettingsServicesImpl implements SettingsServices { @@ -24,34 +21,25 @@ public Setting save(Setting setting) { if (setting.getId() != null) { throw new BadArgException("Setting ID is autogenerated by the server upon creation, and should not be provided."); } - validateSettingOption(setting); + validateSettingOption(setting.getName()); return settingsRepository.save(setting); } - public Setting update(Setting setting) { - validateSettingOption(setting); - if ((settingsRepository.existsByIdAndName(setting.getId(), setting.getName()))) { - return settingsRepository.update(setting); - } else { - throw new BadArgException(String.format("Setting %s does not exist, cannot update", setting.getId())); - } + public Setting update(String name, String value) { + validateSettingOption(name); + Setting setting = settingsRepository.findByName(name) + .orElseThrow(() -> new NotFoundException("Setting with name " + name + " not found.")); + setting.setValue(value); + return settingsRepository.update(setting); } - public List findByName(@Nullable String name) { - List searchResult = name == null ? settingsRepository.findAll() : settingsRepository.findByName(name); - return settingToSettingResponseDTO(searchResult); + public Setting findByName(@NotNull String name) { + return settingsRepository.findByName(name) + .orElseThrow(() -> new NotFoundException("Setting with name " + name + " not found.")); } - public List settingToSettingResponseDTO(List settings) { - List settingResponseDTOs = new ArrayList<>(); - for(Setting setting: settings) { - SettingsResponseDTO dto = new SettingsResponseDTO(); - dto.setId(setting.getId()); - dto.setName(setting.getName()); - dto.setValue(setting.getValue()); - settingResponseDTOs.add(dto); - } - return settingResponseDTOs; + public List findAllSettings() { + return settingsRepository.findAll(); } public Boolean delete(@NotNull UUID id) { @@ -62,8 +50,8 @@ public Boolean delete(@NotNull UUID id) { return true; } - private void validateSettingOption(Setting setting) { - if(!SettingOption.isValidOption(setting.getName())){ + private void validateSettingOption(String name) { + if(!SettingOption.isValidOption(name)){ throw new BadArgException("Provided setting name is invalid."); } } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsUpdateDTO.java b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsUpdateDTO.java deleted file mode 100644 index 54b2183d6d..0000000000 --- a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsUpdateDTO.java +++ /dev/null @@ -1,31 +0,0 @@ -package com.objectcomputing.checkins.services.settings; - -import io.micronaut.core.annotation.Introspected; -import io.swagger.v3.oas.annotations.media.Schema; -import jakarta.validation.constraints.NotBlank; -import jakarta.validation.constraints.NotNull; -import lombok.Getter; -import lombok.Setter; - -import java.util.UUID; - -@Setter -@Getter -@Introspected -public class SettingsUpdateDTO{ - - @NotNull - @Schema(description = "id of the setting") - private UUID id; - - @NotNull - @NotBlank - @Schema(description = "name of the setting") - private String name; - - @NotNull - @NotBlank - @Schema(description = "value of the setting") - private String value; - -} diff --git a/server/src/main/resources/db/common/V104__unique_constraint_on_setting_name.sql b/server/src/main/resources/db/common/V104__unique_constraint_on_setting_name.sql new file mode 100644 index 0000000000..f4a6be9cac --- /dev/null +++ b/server/src/main/resources/db/common/V104__unique_constraint_on_setting_name.sql @@ -0,0 +1 @@ +ALTER TABLE settings ADD CONSTRAINT unique_name UNIQUE (name); diff --git a/server/src/test/java/com/objectcomputing/checkins/services/settings/SettingsControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/settings/SettingsControllerTest.java index d3bbb09f14..692cdcfbe9 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/settings/SettingsControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/settings/SettingsControllerTest.java @@ -16,9 +16,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import java.net.URLEncoder; -import java.nio.charset.StandardCharsets; -import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.UUID; @@ -32,10 +29,6 @@ class SettingsControllerTest extends TestContainersSuite implements RoleFixture, @Inject @Client("/services/settings") private HttpClient client; - - private String encodeValue(String value) { - return URLEncoder.encode(value, StandardCharsets.UTF_8); - } @BeforeEach void createRolesAndPermissions() { @@ -55,11 +48,11 @@ void testGetAllSettingsUnauthorized() { @Test void testPostUnauthorized() { - SettingsCreateDTO newSetting = new SettingsCreateDTO(); + SettingsDTO newSetting = new SettingsDTO(); newSetting.setName("Mode"); newSetting.setValue("Light"); - final HttpRequest request = HttpRequest. + final HttpRequest request = HttpRequest. POST("/", newSetting); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, @@ -106,57 +99,72 @@ void testGETFindByValidName() { final MemberProfile alice = getMemberProfileRepository().save(mkMemberProfile("Alice")); Setting setting = createADefaultSetting(); - final HttpRequest request = HttpRequest.GET(String.format("/?name=%s", setting.getName())) + final HttpRequest request = HttpRequest.GET("/" + setting.getName()) .basicAuth(alice.getWorkEmail(), MEMBER_ROLE); - HttpResponse> response = client.toBlocking() - .exchange(request, Argument.listOf(SettingsResponseDTO.class)); + HttpResponse response = client.toBlocking() + .exchange(request, SettingsResponseDTO.class); assertEquals(HttpStatus.OK, response.getStatus()); - assertEquals(response.body().get(0).getId(), setting.getId()); - assertEquals(response.body().size(), 1); + assertEquals(response.body().getId(), setting.getId()); + assertEquals(response.body().getName(), setting.getName()); + assertEquals(response.body().getValue(), setting.getValue()); + } + + @Test + void testPUTValidDTOButSettingNotFound() { + final MemberProfile lucy = getMemberProfileRepository().save(mkMemberProfile("Lucy")); + SettingsDTO updatedSetting = new SettingsDTO(); + updatedSetting.setName(SettingOption.LOGO_URL.toString()); + updatedSetting.setValue("Missing"); + final HttpRequest request = HttpRequest. + PUT("/", updatedSetting).basicAuth(lucy.getWorkEmail(),ADMIN_ROLE); + HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, + () -> client.toBlocking().exchange(request, Map.class)); + assertEquals(HttpStatus.NOT_FOUND, responseException.getStatus()); + assertEquals("Setting with name %s not found.".formatted(SettingOption.LOGO_URL.toString()), responseException.getMessage()); } @Test - void testGETFindByWrongNameReturnsEmptyBody() { + void testGETFindByWrongNameThrowsException() { final MemberProfile alice = getMemberProfileRepository().save(mkMemberProfile("Alice")); Setting setting = createADefaultSetting(); - final HttpRequest request = HttpRequest.GET(String.format("/?name=%s", encodeValue("random"))) + final HttpRequest request = HttpRequest.GET("/random") .basicAuth(alice.getWorkEmail(), MEMBER_ROLE); - HttpResponse> response = client.toBlocking() - .exchange(request, Argument.listOf(SettingsResponseDTO.class)); + HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, + () -> client.toBlocking().exchange(request, Map.class)); - assertEquals(HttpStatus.OK, response.getStatus()); - assertNotNull(response.body()); - assertEquals(response.body(), new ArrayList<>()); + assertNotNull(responseException); + assertEquals(HttpStatus.NOT_FOUND, responseException.getStatus()); } @Test void testPOSTCreateASetting() { final MemberProfile alice = getMemberProfileRepository().save(mkMemberProfile("Alice")); - SettingsCreateDTO newSetting = new SettingsCreateDTO(); + SettingsDTO newSetting = new SettingsDTO(); newSetting.setName(SettingOption.LOGO_URL.name()); newSetting.setValue("Light"); - final HttpRequest request = HttpRequest. + final HttpRequest request = HttpRequest. POST("/", newSetting).basicAuth(alice.getWorkEmail(), ADMIN_ROLE); final HttpResponse response = client.toBlocking().exchange(request,SettingsResponseDTO.class); assertNotNull(response); assertEquals(HttpStatus.CREATED,response.getStatus()); assertEquals(newSetting.getName(), response.body().getName()); + assertEquals(newSetting.getValue(), response.body().getValue()); } @Test void testPostNullName() { final MemberProfile alice = getMemberProfileRepository().save(mkMemberProfile("Alice")); - SettingsCreateDTO newSetting = new SettingsCreateDTO(); + SettingsDTO newSetting = new SettingsDTO(); newSetting.setValue("value"); - final HttpRequest request = HttpRequest. + final HttpRequest request = HttpRequest. POST("/", newSetting).basicAuth(alice.getWorkEmail(), ADMIN_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, @@ -170,9 +178,9 @@ void testPostNullName() { void testPostNullValue() { final MemberProfile alice = getMemberProfileRepository().save(mkMemberProfile("Alice")); - SettingsCreateDTO newSetting = new SettingsCreateDTO(); + SettingsDTO newSetting = new SettingsDTO(); newSetting.setName(SettingOption.LOGO_URL.name()); - final HttpRequest request = HttpRequest. + final HttpRequest request = HttpRequest. POST("/", newSetting).basicAuth(alice.getWorkEmail(), ADMIN_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, @@ -187,17 +195,17 @@ void testPUTSuccessfulUpdate() { final MemberProfile lucy = getMemberProfileRepository().save(mkMemberProfile("Lucy")); Setting settingToUpdate = createADefaultSetting(); - SettingsUpdateDTO updatedSetting = new SettingsUpdateDTO(); + SettingsDTO updatedSetting = new SettingsDTO(); updatedSetting.setValue("off"); - updatedSetting.setId(settingToUpdate.getId()); updatedSetting.setName(settingToUpdate.getName()); - final HttpRequest request = HttpRequest. + final HttpRequest request = HttpRequest. PUT("/", updatedSetting).basicAuth(lucy.getWorkEmail(),ADMIN_ROLE); final HttpResponse response = client.toBlocking().exchange(request, SettingsResponseDTO.class); assertEquals(HttpStatus.OK, response.getStatus()); - assertEquals(String.format("%s/%s", request.getPath(), updatedSetting.getId()), + assertEquals(updatedSetting.getValue(), response.body().getValue()); + assertEquals(String.format("%s/%s", request.getPath(), settingToUpdate.getId()), response.getHeaders().get("location")); } @@ -206,12 +214,11 @@ void testPUTBadName() { final MemberProfile lucy = getMemberProfileRepository().save(mkMemberProfile("Lucy")); Setting settingToUpdate = createADefaultSetting(); - SettingsUpdateDTO updatedSetting = new SettingsUpdateDTO(); + SettingsDTO updatedSetting = new SettingsDTO(); updatedSetting.setValue("off"); - updatedSetting.setId(UUID.randomUUID()); updatedSetting.setName("wrong"); - final HttpRequest request = HttpRequest. + final HttpRequest request = HttpRequest. PUT("/", updatedSetting).basicAuth(lucy.getWorkEmail(),ADMIN_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, @@ -225,9 +232,9 @@ void testPUTBadName() { void testPUTNoIdOrName() { final MemberProfile lucy = getMemberProfileRepository().save(mkMemberProfile("Lucy")); - SettingsUpdateDTO updatedSetting = new SettingsUpdateDTO(); + SettingsDTO updatedSetting = new SettingsDTO(); - final HttpRequest request = HttpRequest. + final HttpRequest request = HttpRequest. PUT("/", updatedSetting).basicAuth(lucy.getWorkEmail(),ADMIN_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, @@ -283,4 +290,15 @@ void testDELETESettingNoPermission() { assertEquals(HttpStatus.UNAUTHORIZED,responseException.getStatus()); } + + @Test + void testUniqueConstraint() { + Setting setting1 = getSettingsRepository().save(new Setting(SettingOption.LOGO_URL.toString(), "url.com")); + try{ + Setting setting2 = getSettingsRepository().save(new Setting(SettingOption.LOGO_URL.toString(), "url2.com")); + } catch (Exception e) { + assertTrue(e.getMessage().contains("duplicate key value violates unique constraint")); + } + } + }