[CURATOR-148] Order of modifiers on framework commands matters

• Type: Improvement
• Status: Resolved
• Priority: Major
• Resolution: Won't Fix
• Component/s: Client
• Assignee: Unassigned
• Reporter: Mike Drob
• Created: 12/Sep/14 02:43
• Updated: 24/Jan/17 02:26
• Resolved: 24/Jan/17 02:26

Description

I only tested this with the CreateBuilder, but visual inspection makes me think that it is present on other operations as well.

If I want to create a ZK node, using a bunch of the features that curator provides, I have to do it in a specific order. (Arguments removed for clarity)

client.create()
    .compressed()
    .withMode()
    .withACL()
    .inBackground()
    .forPath();

If I unknowingly call inBackground() first, then the only methods available to me are the forPath() variants. Similarly, if I call create().withMode(), then there is longer a way for me to call compressed() on that object.

Even more concerning is that it is impossible to call compressed() and withProtection() on the same chain, regardless of order.

Since each of the fluent-style methods already returns this, it might make sense to modify each method as implemented on CreateBuilder to have a declared return type of CreateBuilder, taking advantage of covariant return types.

Another option, with similar results, would be to remove many of the intermediate interfaces from CreateBuilder like ACLCreateModeBackgroundPathAndBytesable<String> and declare it to be:

interface CreateBuilder extends Pathable<String>,
    Backgroundable<CreateBuilder>,
    Compressible<CreateBuilder>,
    ACLable<CreateBuilder>,
    CreateModable<CreateBuilder>,
    ....

This option is very verbose, however.

A disadvantage of both of these options is that it allows users to call methods multiple times. In some cases, like inBackground(), it won't matter. In other cases, like withACLs() we'd have to make a decision on taking an intersection or "last call wins" approach. That might even differ per call, so we'd have to have careful documentation on each.

Another option is to simply document the behavior (probably on the create() method) and hope that users will see it. Maybe the best solution is to document in a minor release line, and then make breaking changes in a major version?

Issue Links

• is superceded by: CURATOR-99 Java 8 refactoring
• links to: GitHub Pull Request #63