diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a41dc59..8daad0c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. ### Bug Fixes - Keep a single `updateReplicateStatus` method in `ReplicatesService`. (#670) +- Check result has been uploaded for TEE tasks. (#672) ### Quality diff --git a/src/main/java/com/iexec/core/replicate/ReplicatesService.java b/src/main/java/com/iexec/core/replicate/ReplicatesService.java index 76584c96..dcbe19bd 100644 --- a/src/main/java/com/iexec/core/replicate/ReplicatesService.java +++ b/src/main/java/com/iexec/core/replicate/ReplicatesService.java @@ -48,6 +48,7 @@ import static com.iexec.common.replicate.ReplicateStatus.*; import static com.iexec.common.replicate.ReplicateStatusCause.REVEAL_TIMEOUT; +import static com.iexec.commons.poco.chain.DealParams.IPFS_RESULT_STORAGE_PROVIDER; @Slf4j @Service @@ -551,17 +552,14 @@ public boolean isResultUploaded(String chainTaskId) { } public boolean isResultUploaded(TaskDescription task) { - // Offchain computing - basic & tee - if (task.containsCallback()) { - return true; - } + final boolean hasIpfsStorageProvider = IPFS_RESULT_STORAGE_PROVIDER.equals(task.getResultStorageProvider()); - // Cloud computing - tee - if (task.isTeeTask()) { - return true; // pushed from enclave + // Offchain computing or TEE task with private storage + if (task.containsCallback() || (task.isTeeTask() && !hasIpfsStorageProvider)) { + return true; } - // Cloud computing - basic + // Cloud computing, upload to IPFS - basic & TEE return resultService.isResultUploaded(task.getChainTaskId()); } diff --git a/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java b/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java index e19ee356..43502292 100644 --- a/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java +++ b/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java @@ -30,6 +30,8 @@ import io.vavr.control.Either; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.Mockito; @@ -52,6 +54,8 @@ import static com.iexec.common.replicate.ReplicateStatus.*; import static com.iexec.common.replicate.ReplicateStatusModifier.WORKER; +import static com.iexec.commons.poco.chain.DealParams.DROPBOX_RESULT_STORAGE_PROVIDER; +import static com.iexec.commons.poco.chain.DealParams.IPFS_RESULT_STORAGE_PROVIDER; import static com.iexec.commons.poco.utils.TestUtils.*; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.*; @@ -796,12 +800,14 @@ void shouldGetReplicateWithResultUploadedStatus() { // region isResultUploaded - @Test - void shouldCheckResultServiceAndReturnTrue() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void shouldCheckResultServiceAndReturnTrue(boolean isTeeTask) { TaskDescription taskDescription = TaskDescription.builder() .chainTaskId(CHAIN_TASK_ID) .callback(BytesUtils.EMPTY_ADDRESS) - .isTeeTask(false) + .isTeeTask(isTeeTask) + .resultStorageProvider(IPFS_RESULT_STORAGE_PROVIDER) .build(); when(iexecHubService.getTaskDescription(CHAIN_TASK_ID)) .thenReturn(taskDescription); @@ -812,12 +818,14 @@ void shouldCheckResultServiceAndReturnTrue() { verify(resultService).isResultUploaded(CHAIN_TASK_ID); } - @Test - void shouldCheckResultServiceAndReturnFalse() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void shouldCheckResultServiceAndReturnFalse(boolean isTeeTask) { TaskDescription taskDescription = TaskDescription.builder() .chainTaskId(CHAIN_TASK_ID) .callback(BytesUtils.EMPTY_ADDRESS) - .isTeeTask(false) + .isTeeTask(isTeeTask) + .resultStorageProvider(IPFS_RESULT_STORAGE_PROVIDER) .build(); when(iexecHubService.getTaskDescription(CHAIN_TASK_ID)) .thenReturn(taskDescription); @@ -838,12 +846,13 @@ void shouldReturnFalseSinceTaskNotFound() { verify(resultService, never()).isResultUploaded(CHAIN_TASK_ID); } - @Test - void shouldReturnTrueForCallbackTask() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void shouldReturnTrueForCallbackTask(boolean isTeeTask) { TaskDescription taskDescription = TaskDescription.builder() .chainTaskId(CHAIN_TASK_ID) .callback("callback") - .isTeeTask(false) + .isTeeTask(isTeeTask) .build(); when(iexecHubService.getTaskDescription(CHAIN_TASK_ID)) .thenReturn(taskDescription); @@ -854,18 +863,19 @@ void shouldReturnTrueForCallbackTask() { } @Test - void shouldReturnTrueForTeeTask() { + void shouldReturnTrueIfPrivateStorageForTeeTask() { TaskDescription taskDescription = TaskDescription.builder() .chainTaskId(CHAIN_TASK_ID) .callback(BytesUtils.EMPTY_ADDRESS) .isTeeTask(true) + .resultStorageProvider(DROPBOX_RESULT_STORAGE_PROVIDER) .build(); when(iexecHubService.getTaskDescription(CHAIN_TASK_ID)) .thenReturn(taskDescription); boolean isResultUploaded = replicatesService.isResultUploaded(CHAIN_TASK_ID); assertThat(isResultUploaded).isTrue(); - verify(resultService, never()).isResultUploaded(CHAIN_TASK_ID); + verify(resultService, never()).isResultUploaded(any()); } // endregion @@ -992,7 +1002,7 @@ void shouldAuthorizeUpdateOnResultUploadFailed() { .build(); UpdateReplicateStatusArgs updateReplicateStatusArgs = UpdateReplicateStatusArgs .builder() - .taskDescription(TaskDescription.builder().build()) + .taskDescription(TaskDescription.builder().resultStorageProvider(IPFS_RESULT_STORAGE_PROVIDER).build()) .build(); assertThat(replicatesService.canUpdateReplicateStatus(replicate, statusUpdate, updateReplicateStatusArgs)) @@ -1017,24 +1027,6 @@ void shouldNotAuthorizeUpdateOnResultUploadFailedSinceResultUploadedWithCallback .isEqualTo(ReplicateStatusUpdateError.GENERIC_CANT_UPDATE); } - @Test - void shouldNotAuthorizeUpdateOnResultUploadFailedSinceResultUploadedWithTee() { - Replicate replicate = new Replicate(WALLET_WORKER_1, CHAIN_TASK_ID); - replicate.updateStatus(RESULT_UPLOADING, ReplicateStatusModifier.WORKER); - - ReplicateStatusUpdate statusUpdate = ReplicateStatusUpdate.builder() - .modifier(WORKER) - .status(RESULT_UPLOAD_FAILED) - .build(); - UpdateReplicateStatusArgs updateReplicateStatusArgs = UpdateReplicateStatusArgs - .builder() - .taskDescription(TaskDescription.builder().isTeeTask(true).build()) - .build(); - - assertThat(replicatesService.canUpdateReplicateStatus(replicate, statusUpdate, updateReplicateStatusArgs)) - .isEqualTo(ReplicateStatusUpdateError.GENERIC_CANT_UPDATE); - } - @Test void shouldNotAuthorizeUpdateOnContributedSinceNoBlockAvailable() { final Replicate replicate = new Replicate(WALLET_WORKER_1, CHAIN_TASK_ID); @@ -1229,6 +1221,7 @@ void shouldAuthorizeUpdateOnContributeAndFinalizeDone() { .thenReturn(true); when(iexecHubService.getTaskDescription(CHAIN_TASK_ID)).thenReturn(task); when(iexecHubService.isTaskInCompletedStatusOnChain(CHAIN_TASK_ID)).thenReturn(true); + when(resultService.isResultUploaded(CHAIN_TASK_ID)).thenReturn(true); assertThat(replicatesService.canUpdateReplicateStatus(replicate, statusUpdate, null)) .isEqualTo(ReplicateStatusUpdateError.NO_ERROR); @@ -1261,7 +1254,11 @@ void shouldNotAuthorizeUpdateOnContributeAndFinalizeDoneWhenNotUploaded() { .modifier(WORKER) .status(CONTRIBUTE_AND_FINALIZE_DONE) .build(); - final TaskDescription task = TaskDescription.builder().chainTaskId(CHAIN_TASK_ID).build(); + final TaskDescription task = TaskDescription + .builder() + .chainTaskId(CHAIN_TASK_ID) + .resultStorageProvider(IPFS_RESULT_STORAGE_PROVIDER) + .build(); when(iexecHubService.repeatIsRevealedTrue(CHAIN_TASK_ID, WALLET_WORKER_1)) .thenReturn(true);