Netflix/curator — pull request #196 by Answashe (2012-11-01 21:40:02+00:00)
Title: Fix some issues with NamespaceFacade stemming from inconsistent state.

Short version:
The `inTransaction()` method of a NamespaceFacade didn't work. This patch fixes that.

Long version:
Many methods in `CuratorFrameworkImpl` checked that it was started by checking the state directly rather than calling the `isStarted()` method, but the copy constructor did not copy the state. `NamespaceFacade` overrode most, but not all, methods that checked state - presumably to get around that problem. Methods that it did not override (such as `inTransaction()`) would thus incorrectly report that the facade was not started and fail.

This patch improves the situation in a few ways.

1) The copy constructor now copies the `state` field, so the facade will properly see state changes. So things that do check `state` should be able to do so.
2) All of the `state.get() == State.STARTED` checks outside of the `isStarted()` method are changed to `isStarted()` calls. This way classes that inherit from `CuratorFrameworkImpl` can override `isStarted()` can get consistent behavior.
3) Many methods in `NamespaceFacade` no longer need to be overrides, so they are removed.

Additionally, creation of the `NamespaceFacadeCache` was moved to the end of the `CuratorFrameworkImpl` constructor, since it creates a facade for the `null` namespace during its instantiation. This would in turn call the `CuratorFrameworkImpl` copy constructor, but not all members of the parent had been initialized yet. Perhaps a better solution would be to move the instantion of the NamespaceFacadeCache to the `start()` method, or maybe have the NamespaceFacadeCache instantiate the facade for the `null` namespace lazily.