-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Extract BibEntry endpoints into BibEntryResource and add tests #13731
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
base: main
Are you sure you want to change the base?
Extract BibEntry endpoints into BibEntryResource and add tests #13731
Conversation
|
||
JsonNode root = mapper.readTree(response); | ||
|
||
assertTrue(root.isArray(), "CSL JSON must be a JSON array"); |
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.
We normally check the complete content at once. Then, its easier to know whats going wrong, because IntellIj displays the diff
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.
Thanks for the feedback , i work on it , it is ready
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.
79168dd
to
0b17f24
Compare
@trag-bot didn't find any issues in the code! ✅✨ |
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.
codewise looks good to me, but I have no clue if this is correct otherwise
Hello @Siedlerchr Thanks for the Feedback, I made changes regarding the tasks on this issue #13588 |
Also looks like you added the json ones (which for itself looks on first sight good and can be kept) but you removed the html and plain string representation for the responses. If I understand the task correct they are the ones that should be moved to the new resource. |
/** | ||
* At http://localhost:23119/libraries/{id}/entries/{entryId} <br><br> | ||
* | ||
* Combines attributes of a given BibEntry into a basic entry preview for as plain text. | ||
* | ||
* @param id The name of the library | ||
* @param entryId The CitationKey of the BibEntry | ||
* @return a basic entry preview as plain text | ||
* @throws IOException | ||
* @throws NotFoundException | ||
*/ | ||
@GET | ||
@Path("entries/{entryId}") | ||
@Produces(MediaType.TEXT_PLAIN + ";charset=UTF-8") | ||
public String getPlainRepresentation(@PathParam("id") String id, @PathParam("entryId") String entryId) throws IOException { | ||
BibDatabaseContext databaseContext = getDatabaseContext(id); | ||
List<BibEntry> entriesByCitationKey = databaseContext.getDatabase().getEntriesByCitationKey(entryId); | ||
if (entriesByCitationKey.isEmpty()) { | ||
throw new NotFoundException("Entry with citation key '" + entryId + "' not found in library " + id); | ||
} | ||
if (entriesByCitationKey.size() > 1) { | ||
LOGGER.warn("Multiple entries found with citation key '{}'. Using the first one.", entryId); | ||
} | ||
|
||
// TODO: Currently, the preview preferences are in GUI package, which is not accessible here. | ||
// build the preview | ||
BibEntry entry = entriesByCitationKey.getFirst(); | ||
|
||
String author = entry.getField(StandardField.AUTHOR).orElse("(N/A)"); | ||
String title = entry.getField(StandardField.TITLE).orElse("(N/A)"); | ||
String journal = entry.getField(StandardField.JOURNAL).orElse("(N/A)"); | ||
String volume = entry.getField(StandardField.VOLUME).orElse("(N/A)"); | ||
String number = entry.getField(StandardField.NUMBER).orElse("(N/A)"); | ||
String pages = entry.getField(StandardField.PAGES).orElse("(N/A)"); | ||
String releaseDate = entry.getField(StandardField.DATE).orElse("(N/A)"); | ||
|
||
// the only difference to the HTML version of this method is the format of the output: | ||
String preview = | ||
"Author: " + author | ||
+ "\nTitle: " + title | ||
+ "\nJournal: " + journal | ||
+ "\nVolume: " + volume | ||
+ "\nNumber: " + number | ||
+ "\nPages: " + pages | ||
+ "\nReleased on: " + releaseDate; | ||
|
||
return preview; | ||
} |
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.
Those are the ones that should be moved to the new resource.
/** | ||
* At http://localhost:23119/libraries/{id}/entries/{entryId} <br><br> | ||
* | ||
* Combines attributes of a given BibEntry into a basic entry preview for as HTML text. | ||
* | ||
* @param id The name of the library | ||
* @param entryId The CitationKey of the BibEntry | ||
* @return a basic entry preview as HTML text | ||
* @throws IOException | ||
*/ | ||
@GET | ||
@Path("entries/{entryId}") | ||
@Produces(MediaType.TEXT_HTML + ";charset=UTF-8") | ||
public String getHTMLRepresentation(@PathParam("id") String id, @PathParam("entryId") String entryId) throws IOException { | ||
List<BibEntry> entriesByCitationKey = getDatabaseContext(id).getDatabase().getEntriesByCitationKey(entryId); | ||
if (entriesByCitationKey.isEmpty()) { | ||
throw new NotFoundException("Entry with citation key '" + entryId + "' not found in library " + id); | ||
} | ||
if (entriesByCitationKey.size() > 1) { | ||
LOGGER.warn("Multiple entries found with citation key '{}'. Using the first one.", entryId); | ||
} | ||
|
||
// TODO: Currently, the preview preferences are in GUI package, which is not accessible here. | ||
// build the preview | ||
BibEntry entry = entriesByCitationKey.getFirst(); | ||
|
||
String author = entry.getField(StandardField.AUTHOR).orElse("(N/A)"); | ||
String title = entry.getField(StandardField.TITLE).orElse("(N/A)"); | ||
String journal = entry.getField(StandardField.JOURNAL).orElse("(N/A)"); | ||
String volume = entry.getField(StandardField.VOLUME).orElse("(N/A)"); | ||
String number = entry.getField(StandardField.NUMBER).orElse("(N/A)"); | ||
String pages = entry.getField(StandardField.PAGES).orElse("(N/A)"); | ||
String releaseDate = entry.getField(StandardField.DATE).orElse("(N/A)"); | ||
|
||
// the only difference to the plain text version of this method is the format of the output: | ||
String preview = | ||
"<strong>Author:</strong> " + author + "<br>" + | ||
"<strong>Title:</strong> " + title + "<br>" + | ||
"<strong>Journal:</strong> " + journal + "<br>" + | ||
"<strong>Volume:</strong> " + volume + "<br>" + | ||
"<strong>Number:</strong> " + number + "<br>" + | ||
"<strong>Pages:</strong> " + pages + "<br>" + | ||
"<strong>Released on:</strong> " + releaseDate; | ||
|
||
return preview; | ||
} |
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.
those too
|
||
### | ||
GET http://localhost:23119/libraries/demo/entries/Corti_2009 |
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.
Please add a heading like on the others and move it up to the other cases which handle the entries itself, the end is for error cases. Also add that it only accepts the according mediatype
Closes #13588
Steps to test
Mandatory checks
CHANGELOG.md
described in a way that is understandable for the average user (if change is visible to the user)