api paths decorated with :.+ produce invalid angular code

[inthegarage]

Description

If I have the following api path:

/myservice/{myvar:.+}:

Then this produces invalid angular code with the typescript-angular plugin.

Swagger-codegen version

2.3.0

Swagger declaration file content or url

Any API path with:
/myservice/{myvar:.+}:

Command line used for generation

mvn clean compile

Steps to reproduce

Simply put that into your path and use the typescript-angular plugin to generate. The default.service.ts generation is invalid:

${this.basePath}/myservice/${encodeURIComponent(String(myvar:.+))}`,

Produces the following ts compile errors:

[ERROR] src/app/api/default.service.ts(172,132): error TS1005: ',' expected.
[ERROR] src/app/api/default.service.ts(172,133): error TS1135: Argument expression expected.
[ERROR] src/app/api/default.service.ts(172,135): error TS1109: Expression expected.

We will have to regress to 2.2.3 version for the time being. The :.+ construct was working fine in 2.2.3 and enables the system to process variables with "extra" characters such as slashes and the like.

[macjohnny]

@inthegarage could you explain what you mean by

process variables with "extra" characters such as slashes and the like.

Since the encodeURIComponent() function was wrapped around the variable, any string content should work as a parameter.

See also #6525 and #6295

[inthegarage]

@macjohnny The :.+ construct is needed so it doesn't think it is part of the path (client and server side) otherwise the match is not made. It isn't really anything to do with encoding the server, just won't understand and think you are trying to reach a path with slashes. Therefore you have to use the construct above and that produces server side code like the following:

@RequestMapping(value = "/myservice/{myvar:.+}",

So the issue is, to get server sides to recognise we have to use this construct, however that produces invalid angular code.

[macjohnny]

@inthegarage can you provide an example incl. the request url?
if you want to match e.g. /my/api/with/some/parameter you could define
/my/api/{param1}/{param2}/{param3}, where you could simply ignore param1 and param2.

on the other hand
/my/api/with%2Fsome%2Fparameter will match the api endpoint
/my/api/{param}, and {param} will contain the value with/some/parameter

[inthegarage]

@macjohnny
Thanks for the reply. The issue is all to do with how Spring handles slashes and parameters. The issue is best described in this SO post:

https://stackoverflow.com/questions/31421061/how-to-handle-requests-that-includes-forward-slashes

To get around this we need to use ":.+" added to the end of the parameter. This is handled well in the yaml defintiion and the spring generator creates the correct mapping. This has to be the mapping as Spring will not handle it correctly otherwise.

In 2.2.3 this was also handled with no problem by the typescript-angular2 plugin (now typescript-angular), however in 2.3.0 this is no longer handled correctly and invalid typescript is being produced.
The errors and erroneous line are shown above.

For instance the following yaml demonstrates this:

  /roles/{id:.+}:
    get:
      operationId: getRoleById
      parameters:
        - name: id
          in: path
          required: true
          type: string
      produces:
        - application/json
      responses:
        '200':
          description: Ok
          schema:
            $ref: '#/definitions/role'
        '404':
          description: not found
          schema:
            $ref: '#/definitions/errorModel'
        default:
          description: unexpected error
          schema:
            $ref: '#/definitions/errorModel'

The generate source code that fails for example is:

        return this.httpClient.get<Role>(`${this.basePath}/roles/${encodeURIComponent(String(id:.+))}`,
            {
                withCredentials: this.configuration.withCredentials,
                headers: headers,
                observe: observe,
                reportProgress: reportProgress
            }
        );

The error is:

[ERROR] ERROR in src/app/api/default.service.ts(557,96): error TS1005: ',' expected.

In Spring the mapping is generated just fine:

    @ApiOperation(value = "", nickname = "getRoleById", notes = "", response = Role.class, tags={  })
    @ApiResponses(value = { 
        @ApiResponse(code = 200, message = "Ok", response = Role.class),
        @ApiResponse(code = 404, message = "not found", response = ErrorModel.class),
        @ApiResponse(code = 200, message = "unexpected error", response = ErrorModel.class) })
    @RequestMapping(value = "/roles/{id:.+}",
        produces = { "application/json" }, 
        method = RequestMethod.GET)
    ResponseEntity<Role> getRoleById(@ApiParam(value = "",required=true) @PathVariable("id") String id);

Do you have enough information to investigate?

[inthegarage]

Note with version 2.3.1 the error has now changed slightly, although invalid code is still being produced:
error TS2552: Cannot find name 'Id_'. Did you mean 'Id'?

return this.httpClient.get(${this.basePath}/roles/${encodeURIComponent(String(Id_))},

Please continue to investigate.

[macjohnny]

@inthegarage could you try if #7479 fixes your issue?
just build https://github.com/macjohnny/swagger-codegen/tree/bugfix/7476-typescript-angular-path-parameter-date and give it a try

[inthegarage]

@macjohnny Yes will build and give it a go.

[inthegarage]

@macjohnny Yes that has worked for this particular problem and the URLs were dealt with correctly. I did notice however that the code produced in default.service.ts was using a few deprecated items and that some of the other build objects seemed "a little out of date". I take it this is just because of your personal branch and will be corrected when merged?
Also which release will this be in 2.3.2? Please confirm.

[macjohnny]

@inthegarage

I did notice however that the code produced in default.service.ts was using a few deprecated items and that some of the other build objects seemed "a little out of date".

Can you clarify what you mean by this? Can you specify which items are outdated?
My branch is based on the current master branch, so this should not be an issue. Which angular version are you using?

Also which release will this be in 2.3.2? Please confirm.

I hope this will be released with 2.4.0, but this has to be set by @wing328

[inthegarage]

@macjohnny

I am using Angular 5+
Deprecated items, seemed to be things such as Http instead of HttpClient and UrlSearchParams which are both deprecated. UrlSearchParams, is just simply replaced by an options item.

I also saw that there seemed to be items generated that were not linked into the models.ts file. I will get you further details.

[macjohnny]

@inthegarage please set the angular version to 4.3 when using the typescript angular generator, which generates api code that uses HttpClient instead of Http, so there should not be any deprecated items.

[inthegarage]

@macjohnny Not sure I've come across that option for the new typescript-angular plugin, do you have a link to the docs?

[macjohnny]

@inthegarage you can find the options with the following command:

java -jar .\modules\swagger-codegen-cli\target\swagger-codegen-cli.jar config-help -l typescript-angular

as 4.3 this is the default value for ngVersion, I guess somewhere in your config you have set the version, e.g. in your pom.xml for swagger-codegen-maven-plugin

[inthegarage]

Here is my maven configuration part:

            <configuration>
              <inputSpec>${project.basedir}/swaggerconfig/api.yaml</inputSpec>
              <language>typescript-angular</language>
              <ngVersion>4.3</ngVersion>
              <output>${project.basedir}/src/app/</output>
            </configuration>

This produces code such as:

        let requestOptions: RequestOptionsArgs = new RequestOptions({
            method: RequestMethod.Get,
            headers: headers,
            search: queryParameters,
            withCredentials:this.configuration.withCredentials
        });

RequestOptionsArgs, RequestMethod and RequestOptions are all deprecated. As is the following:

    let queryParameters = new URLSearchParams('', new CustomQueryEncoderHelper());

Do I have an incorrect maven configuration?
In the main API module the following is also seen:

imports: [ CommonModule, HttpModule ],

HttpClient Module is the newer one to use.

[macjohnny]

@inthegarage you should set ngVersion inside <configOptions>, i.e.

                                <configuration>
                                    <inputSpec>${project.basedir}/swaggerconfig/api.yaml</inputSpec>
                                    <language>typescript-angular</language>
                                    <output>${project.basedir}/src/app/</output>
                                    <configOptions>
                                       <ngVersion>4.3</ngVersion>
                                    </configOptions>

however, this should already be the default...
@kenisteward @sebastianhaas any ideas?

[macjohnny]

The branch https://github.com/macjohnny/swagger-codegen/tree/bugfix/7476-typescript-angular-path-parameter-date should be based on the current master, as it is only 4 commits behind the swagger-api:master:
https://github.com/macjohnny/swagger-codegen/compare/bugfix/7476-typescript-angular-path-parameter-date...swagger-api:master

[inthegarage]

I have now updated my configuration, which seems to have been accepted. However the output remains the same, namely it is using deprecated items.

[macjohnny]

@inthegarage there is a configuration issue somewhere, as e.g. the sample generation works fine for 4.3 and also in our project we don't specify a version and it generates client code that uses HttpClient

[inthegarage]

@macjohnny I think the branch is correct, in so far it fixes the ":.+" issue. Whether it contains suitable to remove deprecated items I'd have to go through the source my drive.

[macjohnny]

can you verify that in .swagger-codegen/VERSION you have

2.4.0-SNAPSHOT

i.e. you are using the snapshot version of the https://github.com/macjohnny/swagger-codegen/tree/bugfix/7476-typescript-angular-path-parameter-date branch?

[inthegarage]

@macjohnny working on it, moment please.

[inthegarage]

@macjohnny
My git status says the following:
HEAD detached at origin/bugfix/7476-typescript-angular-path-parameter-date
nothing to commit, working tree clean

My VERSION file says the following:

2.4.0-SNAPSHOT

The default.service.ts and api.module.ts continue to have deprecated items in them. The config options is as follows:

4

Option "4.3" is not permitted. Please advise, thank you.

[macjohnny]

setting ngVersion to 4.3 should definitely be possible if you are using the correct artifact, since the samples could be generated as well.

Can you run maven clean.
Moreover, could you post the result of git log?
Can you post the configuration that leads to

Option "4.3" is not permitted.

[inthegarage]

@macjohnny I think it may have been an old config, as the clean has done the job.
No more deprecations! I think this fix is fine, so please close and merge as necessary, thank you.

[macjohnny]

@inthegarage could you post the success of your test in the PR #7479? you could also do a code review.

[inthegarage]

@macjohnny @wing328
After getting around my build compile issue, I find that sadly this fix hasn't worked a 100%.
Please check the enclosed project. The default.service.ts still contains an issue:
swagger-test.zip

You can do a mvn clean install in this project.
The error that is shown is as follows:

[ERROR] �[0m�[1m�[31mERROR in src/app/api/default.service.ts(90,101): error TS2304: Cannot find name 'packageId_'.�[39m�[22m�[0m

It appends an extra _ to the end of the variable. Could you take a look for me?
I'm sorry I've not been able to devote much time to this and the other spurious error has not helped.
Thank you.

[macjohnny]

@inthegarage the api parameter in the method signature is called contentPackageId,

public editHeader(contentPackageId: string, ...

but the request calls

this.httpClient.get<Header>(`${this.basePath}/editheader/${encodeURIComponent(String(packageId_))}`

Could you please provide a minimum swagger definition file?

[inthegarage]

That is in the file attached and also here: (swagger-test.zip)

# API for the next sprint, used for spec'ing the next sprint.
# This will be copied into api.yaml when the next sprint starts.errorModel
swagger: '2.0'
info:
  version: '0.1.0'
  title: TEST API
  description: TEST  API
  termsOfService: http://
  contact:
    name: TBC
    email: [email protected]
    url: http://www.entitygroup.com
  license:
    name: TBC
    url: http://
host: www.tester.com
basePath: /api
schemes:
  - https
produces:
  - application/json
paths:
  /users:
    operationId: getUsers
    get:
      responses:
        "200":
          description:  Describe the 200 response in more detail
  /editheader/{packageId:.+}:
    get:
      description: EditHeader
      operationId: EditHeader
      parameters:
        - name: contentPackageId
          in: path
          description: Call the refresh workflow for post editing.
          required: true
          type: string
      produces:
        - application/json
      responses:
        '200':
          description: File upload completion OK
          schema:
            $ref: '#/definitions/header'
        '404':
          description: file not found
          schema:
            $ref: '#/definitions/errorModel'
        default:
          description: unexpected error
          schema:
            $ref: '#/definitions/errorModel'
definitions:
  testingstate:
    type: string
    description: one of [TEST, TEST1, TEST2]
    enum: [TEST, TEST1, TEST2]

  header:
    type: object
    description: eader
    properties:
      parm1:
        type: string
      parm2:
        type: string
        format: date-time
      parm3:
        type: boolean
      parm4:
        type: number
      parm5:
        type: number
      parm6:
        type: number
      parm7:
        type: number
      parm8:
        type: boolean

  errorModel:
    type: object
    required:
      - code
      - message
    properties:
      code:
        type: integer
        format: int32
      message:
        type: string

If you download the swagger test file, you will see the entire build structure and be able to do a mvn clean install.

[macjohnny]

@inthegarage your parameter name in the path does not match the parameter name in the parameters section:

  /editheader/{packageId:.+}:
    get:
      description: EditHeader
      operationId: EditHeader
      parameters:
        - name: contentPackageId
          in: path
          description: Call the refresh workflow for post editing.
          required: true
          type: string

With the following sample swagger definition containing correct parameter names

swagger: "2.0"
info:
  version: "1.0.0"
  title: minimal
  description: Example of the bare minimum Swagger spec
host: localhost:3000 # otherwise, a random port will be used each time
paths:
  /editheader/{packageId:.+}:
    get:
      description: EditHeader
      operationId: EditHeader
      parameters:
        - name: packageId
          in: path
          description: Call the refresh workflow for post editing.
          required: true
          type: string
  /users:
    get:
      responses:
        "200":
          description:  Describe the 200 response in more detail
definitions:
  test:
    type: string
    description: one of [TEST, TEST1, TEST2]
    enum: [TEST, TEST1, TEST2]

I get a valid output:

    /**
     * 
     * EditHeader
     * @param packageId Call the refresh workflow for post editing.
     * @param observe set whether or not to return the data Observable as the body, response or events. defaults to returning the body.
     * @param reportProgress flag to report request and response progress.
     */
    public editHeader(packageId: string, observe?: 'body', reportProgress?: boolean): Observable<any>;
    public editHeader(packageId: string, observe?: 'response', reportProgress?: boolean): Observable<HttpResponse<any>>;
    public editHeader(packageId: string, observe?: 'events', reportProgress?: boolean): Observable<HttpEvent<any>>;
    public editHeader(packageId: string, observe: any = 'body', reportProgress: boolean = false ): Observable<any> {
        if (packageId === null || packageId === undefined) {
            throw new Error('Required parameter packageId was null or undefined when calling editHeader.');
        }

         ...

        return this.httpClient.get<any>(`${this.basePath}/editheader/${encodeURIComponent(String(packageId))}`,
            {
                withCredentials: this.configuration.withCredentials,
                headers: headers,
                observe: observe,
                reportProgress: reportProgress
            }
        );
    }

Using https://github.com/macjohnny/swagger-codegen/tree/bugfix/7476-typescript-angular-path-parameter-date

Please make sure you are actually building / using the jar file built from https://github.com/macjohnny/swagger-codegen/tree/bugfix/7476-typescript-angular-path-parameter-date

[inthegarage]

@macjohnny Many thanks for the information. I can confirm that has corrected the issue. Looks like this parameter wasn't used properly/at all in typescript2/2.2.3 version. So the error has now been flagged up which is nice in a way. Thankfully no problem with this change.

[webron]

A little bit late to the party, but what you're trying to do here is not valid by the spec, and we should not support it in the codegen.

Having /editheader/{packageId:.+}: as the path, literally means there's a parameter called packageId:.+ and not packageId. The spec does not support path-like parameters. If the functionality has already been added to the codegen, it needs to be removed (cc: @wing328).

[inthegarage]

@webron The. + pararameters are valid in a spring environment and this what is being supported here, otherwise parameters that have complex values are not processed properly. In spring generator the parameter is packageId and allows any following characters.
The. + are removed by the typescript processor as you say they aren't valid in the url part for clients.
This change is working correctly and shouldn't be removed @wing328 @macjohnny

[macjohnny]

See also
OAI/OpenAPI-Specification#502
and OAI/OpenAPI-Specification#291

[webron]

@inthegarage they are valid in Spring, and other frameworks, they are not valid in the OpenAPI Specification. This should not and cannot be supported.

[macjohnny]

@webron so I guess I will remove the check for colons in #7479 and simply parse anything inside the curly brackets as the variable name, right?

[inthegarage]

@webron
Then you have a currently have situation where the swagger Spring plugin is producing valid API interfaces and the Angular Typescript one is not. The swagger plugin is supporting Spring (I don't know about others) and therefore it makes sense that everything should be consistent.
The change to typescript, was not to support these URLs, but to avoid producing invalid and uncompilable Typescript. The URL provided in the generated TS service file was matching the specifications quoted.
You can't have 1/2 the plugins accepting this format and 1/2 of them not, that makes no sense at-all.

@wing328 @macjohnny I'm disappointed and frustrated currently at this plugin. It's not produced valid code for Angular 2+ for some time now (5+months?). I've either had to delete files or replace them with my own after generation. Is this really the goal of this project?

@macjohnny your suggestion will produce invalid code again if the project is used for both spring and angular 4+.

[webron]

@macjohnny that's correct.

@inthegarage - it should be removed from all generators, not just the Spring one. I agree that supporting half and half doesn't make sense. The codegen needs to be compliant with the spec across the board.

That said - I'm not saying this cannot be supported at all. It can be supported through extensions as long as they make a compliant API definition. The ways to support extensions need to be discussed, but that's a separate topic. I'd also encourage any user affected by this to voice their opinion on the specification repo itself to help guide future versions.

[kenisteward]

@inthegarage
What do you mean by it doesn't produce valid output? Using proper specification I've had no issues generating proper yaml for my clients that are actually published to npm now
@herd/angular-client
@herd/angularjs-client
@herd/node-client

The angular client is being generated for 4.1.3 right now but I can say that we can confidently upgrade to 5.0.0 once some internal changes occur.

I also agree with @webron it doesn't make sense to support 1/2 and 1/2 and it is unfortunate it made it into other things as the wrong spec.

[macjohnny]

@kenisteward I can report that the code generated also works with Angular 5+, we are currently using it in such an environment.

[macjohnny]

Related issue: #4290

[macjohnny]

@webron @kenisteward @wing328 can we agree on merging #7479 and have a separate issue for explicitly not-supporting colons in the path variable names?

[inthegarage]

@kenisteward @macjohnny Our project is large and has to support both translating into Spring and Angular5. I dare say for more straight-forward projects that this plugin does work. However we've had several issues over the past months which are preventing us from upgrading to anywhere near the latest version.

• Issues with the old typescript-angular2 plugin that has been obsoleted.
• Issues with OpaqueToken and InjectorToken, some of which was caused by the upgrade to Angular.
• An issue I know that was fixed by William, that was necessary.
• Finally this issue that has meant we cannot use this plugin anymore as we have to support spring wildcard decorators.
  As you can see from issue 4290, that goes all the way back to November, and the outcome of this ticket means that this lack of compatibility will be dragging on even further. We've decided to bin the generated services code for client at the moment and we will proceed with our own. Hardly ideal, but there you go, we don't have much choice.

[inthegarage]

@webron @wing328 If I can assist by all means. However some of this will be with respect to Spring that is already supporting wildcard decorators. I can't imagine that they are going to remove this feature, or change it any time soon.
I see issue #4290 is still outstanding and obviously there was going to be some work around this - is it still applicable though?
Also the request by @macjohnny to merge #7479 means that we are still supporting definitions with colons in, albeit the outputted urls will not have any in. As some of test data used by that change was as follows:

/pet/{birthday}/foo/{id:.+}:

I think we need a way forward here.

[macjohnny]

@inthegarage for the time being, you could still use the custom-build jar from the sources in https://github.com/macjohnny/swagger-codegen/tree/bugfix/7476-typescript-angular-path-parameter-date to generate your client code.

[inthegarage]

@macjohnny worth a go I will see if I can get it going. Thanks.

[webron]

I'm sorry, but no. The spec changes and evolves, and a future version may support it. Until then, we cannot support it by conventional means other than extensions. It's just like matrix parameters that are supported by some languages/frameworks were not supported in OAS2 and are supported in OAS3. I understand this is inconvenient, but this tool is meant to be compliant with the spec. Any requests for changes should go to the spec itself and not here.