From a37702744c8349e141ab8a646f348a1ac4ffbce3 Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Tue, 19 Mar 2024 17:35:14 +0200 Subject: [PATCH] SDK improvements (#1693) Signed-off-by: Marinov Avgustin --- .../eclipse/hawkbit/security/DosFilter.java | 2 +- .../remote/entity/AbstractActionEvent.java | 57 +++++-------------- .../remote/entity/RemoteEntityEvent.java | 25 ++++---- .../management/JpaRolloutGroupManagement.java | 7 +-- .../ddi/rest/resource/DdiRootController.java | 6 +- .../hawkbit/sdk/device/DdiController.java | 30 +++++++--- .../hawkbit/sdk/device/UpdateHandler.java | 39 ++++++------- .../hawkbit/sdk/device/UpdateStatus.java | 20 +++---- 8 files changed, 84 insertions(+), 102 deletions(-) diff --git a/hawkbit-http-security/src/main/java/org/eclipse/hawkbit/security/DosFilter.java b/hawkbit-http-security/src/main/java/org/eclipse/hawkbit/security/DosFilter.java index 6df8154b90..c913b0c3c3 100644 --- a/hawkbit-http-security/src/main/java/org/eclipse/hawkbit/security/DosFilter.java +++ b/hawkbit-http-security/src/main/java/org/eclipse/hawkbit/security/DosFilter.java @@ -166,7 +166,7 @@ private static boolean checkIpFails(final String ip) { } private static boolean handleMissingIpAddress(final HttpServletResponse response) { - log.error("Failed to get peer IP adress"); + log.error("Failed to get peer IP address"); response.setStatus(HttpStatus.INTERNAL_SERVER_ERROR.value()); return false; } diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/event/remote/entity/AbstractActionEvent.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/event/remote/entity/AbstractActionEvent.java index b4ac425c53..5d68120ef1 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/event/remote/entity/AbstractActionEvent.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/event/remote/entity/AbstractActionEvent.java @@ -9,14 +9,22 @@ */ package org.eclipse.hawkbit.repository.event.remote.entity; -import java.util.Objects; +import java.io.Serial; +import lombok.Data; +import lombok.EqualsAndHashCode; +import lombok.ToString; import org.eclipse.hawkbit.repository.model.Action; /** * Defines the remote event of creating a new {@link Action}. */ +@Data +@EqualsAndHashCode(callSuper = true) +@ToString(callSuper = false) public abstract class AbstractActionEvent extends RemoteEntityEvent { + + @Serial private static final long serialVersionUID = 1L; private final Long targetId; @@ -36,16 +44,11 @@ protected AbstractActionEvent() { /** * Constructor * - * @param action - * the created action - * @param targetId - * targetId identifier (optional) - * @param rolloutId - * rollout identifier (optional) - * @param rolloutGroupId - * rollout group identifier (optional) - * @param applicationId - * the origin application id + * @param action the created action + * @param targetId targetId identifier (optional) + * @param rolloutId rollout identifier (optional) + * @param rolloutGroupId rollout group identifier (optional) + * @param applicationId the origin application id */ protected AbstractActionEvent(final Action action, final Long targetId, final Long rolloutId, final Long rolloutGroupId, final String applicationId) { @@ -54,34 +57,4 @@ protected AbstractActionEvent(final Action action, final Long targetId, final Lo this.rolloutId = rolloutId; this.rolloutGroupId = rolloutGroupId; } - - public Long getTargetId() { - return targetId; - } - - public Long getRolloutId() { - return rolloutId; - } - - public Long getRolloutGroupId() { - return rolloutGroupId; - } - - @Override - public boolean equals(final Object o) { - if (this == o) - return true; - if (o == null || getClass() != o.getClass()) - return false; - if (!super.equals(o)) - return false; - final AbstractActionEvent that = (AbstractActionEvent) o; - return Objects.equals(targetId, that.targetId) && Objects.equals(rolloutId, that.rolloutId) - && Objects.equals(rolloutGroupId, that.rolloutGroupId); - } - - @Override - public int hashCode() { - return Objects.hash(super.hashCode(), targetId, rolloutId, rolloutGroupId); - } -} +} \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/event/remote/entity/RemoteEntityEvent.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/event/remote/entity/RemoteEntityEvent.java index 2fdcd5294c..9b99f556b7 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/event/remote/entity/RemoteEntityEvent.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/event/remote/entity/RemoteEntityEvent.java @@ -9,8 +9,11 @@ */ package org.eclipse.hawkbit.repository.event.remote.entity; +import java.io.Serial; import java.util.Optional; +import lombok.AccessLevel; +import lombok.NoArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.eclipse.hawkbit.repository.event.remote.EventEntityManagerHolder; import org.eclipse.hawkbit.repository.event.remote.RemoteIdEvent; @@ -19,32 +22,24 @@ import com.fasterxml.jackson.annotation.JsonIgnore; /** - * A base definition class for remote events which contain a tenant aware base - * entity. + * A base definition class for remote events which contain a tenant aware base entity. * * @param the type of the entity */ +@NoArgsConstructor(access = AccessLevel.PROTECTED) @Slf4j public class RemoteEntityEvent extends RemoteIdEvent { + @Serial private static final long serialVersionUID = 1L; private transient E entity; - /** - * Default constructor. - */ - protected RemoteEntityEvent() { - // for serialization libs like jackson - } - /** * Constructor. * - * @param baseEntity - * the base entity - * @param applicationId - * the origin application id + * @param baseEntity the base entity + * @param applicationId the origin application id */ protected RemoteEntityEvent(final E baseEntity, final String applicationId) { super(baseEntity.getId(), baseEntity.getTenant(), baseEntity.getClass(), applicationId); @@ -63,8 +58,8 @@ public Optional getEntity() { private E reloadEntityFromRepository() { try { final Class clazz = (Class) Class.forName(getEntityClass()); - return EventEntityManagerHolder.getInstance().getEventEntityManager().findEntity(getTenant(), getEntityId(), - clazz); + return EventEntityManagerHolder.getInstance().getEventEntityManager().findEntity( + getTenant(), getEntityId(), clazz); } catch (final ClassNotFoundException e) { log.error("Cannot reload entity because class is not found", e); } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaRolloutGroupManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaRolloutGroupManagement.java index 36d769671a..3e7753121e 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaRolloutGroupManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaRolloutGroupManagement.java @@ -95,7 +95,6 @@ public JpaRolloutGroupManagement(final RolloutGroupRepository rolloutGroupReposi final TargetRepository targetRepository, final EntityManager entityManager, final VirtualPropertyReplacer virtualPropertyReplacer, final RolloutStatusCache rolloutStatusCache, final Database database) { - this.rolloutGroupRepository = rolloutGroupRepository; this.rolloutRepository = rolloutRepository; this.actionRepository = actionRepository; @@ -108,7 +107,7 @@ public JpaRolloutGroupManagement(final RolloutGroupRepository rolloutGroupReposi @Override public Optional get(final long rolloutGroupId) { - return rolloutGroupRepository.findById(rolloutGroupId).map(rg -> (RolloutGroup) rg); + return rolloutGroupRepository.findById(rolloutGroupId).map(RolloutGroup.class::cast); } @Override @@ -146,7 +145,7 @@ public Page findByRolloutWithDetailedStatus(final Pageable pageabl .collect(Collectors.toList()); if (rolloutGroupIds.isEmpty()) { - // groups might already deleted, so return empty list. + // groups might have been already deleted, so return empty list. return new PageImpl<>(Collections.emptyList()); } @@ -155,7 +154,7 @@ public Page findByRolloutWithDetailedStatus(final Pageable pageabl for (final JpaRolloutGroup rolloutGroup : rolloutGroups) { final TotalTargetCountStatus totalTargetCountStatus = new TotalTargetCountStatus( - allStatesForRollout.get(rolloutGroup.getId()), Long.valueOf(rolloutGroup.getTotalTargets()), + allStatesForRollout.get(rolloutGroup.getId()), (long)rolloutGroup.getTotalTargets(), rolloutGroup.getRollout().getActionType()); rolloutGroup.setTotalTargetCountStatus(totalTargetCountStatus); } diff --git a/hawkbit-rest/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootController.java b/hawkbit-rest/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootController.java index ea27a59f0c..be72ad8d3d 100644 --- a/hawkbit-rest/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootController.java +++ b/hawkbit-rest/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootController.java @@ -332,7 +332,7 @@ private static HandlingType calculateUpdateType(final Action action, final Handl public ResponseEntity postDeploymentBaseActionFeedback(@Valid @RequestBody final DdiActionFeedback feedback, @PathVariable("tenant") final String tenant, @PathVariable("controllerId") final String controllerId, @PathVariable("actionId") @NotNull final Long actionId) { - log.debug("provideBasedeploymentActionFeedback for target [{},{}]: {}", controllerId, actionId, feedback); + log.debug("postDeploymentBaseActionFeedback for target [{},{}]: {}", controllerId, actionId, feedback); final Target target = findTarget(controllerId); final Action action = findActionForTarget(actionId, target); @@ -350,7 +350,6 @@ public ResponseEntity postDeploymentBaseActionFeedback(@Valid @RequestBody controllerManagement.addUpdateActionStatus(generateUpdateStatus(feedback, controllerId, actionId)); return ResponseEntity.ok().build(); - } private ActionStatusCreate generateUpdateStatus(final DdiActionFeedback feedback, final String controllerId, @@ -761,5 +760,4 @@ public ResponseEntity deactivateAutoConfirmation(final String tenant, fina confirmationManagement.deactivateAutoConfirmation(controllerId); return new ResponseEntity<>(HttpStatus.OK); } - -} +} \ No newline at end of file diff --git a/hawkbit-sdk/hawkbit-sdk-device/src/main/java/org/eclipse/hawkbit/sdk/device/DdiController.java b/hawkbit-sdk/hawkbit-sdk-device/src/main/java/org/eclipse/hawkbit/sdk/device/DdiController.java index f2b4cb8568..bcce7535c7 100644 --- a/hawkbit-sdk/hawkbit-sdk-device/src/main/java/org/eclipse/hawkbit/sdk/device/DdiController.java +++ b/hawkbit-sdk/hawkbit-sdk-device/src/main/java/org/eclipse/hawkbit/sdk/device/DdiController.java @@ -73,11 +73,13 @@ public class DdiController { private volatile ScheduledExecutorService executorService; private volatile Long currentActionId; + private volatile Long lastActionId; + /** * Creates a new device instance. * * @param tenant the tenant of the device belongs to - * @param controller the the controller + * @param controller the controller * @param hawkbitClient a factory for creating to {@link DdiRootControllerRestApi} (and used) * for communication to hawkBit */ @@ -102,6 +104,7 @@ public void start(final ScheduledExecutorService executorService) { public void stop() { executorService = null; + lastActionId = null; currentActionId = null; } @@ -121,6 +124,12 @@ private void poll() { getRequiredLink(controllerBase, DEPLOYMENT_BASE_LINK).flatMap(this::getActionWithDeployment).ifPresentOrElse(actionWithDeployment -> { final long actionId = actionWithDeployment.getKey(); if (currentActionId == null) { + if (lastActionId != null && lastActionId == actionId) { + log.info(LOG_PREFIX + "Still receive the last action {}", + getTenantId(), getControllerId(), actionId); + return; + } + log.info(LOG_PREFIX + "Process action {}", getTenantId(), getControllerId(), actionId); final DdiDeployment deployment = actionWithDeployment.getValue().getDeployment(); @@ -132,13 +141,13 @@ private void poll() { updateHandler.getUpdateProcessor(this, updateType, modules)); } else if (currentActionId != actionId) { // TODO - cancel and start new one? - log.info(LOG_PREFIX + "Action {} is canceled while in process!", getTenantId(), - getControllerId(), getCurrentActionId()); + log.info(LOG_PREFIX + "Action {} is canceled while in process (new {})!", getTenantId(), + getControllerId(), currentActionId, actionId); } // else same action - already processing }, () -> { if (currentActionId != null) { // TODO - cancel current? - log.info(LOG_PREFIX + "Action {} is canceled while in process!", getTenantId(), + log.info(LOG_PREFIX + "Action {} is canceled while in process (not returned)!", getTenantId(), getControllerId(), getCurrentActionId()); } }); @@ -218,10 +227,17 @@ public void updateAttribute(final String mode, final String key, final String va void sendFeedback(final UpdateStatus updateStatus) { log.debug(LOG_PREFIX + "Send feedback {} -> {}", getTenantId(), getControllerId(), currentActionId, updateStatus); - getDdiApi().postDeploymentBaseActionFeedback( - updateStatus.feedback(), getTenantId(), getControllerId(), currentActionId); + try { + getDdiApi().postDeploymentBaseActionFeedback(updateStatus.feedback(), getTenantId(), getControllerId(), + currentActionId); + } catch (final RuntimeException e) { + log.error(LOG_PREFIX + "Failed to send feedback {} -> {}", getTenantId(), getControllerId(), + currentActionId, updateStatus, e); + } + if (updateStatus.status() == UpdateStatus.Status.SUCCESSFUL || - updateStatus.status() == UpdateStatus.Status.ERROR) { + updateStatus.status() == UpdateStatus.Status.FAILURE) { + lastActionId = currentActionId; currentActionId = null; } } diff --git a/hawkbit-sdk/hawkbit-sdk-device/src/main/java/org/eclipse/hawkbit/sdk/device/UpdateHandler.java b/hawkbit-sdk/hawkbit-sdk-device/src/main/java/org/eclipse/hawkbit/sdk/device/UpdateHandler.java index 98894498aa..cef2e7498b 100644 --- a/hawkbit-sdk/hawkbit-sdk-device/src/main/java/org/eclipse/hawkbit/sdk/device/UpdateHandler.java +++ b/hawkbit-sdk/hawkbit-sdk-device/src/main/java/org/eclipse/hawkbit/sdk/device/UpdateHandler.java @@ -95,25 +95,27 @@ public UpdateProcessor( @Override public void run() { - ddiController.sendFeedback(new UpdateStatus(UpdateStatus.Status.RUNNING, List.of("Update begins!"))); + ddiController.sendFeedback(new UpdateStatus(UpdateStatus.Status.PROCEEDING, List.of("Update begin ..."))); if (!CollectionUtils.isEmpty(modules)) { try { final UpdateStatus updateStatus = download(); ddiController.sendFeedback(updateStatus); - if (updateStatus.status() == UpdateStatus.Status.ERROR) { + if (updateStatus.status() == UpdateStatus.Status.FAILURE) { return; } else { - ddiController.sendFeedback(update()); + if (updateType != DdiDeployment.HandlingType.SKIP) { + ddiController.sendFeedback(update()); + } } } finally { cleanup(); } } - if (updateType != DdiDeployment.HandlingType.SKIP) { + if (updateType == DdiDeployment.HandlingType.SKIP) { ddiController.sendFeedback( - new UpdateStatus(UpdateStatus.Status.SUCCESSFUL, List.of("Update complete!"))); + new UpdateStatus(UpdateStatus.Status.SUCCESSFUL, List.of("Update (download-only) completed."))); } } @@ -126,11 +128,11 @@ public void run() { protected UpdateStatus download() { ddiController.sendFeedback( new UpdateStatus( - UpdateStatus.Status.DOWNLOADING, + UpdateStatus.Status.DOWNLOAD, modules.stream().flatMap(mod -> mod.getArtifacts().stream()) - .map(art -> "Download starts for: " + art.getFilename() + + .map(art -> "Download start for: " + art.getFilename() + " with size " + art.getSize() + - " and hashes " + art.getHashes()) + " and hashes " + art.getHashes() + " ...") .collect(Collectors.toList()))); log.info(LOG_PREFIX + "Start download", ddiController.getTenantId(), ddiController.getControllerId()); @@ -146,28 +148,28 @@ protected UpdateStatus download() { } })); - log.info(LOG_PREFIX + "Download complete", ddiController.getTenantId(), ddiController.getControllerId()); + log.info(LOG_PREFIX + "Download complete.", ddiController.getTenantId(), ddiController.getControllerId()); final List messages = new LinkedList<>(); - messages.add("Download complete!"); + messages.add("Download complete."); updateStatusList.forEach(download -> messages.addAll(download.messages())); return new UpdateStatus( - updateStatusList.stream().anyMatch(status -> status.status() == UpdateStatus.Status.ERROR) ? - UpdateStatus.Status.ERROR : UpdateStatus.Status.DOWNLOADED, + updateStatusList.stream().anyMatch(status -> status.status() == UpdateStatus.Status.FAILURE) ? + UpdateStatus.Status.FAILURE : UpdateStatus.Status.DOWNLOADED, messages); } /** - * Extension point. Called after all artifacts has been successfully downloadec. An overriding implementation + * Extension point. Called after all artifacts has been successfully downloaded. An overriding implementation * may get the {@link #downloads} map and apply them */ protected UpdateStatus update() { log.info(LOG_PREFIX + "Updated", ddiController.getTenantId(), ddiController.getControllerId()); - return new UpdateStatus(UpdateStatus.Status.SUCCESSFUL, List.of("Update applied")); + return new UpdateStatus(UpdateStatus.Status.SUCCESSFUL, List.of("Update complete.")); } /** - * Extension point. Called after download and update has been finished. By default it deletes all downloaded + * Extension point. Called after download and update has been finished. By default, it deletes all downloaded * files (if any). */ protected void cleanup() { @@ -189,8 +191,7 @@ private void handleArtifact( artifact.getLink("download").ifPresentOrElse( // HTTPS link -> status.add(downloadUrl(link.getHref(), gatewayToken, targetToken, - artifact.getHashes(), artifact.getSize())) - , + artifact.getHashes(), artifact.getSize())), // HTTP () -> status.add(downloadUrl( artifact.getLink("download-http") @@ -216,7 +217,7 @@ private UpdateStatus downloadUrl( log.error(LOG_PREFIX + "Failed to download {}", ddiController.getTenantId(), ddiController.getControllerId(), url, e); return new UpdateStatus( - UpdateStatus.Status.ERROR, + UpdateStatus.Status.FAILURE, List.of("Failed to download " + url + ": " + e.getMessage())); } } @@ -273,7 +274,7 @@ private UpdateStatus readAndCheckDownloadUrl(final String url, final String gate ddiController.getTenantId(), ddiController.getControllerId()); } downloadHandler.finished(ArtifactHandler.DownloadHandler.Status.ERROR); - return new UpdateStatus(UpdateStatus.Status.ERROR, List.of(message)); + return new UpdateStatus(UpdateStatus.Status.FAILURE, List.of(message)); } }); } diff --git a/hawkbit-sdk/hawkbit-sdk-device/src/main/java/org/eclipse/hawkbit/sdk/device/UpdateStatus.java b/hawkbit-sdk/hawkbit-sdk-device/src/main/java/org/eclipse/hawkbit/sdk/device/UpdateStatus.java index aa095279e7..d925898585 100644 --- a/hawkbit-sdk/hawkbit-sdk-device/src/main/java/org/eclipse/hawkbit/sdk/device/UpdateStatus.java +++ b/hawkbit-sdk/hawkbit-sdk-device/src/main/java/org/eclipse/hawkbit/sdk/device/UpdateStatus.java @@ -24,29 +24,29 @@ public record UpdateStatus(Status status, List messages) { public enum Status { /** - * Update has been successful and response the successful update. + * Update is running (intermediate status). */ - SUCCESSFUL(DdiStatus.ExecutionStatus.CLOSED, DdiResult.FinalResult.SUCCESS, 200), + PROCEEDING(DdiStatus.ExecutionStatus.PROCEEDING, DdiResult.FinalResult.NONE, null), /** - * Update has been not successful and response the error update. + * Device starts to download. */ - ERROR(DdiStatus.ExecutionStatus.CLOSED, DdiResult.FinalResult.FAILURE, null), + DOWNLOAD(DdiStatus.ExecutionStatus.DOWNLOAD, DdiResult.FinalResult.NONE, null), /** - * Update is running (intermediate status). + * Device is finished with downloading. */ - RUNNING(DdiStatus.ExecutionStatus.PROCEEDING, DdiResult.FinalResult.NONE, null), + DOWNLOADED(DdiStatus.ExecutionStatus.DOWNLOADED, DdiResult.FinalResult.NONE, null), /** - * Device starts to download. + * Update has been successful and response the successful update. */ - DOWNLOADING(DdiStatus.ExecutionStatus.DOWNLOAD, DdiResult.FinalResult.NONE, null), + SUCCESSFUL(DdiStatus.ExecutionStatus.CLOSED, DdiResult.FinalResult.SUCCESS, 200), /** - * Device is finished with downloading. + * Update has been not successful and response the error update. */ - DOWNLOADED(DdiStatus.ExecutionStatus.DOWNLOADED, DdiResult.FinalResult.NONE, null); + FAILURE(DdiStatus.ExecutionStatus.CLOSED, DdiResult.FinalResult.FAILURE, null); private final DdiStatus.ExecutionStatus executionStatus; private final DdiResult.FinalResult finalResult;