Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Javascript client produces a wrong object for a string enum type that is used with $ref #4819

Open
knzm opened this issue Feb 17, 2017 · 39 comments

Comments

@knzm
Copy link

knzm commented Feb 17, 2017

Description

I got something like { '0': 'f', '1': 'i', '2': 's', '3': 'h' } for an enum type. It should be "fish".

Swagger-codegen version

2.2.2-SNAPSHOT (revision: bb81fc1)

Swagger declaration file content or url
swagger: "2.0"
info:
  version: 1.0.0
schemes:
  - http
paths:
  /pet:
    post:
      parameters:
      responses:
        '405':
          description: Invalid input

definitions:
  EnumArrays:
    type: object
    properties:
      array_enum:
        type: array
        items:
          type: string
          enum:
            - fish
            - crab
      array_enum_ref:
        type: array
        items:
          $ref: '#/definitions/StringEnumObject'
  StringEnumObject:
    type: string
    enum:
      - fish
      - crab
Command line used for generation
$ java -jar modules/swagger-codegen-cli/target/swagger-codegen-cli.jar generate -i test.yaml -l javascript -o output
Steps to reproduce
  1. Run codegen by the above command
  2. Cd into output directory, then run npm install
  3. Start a node interactive shell and type them:
$ node
> var SwaggerClient = require('./src')
> SwaggerClient.EnumArrays.constructFromObject({"array_enum": ["fish", "crab"]})
exports { array_enum: [ 'fish', 'crab' ] }
> SwaggerClient.EnumArrays.constructFromObject({"array_enum_ref": ["fish", "crab"]})
exports {
  array_enum_ref: 
   [ { '0': 'f', '1': 'i', '2': 's', '3': 'h' },
     { '0': 'c', '1': 'r', '2': 'a', '3': 'b' } ] }
Related issues

I did not find any known issues about it. However, there were a number of bugs related to enum types with Javascript client in the past.

#3639 [javascript client] constructFromObject calls undefined function constructFromObject on enums
#3416 [JavaScript] String Enum types produce invalid code
#2102 [JavaScript] Enums are not exported correctly

Suggest a Fix

I can fix the issue by modifying convertToType method defined in ApiClient.mustache like this:

