picocli icon indicating copy to clipboard operation
picocli copied to clipboard

mixin inheritance causing DuplicateOptionAnnotationsException

Open jinyius opened this issue 3 years ago • 4 comments

i'm writing modular code for various data pipelines. i have various "segments" that provide logical functionality for the pipeline. i currently have the cli args tightly coupled with these segments and use the @Mixin annotation to appropriately include these segments in the different pipeline binaries.

i'm hitting a DuplicateOptionAnnotationsException because i use a base class for two segments that share common arguments. here's the setup in toy classes:

// Base class of mixin hold shared functionality for dealing w/ Kafka.
class KafkaSegment {
  @Option(...)
  String server = "";
)

// Kafka source related logic needs stuff from KafkaSegment and itself
class KafkaSource extends KafkaSegment {
  @Option(...)
  boolean sourceArg = true;
}

// Kafka sink related logic needs stuff from KafkaSegment and itself
class KafkaSink extends KafkaSegment {
  @Option(...)
  long sinkArg = 42L;
}

// A Command class
@CommandLine.Command(...)
class SomePipelineJob {
  @CommandLine.Mixin final KafkaSource = new KafkaSource();
  @CommandLine.Mixin final KafakSink = new KafkaSink();
}

here, since the source and sink mixins are included, all options in KafkaSegment are defined twice, so i get the dupe exception. i tried @Spec(MIXEE) but that's somewhat inelegant , and likewise, defining common args at the command level is also inelegant b/c it breaks encapsulation.

i tried making the options in KafkaSegment static, but it doesn't work as expected. generally, it'd be nice to treat static @Option fields as if they're defined once, even if it's coming in from mixins that have inherited the fields from a parent class. thereafter, the value supplied for the parent arg via cli should be available for both child mixins.

any help here would be appreciated. for now, i'm going to try and make KafkaSegment a separate mixin and try to compose the logic in the "child" segments by passing in a single instance of KafkaSegment to each of their constructors.

jinyius avatar Dec 21 '21 23:12 jinyius

mental data model wise, using DI w/ singleton injection is what i'd want for the parent segment classes. would using an interface instead of class inheritance help here? should i use a transformer to mess w/ the model?

jinyius avatar Dec 21 '21 23:12 jinyius

Sorry for my late response.

~~If you need to combine inheritance with mixins, then one idea would be to de-normalize the mixins:~~

  • ~~move @Option-annotated fields out of the mixin superclass to avoid the duplicate option error~~
  • ~~retain only shared functionality (that is, shared logic, not shared data)~~
  • ~~consider making the mixin superclass an abstract class with an abstract getter for the shared data~~

UPDATE: please ignore this comment and go to the next comment instead. 😓

// Base class of mixin hold shared functionality for dealing w/ Kafka.
abstract class KafkaSegment {
    protected abstract String getServer(); // <-- only shared functionality, no shared data

    public void sharedLogic() {
        String server = getServer();
        // do something...
    }
)

// Kafka source related logic needs stuff from KafkaSegment and itself
class KafkaSource extends KafkaSegment {
  @Option(...)
  String server = ""; // <-- data is denormalized to avoid the duplicate option error
  protected String getServer() { return server; }

  @Option(...)
  boolean sourceArg = true;
}

// Kafka sink related logic needs stuff from KafkaSegment and itself
class KafkaSink extends KafkaSegment {
  @Option(...)
  String server = ""; // <-- data is denormalized to avoid the duplicate option error
  protected String getServer() { return server; }

  @Option(...)
  long sinkArg = 42L;
}

remkop avatar Feb 11 '22 07:02 remkop

Oops. I did not think that through... 😅 The above solution will still throw a duplication option error... Sorry about that.

Take 2.

The duplicate option error arises because the command has two mixins. What if we make that into just one mixin?

@CommandLine.Command(...)
class SomePipelineJob {
  @CommandLine.Mixin final KafkaFacade = new KafkaFacade();
}

This KafkaFacade class contains all options of both KafkaSource and KafkaSink. If some parts of the logic only need source-related functionality, we can introduce a KafkaSource interface that exposes only source related data and functionality. Similarly for KafkaSink.

class KafkaFacade implements KafkaSource, KafkaSink {
  @Option(...)
  String server = "";

  @Option(...)
  boolean sourceArg = true;

  @Option(...)
  long sinkArg = 42L;

  public String getServer() { return server; }
  public String getSourceArg() { return sourceArg; }
  public String getSinkArg() { return sinkArg; }
}

interface KafkaSource {
    String getServer();
    String getSourceArg();
}

interface KafkaSink {
    String getServer();
    String getSinkArg();
}

remkop avatar Feb 11 '22 08:02 remkop

i wanted to have method implementations (concrete classes) for the source and sink.

~~i guess i can try to use java8+'s default implementations in interfaces... lemme try and get back to you on this.~~ i do recall that i did try using default implementations on interfaces before, but this did not work. i'll try and remember why and follow up here.

jinyius avatar Apr 14 '22 06:04 jinyius