Improve the /download endpoint
#311
Merged
Conversation
This file contains 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
From now on, this enum will be used to indicate supported export formats within the platform. Each export format is denoted by the name of its extension. At the same time, it is also associated with a media type, which is indicated when streaming data to the client.
Aptly named `StringToExportFormatConverter`, it will be implicitly used by the controller in mapping the `String` path variable to an `ExportFormat` value.
Aptly named `ExportFormatToJsonFactoryConverter`, its use is not immediately apparent. We will use it in the `download` route to generate a factory based on the specified `ExportFormat`.
This would lead to redundant search parameter information being included in the exports. This is no longer the case, and the only internally used convenience getters are no longer generated in Jackson-produced exports.
This will realistically only impact fields with a `length`/`size` attribute. While only `String` fields are affected at the moment, this will also extend to list fields once we extend search to plural parameters.
This contains too many changes to be split into smaller, consecutive commits, so I will group them all into a larger commit. Why was this change even made in the first place? Originally, I intended to add the new `ExportFormat` and call it a day, but I noticed that there were some memory allocation issues in the download that were caused by: 1) Not using a `@PersistenceContext` entity manager 2) Not clearing the entity manager cache on a regular basis With both changes, the memory issues have been addressed. After that, my goal was only to improve the readability of the overall code. The new code did see the removal of the `exportFormats` bean, as well as changes to the `DownloadLinkBuilder`. Another change is that CSV column values will now always be quoted.
Currently only provides a means for hiding the thrown exception for the `CheckedRunnable`, and converting it to a regular `Runnable`. This will be useful in code that depends on non-throwing lambdas.
The advantage of this new class is that it accepts an arbitrary number of `Runnable` instances to execute when a specified counter condition is met. This makes the code more reusable overall. The reason this was done was to allow periodic flushes of the `JsonGenerator`. Although I'm not 100% sure if this improves the performance, we will revert it's flushing if it is determined to be of no use. This change was also the reason we introduced the `CheckedRunnable` and its `IOException`-specialized counterpart.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/main/java/ch/usi/si/seart/controller/GitRepoController.java
Dismissed
Show dismissed
Hide dismissed
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.
What started as a need for a simple change quickly escalated into a complete refactoring once a plethora of hidden issues was discovered. Here's what this PR includes:
ExportFormat;JsonGeneratoris now periodically flushed during writes;JsonFactoryinstances are now obtained from a dedicated converter;ExportConfigtoJacksonConfig;HateoasConfigfor creating link builder beans;