Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Search in PDF Files #2838

Merged
merged 122 commits into from
Jul 14, 2021
Merged

Search in PDF Files #2838

merged 122 commits into from
Jul 14, 2021

Conversation

LinusDietz
Copy link
Member

@LinusDietz LinusDietz commented May 12, 2017 •
edited by btut
Loading

This implements a fulltext-search feature for linked PDF files.

search

All linked pdf files are indexed using Apache Lucene. Search-results are displayed in a google-esque fashion in an additional tab in the entry editor (visible only if a fulltext search is active).
Users can enable/disable fulltext-search with a toggle-button in the search-bar.

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)
  • If you changed the localization: Did you run gradle localizationUpdate?

Braunch and others added 16 commits August 2, 2016 19:19
@Braunch
Add lucene to gradle.build dependencies
321a0f0
@Braunch
Implement first attempt on a lucene powerded indexed search machine f…
2580757 
…or pdf documents
@Braunch
Ignore test and improve search handler and content reader to map bibt…
a9d85dc 
…exkey from db
@Braunch
Create search result wrapper classes for better readability of future…
753c25d 
… code
@Braunch
Add lucene score and collector for search meta data retrieval in the …
6d1cb44 
…resultset
@Braunch
Add example pdf and create test skeleton
1af3f1a
@LinusDietz
Merge branch 'PDFSearchFeature' of github.com:Braunch/jabref into lucene
40609ad
@LinusDietz
import fulltext search from Braunch:PDFSearchFeature
8787bcd
@LinusDietz
Merge branch 'master' of github.com:JabRef/jabref into lucene
d51919e
@LinusDietz
Refactored the PDFContentReader
692316b
@LinusDietz
Put DocumentReader under test
dacbc3c
@LinusDietz
Use current Lucene Release, Indexing is working now.
2663798
@LinusDietz
Revised package structure
7086a8f
@LinusDietz
added stem analysis
444bd39
@LinusDietz
Implemented Searcher Test
8547873
@LinusDietz
Improve Naming
c9cf893
@LinusDietz LinusDietz added external files status: waiting-for-feedback The submitter or other users need to provide more information about the issue labels May 12, 2017
tobiasdiez
tobiasdiez requested changes May 13, 2017
View reviewed changes
Copy link
Member

@tobiasdiez tobiasdiez left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wooohoooo! Thanks for you efforts to get the PDF search woring!

Just a few initial remarks for now; I will continue reviewing the next days.

build.gradle Outdated Show resolved Hide resolved
gradle/wrapper/gradle-wrapper.properties Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/pdf/search/indexing/DocumentReader.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/pdf/search/indexing/DocumentReader.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/pdf/search/indexing/DocumentReader.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/pdf/search/indexing/DocumentReader.java Outdated Show resolved Hide resolved
Siedlerchr
Siedlerchr reviewed May 14, 2017
View reviewed changes
build.gradle Outdated Show resolved Hide resolved
Siedlerchr
Siedlerchr requested changes May 14, 2017
View reviewed changes
Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments, some unnecessary mocking and some little improvements

@LinusDietz LinusDietz changed the title Search in PDF Files [WIP] Search in PDF Files May 14, 2017
@LinusDietz LinusDietz removed the status: waiting-for-feedback The submitter or other users need to provide more information about the issue label May 14, 2017
koppor
koppor requested changes Jul 13, 2021
View reviewed changes
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nitpicks, it is good to go otherwise

