Expose export formats in native API#10739
Conversation
pdurbin
left a comment
There was a problem hiding this comment.
@julian-schneider good idea! Thanks for the pull request. I left you some comments.
| @GET | ||
| @Path("exportFormats") | ||
| public Response getExportFormats() { | ||
| JsonArrayBuilder responseModel = Json.createArrayBuilder(); |
There was a problem hiding this comment.
Did you consider returning an object instead? Just this week I pushed a commit to return via API if feature flags are enabled or not and I played with returning an array but ended up liking object better. Here's the code: 771d85a (for #10732)
I'm suggesting something like this:
{
"status": "OK",
"data": {
"OAI_ORE": {
"displayName": "OAI_ORE"
},
"oai_dc": {
"displayName": "Dublin Core"
}
}
}
Alternatively, perhaps you could add a second endpoint for getting info about a single export format (e.g. /api/info/exportFormats/oai_dc). That way you can easily get info on that format instead of having to search through an array to find the element that matches oai_dc.
Just some thoughts! 😄 Maybe the array is actually better.
There was a problem hiding this comment.
Yeah an object is probably better, especially with the increased amount of properties, so I switched this in cc99729
There was a problem hiding this comment.
| Get Export Formats | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| Get the available export formats. The response contains a list of objects each containing the display name and format name for an available exporter. |
There was a problem hiding this comment.
Does this include external exporters? I assume so. Can we please mention they are included as well?
There was a problem hiding this comment.
As far as I know, all exporters, standard and custom, are managed by the ExportService, and are consequently all listed. I improved the docs in 3b2c8e5.
|
Thanks for your feedback @pdurbin - I will be absent for two weeks, so some time will pass before I revise this. See you then! |
|
The response is now an object, and contains more properties per format, as suggested. New response looks like this: (expand response) |
|
@pdurbin I believe I have addressed all the feedback you gave, have a nice day! |
pdurbin
left a comment
There was a problem hiding this comment.
I haven't run the code yet (or in a while, I forget) but I left a couple more suggestions.
Please also merge the latest from develop.
|
@johannes-darms I do like the PR a lot. @cmbz @scolapasta what do you think? Can we put a 6.5 milestone on it or at least put it in "sprint ready"? Also, I agree with Johannes that the SPA will need this some day. We might want to see what @ekraffmiller @ChengShi-1 @GPortas and @g-saracca think about the output, if they are happy with it. |
|
2024/10/15: Added to sprint ready after conversation with @pdurbin |
There was a problem hiding this comment.
@julian-schneider this is looking really good. I left you a few comments and a PR-for-this-PR ( julian-schneider#1 ) to consider. Also, can you please merge the latest from "develop" into your branch? Thanks!
| for (String[] labels : instance.getExportersLabels()) { | ||
| try { | ||
| Exporter exporter = instance.getExporter(labels[1]); |
There was a problem hiding this comment.
This lookup seems a bit awkward but it's not your fault. I wonder if we should add a method to ExportService like this...
public Collection<Exporter> getExporters() {
return exporterMap.values();
}
... so that it can be used like this in places like Info.java:
for (Exporter exporter : instance.getExporters()) {
//...
}
But I dunno. Maybe there's a reason why we don't offer a "getExporters" method like this.
There was a problem hiding this comment.
I agree that the steps I had to use to get the exporters were a bit awkward. If you think it would be in scope, I can add a method like that to this PR.
There was a problem hiding this comment.
No, let's think about it outside this PR. You're welcome to create an issue, though, if you like!
remove Gson and test based on content, not string match IQSS#10739
Co-authored-by: Philip Durbin <philipdurbin@gmail.com>
|
@julian-schneider tests are failing in InfoIT.testGetExportFormats. Can you please take a look? |
pdurbin
left a comment
There was a problem hiding this comment.
As of f3b72c6 tests are passing: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-10739/9/testReport/
Thanks, @julian-schneider! Approved.
In DMs we did discuss how if there's a lot of churn in our formats we can do some mocking. Still, it's valuable to actually exercise the API endpoint itself, even if we make the assertions less strict in the future.
|
No issues with PR discovered. |

What this PR does / why we need it: The PR adds a method that exposes the full list of available exporters (display- and format names). This is especially useful for instances that have added custom formats.
Which issue(s) this PR closes: I know no pre-existing issues for this.
Suggestions on how to test this: New method found here:
/api/info/exportFormatsDoes this PR introduce a user interface change? If mockups are available, please link/include them here: No, just API and documentation changes.
API answer looks like this:
Rendered docs entry looks like this:

Live version: https://dataverse-guide--10739.org.readthedocs.build/en/10739/api/native-api.html#get-export-formats
Is there a release notes update needed for this change?:
New API method for listing the available exporters. Found at
/api/info/exportFormats, produces display names and format names.