[NAE 1843] Import/Export services #296
Draft
+3,792
−26
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- added CaseExporter class - implementation of case export - work in progress
- added CaseImporter - added service CaseImportExportService and controller CaseImportExportController - added schema location to properties - functionality changes to CaseExporter - added full xsd schema to petriflow_schema.xsd for testing purposes - added new dependecy zip4j - work in progress
- service name typo fix
- changes to case import and export functionality - translations are now exported - import and export of roles removed - implementation of zip service added - work in progress on tests
- removal of unused dependency
SamuelPalaj
requested review from
Retoocs,
tuplle,
camperko,
Kovy95,
MartinSiran,
machacjozef and
mazarijuraj
Retoocs
requested changes
Mar 24, 2025
src/main/java/com/netgrif/application/engine/workflow/domain/CaseExportFiles.java
Outdated
Show resolved
Hide resolved
src/main/java/com/netgrif/application/engine/archive/ZipService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/netgrif/application/engine/workflow/domain/CaseExportFiles.java
Outdated
Show resolved
Hide resolved
src/main/java/com/netgrif/application/engine/workflow/service/CaseExportImportService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/netgrif/application/engine/workflow/service/CaseExportImportService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/netgrif/application/engine/workflow/service/CaseExportImportService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/netgrif/application/engine/workflow/service/CaseExportImportService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/netgrif/application/engine/workflow/service/CaseExporter.java
Outdated
Show resolved
Hide resolved
src/main/java/com/netgrif/application/engine/workflow/service/CaseExporter.java
Outdated
Show resolved
Hide resolved
- CaseImportExportTest tests improved, added test net nae_1843.xml - added nae.case.export.file-name property - improved error handling - multiple bug fixes and small optimizations
Retoocs
requested changes
Mar 31, 2025
src/test/groovy/com/netgrif/application/engine/workflow/CaseImportExportTest.groovy
Outdated
Show resolved
Hide resolved
src/test/groovy/com/netgrif/application/engine/workflow/CaseImportExportTest.groovy
Show resolved
Hide resolved
- PR comment changes
tuplle
requested changes
Apr 27, 2025
| for (String fileName : field.right) { | ||
| String filePath = storageService.getPath(caseId, storageField.getStringId(), fileName); | ||
| InputStream fis = storageService.get(storageField, filePath); | ||
| String newFileName = caseId.concat(File.separator).concat(storageField.getStringId()).concat(File.separator).concat(fileName); |
There was a problem hiding this comment.
Better than concatenating strings with File.separator will be to use Paths.get() ...
Paths.get(caseId, storageField.toString(), filename)
| import lombok.Data; | ||
|
|
||
| @Data | ||
| public class CaseExportProperties { |
There was a problem hiding this comment.
Why is this a separate class? This class is used only in the properties bean.
| @Slf4j | ||
| @Data | ||
| @Component | ||
| @ConfigurationProperties(prefix = "nae.case") |
There was a problem hiding this comment.
As this is the bean for properties specifically for case import and export, the prefix should be unique nae.case.export .
| try { | ||
| marshallCase(xmlCases); | ||
| } catch (JAXBException e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
Please log the error even if it is rethrown right after. At least log.error(e.getMeesage(), e)
| @RestController() | ||
| @RequestMapping("/api/case") | ||
| @AllArgsConstructor | ||
| public class CaseImportExportController { |
There was a problem hiding this comment.
For consistency reasons, shouldn't there be two controllers? One is for export endpoints, and the other is for import endpoints.
| <xs:include schemaLocation="https://petriflow.com/petriflow.schema.xsd"/> | ||
|
|
||
| <!--todo use include after new schema is published--> | ||
| <xs:element name="cases"> |
There was a problem hiding this comment.
This schema should be in Petriflow project, and only reference should be here.
There was a problem hiding this comment.
This was meant as a temporary workaround for development purposes, schema will be referenced after updated version is released
- implemented changes requested by PR reviewers - CaseImportExportController split into 2 separate controller CaseExportController and CaseImportController - CaseExportFiles refactored to be more readable, added StorageFieldWithFileNames wrapper class - CaseImportExportProperties renamed to CaseImportProperties and removed redundant nested object
- refactor of importCasesWithFiles method to get rid of redundant database calls in saveFiles
|
For me, this draft PR looks good at the moment. I will come back once the tests and documentation are pushed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Implements NAE-1843
Dependencies
No new dependencies were introduced
Third party dependencies
No new dependencies were introduced
Blocking Pull requests
There are no dependencies on other PR
How Has Been This Tested?
Manually, unit tests are WIP
Test Configuration
Checklist: