Make sure that the json property is set when the content is JSON (#140)

* Change all calls to request to use the body property instead of the json property * json property is now set if the produces or consumes is a JSON media type
apigee-127 · May 5, 2017 · fbaafcb · fbaafcb
1 parent 5ac4575
commit fbaafcb
Show file tree

Hide file tree

Showing 36 changed files with 518 additions and 82 deletions.
diff --git a/index.js b/index.js
@@ -559,6 +559,7 @@ handlebars.registerHelper('length', helpers.length);
 handlebars.registerHelper('pathify', helpers.pathify);
 handlebars.registerHelper('printJSON', helpers.printJSON);
 handlebars.registerHelper('requestDataParamFormatter', helpers.requestDataParamFormatter);
+handlebars.registerHelper('isJsonRepresentation', helpers.isJsonRepresentation);
 
 
 module.exports = {

diff --git a/lib/helpers.js b/lib/helpers.js
@@ -3,7 +3,6 @@
 var _ = require('lodash');
 var strObj = require('string');
 var url = require('url');
-var TYPE_JSON = 'application/json';
 var len;
 
 module.exports = {
@@ -15,7 +14,8 @@ module.exports = {
   printJSON: printJSON,
   len: len,
   setLen: setLen,
-  requestDataParamFormatter : requestDataParamFormatter
+  requestDataParamFormatter : requestDataParamFormatter,
+  isJsonRepresentation : isJsonRepresentation
 };
 
 function setLen(descriptionLength) {
@@ -60,13 +60,30 @@ function validateResponse(type, noSchema,
       'needs 2 parameters');
   }
 
-  if (!noSchema && type === TYPE_JSON) {
+  if (!noSchema && isJsonMediaType(type)) {
     return options.fn(this);
   } else {
     return options.inverse(this);
   }
 }
 
+/**
+ * decides if this request/response has a JSON representation
+ * @param {string} contentType    the media type of the request
+ * @param {string} returnType     the media type of the response
+ */
+function isJsonRepresentation(contentType, returnType, options) {
+  return (isJsonMediaType(contentType) || isJsonMediaType(returnType)) ? options.fn(this) : options.inverse(this);
+}
+
+/**
+ * determines if the mediatype is json
+ * @param {string} type   content type to be evaluated
+ */
+function isJsonMediaType(type) {
+  return type.trim().toLowerCase().substr(type.length - 4) === 'json';
+}
+
 /**
  * replaces path params with obvious indicator for filling values
  * (i.e. if any part of the path is surrounded in curly braces {})

diff --git a/templates/request/delete/delete.handlebars b/templates/request/delete/delete.handlebars
@@ -7,6 +7,9 @@
       {{/validateResponse}}
       request({
         url: '{{pathify path pathParams}}',
+        {{#isJsonRepresentation contentType returnType}}
+        json: true,
+        {{/isJsonRepresentation}}
         {{#ifCond queryParameters queryApiKey}}
         qs: {
           {{#if queryApiKey}}{{queryApiKey.type}}: process.env.{{queryApiKey.name}}{{#if queryParameters}},

diff --git a/templates/request/get/get.handlebars b/templates/request/get/get.handlebars
@@ -7,6 +7,9 @@
       {{/validateResponse}}
       request({
         url: '{{pathify path pathParams}}',
+        {{#isJsonRepresentation contentType returnType}}
+        json: true,
+        {{/isJsonRepresentation}}
         {{#ifCond queryParameters queryApiKey}}
         qs: {
           {{#if queryApiKey}}{{queryApiKey.type}}: process.env.{{queryApiKey.name}}{{#if queryParameters}},

diff --git a/templates/request/patch/patch.handlebars b/templates/request/patch/patch.handlebars
@@ -7,6 +7,9 @@
       {{/validateResponse}}
       request({
         url: '{{pathify path pathParams}}',
+        {{#isJsonRepresentation contentType returnType}}
+        json: true,
+        {{/isJsonRepresentation}}
         {{#ifCond queryParameters queryApiKey}}
         qs: {
           {{#if queryApiKey}}{{queryApiKey.type}}: process.env.{{queryApiKey.name}}{{#if queryParameters}},
@@ -20,10 +23,10 @@
           Authorization: '{{headerSecurity.type}} ' + process.env.{{headerSecurity.name}}{{/if}}
         },
         {{#is contentType 'application/json'}}
-        {{#if request}}json: {
+        {{#if request}}body: {
         {{ request }}
         }
-        {{else}}json: {
+        {{else}}body: {
           {{#each bodyParameters}}
           {{this.name}}: 'DATA GOES HERE'{{#unless @last}},{{/unless}}
           {{/each}}
@@ -122,7 +125,7 @@
               Authorization: '{{headerSecurity.type}} ' + process.env.{{headerSecurity.name}}{{/if}}
             },
             {{#is contentType 'application/json'}}
-            json: {
+            body: {
               {{#each bodyParameters}}
               {{this.name}}: 'DATA GOES HERE'{{#unless @last}},{{/unless}}
               {{/each}}

diff --git a/templates/request/post/post.handlebars b/templates/request/post/post.handlebars
@@ -7,6 +7,9 @@
       {{/validateResponse}}
       request({
         url: '{{pathify path pathParams}}',
+        {{#isJsonRepresentation contentType returnType}}
+        json: true,
+        {{/isJsonRepresentation}}
         {{#ifCond queryParameters queryApiKey}}
         qs: {
           {{#if queryApiKey}}{{queryApiKey.type}}: process.env.{{queryApiKey.name}}{{#if queryParameters}},
@@ -21,7 +24,7 @@
           Authorization: '{{headerSecurity.type}} ' + process.env.{{headerSecurity.name}}{{/if}}
         },
         {{#is contentType 'application/json'}}
-        {{#if request}}json: {{ request }}{{else}}json: {
+        {{#if request}}body: {{ request }}{{else}}body: {
           {{#each bodyParameters}}
           {{this.name}}: 'DATA GOES HERE'{{#unless @last}},{{/unless}}
           {{/each}}
@@ -120,7 +123,7 @@
               Authorization: '{{headerSecurity.type}} ' + process.env.{{headerSecurity.name}}{{/if}}
             },
             {{#is contentType 'application/json'}}
-            json: {
+            body: {
               {{#each bodyParameters}}
               {{this.name}}: 'DATA GOES HERE'{{#unless @last}},{{/unless}}
               {{/each}}

diff --git a/templates/request/put/put.handlebars b/templates/request/put/put.handlebars
@@ -7,6 +7,9 @@
       {{/validateResponse}}
       request({
         url: '{{pathify path pathParams}}',
+        {{#isJsonRepresentation contentType returnType}}
+        json: true,
+        {{/isJsonRepresentation}}
         {{#ifCond queryParameters queryApiKey}}
         qs: {
           {{#if queryApiKey}}{{queryApiKey.type}}: process.env.{{queryApiKey.name}}{{#if queryParameters}},
@@ -21,7 +24,7 @@
           Authorization: '{{headerSecurity.type}} ' + process.env.{{headerSecurity.name}}{{/if}}
         },
         {{#is contentType 'application/json'}}
-        {{#if request}}json: {{ request }}{{else}}json: {
+        {{#if request}}body: {{ request }}{{else}}body: {
         {{#each bodyParameters}}
           {{this.name}}: 'DATA GOES HERE'{{#unless @last}},{{/unless}}
         {{/each}}
@@ -120,7 +123,7 @@
               Authorization: '{{headerSecurity.type}} ' + process.env.{{headerSecurity.name}}{{/if}}
             },
             {{#is contentType 'application/json'}}
-            json: {
+            body: {
               {{#each bodyParameters}}
               {{this.name}}: 'DATA GOES HERE'{{#unless @last}},{{/unless}}
               {{/each}}

diff --git a/test/jsonProperty/compare/request/expect/no-property-user-test.js b/test/jsonProperty/compare/request/expect/no-property-user-test.js
@@ -0,0 +1,67 @@
+'use strict';
+var chai = require('chai');
+var request = require('request');
+var expect = chai.expect;
+
+describe('/user', function() {
+  describe('get', function() {
+    it('should respond with 200 OK', function(done) {
+      request({
+        url: 'https://api.uber.com/user',
+        json: true,
+        method: 'GET',
+        headers: {
+          'Content-Type': 'application/json'
+        }
+      },
+      function(error, res, body) {
+        if (error) return done(error);
+
+        expect(res.statusCode).to.equal(200);
+
+        expect(body).to.equal(null); // non-json response or no schema
+        done();
+      });
+    });
+
+    it('should respond with 400 NOT OK', function(done) {
+      request({
+        url: 'https://api.uber.com/user',
+        json: true,
+        method: 'GET',
+        headers: {
+          'Content-Type': 'application/json'
+        }
+      },
+      function(error, res, body) {
+        if (error) return done(error);
+
+        expect(res.statusCode).to.equal(400);
+
+        expect(body).to.equal(null); // non-json response or no schema
+        done();
+      });
+    });
+
+    it('should respond with 500 SERVER ERROR', function(done) {
+      request({
+        url: 'https://api.uber.com/user',
+        json: true,
+        method: 'GET',
+        headers: {
+          'Content-Type': 'application/json'
+        }
+      },
+      function(error, res, body) {
+        if (error) return done(error);
+
+        expect(res.statusCode).to.equal(500);
+
+        expect(body).to.equal(null); // non-json response or no schema
+        done();
+      });
+    });
+
+  });
+
+});
diff --git a/test/jsonProperty/compare/request/expect/non-json-user-test.js b/test/jsonProperty/compare/request/expect/non-json-user-test.js
@@ -0,0 +1,67 @@
+'use strict';
+var chai = require('chai');
+var request = require('request');
+var expect = chai.expect;
+
+describe('/user', function() {
+  describe('post', function() {
+    it('should respond with 200 OK', function(done) {
+      request({
+        url: 'https://api.uber.com/user',
+        method: 'POST',
+        headers: {
+          'Content-Type': 'application/xml'
+        },
+        body: 'XML STRING GOES HERE'
+      },
+      function(error, res, body) {
+        if (error) return done(error);
+
+        expect(res.statusCode).to.equal(200);
+
+        expect(body).to.equal(null); // non-json response or no schema
+        done();
+      });
+    });
+
+    it('should respond with 400 NOT OK', function(done) {
+      request({
+        url: 'https://api.uber.com/user',
+        method: 'POST',
+        headers: {
+          'Content-Type': 'application/xml'
+        },
+        body: 'XML STRING GOES HERE'
+      },
+      function(error, res, body) {
+        if (error) return done(error);
+
+        expect(res.statusCode).to.equal(400);
+
+        expect(body).to.equal(null); // non-json response or no schema
+        done();
+      });
+    });
+
+    it('should respond with 500 SERVER ERROR', function(done) {
+      request({
+        url: 'https://api.uber.com/user',
+        method: 'POST',
+        headers: {
+          'Content-Type': 'application/xml'
+        },
+        body: 'XML STRING GOES HERE'
+      },
+      function(error, res, body) {
+        if (error) return done(error);
+
+        expect(res.statusCode).to.equal(500);
+
+        expect(body).to.equal(null); // non-json response or no schema
+        done();
+      });
+    });
+
+  });
+
+});
diff --git a/test/jsonProperty/swagger-no-type.json b/test/jsonProperty/swagger-no-type.json
@@ -0,0 +1,28 @@
+{
+    "swagger": "2.0",
+    "info": {
+        "version": "0.0.0",
+        "title": "Simple API"
+    },
+    "host": "api.uber.com",
+    "schemes": [
+        "https"
+    ],
+    "paths": {
+        "/user": {
+            "get": {
+                "responses": {
+                    "200": {
+                        "description": "OK"
+                    },
+                    "400": {
+                        "description": "NOT OK"
+                    },
+                    "500": {
+                        "description": "SERVER ERROR"
+                    }
+                }
+            }
+        }
+    }
+}
diff --git a/test/jsonProperty/swagger-non-json.json b/test/jsonProperty/swagger-non-json.json
@@ -0,0 +1,34 @@
+{
+    "swagger": "2.0",
+    "info": {
+        "version": "0.0.0",
+        "title": "Simple API"
+    },
+    "host": "api.uber.com",
+    "schemes": [
+        "https"
+    ],
+    "paths": {
+        "/user": {
+            "post": {
+                "consumes": [
+                    "application/xml"
+                ],
+                "produces": [
+                    "application/xml"
+                ],
+                "responses": {
+                    "200": {
+                        "description": "OK"
+                    },
+                    "400": {
+                        "description": "NOT OK"
+                    },
+                    "500": {
+                        "description": "SERVER ERROR"
+                    }
+                }
+            }
+        }
+    }
+}