GH 1256. Fix cached modeled framework list

[kotman12]

Fix CachedModeledFramework::list so it returns unfiltered list of children based on the cache path (not client path). See #1256 for more details.

Also added really minor fix to CachedModeledFramework::readThrough(Stat). This surfaced after I enhanced the tests a bit. This bug predates #1250

[tisonkun]

@kotman12 This should close #1256?

[kotman12]

@kotman12 This should close #1256?

@tisonkun I hope so, yes!

[kezhuw]

Looks good.

But I was confused in review regarding the CachedModeledFramework::list and ModeledCache::currentChildren.

• CachedModeledFramework::list ignores data tree hierarchy. child does not change the result.
• ModeledCache::currentChildren includes the path itself and and is recursive.

I think we probably should hightlight some to gain attention.

[kotman12]

@kezhuw thanks! A few observations here:

CachedModeledFramework::list indeed ignores data hierarchy and returns all descendants of the base path but this is consistent with the API doc (as well as previous behavior):

Return the instances of the base path of this cached framework

Is your suggestion to add the word "recursively" to the doc somewhere in ModeledCache::currentChildren and CachedModeledFramework::list to stress that it recursively returns children's children? My immediate word of choice is "descendant" but I don't see it used anywhere in the curator docs so I don't want to introduce new terminology.

Tangentially, something I found strange is that CachedModeledFramework::parent is unimplemented but you can still potentially get the parent's modeled data if it is under the cache's base path.

[kezhuw]

@kotman12 Merged. Thank you for your contribution!

but this is consistent with the API doc

Yeh, I saw.

Regarding ModeledCache::currentChildren, I think it is normal that "children" is not recursive just like ZooKeeper::getChildren. Given that it is a public api, I think there is no much room for us to change except documentation.