-
Notifications
You must be signed in to change notification settings - Fork 532
Description
What steps does it take to reproduce the issue?
call the /api/admin/settings/ method with an invalid name - the response is 500 and a long stack trace appears in the server.log
-
When does this issue occur?
-
Which page(s) does it occurs on?
-
What happens?
-
To whom does it occur (all users, curators, superusers)?
-
What did you expect to happen?
The response should be a 400.
It appears that this method is incorrectly catching IllegalArgumentException instead of the new SettingsValidationException in related methods.
dataverse/src/main/java/edu/harvard/iq/dataverse/api/Admin.java
Lines 268 to 277 in f5049ac
| public Response getSetting(@PathParam("name") String name) { | |
| try { | |
| SettingsServiceBean.validateSettingName(name); | |
| String content = settingsSvc.get(name); | |
| return (content != null) ? ok(content) : notFound("Setting " + name + " not found."); | |
| } catch (IllegalArgumentException iae) { | |
| return error(Response.Status.BAD_REQUEST, iae.getMessage()); | |
| } | |
| } |
I think this could be fixed relatively simply (change the exception), but, looking at the code and the fact that it uses the now deprecated settingsSvc.get() method, a slightly larger refactor might be better. I.e. if we need to call SettingsServiceBean.Key.parse() to get the key to be able to call getValueForKey() (as recommended in the deprecation notice), it probably doesn't make sense to call validateSettingName first, which just calls Key.parse internally, and then call parse again to get the Key - probably easier to just call parse and, if the return is null, return a BAD_REQUEST error - no try/catch required. That change would probably make sense in the other settings api calls as well.
Or - should get(String) not be deprecated but throw SettingsValidationException? (would take some refactoring in SettingsServiceBean)
Which version of Dataverse are you using?
Any related open or closed issues to this bug report?
Screenshots:
No matter the issue, screenshots are always welcome.
To add a screenshot, please use one of the following formats and/or methods described here:
Are you thinking about creating a pull request for this issue?
Help is always welcome, is this bug something you or your organization plan to fix?
No plan right now.