against trunk r1411996
* Why do you need both afterRefresh() and afterMaybeRefresh()? Is it because NRTManager tests failures, or was there a different reason?
Yes only reason, not very clean , not sure if NRT actually needs to be able to call something after every maybeRefresh or if its a test/impl restriction (probably latter?)
* Why did you add the same test to both NRTManager and SearcherManager tests? Isn't it enough to test it once on e.g. a dummy ReferenceManager extension? The test only verifies that the listeners are called.
True but there wasn't an existing ReferenceManager test, and it verifies they both call super.* in the override, but as stated in one of your next points, this can be done differently if the classes should be final.
* Should Listener be renamed to RefreshListener?
Thought of that aswel, but then it felt strange to have the afterClose in there too.
* Now that you have the listener, is there a reason for a protected afterRefresh() and afterMaybeRefresh()? Aren't the listeners enough?
- On that note, I kinda like the listeners approach, so maybe we should add a RefreshListener and CloseListener and get rid of the protected methods? The listeners also allow us to keep the classes final, yet still have some sort of extension point.
* Is it really necessary to use CopyOnWriteArrayList? Do we really expect an application will install a listener after the manager has already started and serviced calls?
- It seems like a setup method to me, and I'm fine if we document that you should call these methods before any other method of the class is called.
Yeah, its a bit of a trade of, i agree it's a setup method, but i didn't like the constructor explosion with adding it as an optional constructor parameter, and then just having a setter with no thread visibility guarantees in a class that's made to be used multithreaded felt wrong too, so opted for the CopyOnWrite list, there are already other listeners this way in NRTManager. Other option is maybe to piggy back on the refresh lock in the setter maybe. Or just document it as you say