[ALL] [3.0] Body param is now first in parameters list, used to be last

[ivan-gomes]

Description

When an operation has multiple types of parameters, i.e. query and body, the order of these parameters provided to the code generators in 2.x was such that body was last. This resulted in the generated clients to place the body argument after all the query arguments.

Example:

public Artifacts deleteArtifactsInBatch(String projectId, String refId, Artifacts body) throws ApiException {

In 3.0, the order has been changed such that body is first, before query parameters - verified with -DdebugOperations=true.

Example:

public Artifacts deleteArtifactsInBatch(Artifacts body, String projectId, String refId) throws ApiException {

This results in an API breakage in any language where argument order is used.

Swagger-codegen version

3.0.4 and 3.0.5-SNAPSHOT

Swagger declaration file content or url

  /projects/{project_id}/refs/{ref_id}/artifacts:
    parameters:
    - name: "project_id"
      in: "path"
      required: true
      description: "project identifier"
      type: "string"
    - name: "ref_id"
      in: "path"
      required: true
      description: "ref identifier"
      type: "string"
    delete:
      tags:
      - "artifact"
      summary: "Delete artifact(s) in batch"
      description: ""
      operationId: "deleteArtifactsInBatch"
      parameters:
      - in: "body"
        name: "body"
        description: ""
        schema:
          $ref: "#/definitions/Artifacts"
        required: true

Command line used for generation

java -jar swagger-codegen-cli-3.0.4.jar generate -i /path/to/swagger.yaml -l java -o /var/tmp/java-api-client -DdebugOperations=true

Steps to reproduce

See above.

Related issues/PRs

Suggest a fix/enhancement

The ordering of operations.contents.parameters (and perhaps allParams to ease the migration) in 3.0 should be made consistent with that in 2.x, namely putting body last. This should result in code generators for all languages maintaining the same order as it did in 2.x.

[skrysmanski]

It should be noted that this only happens for "none-form-data" bodies - as form-data bodies are "exploded" into individual parameters.

[skrysmanski]

The cause of this problem is that in DefaultCodegenConfig.fromOperation() request bodies are processed before operation parameters.

"Unfortunately", the class DefaultCodegenConfig is not part of this repository but rather of swagger-codegen-generators. I'm not sure whether this issue should be moved - maybe @HugoMario can shed some light on this?

[HugoMario]

you're right @skrysmanski about why body params are placed as first parameters. i could fix this, but would like to discuss if this is really a problem.

[skrysmanski]

In principle, I agree with @ivan-gomes that the parameter order should not change between v2 and v3, if possible.

Unfortunately, we also have to look at reality here and there may already be projects out there that expect the body param to be the first. Changing this would be a breaking change for them.

The most sensible solution would be to make this configurable; however I'm not sure whether the amount of work required to implement something like this would be justified.

[HugoMario]

@skrysmanski , i like the idea to make this configurable, we could add an option on codegen or use a vendor extension for that. This last would be quicker for me.

[cricketer]

There might be a misunderstanding with the generated code?
In my case, the parameter types are correct, but the first parameter is always named body instead of the type name as in v2.

v2: public changePasswordUsingPOST(passwordChangeDto: PasswordChangeDTO, observe?: 'body', reportProgress?: boolean): Observable<any>;
v3: public changePasswordUsingPOST(body: PasswordChangeDTO, observe?: 'body', reportProgress?: boolean): Observable<any>;

[JavascriptMick]

+1 for this. in cases where the query param specifies the resource upon which to act and the requestBody contains information about what to do to it... it just makes more sense to put the query params first. Also, this was the previous behaviour. Also +1 to @cricketer comment above, you used to be able to name the 'body' parameter and now it appears we are stuck with 'body' as the parameter name. This is poor because it is allowing the implementation (http) to leak into the code api

[a-en]

In scope of the question of parameters ordering -- right now with generator version 2.2.2 we have parameters of the generated method listed in exactly the same order they go in the source specification. With upgrade to version 3.x.x it doesn't work that way anymore. And I don't see they way to fix this: when conversion to io.swagger.v3.oas.models.Operation from io.swagger.models.Operation happens, body param is not added to the list of operation's parameters. So that it is not possible to restore original order in DefaultCodegenConfig.fromOperation() method.

@HugoMario could you take a look & confirm if this is right interpretation? And, probably, propose a way to have same behavior as 2.2.2 version

[vithu30]

Is there any update on this issue? Will it be made as configurable in future release? It is affecting the users who is migrating from older versions

[Plonk42]

Hey, any update on this one? Can we at least get a programmatic workaround to get back to the previous ordering? It's breaking all our existing consumers of the generated library :(

[prateekshashashidhar]

+1 Is there any workaround for this?

[Plonk42]

If it can help people, here is the workaround I came with to generate my Python library. It set the path params first, then the body param, then the query params. You need to edit the comparator if you have more params type (form, header...).

	private void generatePythonLib(OpenAPI api) {
	
		CodegenConfig config = new PythonClientCodegen() {
		
			// workaround for https://github.com/swagger-api/swagger-codegen/issues/9103
			// path params first, then body, then query params
			Comparator<CodegenParameter> comp = new Comparator<CodegenParameter>() {
				@Override
				public int compare(CodegenParameter p1, CodegenParameter p2) {
					if (p1.getIsPathParam()) {
						if (p2.getIsPathParam())
							return 0;
						else
							return -1;
					} else if (p1.getIsBodyParam()) {
						if (p2.getIsPathParam())
							return 1;
						else
							return -1;
					} else if (p1.getIsQueryParam()) {
						if (p2.getIsQueryParam())
							return 0;
						else
							return 1;
					}
					return 0;
				}
			};

			@Override
			public Map<String, Object> postProcessOperations(Map<String, Object> objs) {
				Map<String, Object> operations = (Map<String, Object>) objs.get("operations");
				List<CodegenOperation> operation = (List<CodegenOperation>) operations.get("operation");
				for (CodegenOperation op : operation) {
					for (CodegenContent content : op.getContents()) {
						List<CodegenParameter> params = content.getParameters();
						if (!params.isEmpty()) {
							// sort with our custom comparator ...
							params.sort(comp);
							// ... then restore "x-has-more" flag for all params
							params.forEach(p -> p.getVendorExtensions().put(CodegenConstants.HAS_MORE_EXT_NAME, true));
							params.get(params.size() - 1).getVendorExtensions().put(CodegenConstants.HAS_MORE_EXT_NAME, false);
						}
					}
				}
				return super.postProcessOperations(objs);
			}

		};

		ClientOpts opts = new ClientOpts();
		ClientOptInput input = new ClientOptInput()
				.config(config)
				.opts(opts)
				.openAPI(api);

		new DefaultGenerator()
				.opts(input)
				.generate();
	}