Skip to content

Commit

Permalink
Add timestamp to Actions (#2113)
Browse files Browse the repository at this point in the history
* Add timestamp to Actions

Signed-off-by: Vasil Ilchev <[email protected]>

* Add Timestamp to All Actions Feedback DDI/DMF

* After review

* Removed Action timestamp as we have timestamp in each ActionStatus so use that instead

* Unify to use everywhere System.currentTimeMillis()

* Add constructor w/o timestamp to DmfActionUpdateStatus

---------

Signed-off-by: Vasil Ilchev <[email protected]>
Co-authored-by: vasilchev <[email protected]>
  • Loading branch information
vasilchev and vasilchev authored Dec 4, 2024
1 parent 58e427d commit b4215a9
Show file tree
Hide file tree
Showing 17 changed files with 96 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
*
* <p>
* The answer header would look like: {
* "time": "20140511T121314",
* "timestamp": "1733218554123",
* "status": {
* "execution": "closed",
* "result": {
Expand All @@ -47,8 +47,8 @@
@JsonIgnoreProperties(ignoreUnknown = true)
public class DdiActionFeedback {

@Schema(description = "Timestamp of the action", example = "2023-08-03T12:31:41.890992967Z")
private final String time;
@Schema(description = "Timestamp of the action in milliseconds since epoch", example = "1627997501890")
private final Long timestamp;

@NotNull
@Valid
Expand All @@ -57,14 +57,19 @@ public class DdiActionFeedback {
/**
* Constructs an action-feedback
*
* @param time time of feedback
* @param timestamp time of feedback
* @param status status to be appended to the action
*/
@JsonCreator
public DdiActionFeedback(
@JsonProperty(value = "time") final String time,
@JsonProperty(value = "status", required = true) final DdiStatus status) {
this.time = time;
@JsonProperty(value = "status", required = true) final DdiStatus status,
@JsonProperty(value = "timestamp") final Long timestamp) {
this.status = status;
this.timestamp = timestamp != null ? timestamp : System.currentTimeMillis();
}

public DdiActionFeedback(final DdiStatus status) {
this(status, null);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class DdiActionFeedbackTest {
void shouldSerializeAndDeserializeObjectWithoutOptionalValues() throws IOException {
// Setup
final DdiStatus ddiStatus = new DdiStatus(DdiStatus.ExecutionStatus.CLOSED, null, null, Collections.emptyList());
final DdiActionFeedback ddiActionFeedback = new DdiActionFeedback(null, ddiStatus);
final DdiActionFeedback ddiActionFeedback = new DdiActionFeedback(ddiStatus);

// Test
final String serializedDdiActionFeedback = mapper.writeValueAsString(ddiActionFeedback);
Expand All @@ -53,46 +53,55 @@ void shouldSerializeAndDeserializeObjectWithoutOptionalValues() throws IOExcepti
@Description("Verify the correct serialization and deserialization of the model with all values provided")
void shouldSerializeAndDeserializeObjectWithOptionalValues() throws IOException {
// Setup
final String time = Instant.now().toString();
final Long timestamp = System.currentTimeMillis();
final DdiResult ddiResult = new DdiResult(DdiResult.FinalResult.SUCCESS, new DdiProgress(10, 10));
final DdiStatus ddiStatus = new DdiStatus(DdiStatus.ExecutionStatus.CLOSED, ddiResult, 200, Collections.singletonList("myMessage"));
final DdiActionFeedback ddiActionFeedback = new DdiActionFeedback(time, ddiStatus);
final DdiActionFeedback ddiActionFeedback = new DdiActionFeedback(ddiStatus, timestamp);

// Test
final String serializedDdiActionFeedback = mapper.writeValueAsString(ddiActionFeedback);
final DdiActionFeedback deserializedDdiActionFeedback = mapper.readValue(serializedDdiActionFeedback, DdiActionFeedback.class);

assertThat(serializedDdiActionFeedback).contains(time);
assertThat(deserializedDdiActionFeedback.getTime()).isEqualTo(time);
assertThat(deserializedDdiActionFeedback.getTimestamp()).isEqualTo(timestamp);
assertThat(deserializedDdiActionFeedback.getStatus()).hasToString(ddiStatus.toString());
}

@Test
@Description("Verify that deserialization fails for known properties with a wrong datatype")
void shouldFailForObjectWithWrongDataTypes() throws IOException {
// Setup
final String serializedDdiActionFeedback = "{\"time\":\"20190809T121314\",\"status\":{\"execution\": [closed],\"result\":null,\"details\":[]}}";

final String serializedDdiActionFeedback = """
{
"timestamp" : "1627997501890",
"status" : {
"execution" : "[closed]",
"result" : null,
"details" : []
}
}
""";
assertThatExceptionOfType(MismatchedInputException.class).isThrownBy(
() -> mapper.readValue(serializedDdiActionFeedback, DdiActionFeedback.class));
}

@Test
@Description("Verify that deserialization works if optional fields are not parsed")
void shouldConvertItWithoutOptionalFieldTime() throws JsonProcessingException {
void shouldConvertItWithoutOptionalFieldTimestamp() throws JsonProcessingException {
// Setup
final String serializedDdiActionFeedback = "{\n" + //
" \"status\" : {\n" + //
" \"result\" : {\n" + //
" \"finished\" : \"none\"\n" + //
" },\n" + //
" \"execution\" : \"download\",\n" + //
" \"details\" : [ \"Some message\" ]\n" + //
" }\n" + //
"}";//
final String serializedDdiActionFeedback = """
{
"status" : {
"result" : {
"finished" : "none"
},
"execution" : "download",
"details" : [ "Some message" ]
}
}
""";

assertThat(mapper.readValue(serializedDdiActionFeedback, DdiActionFeedback.class)).satisfies(deserializedDdiActionFeedback -> {
assertThat(deserializedDdiActionFeedback.getTime()).isNull();
assertThat(deserializedDdiActionFeedback.getTimestamp()).isNotNull();
assertThat(deserializedDdiActionFeedback.getStatus()).isNotNull();
assertThat(deserializedDdiActionFeedback.getStatus().getResult()).isNotNull();
assertThat(deserializedDdiActionFeedback.getStatus().getResult().getFinished()).isEqualTo(DdiResult.FinalResult.NONE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ private static void addMessageIfEmpty(final String text, final List<String> mess

private static ActionStatusCreate generateActionCancelStatus(
final DdiActionFeedback feedback, final Target target, final Long actionId, final EntityFactory entityFactory) {
final ActionStatusCreate actionStatusCreate = entityFactory.actionStatus().create(actionId);
final ActionStatusCreate actionStatusCreate = entityFactory.actionStatus().create(actionId).occurredAt(feedback.getTimestamp());
final List<String> messages = new ArrayList<>();
final Status status;
switch (feedback.getStatus().getExecution()) {
Expand Down Expand Up @@ -616,7 +616,7 @@ private ActionStatus checkAndLogDownload(final HttpServletRequest request, final
}

private ActionStatusCreate generateUpdateStatus(final DdiActionFeedback feedback, final String controllerId, final Long actionId) {
final ActionStatusCreate actionStatusCreate = entityFactory.actionStatus().create(actionId);
final ActionStatusCreate actionStatusCreate = entityFactory.actionStatus().create(actionId).occurredAt(feedback.getTimestamp());
final List<String> messages = new ArrayList<>();

if (!CollectionUtils.isEmpty(feedback.getStatus().getDetails())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ protected String getJsonActionFeedback(
final DdiStatus.ExecutionStatus executionStatus, final DdiResult ddiResult,
final List<String> messages) throws JsonProcessingException {
final DdiStatus ddiStatus = new DdiStatus(executionStatus, ddiResult, null, messages);
return OBJECT_MAPPER.writeValueAsString(new DdiActionFeedback(Instant.now().toString(), ddiStatus));
return OBJECT_MAPPER.writeValueAsString(new DdiActionFeedback(ddiStatus));
}

protected String getJsonActionFeedback(
Expand All @@ -308,7 +308,7 @@ protected String getJsonActionFeedback(
final DdiStatus.ExecutionStatus executionStatus,
final DdiResult.FinalResult finalResult, final Integer code, final List<String> messages) throws JsonProcessingException {
final DdiStatus ddiStatus = new DdiStatus(executionStatus, new DdiResult(finalResult, new DdiProgress(2, 5)), code, messages);
return OBJECT_MAPPER.writeValueAsString(new DdiActionFeedback(Instant.now().toString(), ddiStatus));
return OBJECT_MAPPER.writeValueAsString(new DdiActionFeedback(ddiStatus));
}

protected String getJsonConfirmationFeedback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ protected Message createPingMessage(final String correlationId, final String ten

protected void createAndSendActionStatusUpdateMessage(final String target, final long actionId,
final DmfActionStatus status) {
final DmfActionUpdateStatus dmfActionUpdateStatus = new DmfActionUpdateStatus(actionId, status);
final DmfActionUpdateStatus dmfActionUpdateStatus = new DmfActionUpdateStatus(actionId, status, System.currentTimeMillis());

final Message eventMessage = createUpdateActionEventMessage(dmfActionUpdateStatus);
eventMessage.getMessageProperties().getHeaders().put(MessageHeaderKey.THING_ID, target);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public class DmfActionUpdateStatus {

private final Long actionId;
private final DmfActionStatus actionStatus;
private final long timestamp;

@JsonProperty
private Long softwareModuleId;
Expand All @@ -46,9 +47,14 @@ public class DmfActionUpdateStatus {
private Integer code;

public DmfActionUpdateStatus(@JsonProperty(value = "actionId", required = true) final Long actionId,
@JsonProperty(value = "actionStatus", required = true) final DmfActionStatus actionStatus) {
@JsonProperty(value = "actionStatus", required = true) final DmfActionStatus actionStatus, @JsonProperty(value = "timestamp") final Long timestamp) {
this.actionId = actionId;
this.actionStatus = actionStatus;
this.timestamp = timestamp != null ? timestamp : System.currentTimeMillis();
}

public DmfActionUpdateStatus(final Long actionId, final DmfActionStatus actionStatus) {
this(actionId, actionStatus, null);
}

@JsonIgnore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,4 +131,5 @@ public class MgmtAction extends MgmtBaseEntity {
@JsonProperty
@Schema(description = "If created by external system this field contains the external reference for the action")
private String externalRef;

}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ public class MgmtActionStatus {
@Schema(example = "1691065929524")
private Long reportedAt;

@JsonProperty
@Schema(example = "1691065929524")
private Long timestamp;

@JsonProperty
@Schema(example = "200")
private Integer code;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ private static MgmtActionStatus toResponse(final ActionStatus actionStatus, fina

result.setMessages(messages);
result.setReportedAt(actionStatus.getCreatedAt());
result.setTimestamp(actionStatus.getOccurredAt());
result.setStatusId(actionStatus.getId());
result.setType(actionStatus.getStatus().name().toLowerCase());
actionStatus.getCode().ifPresent(result::setCode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public final class TimestampCalculator {
* @return <em>overdue_ts</em> in milliseconds since Unix epoch as long value
*/
public static long calculateOverdueTimestamp() {
return Instant.now().toEpochMilli() - getDurationForKey(TenantConfigurationKey.POLLING_TIME_INTERVAL).toMillis()
return System.currentTimeMillis() - getDurationForKey(TenantConfigurationKey.POLLING_TIME_INTERVAL).toMillis()
- getDurationForKey(TenantConfigurationKey.POLLING_OVERDUE_TIME_INTERVAL).toMillis();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
package org.eclipse.hawkbit.repository.rsql;

import java.io.Serial;
import java.time.Instant;

import org.apache.commons.lang3.text.StrLookup;
import org.apache.commons.lang3.text.StrSubstitutor;
Expand Down Expand Up @@ -54,7 +53,7 @@ public String lookup(final String rhs) {
String resolved = null;

if ("now_ts".equalsIgnoreCase(rhs)) {
resolved = String.valueOf(Instant.now().toEpochMilli());
resolved = String.valueOf(System.currentTimeMillis());
} else if ("overdue_ts".equalsIgnoreCase(rhs)) {
resolved = String.valueOf(TimestampCalculator.calculateOverdueTimestamp());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ protected JpaAction getActionAndThrowExceptionIfNotFound(final Long actionId) {
.orElseThrow(() -> new EntityNotFoundException(Action.class, actionId));
}

protected void onActionStatusUpdate(final Action.Status updatedActionStatus, final JpaAction action) {
protected void onActionStatusUpdate(final JpaActionStatus newActionStatus, final JpaAction action) {
// can be overwritten to intercept the persistence of the action status
}

Expand Down Expand Up @@ -165,7 +165,7 @@ private Action handleAddUpdateActionStatus(final JpaActionStatus actionStatus, f
assertActionStatusMessageQuota(actionStatus);
actionStatus.setAction(action);

onActionStatusUpdate(actionStatus.getStatus(), action);
onActionStatusUpdate(actionStatus, action);

actionStatusRepository.save(actionStatus);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ public void deactivateAutoConfirmation(String controllerId) {
}

@Override
protected void onActionStatusUpdate(final Status updatedActionStatus, final JpaAction action) {
if (updatedActionStatus == Status.RUNNING && action.isActive()) {
protected void onActionStatusUpdate(final JpaActionStatus newActionStatus, final JpaAction action) {
if (newActionStatus.getStatus() == Status.RUNNING && action.isActive()) {
action.setStatus(Status.RUNNING);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,9 @@ public int getWeightConsideringDefault(final Action action) {
}

@Override
protected void onActionStatusUpdate(final Action.Status updatedActionStatus, final JpaAction action) {
protected void onActionStatusUpdate(final JpaActionStatus newActionStatus, final JpaAction action) {
final Action.Status updatedActionStatus = newActionStatus.getStatus();
final long occurredAt = newActionStatus.getOccurredAt();
switch (updatedActionStatus) {
case ERROR: {
final JpaTarget target = (JpaTarget) action.getTarget();
Expand All @@ -186,7 +188,7 @@ protected void onActionStatusUpdate(final Action.Status updatedActionStatus, fin
break;
}
case FINISHED: {
handleFinishedAndStoreInTargetStatus(action).ifPresent(this::requestControllerAttributes);
handleFinishedAndStoreInTargetStatus(occurredAt, action).ifPresent(this::requestControllerAttributes);
break;
}
case DOWNLOADED: {
Expand Down Expand Up @@ -865,33 +867,35 @@ private void handleErrorOnAction(final JpaAction mergedAction, final JpaTarget m
* @return a present controllerId in case the attributes needs to be
* requested.
*/
private Optional<String> handleFinishedAndStoreInTargetStatus(final JpaAction action) {
private Optional<String> handleFinishedAndStoreInTargetStatus(final long occurredAt, final JpaAction action) {
final JpaTarget target = (JpaTarget) action.getTarget();
action.setActive(false);
action.setStatus(Status.FINISHED);
final JpaDistributionSet ds = (JpaDistributionSet) entityManager.merge(action.getDistributionSet());

target.setInstalledDistributionSet(ds);
target.setInstallationDate(System.currentTimeMillis());
if (target.getInstallationDate() == null || target.getInstallationDate() < occurredAt) {
final JpaDistributionSet ds = (JpaDistributionSet) entityManager.merge(action.getDistributionSet());

target.setInstalledDistributionSet(ds);
target.setInstallationDate(occurredAt);

// Target reported an installation of a DOWNLOAD_ONLY assignment, the
// assigned DS has to be adapted
// because the currently assigned DS can be unequal to the currently
// installed DS (the downloadOnly DS)
if (isDownloadOnly(action)) {
target.setAssignedDistributionSet((JpaDistributionSet) action.getDistributionSet());
}

// Target reported an installation of a DOWNLOAD_ONLY assignment, the
// assigned DS has to be adapted
// because the currently assigned DS can be unequal to the currently
// installed DS (the downloadOnly DS)
if (isDownloadOnly(action)) {
target.setAssignedDistributionSet((JpaDistributionSet) action.getDistributionSet());
}
// check if the assigned set is equal to the installed set (not
// necessarily the case as another update might be pending already).
if (target.getAssignedDistributionSet() != null
&& target.getAssignedDistributionSet().getId().equals(target.getInstalledDistributionSet().getId())) {
target.setUpdateStatus(TargetUpdateStatus.IN_SYNC);
}

// check if the assigned set is equal to the installed set (not
// necessarily the case as another update might be pending already).
if (target.getAssignedDistributionSet() != null
&& target.getAssignedDistributionSet().getId().equals(target.getInstalledDistributionSet().getId())) {
target.setUpdateStatus(TargetUpdateStatus.IN_SYNC);
targetRepository.save(target);
entityManager.detach(ds);
}

targetRepository.save(target);
entityManager.detach(ds);

return Optional.of(target.getControllerId());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import java.util.Map;
import java.util.Optional;

import jakarta.persistence.Access;
import jakarta.persistence.AccessType;
import jakarta.persistence.CascadeType;
import jakarta.persistence.Column;
import jakarta.persistence.ConstraintMode;
Expand Down Expand Up @@ -51,6 +53,7 @@
import org.eclipse.hawkbit.repository.model.RolloutGroup;
import org.eclipse.hawkbit.repository.model.Target;
import org.eclipse.hawkbit.repository.model.helper.EventPublisherHolder;
import org.springframework.data.annotation.CreatedDate;

/**
* JPA implementation of {@link Action}.
Expand Down Expand Up @@ -283,6 +286,7 @@ public Optional<Integer> getLastActionStatusCode() {
return Optional.ofNullable(lastActionStatusCode);
}


@Override
public Optional<ZonedDateTime> getMaintenanceWindowStartTime() {
return MaintenanceScheduleHelper.getNextMaintenanceWindow(maintenanceWindowSchedule, maintenanceWindowDuration,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ void targetSearchWithVariousFilterCombinations() {

final DistributionSet installedSet = testdataFactory.createDistributionSet("another");

final Long lastTargetQueryNotOverdue = Instant.now().toEpochMilli();
final Long lastTargetQueryNotOverdue = System.currentTimeMillis();
final Long lastTargetQueryAlwaysOverdue = 0L;

final String targetDsAIdPref = "targ-A";
Expand Down Expand Up @@ -276,7 +276,7 @@ void targetSearchWithOrderByDistributionSetAndSortParam() {
void targetSearchWithOverdueFilterAndOrderByDistributionSet() {

final Long lastTargetQueryAlwaysOverdue = 0L;
final long lastTargetQueryNotOverdue = Instant.now().toEpochMilli();
final long lastTargetQueryNotOverdue = System.currentTimeMillis();

final Long[] overdueMix = { lastTargetQueryAlwaysOverdue, lastTargetQueryNotOverdue,
lastTargetQueryAlwaysOverdue, null, lastTargetQueryAlwaysOverdue };
Expand Down
Loading

0 comments on commit b4215a9

Please sign in to comment.