Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decouple generation workflow from DefaultGenerator and CodegenConfig types #841

Open
jimschubert opened this issue Aug 18, 2018 · 0 comments

Comments

@jimschubert
Copy link
Member

Description

Some discussion around this exists in #503. I'm copying here for context.


Conceptually, we have the following concerns:

  • Generation workflow
  • Templating engine
  • Global Options
  • Framework generator
  • Language definition
  • Generator Options
  • Spec loading
  • Spec parsing
  • API definition ADT (e.g. CodegenModel)
  • Template object ADT (currently, a map)
  • potentially others I can't think of at the moment

Everything above is rolled into DefaultGenerator and DefaultCodegen (or derived class). There's very little reusability. Where we have reusability at the language level, it is done as an abstract base class. This is problematic because changes in the base class for a target framework (you may be asking why framework logic exists in a language base class…) inadvertently introduces a bug in other generators sharing the same base class.

Our current design makes a refactor a little more difficult because the entry point into the process is DefaultGenerator, which holds a reference to the generator (CodegenConfig/DefaultCodegen instance). We use a form of delegation from the generator to the "config" component at almost every step of the workflow. This is intimidating for new contributors because it's not only undocumented, but is hard to follow even while debugging.


There are cases where our workflow could be simplified by sharing the process but offloading language-specific or framework-specific modifications off to the CodegenConfig classes.

In other cases, users may not want the processing workflow to be done in the way as it is in DefaultGenerator. There's no clean way to provide any generator other than the default, so it seems like the code diverged at some point from its originally intended design.

Suggest a fix/enhancement

This will be a major breaking change. But I recommend that we do the following:

Part 1 Renames

  • Rename Generator to Workflow
  • Rename AbstractGenerator to AbstractWorkflow
  • Rename DefaultGenerator to DefaultWorkflow
  • Rename CodegenConfig to Generator
  • Rename DeafultConfig to GeneratorBase
  • Rename ClientOpts to WorkflowOptions
  • Rename or remove ClientOptInput (type is not really needed, see "New Functionality" below)

Part 2 Refactors

  • Add a property to GeneratorBase (prev DefaultConfig) which exposes a Worfkflow (prev Generator).
    • This allows each generator to have control over its desired workflow.
    • This allows users to extend existing or new generators with their own new or derived workflows
  • Add a property to GeneratorBase (prev DefaultConfig) which exposes the desired templating engine. Vague details here, specifically because Templating Engine API and handlebars support #690 and Data object classes for templating context #837 are in progress at the time of writing this
    • This allows generators to define their preferred templating engine
    • The executor will need to inject either the Generator or templating engine instance into the Workflow option for generation

Part 3 New Functionality

  • Create a new class called Executor which does the following (basically everything in current Generator#generate up until the call to DefaultGenerator.generate):
    • Parses the spec
    • Resolves the Generator (prev CodegenConfig) type
    • Invokes the Workflow functionality (passing desired template engine into workflow's run/execute method)
    • Resolves all generator/workflow modifications from System Properties (See current DefaultGenerator#configureGeneratorProperties and Generator#generate), and sets these properties either on the instance or on additionalProperties before invoking the desired workflow
    • Passes WorkflowOptions (prev ClientOpts) to Workflow instance
  • Add CLI option to override workflow and/or template engine

There may be other concerns not listed above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants