-
Notifications
You must be signed in to change notification settings - Fork 495
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
Rewrite DatasetVersionFilesServiceBean QueryDSL queries using JPA Criteria and remove the QueryDSL library from the application dependencies #10041
Conversation
…t using JPA Criteria (pending accessStatus search criteria filtering)
…tPerContentType using JPA Criteria
…tPerCategoryName using JPA Criteria
…tPerTabularTagName using JPA Criteria
…ion and improve legibility
…tByAccessStatus using JPA Criteria
…e using JPA Criteria
…e-querydsl-queries
This comment has been minimized.
This comment has been minimized.
…ing JPA Criteria and removed QueryDSL references from the class
This comment has been minimized.
This comment has been minimized.
📦 Pushed preview images as
🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
@GPortas I think you can safely update the description to say this PR closes this issue as well: I say this because I checked https://coveralls.io/github/IQSS/dataverse and this branch sent the first Coveralls report we've received since branches that pull in the following PR was merged: Here's a screenshot: |
As mentioned in standup and Slack, I kicked off API tests here: https://github.com/gdcc/api-test-runner/actions/runs/6642486541 Using https://github.com/gdcc/api-test-runner Click Actions, then manual then “Run workflow” and change the branch like this: Here’s the issue to automate this (need prioritization): |
@pdurbin API tests are passing on: https://github.com/gdcc/api-test-runner/actions/runs/6642486541 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks
What this PR does / why we need it:
As decided at the frontend weekly meeting on 19th October, DatasetVersionFilesServiceBean has been rewritten using JPA Criteria instead of QueryDSL. The QueryDSL library has been removed.
Which issue(s) this PR closes:
Suggestions on how to test this:
The following API endpoints use the rewritten queries:
getVersionFiles /datasets/{id}/versions/{versionId}/files (https://github.com/IQSS/dataverse/blob/develop/doc/sphinx-guides/source/api/native-api.rst#list-files-in-a-dataset)
getVersionFileCounts /datasets/{id}/versions/{versionId}/files/counts (https://github.com/IQSS/dataverse/blob/develop/doc/sphinx-guides/source/api/native-api.rst#get-file-counts-in-a-dataset)
getDownloadSize /datasets/{id}/versions/{versionId}/downloadsize (https://github.com/IQSS/dataverse/blob/develop/doc/sphinx-guides/source/api/native-api.rst#get-the-size-of-downloading-all-the-files-of-a-dataset-version)
Despite we can test manual curl calls to these endpoints by sending different combinations of parameters (Described in the Native API documentation) and verify that they work correctly, testing all cases is extensive. Integration tests mainly cover all possible scenarios, so to verify the new implementation we can check that all test cases still pass as they did with the previous QueryDSL implementation.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No
Is there a release notes update needed for this change?:
No