DD-387 Add license entity and api

[JingMa87]

What this PR does / why we need it: Adds DB table, API and business logic for multiple licenses.

Which issue(s) this PR closes: -

Closes #-

Special notes for your reviewer: -

Suggestions on how to test this: Use postman or curl to GET, POST, PUT and DELETE. Here's an example json you can post:

{
    "name": "Apache License",
    "shortDescription": "License description",
    "uri": "www.apache.com",
    "iconUrl": "www.icon.com",
    "active": false
}

Does this PR introduce a user interface change? If mockups are available, please link/include them here: -

Is there a release notes update needed for this change?: -

Additional documentation: -

[PaulBoon]

Just changed the base, original PR was done against the develop branch and it should have been done against the multi-license feature branch

[PaulBoon]

About the API

• you don't specify an ID, this is 'generated' by the db I guess, or otherwise. The caller of a create /POST does not know this ID and it would be nice to return it in the response for further processing later on by the client.
• Why not and endpoint with the name as a 'selector', this one is constained unique and the caller knows it when creating, same for URI, but that probably always needs url escaping. So @Path("/licenses/{name}") actually, if you have name as selector, you can do without the ID selector.

[PaulBoon]

What would cause this to happen, returning null is not very informative. Maybe we could have this throw an exception with some meaningful information

[janvanmansum]

I agree.

[JingMa87]

That's also how I prefer to do it, but in the dataverse code base you'll see that it's quite common to log some info and return null. I can throw more exceptions but we will be diverting from the dataverse code base.

[janvanmansum]

There are many things in the Dataverse code base that are suboptimal practises. Let's not adjust to the lowest common denominator so te speak.

[PaulBoon]

After some discussion it seems best to add @path("/licenses/{name}"), but leave the ones with id 'as-is'.

[janvanmansum]

A good start, following the established patterns of Dataverse.

Points for improvement:

• The practise of returning null when there is a problem and assuming what the problem was at the calling end is cutting corners a bit too much. See comments in the text.
• I see you like short variable names. I agree for small scopes such as these. For larger scopes they should be longer. (See Robert C Martin's Clean Code, rule N5: choose long names for long scopes.)
• Don't use l or O as variable names though. They look too much like 1 and 0 in many fonts.
• There seems to be no formatting policy in the Dataverse code base, but keep indenting consitent in the sections you edit.

[janvanmansum]

I agree.

[JingMa87]

@janvanmansum I addressed the requested changes, you can test it if you want.

[janvanmansum]

When would this throw a URISyntaxException? And how could this realistically happen?

[JingMa87]

@janvanmansum This would throw a URISyntaxException if the URI String in the DB isn't a valid URI. Realistically this would never happen because a URI can only be inserted or updated in the DB if it's a correct URI object, I tested this. However, if someone directly tampers with the data in the DB, the exception might be thrown. Also, the exception is automatically thrown when trying to form a URI object so it has to be handled.

[janvanmansum]

As discussed, please wrap in an IllegalStateException. Rationale: the database has been corrupted in some way.

[janvanmansum]

@JingMa87: The specs have not been fully implemented yet. I would like to merge this PR and then see small PRs to fix all the problems and unimplemented parts. Also, let's get rid of Rick Astley references.

I have planned a call for tomorrow to finish up this PR.