Lucene - Core
  1. Lucene - Core
  2. LUCENE-2753

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

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor 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

      Hide
      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
      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
      Simon Willnauer added a comment -

      Shai, looks good to me!

      +1 to commit

      Show
      Simon Willnauer added a comment - Shai, looks good to me! +1 to commit
      Hide
      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
      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
      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
      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
      Uwe Schindler added a comment -

      We cannot declare both methods But backwards does not fail now

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

      Bulk close for 3.1

      Show
      Grant Ingersoll added a comment - Bulk close for 3.1

        People

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

          Dates

          • Created:
            Updated:
            Resolved:

            Development