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.

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

feat: P4ADEV-1545 fdr ingestion actity implementation retrieval via query file validation #22

Open
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

macacia
Copy link
Collaborator

@macacia macacia commented Nov 27, 2024

Description

Implementation of the following steps:

  • validate the file (it exists, md5)
  • decrypt (in temporary directory)
  • zip security checks? (is it a zip?)
  • possible unzip (in temporary directory)

List of Changes

Motivation and Context

How Has This Been Tested?

  • Pre-Deploy Test
    • Unit
    • Integration (Narrow)
  • Post-Deploy Test
    • Isolated Microservice
    • Broader Integration
    • Acceptance
    • Performance & Load

Types of changes

  • PATCH - Bug fix (backwards compatible bug fixes)
  • MINOR - New feature (add functionality in a backwards compatible manner)
  • MAJOR - Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Copy link

sonarcloud bot commented Nov 27, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
3 Security Hotspots
38.6% Coverage on New Code (required ≥ 80%)
E Security Rating on New Code (required ≥ A)
D Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Copy link

sonarcloud bot commented Nov 29, 2024

* Interface for the ReportingFlowIngestionActivity.
* Defines methods for processing files based on an IngestionFlow ID.
*/
public interface ReportingFlowIngestionActivity {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public interface ReportingFlowIngestionActivity {
public interface PaymentsReportingIngestionFlowActivity {


/**
* Interface for the ReportingFlowIngestionActivity.
* Defines methods for processing files based on an IngestionFlow ID.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Defines methods for processing files based on an IngestionFlow ID.
* Defines methods for processing payments reporting files based on an IngestionFlow ID.

public interface ReportingFlowIngestionActivity {

/**
* Processes a file based on the provided IngestionFlow ID.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Processes a file based on the provided IngestionFlow ID.
* Processes a payments reporting file based on the provided IngestionFlow ID.

Comment on lines 13 to 16
/**
* Implementation of the `ReportingFlowIngestionActivity` interface.
* Manages the ingestion of reporting flow files, including validation and processing.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the javadoc is required just on Activities interface (Class and methods), the implementation will inherit it.
(also the DAO interfaces, just methods, should have javadoc in order to describe what should be implemented)


@Slf4j
@Component
public class ReportingFlowIngestionActivityImpl implements ReportingFlowIngestionActivity {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public class ReportingFlowIngestionActivityImpl implements ReportingFlowIngestionActivity {
public class PaymentsReportingIngestionFlowActivityImpl implements PaymentsReportingIngestionFlowActivity {

*
* @param zipEntry the ZIP entry to validate.
* @param targetDir the target directory for extraction.
* @return a safe, normalized path within the target directory.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @return a safe, normalized path within the target directory.
* @return a safe, normalized path towards zipEntry within the target directory.

* <li>An I/O error occurs during extraction.</li>
* </ul>
*/
public static void unzip(Path source, Path target) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method could return the list of unzipped Path

log.debug("Validating ZIP file: {}", temporaryZipFilePath);
FileUtils.isArchive(temporaryZipFilePath);

String unzippedFilename = filenameNoCipher.replace(".zip", ".xml");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FileUtils.unzip will accept a directory, not the final unzipped filename
thus you have to call the unzip with just the temporaryPath, using its returned value to retrieve the Path to the unzipped file (you have to modify unzip method to return the list of unzip files

we could let this setupProcess to return a List or (letting the invoking method to validate the presence of just 1 file) or set here the check of the unzipped file number

Path encryptedFilePath = relativePathDir.resolve(filename);
FileUtils.validateFile(encryptedFilePath);

Path temporaryPath = relativePathDir.resolve(TEMPORARY_PATH);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if TEMPORARY_PATH will be Paths.of("/tmp"), this could be
TEMPORARY_PATH.resolve(relativePathDir.subpath(0, relativePathDir.getNameCount()))

* @return the path to the extracted XML file.
* @throws IOException if any file operation fails during the setup process.
*/
public Path setUpProcess(String relativePath, String filename) throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class could be just IngestionFileRetrieverService instead of a generic Handler

Suggested change
public Path setUpProcess(String relativePath, String filename) throws IOException {
public Path retrieveFile(String relativePath, String filename) throws IOException {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants