search

swagger-api / swagger-codegen

swagger-codegen contains a template-driven engine to generate documentation, API clients and server stubs in different languages by parsing your OpenAPI / Swagger definition.
http://swagger.io
Apache License 2.0
16.91k stars 6.03k forks source link

Customization of folder structure for a template #3515

Closed osher closed 6 years ago

osher commented 8 years ago
Description

I'm not sure I'm in the right place, please bear with me...

  1. I'm evaluating the tools for a nodejs team.
  2. I need to customize the folder structure: add a generated tests folder, separate the web-controllers from the "services" (uh, we don't call them services in node...) and such.

For example, 

eval-codegen-nodejs
│   .swagger-codegen-ignore
│   index.js
│   LICENSE
│   package.json
│   README.md
├───api
│       swagger.yaml
└───controllers
        <name>.js
        <name>Service.js

to: 

eval-codegen-nodejs
│   .swagger-codegen-ignore
│   LICENSE
│   package.json
│   README.md
├───bin
│   └──server.js
├───scheme
│   └──scheme.yaml
├───lib
│   └──<name>.js
├───web
│   └──<name>.js
└─── test
    ├───lib.web.index.js
    └───lib.

I don't find any documentations about it :( Can anybody point me to the right direction?

Swagger-codegen version

cloned from master + a PR I applied (which was merged to master) So call it 2.2.1-latest?

Swagger declaration file content or url

This is not a bug the spec file produces.

Command line used for generation
java -jar swagger-codegen\modules\swagger-codegen-cli\target\swagger-codegen-cli.jar generate -l nodejs-server -o eval-codegen-hello -i hello.openapi.yaml -t swagger-codegen-nodejs

(windows user)

Steps to reproduce

It works, but I want it to work differently. I want to control the folder-structure.

Related issues

none

Suggest a Fix

Maybe it's a completely new feature request.

Ideally, As a user - I want to provide a path, and a list of a per-controller files (call it option 1) or - alternatively - a project dir path, a controller-router dir path (call it option 2)

Then, the expected behavior would be:

every file in the project dir recursively is bound with the model once and rendered to the target folder, preserving relative path it's found in the source dir

if we're doing 1st option - the files listed as per-controller-router files are excluded of the first round, but are bound once per controller-router with a model that features also a reference to the current operation (can be null when binding project files)

if we're doing 2nd option - then, recursively, per controller-router - every file in the controller dir path is bound with the model once and rendered to the target folder, again preserving relative path it's was found in the source dir (2nd option probably allow more code-reuse with the folder-structure recursion and all...)

I deem this will grant enough control to use codeGen to provide any folder structure.

osher commented 8 years ago

I've done some more research. It looks like the nodejs scene has progressed a lot since last the nodejs templates that are suggested here were created.

Currently, there are two main generators in the swagger family: node-swagger and this project.

Comparison in short:

This project is more advanced in the manner that it generates handlers capable of responding mock replies based on the responses declared in the spec, where node-swagger generates only a boilerplate from which the developer has to proceed by himself. However, the node-server generated by codeGen renders a solution that is a very low-level project and in modern terms - even partial. Node-swagger also generates tests, and acts as a minimalist CLI task-runner that lets you for instance in one command launch the server and the swagger-editor.

So codeGen is more advanced in that it creates a server capable of returning mock replies but node-swagger is more advanced in the tools that it brings to the generated project, generation of tests, the folder-structure it lets you work with, and the CLI workflow helpers.

By supporting folder-structure customization, IMHO - this project could gain an additional edge aside to the mock responses. If I could customize the folder structure like I described above - I could create a template for this engine that renders a project that brings modern tools AND is capable of rendering mock replies. I suppose that with the right configuration - one could still use node-swagger's CLI helpers - and enjoy all benefits.

Thoughts?

wing328 commented 8 years ago

@osher thanks for the feedback on the nodejs-server generator.

For nodejs server-side project, is there a recommended folder structure for the project?

Technically it's possible to introduce CLI options to customize the folder for different auto-generated JS files but I think it's important to understand if there's any best practices for nodejs folder structure.

For auto-generated test cases, we have that for other language generator (but not all), e.g. Ruby, Python, PHP, C#, Java, etc: https://github.com/swagger-api/swagger-codegen/issues/2276 and we're looking for contribution to add those for nodejs-server. Let me know if you want to contribute on that.

osher commented 8 years ago

For nodejs server-side project, is there a recommended folder structure for the project?

Such a typical question for Java folk ;) anyway - Good question, I'm happy you asked. You deserve a good answer, however I'll need a little time to write it well. Sorry to keep you waiting - I now just wanted to let you know I heard you

