diff --git a/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfigDataTest.java b/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfigDataTest.java index d4e299a06..84360c562 100644 --- a/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfigDataTest.java +++ b/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfigDataTest.java @@ -88,7 +88,7 @@ class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { .andExpect(jsonPath("$.config.polling.sleep", equalTo("00:01:00"))) .andExpect(jsonPath("$._links.configData.href", equalTo( "http://localhost/" + AccessContext.tenant() + "/controller/v1/4712/configData"))); - Thread.sleep(1); // is required: otherwise processing the next line is + waitMillis(1); // is required: otherwise processing the next line is // often too fast and // the following assert will fail assertThat(targetManagement.getByControllerId("4712") .getLastTargetQuery()) diff --git a/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DosFilterTest.java b/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DosFilterTest.java index d255764b2..6246cd1fd 100644 --- a/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DosFilterTest.java +++ b/hawkbit-ddi/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DosFilterTest.java @@ -33,12 +33,11 @@ import org.springframework.web.context.WebApplicationContext; /** * Test potential DOS attack scenarios and check if the filter prevents them. - */ -@ActiveProfiles({ "test" }) -/** + *

* Feature: Component Tests - REST Security
* Story: Denial of Service protection filter */ +@ActiveProfiles({ "test" }) class DosFilterTest extends AbstractDDiApiIntegrationTest { private static final String X_FORWARDED_FOR = HawkbitSecurityProperties.Clients.X_FORWARDED_FOR; @@ -113,7 +112,7 @@ class DosFilterTest extends AbstractDDiApiIntegrationTest { void acceptableGetLoad() throws Exception { for (int x = 0; x < 3; x++) { // sleep for one second - Thread.sleep(1100); + waitMillis(1100); for (int i = 0; i < 9; i++) { mvc.perform(get("/{tenant}/controller/v1/4711", AccessContext.tenant()) .header(X_FORWARDED_FOR, "10.0.0.1")) @@ -159,7 +158,7 @@ class DosFilterTest extends AbstractDDiApiIntegrationTest { for (int x = 0; x < 5; x++) { // sleep for one second - Thread.sleep(1100); + waitMillis(1100); for (int i = 0; i < 9; i++) { mvc.perform(post("/{tenant}/controller/v1/4711/deploymentBase/" + actionId + "/feedback", diff --git a/hawkbit-mgmt/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/rest/api/MgmtTenantManagementRestApi.java b/hawkbit-mgmt/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/rest/api/MgmtTenantManagementRestApi.java index ac7b1210d..46a43e8ae 100644 --- a/hawkbit-mgmt/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/rest/api/MgmtTenantManagementRestApi.java +++ b/hawkbit-mgmt/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/rest/api/MgmtTenantManagementRestApi.java @@ -11,8 +11,6 @@ package org.eclipse.hawkbit.mgmt.rest.api; import static org.eclipse.hawkbit.mgmt.rest.api.MgmtRestConstants.TENANT_ORDER; -import java.io.Serializable; -import java.util.List; import java.util.Map; import io.swagger.v3.oas.annotations.Operation; @@ -28,6 +26,7 @@ import org.eclipse.hawkbit.mgmt.json.model.system.MgmtSystemTenantConfigurationV import org.eclipse.hawkbit.rest.OpenApi; import org.eclipse.hawkbit.rest.json.model.ExceptionInfo; import org.springframework.hateoas.MediaTypes; +import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.DeleteMapping; @@ -35,6 +34,7 @@ import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.ResponseStatus; /** * REST Resource for handling tenant specific configuration operations. @@ -74,38 +74,6 @@ public interface MgmtTenantManagementRestApi { produces = { MediaTypes.HAL_JSON_VALUE, MediaType.APPLICATION_JSON_VALUE }) ResponseEntity> getTenantConfiguration(); - /** - * Handles the DELETE request of deleting a tenant specific configuration value. - * - * @param keyName the Name of the configuration key - * @return if the given configuration value exists and could be deleted HTTP OK. In any failure the JsonResponseExceptionHandler is handling - * the response. - */ - @Operation(summary = "Delete a tenant specific configuration value", description = "The DELETE request removes a " + - "tenant specific configuration value for the tenant. Afterwards the global default value is used. " + - "Required Permission: TENANT_CONFIGURATION") - @ApiResponses(value = { - @ApiResponse(responseCode = "204", description = "Successfully deleted"), - @ApiResponse(responseCode = "400", description = "Bad Request - e.g. invalid parameters", - content = @Content(mediaType = "application/json", schema = @Schema(implementation = ExceptionInfo.class))), - @ApiResponse(responseCode = "401", description = "The request requires user auth.", - content = @Content(mediaType = "application/json", schema = @Schema(hidden = true))), - @ApiResponse(responseCode = "403", - description = "Insufficient permissions, entity is not allowed to be changed (i.e. read-only) or " + - "data volume restriction applies.", - content = @Content(mediaType = "application/json", schema = @Schema(hidden = true))), - @ApiResponse(responseCode = "405", description = "The http request method is not allowed on the resource.", - content = @Content(mediaType = "application/json", schema = @Schema(hidden = true))), - @ApiResponse(responseCode = "406", description = "In case accept header is specified and not application/json.", - content = @Content(mediaType = "application/json", schema = @Schema(hidden = true))), - @ApiResponse(responseCode = "429", description = "Too many requests. The server will refuse further attempts " + - "and the client has to wait another second.", - content = @Content(mediaType = "application/json", schema = @Schema(hidden = true))) - }) - @DeleteMapping(value = MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", - produces = { MediaTypes.HAL_JSON_VALUE, MediaType.APPLICATION_JSON_VALUE }) - ResponseEntity deleteTenantConfigurationValue(@PathVariable("keyName") String keyName); - /** * Handles the GET request of receiving a tenant specific configuration value. * @@ -138,16 +106,13 @@ public interface MgmtTenantManagementRestApi { }) @GetMapping(value = MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", produces = { MediaTypes.HAL_JSON_VALUE, MediaType.APPLICATION_JSON_VALUE }) - ResponseEntity getTenantConfigurationValue( - @PathVariable("keyName") String keyName); + ResponseEntity getTenantConfigurationValue(@PathVariable("keyName") String keyName); /** * Handles the PUT request for updating a tenant specific configuration value. * * @param keyName the name of the configuration key * @param configurationValueRest the new value for the configuration - * @return if the given configuration value exists and could be get HTTP OK. In any failure the JsonResponseExceptionHandler is handling the - * response. */ @Operation(summary = "Update a tenant specific configuration value.", description = "The PUT request changes a " + "configuration value of a specific configuration key for the tenant. " + @@ -181,7 +146,8 @@ public interface MgmtTenantManagementRestApi { @PutMapping(value = MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", consumes = { MediaTypes.HAL_JSON_VALUE, MediaType.APPLICATION_JSON_VALUE }, produces = { MediaTypes.HAL_JSON_VALUE, MediaType.APPLICATION_JSON_VALUE }) - ResponseEntity updateTenantConfigurationValue( + @ResponseStatus(HttpStatus.NO_CONTENT) + void updateTenantConfigurationValue( @PathVariable("keyName") String keyName, @RequestBody MgmtSystemTenantConfigurationValueRequest configurationValueRest); @@ -189,8 +155,6 @@ public interface MgmtTenantManagementRestApi { * Handles the PUT request for updating a batch of tenant specific configurations * * @param configurationValueMap a Map of name - value pairs for the configurations - * @return if the given configurations values exists and could be get HTTP OK. In any failure the JsonResponseExceptionHandler is handling - * the response. */ @Operation(summary = "Batch update of tenant configuration.", description = "The PUT request updates the whole " + "configuration for the tenant. Required Permission: TENANT_CONFIGURATION") @@ -221,6 +185,39 @@ public interface MgmtTenantManagementRestApi { @PutMapping(value = MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs", consumes = { MediaTypes.HAL_JSON_VALUE, MediaType.APPLICATION_JSON_VALUE }, produces = { MediaTypes.HAL_JSON_VALUE, MediaType.APPLICATION_JSON_VALUE }) - ResponseEntity> updateTenantConfiguration( - @RequestBody Map configurationValueMap); + @ResponseStatus(HttpStatus.NO_CONTENT) + void updateTenantConfiguration(@RequestBody Map configurationValueMap); + + /** + * Handles the DELETE request of deleting a tenant specific configuration value. + * + * @param keyName the Name of the configuration key + */ + @Operation(summary = "Delete a tenant specific configuration value", description = "The DELETE request removes a " + + "tenant specific configuration value for the tenant. Afterwards the global default value is used. " + + "Required Permission: TENANT_CONFIGURATION") + @ApiResponses(value = { + @ApiResponse(responseCode = "204", description = "Successfully deleted"), + @ApiResponse(responseCode = "400", description = "Bad Request - e.g. invalid parameters", + content = @Content(mediaType = "application/json", schema = @Schema(implementation = ExceptionInfo.class))), + @ApiResponse(responseCode = "401", description = "The request requires user auth.", + content = @Content(mediaType = "application/json", schema = @Schema(hidden = true))), + @ApiResponse(responseCode = "403", + description = "Insufficient permissions, entity is not allowed to be changed (i.e. read-only) or " + + "data volume restriction applies.", + content = @Content(mediaType = "application/json", schema = @Schema(hidden = true))), + @ApiResponse(responseCode = "404", description = "The key to remove is not found.", + content = @Content(mediaType = "application/json", schema = @Schema(hidden = true))), + @ApiResponse(responseCode = "405", description = "The http request method is not allowed on the resource.", + content = @Content(mediaType = "application/json", schema = @Schema(hidden = true))), + @ApiResponse(responseCode = "406", description = "In case accept header is specified and not application/json.", + content = @Content(mediaType = "application/json", schema = @Schema(hidden = true))), + @ApiResponse(responseCode = "429", description = "Too many requests. The server will refuse further attempts " + + "and the client has to wait another second.", + content = @Content(mediaType = "application/json", schema = @Schema(hidden = true))) + }) + @DeleteMapping(value = MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", + produces = { MediaTypes.HAL_JSON_VALUE, MediaType.APPLICATION_JSON_VALUE }) + @ResponseStatus(HttpStatus.NO_CONTENT) + void deleteTenantConfigurationValue(@PathVariable("keyName") String keyName); } \ No newline at end of file diff --git a/hawkbit-mgmt/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java b/hawkbit-mgmt/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java index 76ce38cc0..8aac6a1ef 100644 --- a/hawkbit-mgmt/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java +++ b/hawkbit-mgmt/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java @@ -9,9 +9,7 @@ */ package org.eclipse.hawkbit.mgmt.rest.resource; -import java.io.Serializable; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Objects; @@ -22,10 +20,10 @@ import org.eclipse.hawkbit.mgmt.json.model.system.MgmtSystemTenantConfigurationV import org.eclipse.hawkbit.mgmt.rest.api.MgmtTenantManagementRestApi; import org.eclipse.hawkbit.mgmt.rest.resource.mapper.MgmtTenantManagementMapper; import org.eclipse.hawkbit.repository.SystemManagement; +import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; import org.eclipse.hawkbit.repository.exception.InsufficientPermissionException; import org.eclipse.hawkbit.repository.exception.TenantConfigurationValidatorException; import org.eclipse.hawkbit.repository.helper.TenantConfigHelper; -import org.eclipse.hawkbit.repository.model.TenantConfigurationValue; import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; @@ -70,20 +68,6 @@ public class MgmtTenantManagementResource implements MgmtTenantManagementRestApi return ResponseEntity.ok(tenantConfigurationValueMap); } - @Override - @AuditLog(entity = "TenantConfiguration", type = AuditLog.Type.DELETE, description = "Delete AccessContext Configuration Value") - public ResponseEntity deleteTenantConfigurationValue(final String keyName) { - // Default DistributionSet Type cannot be deleted as is part of TenantMetadata - if (isDefaultDistributionSetTypeKey(keyName)) { - return ResponseEntity.badRequest().build(); - } - - TenantConfigHelper.getTenantConfigurationManagement().deleteConfiguration(keyName); - - log.debug("{} config value deleted, return status {}", keyName, HttpStatus.OK); - return ResponseEntity.noContent().build(); - } - @Override public ResponseEntity getTenantConfigurationValue(final String keyName) { return ResponseEntity.ok(loadTenantConfigurationValueBy(keyName)); @@ -91,61 +75,56 @@ public class MgmtTenantManagementResource implements MgmtTenantManagementRestApi @Override @AuditLog(entity = "TenantConfiguration", type = AuditLog.Type.UPDATE, description = "Update AccessContext Configuration Value") - public ResponseEntity updateTenantConfigurationValue( - final String keyName, final MgmtSystemTenantConfigurationValueRequest configurationValueRest) { - Serializable configurationValue = configurationValueRest.getValue(); - final MgmtSystemTenantConfigurationValue responseUpdatedValue; + public void updateTenantConfigurationValue(final String keyName, final MgmtSystemTenantConfigurationValueRequest configurationValueRest) { + final Object configurationValue = configurationValueRest.getValue(); if (isDefaultDistributionSetTypeKey(keyName)) { - responseUpdatedValue = updateDefaultDsType(configurationValue); + updateDefaultDsType(configurationValue); } else { - final TenantConfigurationValue updatedTenantConfigurationValue = TenantConfigHelper + TenantConfigHelper .getTenantConfigurationManagement() .addOrUpdateConfiguration(keyName, configurationValueRest.getValue()); - responseUpdatedValue = MgmtTenantManagementMapper.toResponseTenantConfigurationValue(keyName, updatedTenantConfigurationValue); } - - return ResponseEntity.ok(responseUpdatedValue); } @Override @AuditLog(entity = "TenantConfiguration", type = AuditLog.Type.UPDATE, description = "Update AccessContext Configuration") - public ResponseEntity> updateTenantConfiguration( - final Map configurationValueMap) { + public void updateTenantConfiguration(final Map configurationValueMap) { final boolean containsNull = configurationValueMap.keySet().stream().anyMatch(Objects::isNull); if (containsNull) { - return ResponseEntity.badRequest().build(); + throw new IllegalArgumentException(); } - //Try update TenantMetadata first - final Serializable defaultDsTypeValueUpdate = configurationValueMap.remove(MgmtTenantManagementMapper.DEFAULT_DISTRIBUTION_SET_TYPE_KEY); + // try update TenantMetadata first + final Object defaultDsTypeValueUpdate = configurationValueMap.remove(MgmtTenantManagementMapper.DEFAULT_DISTRIBUTION_SET_TYPE_KEY); Long oldDefaultDsType = null; MgmtSystemTenantConfigurationValue updatedDefaultDsType = null; if (defaultDsTypeValueUpdate != null) { oldDefaultDsType = systemManagement.getTenantMetadata().getDefaultDsType().getId(); updatedDefaultDsType = updateDefaultDsType(defaultDsTypeValueUpdate); } - //try update TenantConfiguration, in case of Error -> rollback TenantMetadata - final Map> tenantConfigurationValues; + // try update TenantConfiguration, in case of Error -> rollback TenantMetadata try { - tenantConfigurationValues = TenantConfigHelper.getTenantConfigurationManagement().addOrUpdateConfiguration(configurationValueMap); + TenantConfigHelper.getTenantConfigurationManagement().addOrUpdateConfiguration(configurationValueMap); } catch (Exception ex) { - //if DefaultDsType was updated, rollback it in case of TenantConfiguration update. + // if DefaultDsType was updated, rollback it in case of TenantConfiguration update. if (updatedDefaultDsType != null) { systemManagement.updateTenantMetadata(oldDefaultDsType); } throw ex; } + } - final List tenantConfigurationListUpdated = new java.util.ArrayList<>( - tenantConfigurationValues.entrySet().stream() - .map(entry -> MgmtTenantManagementMapper.toResponseTenantConfigurationValue(entry.getKey(), entry.getValue())) - .toList()); - if (updatedDefaultDsType != null) { - tenantConfigurationListUpdated.add(updatedDefaultDsType); + @Override + @AuditLog(entity = "TenantConfiguration", type = AuditLog.Type.DELETE, description = "Delete AccessContext Configuration Value") + public void deleteTenantConfigurationValue(final String keyName) { + // Default DistributionSet Type cannot be deleted as is part of TenantMetadata + if (isDefaultDistributionSetTypeKey(keyName)) { + throw new EntityNotFoundException("Configuration key not found", keyName); } - return ResponseEntity.ok(tenantConfigurationListUpdated); + TenantConfigHelper.getTenantConfigurationManagement().deleteConfiguration(keyName); + log.debug("{} config value deleted", keyName); } private static boolean isDefaultDistributionSetTypeKey(String keyName) { @@ -164,7 +143,7 @@ public class MgmtTenantManagementResource implements MgmtTenantManagementRestApi return response; } - private MgmtSystemTenantConfigurationValue updateDefaultDsType(Serializable defaultDsType) { + private MgmtSystemTenantConfigurationValue updateDefaultDsType(final Object defaultDsType) { final long updateDefaultDsType; try { updateDefaultDsType = ((Number) defaultDsType).longValue(); diff --git a/hawkbit-mgmt/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResourceTest.java b/hawkbit-mgmt/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResourceTest.java index 9aa5e6640..a5f19a599 100644 --- a/hawkbit-mgmt/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResourceTest.java +++ b/hawkbit-mgmt/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResourceTest.java @@ -9,6 +9,7 @@ */ package org.eclipse.hawkbit.mgmt.rest.resource; +import static org.eclipse.hawkbit.mgmt.rest.api.MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING; import static org.eclipse.hawkbit.repository.test.util.SecurityContextSwitch.callAs; import static org.eclipse.hawkbit.repository.test.util.SecurityContextSwitch.getAs; import static org.eclipse.hawkbit.repository.test.util.SecurityContextSwitch.withUser; @@ -28,7 +29,6 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. import com.fasterxml.jackson.databind.ObjectMapper; import org.eclipse.hawkbit.auth.SpPermission; import org.eclipse.hawkbit.mgmt.json.model.system.MgmtSystemTenantConfigurationValueRequest; -import org.eclipse.hawkbit.mgmt.rest.api.MgmtRestConstants; import org.eclipse.hawkbit.repository.DistributionSetTypeManagement; import org.eclipse.hawkbit.repository.model.DistributionSetType; import org.eclipse.hawkbit.rest.util.MockMvcResultPrinter; @@ -51,23 +51,22 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg * Handles GET request for receiving all tenant specific configurations. */ @Test - void getTenantConfigurations() throws Exception { - mvc.perform(get(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs")) + void getTenantConfigurations() throws Exception { + mvc.perform(get(SYSTEM_V1_REQUEST_MAPPING + "/configs")) .andDo(MockMvcResultPrinter.print()) .andExpect(status().isOk()) - //check for TenantMetadata additional properties + // check for TenantMetadata additional properties .andExpect(jsonPath("$.['" + DEFAULT_DISTRIBUTION_SET_TYPE_KEY + "']").exists()) .andExpect(jsonPath("$.['" + DEFAULT_DISTRIBUTION_SET_TYPE_KEY + "'].value", equalTo(getActualDefaultDsType().intValue()))); - } /** * Handles GET request for receiving a tenant specific configuration. */ @Test - void getTenantConfiguration() throws Exception { - //Test TenantConfiguration property - mvc.perform(get(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY)) + void getTenantConfiguration() throws Exception { + // test TenantConfiguration property + mvc.perform(get(SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY)) .andDo(MockMvcResultPrinter.print()) .andExpect(status().isOk()); } @@ -76,10 +75,9 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg * Handles GET request for receiving (TenantMetadata - DefaultDsType) a tenant specific configuration. */ @Test - void getTenantMetadata() throws Exception { - //Test TenantMetadata property - mvc.perform(get(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", - DEFAULT_DISTRIBUTION_SET_TYPE_KEY)) + void getTenantMetadata() throws Exception { + // test TenantMetadata property + mvc.perform(get(SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", DEFAULT_DISTRIBUTION_SET_TYPE_KEY)) .andDo(MockMvcResultPrinter.print()) .andExpect(status().isOk()) .andExpect(jsonPath("$.value", equalTo(getActualDefaultDsType().intValue()))); @@ -89,37 +87,37 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg * Handles PUT request for settings values in tenant specific configuration. */ @Test - void putTenantConfiguration() throws Exception { + void putTenantConfiguration() throws Exception { final MgmtSystemTenantConfigurationValueRequest bodyPut = new MgmtSystemTenantConfigurationValueRequest(); bodyPut.setValue("exampleToken"); final ObjectMapper mapper = new ObjectMapper(); final String json = mapper.writeValueAsString(bodyPut); - mvc.perform(put(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", - AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY).content(json) - .contentType(MediaType.APPLICATION_JSON)) + mvc.perform(put(SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY) + .contentType(MediaType.APPLICATION_JSON) + .content(json)) .andDo(MockMvcResultPrinter.print()) - .andExpect(status().isOk()); + .andExpect(status().isNoContent()); } /** * Handles PUT request for settings values (TenantMetadata - DefaultDsType) in tenant specific configuration, which is TenantMetadata */ @Test - void putTenantMetadata() throws Exception { + void putTenantMetadata() throws Exception { final MgmtSystemTenantConfigurationValueRequest bodyPut = new MgmtSystemTenantConfigurationValueRequest(); - long updatedTestDefaultDsType = createTestDistributionSetType(); + final long updatedTestDefaultDsType = createTestDistributionSetType(); bodyPut.setValue(updatedTestDefaultDsType); final ObjectMapper mapper = new ObjectMapper(); final String json = mapper.writeValueAsString(bodyPut); - mvc.perform(put(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", - DEFAULT_DISTRIBUTION_SET_TYPE_KEY).content(json) - .contentType(MediaType.APPLICATION_JSON)) + mvc.perform(put(SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", DEFAULT_DISTRIBUTION_SET_TYPE_KEY) + .contentType(MediaType.APPLICATION_JSON) + .content(json)) .andDo(MockMvcResultPrinter.print()) - .andExpect(status().isOk()); + .andExpect(status().isNoContent()); //check if after Rest success, value is really changed in TenantMetadata assertEquals(updatedTestDefaultDsType, getActualDefaultDsType(), @@ -131,14 +129,14 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg */ @Test void putTenantMetadataFails() throws Exception { - long oldDefaultDsType = getActualDefaultDsType(); - //try an invalid input + final long oldDefaultDsType = getActualDefaultDsType(); + // try an invalid input String newDefaultDsType = new JSONObject().put("value", true).toString(); assertDefaultDsTypeUpdateBadRequestFails(newDefaultDsType, oldDefaultDsType, status().isBadRequest()); - //try an invalid input + // try an invalid input newDefaultDsType = new JSONObject().put("value", "someInvalidInput").toString(); assertDefaultDsTypeUpdateBadRequestFails(newDefaultDsType, oldDefaultDsType, status().isBadRequest()); - //try valid input, but the given DistributionSetType Id does not exist.. + // try valid input, but the given DistributionSetType Id does not exist.. newDefaultDsType = new JSONObject().put("value", 99999).toString(); assertDefaultDsTypeUpdateBadRequestFails(newDefaultDsType, oldDefaultDsType, status().isNotFound()); } @@ -151,13 +149,15 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg final String bodyActivate = new JSONObject().put("value", true).toString(); final String bodyDeactivate = new JSONObject().put("value", false).toString(); - mvc.perform(put(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", MULTI_ASSIGNMENTS_ENABLED) - .content(bodyActivate).contentType(MediaType.APPLICATION_JSON)) + mvc.perform(put(SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", MULTI_ASSIGNMENTS_ENABLED) + .content(bodyActivate) + .contentType(MediaType.APPLICATION_JSON)) .andDo(MockMvcResultPrinter.print()) - .andExpect(status().isOk()); + .andExpect(status().isNoContent()); - mvc.perform(put(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", MULTI_ASSIGNMENTS_ENABLED) - .content(bodyDeactivate).contentType(MediaType.APPLICATION_JSON)) + mvc.perform(put(SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", MULTI_ASSIGNMENTS_ENABLED) + .content(bodyDeactivate) + .contentType(MediaType.APPLICATION_JSON)) .andDo(MockMvcResultPrinter.print()) .andExpect(status().isForbidden()); } @@ -167,14 +167,15 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg */ @Test void changeBatchConfigurationShouldFailOnInvalidTenantConfiguration() throws Exception { - //in this scenario - // some TenantConfiguration are not valid, - // TenantMetadata - DefaultDSType ID is valid, - //in the end batch configuration update must fail, and thus, not a single config should be actually changed + // in this scenario + // some TenantConfiguration are not valid, + // TenantMetadata - DefaultDSType ID is valid, + // in the end batch configuration update must fail, and thus, not a single config should be actually changed long testValidDistributionSetType = createTestDistributionSetType(); boolean oldRolloutApprovalConfig = (Boolean) tenantConfigurationManagement().getConfigurationValue(ROLLOUT_APPROVAL_ENABLED).getValue(); - String oldAuthGatewayToken = (String) tenantConfigurationManagement().getConfigurationValue(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY).getValue(); - //test TenantConfiguration with invalid config value, and a valid TenantMetadata - Default DistributionSetType id + String oldAuthGatewayToken = (String) tenantConfigurationManagement().getConfigurationValue(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY) + .getValue(); + // test TenantConfiguration with invalid config value, and a valid TenantMetadata - Default DistributionSetType id assertBatchConfigurationFails(!oldRolloutApprovalConfig, "invalid-config-value", oldAuthGatewayToken + "randomSuffix0", testValidDistributionSetType, status().isBadRequest()); } @@ -184,29 +185,31 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg */ @Test void changeBatchConfigurationShouldOnInvalidTenantMetadata() throws Exception { - //in this scenario - // all TenantConfiguration have valid and new values - using old values, inverted - // TenantMetadata - DefaultDSType ID is invalid - //in the end batch configuration update must fail, and thus, not a single config should be actually changed. - boolean oldRolloutApprovalConfig = (Boolean) tenantConfigurationManagement().getConfigurationValue(ROLLOUT_APPROVAL_ENABLED).getValue(); - boolean oldAuthGatewayTokenEnabled = (Boolean) tenantConfigurationManagement() + // in this scenario + // all TenantConfiguration have valid and new values - using old values, inverted + // TenantMetadata - DefaultDSType ID is invalid + // in the end batch configuration update must fail, and thus, not a single config should be actually changed. + final boolean oldRolloutApprovalConfig = (Boolean) tenantConfigurationManagement() + .getConfigurationValue(ROLLOUT_APPROVAL_ENABLED).getValue(); + final boolean oldAuthGatewayTokenEnabled = (Boolean) tenantConfigurationManagement() .getConfigurationValue(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_ENABLED).getValue(); - String oldAuthGatewayToken = (String) tenantConfigurationManagement().getConfigurationValue(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY).getValue(); + final String oldAuthGatewayToken = (String) tenantConfigurationManagement() + .getConfigurationValue(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY).getValue(); - //invalid TenantMetadata Default DistributionSetType, it is expected to be a number. Testing invalid type - string - //not a single configuration should be changed after the failure + // invalid TenantMetadata Default DistributionSetType, it is expected to be a number. Testing invalid type - string + // not a single configuration should be changed after the failure Object testInvalidDistributionSetType = "someInvalidInput"; assertBatchConfigurationFails(!oldRolloutApprovalConfig, !oldAuthGatewayTokenEnabled, oldAuthGatewayToken + "randomSuffix1", testInvalidDistributionSetType, status().isBadRequest()); - //invalid TenantMetadata Default DistributionSetType, it is expected to be a number. Testing invalid type - bool - //not a single configuration should be changed after the failure + // invalid TenantMetadata Default DistributionSetType, it is expected to be a number. Testing invalid type - bool + // not a single configuration should be changed after the failure testInvalidDistributionSetType = true; assertBatchConfigurationFails(!oldRolloutApprovalConfig, !oldAuthGatewayTokenEnabled, oldAuthGatewayToken + "randomSuffix2", testInvalidDistributionSetType, status().isBadRequest()); - //Valid TenantMetadata Default DistributionSetType, it is expected to be a number. Testing valid type - but given DistributionSetType Id does not exist. - //not a single configuration should be changed after the failure + // valid TenantMetadata Default DistributionSetType, it is expected to be a number. Testing valid type - but given DistributionSetType Id does not exist. + // not a single configuration should be changed after the failure testInvalidDistributionSetType = 9999; assertBatchConfigurationFails(!oldRolloutApprovalConfig, !oldAuthGatewayTokenEnabled, oldAuthGatewayToken + "randomSuffix2", testInvalidDistributionSetType, status().isNotFound()); @@ -217,23 +220,23 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg */ @Test void changeBatchConfiguration() throws Exception { - long updatedDistributionSetType = createTestDistributionSetType(); - boolean updatedRolloutApprovalEnabled = true; - boolean updatedAuthGatewayTokenEnabled = true; - String updatedAuthGatewayTokenKey = "54321"; - JSONObject configuration = new JSONObject(); + final long updatedDistributionSetType = createTestDistributionSetType(); + final boolean updatedRolloutApprovalEnabled = true; + final boolean updatedAuthGatewayTokenEnabled = true; + final String updatedAuthGatewayTokenKey = "54321"; + final JSONObject configuration = new JSONObject(); configuration.put(ROLLOUT_APPROVAL_ENABLED, updatedRolloutApprovalEnabled); configuration.put(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_ENABLED, updatedAuthGatewayTokenEnabled); configuration.put(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY, updatedAuthGatewayTokenKey); configuration.put(DEFAULT_DISTRIBUTION_SET_TYPE_KEY, updatedDistributionSetType); - String body = configuration.toString(); + final String body = configuration.toString(); - mvc.perform(put(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs") .content(body).contentType(MediaType.APPLICATION_JSON)) + mvc.perform(put(SYSTEM_V1_REQUEST_MAPPING + "/configs").content(body).contentType(MediaType.APPLICATION_JSON)) .andDo(MockMvcResultPrinter.print()) - .andExpect(status().isOk()); + .andExpect(status().isNoContent()); - //assert all changes were applied after Rest Success + // assert all changes were applied after Rest Success assertEquals(updatedDistributionSetType, getActualDefaultDsType(), "Change BatchConfiguration was successful but TenantMetadata - Default DistributionSetType was not actually changed."); assertEquals(updatedRolloutApprovalEnabled, tenantConfigurationManagement().getConfigurationValue(ROLLOUT_APPROVAL_ENABLED).getValue(), @@ -255,20 +258,23 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg final String bodyDeactivate = new JSONObject().put("value", false).toString(); // enable Multi-Assignments - mvc.perform(put(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", MULTI_ASSIGNMENTS_ENABLED) - .content(bodyActivate).contentType(MediaType.APPLICATION_JSON)) + mvc.perform(put(SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", MULTI_ASSIGNMENTS_ENABLED) + .content(bodyActivate) + .contentType(MediaType.APPLICATION_JSON)) .andDo(MockMvcResultPrinter.print()) - .andExpect(status().isOk()); + .andExpect(status().isNoContent()); // try to enable Auto-Close - mvc.perform(put(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED) - .content(bodyActivate).contentType(MediaType.APPLICATION_JSON)) + mvc.perform(put(SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED) + .content(bodyActivate) + .contentType(MediaType.APPLICATION_JSON)) .andDo(MockMvcResultPrinter.print()) .andExpect(status().isForbidden()); // try to disable Auto-Close - mvc.perform(put(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED) - .content(bodyDeactivate).contentType(MediaType.APPLICATION_JSON)) + mvc.perform(put(SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED) + .content(bodyDeactivate) + .contentType(MediaType.APPLICATION_JSON)) .andDo(MockMvcResultPrinter.print()) .andExpect(status().isForbidden()); } @@ -278,8 +284,7 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg */ @Test void deleteTenantConfiguration() throws Exception { - mvc.perform(delete(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", - AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY)) + mvc.perform(delete(SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY)) .andDo(MockMvcResultPrinter.print()) .andExpect(status().isNoContent()); } @@ -289,10 +294,9 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg */ @Test void deleteTenantMetadataFail() throws Exception { - mvc.perform(delete(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", - DEFAULT_DISTRIBUTION_SET_TYPE_KEY)) + mvc.perform(delete(SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", DEFAULT_DISTRIBUTION_SET_TYPE_KEY)) .andDo(MockMvcResultPrinter.print()) - .andExpect(status().isBadRequest()); + .andExpect(status().isNotFound()); } /** @@ -301,25 +305,24 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg @Test void getTenantConfigurationReadGWToken() throws Exception { getAs(withUser("tenant_admin", SpPermission.TENANT_CONFIGURATION), () -> { - tenantConfigurationManagement().addOrUpdateConfiguration( - AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY, "123"); + tenantConfigurationManagement().addOrUpdateConfiguration(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY, "123"); return null; }); // TODO - should be able to read with TENANT_CONFIGURATION but somehow here the role hierarchy doesn't play // checked in mgmt / update server runtime PreAuthorizeEnabledTest callAs(withUser("tenant_admin", SpPermission.READ_TENANT_CONFIGURATION, SpPermission.READ_GATEWAY_SECURITY_TOKEN), () -> { - mvc.perform(get(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs")) - .andDo(MockMvcResultPrinter.print()) - .andDo(m -> System.out.println("-> 1: " + m.getResponse().getContentAsString())) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.['" + AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY + "']").exists()) - .andExpect(jsonPath("$.['" + AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY + "'].value", equalTo("123"))); - return null; - }); + mvc.perform(get(SYSTEM_V1_REQUEST_MAPPING + "/configs")) + .andDo(MockMvcResultPrinter.print()) + .andDo(m -> System.out.println("-> 1: " + m.getResponse().getContentAsString())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.['" + AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY + "']").exists()) + .andExpect(jsonPath("$.['" + AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY + "'].value", equalTo("123"))); + return null; + }); callAs(withUser("tenant_read", SpPermission.READ_TENANT_CONFIGURATION), () -> { - mvc.perform(get(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs")) + mvc.perform(get(SYSTEM_V1_REQUEST_MAPPING + "/configs")) .andDo(MockMvcResultPrinter.print()) .andDo(m -> System.out.println("-> 2: " + m.getResponse().getContentAsString())) .andExpect(status().isOk()) @@ -329,17 +332,18 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg } private Long createTestDistributionSetType() { - DistributionSetType testDefaultDsType = distributionSetTypeManagement.create(DistributionSetTypeManagement.Create.builder() + final DistributionSetType testDefaultDsType = distributionSetTypeManagement.create(DistributionSetTypeManagement.Create.builder() .key("test123").name("TestName123").description("TestDefaultDsType").build()); - testDefaultDsType = distributionSetTypeManagement - .update(DistributionSetTypeManagement.Update.builder().id(testDefaultDsType.getId()).description("TestDefaultDsType").build()); - return testDefaultDsType.getId(); + return distributionSetTypeManagement + .update(DistributionSetTypeManagement.Update.builder().id(testDefaultDsType.getId()).description("TestDefaultDsType").build()) + .getId(); } - private void assertDefaultDsTypeUpdateBadRequestFails(String newDefaultDsType, long oldDefaultDsType, ResultMatcher resultMatchers) + private void assertDefaultDsTypeUpdateBadRequestFails( + final String newDefaultDsType, final long oldDefaultDsType, final ResultMatcher resultMatchers) throws Exception { - mvc.perform(put(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", - DEFAULT_DISTRIBUTION_SET_TYPE_KEY).content(newDefaultDsType) + mvc.perform(put(SYSTEM_V1_REQUEST_MAPPING + "/configs/{keyName}", DEFAULT_DISTRIBUTION_SET_TYPE_KEY) + .content(newDefaultDsType) .contentType(MediaType.APPLICATION_JSON)) .andDo(MockMvcResultPrinter.print()) .andExpect(resultMatchers); @@ -347,23 +351,25 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg "Rest endpoint for updating DefaultDistributionType failed, but actual value changed unexpectedly."); } - private void assertBatchConfigurationFails(Object newRolloutApprovalEnabled, Object newAuthGatewayTokenEnabled, Object newGatewayToken, - Object newDistributionSetTypeId, ResultMatcher resultMatchers) throws Exception { + private void assertBatchConfigurationFails( + final Object newRolloutApprovalEnabled, final Object newAuthGatewayTokenEnabled, final Object newGatewayToken, + final Object newDistributionSetTypeId, final ResultMatcher resultMatchers) throws Exception { long oldDefaultDsType = getActualDefaultDsType(); boolean oldRolloutApprovalConfig = (Boolean) tenantConfigurationManagement().getConfigurationValue(ROLLOUT_APPROVAL_ENABLED).getValue(); - boolean oldAuthGatewayTokenEnabled = (Boolean) tenantConfigurationManagement().getConfigurationValue(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_ENABLED) + boolean oldAuthGatewayTokenEnabled = (Boolean) tenantConfigurationManagement() + .getConfigurationValue(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_ENABLED) + .getValue(); + String oldAuthGatewayToken = (String) tenantConfigurationManagement().getConfigurationValue(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY) .getValue(); - String oldAuthGatewayToken = (String) tenantConfigurationManagement().getConfigurationValue(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY).getValue(); - JSONObject configuration = new JSONObject(); + final JSONObject configuration = new JSONObject(); configuration.put(ROLLOUT_APPROVAL_ENABLED, newRolloutApprovalEnabled); configuration.put(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_ENABLED, newAuthGatewayTokenEnabled); configuration.put(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY, newGatewayToken); configuration.put(DEFAULT_DISTRIBUTION_SET_TYPE_KEY, newDistributionSetTypeId); String body = configuration.toString(); - mvc.perform(put(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs") - .content(body).contentType(MediaType.APPLICATION_JSON)) + mvc.perform(put(SYSTEM_V1_REQUEST_MAPPING + "/configs").content(body).contentType(MediaType.APPLICATION_JSON)) .andDo(MockMvcResultPrinter.print()) .andExpect(resultMatchers); //Check if TenantMetadata and TenantConfiguration is not changed as Batch config failed @@ -374,7 +380,8 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg assertEquals(oldAuthGatewayTokenEnabled, tenantConfigurationManagement().getConfigurationValue(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_ENABLED).getValue(), "Batch configuration update Failed, but TenantConfiguration was actually changed."); - assertEquals(oldAuthGatewayToken, tenantConfigurationManagement().getConfigurationValue(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY).getValue(), + assertEquals(oldAuthGatewayToken, + tenantConfigurationManagement().getConfigurationValue(AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY).getValue(), "Batch configuration update Failed, but TenantConfiguration was actually changed."); } diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TenantConfigurationManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TenantConfigurationManagement.java index a06ea0f0b..38633ff56 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TenantConfigurationManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TenantConfigurationManagement.java @@ -9,7 +9,6 @@ */ package org.eclipse.hawkbit.repository; -import java.io.Serializable; import java.util.Map; import java.util.function.Function; @@ -38,25 +37,23 @@ public interface TenantConfigurationManagement extends PermissionSupport { * * @param keyName the key of the configuration * @param value the configuration value which will be written into the database. - * @return the configuration value which was just written into the database. * @throws TenantConfigurationValidatorException if the {@code propertyType} and the value in general does not match the expected type and * format defined by the Key * @throws ConversionFailedException if the property cannot be converted to the given */ @PreAuthorize(value = SpringEvalExpressions.HAS_UPDATE_REPOSITORY) - TenantConfigurationValue addOrUpdateConfiguration(String keyName, T value); + void addOrUpdateConfiguration(String keyName, Object value); /** * Adds or updates a specific configuration for a specific tenant. * * @param configurations map containing the key - value of the configuration - * @return map of all configuration values which were written into the database. * @throws TenantConfigurationValidatorException if the {@code propertyType} and the value in general does not * match the expected type and format defined by the Key * @throws ConversionFailedException if the property cannot be converted to the given */ @PreAuthorize(value = SpringEvalExpressions.HAS_UPDATE_REPOSITORY) - Map> addOrUpdateConfiguration(Map configurations); + void addOrUpdateConfiguration(Map configurations); /** * Retrieves a configuration value from the e.g. tenant overwritten configuration values or in case the tenant does not a have a specific @@ -70,7 +67,7 @@ public interface TenantConfigurationManagement extends PermissionSupport { * @throws ConversionFailedException if the property cannot be converted to the given {@code propertyType} */ @PreAuthorize(value = SpringEvalExpressions.HAS_READ_REPOSITORY) - TenantConfigurationValue getConfigurationValue(String keyName); + TenantConfigurationValue getConfigurationValue(String keyName); /** * Retrieves a configuration value from the e.g. tenant overwritten configuration values or in case the tenant does not a have a specific @@ -86,7 +83,7 @@ public interface TenantConfigurationManagement extends PermissionSupport { * @throws ConversionFailedException if the property cannot be converted to the given {@code propertyType} */ @PreAuthorize(value = SpringEvalExpressions.HAS_READ_REPOSITORY) - TenantConfigurationValue getConfigurationValue(String keyName, Class propertyType); + TenantConfigurationValue getConfigurationValue(String keyName, Class propertyType); /** * Deletes a specific configuration for the current tenant. Does nothing in case there is no tenant specific configuration value. diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/TenantConfigurationValue.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/TenantConfigurationValue.java index 527e6e22a..4e38a5e1b 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/TenantConfigurationValue.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/TenantConfigurationValue.java @@ -9,9 +9,6 @@ */ package org.eclipse.hawkbit.repository.model; -import java.io.Serial; -import java.io.Serializable; - import lombok.Builder; import lombok.Data; @@ -22,10 +19,7 @@ import lombok.Data; */ @Data @Builder -public final class TenantConfigurationValue implements Serializable { - - @Serial - private static final long serialVersionUID = 1L; +public final class TenantConfigurationValue { private T value; private Long lastModifiedAt; diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/aspects/ExceptionMappingAspectHandler.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/aspects/ExceptionMappingAspectHandler.java index 78a95a750..11925b5bf 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/aspects/ExceptionMappingAspectHandler.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/aspects/ExceptionMappingAspectHandler.java @@ -27,17 +27,17 @@ import org.springframework.core.Ordered; public class ExceptionMappingAspectHandler implements Ordered { /** - * Catches exceptions of the {@link TransactionManager} and wrap them to custom exceptions. + * Catches exceptions the {@link TransactionManager} and wrap them to custom exceptions. * - * @param ex the thrown and catched exception - * @throws Throwable + * @param e the thrown and caught exception + * @throws Throwable the mapped exception */ - @AfterThrowing(pointcut = "execution( * org.eclipse.hawkbit.repository.jpa.management.*Management.*(..))", throwing = "ex") + @AfterThrowing(pointcut = "execution( * org.eclipse.hawkbit.repository.jpa.management.*Management.*(..))", throwing = "e") // Exception for squid:S00112, squid:S1162 // It is a AspectJ proxy which deals with exceptions. @SuppressWarnings({ "squid:S00112", "squid:S1162" }) - public void catchAndWrapJpaExceptionsService(final Exception ex) throws Throwable { - throw ExceptionMapper.map(ex); + public void catchAndWrapJpaExceptionsService(final Exception e) throws Throwable { + throw ExceptionMapper.map(e); } @Override diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTenantConfigurationManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTenantConfigurationManagement.java index ef031f34d..a0511d0f3 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTenantConfigurationManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTenantConfigurationManagement.java @@ -16,19 +16,17 @@ import static org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationPrope import static org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey.POLLING_TIME; import static org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey.REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED; -import java.io.Serializable; import java.time.Duration; import java.time.Instant; import java.time.LocalDateTime; import java.time.ZoneId; import java.time.temporal.ChronoUnit; -import java.util.ArrayList; -import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.function.Function; -import java.util.stream.Collectors; +import lombok.NonNull; import lombok.extern.slf4j.Slf4j; import org.eclipse.hawkbit.auth.SpPermission; import org.eclipse.hawkbit.context.AccessContext; @@ -95,7 +93,7 @@ public class JpaTenantConfigurationManagement implements TenantConfigurationMana } @Override - public void onApplicationEvent(final ContextRefreshedEvent event) { + public void onApplicationEvent(@NonNull final ContextRefreshedEvent event) { // Sets the proxy / bean from the context in order to be used via proxy and onore things like @PreAuthorize and @Transactional TenantConfigHelper.setTenantConfigurationManagement(applicationContext.getBean(JpaTenantConfigurationManagement.class)); } @@ -104,25 +102,25 @@ public class JpaTenantConfigurationManagement implements TenantConfigurationMana @Transactional @Retryable(retryFor = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) - public TenantConfigurationValue addOrUpdateConfiguration(final String keyName, final T value) { - return addOrUpdateConfiguration0(Map.of(keyName, value)).values().iterator().next(); + public void addOrUpdateConfiguration(final String keyName, final Object value) { + addOrUpdateConfiguration0(Map.of(keyName, value)); } @Override @Transactional @Retryable(retryFor = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) - public Map> addOrUpdateConfiguration(final Map configurations) { - return addOrUpdateConfiguration0(configurations); + public void addOrUpdateConfiguration(final Map configurations) { + addOrUpdateConfiguration0(configurations); } @Override - public TenantConfigurationValue getConfigurationValue(final String keyName) { + public TenantConfigurationValue getConfigurationValue(final String keyName) { return getConfigurationValue0(keyName, null); } @Override - public TenantConfigurationValue getConfigurationValue(final String keyName, final Class propertyType) { + public TenantConfigurationValue getConfigurationValue(final String keyName, final Class propertyType) { return getConfigurationValue0(keyName, propertyType); } @@ -178,56 +176,47 @@ public class JpaTenantConfigurationManagement implements TenantConfigurationMana } private void checkAccess(final String keyName) { - if (AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY.equalsIgnoreCase(keyName)) { - if (!AccessContext.isCurrentThreadSystemCode() && !SpPermission.hasPermission(READ_GATEWAY_SECURITY_TOKEN)) { - throw new InsufficientPermissionException( - "Can't read gateway security token! " + READ_GATEWAY_SECURITY_TOKEN + " is required!"); - } + if (AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY.equalsIgnoreCase(keyName) + && !AccessContext.isCurrentThreadSystemCode() && !SpPermission.hasPermission(READ_GATEWAY_SECURITY_TOKEN)) { + throw new InsufficientPermissionException("Can't read gateway security token! " + READ_GATEWAY_SECURITY_TOKEN + " is required!"); } } - @SuppressWarnings("unchecked") - private Map> addOrUpdateConfiguration0(final Map configurations) { - final List configurationList = new ArrayList<>(); - configurations.forEach((keyName, value) -> { - final TenantConfigurationKey configurationKey = tenantConfigurationProperties.fromKeyName(keyName); + private void addOrUpdateConfiguration0(final Map configurations) { + tenantConfigurationRepository.saveAll(configurations.entrySet().stream().map(e -> { + final TenantConfigurationKey configurationKey = tenantConfigurationProperties.fromKeyName(e.getKey()); final Class targetType = configurationKey.getDataType(); - Object convertedValue = getConvertedValue(value, targetType); - validateConfigurationValue(value, configurationKey, convertedValue); + final Object convertedValue; + try { + convertedValue = CONVERSION_SERVICE.convert(e.getValue(), targetType); + } catch (final ConversionException | IllegalArgumentException ex) { + throw new TenantConfigurationValidatorException(String.format( + "Cannot convert the value %s of type %s to the type %s defined by the configuration key.", + e.getValue(), e.getValue().getClass(), targetType)); + } + final String valueToString = Optional.ofNullable(convertedValue) + .map(Object::toString) + .orElse(null); + validateConfigurationValue(configurationKey, convertedValue); JpaTenantConfiguration tenantConfiguration = tenantConfigurationRepository.findByKey(configurationKey.getKeyName()); if (tenantConfiguration == null) { - tenantConfiguration = new JpaTenantConfiguration(configurationKey.getKeyName(), convertedValue.toString()); + tenantConfiguration = new JpaTenantConfiguration(configurationKey.getKeyName(), valueToString); } else { - tenantConfiguration.setValue(convertedValue.toString()); + tenantConfiguration.setValue(valueToString); } - assertValueChangeIsAllowed(keyName, tenantConfiguration); - configurationList.add(tenantConfiguration); - }); - return tenantConfigurationRepository.saveAll(configurationList). - stream(). - collect(Collectors.toMap( - JpaTenantConfiguration::getKey, - updatedTenantConfiguration -> TenantConfigurationValue. builder() - .global(false) - .createdBy(updatedTenantConfiguration.getCreatedBy()) - .createdAt(updatedTenantConfiguration.getCreatedAt()) - .lastModifiedAt(updatedTenantConfiguration.getLastModifiedAt()) - .lastModifiedBy(updatedTenantConfiguration.getLastModifiedBy()) - .value(CONVERSION_SERVICE.convert( - updatedTenantConfiguration.getValue(), - (Class) configurations.get(updatedTenantConfiguration.getKey()).getClass())) - .build())); + assertValueChangeIsAllowed(e.getKey(), tenantConfiguration); + return tenantConfiguration; + }).toList()); } - private void validateConfigurationValue(final T value, final TenantConfigurationKey configurationKey, - final Object convertedValue) { - configurationKey.validate(convertedValue, applicationContext); + private void validateConfigurationValue(final TenantConfigurationKey configurationKey, final Object value) { + configurationKey.validate(value, applicationContext); // additional validation for specific configuration keys if (POLLING_TIME.equals(configurationKey.getKeyName())) { - final PollingTime pollingTime = new PollingTime(value.toString()); + final PollingTime pollingTime = new PollingTime(String.valueOf(value)); if (!ObjectUtils.isEmpty(pollingTime.getOverrides())) { // validate that the QL strings are valid RSQL queries, // nevertheless always when parse them we shall be prepared to catch exceptions if the parsers @@ -237,62 +226,36 @@ public class JpaTenantConfigurationManagement implements TenantConfigurationMana } } - private static Object getConvertedValue(final T value, final Class targetType) { - Object convertedValue = value; - if (!targetType.isAssignableFrom(value.getClass())) { - try { - // if not assignable and it is a number - try conversion - // for example tries to assign Integer to Long - if (value instanceof Number number && Number.class.isAssignableFrom(targetType)) { - log.debug("Type {} not assignable from {} . Will try conversion.", targetType, value.getClass()); - convertedValue = CONVERSION_SERVICE.convert(number, targetType); - if (convertedValue == null) { - throw new IllegalArgumentException( - String.format("Failed to convert %s. Convertor returned null as a result", value)); - } - } else { - throw new IllegalArgumentException( - String.format("Value %s is not a Number but %s and cannot perform conversion converted.", value, value.getClass())); - } - } catch (final ConversionException | IllegalArgumentException ex) { - throw new TenantConfigurationValidatorException(String.format( - "Cannot convert the value %s of type %s to the type %s defined by the configuration key.", - value, value.getClass(), targetType)); - } - } - return convertedValue; - } - @SuppressWarnings("unchecked") - private TenantConfigurationValue getConfigurationValue0(final String keyName, final Class propertyType) { + private TenantConfigurationValue getConfigurationValue0(final String keyName, final Class propertyType) { checkAccess(keyName); final TenantConfigurationKey key = tenantConfigurationProperties.fromKeyName(keyName); - if (propertyType != null) { + final Class type; + if (propertyType == null) { + type = (Class) key.getDataType(); + } else { validateTenantConfigurationDataType(key, propertyType); + type = propertyType; } - final TenantConfiguration tenantConfiguration = TenantAwareCacheManager.getInstance().getCache(CACHE_TENANT_CONFIGURATION_NAME) + final TenantConfiguration tenantConfiguration = TenantAwareCacheManager.getInstance() + .getCache(CACHE_TENANT_CONFIGURATION_NAME) .get(key.getKeyName(), () -> tenantConfigurationRepository.findByKey(key.getKeyName())); - return buildTenantConfigurationValueByKey(key, propertyType == null ? (Class) key.getDataType() : propertyType, tenantConfiguration); - } - - private TenantConfigurationValue buildTenantConfigurationValueByKey( - final TenantConfigurationKey configurationKey, final Class propertyType, final TenantConfiguration tenantConfiguration) { if (tenantConfiguration != null) { - return TenantConfigurationValue. builder().global(false) + return TenantConfigurationValue. builder() + .global(false) .createdBy(tenantConfiguration.getCreatedBy()) .createdAt(tenantConfiguration.getCreatedAt()) .lastModifiedAt(tenantConfiguration.getLastModifiedAt()) .lastModifiedBy(tenantConfiguration.getLastModifiedBy()) - .value(CONVERSION_SERVICE.convert(tenantConfiguration.getValue(), propertyType)).build(); - } else if (configurationKey.getDefaultValue() != null) { - return TenantConfigurationValue. builder().global(true) - .createdBy(null) - .createdAt(null) - .lastModifiedAt(null) - .lastModifiedBy(null) - .value(getGlobalConfigurationValue0(configurationKey.getKeyName(), propertyType)).build(); + .value(CONVERSION_SERVICE.convert(tenantConfiguration.getValue(), type)) + .build(); + } else if (key.getDefaultValue() != null) { + return TenantConfigurationValue. builder() + .global(true) + .value(getGlobalConfigurationValue0(key.getKeyName(), type)) + .build(); } else { return null; } @@ -342,7 +305,9 @@ public class JpaTenantConfigurationManagement implements TenantConfigurationMana private void assertAutoCloseValueChange(final String key) { if (REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED.equals(key) - && Boolean.TRUE.equals(getConfigurationValue0(MULTI_ASSIGNMENTS_ENABLED, Boolean.class).getValue())) { + && Boolean.TRUE.equals(Optional.ofNullable(getConfigurationValue0(MULTI_ASSIGNMENTS_ENABLED, Boolean.class)) + .map(TenantConfigurationValue::getValue) + .orElse(null))) { log.debug("The property '{}' must not be changed because the Multi-Assignments feature is currently enabled.", key); throw new TenantConfigurationValueChangeNotAllowedException(); } @@ -350,7 +315,7 @@ public class JpaTenantConfigurationManagement implements TenantConfigurationMana private void assertBatchAssignmentValueChange(final String key, final JpaTenantConfiguration valueChange) { if (BATCH_ASSIGNMENTS_ENABLED.equals(key) && Boolean.parseBoolean(valueChange.getValue())) { - JpaTenantConfiguration multiConfig = tenantConfigurationRepository.findByKey(MULTI_ASSIGNMENTS_ENABLED); + final JpaTenantConfiguration multiConfig = tenantConfigurationRepository.findByKey(MULTI_ASSIGNMENTS_ENABLED); if (multiConfig != null && Boolean.parseBoolean(multiConfig.getValue())) { log.debug( "The Batch-Assignments '{}' feature cannot be enabled as it contradicts with the Multi-Assignments feature, which is already enabled .", diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/autocleanup/AutoActionCleanupTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/autocleanup/AutoActionCleanupTest.java index 65f6a7b35..94390215c 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/autocleanup/AutoActionCleanupTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/autocleanup/AutoActionCleanupTest.java @@ -188,7 +188,7 @@ class AutoActionCleanupTest extends AbstractJpaIntegrationTest { assertThat(actionRepository.count()).isEqualTo(3); // wait for expiry to elapse - Thread.sleep(800); + waitMillis(800); autoActionCleanup.run(); diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/cluster/DistributedLockTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/cluster/DistributedLockTest.java index 626736c33..6926d8a20 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/cluster/DistributedLockTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/cluster/DistributedLockTest.java @@ -77,16 +77,19 @@ class DistributedLockTest extends AbstractJpaIntegrationTest { } @Bean - LockRepository lockRepository0(final DataSource dataSource, final LockProperties lockProperties, final PlatformTransactionManager txManager) { + LockRepository lockRepository0(final DataSource dataSource, final LockProperties lockProperties, + final PlatformTransactionManager txManager) { return lockRepository(dataSource, lockProperties, txManager); } @Bean - LockRepository lockRepository1(final DataSource dataSource, final LockProperties lockProperties, final PlatformTransactionManager txManager) { + LockRepository lockRepository1(final DataSource dataSource, final LockProperties lockProperties, + final PlatformTransactionManager txManager) { return lockRepository(dataSource, lockProperties, txManager); } - private LockRepository lockRepository(final DataSource dataSource, final LockProperties lockProperties, final PlatformTransactionManager txManager) { + private LockRepository lockRepository(final DataSource dataSource, final LockProperties lockProperties, + final PlatformTransactionManager txManager) { final DefaultLockRepository repository = new DistributedLockRepository(dataSource, lockProperties, txManager); repository.setPrefix("SP_"); return repository; @@ -96,7 +99,7 @@ class DistributedLockTest extends AbstractJpaIntegrationTest { /** * Test to verify that lock is kept while ping runs */ - @SuppressWarnings({"java:S2925"}) + @SuppressWarnings({ "java:S2925" }) @Test void keepLockAlive() { final LockRegistry lockRegistry0 = new JdbcLockRegistry(lockRepository0); @@ -119,13 +122,13 @@ class DistributedLockTest extends AbstractJpaIntegrationTest { final AtomicBoolean lock11Locked = new AtomicBoolean(); // state of the lock11 log.info("Starting test"); // service 0 must be able to lock lockKey0 - assertThat(lock00.tryLock()).isTrue(); + assertThat(lock00.tryLock()).isTrue(); try { assertThat(lockRepository0.isAcquired(path0)).isTrue(); // check db state - + final Thread lockThread1 = new Thread(() -> { // asserts lockKey1 is free and could be locked - assertThat(lock11.tryLock()).isTrue(); + assertThat(lock11.tryLock()).isTrue(); assertThat(lockRepository1.isAcquired(path1)).isTrue(); // check db state try { @@ -140,20 +143,14 @@ class DistributedLockTest extends AbstractJpaIntegrationTest { assertThat(lock01.tryLock()).isFalse(); assertThat(lockRepository1.isAcquired(path0)).isFalse(); // check db state - try { - Thread.sleep(Math.min(1, lockProperties.getTtl() / 4)); - } catch (final InterruptedException e) { - if (Thread.interrupted()) { - Thread.currentThread().interrupt(); - } - } + waitMillis(Math.min(1, lockProperties.getTtl() / 4)); } - } catch (final AssertionError e) { + } catch (final AssertionError e) { log.error("lockRepository1 has locked lockKey0 which has to be in lockRepository0 possession!", e); lock01Obtained.set(true); lock01.unlock(); } - + assertThat(lockRepository0.isAcquired(path1)).isFalse(); // check db state assertThat(lockRepository1.isAcquired(path1)).isTrue(); // check db state } finally { @@ -183,13 +180,7 @@ class DistributedLockTest extends AbstractJpaIntegrationTest { } } - try { - Thread.sleep(Math.min(1, lockProperties.getTtl() / 4)); - } catch (final InterruptedException e) { - if (Thread.interrupted()) { - Thread.currentThread().interrupt(); - } - } + waitMillis(Math.min(1, lockProperties.getTtl() / 4)); } } @@ -205,7 +196,7 @@ class DistributedLockTest extends AbstractJpaIntegrationTest { assertThat(lock01Obtained).isFalse(); // assert that service 1 has been able to acquire the lock 1 assertThat(lock11Obtained).isTrue(); - + assertThat(lockRepository0.isAcquired(path0)).isTrue(); // check db state assertThat(lockRepository1.isAcquired(path0)).isFalse(); // check db state } finally { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TenantConfigurationManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TenantConfigurationManagementTest.java index e374b1f4a..bc4c689dd 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TenantConfigurationManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TenantConfigurationManagementTest.java @@ -18,6 +18,8 @@ import java.time.Duration; import java.util.HashMap; import java.util.Map; +import lombok.NonNull; +import org.awaitility.Awaitility; import org.eclipse.hawkbit.repository.TenantConfigurationManagement; import org.eclipse.hawkbit.repository.exception.InvalidTenantConfigurationKeyException; import org.eclipse.hawkbit.repository.exception.TenantConfigurationValidatorException; @@ -28,6 +30,7 @@ import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.T import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.opentest4j.AssertionFailedError; import org.springframework.context.EnvironmentAware; import org.springframework.core.env.Environment; @@ -48,7 +51,7 @@ class TenantConfigurationManagementTest extends AbstractJpaIntegrationTest imple } @Override - public void setEnvironment(final Environment env) { + public void setEnvironment(@NonNull final Environment env) { this.env = env; } @@ -86,12 +89,20 @@ class TenantConfigurationManagementTest extends AbstractJpaIntegrationTest imple tenantConfigurationManagement.addOrUpdateConfiguration( TenantConfigurationKey.AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY, newConfigurationValue2); + // sometimes it reads old value, maybe if read too early. wait to settle up? + waitMillis(100); + // verify that new configuration value is used final TenantConfigurationValue updatedConfigurationValue2 = tenantConfigurationManagement .getConfigurationValue(TenantConfigurationKey.AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY, String.class); assertThat(updatedConfigurationValue2.isGlobal()).isFalse(); - assertThat(updatedConfigurationValue2.getValue()).isEqualTo(newConfigurationValue2); + try { + assertThat(updatedConfigurationValue2.getValue()).isEqualTo(newConfigurationValue2); + } catch (final AssertionFailedError e) { + Awaitility.await().atMost(Duration.ofSeconds(20)).pollInterval(Duration.ofMillis(100)) + .untilAsserted(() -> assertThat(updatedConfigurationValue2.getValue()).isEqualTo(newConfigurationValue2)); + } } /** @@ -117,7 +128,7 @@ class TenantConfigurationManagementTest extends AbstractJpaIntegrationTest imple */ @Test void batchUpdateTenantSpecificConfiguration() { - final Map configuration = new HashMap<>() {{ + final Map configuration = new HashMap<>() {{ put(TenantConfigurationKey.AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY, "token_123"); put(TenantConfigurationKey.ROLLOUT_APPROVAL_ENABLED, true); }}; @@ -165,7 +176,7 @@ class TenantConfigurationManagementTest extends AbstractJpaIntegrationTest imple */ @Test void batchWrongTenantConfigurationValueTypeThrowsException() { - final Map configuration = new HashMap<>() {{ + final Map configuration = new HashMap<>() {{ put(TenantConfigurationKey.AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY, "token_123"); put(TenantConfigurationKey.ROLLOUT_APPROVAL_ENABLED, true); put(TenantConfigurationKey.AUTHENTICATION_GATEWAY_SECURITY_TOKEN_ENABLED, "wrong"); @@ -216,12 +227,19 @@ class TenantConfigurationManagementTest extends AbstractJpaIntegrationTest imple * Test that an Exception is thrown, when an integer is stored but a string expected. */ @Test - void storesIntegerWhenStringIsExpected() { - final String configKey = TenantConfigurationKey.AUTHENTICATION_GATEWAY_SECURITY_TOKEN_KEY; - final Integer wrongDatType = 123; - assertThatThrownBy(() -> tenantConfigurationManagement.addOrUpdateConfiguration(configKey, wrongDatType)) + void storesStringWhenIntegerIsExpected() { + final String configKey = TenantConfigurationKey.ACTION_CLEANUP_ON_QUOTA_HIT_PERCENTAGE; + final String wrongDataType = "123f"; + assertThatThrownBy(() -> tenantConfigurationManagement.addOrUpdateConfiguration(configKey, wrongDataType)) .as("Should not have worked as integer is not a string") .isInstanceOf(TenantConfigurationValidatorException.class); + + final Integer correctDataType = 123; + tenantConfigurationManagement.addOrUpdateConfiguration(configKey, String.valueOf(correctDataType)); + assertThat(tenantConfigurationManagement.getConfigurationValue(configKey, Integer.class).getValue()).isEqualTo(correctDataType); + tenantConfigurationManagement.addOrUpdateConfiguration(configKey, correctDataType); + assertThat(tenantConfigurationManagement.getConfigurationValue(configKey, Integer.class).getValue()).isEqualTo(correctDataType); + } /** @@ -240,10 +258,9 @@ class TenantConfigurationManagementTest extends AbstractJpaIntegrationTest imple * Test that an Exception is thrown, when an integer is stored as PollingTime. */ @Test - void storesIntegerWhenPollingIntervalIsExpected() { + void storesBadPollingIntervalIsExpected() { final String configKey = TenantConfigurationKey.POLLING_TIME; - final Integer wrongDataType = 123; - assertThatThrownBy(() -> tenantConfigurationManagement.addOrUpdateConfiguration(configKey, wrongDataType)) + assertThatThrownBy(() -> tenantConfigurationManagement.addOrUpdateConfiguration(configKey, "wrongDataType")) .as("Should not have worked as integer is not a time field") .isInstanceOf(TenantConfigurationValidatorException.class); } @@ -350,16 +367,6 @@ class TenantConfigurationManagementTest extends AbstractJpaIntegrationTest imple Assertions.assertEquals(2592000000L, autoCleanupDaysInMs); } - @Test - void throwExceptionIfTryingToConvertOtherValueThanNumber() { - final String configKey = TenantConfigurationKey.ACTION_CLEANUP_AUTO_EXPIRY; - // set auto cleanup for 1 day in String ms - assertThatThrownBy(() -> - tenantConfigurationManagement.addOrUpdateConfiguration(configKey, "86400000")) - .as("Cannot convert the value 86400000 of type String to the type Long defined by the configuration key.") - .isInstanceOf(TenantConfigurationValidatorException.class); - } - private static Duration getDurationByTimeValues(final long hours, final long minutes, final long seconds) { return Duration.ofHours(hours).plusMinutes(minutes).plusSeconds(seconds); } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/tenancy/MultiTenancyEntityTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/tenancy/MultiTenancyEntityTest.java index dd1e08fdd..c9cdbbc2c 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/tenancy/MultiTenancyEntityTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/tenancy/MultiTenancyEntityTest.java @@ -18,7 +18,6 @@ import java.util.Collection; import java.util.List; import java.util.concurrent.Callable; -import org.eclipse.hawkbit.context.AccessContext; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; import org.eclipse.hawkbit.repository.jpa.AbstractJpaIntegrationTest; import org.eclipse.hawkbit.repository.model.DistributionSet; diff --git a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java index 122bdc7d6..36a5fd6de 100644 --- a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java +++ b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java @@ -482,11 +482,15 @@ public abstract class AbstractIntegrationTest { protected void waitNextMillis() { final long createTime = System.currentTimeMillis(); while (System.currentTimeMillis() == createTime) { - try { - Thread.sleep(1); - } catch (final InterruptedException e) { - Thread.currentThread().interrupt(); - } + waitMillis(1); + } + } + + protected void waitMillis(final long millis) { + try { + Thread.sleep(millis); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); } }