--- a/modules/swagger-codegen/src/main/resources/Javascript/ApiClient.mustache
+++ b/modules/swagger-codegen/src/main/resources/Javascript/ApiClient.mustache
@@ -470,7 +470,7 @@
         if (type === Object) {
           // generic object, return directly
           return data;
-        } else if (typeof type === 'function') {
+        } else if (typeof type.constructFromObject === 'function') {
           // for model type like: User
           return type.constructFromObject(data);
         } else if (Array.isArray(type)) {

I think ApiClient.convertToType() can always call type.constructFromObject() for types that have constructFromObject method. But I'm not sure about its side effects.

@advance512
Copy link

I can confirm this. It is a major bug that basically prevents using arrays of enums.

  Language:
    type: string
    enum:
      - LANGUAGE_ar
      - LANGUAGE_bg
      - LANGUAGE_ca
      - LANGUAGE_zh_CN
      - LANGUAGE_zh_TW
      - LANGUAGE_hr
      - LANGUAGE_cs
      - LANGUAGE_da
      - LANGUAGE_nl
      - LANGUAGE_en
      - LANGUAGE_et
      - LANGUAGE_tl
      - LANGUAGE_fi
      - LANGUAGE_fr
      - LANGUAGE_de
      - LANGUAGE_el
      - LANGUAGE_iw
      - LANGUAGE_hi
      - LANGUAGE_hu
      - LANGUAGE_is
      - LANGUAGE_id
      - LANGUAGE_it
      - LANGUAGE_ja
      - LANGUAGE_ko
      - LANGUAGE_lv
      - LANGUAGE_lt
      - LANGUAGE_ms
      - LANGUAGE_no
      - LANGUAGE_fa
      - LANGUAGE_pl
      - LANGUAGE_pt
      - LANGUAGE_ro
      - LANGUAGE_ru
      - LANGUAGE_sr
      - LANGUAGE_sk
      - LANGUAGE_sl
      - LANGUAGE_es
      - LANGUAGE_sv
      - LANGUAGE_th
      - LANGUAGE_tr
      - LANGUAGE_uk
      - LANGUAGE_ur
      - LANGUAGE_vi


  SomeDefinition:
    type: object
    properties:
      languages:
        type: array
        minItems: 0
        maxItems: 3
        uniqueItems: true
        items:
          $ref: '#/definitions/Language'

Instead of returning an array of strings, I return an array of characters - e.g.
['L', 'A', 'N', 'G', 'U', 'A', 'G', 'E', '_', 'a', 'r']

This is because in the generated ApiClient.js, in convertToType() you see:

        } else if (typeof type === 'object') {
          // for plain object type like: {'String': 'Integer'}
          var keyType, valueType;
          for (var k in type) {
            if (type.hasOwnProperty(k)) {
              keyType = k;
              valueType = type[k];
              break;
            }
          }
          var result = {};
          for (var k in data) {
            if (data.hasOwnProperty(k)) {
              var key = exports.convertToType(k, keyType);
              var value = exports.convertToType(data[k], valueType);
              result[key] = value;
            }
          }
          return result;

Now, the $ref should return a type of string, but instead it returns an object.. and then in for (var k in data) it just iterates over the characters of the string.

This looks similar to another bug I found that was with refs to primitive types:
#4973

@advance512
Copy link

I managed to go around this issue by explicitly including the type:

  SomeDefinition:
    type: object
    properties:
      languages:
        type: array
        minItems: 0
        maxItems: 3
        uniqueItems: true
        items:
          type: string
          enum:
                  - LANGUAGE_ar
                  - LANGUAGE_bg
                  - LANGUAGE_ca
                  - LANGUAGE_zh_CN
                  - LANGUAGE_zh_TW
                  - LANGUAGE_hr
                  - LANGUAGE_cs
                  - LANGUAGE_da
                  - LANGUAGE_nl
                  - LANGUAGE_en
                  - LANGUAGE_et
                  - LANGUAGE_tl
                  - LANGUAGE_fi
                  - LANGUAGE_fr
                  - LANGUAGE_de
                  - LANGUAGE_el
                  - LANGUAGE_iw
                  - LANGUAGE_hi
                  - LANGUAGE_hu
                  - LANGUAGE_is
                  - LANGUAGE_id
                  - LANGUAGE_it
                  - LANGUAGE_ja
                  - LANGUAGE_ko
                  - LANGUAGE_lv
                  - LANGUAGE_lt
                  - LANGUAGE_ms
                  - LANGUAGE_no
                  - LANGUAGE_fa
                  - LANGUAGE_pl
                  - LANGUAGE_pt
                  - LANGUAGE_ro
                  - LANGUAGE_ru
                  - LANGUAGE_sr
                  - LANGUAGE_sk
                  - LANGUAGE_sl
                  - LANGUAGE_es
                  - LANGUAGE_sv
                  - LANGUAGE_th
                  - LANGUAGE_tr
                  - LANGUAGE_uk
                  - LANGUAGE_ur
                  - LANGUAGE_vi

Very sub-optimal, but it works for now.

@wing328
Copy link
Contributor

wing328 commented Mar 30, 2017

@knzm can you please submit a PR so that we can review the fix?

@wing328
Copy link
Contributor

wing328 commented Mar 30, 2017

@advance512 can you try the fix suggested by @knzm to see if that works for you?

@advance512
Copy link

Unfortunately, his fix doesn't help me since I am dependent on auto-generated code and don't want to add any manual steps in. I can try his fix one time, just to see if it helps. Will try to do it this week and update

@wing328
Copy link
Contributor

wing328 commented Mar 30, 2017 via email

@kirpit
Copy link
Contributor

kirpit commented May 22, 2017

I have tried to reproduce this bug with the exact given specs (current latest master) but looks like problem is already gone, despite to the suggested change (for } else if (typeof type === 'function') {) is not there.

Here are the pieces of correctly generated model definitions:

EnumArrays:

  /**
   * Allowed values for the <code>arrayEnum</code> property.
   * @enum {String}
   * @readonly
   */
  exports.ArrayEnumEnum = {
    /**
     * value: "fish"
     * @const
     */
    "fish": "fish",
    /**
     * value: "crab"
     * @const
     */
    "crab": "crab"  };

StringEnumObject:

  /**
   * Enum class StringEnumObject.
   * @enum {}
   * @readonly
   */
  var exports = {
    /**
     * value: "fish"
     * @const
     */
    "fish": "fish",
    /**
     * value: "crab"
     * @const
     */
    "crab": "crab"  };

@knzm, any chance to confirm and close the issue?

@wing328
Copy link
Contributor

wing328 commented May 25, 2017

@kirpit thanks for performing more tests to confirm the issue no longer exists.

@knzm please try the latest master and let us know if you still encounter the issue.

@wing328 wing328 closed this as completed May 25, 2017
@advance512
Copy link

advance512 commented Aug 5, 2017

I still see this issue.

Using swagger-codegen-cli-3.0.0-20170727.135949-1.jar with the command line:

java -jar ~/Downloads/swagger-codegen-cli-3.0.0-20170727.135949-1.jar generate -i ../fizzle.yaml -l javascript --additional-properties usePromises=true --additional-properties useES6=false -o ./javascript/

I see that arrays of strings get converted into arrays of objects with indexes as keys and each char as the value. e.g.

[
  {
    "0": "I",
    "1": "N",
    "2": "C",
    "3": "R",
    "4": "E",
    "5": "A",
    "6": "S",
    "7": "E",
    "8": "_",
    "9": "S",
    "10": "E",
    "11": "A",
    "12": "R",
    "13": "C",
    "14": "H",
    "15": "_",
    "16": "E",
    "17": "N",
    "18": "G",
    "19": "I",
    "20": "N",
    "21": "E",
    "22": "_",
    "23": "R",
    "24": "A",
    "25": "N",
    "26": "K",
    "27": "I",
    "28": "N",
    "29": "G",
    "30": "S"
  },
  {
    "0": "G",
    "1": "E",
    "2": "T",
    "3": "_",
    "4": "B",
    "5": "L",
    "6": "O",
    "7": "G",
    "8": "S",
    "9": "_",
    "10": "E",
    "11": "N",
    "12": "D",
    "13": "O",
    "14": "R",
    "15": "S",
    "16": "E",
    "17": "M",
    "18": "E",
    "19": "N",
    "20": "T"
  },
  {
    "0": "I",
    "1": "N",
    "2": "C",
    "3": "R",
    "4": "E",
    "5": "A",
    "6": "S",
    "7": "E",
    "8": "_",
    "9": "B",
    "10": "R",
    "11": "A",
    "12": "N",
    "13": "D",
    "14": "_",
    "15": "A",
    "16": "W",
    "17": "A",
    "18": "R",
    "19": "E",
    "20": "N",
    "21": "E",
    "22": "S",
    "23": "S"
  },
  {
    "0": "G",
    "1": "E",
    "2": "N",
    "3": "E",
    "4": "R",
    "5": "A",
    "6": "T",
    "7": "E",
    "8": "_",
    "9": "M",
    "10": "O",
    "11": "R",
    "12": "E",
    "13": "_",
    "14": "T",
    "15": "R",
    "16": "A",
    "17": "F",
    "18": "F",
    "19": "I",
    "20": "C"
  }
]

This should be, of course, an simple array of 4 strings.

[
  "INCREASE_SEARCH_ENGINE_RANKINGS",
  "GET_BLOGS_ENDORSEMENT",
  "INCREASE_BRAND_AWARENESS",
  "GENERATE_MORE_TRAFFIC"
]

Once again, looking at the generated ApiClient.js:518, you see that:

        } else if (Array.isArray(type)) {
          // for array type like: ['String']
          var itemType = type[0];
          return data.map(function(item) {
            return exports.convertToType(item, itemType);
          });

Now, wth the context being:

data = [
  "INCREASE_SEARCH_ENGINE_RANKINGS",
  "GET_BLOGS_ENDORSEMENT",
  "INCREASE_BRAND_AWARENESS",
  "GENERATE_MORE_TRAFFIC"
]; 
type = [
  {
    "GENERATE_MORE_TRAFFIC": "GENERATE_MORE_TRAFFIC",
    "GENERATE_MORE_CONVERSIONS": "GENERATE_MORE_CONVERSIONS",
    "INCREASE_SEARCH_ENGINE_RANKINGS": "INCREASE_SEARCH_ENGINE_RANKINGS",
    "SOCIAL_MEDIA_PRESENCE": "SOCIAL_MEDIA_PRESENCE",
    "GET_BLOGS_ENDORSEMENT": "GET_BLOGS_ENDORSEMENT",
    "ENTER_A_NEW_MARKET": "ENTER_A_NEW_MARKET",
    "IMPROVE_BRAND_IMAGE": "IMPROVE_BRAND_IMAGE",
    "INCREASE_BRAND_AWARENESS": "INCREASE_BRAND_AWARENESS",
    "APP_INSTALLS": "APP_INSTALLS",
    "LEADS_OR_SIGN_UP": "LEADS_OR_SIGN_UP",
    constructFromObject(obj): function(object) {
        return object;
     },
  }
];

the type is an Array of enum strings, so itemType becomes the Object shown above, and then you recursively enter exports.convertToType(data, type) which is then with data being a string with an enum val (e.g. "INCREASE_SEARCH_ENGINE_RANKINGS") and type being the Object, so you get to this part:

        } else if (typeof type === 'object') {
            if (type.hasOwnProperty(k)) {
              keyType = k;
              valueType = type[k];
              break;
            }
          }
          var result = {};
          for (var k in data) {
            if (data.hasOwnProperty(k)) {
              var key = exports.convertToType(k, keyType);
              var value = exports.convertToType(data[k], valueType);
              result[key] = value;
            }
          }
          return result;
        } else {

Now, when you get to the line: for (var k in data) { everything gets jumbled.

for..in used on a string has the following behavior:

> for (var k in "abcdef") { console.log(k); }
0
1
2
3
4
5

These are actually "0", "1", "2". Not even numeric values.

Now, remember, data is a string. So k becomes the index of the first character in string form, e.g. "0", and key gets the value of k, while value get the char itself. This is written into the result as seen above.

{
    "0": "I",
    "1": "N",
    "2": "C",
    "3": "R",
    "4": "E",
    "5": "A",
    ...

The issue here is that we get to a point where the type is an object with possible string Enum values, the data is a string, but instead of looking at the entire string as the string value to check against the Enum we get a index -> character Object.

@advance512
Copy link

@kirpit - is the issue only fixed with ES6 usage, or did you also check useES6=false output?

@advance512
Copy link

@wing328 - please reopen this. The issue persists even with the latest trunk as shown above. :)

@wing328 wing328 reopened this Aug 5, 2017
@kirpit
Copy link
Contributor

kirpit commented Nov 4, 2017

I don't understand, I again tried against the current master 4dafa5b and results for both fish / crab and languages were generated correctly.

There has been also kind of trimming for the attribute names in Language class which is cool:

export default class Language {
    
        /**
         * value: "LANGUAGE_ar"
         * @const
         */
        ar = "LANGUAGE_ar";

        // snip..

    /**
    * Returns a <code>Language</code> enum value from a Javascript object name.
    * @param {Object} data The plain JavaScript object containing the name of the enum value.
    * @return {module:model/Language} The enum <code>Language</code> value.
    */
    static constructFromObject(object) {
        return object;
    }
}

And yes also checked the -D useES6=false usage:

(function(root, factory) {
    // snip...
}(this, function(ApiClient) {
  'use strict';


  /**
   * Enum class Language.
   * @enum {}
   * @readonly
   */
  var exports = {
    /**
     * value: "LANGUAGE_ar"
     * @const
     */
    "ar": "LANGUAGE_ar",

    // snip ...

  /**
   * Returns a <code>Language</code> enum value from a Javascript object name.
   * @param {Object} data The plain JavaScript object containing the name of the enum value.
   * @return {module:model/Language} The enum <code>Language</code> value.
   */
  exports.constructFromObject = function(object) {
    return object;
  }

  return exports;
}));

@advance512
Copy link

Are you using $ref?

@advance512
Copy link

Any update on this, @kirpit ?

I am still seeing this issue, even now..

@kirpit
Copy link
Contributor

kirpit commented Mar 11, 2018

Can you please paste the specs and write the commands that you are able to reproduce the error on current master?

@advance512
Copy link

advance512 commented Apr 12, 2018

@kirpit - I am offering the best reproduction instructions that I can think of.


Please do the following:

  1. Use the following fizzle.yml file:
swagger: '2.0'
info:
  version: 0.8.127
  title: Fizzle
  description: Example
schemes:
  - http
host: localhost:5000
basePath: /fizzle
consumes:
  - application/json
produces:
  - application/json

paths:
  /ping:
    get:
      responses:
        '200':
          description: Ping reponse.
          schema:
            $ref: '#/definitions/SomeObject'
        default:
          $ref: '#/responses/TotallyUnexpectedResponse'

definitions:

  Language:
    type: string
    enum:
      - LANGUAGE_ar
      - LANGUAGE_bg
      - LANGUAGE_ca
      - LANGUAGE_zh_CN
      - LANGUAGE_zh_TW
      - LANGUAGE_hr
      - LANGUAGE_cs
      - LANGUAGE_da
      - LANGUAGE_nl
      - LANGUAGE_en
      - LANGUAGE_et
      - LANGUAGE_tl
      - LANGUAGE_fi
      - LANGUAGE_fr
      - LANGUAGE_de
      - LANGUAGE_el
      - LANGUAGE_iw
      - LANGUAGE_hi
      - LANGUAGE_hu
      - LANGUAGE_is
      - LANGUAGE_id
      - LANGUAGE_it
      - LANGUAGE_ja
      - LANGUAGE_ko
      - LANGUAGE_lv
      - LANGUAGE_lt
      - LANGUAGE_ms
      - LANGUAGE_no
      - LANGUAGE_fa
      - LANGUAGE_pl
      - LANGUAGE_pt
      - LANGUAGE_ro
      - LANGUAGE_ru
      - LANGUAGE_sr
      - LANGUAGE_sk
      - LANGUAGE_sl
      - LANGUAGE_es
      - LANGUAGE_sv
      - LANGUAGE_th
      - LANGUAGE_tr
      - LANGUAGE_uk
      - LANGUAGE_ur
      - LANGUAGE_vi

  SomeObject:
    type: object
    properties:
      languages:
        type: array
        minItems: 0
        maxItems: 3
        uniqueItems: true
        items:
          $ref: '#/definitions/Language'
      justOneLanguage:
        $ref: '#/definitions/Language'
      someInt:
        type: integer

responses:
  TotallyUnexpectedResponse:
    description: A totally unexpected response
  1. Download the following swagger-codegen (which is the latest master):
    swagger-codegen-cli-2.4.0-20180412.075009-225.jar

  2. Use the following command line:
    java -jar swagger-codegen-cli-2.4.0-20180412.075009-225.jar generate -i fizzle.yml -l javascript -o output --additional-properties usePromises=true --additional-properties useES6=false

  3. Open the file ./output/src/ApiClient.js. Starting in line 521, you will see the following code:

        if (type === Object) {
          // generic object, return directly
          return data;
        } else if (typeof type === 'function') {
          // for model type like: User
          return type.constructFromObject(data);
        } else if (Array.isArray(type)) {
          // for array type like: ['String']
          var itemType = type[0];
          return data.map(function(item) {
            return exports.convertToType(item, itemType);
          });
        } else if (typeof type === 'object') {
          // for plain object type like: {'String': 'Integer'}
          var keyType, valueType;
          for (var k in type) {
            if (type.hasOwnProperty(k)) {
              keyType = k;
              valueType = type[k];
              break;
            }
          }
          var result = {};
          for (var k in data) {
            if (data.hasOwnProperty(k)) {
              var key = exports.convertToType(k, keyType);
              var value = exports.convertToType(data[k], valueType);
              result[key] = value;
            }
          }
          return result;
        } else {
          // for unknown type, return the data directly
          return data;
        }
  1. Now, imagine we actually use this client library with a server that returns the following result:
{
  "languages": [
    "LANGUAGE_en",
    "LANGUAGE_fr"
],
  "justOneLanguage": "LANGUAGE_ar",
  someInt: 3
}

As usual, convertToType() is called to convert data received from the server to appropriate types. In our case, when handling the languages member of the result, convertToType() is called with the following arguments:

data = [
  "LANGUAGE_en",
  "LANGUAGE_fr"
]; 
type = [
  {
    "LANGUAGE_ar": "LANGUAGE_ar",
    "LANGUAGE_bg": "LANGUAGE_bg",
    .
    .
    "LANGUAGE_ur": "LANGUAGE_ur",
    "LANGUAGE_vi": "LANGUAGE_vi",
    constructFromObject(obj): function(object) {
        return object;
     },
  }
];

The type represents an Array of enum strings. In convertToType() we get to line 527, and recurse, calling convertToType() again. This time, we get called with the following context:

data = "LANGUAGE_en"; 
type = {
    "LANGUAGE_ar": "LANGUAGE_ar",
    "LANGUAGE_bg": "LANGUAGE_bg",
    .
    .
    "LANGUAGE_ur": "LANGUAGE_ur",
    "LANGUAGE_vi": "LANGUAGE_vi",
    constructFromObject(obj): function(object) {
        return object;
     },
  };

Now, as you run convertToType() you get to this part:

        } else if (typeof type === 'object') {
          // for plain object type like: {'String': 'Integer'}
          var keyType, valueType;
          for (var k in type) {
            if (type.hasOwnProperty(k)) {
              keyType = k;
              valueType = type[k];
              break;
            }
          }
          var result = {};
          for (var k in data) {
            if (data.hasOwnProperty(k)) {
              var key = exports.convertToType(k, keyType);
              var value = exports.convertToType(data[k], valueType);
              result[key] = value;
            }
          }
          return result;
        } else {

Now, when you get to the line: for (var k in data) { everything gets jumbled.
Remember data is a string, with the value "LANGUAGE_en".
for..in used on a string has the following behavior:

> for (var k in "abcdef") { console.log(k); }
0
1
2
3
4
5

These are actually "0", "1", "2". Not even numeric values.

Again, remember, data is a string. So k becomes the index of the first character in string form, e.g. "0". Then, we get to this code:

var key = exports.convertToType(k, keyType);
var value = exports.convertToType(data[k], valueType);
result[key] = value;

So key gets the value of k, while value get the char itself, a part of the string.
This is written into the result as seen above. This is how it ends up looking:

result = {
    "0": "L",
    "1": "A",
    "2": "N",
    "3": "G",
    "4": "U",
    "5": "A",
    ...

So finally, when the initial call to convertToType() ends, we end up with a result that looks like:

result = [
  {
    "0": "L",
    "1": "A",
    "2": "N",
    "3": "G",
    "4": "U",
    "5": "A",
    "6": "G",
    "7": "E",
    "8": "_",
    "9": "e",
    "10": "n",
  },
  {
    "0": "L",
    "1": "A",
    "2": "N",
    "3": "G",
    "4": "U",
    "5": "A",
    "6": "G",
    "7": "E",
    "8": "_",
    "9": "f",
    "10": "r",
  }
]

The issue here is that we get to a point where the type is an object with possible string Enum values, the data is a string, but instead of looking at the entire string as the string value to check against the Enum we get a index -> character Object.


@kirpit, is there is anything more I can offer, let me know.

@advance512
Copy link

I'll add that I am still getting the same issue with v3.0.0, e.g. using swagger-codegen-cli-3.0.0-20170727.135949-1.jar as above.

@webron
Copy link
Contributor

webron commented Apr 12, 2018

@adamholdenyall aval lo asinu migratzya shel javascript le-3.0.0. ze lo ya'avod.

@webron
Copy link
Contributor

webron commented Apr 12, 2018

er, @advance512, sorry.

@advance512
Copy link

@webron huh? :)

I am using the latest master, but the same issue happens with 3.0.0 too.
Not sure what you meant by no migration was done to 3.0.0...

@webron
Copy link
Contributor

webron commented Apr 12, 2018

@advance512 ma kore?

Version 3.0.0 of the codegen requires a migration of the generators/templates before they can be used with it. At the moment we have about 5-6 migrated generators, JavaScript is not one of them. More information in the release notes - https://github.com/swagger-api/swagger-codegen/releases/tag/v3.0.0-rc0.

@advance512
Copy link

@webron Hakol tov. :)
Please note that I am talking about swagger-codegen-cli-2.4.0-20180412.075009-225.jar.

@kirpit
Just to make things easier, I created a reproducer for this issue.
https://github.com/advance512/swagger-codegen-issue-4819

You can follow the instructions and see the issue occurring for yourself.
I am hoping this will help with finally solving it.

@webron
Copy link
Contributor

webron commented Apr 12, 2018

@advance512 - okay, you were the one who mentioned swagger-codegen-cli-3.0.0-20170727.135949-1.jar and I'm just saying that's not going to work at the moment.

@advance512
Copy link

advance512 commented Apr 12, 2018

@webron can you explain what you mean?
We've been using swagger-codegen-cli-3.0.0-20170727.135949-1.jar as our main Swagger client generating tool for a while now and have faced no issues. (Except for the bugs I reported.)

@webron
Copy link
Contributor

webron commented Apr 12, 2018

I'm confused. One time you're talking about swagger-codegen-cli-3.0.0-20170727.135949-1.jar another time on swagger-codegen-cli-2.4.0-20180412.075009-225.jar.

3.0.0 officially supports a very limited set of languages right now. 2.4.0 supports the entire set.

@advance512
Copy link

@webron Okay, let me make it clear then:

  1. This entire thread talks about a bug that I have seen (again) today in swagger-codegen-cli-2.4.0-20180412.075009-225.jar, which is the latest master build, and that @knzm has seen more than a year ago in 2.2.2-SNAPSHOT (revision: bb81fc1).
    This is the main, most important point.

  2. Regardless of this, I have also seen this same issue with swagger-codegen-cli-3.0.0-20170727.135949-1.jar. I have created a separate branch in the issue reproduction of this issue, using this version of swagger-codegen:
    https://github.com/advance512/swagger-codegen-issue-4819/tree/swagger-codegen-cli-3.0.0-20170727.135949-1.jar
    You can check it out to verify. The command-line is exactly as shown above.

  3. We currently use swagger-codegen-cli-3.0.0-20170727.135949-1.jar to generate our JavaScript client libraries, which are based on a Swagger definition that sidesteps the issue reported here, so we do not face any immediate problems. Now, whether a very limited set of languages is supported with the latest 3.0.0 or not, I do not know. What I do know is that we definitely use whatever is supported in swagger-codegen-cli-3.0.0-20170727.135949-1.jar, and that it works for us.

I hope all of this makes sense.

@advance512
Copy link

advance512 commented Apr 12, 2018

BTW, @kirpit, @wing328, @webron , I think I found the root of the issue.

If you look at the generated constructFromObject function for SomeObject, you'll see:

  /**
   * Constructs a <code>SomeObject</code> from a plain JavaScript object, optionally creating a new instance.
   * Copies all relevant properties from <code>data</code> to <code>obj</code> if supplied or a new instance if not.
   * @param {Object} data The plain JavaScript object bearing properties of interest.
   * @param {module:model/SomeObject} obj Optional instance to populate.
   * @return {module:model/SomeObject} The populated <code>SomeObject</code> instance.
   */
  exports.constructFromObject = function(data, obj) {
    if (data) {
      obj = obj || new exports();

      if (data.hasOwnProperty('languages')) {
        obj['languages'] = ApiClient.convertToType(data['languages'], [Language]);
      }
      if (data.hasOwnProperty('justOneLanguage')) {
        obj['justOneLanguage'] = Language.constructFromObject(data['justOneLanguage']);
      }
      if (data.hasOwnProperty('someInt')) {
        obj['someInt'] = ApiClient.convertToType(data['someInt'], 'Number');
      }
    }
    return obj;
  }

Note the difference way of treating enums.
Array of enums:
obj['languages'] = ApiClient.convertToType(data['languages'], [Language]);
Single enum:
obj['justOneLanguage'] = Language.constructFromObject(data['justOneLanguage']);

So, a single enum gets created using Language.constructFromObject, but an enum in array gets created using this part of ApiClient.convertToType:

    } else if (typeof type === 'object') {
          // for plain object type like: {'String': 'Integer'}
          var keyType, valueType;
          for (var k in type) {
            if (type.hasOwnProperty(k)) {
              keyType = k;
              valueType = type[k];
              break;
            }
          }
          var result = {};
          for (var k in data) {
            if (data.hasOwnProperty(k)) {
              var key = exports.convertToType(k, keyType);
              var value = exports.convertToType(data[k], valueType);
              result[key] = value;
            }
          }
          return result;
        }

which does not even contain a call to a constructFromObject type function.

This seems to originate from here:
https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/JavascriptClientCodegen.java#L621

@advance512
Copy link

On second thought, I think @knzm 's fix is worth a shot:

--- a/modules/swagger-codegen/src/main/resources/Javascript/ApiClient.mustache
+++ b/modules/swagger-codegen/src/main/resources/Javascript/ApiClient.mustache
@@ -470,7 +470,7 @@
         if (type === Object) {
           // generic object, return directly
           return data;
-        } else if (typeof type === 'function') {
+        } else if (typeof type.constructFromObject === 'function') {
           // for model type like: User
           return type.constructFromObject(data);
         } else if (Array.isArray(type)) {

@eliranamar
Copy link

I also encountered @knzm problem with this.

@Nusnus
Copy link

Nusnus commented Apr 16, 2018

Hey guys, I am dealing with this bug for some time now and it's really time wasting (especially today and I really hate when I finally realize it was this issue).
Are there any estimations or actually any progress in regard to when will it be fixed?

Thanks.

@t0ch1k
Copy link

t0ch1k commented Apr 22, 2018

Hi guys! When this issue will be fixed?

@advance512
Copy link

Hi there, @kirpit @wing328 @webron

Any chance you can have a look?
Or tell me what can be done to help and advance this fix?

@ap911
Copy link

ap911 commented Jun 11, 2018

Hi All

any plans on fixing the issue?

@advance512
Copy link

Hey @kirpit @wing328 @webron,

3.5 months since we got any sort of real update.. is this bug being fixed?
Can I sponsor it being fixed?

@advance512
Copy link

@kirpit @wing328 @webron
Any update?

@ap911 did you find a workaround?

@advance512
Copy link

I can confirm this still happens in:

swagger-codegen-cli-2.4.0-20181113.142213-347.jar

https://github.com/advance512/swagger-codegen-issue-4819/tree/swagger-codegen-cli-2.4.0-20181113.142213-347.jar

@advance512
Copy link

@CodeNinjai
Is there any way we can sponsor this bug being fixed?

@advance512
Copy link

advance512 commented Nov 14, 2018

@advance512
Copy link

FYI, this was fixed in OpenAPITools/openapi-generator#1429

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants