Add Endpoint to importCase from s3 directrory#143
Conversation
76060e4 to
df048e9
Compare
73d9dd2 to
88313fd
Compare
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
88313fd to
ca19182
Compare
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
… from S3 Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
1dec525 to
4251db0
Compare
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
f77494c to
f1eec67
Compare
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
| .param("fileName", fileName)) | ||
| .andExpect(status().isOk()); | ||
|
|
||
| assertNotNull(outputDestination.receive(1000, caseImportDestination)); |
There was a problem hiding this comment.
You can check content of notification,
THEN getObjet from s3 then check file size from resource and file size from s3
| .param("caseUuid", caseUuid.toString()) | ||
| .param("folderName", folderName) | ||
| .param("fileName", fileName)) | ||
| .andExpect(status().isInternalServerError()); |
There was a problem hiding this comment.
Should detail which message error expected and why for example, bad folder name
There was a problem hiding this comment.
No! If i do that another test will fail in another test file.
There was a problem hiding this comment.
Yes, there's a weird behavior with datasources.
But test(2)Case.xiidm was to test a specific bug.
Name it zippedTestCase.zip and it will work
src/main/java/com/powsybl/caseserver/datasource/utils/S3MultiPartFile.java
Outdated
Show resolved
Hide resolved
…artFile.java Co-authored-by: Thang PHAM <117309322+thangqp@users.noreply.github.com> Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
| @RequestParam(value = "withExpiration", required = false, defaultValue = "false") boolean withExpiration, | ||
| @RequestParam(value = "withIndexation", required = false, defaultValue = "false") boolean withIndexation) { | ||
|
|
||
| try (S3MultiPartFile mpf = new S3MultiPartFile(caseService, caseUuid, folderName, fileName + ZIP_EXTENSION, "application/zip")) { |
There was a problem hiding this comment.
Move this code to the service.
Just keep something like :
UUID uuid = caseService.importCaseFromS3(....)
return ResponseEntity.ok().body(uuid);
| @ApiResponse(responseCode = "404", description = "Source case not found"), | ||
| @ApiResponse(responseCode = "500", description = "An error occurred during the case file creation")}) | ||
| public ResponseEntity<UUID> createCase( | ||
| @RequestParam("caseUuid") UUID caseUuid, |
There was a problem hiding this comment.
caseUuid, folderName are too specific to exportNetwork structure.
You can give only one parameter folderKey
And you fill the filename something like new Path(caseKey).getFileName()
| @RequestParam(value = "withExpiration", required = false, defaultValue = "false") boolean withExpiration, | ||
| @RequestParam(value = "withIndexation", required = false, defaultValue = "false") boolean withIndexation) { | ||
|
|
||
| try (S3MultiPartFile mpf = new S3MultiPartFile(caseService, caseUuid, folderName, fileName + ZIP_EXTENSION, "application/zip")) { |
There was a problem hiding this comment.
Do not give the service to the S3MultiPartFile constructor. It's a bad practice.
Write the file on /tmp in the service. And then only give the Path to the constructor
There was a problem hiding this comment.
the problem is if we don't do that we have to manage file lifecycle (create, delete) each time from outside
src/main/java/com/powsybl/caseserver/datasource/utils/S3MultiPartFile.java
Outdated
Show resolved
Hide resolved
|
|
||
| @Override | ||
| public void transferTo(File dest) throws IOException, IllegalStateException { | ||
| Files.copy(tempFile, dest.toPath(), StandardCopyOption.REPLACE_EXISTING); |
There was a problem hiding this comment.
throw new UnsupportedOperationException("Not supported.");
or
transferTo(dest.toPath());
?
| return UUID.fromString(keyWithoutRootDirectory.substring(0, firstSlash)); | ||
| } | ||
|
|
||
| public Optional<InputStream> getInputStreamFromS3(UUID caseUuid, String folderName, String fileName) { |
There was a problem hiding this comment.
duplicated code from getCaseStream(). Code to put in common in a method
…artFile.java Co-authored-by: etiennehomer <etiennehomer@gmail.com> Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
| } | ||
|
|
||
| public UUID importCase(String folderKey, String contentType, boolean withExpiration, boolean withIndexation) throws IOException { | ||
|
|
|
|
||
| private final String name; | ||
| private final String contentType; | ||
| private final CaseService caseService; |
src/main/java/com/powsybl/caseserver/datasource/utils/S3MultiPartFile.java
Outdated
Show resolved
Hide resolved
| return caseUuid; | ||
| } | ||
|
|
||
| public UUID importCase(String folderKey, String contentType, boolean withExpiration, boolean withIndexation) throws IOException { |
There was a problem hiding this comment.
| public UUID importCase(String folderKey, String contentType, boolean withExpiration, boolean withIndexation) throws IOException { | |
| public UUID importCase(String caseFileKey, String contentType, boolean withExpiration, boolean withIndexation) throws IOException { |
There was a problem hiding this comment.
Yes, there's a weird behavior with datasources.
But test(2)Case.xiidm was to test a specific bug.
Name it zippedTestCase.zip and it will work
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
Co-authored-by: etiennehomer <etiennehomer@gmail.com> Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
…artFile.java Co-authored-by: etiennehomer <etiennehomer@gmail.com> Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
| * @author Bassel El Cheikh <bassel.el-cheikh_externe at rte-france.com> | ||
| */ | ||
|
|
||
| public class S3MultiPartFile implements MultipartFile, Closeable { |
There was a problem hiding this comment.
| public class S3MultiPartFile implements MultipartFile, Closeable { | |
| public class TmpMultiPartFile implements MultipartFile, Closeable { |
As there is no more S3 notion in this class now
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
Co-authored-by: etiennehomer <etiennehomer@gmail.com> Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
|



Please check if the PR fulfills these requirements
Does this PR already have an issue describing the problem?
No.
What kind of change does this PR introduce?
Feature
What is the current behavior?
Cases can be imported from a file.
What is the new behavior (if this is a feature change)?
Cases can be imported also from an s3 key now.
Does this PR introduce a breaking change or deprecate an API?