More code improvements

Signed-off-by: Alan Cha <[email protected]>

Author: Alan-Cha

packages/openapi-to-graphql/lib/oas_3_tools.js

@@ -453,11 +453,6 @@ export function getSchemaType(
     return schema.type
   }
 
-  // CASE: nullable - default to string
-  if (typeof schema.nullable !== 'undefined') {
-    return 'string'
-  }
-
   return null
 }
 

packages/openapi-to-graphql/lib/oas_3_tools.js.map

@@ -487,7 +487,17 @@ export function createDataDef(
 
       const def: DataDefinition = {
         preferredName,
+
+        /**
+         * Note that schema may contain $ref or schema composition (e.g. allOf)
+         *
+         * TODO: the schema is used in getSchemaIndex, which allows us to check
+         * whether a dataDef has already been created for that particular
+         * schema and name pair. The look up should resolve references but
+         * currently, it does not.
+         */
         schema,
+
         type,
         subDefinitions: undefined,
         links: saneLinks,
@@ -572,16 +582,23 @@ function getSchemaIndex(
   schema: SchemaObject,
   dataDefs: DataDefinition[]
 ): number {
-  let index = -1
-  for (let def of dataDefs) {
-    index++
+  /**
+   * TODO: instead of iterating through the whole list every time, create a
+   * hashing function and store all of the DataDefinitions in a hashmap.
+   */
+  for (let index = 0; index < dataDefs.length; index++) {
+    const def = dataDefs[index]
 
+    /**
+     * TODO: deepEquals is not sufficient. We also need to resolve references.
+     * However, deepEquals should work for vast majority of cases.
+     */
     if (preferredName === def.preferredName && deepEqual(schema, def.schema)) {
       return index
     }
   }
 
-  // If the schema could not be found in the master list
+  // The schema could not be found in the master list
   return -1
 }
 

packages/openapi-to-graphql/lib/preprocessor.js

@@ -15,10 +15,9 @@ import {
   SchemaObject,
   ParameterObject,
   ReferenceObject,
-  LinkObject,
-  LinksObject
+  LinkObject
 } from './types/oas3'
-import { Args, Field, GraphQLType } from './types/graphql'
+import { Args, GraphQLType } from './types/graphql'
 import {
   GraphQLScalarType,
   GraphQLObjectType,
@@ -67,20 +66,20 @@ type CreateOrReuseOtParams = {
   data: PreprocessingData
 }
 
-type ReuseOrCreateListParams = {
+type CreateOrReuseListParams = {
   def: DataDefinition
   operation?: Operation
   iteration: number
   isInputObjectType: boolean
   data: PreprocessingData
 }
 
-type ReuseOrCreateEnum = {
+type CreateOrReuseEnumParams = {
   def: DataDefinition
   data: PreprocessingData
 }
 
-type ReuseOrCreateScalar = {
+type CreateOrReuseScalarParams = {
   def: DataDefinition
   data: PreprocessingData
 }
@@ -134,7 +133,7 @@ export function getGraphQLType({
 
     // CASE: array - create ArrayType
   } else if (type === 'array') {
-    return reuseOrCreateList({
+    return createOrReuseList({
       def,
       operation,
       data,
@@ -144,7 +143,7 @@ export function getGraphQLType({
 
     // CASE: enum - create EnumType
   } else if (type === 'enum') {
-    return reuseOrCreateEnum({
+    return createOrReuseEnum({
       def,
       data
     })
@@ -229,6 +228,7 @@ function createOrReuseOt({
   if (
     typeof def.schema.properties === 'undefined' &&
     typeof def.schema.allOf === 'undefined' // allOf can provide all the properties
+    // TODO: Add oneOf and anyOf
   ) {
     handleWarning({
       typeKey: 'OBJECT_MISSING_PROPERTIES',
@@ -305,13 +305,13 @@ function createOrReuseOt({
 /**
  * Returns an existing List or creates a new one, and stores it in data
  */
-function reuseOrCreateList({
+function createOrReuseList({
   def,
   operation,
   iteration,
   isInputObjectType,
   data
-}: ReuseOrCreateListParams): GraphQLList<any> {
+}: CreateOrReuseListParams): GraphQLList<any> {
   const name = isInputObjectType ? def.iotName : def.otName
 
   // Try to reuse existing Object Type
@@ -359,15 +359,23 @@ function reuseOrCreateList({
 }
 
 /**
- * Returns an existing Enum Type or creates a new one, and stores it in data
+ * Returns an existing enum type or creates a new one, and stores it in data
  */
-function reuseOrCreateEnum({ def, data }: ReuseOrCreateEnum): GraphQLEnumType {
-  // Try to reuse existing Enum Type
+function createOrReuseEnum({
+  def,
+  data
+}: CreateOrReuseEnumParams): GraphQLEnumType {
+  /**
+   * Try to reuse existing enum type
+   *
+   * Enum types do not have an input variant so only check def.ot
+   */
   if (def.ot && typeof def.ot !== 'undefined') {
     translationLog(`Reuse GraphQLEnumType '${def.otName}'`)
     return def.ot as GraphQLEnumType
   } else {
     translationLog(`Create GraphQLEnumType '${def.otName}'`)
+
     const values = {}
     def.schema.enum.forEach(e => {
       // Force enum values to string
@@ -381,14 +389,18 @@ function reuseOrCreateEnum({ def, data }: ReuseOrCreateEnum): GraphQLEnumType {
       name: def.otName,
       values
     })
+
     return def.ot
   }
 }
 
 /**
  * Returns the GraphQL scalar type matching the given JSON schema type
  */
-function getScalarType({ def, data }: ReuseOrCreateScalar): GraphQLScalarType {
+function getScalarType({
+  def,
+  data
+}: CreateOrReuseScalarParams): GraphQLScalarType {
   const type = def.type
 
   switch (type) {
@@ -411,9 +423,7 @@ function getScalarType({ def, data }: ReuseOrCreateScalar): GraphQLScalarType {
       def.ot = GraphQLJSON
       break
     default:
-      // If the type is not known, try to stringify
-      def.ot = GraphQLString
-      break
+      throw new Error(`Cannot process schema type '${def.type}'.`)
   }
 
   return def.ot as GraphQLScalarType