[BUG][typescript-angular] Custom typeMapping for file is overriden in generator

[Adelrisk]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?

    Foo:
      type: object
      properties:
        msg:
          type: string
          format: binary

The generator produces msg with the type Blob.

class Foo {
    msg: Blob;
}

Regardless of the various mappings I attempt to set, I cannot find the correct mapping declaration to ensure that this is instead mapped to string. The obvious typeMapping file=string should work, but it's overriden in code here: https://github.com/OpenAPITools/openapi-generator/blob/v4.0.2/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptAngularClientCodegen.java#L282

• Have you validated the input using an OpenAPI validator (example)?
  Yes

• What's the version of OpenAPI Generator used?
  4.0.2

• Have you search for related issues/PRs?
  The keyword blob returns too many deep links into github code. So I looked at the code.

• What's the actual output vs expected output?

I would like to generate the following:

class Foo {
    msg: string;
}

I'm migrating from the io.swagger tool and would like to avoid breaking changes. Blob breaks existing code.

Suggest a fix

If I were to have a stab at this, I think the complete removal of the overriding method would be enough: https://github.com/OpenAPITools/openapi-generator/blob/v4.0.2/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptAngularClientCodegen.java#L280

    @Override
    public String getTypeDeclaration(Schema p) {
        if (ModelUtils.isFileSchema(p)) {
            return "Blob";
        } else {
            return super.getTypeDeclaration(p);
        }
}

The correct default behaviour should already be provided with the typeMapping in the constructor:

typeMapping.put("file", "Blob");

However, there is so much going on here, I just can't guesstimate what else this might effect. Such as this method:

    @Override
    public boolean isDataTypeFile(final String dataType) {
        return dataType != null && dataType.equals("Blob");
    }

[auto-labeler]

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

[macjohnny]

@Adelrisk one way to overcome the problem in your project would be to add a post-processor, which should be quite easy.

On the other hand, a profound analysis of the impact of your suggested solution would be very welcome.

[Adelrisk]

@macjohnny Thanks for the tip, I'll look into the post-processor as a workaround.

If the other hand is about a polite request from me to provide the necessary profound analysis of the impact of my suggestion, then here it is:

It appears that this tool (OpenAPITools/openapi-generator) builds on a framework of concepts and conventions to enable generator-specific customization and extensibility. For some reason, this framework was not sufficient for the typescript-angular generator, and the type-mappings had to be supplemented with additional overrides to ensure the system works.

Either this code with the overrides is older than the successful support for the type mappings in the framework, or it is important because of reasons still unknown. Whoever can look at the following code and answer these questions would be able to provide a profound analysis of the impact of my suggested solution, or possibly even say what the real solution should be.

https://github.com/OpenAPITools/openapi-generator/blob/v4.0.2/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptAngularClientCodegen.java#L275

  @Override
  public boolean isDataTypeFile(final String dataType) {
    return dataType != null && dataType.equals("Blob");
  }

https://github.com/OpenAPITools/openapi-generator/blob/v4.0.2/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractTypeScriptClientCodegen.java#L123

  // AbstractTypeScriptClientCodegen.java#L123
  typeMapping.put("Map", "any");
  typeMapping.put("map", "any");
  typeMapping.put("date", "string");
  typeMapping.put("DateTime", "Date");
  typeMapping.put("binary", "any");
  typeMapping.put("File", "any");
  typeMapping.put("ByteArray", "string");

https://github.com/OpenAPITools/openapi-generator/blob/v4.0.2/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptAngularClientCodegen.java#L70

typeMapping.put("file", "Blob");

• Why are the mappings only sometimes case-sensitive?
• How does the function isDataTypeFile normally play a role in the framework?
• Why is the functionality for type-mappings insufficient that a hard-coded override in getTypeDeclaration is required?
• Does this invalidate the ability to provide custom type mappings for File? (This may be another problem.)
• How would one go about testing changes to the file TypeScriptAngularClientCodegen.java.

In no way am I qualified to provide these answers with my current knowledge of the project.

[macjohnny]

maybe @roni-frantchi @akehir @topce have an idea?

[akehir]

I'm tempted to say, that the behaviour is correct. If you specify a string of format binary, it shouldn't be a string in Typescript.

If you check the OAS Specification for Data Types, then we see that string can have the formats:

type	format	Comments
string
string	byte	base64 encoded characters
string	binary	any sequence of octets
string	date	As defined by full-date - RFC3339
string	date-time	As defined by date-time - RFC3339
string	password	A hint to UIs to obscure input

In other words, the following 2 examples would both give the msg?: string output.

Generated code:

export interface Foo { 
    msg?: string;
}

Specification 1

"definitions" : {
    "Foo" : {
      "type" : "object",
      "properties" : {
        "msg" : {
          "type" : "string",
          "format" : "byte"
        }
      }
    }
  },

