[WIP] Refactored around the FileAnnotationCache.#2557
Conversation
…orks as intended, i.e., fully automatically instead of manual maintenance.
|
|
||
| class DeleteAction extends AbstractAction { | ||
| public DeleteAction() { | ||
| DeleteAction() { |
There was a problem hiding this comment.
I think, checkstyle or codacy or some other toal moans because of package private. Nevertheless, I think, it is OK to reduce visibility.
There was a problem hiding this comment.
It seems like codacy does not like default visibility at all
|
|
||
| if(!tab.isInitialized) { | ||
|
|
||
| try { |
There was a problem hiding this comment.
indent? - Think, indent is wrong here.
| * shows those from the first file in the comments tab | ||
| * Adds pdf annotations from all attached pdf files belonging to the entry selected in the main table and | ||
| * shows those from the first file in the file annotations tab | ||
| * @throws IOException |
There was a problem hiding this comment.
I would remove that, @throws javadoc, because it does not add any more information.
| public List<ParsedFileField> getFilteredFileList() { | ||
| return FileField.parse(this.entry.getField(FieldName.FILE).get()).stream() | ||
| .filter(parsedFileField -> parsedFileField.getLink().toLowerCase().endsWith(".pdf")) | ||
| .filter(parsedFileField -> !parsedFileField.getLink().contains("www.")).collect(Collectors.toList()); |
There was a problem hiding this comment.
I'm not sure whether this line is really valid. Did you find a test case for that? I assume, a user put an URL in the file field. This should not be checked here, but at other places (integrity check, ...)
There was a problem hiding this comment.
actually this is a one-to-one copy of what has been there before. I don't have the domain-specific insight if it makes sense.
koppor
left a comment
There was a problem hiding this comment.
Besides minor comments: looks good to me. Thank you for the effort!
|
Hey, I have tried to fix the issues from your review. Check my comments on the particular parts above. |
…or-fileAnnotations-caching
…or-fileAnnotations-caching
tobiasdiez
left a comment
There was a problem hiding this comment.
LGTM. Thank you for your contribution!
* upstream/master: Revert "Update gradle from 3.3 to 3.4" Localization tests (#2582) Update IEEEJournalList (#2579) Keyword - Special field synchronization (#2583) Update gradle from 3.3 to 3.4 Check similarity of entry when using DOI retrieval with ArXiV (#2575) Add logic for new Sciencedirect pages (#2576) [WIP] Refactored around the FileAnnotationCache. (#2557)
The Cache now works as intended, i.e., fully automatically instead of manual maintenance.