From 21a4ae3cb80c4b38bd91d3ec3b247727d90f0de3 Mon Sep 17 00:00:00 2001 From: Michael Herdt <55577866+herdt-michael@users.noreply.github.com> Date: Mon, 26 Apr 2021 16:15:20 +0200 Subject: [PATCH] Refactor AutoAssignExecutor to improve the extensibility (#1110) * Refactor AutoAssignExecutor to improve the extensibility. Signed-off-by: Michael Herdt * introduce protected getters Signed-off-by: Michael Herdt * Refactor auto assign executor. Create deployment requests based on list of controllerIds. Signed-off-by: Michael Herdt * Fix review findings Signed-off-by: Michael Herdt * Surround consumer with a try catch block to continue assignment process for other filter. Execute assignment in user context. Signed-off-by: Michael Herdt --- .../AbstractAutoAssignExecutor.java | 183 ++++++++++++++++++ .../jpa/autoassign/AutoAssignChecker.java | 118 +---------- 2 files changed, 193 insertions(+), 108 deletions(-) create mode 100644 hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/autoassign/AbstractAutoAssignExecutor.java diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/autoassign/AbstractAutoAssignExecutor.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/autoassign/AbstractAutoAssignExecutor.java new file mode 100644 index 000000000..ee281fde8 --- /dev/null +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/autoassign/AbstractAutoAssignExecutor.java @@ -0,0 +1,183 @@ +/** + * Copyright (c) 2021 Bosch.IO GmbH and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.repository.jpa.autoassign; + +import java.util.List; +import java.util.Objects; +import java.util.function.Consumer; +import java.util.stream.Collectors; + +import org.eclipse.hawkbit.repository.DeploymentManagement; +import org.eclipse.hawkbit.repository.TargetFilterQueryManagement; +import org.eclipse.hawkbit.repository.autoassign.AutoAssignExecutor; +import org.eclipse.hawkbit.repository.jpa.utils.DeploymentHelper; +import org.eclipse.hawkbit.repository.model.Action; +import org.eclipse.hawkbit.repository.model.DeploymentRequest; +import org.eclipse.hawkbit.repository.model.TargetFilterQuery; +import org.eclipse.hawkbit.tenancy.TenantAware; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Pageable; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.annotation.Isolation; +import org.springframework.util.StringUtils; + +/** + * Abstract implementation of an AutoAssignExecutor + */ +public abstract class AbstractAutoAssignExecutor implements AutoAssignExecutor { + + private static final Logger LOGGER = LoggerFactory.getLogger(AbstractAutoAssignExecutor.class); + + /** + * The message which is added to the action status when a distribution set is + * assigned to an target. First %s is the name of the target filter. + */ + private static final String ACTION_MESSAGE = "Auto assignment by target filter: %s"; + + /** + * Maximum for target filter queries with auto assign DS activated. + */ + private static final int PAGE_SIZE = 1000; + + private final TargetFilterQueryManagement targetFilterQueryManagement; + + private final DeploymentManagement deploymentManagement; + + private final PlatformTransactionManager transactionManager; + + private final TenantAware tenantAware; + + /** + * Constructor + * + * @param targetFilterQueryManagement + * to get all target filter queries + * @param deploymentManagement + * to assign distribution sets to targets + * @param transactionManager + * to run transactions + * @param tenantAware + * to handle the tenant context + */ + protected AbstractAutoAssignExecutor(final TargetFilterQueryManagement targetFilterQueryManagement, + final DeploymentManagement deploymentManagement, final PlatformTransactionManager transactionManager, + final TenantAware tenantAware) { + this.targetFilterQueryManagement = targetFilterQueryManagement; + this.deploymentManagement = deploymentManagement; + this.transactionManager = transactionManager; + this.tenantAware = tenantAware; + } + + protected TargetFilterQueryManagement getTargetFilterQueryManagement() { + return targetFilterQueryManagement; + } + + protected DeploymentManagement getDeploymentManagement() { + return deploymentManagement; + } + + protected PlatformTransactionManager getTransactionManager() { + return transactionManager; + } + + protected TenantAware getTenantAware() { + return tenantAware; + } + + protected void forEachFilterWithAutoAssignDS(final Consumer consumer) { + Page filterQueries; + Pageable query = PageRequest.of(0, PAGE_SIZE); + + do { + filterQueries = targetFilterQueryManagement.findWithAutoAssignDS(query); + + filterQueries.forEach(filterQuery -> { + try { + runInUserContext(filterQuery, () -> consumer.accept(filterQuery)); + } catch (final RuntimeException ex) { + LOGGER.debug( + "Exception on forEachFilterWithAutoAssignDS execution for tenant {} with filter id {}. Continue with next filter query.", + filterQuery.getTenant(), filterQuery.getId(), ex); + LOGGER.error( + "Exception on forEachFilterWithAutoAssignDS execution for tenant {} with filter id {} and error message [{}]. " + + "Continue with next filter query.", + filterQuery.getTenant(), filterQuery.getId(), ex.getMessage()); + } + }); + } while ((query = filterQueries.nextPageable()) != Pageable.unpaged()); + } + + /** + * Runs target assignments within a dedicated transaction for a given list of + * controllerIDs + * + * @param targetFilterQuery + * the target filter query + * @param controllerIds + * the controllerIDs + * @return count of targets + */ + protected int runTransactionalAssignment(final TargetFilterQuery targetFilterQuery, + final List controllerIds) { + final String actionMessage = String.format(ACTION_MESSAGE, targetFilterQuery.getName()); + + return DeploymentHelper.runInNewTransaction(getTransactionManager(), "autoAssignDSToTargets", + Isolation.READ_COMMITTED.value(), status -> { + + final List deploymentRequests = mapToDeploymentRequests(controllerIds, + targetFilterQuery); + + final int count = deploymentRequests.size(); + if (count > 0) { + getDeploymentManagement().assignDistributionSets( + getAutoAssignmentInitiatedBy(targetFilterQuery), deploymentRequests, actionMessage); + } + return count; + }); + } + + /** + * Creates a list of {@link DeploymentRequest} for given list of controllerIds + * and {@link TargetFilterQuery} + * + * @param controllerIds + * list of controllerIds + * @param filterQuery + * the query the targets have to match + * @return list of deployment request + */ + protected List mapToDeploymentRequests(final List controllerIds, + final TargetFilterQuery filterQuery) { + // the action type is set to FORCED per default (when not explicitly + // specified) + final Action.ActionType autoAssignActionType = filterQuery.getAutoAssignActionType() == null + ? Action.ActionType.FORCED + : filterQuery.getAutoAssignActionType(); + + return controllerIds.stream() + .map(controllerId -> DeploymentManagement + .deploymentRequest(controllerId, filterQuery.getAutoAssignDistributionSet().getId()) + .setActionType(autoAssignActionType).setWeight(filterQuery.getAutoAssignWeight().orElse(null)) + .build()) + .collect(Collectors.toList()); + } + + protected void runInUserContext(final TargetFilterQuery targetFilterQuery, final Runnable handler) { + DeploymentHelper.runInNonSystemContext(handler, + () -> Objects.requireNonNull(getAutoAssignmentInitiatedBy(targetFilterQuery)), tenantAware); + } + + protected static String getAutoAssignmentInitiatedBy(final TargetFilterQuery targetFilterQuery) { + return StringUtils.isEmpty(targetFilterQuery.getAutoAssignInitiatedBy()) ? targetFilterQuery.getCreatedBy() + : targetFilterQuery.getAutoAssignInitiatedBy(); + } +} diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/autoassign/AutoAssignChecker.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/autoassign/AutoAssignChecker.java index 9ee5852ad..ec79a7047 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/autoassign/AutoAssignChecker.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/autoassign/AutoAssignChecker.java @@ -9,7 +9,6 @@ package org.eclipse.hawkbit.repository.jpa.autoassign; import java.util.List; -import java.util.Objects; import java.util.stream.Collectors; import javax.persistence.PersistenceException; @@ -18,23 +17,16 @@ import org.eclipse.hawkbit.exception.AbstractServerRtException; import org.eclipse.hawkbit.repository.DeploymentManagement; import org.eclipse.hawkbit.repository.TargetFilterQueryManagement; import org.eclipse.hawkbit.repository.TargetManagement; -import org.eclipse.hawkbit.repository.autoassign.AutoAssignExecutor; -import org.eclipse.hawkbit.repository.jpa.utils.DeploymentHelper; -import org.eclipse.hawkbit.repository.model.Action.ActionType; -import org.eclipse.hawkbit.repository.model.DeploymentRequest; -import org.eclipse.hawkbit.repository.model.DistributionSet; +import org.eclipse.hawkbit.repository.jpa.configuration.Constants; import org.eclipse.hawkbit.repository.model.Target; import org.eclipse.hawkbit.repository.model.TargetFilterQuery; import org.eclipse.hawkbit.tenancy.TenantAware; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.transaction.PlatformTransactionManager; -import org.springframework.transaction.annotation.Isolation; import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; -import org.springframework.util.StringUtils; /** * Checks if targets need a new distribution set (DS) based on the target filter @@ -43,32 +35,12 @@ import org.springframework.util.StringUtils; * retrieved. All targets get listed per target filter query, that match the TFQ * and that don't have the auto assign DS in their action history. */ -public class AutoAssignChecker implements AutoAssignExecutor { +public class AutoAssignChecker extends AbstractAutoAssignExecutor { private static final Logger LOGGER = LoggerFactory.getLogger(AutoAssignChecker.class); - /** - * Maximum for target filter queries with auto assign DS Maximum for targets - * that are fetched in one turn - */ - private static final int PAGE_SIZE = 1000; - - /** - * The message which is added to the action status when a distribution set is - * assigned to an target. First %s is the name of the target filter. - */ - private static final String ACTION_MESSAGE = "Auto assignment by target filter: %s"; - - private final TargetFilterQueryManagement targetFilterQueryManagement; - private final TargetManagement targetManagement; - private final DeploymentManagement deploymentManagement; - - private final PlatformTransactionManager transactionManager; - - private final TenantAware tenantAware; - /** * Instantiates a new auto assign checker * @@ -86,11 +58,8 @@ public class AutoAssignChecker implements AutoAssignExecutor { public AutoAssignChecker(final TargetFilterQueryManagement targetFilterQueryManagement, final TargetManagement targetManagement, final DeploymentManagement deploymentManagement, final PlatformTransactionManager transactionManager, final TenantAware tenantAware) { - this.targetFilterQueryManagement = targetFilterQueryManagement; + super(targetFilterQueryManagement, deploymentManagement, transactionManager, tenantAware); this.targetManagement = targetManagement; - this.deploymentManagement = deploymentManagement; - this.transactionManager = transactionManager; - this.tenantAware = tenantAware; } @Override @@ -98,15 +67,7 @@ public class AutoAssignChecker implements AutoAssignExecutor { public void check() { LOGGER.debug("Auto assigned check call"); - final PageRequest pageRequest = PageRequest.of(0, PAGE_SIZE); - - final Page filterQueries = targetFilterQueryManagement.findWithAutoAssignDS(pageRequest); - - // make sure the filter queries are executed in the order of weights - for (final TargetFilterQuery filterQuery : filterQueries) { - runInUserContext(filterQuery, () -> checkByTargetFilterQueryAndAssignDS(filterQuery)); - } - + forEachFilterWithAutoAssignDS(this::checkByTargetFilterQueryAndAssignDS); } /** @@ -119,80 +80,21 @@ public class AutoAssignChecker implements AutoAssignExecutor { */ private void checkByTargetFilterQueryAndAssignDS(final TargetFilterQuery targetFilterQuery) { try { - final DistributionSet distributionSet = targetFilterQuery.getAutoAssignDistributionSet(); - int count; do { - count = runTransactionalAssignment(targetFilterQuery, distributionSet.getId()); + final List controllerIds = targetManagement + .findByTargetFilterQueryAndNonDS(PageRequest.of(0, Constants.MAX_ENTRIES_IN_STATEMENT), + targetFilterQuery.getAutoAssignDistributionSet().getId(), targetFilterQuery.getQuery()) + .getContent().stream().map(Target::getControllerId).collect(Collectors.toList()); + count = runTransactionalAssignment(targetFilterQuery, controllerIds); - } while (count == PAGE_SIZE); + } while (count == Constants.MAX_ENTRIES_IN_STATEMENT); } catch (PersistenceException | AbstractServerRtException e) { LOGGER.error("Error during auto assign check of target filter query " + targetFilterQuery.getId(), e); } } - - /** - * Runs one page of target assignments within a dedicated transaction - * - * @param targetFilterQuery - * the target filter query - * @param dsId - * distribution set id to assign - * @return count of targets - */ - private int runTransactionalAssignment(final TargetFilterQuery targetFilterQuery, final Long dsId) { - final String actionMessage = String.format(ACTION_MESSAGE, targetFilterQuery.getName()); - - return DeploymentHelper.runInNewTransaction(transactionManager, "autoAssignDSToTargets", - Isolation.READ_COMMITTED.value(), status -> { - final List deploymentRequests = createAssignmentRequests( - targetFilterQuery.getQuery(), dsId, targetFilterQuery.getAutoAssignActionType(), - targetFilterQuery.getAutoAssignWeight().orElse(null), PAGE_SIZE); - final int count = deploymentRequests.size(); - if (count > 0) { - deploymentManagement.assignDistributionSets(getAutoAssignmentInitiatedBy(targetFilterQuery), - deploymentRequests, actionMessage); - } - return count; - }); - } - - /** - * Gets all matching targets with the designated action from the target - * management - * - * @param targetFilterQuery - * the query the targets have to match - * @param dsId - * dsId the targets are not allowed to have in their action history - * @param count - * maximum amount of targets to retrieve - * @return list of targets with action type - */ - private List createAssignmentRequests(final String targetFilterQuery, final Long dsId, - final ActionType type, final Integer weight, final int count) { - final Page targets = targetManagement.findByTargetFilterQueryAndNonDS(PageRequest.of(0, count), dsId, - targetFilterQuery); - // the action type is set to FORCED per default (when not explicitly - // specified) - final ActionType autoAssignActionType = type == null ? ActionType.FORCED : type; - - return targets.getContent().stream().map(t -> DeploymentManagement.deploymentRequest(t.getControllerId(), dsId) - .setActionType(autoAssignActionType).setWeight(weight).build()).collect(Collectors.toList()); - } - - private void runInUserContext(final TargetFilterQuery targetFilterQuery, final Runnable handler) { - DeploymentHelper.runInNonSystemContext(handler, - () -> Objects.requireNonNull(getAutoAssignmentInitiatedBy(targetFilterQuery)), tenantAware); - } - - private static String getAutoAssignmentInitiatedBy(final TargetFilterQuery targetFilterQuery) { - return StringUtils.isEmpty(targetFilterQuery.getAutoAssignInitiatedBy()) ? - targetFilterQuery.getCreatedBy() : - targetFilterQuery.getAutoAssignInitiatedBy(); - } }