From 046cbdbb298856b70e62035e5fffe6c04fa10cab Mon Sep 17 00:00:00 2001 From: Denislav Prinov Date: Wed, 26 Jul 2023 16:59:03 +0300 Subject: [PATCH 1/7] Added an endpoint for batch update of system configurations Signed-off-by: Denislav Prinov --- ...SystemTenantConfigurationValueRequest.java | 23 +++++++++++++++++-- .../rest/api/MgmtTenantManagementRestApi.java | 18 +++++++++++++++ .../MgmtTenantManagementResource.java | 19 +++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/json/model/system/MgmtSystemTenantConfigurationValueRequest.java b/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/json/model/system/MgmtSystemTenantConfigurationValueRequest.java index 45e39cd70..3cde96139 100644 --- a/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/json/model/system/MgmtSystemTenantConfigurationValueRequest.java +++ b/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/json/model/system/MgmtSystemTenantConfigurationValueRequest.java @@ -22,9 +22,19 @@ @JsonIgnoreProperties(ignoreUnknown = true) public class MgmtSystemTenantConfigurationValueRequest { + @JsonProperty + private String name; @JsonProperty(required = true) private Serializable value; + /** + * + * @return the name of the MgmtSystemTenantConfigurationValueRequest + */ + public String getName() { + return name; + } + /** * * @return the value of the MgmtSystemTenantConfigurationValueRequest @@ -34,15 +44,24 @@ public Serializable getValue() { } /** - * Sets the MgmtSystemTenantConfigurationValueRequest + * Sets the value of the MgmtSystemTenantConfigurationValueRequest * * @param value */ public void setValue(final Object value) { if (!(value instanceof Serializable)) { - throw new IllegalArgumentException("The value muste be a instance of " + Serializable.class.getName()); + throw new IllegalArgumentException("The value must be a instance of " + Serializable.class.getName()); } this.value = (Serializable) value; } + /** + * Sets the name of the MgmtSystemTenantConfigurationValueRequest + * + * @param name + */ + public void setName(final String name) { + this.name = name; + } + } diff --git a/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/rest/api/MgmtTenantManagementRestApi.java b/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/rest/api/MgmtTenantManagementRestApi.java index dbfc9de48..951ec9254 100644 --- a/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/rest/api/MgmtTenantManagementRestApi.java +++ b/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/rest/api/MgmtTenantManagementRestApi.java @@ -8,6 +8,7 @@ */ package org.eclipse.hawkbit.mgmt.rest.api; +import java.util.List; import java.util.Map; import org.eclipse.hawkbit.mgmt.json.model.system.MgmtSystemTenantConfigurationValue; @@ -19,6 +20,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; /** * REST Resource for handling tenant specific configuration operations. @@ -83,4 +85,20 @@ ResponseEntity getTenantConfigurationValue( ResponseEntity updateTenantConfigurationValue( @PathVariable("keyName") String keyName, MgmtSystemTenantConfigurationValueRequest configurationValueRest); + /** + * Handles the PUT request for updating a batch of tenant specific configurations + * + * @param configurationValueList + * a list 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. + */ + @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 List configurationValueList); + } diff --git a/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java b/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java index d14cb66c7..5462167a2 100644 --- a/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java +++ b/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java @@ -9,7 +9,9 @@ package org.eclipse.hawkbit.mgmt.rest.resource; import java.io.Serializable; +import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import org.eclipse.hawkbit.mgmt.json.model.system.MgmtSystemTenantConfigurationValue; import org.eclipse.hawkbit.mgmt.json.model.system.MgmtSystemTenantConfigurationValueRequest; @@ -76,4 +78,21 @@ public ResponseEntity updateTenantConfigurat return ResponseEntity.ok(MgmtTenantManagementMapper.toResponse(keyName, updatedValue)); } + @Override + public ResponseEntity> updateTenantConfiguration( + List configurationValueList) { + + boolean containsNull = configurationValueList.stream() + .anyMatch(configurationValue -> configurationValue.getName() == null); + + if (containsNull) { + return ResponseEntity.badRequest().build(); + } + + return ResponseEntity.ok(configurationValueList.stream().map(configurationValue -> MgmtTenantManagementMapper.toResponse(configurationValue.getName(), + tenantConfigurationManagement + .addOrUpdateConfiguration(configurationValue.getName(), configurationValue.getValue()))) + .collect(Collectors.toList())); + } + } From a536ad75971ce564c040e05af3bd336221604583 Mon Sep 17 00:00:00 2001 From: Denislav Prinov Date: Thu, 27 Jul 2023 11:02:36 +0300 Subject: [PATCH 2/7] batch db save Signed-off-by: Denislav Prinov --- .../TenantConfigurationManagement.java | 17 ++++ .../jpa/JpaTenantConfigurationManagement.java | 86 +++++++++++++------ .../MgmtTenantManagementResource.java | 11 ++- 3 files changed, 82 insertions(+), 32 deletions(-) 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 7dc73d971..11a7b2157 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,6 +9,7 @@ package org.eclipse.hawkbit.repository; import java.io.Serializable; +import java.util.Map; import org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions; import org.eclipse.hawkbit.repository.model.TenantConfiguration; @@ -44,6 +45,22 @@ public interface TenantConfigurationManagement { @PreAuthorize(value = SpringEvalExpressions.HAS_AUTH_TENANT_CONFIGURATION) TenantConfigurationValue addOrUpdateConfiguration(String configurationKeyName, T 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_AUTH_TENANT_CONFIGURATION) + Map> addOrUpdateConfiguration(Map configurations); + /** * Build the tenant configuration by the given key * diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java index 4c453bf6c..316ead12c 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java @@ -13,6 +13,11 @@ import static org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey.REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED; import java.io.Serializable; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; import org.eclipse.hawkbit.repository.TenantConfigurationManagement; import org.eclipse.hawkbit.repository.exception.TenantConfigurationValueChangeNotAllowedException; @@ -26,6 +31,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.cache.Cache; +import org.springframework.cache.CacheManager; import org.springframework.cache.annotation.CacheEvict; import org.springframework.cache.annotation.Cacheable; import org.springframework.context.ApplicationContext; @@ -55,6 +62,9 @@ public class JpaTenantConfigurationManagement implements TenantConfigurationMana @Autowired private ApplicationContext applicationContext; + @Autowired + private CacheManager cacheManager; + private static final ConfigurableConversionService conversionService = new DefaultConversionService(); @Override @@ -133,45 +143,65 @@ public T getGlobalConfigurationValue(final String configurationKeyName, fina } @Override - @CacheEvict(value = "tenantConfiguration", key = "#configurationKeyName") - @Transactional - @Retryable(include = { - ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) public TenantConfigurationValue addOrUpdateConfiguration( final String configurationKeyName, final T value) { + return addOrUpdateConfiguration(Collections.singletonMap(configurationKeyName, value)).values().iterator().next(); + } - final TenantConfigurationKey configurationKey = tenantConfigurationProperties.fromKeyName(configurationKeyName); + @Override + @Transactional + @Retryable(include = { + ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) + public Map> addOrUpdateConfiguration(Map configurations) { - if (!configurationKey.getDataType().isAssignableFrom(value.getClass())) { - throw new TenantConfigurationValidatorException(String.format( - "Cannot parse the value %s of type %s into the type %s defined by the configuration key.", value, - value.getClass(), configurationKey.getDataType())); + //@CacheEvict + Cache cache = cacheManager.getCache("tenantConfiguration"); + if (cache != null) { + configurations.keySet().forEach(cache::evict); } - configurationKey.validate(applicationContext, value); + List configurationList = new ArrayList<>(); + configurations.forEach((configurationKeyName, value) -> { + final TenantConfigurationKey configurationKey = tenantConfigurationProperties.fromKeyName(configurationKeyName); - JpaTenantConfiguration tenantConfiguration = tenantConfigurationRepository - .findByKey(configurationKey.getKeyName()); - - if (tenantConfiguration == null) { - tenantConfiguration = new JpaTenantConfiguration(configurationKey.getKeyName(), value.toString()); - } else { - tenantConfiguration.setValue(value.toString()); - } + if (!configurationKey.getDataType().isAssignableFrom(value.getClass())) { + throw new TenantConfigurationValidatorException(String.format( + "Cannot parse the value %s of type %s into the type %s defined by the configuration key.", value, + value.getClass(), configurationKey.getDataType())); + } - assertValueChangeIsAllowed(configurationKeyName, tenantConfiguration); + configurationKey.validate(applicationContext, value); - final JpaTenantConfiguration updatedTenantConfiguration = tenantConfigurationRepository - .save(tenantConfiguration); + JpaTenantConfiguration tenantConfiguration = tenantConfigurationRepository + .findByKey(configurationKey.getKeyName()); - @SuppressWarnings("unchecked") - final Class clazzT = (Class) value.getClass(); + if (tenantConfiguration == null) { + tenantConfiguration = new JpaTenantConfiguration(configurationKey.getKeyName(), value.toString()); + } else { + tenantConfiguration.setValue(value.toString()); + } - return TenantConfigurationValue. builder().global(false).createdBy(updatedTenantConfiguration.getCreatedBy()) - .createdAt(updatedTenantConfiguration.getCreatedAt()) - .lastModifiedAt(updatedTenantConfiguration.getLastModifiedAt()) - .lastModifiedBy(updatedTenantConfiguration.getLastModifiedBy()) - .value(conversionService.convert(updatedTenantConfiguration.getValue(), clazzT)).build(); + assertValueChangeIsAllowed(configurationKeyName, tenantConfiguration); + configurationList.add(tenantConfiguration); + }); + + List jpaTenantConfigurations = tenantConfigurationRepository + .saveAll(configurationList); + + return jpaTenantConfigurations.stream().collect(Collectors.toMap( + JpaTenantConfiguration::getKey, + updatedTenantConfiguration -> { + @SuppressWarnings("unchecked") + final Class clazzT = (Class) configurations.get(updatedTenantConfiguration.getKey()).getClass(); + + return TenantConfigurationValue.builder().global(false) + .createdBy(updatedTenantConfiguration.getCreatedBy()) + .createdAt(updatedTenantConfiguration.getCreatedAt()) + .lastModifiedAt(updatedTenantConfiguration.getLastModifiedAt()) + .lastModifiedBy(updatedTenantConfiguration.getLastModifiedBy()) + .value(conversionService.convert(updatedTenantConfiguration.getValue(), clazzT)) + .build(); + })); } /** diff --git a/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java b/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java index 5462167a2..317099707 100644 --- a/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java +++ b/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java @@ -89,10 +89,13 @@ public ResponseEntity> updateTenantConf return ResponseEntity.badRequest().build(); } - return ResponseEntity.ok(configurationValueList.stream().map(configurationValue -> MgmtTenantManagementMapper.toResponse(configurationValue.getName(), - tenantConfigurationManagement - .addOrUpdateConfiguration(configurationValue.getName(), configurationValue.getValue()))) - .collect(Collectors.toList())); + Map> tenantConfigurationValues = tenantConfigurationManagement.addOrUpdateConfiguration(configurationValueList.stream() + .collect(Collectors.toMap( + MgmtSystemTenantConfigurationValueRequest::getName, + MgmtSystemTenantConfigurationValueRequest::getValue))); + + return ResponseEntity.ok(tenantConfigurationValues.entrySet().stream().map(entry -> + MgmtTenantManagementMapper.toResponse(entry.getKey(), entry.getValue())).collect(Collectors.toList())); } } From 769908c89e753fbb7ae0ff3ffd7ddfd1811a45a4 Mon Sep 17 00:00:00 2001 From: Denislav Prinov Date: Tue, 1 Aug 2023 17:14:24 +0300 Subject: [PATCH 3/7] Review changes and added tests Signed-off-by: Denislav Prinov --- .../jpa/JpaTenantConfigurationManagement.java | 11 +++-- .../TenantConfigurationManagementTest.java | 38 +++++++++++++- .../MgmtTenantManagementResourceTest.java | 49 +++++++++++++++++++ 3 files changed, 94 insertions(+), 4 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java index 316ead12c..a780354e4 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java @@ -143,6 +143,9 @@ public T getGlobalConfigurationValue(final String configurationKeyName, fina } @Override + @Transactional + @Retryable(include = { + ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) public TenantConfigurationValue addOrUpdateConfiguration( final String configurationKeyName, final T value) { return addOrUpdateConfiguration(Collections.singletonMap(configurationKeyName, value)).values().iterator().next(); @@ -156,9 +159,6 @@ public Map> addOrUp //@CacheEvict Cache cache = cacheManager.getCache("tenantConfiguration"); - if (cache != null) { - configurations.keySet().forEach(cache::evict); - } List configurationList = new ArrayList<>(); configurations.forEach((configurationKeyName, value) -> { @@ -188,9 +188,14 @@ public Map> addOrUp List jpaTenantConfigurations = tenantConfigurationRepository .saveAll(configurationList); + if (cache != null) { + configurations.keySet().forEach(cache::evict); + } + return jpaTenantConfigurations.stream().collect(Collectors.toMap( JpaTenantConfiguration::getKey, updatedTenantConfiguration -> { + @SuppressWarnings("unchecked") final Class clazzT = (Class) configurations.get(updatedTenantConfiguration.getKey()).getClass(); diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/TenantConfigurationManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/TenantConfigurationManagementTest.java index 24a2f7378..fc50fefc9 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/TenantConfigurationManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/TenantConfigurationManagementTest.java @@ -73,7 +73,7 @@ public void storeTenantSpecificConfigurationAsString() { @Test @Description("Tests that the tenant specific configuration can be updated") - public void updateTenantSpecifcConfiguration() { + public void updateTenantSpecificConfiguration() { final String configKey = TenantConfigurationKey.AUTHENTICATION_MODE_GATEWAY_SECURITY_TOKEN_KEY; final String value1 = "firstValue"; final String value2 = "secondValue"; @@ -89,6 +89,22 @@ public void updateTenantSpecifcConfiguration() { .isEqualTo(value2); } + @Test + @Description("Tests that the tenant specific configuration can be batch updated") + public void batchUpdateTenantSpecificConfiguration() { + Map configuration = new HashMap<>() {{ + put(TenantConfigurationKey.AUTHENTICATION_MODE_GATEWAY_SECURITY_TOKEN_KEY, "token_123"); + put(TenantConfigurationKey.ROLLOUT_APPROVAL_ENABLED, true); + }}; + + // add value first + tenantConfigurationManagement.addOrUpdateConfiguration(configuration); + assertThat(tenantConfigurationManagement.getConfigurationValue(TenantConfigurationKey.AUTHENTICATION_MODE_GATEWAY_SECURITY_TOKEN_KEY, String.class).getValue()) + .isEqualTo("token_123"); + assertThat(tenantConfigurationManagement.getConfigurationValue(TenantConfigurationKey.ROLLOUT_APPROVAL_ENABLED, Boolean.class).getValue()) + .isEqualTo(true); + } + @Test @Description("Tests that the configuration value can be converted from String to Integer automatically") public void storeAndUpdateTenantSpecificConfigurationAsBoolean() { @@ -118,6 +134,26 @@ public void wrongTenantConfigurationValueTypeThrowsException() { } } + @Test + @Description("Tests that the get configuration throws exception in case the value is the wrong type") + public void batchWrongTenantConfigurationValueTypeThrowsException() { + Map configuration = new HashMap<>() {{ + put(TenantConfigurationKey.AUTHENTICATION_MODE_GATEWAY_SECURITY_TOKEN_KEY, "token_123"); + put(TenantConfigurationKey.ROLLOUT_APPROVAL_ENABLED, true); + put(TenantConfigurationKey.AUTHENTICATION_MODE_GATEWAY_SECURITY_TOKEN_ENABLED, "wrong"); + }}; + + try { + tenantConfigurationManagement.addOrUpdateConfiguration(configuration); + fail("should not have worked as type is wrong"); + } catch (final TenantConfigurationValidatorException e) { + assertThat(tenantConfigurationManagement.getConfigurationValue(TenantConfigurationKey.AUTHENTICATION_MODE_GATEWAY_SECURITY_TOKEN_KEY, String.class).getValue()) + .isNotEqualTo("token_123"); + assertThat(tenantConfigurationManagement.getConfigurationValue(TenantConfigurationKey.ROLLOUT_APPROVAL_ENABLED, Boolean.class).getValue()) + .isNotEqualTo(true); + } + } + @Test @Description("Tests that a deletion of a tenant specific configuration deletes it from the database.") public void deleteConfigurationReturnNullConfiguration() { diff --git a/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResourceTest.java b/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResourceTest.java index a9e4bd5c9..13091bbe0 100644 --- a/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResourceTest.java +++ b/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResourceTest.java @@ -13,6 +13,7 @@ import org.eclipse.hawkbit.mgmt.rest.api.MgmtRestConstants; import org.eclipse.hawkbit.rest.util.MockMvcResultPrinter; +import org.json.JSONArray; import org.json.JSONObject; import org.junit.jupiter.api.Test; import org.springframework.http.MediaType; @@ -32,6 +33,10 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg private static final String KEY_MULTI_ASSIGNMENTS = "multi.assignments.enabled"; private static final String KEY_AUTO_CLOSE = "repository.actions.autoclose.enabled"; + private static final String ROLLOUT_APPROVAL_ENABLED = "rollout.approval.enabled"; + + private static final String AUTHENTICATION_GATEWAYTOKEN_ENABLED = "authentication.gatewaytoken.enabled"; + @Test @Description("The 'multi.assignments.enabled' property must not be changed to false.") @@ -48,6 +53,50 @@ public void deactivateMultiAssignment() throws Exception { .andExpect(status().isForbidden()); } + @Test + @Description("The Batch configuration should be applied") + public void changeBatchConfiguration() throws Exception { + JSONObject rolloutApprovalConfiguration = new JSONObject(); + rolloutApprovalConfiguration.put("name", ROLLOUT_APPROVAL_ENABLED); + rolloutApprovalConfiguration.put("value", true); + + JSONObject tokenEnabledConfiguration = new JSONObject(); + tokenEnabledConfiguration.put("name", AUTHENTICATION_GATEWAYTOKEN_ENABLED); + tokenEnabledConfiguration.put("value", true); + + JSONArray jsonArray = new JSONArray(); + jsonArray.put(rolloutApprovalConfiguration); + jsonArray.put(tokenEnabledConfiguration); + + String body = jsonArray.toString(); + + mvc.perform(put(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs") + .content(body).contentType(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print()) + .andExpect(status().isOk()); + } + + @Test + @Description("The Batch configuration should not be applied") + public void changeBatchConfigurationFail() throws Exception { + JSONObject rolloutApprovalConfiguration = new JSONObject(); + rolloutApprovalConfiguration.put("name", ROLLOUT_APPROVAL_ENABLED); + rolloutApprovalConfiguration.put("value", true); + + JSONObject tokenEnabledConfiguration = new JSONObject(); + tokenEnabledConfiguration.put("name", AUTHENTICATION_GATEWAYTOKEN_ENABLED); + tokenEnabledConfiguration.put("value", "fail"); + + JSONArray jsonArray = new JSONArray(); + jsonArray.put(rolloutApprovalConfiguration); + jsonArray.put(tokenEnabledConfiguration); + + String body = jsonArray.toString(); + + mvc.perform(put(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs") + .content(body).contentType(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print()) + .andExpect(status().isBadRequest()); + } + @Test @Description("The 'repository.actions.autoclose.enabled' property must not be modified if Multi-Assignments is enabled.") public void autoCloseCannotBeModifiedIfMultiAssignmentIsEnabled() throws Exception { From 2fb15b98e265fc6ddfadadb68636d1e354777cbe Mon Sep 17 00:00:00 2001 From: Denislav Prinov Date: Tue, 1 Aug 2023 17:49:06 +0300 Subject: [PATCH 4/7] Evict cache only if transaction is commited - such as @CacheEvict Signed-off-by: Denislav Prinov --- .../jpa/JpaTenantConfigurationManagement.java | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java index a780354e4..795c75b0c 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java @@ -42,6 +42,8 @@ import org.springframework.retry.annotation.Backoff; import org.springframework.retry.annotation.Retryable; import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; import org.springframework.validation.annotation.Validated; /** @@ -148,7 +150,7 @@ public T getGlobalConfigurationValue(final String configurationKeyName, fina ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) public TenantConfigurationValue addOrUpdateConfiguration( final String configurationKeyName, final T value) { - return addOrUpdateConfiguration(Collections.singletonMap(configurationKeyName, value)).values().iterator().next(); + return addOrUpdateConfiguration0(Collections.singletonMap(configurationKeyName, value)).values().iterator().next(); } @Override @@ -156,9 +158,22 @@ public TenantConfigurationValue addOrUpdateConfigura @Retryable(include = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) public Map> addOrUpdateConfiguration(Map configurations) { + return addOrUpdateConfiguration0(configurations); + } - //@CacheEvict - Cache cache = cacheManager.getCache("tenantConfiguration"); + private Map> addOrUpdateConfiguration0(Map configurations) { + + // Register a callback to be invoked after the transaction is committed - for cache eviction + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCommit() { + TransactionSynchronization.super.afterCommit(); + Cache cache = cacheManager.getCache("tenantConfiguration"); + if (cache != null) { + configurations.keySet().forEach(cache::evict); + } + } + }); List configurationList = new ArrayList<>(); configurations.forEach((configurationKeyName, value) -> { @@ -188,17 +203,12 @@ public Map> addOrUp List jpaTenantConfigurations = tenantConfigurationRepository .saveAll(configurationList); - if (cache != null) { - configurations.keySet().forEach(cache::evict); - } - return jpaTenantConfigurations.stream().collect(Collectors.toMap( JpaTenantConfiguration::getKey, updatedTenantConfiguration -> { @SuppressWarnings("unchecked") final Class clazzT = (Class) configurations.get(updatedTenantConfiguration.getKey()).getClass(); - return TenantConfigurationValue.builder().global(false) .createdBy(updatedTenantConfiguration.getCreatedBy()) .createdAt(updatedTenantConfiguration.getCreatedAt()) From 0739f2ebce201e3f1a0c01902fe0f50b6a660673 Mon Sep 17 00:00:00 2001 From: Denislav Prinov Date: Tue, 1 Aug 2023 17:56:54 +0300 Subject: [PATCH 5/7] refactoring Signed-off-by: Denislav Prinov --- .../hawkbit/repository/jpa/JpaTenantConfigurationManagement.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java index 795c75b0c..6e06b206e 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java @@ -167,7 +167,6 @@ private Map> addOrU TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { @Override public void afterCommit() { - TransactionSynchronization.super.afterCommit(); Cache cache = cacheManager.getCache("tenantConfiguration"); if (cache != null) { configurations.keySet().forEach(cache::evict); From da5b4c1b37f3d8729f2369cbacf42bf86239f29a Mon Sep 17 00:00:00 2001 From: Denislav Prinov Date: Tue, 1 Aug 2023 18:44:51 +0300 Subject: [PATCH 6/7] Using AfterTransactionCommitExecutor for cache eviction Signed-off-by: Denislav Prinov --- .../jpa/JpaTenantConfigurationManagement.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java index 6e06b206e..b7b3a5efd 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java @@ -22,6 +22,7 @@ import org.eclipse.hawkbit.repository.TenantConfigurationManagement; import org.eclipse.hawkbit.repository.exception.TenantConfigurationValueChangeNotAllowedException; import org.eclipse.hawkbit.repository.jpa.configuration.Constants; +import org.eclipse.hawkbit.repository.jpa.executor.AfterTransactionCommitExecutor; import org.eclipse.hawkbit.repository.jpa.model.JpaTenantConfiguration; import org.eclipse.hawkbit.repository.model.TenantConfiguration; import org.eclipse.hawkbit.repository.model.TenantConfigurationValue; @@ -42,8 +43,6 @@ import org.springframework.retry.annotation.Backoff; import org.springframework.retry.annotation.Retryable; import org.springframework.transaction.annotation.Transactional; -import org.springframework.transaction.support.TransactionSynchronization; -import org.springframework.transaction.support.TransactionSynchronizationManager; import org.springframework.validation.annotation.Validated; /** @@ -67,6 +66,9 @@ public class JpaTenantConfigurationManagement implements TenantConfigurationMana @Autowired private CacheManager cacheManager; + @Autowired + private AfterTransactionCommitExecutor afterCommitExecutor; + private static final ConfigurableConversionService conversionService = new DefaultConversionService(); @Override @@ -164,13 +166,10 @@ public Map> addOrUp private Map> addOrUpdateConfiguration0(Map configurations) { // Register a callback to be invoked after the transaction is committed - for cache eviction - TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { - @Override - public void afterCommit() { - Cache cache = cacheManager.getCache("tenantConfiguration"); - if (cache != null) { - configurations.keySet().forEach(cache::evict); - } + afterCommitExecutor.afterCommit(() -> { + Cache cache = cacheManager.getCache("tenantConfiguration"); + if (cache != null) { + configurations.keySet().forEach(cache::evict); } }); From 2bbbc5c53babd674c9d1713189ccd3b92bf0576e Mon Sep 17 00:00:00 2001 From: Denislav Prinov Date: Wed, 2 Aug 2023 10:37:20 +0300 Subject: [PATCH 7/7] Change request body Signed-off-by: Denislav Prinov --- .../jpa/JpaTenantConfigurationManagement.java | 10 ++--- .../TenantConfigurationManagementTest.java | 2 +- ...SystemTenantConfigurationValueRequest.java | 21 +--------- .../rest/api/MgmtTenantManagementRestApi.java | 7 ++-- .../MgmtTenantManagementResource.java | 16 ++++---- .../MgmtTenantManagementResourceTest.java | 39 +++++++------------ 6 files changed, 32 insertions(+), 63 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java index b7b3a5efd..ee443f1de 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantConfigurationManagement.java @@ -147,6 +147,7 @@ public T getGlobalConfigurationValue(final String configurationKeyName, fina } @Override + @CacheEvict(value = "tenantConfiguration", key = "#configurationKeyName") @Transactional @Retryable(include = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) @@ -160,11 +161,6 @@ public TenantConfigurationValue addOrUpdateConfigura @Retryable(include = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) public Map> addOrUpdateConfiguration(Map configurations) { - return addOrUpdateConfiguration0(configurations); - } - - private Map> addOrUpdateConfiguration0(Map configurations) { - // Register a callback to be invoked after the transaction is committed - for cache eviction afterCommitExecutor.afterCommit(() -> { Cache cache = cacheManager.getCache("tenantConfiguration"); @@ -173,6 +169,10 @@ private Map> addOrU } }); + return addOrUpdateConfiguration0(configurations); + } + + private Map> addOrUpdateConfiguration0(Map configurations) { List configurationList = new ArrayList<>(); configurations.forEach((configurationKeyName, value) -> { final TenantConfigurationKey configurationKey = tenantConfigurationProperties.fromKeyName(configurationKeyName); diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/TenantConfigurationManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/TenantConfigurationManagementTest.java index fc50fefc9..be3b67b24 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/TenantConfigurationManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/TenantConfigurationManagementTest.java @@ -102,7 +102,7 @@ public void batchUpdateTenantSpecificConfiguration() { assertThat(tenantConfigurationManagement.getConfigurationValue(TenantConfigurationKey.AUTHENTICATION_MODE_GATEWAY_SECURITY_TOKEN_KEY, String.class).getValue()) .isEqualTo("token_123"); assertThat(tenantConfigurationManagement.getConfigurationValue(TenantConfigurationKey.ROLLOUT_APPROVAL_ENABLED, Boolean.class).getValue()) - .isEqualTo(true); + .isTrue(); } @Test diff --git a/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/json/model/system/MgmtSystemTenantConfigurationValueRequest.java b/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/json/model/system/MgmtSystemTenantConfigurationValueRequest.java index 3cde96139..5c160b0fb 100644 --- a/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/json/model/system/MgmtSystemTenantConfigurationValueRequest.java +++ b/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/json/model/system/MgmtSystemTenantConfigurationValueRequest.java @@ -22,19 +22,9 @@ @JsonIgnoreProperties(ignoreUnknown = true) public class MgmtSystemTenantConfigurationValueRequest { - @JsonProperty - private String name; @JsonProperty(required = true) private Serializable value; - /** - * - * @return the name of the MgmtSystemTenantConfigurationValueRequest - */ - public String getName() { - return name; - } - /** * * @return the value of the MgmtSystemTenantConfigurationValueRequest @@ -48,20 +38,11 @@ public Serializable getValue() { * * @param value */ + public void setValue(final Object value) { if (!(value instanceof Serializable)) { throw new IllegalArgumentException("The value must be a instance of " + Serializable.class.getName()); } this.value = (Serializable) value; } - - /** - * Sets the name of the MgmtSystemTenantConfigurationValueRequest - * - * @param name - */ - public void setName(final String name) { - this.name = name; - } - } diff --git a/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/rest/api/MgmtTenantManagementRestApi.java b/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/rest/api/MgmtTenantManagementRestApi.java index 951ec9254..cf4be7d70 100644 --- a/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/rest/api/MgmtTenantManagementRestApi.java +++ b/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/rest/api/MgmtTenantManagementRestApi.java @@ -8,6 +8,7 @@ */ package org.eclipse.hawkbit.mgmt.rest.api; +import java.io.Serializable; import java.util.List; import java.util.Map; @@ -88,8 +89,8 @@ ResponseEntity updateTenantConfigurationValu /** * Handles the PUT request for updating a batch of tenant specific configurations * - * @param configurationValueList - * a list of name - value pairs for the 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 @@ -99,6 +100,6 @@ ResponseEntity updateTenantConfigurationValu MediaTypes.HAL_JSON_VALUE, MediaType.APPLICATION_JSON_VALUE }, produces = { MediaTypes.HAL_JSON_VALUE, MediaType.APPLICATION_JSON_VALUE }) ResponseEntity> updateTenantConfiguration( - @RequestBody List configurationValueList); + @RequestBody Map configurationValueMap); } diff --git a/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java b/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java index 317099707..cebc32fae 100644 --- a/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java +++ b/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResource.java @@ -11,7 +11,7 @@ import java.io.Serializable; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; +import java.util.Objects; import org.eclipse.hawkbit.mgmt.json.model.system.MgmtSystemTenantConfigurationValue; import org.eclipse.hawkbit.mgmt.json.model.system.MgmtSystemTenantConfigurationValueRequest; @@ -80,22 +80,20 @@ public ResponseEntity updateTenantConfigurat @Override public ResponseEntity> updateTenantConfiguration( - List configurationValueList) { + Map configurationValueMap) { - boolean containsNull = configurationValueList.stream() - .anyMatch(configurationValue -> configurationValue.getName() == null); + boolean containsNull = configurationValueMap.keySet().stream() + .anyMatch(Objects::isNull); if (containsNull) { return ResponseEntity.badRequest().build(); } - Map> tenantConfigurationValues = tenantConfigurationManagement.addOrUpdateConfiguration(configurationValueList.stream() - .collect(Collectors.toMap( - MgmtSystemTenantConfigurationValueRequest::getName, - MgmtSystemTenantConfigurationValueRequest::getValue))); + Map> tenantConfigurationValues = tenantConfigurationManagement + .addOrUpdateConfiguration(configurationValueMap); return ResponseEntity.ok(tenantConfigurationValues.entrySet().stream().map(entry -> - MgmtTenantManagementMapper.toResponse(entry.getKey(), entry.getValue())).collect(Collectors.toList())); + MgmtTenantManagementMapper.toResponse(entry.getKey(), entry.getValue())).toList()); } } diff --git a/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResourceTest.java b/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResourceTest.java index 13091bbe0..c1f430961 100644 --- a/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResourceTest.java +++ b/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTenantManagementResourceTest.java @@ -13,7 +13,6 @@ import org.eclipse.hawkbit.mgmt.rest.api.MgmtRestConstants; import org.eclipse.hawkbit.rest.util.MockMvcResultPrinter; -import org.json.JSONArray; import org.json.JSONObject; import org.junit.jupiter.api.Test; import org.springframework.http.MediaType; @@ -37,6 +36,10 @@ public class MgmtTenantManagementResourceTest extends AbstractManagementApiInteg private static final String AUTHENTICATION_GATEWAYTOKEN_ENABLED = "authentication.gatewaytoken.enabled"; + private static final String AUTHENTICATION_GATEWAYTOKEN_KEY = "authentication.gatewaytoken.key"; + + + @Test @Description("The 'multi.assignments.enabled' property must not be changed to false.") @@ -56,19 +59,12 @@ public void deactivateMultiAssignment() throws Exception { @Test @Description("The Batch configuration should be applied") public void changeBatchConfiguration() throws Exception { - JSONObject rolloutApprovalConfiguration = new JSONObject(); - rolloutApprovalConfiguration.put("name", ROLLOUT_APPROVAL_ENABLED); - rolloutApprovalConfiguration.put("value", true); - - JSONObject tokenEnabledConfiguration = new JSONObject(); - tokenEnabledConfiguration.put("name", AUTHENTICATION_GATEWAYTOKEN_ENABLED); - tokenEnabledConfiguration.put("value", true); + JSONObject configuration = new JSONObject(); + configuration.put(ROLLOUT_APPROVAL_ENABLED, true); + configuration.put(AUTHENTICATION_GATEWAYTOKEN_ENABLED, true); + configuration.put(AUTHENTICATION_GATEWAYTOKEN_KEY, "1234"); - JSONArray jsonArray = new JSONArray(); - jsonArray.put(rolloutApprovalConfiguration); - jsonArray.put(tokenEnabledConfiguration); - - String body = jsonArray.toString(); + String body = configuration.toString(); mvc.perform(put(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs") .content(body).contentType(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print()) @@ -78,19 +74,12 @@ public void changeBatchConfiguration() throws Exception { @Test @Description("The Batch configuration should not be applied") public void changeBatchConfigurationFail() throws Exception { - JSONObject rolloutApprovalConfiguration = new JSONObject(); - rolloutApprovalConfiguration.put("name", ROLLOUT_APPROVAL_ENABLED); - rolloutApprovalConfiguration.put("value", true); - - JSONObject tokenEnabledConfiguration = new JSONObject(); - tokenEnabledConfiguration.put("name", AUTHENTICATION_GATEWAYTOKEN_ENABLED); - tokenEnabledConfiguration.put("value", "fail"); - - JSONArray jsonArray = new JSONArray(); - jsonArray.put(rolloutApprovalConfiguration); - jsonArray.put(tokenEnabledConfiguration); + JSONObject configuration = new JSONObject(); + configuration.put(ROLLOUT_APPROVAL_ENABLED, true); + configuration.put(AUTHENTICATION_GATEWAYTOKEN_ENABLED, "wrong"); + configuration.put(AUTHENTICATION_GATEWAYTOKEN_KEY, "1234"); - String body = jsonArray.toString(); + String body = configuration.toString(); mvc.perform(put(MgmtRestConstants.SYSTEM_V1_REQUEST_MAPPING + "/configs") .content(body).contentType(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print())