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

fix: Improved memory usage #21

Merged
merged 12 commits into from
Jun 27, 2023
Merged

fix: Improved memory usage #21

merged 12 commits into from
Jun 27, 2023

Conversation

svariant
Copy link
Collaborator

@svariant svariant commented Jun 26, 2023

List of Changes

  • Wrapped in AutoClosable try every InputStream
  • Generate temp directory and temp PDF file on file system
  • Read pdf from file system and save it to blob storage
  • Improved and added unit tests

Motivation and Context

Fix memory problems

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@svariant svariant requested a review from pasqualespica June 26, 2023 10:28
@svariant svariant self-assigned this Jun 26, 2023
@svariant svariant added the enhancement New feature or request label Jun 26, 2023
Copy link
Contributor

@pasqualespica pasqualespica left a comment

Choose a reason for hiding this comment

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

LGTM

BlobStorageResponse blobStorageResponse;

ReceiptBlobClientImpl blobClient = ReceiptBlobClientImpl.getInstance();

//Save to Blob Storage
try {
blobStorageResponse = blobClient.savePdfToBlobStorage(pdfEngineResponse.getPdf(), pdfFileName);
File tempPdf = new File(TEMP_FILE_PATH);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it work well with concurrency ?

@github-actions
Copy link

This pull request does not contain a valid label. Please add one of the following labels: [patch, minor, major, skip]

@github-actions
Copy link

Comment this PR with update_code to update openapi.json and format the code. Consider to use pre-commit to format the code.

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

76.2% 76.2% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

Copy link
Contributor

@pasqualespica pasqualespica left a comment

Choose a reason for hiding this comment

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

🚀 great

@pasqualespica pasqualespica merged commit c0cb893 into main Jun 27, 2023
alessio-cialini pushed a commit that referenced this pull request Oct 17, 2023
* [fix-improve-memory-usage] Managed the closure of input streams that remained open

* [fix-improve-memory-usage] Sending zip template as InputStream - removed file byte array read

* [fix-improve-memory-usage] Pdf-engine response saved to pdf file and re-read before saving it to blob storage

* [fix-improve-memory-usage] Used temporary files with unique names for storing PDFs

* [fix-improve-memory-usage] Manual delete of temp file

* [fix-improve-memory-usage] To be deleted file to commit temp folder

* [fix-improve-memory-usage] Deleted test pdf file

* [fix-improve-memory-usage] Improved temp files generation (with protected folder) and deletion

* [fix-improve-memory-usage] Improved test coverage and testing with temp files

* [fix-improve-memory-usage] Deleted file used to commit temp folder

* [fix-improve-memory-usage] Added temp directory creation and deletion - improved exceptions logging

* [fix-improve-memory-usage] Extracted save temp file method - condition on temp directory existence
@alessio-cialini alessio-cialini deleted the fix-improve-memory-usage branch November 16, 2023 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size/small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants