Refactor nodejs generated code structure #4909
Conversation
| @@ -26,6 +26,6 @@ fi | |||
|
|
|||
| # if you've executed sbt assembly previously it will use that instead. | |||
| export JAVA_OPTS="${JAVA_OPTS} -XX:MaxPermSize=256M -Xmx1024M -DloggerPath=conf/log4j.properties" | |||
| ags="$@ generate -i modules/swagger-codegen/src/test/resources/2_0/petstore.yaml -l nodejs-server -o samples/server/petstore/nodejs" | |||
| ags="$@ generate -t modules/swagger-codegen/src/main/resources/nodejs -i modules/swagger-codegen/src/test/resources/2_0/petstore.yaml -l nodejs-server -o samples/server/petstore/nodejs" | |||
There was a problem hiding this comment.
To make it easier to test template changes, the bin script is now loading templates directly from the file system instead of the compiled jar
| @@ -1,67 +0,0 @@ | |||
| { | |||
There was a problem hiding this comment.
@wing328 I think this file was added on accident? Please let me know and I can restore them if needed.
There was a problem hiding this comment.
The file was added on accident. Thanks for removing it.
| @@ -1,30 +0,0 @@ | |||
| { | |||
There was a problem hiding this comment.
@wing328 same here...
There was a problem hiding this comment.
The file was added on accident. Thanks for removing it.
|
|
||
| public class NodeJSServerCodegen extends DefaultCodegen implements CodegenConfig { | ||
|
|
||
| private static final Logger LOGGER = LoggerFactory.getLogger(NodeJSServerCodegen.class); | ||
|
|
||
| protected String implFolder = "service"; |
There was a problem hiding this comment.
Will adjust PR to make this configurable.
| @@ -1,13 +1,21 @@ | |||
| 'use strict'; | |||
|
|
|||
| var url = require('url'); | |||
| require('../utils/writer.js'); | |||
There was a problem hiding this comment.
pull in utility functions for writeJson
| {{/returnType}} | ||
| **/ | ||
| exports.{{{operationId}}} = function({{#allParams}}{{paramName}}{{#hasMore}},{{/hasMore}}{{/allParams}}) { | ||
| return new Promise(function(resolve, reject) { |
There was a problem hiding this comment.
No more req, res, only using Promise lib.
| writeJson(res, response); | ||
| }) | ||
| .catch(function (response) { | ||
| writeJson(res, response); |
There was a problem hiding this comment.
to send specific response codes, use the respondWithCode function:
writeJson(respondWithCode(500, { message: 'it broked'}));
| Pet.updatePetWithForm(req.swagger.params, res, next); | ||
| var petId = req.swagger.params['petId'].value; | ||
| var name = req.swagger.params['name'].value; | ||
| var status = req.swagger.params['status'].value; |
There was a problem hiding this comment.
note, all input values are extracted in generated code to make it easier to implement the service class.
|
@wing328 if you have any feedback please let me know, otherwise we can merge this. |
|
@fehguy I did some tests and the result is good. |
This is a breaking change in the generated code structure for nodejs.
In all previous versions of the nodejs generated code, there was a sort of silly abstraction layer--there were controllers for receiving calls and service classes, designed to hold business logic. The challenge with the existing structure was that the service class was interacting directly with the response object, and thus there was a mix of business logic and routing. Way too convoluted for most nodejs developers.
This change breaks up the responsibilities. There is a controller and service class separation of concerns still, but they are:
separated into different folders. This makes it easier to ignore changes in generated code in the service folder to avoid overwriting files.
Almost completely abstracting away the request/response cycle from the service class
Use of promises for better code structure
The Almost is because in the service classes, there are scenarios where you need to indicate the response code that goes back over the wire. To do this, a utility class is required, and you can then specify what code you want to send to the caller.