I may not be able to post it today, and this is the end of my week. If not to day - I'll post it on sunday.

wing328 commented 8 years ago

@osher please take your time. We always welcome feedback to make Swagger Codegen better. Your suggested folder structure looks good to me.

What I would like to do it to make sure we're following the NodeJS industry standard/style guide, if any, to avoid changing it again in the future.

osher commented 8 years ago

First - disclaimer:

Any folder-structure would be _opinionated_ - and JS developers are often kinda ...gently put - sensitive to opinionsm. Bottom line is that if you want to address the lion share of the JS community - you want to avoid opinionist structure, and just let them do what the heck the tools they use require. It's a template after all - a template for a generator. Whatever they decide on - will become the repeated form they work with - and it's OK.

To show you around the versatility in the JS world, I will provide 2 structures bellow, but please hear me here:

honest recommendation

I recommend to strive to decouple the generator logic from the folder structure

My heart tells me that if you can do that - you won't need so many generator classes as you now have in the project, because you will work out of the generic base common to all of them. But that's a side-effect of my proposition, it's not my goal. But if we're already here: We want that all the differences between the projects will be described in the source template folder where now it's described partly in template files and partly in hardcoded generator classes. IMHO - for no good reason. (oh, you'd still be able to inherit the base class and override it's method to customize it by need, but in majority of cases - why would you...).

According to this recommendation - the feature would work best if a source folder structure is cloned, and by recursive iteration - every file in that source structure is bound to it's model and rendered into the relative path in the target.

In this I can distinct between 2 types of source template files, according to the model they require and the times they are bound

mm. well. my bad. I missed in my original post a 3rd type. As a JS developer I did not consider it, but I can see cases where it may be needed, especially when I try to think how I would use the same generator to produce typed C#/Java/C++/Erlang/etc... projects. So the 3rd type is:

The user wants e2e tests? great. The template structure will include test files, bound per apiDoc.path. The user wants model tests? great. The template structure will include test files, bound per apiDoc.definition. The user wants serializers? encoders? utilities? great. same: let them add a template. We just need to support binding with the relevant part of the api-doc model, and we don't need a generator per language. All we need to know is if the template should be bound only with swagger document, or bound once per project, once per-path, or once per definition.

Well. There could be a discussion about case in which same controller handles few paths. I have a strong feeling you've solved this modeling problem already. However you do it - I refer the same model and same iteration times as depicted by the current logic. (assuming the x-controller-router & multiple tags bug is solved... :smile: )

sample folder structures

So now, like I promised - here are two options of folder structures.

what is provided by the swagger-node

The swagger-node supports 5 templates for 5 competing nodejs frameworks: connect, express, restify, hapi, sails. The first 4 result with the exact same structure, where the difference is what libs are loaded in the main file, how they are initiated, and how they are accessed by the cotroller & service - so it's basically slightly different text in same files. The sails is more bloated with sails instrumentation, I don't want to get into that. But if we support dynamic folder structure - we cover it too with ease.

The command is swagger project create --framework <framework> The result is:

C:.<foo>
├───api
│   ├───controllers
│   │       hello_world.js
│   │       README.md    - sais: "Place your controller files in this directory"
│   ├───helpers
│   │       README.md     - sais: "Place your helper files in this directory"
│   ├───mocks
│   │       README.md     - sais: "Place your mock files in this directory"
│   └───swagger
│           swagger.yaml
├───config
│       default.yaml
│       README.md        - sais: "Place your config files in this directory"
└───test
    └───api
        ├───controllers
        │       hello_world.js
        │       README.md     - sais: Place your controller tests in this directory
        └───helpers
                README.md      - sais: Place your helper tests in this directory
    .gitignore
    app.js
    package.json
    README.md

However, the boiler-plate there is very simplistic. It does not read a spec-file, and does not use a templating engine to bind it. It's purpose is just to show you an opinionated way (which is already a pitfall in the JS community). The project probably still is in early steps in it's evolution. It's very clean, but the prepared extensibility points are clear the keen eye - so there's lot's of work there.

Anyway - this structure, contradict the opinions in my school. So here:

What I would do if I had the freedom to do it

