apigee-127 / swagger-node-runner

The heart of Swagger-Node
MIT License
102 stars 124 forks source link

204 No Content - Mocked mode #91

Open facundovictor opened 7 years ago

facundovictor commented 7 years ago

I know this may be related to the Issue 79, and it's not my intention to replicate it, but to avoid deviating the conversation of the main thread. Of course, if you think it should go within Issue 79, please let me know and I'll add a reference in it.

Version: "_id": "swagger-node-runner@0.7.2"

Resume: Running swagger in mock mode (-m), and executing a DELETE method that should return 204 (no content) on success, will fail (500 Internal Server Error):

TypeError: Cannot read property 'getExample' of undefined
    at swagger_router (/home/fvictor/Workspace/git_repos/swagger-express/node_modules/swagger-node-runner/fittings/swagger_router.js:120:51)
    at Runner.<anonymous> (/home/fvictor/Workspace/git_repos/swagger-express/node_modules/bagpipes/lib/bagpipes.js:171:7)
    at bound (domain.js:280:14)
    at Runner.runBound (domain.js:293:12)
    at Runner.pipeline (/home/fvictor/Workspace/git_repos/swagger-express/node_modules/pipeworks/pipeworks.js:72:17)
    at Runner.flow (/home/fvictor/Workspace/git_repos/swagger-express/node_modules/pipeworks/pipeworks.js:223:19)
    at Pipeworks.flow (/home/fvictor/Workspace/git_repos/swagger-express/node_modules/pipeworks/pipeworks.js:135:17)
    at Pipeworks.siphon (/home/fvictor/Workspace/git_repos/swagger-express/node_modules/pipeworks/pipeworks.js:186:19)
    at Runner.<anonymous> (/home/fvictor/Workspace/git_repos/swagger-express/node_modules/bagpipes/lib/bagpipes.js:98:22)
    at bound (domain.js:280:14)

How to reproduce it?: As an example, I'm building the next DELETE method:

paths:
  /client/{id}:
    delete:
      # Controller
      operationId: deleteClient
      description: Removes the client that corresponds to the identifiers (if exists).
      parameters:
        - name: id
          type: string
          in: path
          required: true
      responses:
        "204":
          description: No Content
        "404":
          description: Not Found
          schema:
            $ref: "#/definitions/ErrorResponse"

As you know, from w3 spec:

The 204 response MUST NOT include a message-body, and thus is always terminated by the first empty line after the header fields.

And from the Swagger Specification, it's clear that the schema shouldn't be present if we have to return no body (i.e. no content):

Field Name Type Description
schema Schema Object A definition of the response structure. It can be a primitive, an array or an object. If this field does not exist, it means no content is returned as part of the response. As an extension to the Schema Object, its root type value may also be "file". This SHOULD be accompanied by a relevant produces mime-type.

Now, try running swagger project start -m with that endpoint defined and you'll get the error.


My Thoughts:

IMHO, the swagger mock mode objectives are: Help to build the controller tests before start coding the back-end, and allow the front-end side to start developing right away. Thus, the mock mode should work without needing to add mocked controllers, at least for basic and standard crud requests (like the DELETE one).

About the code, I'm seeing that the mock code demands to have defined the request property _mockreturnstatus. But, it's not defined, and the statusCode defaults to 200. Also, as the operation doesn't have defined an answer with a 200 code, it will return undefined. Am I doing something wrong?

    if (mockMode) {

      var statusCode = parseInt(context.request.get('_mockreturnstatus')) || 200;

      var mimetype = context.request.get('accept') || 'application/json';
      var mock = operation.getResponse(statusCode).getExample(mimetype);

      if (mock) {
        debug('returning mock example value', mock);
      } else {
        mock = operation.getResponse(statusCode).getSample();
        debug('returning mock sample value', mock);
      }

      context.headers['Content-Type'] = mimetype;
      context.statusCode = statusCode;

      return cb(null, mock);
    }

And even solving this issue, the library sway won't find a schema to fake:

Response.prototype.getSample = function () {
  var sample;

  if (!_.isUndefined(this.definitionFullyResolved.schema)) {
    sample = helpers.getJSONSchemaMocker()(this.definitionFullyResolved.schema);
  }

  return sample;
};

And this is OK, because the answer should have no content, because the schema isn't specified. But, the swagger runner won't end the response, and the bagpipes library won't end the response right there either.... moving us to Issue 79.

So far, I'm adding a mocked controller like:

function deleteClient (req, res) {
   // TODO: do more mocked stuff with a mocked param
   res.sendStatus(204);
}

But, IMHO, if the idea is to quickly have a fake API to start working with, and only from the swagger specifications, we shouldn't have to write mocked controllers for simple requests like this one. Am I missing something?

ath88 commented 7 years ago

I have the same problem. It would be nice to not have to build mock controllers to have 204 work.