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

DD-648 fix for allow custom terms #130

Conversation

mderuijter
Copy link

What this PR does / why we need it:
Only sets a custom license when :AllowCustomLicense is set to true, otherwise default license is used
Which issue(s) this PR closes:

Closes #DD-648

For testing see issue

try {
return licenseService.getByNameOrUri(licenseNameOrUri);
} catch (NoResultException nre){
throw new JsonParseException("Couldn't find an active license with: " + licenseNameOrUri);

Choose a reason for hiding this comment

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

@mderuijter : is really a JSON parsing problem??

Copy link
Author

Choose a reason for hiding this comment

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

@janvanmansum no, I see now that it's a silly mistake. My train of thought was that only JsonParseExceptions are used here and I had to use that for catching the NoResultException, I realize now it is wrong.

I need to readjust that anyway according to the other exception refactoring

Copy link
Author

Choose a reason for hiding this comment

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

@janvanmansum had another look at the code in this class and the JsonParseException is actually used for every error that happens during the parsing process: JsonParser.java#L575-L582.
So I will change my code and remove the NoResultException try-catch and return a JsonParseException if the License == null

@janvanmansum janvanmansum merged commit 6159d19 into DANS-KNAW:multi-license Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants