paulirwin opened a new pull request, #1104: URL: https://github.com/apache/lucenenet/pull/1104
- [X] You've read the [Contributor Guide](https://github.com/apache/lucenenet/blob/main/CONTRIBUTING.md) and [Code of Conduct](https://www.apache.org/foundation/policies/conduct.html). - [X] You've included unit or integration tests for your change, where applicable. - [X] You've included inline docs for your change, where applicable. - [X] There's an open issue for the PR that you are making. If you'd like to propose a change, please [open an issue](https://github.com/apache/lucenenet/issues/new/choose) to discuss the change or find an existing issue. Fixes the `TestBufferedCharFilter.Test_Ready` failing test. Fixes #1102 ## Description Background: CharFilter.IsReady returns true if any calls to Read are guaranteed not to block. Otherwise, it means that a call may or may not block. This test was failing because StringReader is not a CharFilter and thus it did not have an IsReady property. BufferedCharFilter only is ready if the underlying TextReader is a CharFilter and it returns true for IsReady. So given that BufferedCharFilter already properly cascaded the property, this fixes the failing test by adding a new adapter class to adapt the StringReader to a CharFilter so that it is IsReady-aware. Since StringReaders do not block on calls to Read, this always returns true. This is perhaps a little bit of a leaky abstraction by having to know these details, but I don't think it really harms much in the design. This property only signals whether Read will block or not, and that's not something .NET TextReader implementations care to surface, nor is it used in Lucene. The only other alternatives here would be to have BufferedCharFilter take a CharFilter instead of a TextReader in its constructor, and that would cascade a bunch of mess to callers further up (if it's even possible to do so) and deviate from upstream, or to implement a Reader abstraction to better match Java and have to adapt all TextReaders to that (and all the extra allocations that would go along with that). I don't think either of those alternatives are worth it just for this property. Also, out of all 8 non-test implementations of CharFilter, only BufferedCharFilter overrides IsReady, and it just delegates it to the inner CharFilter (if it is one) anyways. So while this test ensures that it works correctly for BufferedCharFilter for end user code, it doesn't seem to have much utility in Lucene/Lucene.NET, most likely because these calls would usually block (or at least not be guaranteed to block). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@lucenenet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
