using example values instead of faking schema #125

Dhroov7 · 2019-11-06T07:47:47Z

Solving #108

abhijitkane · 2019-11-08T05:18:31Z

@Dhroov7 Is useExamplesValue an option you're exposing? If so, there needs to be a test for that too.

…openapi-to-postman into feature/respecting_examples_in_request_body

…ostman into feature/respecting_examples_in_request_body

abhijitkane · 2019-11-19T05:35:41Z

lib/util.js

  var prop,
    schema = deref.resolveRefs(oldSchema, bodyTypeOption, components);

+  if (resolveTo === 'schema') {


check the copy for the jsdoc descriptions. tells to is incorrect. Every enum option should:

Describe what it does

Explain each of the options

Mention the default

@param {string} resolveTo The desired JSON-generation mechanism (schema: prefer using the JSONschema to generate a fake object, example: use specified examples as-is). Default: schema

Even the desc for bodyTypeOption doesn't read well. Specifies whether the schema being faked is from a request or response.. Also, explain how this affects the output.

abhijitkane · 2019-11-19T05:36:34Z

lib/util.js

+        if (requestType === REQUEST_TYPE.ROOT) {
+          resolveTo = this.options.requestParametersResolution;
+        }
+        else if (requestType === REQUEST_TYPE.EXAMPLE) {


else?
is there a default? what happens if I send something other than root/example to the function?

The default value of resolveTo is an empty string which means if I send something other than root/example then the JSON-schema-faker will fake the schema properties instead of using examples.

the default value of resolveTo should be schema or example. Why do we have a blank string?

abhijitkane · 2019-11-19T05:41:15Z

lib/util.js


-    if (bodyObj.example) {
+    if (bodyObj.example && (this.options.requestParametersResolution === 'example' ||


Can this be simplified a bit? This is in direct violation of item #5 in the code section of our internal guidelines

lib/util.js

abhijitkane · 2019-11-19T05:44:04Z

lib/util.js

+          });
+        }
+        catch (e) {
+          thisOriginalRequest.url.queryParam = {};


what exceptions do you expect?

abhijitkane · 2019-11-19T05:44:44Z

lib/util.js

+          thisOriginalRequest.url += '?';
+          _.forEach(reqParams.query, (queryParam) => {
+            this.convertToPmQueryParameters(queryParam, REQUEST_TYPE.EXAMPLE).forEach((pmParam) => {
+              thisOriginalRequest.url += pmParam.key + '=' + pmParam.value + '&';


won't this lead to an extra & in the end? prefer _.map + _.join

abhijitkane · 2019-11-19T05:47:30Z

test/unit/base.test.js

@@ -161,6 +163,43 @@ describe('CONVERT FUNCTION TESTS ', function() {
          done();
        });
    });
+    describe('[Github #108]- Use example values instead of faking schema', function() {


Violation of #4 in the basic section of our internal code guidelines

…params

abhijitkane · 2019-11-20T14:00:05Z

lib/util.js


    if (header.hasOwnProperty('schema')) {
      if (!this.options.schemaFaker) {
        fakeData = '';
      }
      else {
-        fakeData = safeSchemaFaker(header.schema || {}, this.components, SCHEMA_FORMATS.DEFAULT);
+        fakeData = safeSchemaFaker(header.schema || {}, resolveTo, '', this.components, SCHEMA_FORMATS.DEFAULT);


the request/response-ness should be passed here
parameterSource, maybe?

abhijitkane · 2019-11-20T14:09:04Z

lib/util.js

@@ -1109,7 +1167,7 @@ module.exports = {
                encoding[key].headers[key] = getRefObject(encoding[key].headers[key].$ref);
              }
              encoding[key].headers[key].name = key;
-              formHeaders.push(this.convertToPmHeader(encoding[key].headers[key]));
+              formHeaders.push(this.convertToPmHeader(encoding[key].headers[key], REQUEST_TYPE.ROOT));


add a comment explaining the hard-coded value

abhijitkane · 2019-11-20T14:24:00Z

lib/util.js

+            this.convertToPmQueryParameters(queryParam, REQUEST_TYPE.EXAMPLE).forEach((pmParam) => {
+              requestQueryParams.push(pmParam.key + '=' + pmParam.value);
+            });
+            thisOriginalRequest.url += requestQueryParams.join('&');


needs testing

abhijitkane · 2019-11-21T11:58:02Z

lib/util.js

+  * @param {*} reqParams openapi query params object
+  * @returns {*} array of all query params
+  */
+  setQueryParamsToUrl: function(reqParams) {


.example shouldn't be hard-coded here

incorrectly named

abhijitkane · 2019-11-21T12:03:31Z

lib/util.js

@@ -1475,15 +1541,27 @@ module.exports = {
          thisOriginalRequest.url = '';
        }
        try {
-          thisOriginalRequest.header = item.request.headers.toJSON();
+          thisOriginalRequest.url += '?';
+          requestQueryParams = this.setQueryParamsToUrl(reqParams);


extra var not needed

using example values instead of faking schema

1bd4c2b

Dhroov7 requested a review from abhijitkane November 6, 2019 07:47

Dhroov7 added 6 commits November 13, 2019 16:25

[WIP]: changing originalRequest in responses

ef8e475

Merge branch 'feature/add_options' of https://github.com/postmanlabs/…

8dbc901

…openapi-to-postman into feature/respecting_examples_in_request_body

added support for seperate queryparams and headers in originalRequest

e90ccab

Merge branch 'develop' of https://github.com/postmanlabs/openapi-to-p…

40a646e

…ostman into feature/respecting_examples_in_request_body

added more tests

bee0e8b

removed node version 6 from travis

ab5a616

abhijitkane reviewed Nov 19, 2019

View reviewed changes

lib/util.js Outdated Show resolved Hide resolved

abhijitkane reviewed Nov 19, 2019

View reviewed changes

lib/util.js Outdated

});

}

catch (e) {

thisOriginalRequest.url.queryParam = {};

Copy link

Member

abhijitkane Nov 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what exceptions do you expect?

abhijitkane reviewed Nov 19, 2019

View reviewed changes

Dhroov7 added 8 commits November 19, 2019 17:13

split up consitional statements and added comments

800837e

Merge branch 'develop' into feature/respecting_examples_in_request_body

8ab3155

changed jsDoc for resolveTo and bodyTypeOption, using join for query …

637bd13

…params

changed jsDoc for requestType

0dc96b0

added the default value of resolveTo to schema

4ffb468

added resolveTo default in one more plcae

f50f519

added a function for determining resolveTo example or schema

c83106b

added jsDoc for resolveToExampleorSchema function

aa5db73

abhijitkane reviewed Nov 20, 2019

View reviewed changes

Dhroov7 added 2 commits November 21, 2019 14:47

added a function setting query params to the url

0a5150e

put queryObject inside it

48a4d6f

abhijitkane reviewed Nov 21, 2019

View reviewed changes

changed the function name to convertToPmQueryArray

59160d5

abhijitkane merged commit 884ac9f into develop Nov 21, 2019

umeshp7 deleted the feature/respecting_examples_in_request_body branch January 22, 2020 09:05

snyk-bot mentioned this pull request Mar 15, 2020

[Snyk] Upgrade eslint-plugin-jsdoc from 3.8.0 to 3.15.1 #180

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

using example values instead of faking schema #125

using example values instead of faking schema #125

Dhroov7 commented Nov 6, 2019 •

edited

Loading

abhijitkane commented Nov 8, 2019

abhijitkane Nov 19, 2019 •

edited

Loading

abhijitkane Nov 19, 2019

Dhroov7 Nov 19, 2019

abhijitkane Nov 20, 2019

abhijitkane Nov 19, 2019

abhijitkane Nov 19, 2019

abhijitkane Nov 19, 2019

abhijitkane Nov 19, 2019

abhijitkane Nov 20, 2019

abhijitkane Nov 20, 2019

abhijitkane Nov 20, 2019

abhijitkane Nov 21, 2019

abhijitkane Nov 21, 2019

abhijitkane Nov 21, 2019


		if (bodyObj.example) {
		if (bodyObj.example && (this.options.requestParametersResolution === 'example' \|\|

using example values instead of faking schema #125

using example values instead of faking schema #125

Conversation

Dhroov7 commented Nov 6, 2019 • edited Loading

abhijitkane commented Nov 8, 2019

abhijitkane Nov 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dhroov7 commented Nov 6, 2019 •

edited

Loading

abhijitkane Nov 19, 2019 •

edited

Loading