fix: stack overflow for type tagging during codegen

looker-open-source · Oct 5, 2021 · db568bc · db568bc
1 parent 2ca950e
commit db568bc
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 79 deletions.
diff --git a/packages/sdk-codegen/src/sdkModels.spec.ts b/packages/sdk-codegen/src/sdkModels.spec.ts
@@ -24,8 +24,9 @@
 
  */
 
+import { readFileSync } from 'fs'
 import type * as OAS from 'openapi3-ts'
-import { TestConfig } from './testUtils'
+import { rootFile, TestConfig } from './testUtils'
 import type {
   IEnumType,
   IMethod,
@@ -68,6 +69,13 @@ describe('sdkModels', () => {
     expect(actual).toEqual(expected)
   }
 
+  it('API 4 models', () => {
+    const api4 = readFileSync(rootFile('spec/Looker.4.0.oas.json'), 'utf-8')
+    const api = ApiModel.fromString(api4)
+    const actual = Object.keys(api.typeTags)
+    expect(actual).toHaveLength(29)
+  })
+
   describe('ordering', () => {
     it('has types in sorted order', () => {
       checkSorted(apiTestModel.types)

diff --git a/packages/sdk-codegen/src/sdkModels.ts b/packages/sdk-codegen/src/sdkModels.ts
@@ -231,7 +231,7 @@ export type EnumValueType = string | number
 /**
  * Returns sorted string array for IKeylist type
  * @param list Set of values
- * @returns {string[]} sorted string array of keys
+ * @returns sorted string array of keys
  */
 export const keyValues = (list: KeyList): string[] => {
   if (!list) return []
@@ -241,8 +241,8 @@ export const keyValues = (list: KeyList): string[] => {
 /**
  * Optionally quote a string if quotes are required
  * @param value to convert to string and optionally quote
- * @param {string} quoteChar defaults to "'"
- * @returns {string} the quoted or unquoted value
+ * @param quoteChar defaults to "'"
+ * @returns the quoted or unquoted value
  */
 export const mayQuote = (value: any, quoteChar = `'`): string => {
   const str = value.toString()
@@ -252,9 +252,9 @@ export const mayQuote = (value: any, quoteChar = `'`): string => {
 
 /**
  * Resolve a list of method keys into an IMethod[] in alphabetical order by name
- * @param {IApiModel} api model to use
- * @param {KeyList} refs references to models
- * @returns {IMethod[]} Populated method list. Anything not matched is skipped
+ * @param api model to use
+ * @param refs references to models
+ * @returns Populated method list. Anything not matched is skipped
  */
 export const methodRefs = (api: IApiModel, refs: KeyList): IMethod[] => {
   const keys = keyValues(refs)
@@ -267,28 +267,11 @@ export const methodRefs = (api: IApiModel, refs: KeyList): IMethod[] => {
   return result
 }
 
-/**
- * Resolves first method ref it can find
- * @param api parsed spec
- * @param type tree to walk
- */
-export const firstMethodRef = (api: ApiModel, type: IType): IMethod => {
-  let method = methodRefs(api, type.methodRefs)[0]
-  if (!method) {
-    const parents = typeRefs(api, type.parentTypes)
-    for (const parent of parents) {
-      method = firstMethodRef(api, parent)
-      if (method) break
-    }
-  }
-  return method
-}
-
 /**
  * Resolve a list of method keys into an IType[] in alphabetical order by name
- * @param {IApiModel} api model to use
- * @param {KeyList} refs references to models
- * @returns {IMethod[]} Populated method list. Anything not matched is skipped
+ * @param api model to use
+ * @param refs references to models
+ * @returns Populated method list. Anything not matched is skipped
  */
 export const typeRefs = (api: IApiModel, refs: KeyList): IType[] => {
   const keys = keyValues(refs)
@@ -302,6 +285,32 @@ export const typeRefs = (api: IApiModel, refs: KeyList): IType[] => {
   return result
 }
 
+/**
+ * Resolves first method ref it can find
+ * @param api parsed spec
+ * @param type tree to walk
+ * @param stack call stack to prevent infinite recursion
+ */
+export const firstMethodRef = (
+  api: ApiModel,
+  type: IType,
+  stack: KeyList = new Set<string>()
+): IMethod => {
+  stack.add(type.name)
+
+  let method = methodRefs(api, type.methodRefs)[0]
+  if (!method) {
+    const parents = typeRefs(api, type.parentTypes)
+    for (const parent of parents) {
+      if (!stack.has(parent.name)) {
+        method = firstMethodRef(api, parent, stack)
+      }
+      if (method) break
+    }
+  }
+  return method
+}
+
 /**
  * Returns the first method (if any) that uses the reference type for updating
  * @param api parsed spec

diff --git a/packages/sdk-codegen/src/typescript.gen.spec.ts b/packages/sdk-codegen/src/typescript.gen.spec.ts
@@ -190,7 +190,8 @@ describe('typescript generator', () => {
       const inputs = { look_id: 17, fields }
       const method = apiTestModel.methods.look
       const actual = gen.makeTheCall(method, inputs)
-      const expected = `let response = await sdk.ok(sdk.look(17, '${fields}'))`
+      const expected = `let response = await sdk.ok(sdk.look(
+  17, '${fields}'))`
       expect(actual).toEqual(expected)
     })
 
@@ -207,34 +208,28 @@ describe('typescript generator', () => {
       const inputs = { look_id: 17, body, fields }
       const method = apiTestModel.methods.update_look
       const actual = gen.makeTheCall(method, inputs)
-      const expected = `let response = await sdk.ok(
-  sdk.update_look(
-    17,
-    {
-      title: 'test title',
-      description: 'gen test',
-      query: {
-        model: 'the_look',
-        view: 'users',
-        total: true,
-      },
-    },
-    'id,user_id,title,description'
-  )
-)`
+      const expected = `let response = await sdk.ok(sdk.update_look(
+  17, {
+    title: 'test title',
+    description: 'gen test',
+    query: {
+      model: 'the_look',
+      view: 'users',
+      total: true
+    }
+  }, 'id,user_id,title,description'))`
       expect(actual).toEqual(expected)
     })
 
     it('assigns request params', () => {
       const inputs = { look_id: 17, result_format: 'png' }
       const method = apiTestModel.methods.run_look
       const actual = gen.makeTheCall(method, inputs)
-      const expected = `let response = await sdk.ok(
-  sdk.run_look({
+      const expected = `let response = await sdk.ok(sdk.run_look(
+  {
     look_id: 17,
-    result_format: 'png',
-  })
-)`
+    result_format: 'png'
+  }))`
       expect(actual).toEqual(expected)
     })
 
@@ -247,14 +242,13 @@ describe('typescript generator', () => {
       }
       const method = apiTestModel.methods.create_query_task
       const actual = gen.makeTheCall(method, inputs)
-      const expected = `let response = await sdk.ok(
-  sdk.create_query_task({
+      const expected = `let response = await sdk.ok(sdk.create_query_task(
+  {
     body: {
       query_id: 1,
-      result_format: ResultFormat.csv,
-    },
-  })
-)`
+      result_format: ResultFormat.csv
+    }
+  }))`
       expect(actual).toEqual(expected)
     })
 
@@ -264,11 +258,10 @@ describe('typescript generator', () => {
       }
       const method = apiTestModel.methods.all_users
       const actual = gen.makeTheCall(method, inputs)
-      const expected = `let response = await sdk.ok(
-  sdk.all_users({
-    ids: new DelimArray<number>([1, 2, 3]),
-  })
-)`
+      const expected = `let response = await sdk.ok(sdk.all_users(
+  {
+    ids: new DelimArray<number>([1,2,3])
+  }))`
       expect(actual).toEqual(expected)
     })
 
@@ -302,37 +295,40 @@ describe('typescript generator', () => {
       const inputs = { body, fields }
       const method = apiTestModel.methods.create_merge_query
       const actual = gen.makeTheCall(method, inputs)
-      const expected = `let response = await sdk.ok(
-  sdk.create_merge_query({
+      const expected = `let response = await sdk.ok(sdk.create_merge_query(
+  {
     body: {
-      pivots: ['one', 'two', 'three'],
+      pivots: [
+        'one',
+        'two',
+        'three'
+      ],
       sorts: ['a'],
       source_queries: [
         {
           merge_fields: [
             {
               field_name: 'merge_1',
-              source_field_name: 'source_1',
-            },
+              source_field_name: 'source_1'
+            }
           ],
           name: 'first query',
-          query_id: 1,
+          query_id: 1
         },
         {
           merge_fields: [
             {
               field_name: 'merge_2',
-              source_field_name: 'source_2',
-            },
+              source_field_name: 'source_2'
+            }
           ],
           name: 'second query',
-          query_id: 2,
-        },
-      ],
+          query_id: 2
+        }
+      ]
     },
-    fields: 'id,user_id,title,description',
-  })
-)`
+    fields: 'id,user_id,title,description'
+  }))`
       expect(actual).toEqual(expected)
     })
 
@@ -344,16 +340,15 @@ describe('typescript generator', () => {
       }
       const inputs = { body: query }
       const method = apiTestModel.methods.create_sql_query
-      const expected = `let response = await sdk.ok(
-  sdk.create_sql_query({
+      const expected = `let response = await sdk.ok(sdk.create_sql_query(
+  {
     connection_name: 'looker',
     model_name: 'the_look',
     vis_config: {
       first: 1,
-      second: 'two',
-    },
-  })
-)`
+      second: 'two'
+    }
+  }))`
       const actual = gen.makeTheCall(method, inputs)
       expect(actual).toEqual(expected)
     })