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

In case of range request:
* there could be too many action checks
* there could be too many action statuses - filling the db and hitting the quote

So, this PR skip action existing check and skip logging (following the approach
from #1331 PR by @zyga)

Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>
This commit is contained in:
Avgustin Marinov
2025-01-31 13:04:11 +02:00
committed by GitHub
parent 44487a9575
commit 406f420f4d
5 changed files with 49 additions and 54 deletions

View File

@@ -203,15 +203,21 @@ public class DdiRootController implements DdiRootControllerRestApi {
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;
@@ -238,10 +244,9 @@ public class DdiRootController implements DdiRootControllerRestApi {
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);
@@ -609,22 +614,15 @@ public class DdiRootController implements DdiRootControllerRestApi {
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) {

View File

@@ -57,21 +57,21 @@ import org.springframework.test.web.servlet.MvcResult;
@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);
@@ -160,8 +160,8 @@ public class DdiArtifactDownloadTest extends AbstractDDiApiIntegrationTest {
@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();
@@ -199,13 +199,13 @@ public class DdiArtifactDownloadTest extends AbstractDDiApiIntegrationTest {
"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();
@@ -236,7 +236,7 @@ public class DdiArtifactDownloadTest extends AbstractDDiApiIntegrationTest {
@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);
@@ -312,8 +312,7 @@ public class DdiArtifactDownloadTest extends AbstractDDiApiIntegrationTest {
.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();
@@ -361,10 +360,10 @@ public class DdiArtifactDownloadTest extends AbstractDDiApiIntegrationTest {
}
@Configuration
public static class DownloadTestConfiguration {
static class DownloadTestConfiguration {
@Bean
public Listener cancelEventHandlerStubBean() {
Listener cancelEventHandlerStubBean() {
return new Listener();
}
@@ -373,10 +372,9 @@ public class DdiArtifactDownloadTest extends AbstractDDiApiIntegrationTest {
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();
}
}
}

View File

@@ -800,7 +800,7 @@ class DdiDeploymentBaseTest extends AbstractDDiApiIntegrationTest {
private final Action.Status status;
public ActionStatusCondition(final Action.Status status) {
private ActionStatusCondition(final Action.Status status) {
this.status = status;
}

View File

@@ -69,7 +69,7 @@ import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
*/
@Feature("Component Tests - Direct Device Integration API")
@Story("Installed Base Resource")
public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
@Autowired
ActionStatusRepository actionStatusRepository;
@@ -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("");
@@ -100,7 +100,7 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
@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("");
@@ -115,7 +115,7 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
@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";
@@ -128,7 +128,7 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
@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();
@@ -196,7 +196,7 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
@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();
@@ -252,7 +252,7 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
@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();
@@ -309,7 +309,7 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
@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();
@@ -362,7 +362,7 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
@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("");
@@ -385,7 +385,7 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
@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("");
@@ -419,7 +419,7 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
@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);
@@ -472,7 +472,7 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
@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("");
@@ -501,7 +501,7 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
@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("");
@@ -534,7 +534,7 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
@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()));
@@ -585,7 +585,7 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
@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

View File

@@ -102,8 +102,7 @@ public final class FileStreamingUtil {
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);