First - we use a flat test directory. Any directories in the test folder are reserved to test tools, fixtures and mocks, and paths of test targets are encoded in file-names . instead of /. Second - we put all the files that need coverage tests and JsDoc generation in ./lib, where ./lib/index is the main file in the pacackage. If the package is a service - then a file in ./bin will require ./lib, and pass to it arguments extracted from shell context (like switches, configs read from disk and env-variables). This file in the ./bin will also handle SIGINT and SIGTERM signalls, initiate loggers, and other context-related works. If the package is an ETL job - same. Or a CLI tool to setup some things. donno. all the same :) All files in ./bin execute things and are tested in e2e/integration tests. All files in ./lib declare modules that may be require()ed. are tested by unit tests, tracked by code-coverage, and JsDocs are generated for the modules they declare.

Having that said - assuming the package is named foo - we end with:

├─── bin
│       foo-server
│       foo-etl                         //we don't care about this one - it won't be generated
│       foo                             //we don't care about this one - it won't be generated
├─── config
│   ├─── default
│   │      config.yaml  
│   └─── {env}                  //this will also never be generated. it's domain specific
│          config.yaml  
├─── lib
│   ├─── dal
│   │   {persistence/service/whatever}.js  //this will also never be generated...
│   ├─── model
│   │   {definition}.js
│   └─── web
│       {controller}.js
│       index.js
├─── scheme
│    api-docs.yaml / api-docs.json    //howevre we got it in the -i switch
├─── test
│       lib.model.{definition}.test.js
│       lib.web.{controller}.test.js
│       mocha.opts
└─── test-e2e
        foo.{path}.test.js
        foo-etl.test.js                           //not generated
        foo.test.js                               //not generated
    .gitignore
    .npmignore
    CHANGELOG.md
    package.json
    README.md

I saw somewhere that you already support a flag to suppress Model generations. That's good. Most of loosly-type languages (like JS) won't need models. However, models may be facilitators that aggregate the raw JSON.parse()ed object and facilitate traversing in it, and their generation is usually domain-specific - and that's where the power custom-mustache should come in :D

randavidovitz commented 8 years ago

I also agree that we need to have different folder structure that is more up to date, as a default I would say above will be good starting point

I am not sure its important on phaze 1 to enable client to decide on different folder structure other than above

Must share with you that this project remind me of Yoman

osher commented 8 years ago

I thought of another way to tell what model should be bound with what file.

The proposition relays on encoding in the file name all the information about how it should be bound. This leaves us with just one folder

file-names should also be bound with placeholders, for {{controller}} and {{definition}}

Examples: assume 1 api doc with 8 operations in 4 paths handled by 2 controllers, and 3 definitions:

Please excuse me - I did not take the time to find the entry names you currently use for {{controller}} and {{definition}} - I really have to call it a day (and a week actually) Buy as a semantic person - I'd love to see them adopted and supported side by side to their current names...

I'm not quite happy with the part in the proposition that handles partials. maybe the template directory needs to be split to ./main and ./partials or something in that spirit, but I really have to go.

I'd be happy to hear your thoughts.

Last - I presented my research here in the organization, and showed them the fast responses I got from you. Although the last time I really did Java it was with Java1.2, this organization have a strong Java team, and they are currently considering allocating some dev-time to contribute a PR with this functionality. How does that sound to you?

fehguy commented 8 years ago

There are definitely many opinions for this. I suggest avoiding making this too smart right now, and target a 3.0 release. As it stands, the technique to change folder structures is embedded in java logic and until we do a major redo on the generation step, let's put this on hold.

osher commented 8 years ago

@fehguy: I think I understand. However, we might want to fork and have a start anyway the effort, from which a PR could be applied. It's pending leadership decision here.

In that storyline of developing a template for ourselves - as a first step, my POC is to create a template that uses modern node-js tools for the existing NodeJS server generator. That will be done right after selecting between express, restify, and hapi. (connect is too low level, and sails is overbloated for us). Would you like this as a separate PR?

JeffML commented 7 years ago

I, for one, would just like to see the mocks separated from the controllers, and have the controllers call the mocks based on an environment variable. Something like (pseudocode):

var mock = process.env(‘mock’);

if (mock) {
    exports = require(‘./mocks/<thisfilename>’)
} else {
... /* real service here */
}

I've poked around a bit at the the languages source code, to figure out how I would generated a mock folder; looks like the override apiFileFolder() determines the (single) output folder for the controller api, used by AbstractJavaJAXRSServerCodegen.java. This is just the start of the rabbit hole, I'm pretty sure.

fehguy commented 7 years ago

Hi folks, I've worked on a number of projects using this template and have some follow-up thoughts.

