Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-2753

IndexReader.listCommits should return a List and not an abstract Collection

        Details

        • Type: Improvement
        • Status: Closed
        • Priority: Minor
        • Resolution: Fixed
        • Affects Version/s: None
        • Fix Version/s: 3.1, 4.0-ALPHA
        • Component/s: core/index
        • Labels:
          None
        • Lucene Fields:
          New, Patch Available

          Activity

          Ascending order - Click to sort in descending order
          Hide
          Permalink
          shaie Shai Erera added a comment -

          Patch against 3x:

          • Changes listCommits() signature to return a List<IndexCommit>
          • DirReader.listCommits() sorts the list in the end.
          • Added a test case to TestIndexReader.
          • IndexCommit implements Comparable. Removed impl from CommitPoint (which also removed a redundant duplicate 'gen' member).

          I did not implement ReaderCommit to support deletes. Obtaining the lock for this purpose does not seem the right way to me ... IndexWriter has a deleteUnusedFiles which the application can use. If the app only does IR.listCommits, then being able to delete is an advantage, but otherwise it will need to mess with LockObtainFail exceptions. Not sure it's worth the efforts.

          I believe it is ready to commit. I'll wait a day or two until I commit it. Your comments are welcome.

          Show
          shaie Shai Erera added a comment - Patch against 3x: Changes listCommits() signature to return a List<IndexCommit> DirReader.listCommits() sorts the list in the end. Added a test case to TestIndexReader. IndexCommit implements Comparable. Removed impl from CommitPoint (which also removed a redundant duplicate 'gen' member). I did not implement ReaderCommit to support deletes. Obtaining the lock for this purpose does not seem the right way to me ... IndexWriter has a deleteUnusedFiles which the application can use. If the app only does IR.listCommits, then being able to delete is an advantage, but otherwise it will need to mess with LockObtainFail exceptions. Not sure it's worth the efforts. I believe it is ready to commit. I'll wait a day or two until I commit it. Your comments are welcome.
          Hide
          Permalink
          simonw Simon Willnauer added a comment -

          Shai, looks good to me!

          +1 to commit

          Show
          simonw Simon Willnauer added a comment - Shai, looks good to me! +1 to commit
          Hide
          Permalink
          shaie Shai Erera added a comment -

          Committed revision 1034080 + 1034144 (3x). Due to backwards tests failure, I kept the method signature as returning Collection, and only documented the new behavior.
          Committed revision 1034140 (trunk).

          Show
          shaie Shai Erera added a comment - Committed revision 1034080 + 1034144 (3x). Due to backwards tests failure, I kept the method signature as returning Collection, and only documented the new behavior. Committed revision 1034140 (trunk).
          Hide
          Permalink
          thetaphi Uwe Schindler added a comment -

          Java 5 allows covariant return types. Could we not declare both methods in 3.x and deprecate the old one? In trunk we can remove it and only provide List.

          Show
          thetaphi Uwe Schindler added a comment - Java 5 allows covariant return types. Could we not declare both methods in 3.x and deprecate the old one? In trunk we can remove it and only provide List.
          Hide
          Permalink
          thetaphi Uwe Schindler added a comment -

          We cannot declare both methods But backwards does not fail now

          Show
          thetaphi Uwe Schindler added a comment - We cannot declare both methods But backwards does not fail now
          Hide
          Permalink
          gsingers Grant Ingersoll added a comment -

          Bulk close for 3.1

          Show
          gsingers Grant Ingersoll added a comment - Bulk close for 3.1

            People

            • Assignee:
              shaie Shai Erera
              Reporter:
              shaie Shai Erera
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development