davidledwards/zookeeper

Recursive parameter value is inverted on observe function

Closed this issue · 5 comments

Hi,

When executing the following code:

Try(zk.sync.create("/sample", Array[Byte](), ACL.AnyoneAll, Persistent))
zk.sync.watch {
  case e => log.debug(e.toString)
} observe ("/sample", true)
Try(zk.sync.create("/sample/sample1", Array[Byte](), ACL.AnyoneAll, Persistent))

I notice i've got a ChildrenChanged Event, which should not happend in PersistentRecursive mode (All Children event are treated as NodeEvent => https://zookeeper.apache.org/doc/r3.7.2/apidocs/zookeeper-server/org/apache/zookeeper/AddWatchMode.html#PERSISTENT_RECURSIVE).

Looking at the code from the observe function, I noticed that if the recursive parameter is set to true, the mode that is selected is PERSISTENT, and not PERSISTENT_RECURSIVE, where it should be the other way around, since the parameter name for the boolean is recursive.

def observe(path: String, recursive: Boolean = false): Unit = {
  val mode = if (recursive) AddWatchMode.PERSISTENT else AddWatchMode.PERSISTENT_RECURSIVE
  zk.addWatch(path, watcher, mode)
}

Ugh! This is another reminder of why I need to invest in unit testing of the ZK class. Apologize for that. Will get that fixed shortly.

No worries !
A solution could be to use TestContainers to make end to end tests.
We do have a dedicated TestContainer wrapper in scala for Zookeeper that we use internally.
If you are interested I can ask if we can open source it.

I actually have a test fixture in place that should get the job done. The only unit test that exists for the ZK class is one that verifies the session state. They just need to be expanded to include all public methods.

Released version 1.7.1 - https://github.com/davidledwards/zookeeper/tree/release-1.7.1

Unit tests were added for watches. I'll add unit tests for the rest of the Zookeeper class, but wanted to get this release done sooner rather than later.

Thanks a lot for the fix !