Specification 2

"definitions" : {
    "Foo" : {
      "type" : "object",
      "properties" : {
        "msg" : {
          "type" : "string"
        }
      }
    }
  },

[Adelrisk]

I'm missing the complete context in this answer. Is this an insistence that the described behaviour of the overrides matches the observed behaviour? And maybe there is another problem if this Blob hack is on in the one typescript generator and not in the abstract base.

…

On Tue., 16 Jul. 2019, 10:20 Raphael Ochsenbein, ***@***.***> wrote: I'm tempted to say, that the behaviour is correct. If you specify a 'string' of format 'binary', it shouldn't be a string in Typescript. If you check the OAS Specification for Data Types <https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#data-types>, then we see that string can have the formats: type <#m_-4985561643024994826_dataTypes> | format <#m_-4985561643024994826_dataTypeFormat> | Comments ------ | -------- | -------- string | | | string | byte | base64 encoded characters string | binary | any sequence of octets string | date | As defined by full-date - RFC3339 <https://xml2rfc.ietf.org/public/rfc/html/rfc3339.html#anchor14> string | date-time | As defined by date-time - RFC3339 <https://xml2rfc.ietf.org/public/rfc/html/rfc3339.html#anchor14> string | password | A hint to UIs to obscure input. In other words, the following 2 examples would both give the msg?: string output. Generated code: export interface Foo { msg?: string; } Specification 1 "definitions" : { "Foo" : { "type" : "object", "properties" : { "msg" : { "type" : "string", "format" : "byte" } } } },``` ### Specification 2 ```json"definitions" : { "Foo" : { "type" : "object", "properties" : { "msg" : { "type" : "string" } } } },``` — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3341?email_source=notifications&email_token=ACDKSMDGQGTGUKD344HSMCTP7WAGJA5CNFSM4IBIT472YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2ACY3I#issuecomment-511716461>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACDKSMAHEFP4U2OTHH6OTB3P7WAGJANCNFSM4IBIT47Q> .

[akehir]

@Adelrisk:

I'm just stating that the correct data type to handle binary data is Blob in JavaScript / TypeScript.

If you are trying to use a string as a string as a string type, you'd need to adapt the format part of your API specification.

The same, if I use the typescript-fetch generator with your declaration (my full swagger example) , I get:

export interface Foo {
    /**
     * 
     * @type {Blob}
     * @memberof Foo
     */
    msg?: Blob;
}

Only if I drop the binary format of the specification, as here, I get the msg: string:

export interface Pet {
    /**
     * 
     * @type {string}
     * @memberof Pet
     */
    msg?: string;
}

So, it is your declaration, which seems to be using the wrong format, and neither the typescript-angular nor the typescript-fetch generator behave how you'd expect them to behave.

[akehir]

an insistence that the described behaviour of the overrides matches the observed behaviour? And maybe there is another problem if this Blob hac

So yes, maybe this should be in the abstract base class; but I'd say it's specific to the JavaScript / TypeScript handling of the binary string format.

[macjohnny]

I agree with @akehir, type: string with format: binary should always result in a Blob, while type: string with format: byte results in a string, which matches the implemetation of the specification.
So IMHO we should close this issue.

[Adelrisk]

So, there is no support for allowing custom types instead of Blob in the tooling? Not all types are equal? This is disappointing. I like my type safety in Typescript. Thanks anyway I guess.

…

On Tue., 16 Jul. 2019, 10:55 Esteban Gehring, ***@***.***> wrote: I agree with @akehir <https://github.com/akehir>, type: string with format: binary should always result in a Blob, while type: string with format: byte results in a string, which matches the implemetation of the specification. So IMHO we should close this issue. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3341?email_source=notifications&email_token=ACDKSMDPVXN2CLNDREVNRSDP7WEIDA5CNFSM4IBIT472YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2AFXKQ#issuecomment-511728554>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACDKSMC73CTX5PEJH3EQ2RTP7WEIDANCNFSM4IBIT47Q> .

[macjohnny]

@Adelrisk can't you change format: byte in your spec?

[macjohnny]

So, there is no support for allowing custom types instead of Blob in the
tooling? Not all types are equal?

In general this feature is available, but in this case, I can't really tell you why the type is set explicitly, regardless of any overriding mechanism, but I suppose it is there for some reason. Another way to fix your issue would be to additionally check for type overrides.

[Adelrisk]

Thank you for the first answer that seems to acknowledge the bug in the code or the elephant sharing the room with me. However, the lack of knowledge demonstrates that we are now (finally?) exactly there where I said I am when I analysed the code and the possible way forward. (Tests?) Also, thank you for your time pointing out how a correctly specified API might look like. Thus prepared and informed by the reactions in this thread, I will adopt the workarounds as capable and necessary going forward.