Modularize JabSrv#13908
Conversation
| @Override | ||
| public Void call() throws InterruptedException { | ||
| // The server serves the last opened files (see org.jabref.http.server.LibraryResource.getLibraryPath) | ||
| // The server serves the last opened files (see org.jabref.http.server.resources.LibraryResource.getLibraryPath) |
There was a problem hiding this comment.
The comment does not add new information or reasoning to the code. It merely restates what the code is doing, which is discouraged.
|
@trag-bot didn't find any issues in the code! ✅✨ |
| try { | ||
| libraryAsString = Files.readString(library); | ||
| } catch (IOException e) { | ||
| LOGGER.error("Could not read library {}", library, e); |
There was a problem hiding this comment.
The try block covers too many statements. It should only cover the statement that might throw the exception, improving readability and maintainability.
| * @return a stream to the Chocolate.bib file in the classpath (is null only if the file was moved or there are issues with the classpath) | ||
| */ | ||
| private @Nullable InputStream getChocolateBibAsStream() { | ||
| return BibDatabase.class.getResourceAsStream("/Chocolate.bib"); |
There was a problem hiding this comment.
Returning null from a method is discouraged. Consider using Optional to avoid potential NullPointerExceptions.
| */ | ||
| @GET | ||
| @Produces(MediaType.APPLICATION_JSON) | ||
| public String getJabMapJson(@PathParam("id") String id) throws IOException { |
There was a problem hiding this comment.
The method returns a JSON string directly, which could be null if the file reading fails. It should return an Optional to avoid returning null.
| } | ||
|
|
||
| private java.nio.file.Path getJabMapDemoPath() { | ||
| java.nio.file.Path result = java.nio.file.Path.of(System.getProperty("java.io.tmpdir")).resolve("demo.jmp"); |
There was a problem hiding this comment.
The use of System.getProperty("java.io.tmpdir") is correct, but the path should be managed using modern Java best practices like Path.of() instead of Paths.get().
Pull Request is not mergeable
* upstream/main: Add new check for format (#13909) Consistent casing in fieldnames (#13867) Revert "Pressing TAB in last field in entry editor moves focus to the next ta…" (#13912) Fix YAML Fix on-pr-opened-updated.yml syntax Pressing TAB in last field in entry editor moves focus to the next tab's first field (#13870) Modularize JabSrv (#13908) New translations jabref_en.properties (Italian) (#13907) Remove wrong `assert` statement (#13906) Add .git-blame-ignore-revs (#13884) Do not show transprot info messages (#13904) Pubmed api key support (#13899) Fix warnings for native access Fix automerge workflow (#13903) Add comment on issue on binary (#13902) Have checkstyle and VCS configuration distributed (#13900) Add unknown field to lsp consistency check (#13880) Put config for general tab if missing (#13901) Fix autosave manager exception on shutdown (#13882)
Makes JabSrv more modular
Steps to test
Mandatory checks
CHANGELOG.mdin a way that is understandable for the average user (if change is visible to the user)