src/main/java/org/jabref/gui/JabRefMain.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/pdf/search/indexing/DocumentReader.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/pdf/search/indexing/DocumentReader.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/pdf/search/indexing/PdfIndexer.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/gui/JabRefMain.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/model/database/BibDatabaseContext.java Outdated
@@ -211,4 +215,12 @@ public void convertToLocalDatabase() {
public List<BibEntry> getEntries() {
return database.getEntries();
}

public Path getFulltextIndexPath() {
Path appData = Path.of(AppDirsFactory.getInstance().getUserDataDir(JabRefFrame.FRAME_TITLE, SearchFieldConstants.VERSION, "org.jabref"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the documentation at https://github.com/harawata/appdirs. Magic string jabref is OK for me here.

@koppor koppor mentioned this pull request Jul 13, 2021
Closed
@koppor
Copy link
Member

koppor commented Jul 13, 2021

This fixes #6188

btut and others added 3 commits July 14, 2021 13:28
@btut
Use literal JabRef for index path
bfd2582
@btut @koppor
Apply suggestions from @koppor
46517ff 
Removed empty line and fixed logger format

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
@btut
Checkstyle
9c887a3 
Removed unnecessary import
@calixtus
Copy link
Member

Im still not convinced with using the literal JabRef for index path and would like to talk about that in the call tonight. I wonder, if there isn't a better, more generic solution.

@btut
Copy link
Contributor

btut commented Jul 14, 2021

Im still not convinced with using the literal JabRef for index path and would like to talk about that in the call tonight. I wonder, if there isn't a better, more generic solution.

I agree, especially because it is used in two distinct files and needs to be identical. However, I see how it is also problematic to use the one stored in JabRefFrame. Maybe we should define a new static variable in PdfIndexer?

btut added 3 commits July 14, 2021 13:51
@btut
Removed more dead code
7601ce6
@btut
Add annotations to index
5dc3852 
For now, lets use highlighted and text annotations only.

Fixes 4654
@btut
Checkstyle
110601c
@koppor
Copy link
Member

koppor commented Jul 14, 2021

Result of call - we looked up https://lucene.apache.org/core/8_0_0/core/org/apache/lucene/store/NIOFSDirectory.html

NOTE: NIOFSDirectory is not recommended on Windows because of a bug in how FileChannel.read is implemented in Sun's JRE. Inside of the implementation the position is apparently synchronized. See here for details.

For now, we accept that and think, no issues occur.

@calixtus calixtus merged commit ddce573 into main Jul 14, 2021
@calixtus calixtus deleted the lucene branch July 14, 2021 19:49
@Siedlerchr
Copy link
Member

FYI: mac index file is located under /Users/christophs/Library/Application Support/JabRef/0.3a

@koppor
Copy link
Member

koppor commented Jul 14, 2021

Directory is determined using the appdirs library: https://github.com/harawata/appdirs#appdirs

Siedlerchr added a commit that referenced this pull request Jul 15, 2021
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into jdk17
a798eb5 
* upstream/main: (45 commits)
  Squashed 'buildres/csl/csl-styles/' changes from ec4a4c0..176997d (#7910)
  Update citeproc-java to 3.0.0-alpha.2 (#7911)
  Search in PDF Files (#2838)
  Removed references to apache commons logging (#7907)
  Oobranch c : ootext and rangesort (#7788)
  Bump jackson-datatype-jsr310 from 2.12.3 to 2.12.4 (#7901)
  fix markdownlint
  Bump jackson-dataformat-yaml from 2.12.3 to 2.12.4 (#7899)
  Bump postgresql from 42.2.22 to 42.2.23 (#7902)
  Bump classgraph from 4.8.109 to 4.8.110 (#7900)
  Bump gittools/actions from 0.9.9 to 0.9.10 (#7898)
  Bump flowless from 0.6.3 to 0.6.4 (#7903)
  Bump jsoup from 1.13.1 to 1.14.1 (#7904)
  Bump tika-core from 1.26 to 1.27 (#7897)
  Try even more empty lines to provoke conflicts in CHANGELOG.md after a release
  Fix position in CHANGELOG.md
  Add corporate proxy workaround for Version.getAllAvailableVersions() (#7890)
  Update to Java 16 in build.gradle (#7892)
  Preparing Changelog for the next release cycle
  New development cycle
  ...

# Conflicts:
#	gradle/wrapper/gradle-wrapper.properties
@koppor koppor mentioned this pull request Dec 6, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@btut btut btut left review comments

@Siedlerchr Siedlerchr Siedlerchr requested changes

@tobiasdiez tobiasdiez tobiasdiez requested changes

@calixtus calixtus calixtus approved these changes

@koppor koppor koppor approved these changes

Assignees

@btut btut

Labels
external files project: GSoC type: feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@LinusDietz @lenhard @koppor @Siedlerchr @btut @calixtus @tobiasdiez @Braunch