-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Add http endpoints for JabMap #13519
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
Conversation
# Conflicts: # jabsrv/src/test/rest-api.http
if the server doest find a .jmp, it now returns a mindmap with on a root node saying "JabMap"
|
|
||
| private java.nio.file.Path getJabMapDemoPath() { | ||
| java.nio.file.Path result = java.nio.file.Path.of(System.getProperty("java.io.tmpdir")).resolve("demo.jmp"); | ||
| // TODO: make this debug - and adapt "tinylog.properties" locally to use debug level |
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.
Address this TODO (and remove this line)
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.
unfortunately I don't know why this TODO is here. I don't know what it is adressing tbh. I believe you added it? What did you mean by make this debug - and adapt "tinylog.properties" locally to use debug level?
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.
See my sugestions, it is just about changing the letters error to debug. Easy.
I think, you don't want to inspect the .jmp file. Otherwise, you would need to learn about tinylog.properties outlined at https://devdocs.jabref.org/code-howtos/logging.html
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.
got it!
| responseContext.getHeaders().add("Access-Control-Allow-Origin", "*"); | ||
| } else if (requestOrigin.contains("://localhost")) { | ||
| responseContext.getHeaders().add("Access-Control-Allow-Origin", requestOrigin); | ||
| } else if (requestOrigin.endsWith("://jabref.github.io")) { |
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.
All of this can be simplified to always return the *
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.
done, please check the code again to confirm I understood you (or rather the code x) correctly
jabsrv/src/main/java/org/jabref/http/server/LibraryResource.java
Outdated
Show resolved
Hide resolved
jabsrv/src/main/java/org/jabref/http/server/LibraryResource.java
Outdated
Show resolved
Hide resolved
jabsrv/src/main/java/org/jabref/http/server/LibraryResource.java
Outdated
Show resolved
Hide resolved
jabsrv/src/main/java/org/jabref/http/server/LibraryResource.java
Outdated
Show resolved
Hide resolved
jabsrv/src/main/java/org/jabref/http/server/LibraryResource.java
Outdated
Show resolved
Hide resolved
jabsrv/src/main/java/org/jabref/http/dto/FileAnnotationDTO.java
Outdated
Show resolved
Hide resolved
jabsrv/src/main/java/org/jabref/http/dto/FileAnnotationDTO.java
Outdated
Show resolved
Hide resolved
| */ | ||
| public class PdfAnnotationDTO { | ||
| // save a LinkedPDFFileDTO which contains all necessary info to make a request back to the http server (e. g. to update the annotation content) | ||
| // see https://github.com/JabRef/jabmap/issues/21 first before working with the "path" attribute |
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.
I commented there how to make a path absolute.
jabsrv/src/main/java/org/jabref/http/server/LibraryResource.java
Outdated
Show resolved
Hide resolved
|
|
||
| private java.nio.file.Path getJabMapDemoPath() { | ||
| java.nio.file.Path result = java.nio.file.Path.of(System.getProperty("java.io.tmpdir")).resolve("demo.jmp"); | ||
| // TODO: make this debug - and adapt "tinylog.properties" locally to use debug level |
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.
See my sugestions, it is just about changing the letters error to debug. Easy.
I think, you don't want to inspect the .jmp file. Otherwise, you would need to learn about tinylog.properties outlined at https://devdocs.jabref.org/code-howtos/logging.html
| @Path("entries/{entryId}") | ||
| @Produces(MediaType.TEXT_HTML + ";charset=UTF-8") | ||
| public String getHTMLRepresentation(@PathParam("id") String id, @PathParam("entryId") String entryId) throws IOException { | ||
| // get entry with given citationkey (entryId) |
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.
| // get entry with given citationkey (entryId) |
| // loop through all entries to extract pdfs and paths | ||
| for (BibEntry entry : entries) { | ||
| List<LinkedFile> pathsToFiles = entry.getFiles(); | ||
| if (!pathsToFiles.isEmpty()) { | ||
| for (LinkedFile file : pathsToFiles) { | ||
| // ignore all non pdf files and online references | ||
| if (!file.getFileType().equals("PDF") || LinkedFile.isOnlineLink(file.getLink())) { | ||
| continue; | ||
| } | ||
| // add file to response body | ||
| LinkedPdfFileDTO localPdfFile = new LinkedPdfFileDTO(entry, file); | ||
| response.add(localPdfFile); | ||
| } | ||
| } | ||
| } |
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.
A good excercise would be to convert this to a Java Stream - but this might be too much for now.
ran "rewriteDiscover", "rewriteDryRun" and "rewriteRun" tasks, changed 1 line and removed unnecessary comment
|
@trag-bot didn't find any issues in the code! ✅✨ |
Not relevant for this - all checks passed Therefore manually merging. |
* upstream/main: Add http endpoints for JabMap (#13519)
Aims to tackle #12280
With this PR, the HTTP-Server is expanded to supply JabMap with necessary data. Several Endpoints have been added (see LibraryRessource.java and rest-api.http) as well as some DTOs.
The repo for JabMap with a detailed setup description can be found here
I'm marking this as a draft for now to document changes, check mergability and present it to the dev team. It'll stay like this until its integrated properly (tbd).
Steps to test
Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user)