First, the original idea was from a (ex) java guy. So the delegation between the controller and service is, shall we say, a little lame. I have recently been building a lot of services and have modified the templates to have the same structure--a controller and a service class--but to user Promises and clearly separate the request/response objects between the controller and service class. So the pattern for a method in the controller might be like this:

module.exports.setHeaterState = function setHeaterState (req, res, next) {
  var zoneId = req.swagger.params.zoneId.value;
  var state = req.swagger.params.state.value;

  environment.setHeaterState(zoneId, state)
    .then(function (data) {
      helpers.json(res, data);
    })
    .catch(function (err) {
      helpers.json(res, err, err.code);
    });
};

the helpers.json simply returns the response body to the caller, and optionally with a specific http status code.

The implementation (environment in this example) looks something like this:

exports.setHeaterState = function(zoneId, state) {
  return new Promise(function(resolve, reject) {
    // business logic here
    if(notFound) {
      reject({
        code: 404, status: 'not found'
      });
      return;
    }
    resolve({
      code: 200,
      message: 'set zone ' + zoneId + ' to ' + state
    });
  });
}

Second, the folder structure currently is a pain. Why? Well, there are no shortages of opinions, but from a tooling person, it's nice to have a clear separation of auto-generated classes (controller classes in this case) and human modified ones (service classes). The goal would be that the controller classes are always overwritten, and having them in a separate folder would be easier for this house keeping.

Third, the default routes for, say the docs and the swagger json file, are configurable and pretty much never right with this template set. They should "default to the best practice" and allow for overrides.

I personally don't mind the mock code living in the service classes. The reason is, it gives you something to start with when implementing, and you're going to remove it anyway. If the service has a flag to enable/disable mocking, then it may make sense to have something else to intercept the requests, and therefore a mock folder may make sense.

JeffML commented 7 years ago

Since I am writing against legacy code with its own API, I have my service template written to call an adapter; the idea is that both Controller and Service get overwritten, and the Service template checks for its Adapter... if not found, it will return a stub (mock).

In the service template, I pushed all the stub data to the bottom of the file and reference each stub by method name (e.g., petGET). I wrote a test template and modified codegen to put those in a separate folder.

One thing I didn't like having to do was to modify the NodeJS generator code to add new templates. New template paths, file names, and model type could be read into the generator code via a data structure. It's not that changing/compiling the swagger-codegen code was hard, but all I was doing was adding data inline to methods that could have read that data in from some source.

fehguy commented 7 years ago

Please see https://github.com/swagger-api/swagger-codegen/pull/4909 for my proposed solution for this issue @JeffML @osher

wing328 commented 7 years ago

4909 has been merged into master.

@JeffML @osher please pull the latest master to give it a try.

osher commented 6 years ago

@wing328 - I really appreciate the persistence and the seriousness but I had to move on with other solutions, so I'll have to admit I stopped following. I would like to say I'll try it, but the chances now are rather low - so I wouldn't like to build false expectations...

wing328 commented 6 years ago

@osher no problem. Just open a new issue if you need help from us.

whitte-h commented 3 years ago

@osher in the end, what solution did you use? I am also researching a way to make a custom project / CRUD template with structure/test/docs included, at the moment this is the best candidate. Sorry to bother you with this old thread, thanks

osher commented 3 years ago

wow. that was a long time ago. I basically wrote a nodejs tool based on handlebars as templating engine, and the swagger parser that were modern at the time (don't even remember the name, but it worked great, but only with OAS 2.x). Unfortunately, the organization I was working for was not willing to opensource it, so it died with it AFAIK during this recent corona crisis.

But back then I went all in: The generator read your swagger, generated a server with mock responses based on the declared model, a test suite with a suite per endpoint and a case per parameter-permutation, asserted the validity of the returned model, and a client package to consume your service with. All you had to do was provide server implementation, and sometimes add that complicated specific edge test-case. If you're doing micros - then the happy paths of the integration tests put you in 90% coverage without any effort. The generator itself was tested with a very robust end-to-end suite, proofing validity of all kinds of swagger expressions and expected CLI behavior. It not only tested that the generation succeeds - but also that the generated server runs and that it passes the generated tests, and that the client works with it well... huh. I had fun those days... I even had a spec-first workflow lecture I toured with for a while, before life took me to another direction

Anyway, the customers after that were not much into swagger, so I'm quite outdated by now because I was doing other things (diverted a lot towards the ops side, just until this swinging pendulum will take me back).

If I had to do it again - I'd still chose node and handlebars or mustache as engines, but probably use more modern promisified helpers and OAS 3.x parsers.