Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#1249] Don't create action statuses to log range downloads #2261

Merged
merged 1 commit into from
Jan 31, 2025
Merged

[#1249] Don't create action statuses to log range downloads #2261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 18 additions & 20 deletions ...t-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,15 +203,21 @@ public ResponseEntity<InputStream> downloadArtifact(
if (ifMatch != null && !HttpUtil.matchesHttpHeader(ifMatch, artifact.getSha1Hash())) {
result = new ResponseEntity<>(HttpStatus.PRECONDITION_FAILED);
} else {
final ActionStatus action = checkAndLogDownload(RequestResponseContextHolder.getHttpServletRequest(), target, module.getId());
final Long statusId = action.getId();
result = FileStreamingUtil.writeFileResponse(file, artifact.getFilename(), artifact.getCreatedAt(),
RequestResponseContextHolder.getHttpServletResponse(),
RequestResponseContextHolder.getHttpServletRequest(),
(length, shippedSinceLastEvent,
total) -> eventPublisher.publishEvent(new DownloadProgressEvent(
tenantAware.getCurrentTenant(), statusId, shippedSinceLastEvent,
serviceMatcher != null ? serviceMatcher.getBusId() : bus.getId())));
(length, shippedSinceLastEvent, total) -> {
if (RequestResponseContextHolder.getHttpServletRequest().getHeader("Range") != null) {
// range request - could have too many - so doesn't log action status or push events
return;
}

final ActionStatus actionStatus = logDownload(
RequestResponseContextHolder.getHttpServletRequest(), target, module.getId());
eventPublisher.publishEvent(new DownloadProgressEvent(
tenantAware.getCurrentTenant(), actionStatus.getId(), shippedSinceLastEvent,
serviceMatcher != null ? serviceMatcher.getBusId() : bus.getId()));
});
}
}
return result;
Expand All @@ -238,10 +244,9 @@ public ResponseEntity<Void> downloadArtifactMd5(
final Artifact artifact = module.getArtifactByFilename(fileName)
.orElseThrow(() -> new EntityNotFoundException(Artifact.class, fileName));

checkAndLogDownload(RequestResponseContextHolder.getHttpServletRequest(), target, module.getId());

try {
writeMD5FileResponse(RequestResponseContextHolder.getHttpServletResponse(), artifact.getMd5Hash(), fileName);
logDownload(RequestResponseContextHolder.getHttpServletRequest(), target, module.getId());
} catch (final IOException e) {
log.error("Failed to stream MD5 File", e);
return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
Expand Down Expand Up @@ -609,22 +614,15 @@ private static void writeMD5FileResponse(final HttpServletResponse response, fin
response.getOutputStream().write(content);
}

private ActionStatus checkAndLogDownload(final HttpServletRequest request, final Target target, final Long module) {
private ActionStatus logDownload(final HttpServletRequest request, final Target target, final Long module) {
final Action action = controllerManagement
.getActionForDownloadByTargetAndSoftwareModule(target.getControllerId(), module)
.orElseThrow(() -> new SoftwareModuleNotAssignedToTargetException(module, target.getControllerId()));
final String range = request.getHeader("Range");

final String message;
if (range != null) {
message = RepositoryConstants.SERVER_MESSAGE_PREFIX +
"Target downloads range " + range + " of: " + request.getRequestURI();
} else {
message = RepositoryConstants.SERVER_MESSAGE_PREFIX + "Target downloads " + request.getRequestURI();
}

return controllerManagement.addInformationalActionStatus(
entityFactory.actionStatus().create(action.getId()).status(Status.DOWNLOAD).message(message));
entityFactory.actionStatus()
.create(action.getId())
.status(Status.DOWNLOAD)
.message(RepositoryConstants.SERVER_MESSAGE_PREFIX + "Target downloads " + request.getRequestURI()));
}

private ActionStatusCreate generateUpdateStatus(final DdiActionFeedback feedback, final String controllerId, final Long actionId) {
Expand Down
30 changes: 14 additions & 16 deletions ...resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiArtifactDownloadTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,21 +57,21 @@
@Feature("Component Tests - Direct Device Integration API")
@Story("Artifact Download Resource")
@SpringBootTest(classes = { DownloadTestConfiguration.class })
public class DdiArtifactDownloadTest extends AbstractDDiApiIntegrationTest {
class DdiArtifactDownloadTest extends AbstractDDiApiIntegrationTest {

private static volatile int downLoadProgress = 0;
private static volatile int downloadProgress = 0;
private static volatile long shippedBytes = 0;

private final SimpleDateFormat dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.ENGLISH);

@BeforeEach
public void setup() {
void setup() {
dateFormat.setTimeZone(TimeZone.getTimeZone("GMT"));
}

@Test
@Description("Tests non allowed requests on the artifact ressource, e.g. invalid URI, wrong if-match, wrong command.")
public void invalidRequestsOnArtifactResource() throws Exception {
void invalidRequestsOnArtifactResource() throws Exception {
// create target
final Target target = testdataFactory.createTarget();
final List<Target> targets = Collections.singletonList(target);
Expand Down Expand Up @@ -160,8 +160,8 @@ public void invalidRequestsOnArtifactResource() throws Exception {
@Test
@WithUser(principal = "4712", authorities = "ROLE_CONTROLLER", allSpPermissions = true)
@Description("Tests valid downloads through the artifact resource by identifying the artifact not by ID but file name.")
public void downloadArtifactThroughFileName() throws Exception {
downLoadProgress = 1;
void downloadArtifactThroughFileName() throws Exception {
downloadProgress = 1;
shippedBytes = 0;
assertThat(softwareModuleManagement.findAll(PAGE)).isEmpty();

Expand Down Expand Up @@ -199,13 +199,13 @@ public void downloadArtifactThroughFileName() throws Exception {
"The same file that was uploaded is expected when downloaded");

// download complete
assertThat(downLoadProgress).isEqualTo(10);
assertThat(downloadProgress).isEqualTo(10);
assertThat(shippedBytes).isEqualTo(artifactSize);
}

@Test
@Description("Tests valid MD5SUm file downloads through the artifact resource by identifying the artifact by ID.")
public void downloadMd5sumThroughControllerApi() throws Exception {
void downloadMd5sumThroughControllerApi() throws Exception {
// create target
final Target target = testdataFactory.createTarget();

Expand Down Expand Up @@ -236,7 +236,7 @@ public void downloadMd5sumThroughControllerApi() throws Exception {
@Test
@WithUser(principal = TestdataFactory.DEFAULT_CONTROLLER_ID, authorities = "ROLE_CONTROLLER", allSpPermissions = true)
@Description("Test various HTTP range requests for artifact download, e.g. chunk download or download resume.")
public void rangeDownloadArtifact() throws Exception {
void rangeDownloadArtifact() throws Exception {
// create target
final Target target = testdataFactory.createTarget();
final List<Target> targets = Collections.singletonList(target);
Expand Down Expand Up @@ -312,8 +312,7 @@ public void rangeDownloadArtifact() throws Exception {
.andExpect(header().string("Accept-Ranges", "bytes"))
.andExpect(header().string("Last-Modified", dateFormat.format(new Date(artifact.getCreatedAt()))))
.andExpect(header().longValue("Content-Length", resultLength - 1000))
.andExpect(header().string("Content-Range",
"bytes " + 1000 + "-" + (resultLength - 1) + "/" + resultLength))
.andExpect(header().string("Content-Range", "bytes " + 1000 + "-" + (resultLength - 1) + "/" + resultLength))
.andExpect(header().string("Content-Disposition", "attachment;filename=file1"))
.andReturn();

Expand Down Expand Up @@ -361,10 +360,10 @@ public void rangeDownloadArtifact() throws Exception {
}

@Configuration
public static class DownloadTestConfiguration {
static class DownloadTestConfiguration {

@Bean
public Listener cancelEventHandlerStubBean() {
Listener cancelEventHandlerStubBean() {
return new Listener();
}

Expand All @@ -373,10 +372,9 @@ public Listener cancelEventHandlerStubBean() {
private static class Listener {

@EventListener(classes = DownloadProgressEvent.class)
public static void listen(final DownloadProgressEvent event) {
downLoadProgress++;
void listen(final DownloadProgressEvent event) {
downloadProgress++;
shippedBytes += event.getShippedBytesSinceLast();

}
}
}
2 changes: 1 addition & 1 deletion ...i-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiDeploymentBaseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,7 @@ private static class ActionStatusCondition extends Condition<ActionStatus> {

private final Action.Status status;

public ActionStatusCondition(final Action.Status status) {
private ActionStatusCondition(final Action.Status status) {
this.status = status;
}

Expand Down
30 changes: 15 additions & 15 deletions ...di-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiInstalledBaseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
*/
@Feature("Component Tests - Direct Device Integration API")
@Story("Installed Base Resource")
public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {

@Autowired
ActionStatusRepository actionStatusRepository;
Expand All @@ -78,7 +78,7 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {

@Test
@Description("Ensure that the installed base resource is available as CBOR")
public void installedBaseResourceCbor() throws Exception {
void installedBaseResourceCbor() throws Exception {
final Target target = testdataFactory.createTarget();
final DistributionSet ds = testdataFactory.createDistributionSet("");

Expand All @@ -100,7 +100,7 @@ public void installedBaseResourceCbor() throws Exception {

@Test
@Description("Ensure that assigned version is self assigned version")
public void installedVersion() throws Exception {
void installedVersion() throws Exception {
final Target target = createTargetAndAssertNoActiveActions();
final DistributionSet ds = testdataFactory.createDistributionSet("");

Expand All @@ -115,7 +115,7 @@ public void installedVersion() throws Exception {

@Test
@Description("Ensure that installedVersion is version self assigned")
public void installedVersionNotExist() throws Exception {
void installedVersionNotExist() throws Exception {
final Target target = createTargetAndAssertNoActiveActions();
final String dsName = "unknown";
final String dsVersion = "1.0.0";
Expand All @@ -128,7 +128,7 @@ public void installedVersionNotExist() throws Exception {

@Test
@Description("Test several deployments to a controller. Checks that action is represented as installedBase after installation.")
public void deploymentSeveralActionsInInstalledBase() throws Exception {
void deploymentSeveralActionsInInstalledBase() throws Exception {
// Prepare test data
final Target target = createTargetAndAssertNoActiveActions();

Expand Down Expand Up @@ -196,7 +196,7 @@ public void deploymentSeveralActionsInInstalledBase() throws Exception {

@Test
@Description("Test several deployments of same ds to a controller. Checks that cancelled action in history is not linked as installedBase.")
public void deploymentActionsOfSameDsWithCancelledActionInHistory() throws Exception {
void deploymentActionsOfSameDsWithCancelledActionInHistory() throws Exception {
// Prepare test data
final Target target = createTargetAndAssertNoActiveActions();

Expand Down Expand Up @@ -252,7 +252,7 @@ public void deploymentActionsOfSameDsWithCancelledActionInHistory() throws Excep

@Test
@Description("Test several deployments of same ds to a controller. Checks that latest cancelled action does not override actual installed ds.")
public void deploymentActionsOfSameDsWithCancelledAction() throws Exception {
void deploymentActionsOfSameDsWithCancelledAction() throws Exception {
// Prepare test data
final Target target = createTargetAndAssertNoActiveActions();

Expand Down Expand Up @@ -309,7 +309,7 @@ public void deploymentActionsOfSameDsWithCancelledAction() throws Exception {

@Test
@Description("Test several deployments of same ds to a controller. Checks that latest running action does not override actual installed ds.")
public void deploymentActionsOfSameDsWithRunningAction() throws Exception {
void deploymentActionsOfSameDsWithRunningAction() throws Exception {
// Prepare test data
final Target target = createTargetAndAssertNoActiveActions();

Expand Down Expand Up @@ -362,7 +362,7 @@ public void deploymentActionsOfSameDsWithRunningAction() throws Exception {

@Test
@Description("Test open deployment to a controller. Checks that installedBase returns 404 for a pending action.")
public void installedBaseReturns404ForPendingAction() throws Exception {
void installedBaseReturns404ForPendingAction() throws Exception {
// Prepare test data
final Target target = createTargetAndAssertNoActiveActions();
final DistributionSet ds = testdataFactory.createDistributionSet("");
Expand All @@ -385,7 +385,7 @@ public void installedBaseReturns404ForPendingAction() throws Exception {

@Test
@Description("Ensures that artifacts are found, after the action was already closed.")
public void artifactsOfInstalledActionExist() throws Exception {
void artifactsOfInstalledActionExist() throws Exception {
final Target target = createTargetAndAssertNoActiveActions();
final DistributionSet ds = testdataFactory.createDistributionSet("");

Expand Down Expand Up @@ -419,7 +419,7 @@ public void artifactsOfInstalledActionExist() throws Exception {
@Expect(type = TargetUpdatedEvent.class, count = 2),
@Expect(type = TargetAttributesRequestedEvent.class, count = 1),
@Expect(type = TargetPollEvent.class, count = 1) })
public void deploymentActionInInstalledBase(final Action.ActionType actionType) throws Exception {
void deploymentActionInInstalledBase(final Action.ActionType actionType) throws Exception {
// Prepare test data
final Target target = createTargetAndAssertNoActiveActions();
final DistributionSet ds = testdataFactory.createDistributionSet("", true);
Expand Down Expand Up @@ -472,7 +472,7 @@ public void deploymentActionInInstalledBase(final Action.ActionType actionType)
@Expect(type = TargetUpdatedEvent.class, count = 2),
@Expect(type = TargetAttributesRequestedEvent.class, count = 1),
@Expect(type = TargetPollEvent.class, count = 2) })
public void deploymentDownloadOnlyActionNotInInstalledBase() throws Exception {
void deploymentDownloadOnlyActionNotInInstalledBase() throws Exception {
// Prepare test data
final Target target = testdataFactory.createTarget();
final DistributionSet ds = testdataFactory.createDistributionSet("");
Expand Down Expand Up @@ -501,7 +501,7 @@ public void deploymentDownloadOnlyActionNotInInstalledBase() throws Exception {
@ParameterizedTest
@MethodSource("org.eclipse.hawkbit.ddi.rest.resource.DdiInstalledBaseTest#actionTypeForDeployment")
@Description("Test a failed deployment to a controller. Checks that closed action is not represented as installedBase.")
public void deploymentActionFailedNotInInstalledBase(final Action.ActionType actionType) throws Exception {
void deploymentActionFailedNotInInstalledBase(final Action.ActionType actionType) throws Exception {
// Prepare test data
final Target target = testdataFactory.createTarget();
final DistributionSet ds = testdataFactory.createDistributionSet("");
Expand Down Expand Up @@ -534,7 +534,7 @@ public void deploymentActionFailedNotInInstalledBase(final Action.ActionType act

@Test
@Description("Test to verify that only a specific count of messages are returned based on the input actionHistory for getControllerInstalledAction endpoint.")
public void testActionHistoryCount() throws Exception {
void testActionHistoryCount() throws Exception {
final DistributionSet ds = testdataFactory.createDistributionSet("");
Target savedTarget = testdataFactory.createTarget("911");
savedTarget = getFirstAssignedTarget(assignDistributionSet(ds.getId(), savedTarget.getControllerId()));
Expand Down Expand Up @@ -585,7 +585,7 @@ public void testActionHistoryCount() throws Exception {

@Test
@Description("Test various invalid access attempts to the installed resource und the expected behaviour of the server.")
public void badInstalledAction() throws Exception {
void badInstalledAction() throws Exception {
final Target target = testdataFactory.createTarget(CONTROLLER_ID);

// not allowed methods
Expand Down
3 changes: 1 addition & 2 deletions hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/util/FileStreamingUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ public static ResponseEntity<InputStream> writeFileResponse(final DbArtifact art
return new ResponseEntity<>(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE);
}

// RFC: if the representation is unchanged, send me the part(s) that
// I am requesting in
// RFC: if the representation is unchanged, send me the part(s) that I am requesting in
// Range; otherwise, send me the entire representation.
checkForShortcut(request, etag, lastModified, full, ranges);

Expand Down