[AB-290]-Extracting-type-information from OAS #834

AyushShri · 2024-11-28T13:50:39Z

The changes aims to extract type information along with collection generation from OAS

Sample Type Information Structure

"extractedTypes": { "get/pets": { "request": { "headers": "[]", "pathParam": "[]", "queryParam": "[\n {\n \"keyName\": \"limit1\",\n \"properties\": {\n \"type\": \"integer\",\n \"default\": \"<integer>\",\n \"required\": false,\n \"deprecated\": false\n }\n }\n]" }, "response": { "default": { "body": "{\n \"type\": \"object\",\n \"properties\": {\n \"code\": {\n \"type\": \"integer\",\n \"deprecated\": false\n },\n \"message\": {\n \"type\": \"string\",\n \"deprecated\": false\n }\n },\n \"required\": [\n \"code\",\n \"message\"\n ]\n}", "headers": "[]" } } } }

libV2/index.js

lib/schemapack.js

libV2/schemaUtils.js

barshan23 · 2024-12-30T17:15:19Z

libV2/index.js

@@ -91,16 +92,19 @@ module.exports = {
          // generate the request form the node
          let request = {},
            collectionVariables = [],
-            requestObject = {};
+            requestObject = {},
+            extractedTypesObject = {};


P2: Can we name the variable typesObject instead of extracted types object ?

barshan23 · 2024-12-30T17:20:34Z

libV2/schemaUtils.js

+        if (requiredProperties.has(key) || parentRequired.has(key)) {
+          schemaDetails.required.push(key);
+        }
+        if (prop.$ref) {


Where are we dereferencing this ? I can see we're calling processSchema again in this case, but this function doesn't seems to handle ref anywhere else. Am I missing something here ?

my bad, this is not required as the $ref would be already resolved by resolvedSchema.

libV2/schemaUtils.js

barshan23 · 2025-01-06T07:51:44Z

libV2/schemaUtils.js

+        };
+      }
+      propertyDetails = { keyName, properties };
+      if (keyName && param.schema && param.schema.type) {


Combine this if check with the above one.

libV2/schemaUtils.js

barshan23 · 2025-01-07T08:17:09Z

libV2/schemaUtils.js

+          minLength: prop.minLength !== undefined ? prop.minLength : undefined,
+          maxLength: prop.maxLength !== undefined ? prop.maxLength : undefined,
+          minimum: prop.minimum !== undefined ? prop.minimum : undefined,
+          maximum: prop.maximum !== undefined ? prop.maximum : undefined,
          pattern: prop.pattern || undefined,
          example: prop.example || undefined,


This should also check for undefined since example can also be false or 0 or some other falsy value

libV2/schemaUtils.js

barshan23 · 2025-01-07T10:51:11Z

libV2/schemaUtils.js

+    resolvedResponseBodyResult = resolveBodyData(
+      context, responseContent[bodyType], bodyType, true, code, requestBodyExamples);
+    allBodyData = resolvedResponseBodyResult.generatedBody;
+    resolvedResponseBodyTypes = resolvedResponseBodyResult ?


This check is not needed

barshan23 · 2025-01-07T11:45:19Z

libV2/schemaUtils.js

+      requestIdentifier,
+      requestBlock,
+      typesObject = {};
+    if (requestBody && requestBody.resolvedSchemaTypeObject) {


This is not needed

barshan23 · 2025-01-07T11:45:32Z

libV2/schemaUtils.js

      { pathVariables, collectionVariables } = filterCollectionAndPathVariables(url, pathParams),
      requestBody = resolveRequestBodyForPostmanRequest(context, operationItem[method]),
+      bodyTypes = requestBody && requestBody.resolvedSchemaTypeObject,


bodyTypes -> requestBodyTypes

barshan23 · 2025-01-07T11:47:49Z

libV2/schemaUtils.js

+      } = resolveResponseForPostmanRequest(context, operationItem[method], request);
+
+    requestIdentifier = method + path;
+    requestBlock = { request: unifiedRequestTypes, response: unifiedResponseTypes };


This is not needed, we can add this directly rather than using another variable

barshan23 · 2025-01-07T11:48:07Z

libV2/schemaUtils.js

+      { alwaysInheritAuthentication } = context.computedOptions,
+      requestIdentifier,
+      requestBlock,
+      typesObject = {};


This can be called requestTypesObject

barshan23 · 2025-01-07T11:50:15Z

libV2/schemaUtils.js

@@ -2396,7 +2645,22 @@ module.exports = {
      auth: alwaysInheritAuthentication ? undefined : authHelper
    };

-    const { responses, acceptHeader } = resolveResponseForPostmanRequest(context, operationItem[method], request);
+    const unifiedRequestTypes = {


requestTypes

barshan23 · 2025-01-07T13:18:07Z

libV2/schemaUtils.js

+ * Processes and resolves types from Nested JSON schema structure.
+ *
+ * @param {Object} resolvedSchema - The resolved JSON schema to process.
+ * @param {Set<string>} [parentRequired=new Set()] - A set of parent-required property keys to inherit.


This needs to be removed

barshan23 · 2025-01-07T13:23:15Z

libV2/schemaUtils.js

+  processSchema = (resolvedSchema) => {
+    if (resolvedSchema.type === 'object' && resolvedSchema.properties) {
+      const schemaDetails = {
+          type: resolvedSchema.type || 'unknown',


The unknown needs to be removed

barshan23 · 2025-01-07T13:24:25Z

libV2/schemaUtils.js

+    }
+    else if (resolvedSchema.type === 'array' && resolvedSchema.items) {
+      const arrayDetails = {
+        type: resolvedSchema.type || 'unknown',


Remove the unknown from here

barshan23 · 2025-01-07T13:25:09Z

libV2/schemaUtils.js

+          schemaDetails.required.push(key);
+        }
+        if (prop.properties) {
+          let res = processSchema(prop);


Please use descriptive variable names

barshan23 · 2025-01-07T13:33:47Z

libV2/schemaUtils.js

+      return arrayDetails;
+    }
+    return {
+      type: resolvedSchema.type || 'unknown'


unknown needs to be removed

barshan23 · 2025-01-07T13:54:30Z

libV2/schemaUtils.js

@@ -1630,13 +1727,23 @@ let QUERYPARAM = 'query',
        mode: 'formdata',
        formdata: formDataParams
      },
-      resolvedBody;
+      resolvedBody,
+      result,


Use more descriptive name

barshan23 · 2025-01-07T13:54:54Z

libV2/schemaUtils.js

-    resolvedBody = resolveBodyData(context, requestBodyContent.schema)[0];
+    result = resolveBodyData(context, requestBodyContent.schema);
+    resolvedBody =
+      result && Array.isArray(result.generatedBody) && result.generatedBody.length > 0 ?


These condition checks needs to be simplified, refer the above comment

libV2/schemaUtils.js

barshan23 · 2025-01-07T14:06:58Z

libV2/schemaUtils.js

+          minLength: schema.minLength !== undefined ? schema.minLength : undefined,
+          maxLength: schema.maxLength !== undefined ? schema.maxLength : undefined,
+          minimum: schema.minimum !== undefined ? schema.minimum : undefined,
+          maximum: schema.maximum !== undefined ? schema.maximum : undefined,


These checks are unnecessary and can be removed

barshan23 · 2025-01-07T14:07:16Z

libV2/schemaUtils.js

+
+      if (headerData && headerData.name && headerData.schema && headerData.schema.type) {
+        const { name, schema } = headerData;
+        keyName = name;


keyName variable is not needed

Let's remove this please ^

test/unit/convertV2WithTypes.test.js

barshan23 · 2025-01-08T04:56:51Z

libV2/schemaUtils.js

+    resolvedBody =
+
+    resolvedBodyResult && Array.isArray(resolvedBodyResult.generatedBody) &&
+     resolvedBodyResult.generatedBody[0] ?


This check needs to be simplified 😞

This was simplified based on your comment

I meant we don't need the ternary operator here 🤦

barshan23 · 2025-01-08T04:57:18Z

libV2/schemaUtils.js

+      undefined;
+
+    resolvedSchemaTypeObject = resolvedBodyResult &&
+     resolvedBodyResult.resolvedSchemaType ? resolvedBodyResult.resolvedSchemaType : undefined;


Same as above

barshan23 · 2025-01-08T04:57:45Z

libV2/schemaUtils.js

-    resolvedBody = resolveBodyData(context, requestBodyContent.schema)[0];
+    resolvedBodyResult = resolveBodyData(context, requestBodyContent.schema);
+    resolvedBody =
+    resolvedBodyResult && Array.isArray(resolvedBodyResult.generatedBody) &&


Same as above comment, please take care of these in all the places where you have added such checks

barshan23 · 2025-01-08T04:58:41Z

libV2/schemaUtils.js

+      resolvedBodyResult = resolveBodyData(context, requestContent[bodyType], bodyType);
+      resolvedBody =
+      resolvedBodyResult && Array.isArray(resolvedBodyResult.generatedBody) &&
+       resolvedBodyResult.generatedBody[0] ?
+        resolvedBodyResult.generatedBody[0] :
+        undefined;
+
+      resolvedSchemaTypeObject =
+        resolvedBodyResult && resolvedBodyResult.resolvedSchemaType ?
+          resolvedBodyResult.resolvedSchemaType :
+          undefined;
+


Same as above

barshan23 · 2025-01-08T05:00:26Z

libV2/schemaUtils.js

  },

  resolvePathParamsForPostmanRequest = (context, operationItem, method) => {
    const params = resolvePathItemParams(context, operationItem[method].parameters, operationItem.parameters),
      pmParams = [];
+    let pathParamTypes = [];


This can be combined with the above const declaration

barshan23 · 2025-01-08T05:04:48Z

libV2/schemaUtils.js

+      resolvedExamplesObject = resolvedExamples[0] && resolvedExamples[0].resolvedResponseBodyTypes;
+      responseBodyHeaderObj =
+        {
+          body: JSON.stringify(resolvedExamplesObject, null, 2),


Q: Do we really need to stringify here ?

yes, we are not doing it again

I meant what is the use of this stringify ? keeping it as JS object also works right ?

yes, but i wanted to export the object in such a way that it can be used by collection0definition as it is. so stringified it. and changing it right now might be risky as all the tests would start to break.

Let's remove all these stringify calls since they aren't necessary when we're picking up the E2E test ticket.

https://postmanlabs.atlassian.net/browse/AB-293?focusedCommentId=823082 -> Tracking it here

barshan23 · 2025-01-08T05:06:00Z

test/unit/convertV2WithTypes.test.js

+// Disabling max Length for better visibility of the expectedExtractedTypes
+
+/* eslint-disable one-var */
+// Disabling as we want the checks to run in order of their declaration


This doesn't explain / mean anything when I'm reading this 😢

barshan23 · 2025-01-08T05:06:52Z

test/unit/convertV2WithTypes.test.js

+  };
+
+
+describe('convertV2WithTypes should generate collection confirming to collection schema', function() {


typo confirming -> conforming

barshan23 · 2025-01-08T05:10:10Z

test/unit/convertV2WithTypes.test.js

+    );
+  });
+
+  it('should validate the schema' + testSpec1, function() {


What does validate the schema mean here ? The function being tested does not validate the schema right ? 🤔

barshan23 · 2025-01-08T05:10:36Z

test/unit/convertV2WithTypes.test.js

+
+      const element = Object.values(conversionResult.extractedTypes)[0];
+      const { response } = element;
+      // Get the schema from the response


This comment is unnecessary

barshan23 · 2025-01-08T05:16:21Z

@AyushShri Types are not being generated for responses when the response code is defined as default in OAS. Can you please look into this ?

Screen.Recording.2025-01-08.at.11.23.18.AM.mov

AB-290-WIP-extracting-type-information

cb4599a

jatin3893 added the specification-collection-sync API Builder Initiative label Dec 4, 2024

AyushShri added 9 commits December 6, 2024 03:24

AB-289-flag-fixes

aa645c1

AB-289-added tests and fixes

b54b8fb

AB-289-downgraded-uuid-dep

abaf386

AB-289-added-method-path

d8d9367

AB-289-added-code-reverted-uuid

7217383

AB-289-removed-undefined-props

5ec3f70

AB-289-optimised-structure-and fixed-test-cases

fdc0ca2

AB-289-fixed required and depricated param

9b30f47

AB-289-fixed-format and default param

d226967

AyushShri changed the title ~~AB-290-WIP-extracting-type-information~~ [AB-290]-Extracting-type-information from OAS Dec 24, 2024

AyushShri requested a review from barshan23 December 24, 2024 06:04

barshan23 requested changes Dec 30, 2024

View reviewed changes

AB-289-resolved-comments

8ae2fa5

barshan23 reviewed Jan 6, 2025

View reviewed changes

libV2/schemaUtils.js Outdated Show resolved Hide resolved

barshan23 reviewed Jan 6, 2025

View reviewed changes

libV2/schemaUtils.js Outdated Show resolved Hide resolved

barshan23 requested changes Jan 6, 2025

View reviewed changes

AyushShri added 3 commits January 7, 2025 02:00

AB-289-removed-context-schema-store-and-resolved-comments

6cffffb

AB-289-resolved-bug

1063175

AB-289-resolved-all-bugs

e7abec2

AyushShri requested a review from barshan23 January 7, 2025 08:17

barshan23 reviewed Jan 7, 2025

View reviewed changes

AB-289-resolved-all-comments

c4a9591

barshan23 reviewed Jan 7, 2025

View reviewed changes

AB-289-refactored-code

a8de5ff

barshan23 reviewed Jan 7, 2025

View reviewed changes

test/unit/convertV2WithTypes.test.js Show resolved Hide resolved

AB-289-refactored-code-and-added-test-case

6f30294

AyushShri requested a review from barshan23 January 7, 2025 16:20

barshan23 reviewed Jan 8, 2025

View reviewed changes

AyushShri added 2 commits January 8, 2025 17:02

AB-289-refactored-code

dc4a48a

AB-289-resolved-comments

415f186

barshan23 approved these changes Jan 8, 2025

View reviewed changes

AyushShri merged commit 20aa037 into develop Jan 9, 2025
7 checks passed

barshan23 deleted the AB-289-feature/type-extraction branch January 9, 2025 07:00

		};


		describe('convertV2WithTypes should generate collection confirming to collection schema', function() {

[AB-290]-Extracting-type-information from OAS #834

[AB-290]-Extracting-type-information from OAS #834

Conversation

AyushShri commented Nov 28, 2024 • 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

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

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

barshan23 commented Jan 8, 2025 • edited Loading

AyushShri commented Nov 28, 2024 •

edited

Loading

barshan23 commented Jan 8, 2025 •

edited

Loading