[dart-dio-next] Don't use encodeCollectionQueryParameter on maps

[ahmednfwela]

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master, 5.3.x, 6.0.x
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

CC: @jaumard@josh-burton@amondnet@sbu-WBT@kuhnroyal@agilob

[kuhnroyal]

There doesn't seem to be a sample for this. What are you expecting to be serialized when you pass a map as parameter?

[agilob]

What are the consequences of encoding maps and not encoding maps?

[kuhnroyal]

Well, currently it throws an error. So we can:

1. not specifically handle it (that what the current changes do)
2. somehow encode it but this probably requires full deepObject support, I don't see this happening soon

So I am fine with just not handling it and let it run through the old encodeQueryParameter. Not sure what the output is but it won't throw.

[ahmednfwela]

using encodeQueryParameter returns a raw map that is then passed to dio, and dio handles it correctly as expected in the form of:
field[key]=value

[agilob]

Would be nice to have a test or sample to persist this behaviour

[kuhnroyal]

Please attach a spec snippet for this, we need to add this to the sample spec and add tests.

[agilob]

Since encoding map is a quite common scenario, should existing spec be updated to include maps?

[kuhnroyal]

Yea, I was thinking here: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048
@wing328 What do you think?

[wing328]

Yes, please update https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048 with more test cases.

[wing328]

As discussed, let's add test cases in a separate PR as we would like to include this PR in the upcoming v5.2.1 release this weekend.

[agilob]

Opened PR #10154

[ahmednfwela]

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master, 5.3.x, 6.0.x
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

CC: @jaumard@josh-burton@amondnet@sbu-WBT@kuhnroyal@agilob

[kuhnroyal]

There doesn't seem to be a sample for this. What are you expecting to be serialized when you pass a map as parameter?

[agilob]

What are the consequences of encoding maps and not encoding maps?

[kuhnroyal]

Well, currently it throws an error. So we can:

1. not specifically handle it (that what the current changes do)
2. somehow encode it but this probably requires full deepObject support, I don't see this happening soon

So I am fine with just not handling it and let it run through the old encodeQueryParameter. Not sure what the output is but it won't throw.

[ahmednfwela]

using encodeQueryParameter returns a raw map that is then passed to dio, and dio handles it correctly as expected in the form of:
field[key]=value

[agilob]

Would be nice to have a test or sample to persist this behaviour

[kuhnroyal]

Please attach a spec snippet for this, we need to add this to the sample spec and add tests.

[agilob]

Since encoding map is a quite common scenario, should existing spec be updated to include maps?

[kuhnroyal]

Yea, I was thinking here: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048
@wing328 What do you think?

[wing328]

Yes, please update https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048 with more test cases.

[wing328]

As discussed, let's add test cases in a separate PR as we would like to include this PR in the upcoming v5.2.1 release this weekend.

[agilob]

Opened PR #10154

[ahmednfwela]

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master, 5.3.x, 6.0.x
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

CC: @jaumard@josh-burton@amondnet@sbu-WBT@kuhnroyal@agilob

[kuhnroyal]

There doesn't seem to be a sample for this. What are you expecting to be serialized when you pass a map as parameter?

[agilob]

What are the consequences of encoding maps and not encoding maps?

[kuhnroyal]

Well, currently it throws an error. So we can:

1. not specifically handle it (that what the current changes do)
2. somehow encode it but this probably requires full deepObject support, I don't see this happening soon

So I am fine with just not handling it and let it run through the old encodeQueryParameter. Not sure what the output is but it won't throw.

[ahmednfwela]

using encodeQueryParameter returns a raw map that is then passed to dio, and dio handles it correctly as expected in the form of:
field[key]=value

[agilob]

Would be nice to have a test or sample to persist this behaviour

[kuhnroyal]

Please attach a spec snippet for this, we need to add this to the sample spec and add tests.

[agilob]

Since encoding map is a quite common scenario, should existing spec be updated to include maps?

[kuhnroyal]

Yea, I was thinking here: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048
@wing328 What do you think?

[wing328]

Yes, please update https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048 with more test cases.

[wing328]

As discussed, let's add test cases in a separate PR as we would like to include this PR in the upcoming v5.2.1 release this weekend.

[agilob]

Opened PR #10154

[ahmednfwela]

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master, 5.3.x, 6.0.x
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

CC: @jaumard@josh-burton@amondnet@sbu-WBT@kuhnroyal@agilob

[kuhnroyal]

There doesn't seem to be a sample for this. What are you expecting to be serialized when you pass a map as parameter?

[agilob]

What are the consequences of encoding maps and not encoding maps?

[kuhnroyal]

Well, currently it throws an error. So we can:

1. not specifically handle it (that what the current changes do)
2. somehow encode it but this probably requires full deepObject support, I don't see this happening soon

So I am fine with just not handling it and let it run through the old encodeQueryParameter. Not sure what the output is but it won't throw.

[ahmednfwela]

using encodeQueryParameter returns a raw map that is then passed to dio, and dio handles it correctly as expected in the form of:
field[key]=value

[agilob]

Would be nice to have a test or sample to persist this behaviour

[kuhnroyal]

Please attach a spec snippet for this, we need to add this to the sample spec and add tests.

[agilob]

Since encoding map is a quite common scenario, should existing spec be updated to include maps?

[kuhnroyal]

Yea, I was thinking here: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048
@wing328 What do you think?

[wing328]

Yes, please update https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048 with more test cases.

[wing328]

As discussed, let's add test cases in a separate PR as we would like to include this PR in the upcoming v5.2.1 release this weekend.

[agilob]

Opened PR #10154

[ahmednfwela]

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master, 5.3.x, 6.0.x
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

CC: @jaumard@josh-burton@amondnet@sbu-WBT@kuhnroyal@agilob

[kuhnroyal]

There doesn't seem to be a sample for this. What are you expecting to be serialized when you pass a map as parameter?

[agilob]

What are the consequences of encoding maps and not encoding maps?

[kuhnroyal]

Well, currently it throws an error. So we can:

1. not specifically handle it (that what the current changes do)
2. somehow encode it but this probably requires full deepObject support, I don't see this happening soon

So I am fine with just not handling it and let it run through the old encodeQueryParameter. Not sure what the output is but it won't throw.

[ahmednfwela]

using encodeQueryParameter returns a raw map that is then passed to dio, and dio handles it correctly as expected in the form of:
field[key]=value

[agilob]

Would be nice to have a test or sample to persist this behaviour

[kuhnroyal]

Please attach a spec snippet for this, we need to add this to the sample spec and add tests.

[agilob]

Since encoding map is a quite common scenario, should existing spec be updated to include maps?

[kuhnroyal]

Yea, I was thinking here: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048
@wing328 What do you think?

[wing328]

Yes, please update https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048 with more test cases.

[wing328]

As discussed, let's add test cases in a separate PR as we would like to include this PR in the upcoming v5.2.1 release this weekend.

[agilob]

Opened PR #10154

[ahmednfwela]

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master, 5.3.x, 6.0.x
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

CC: @jaumard@josh-burton@amondnet@sbu-WBT@kuhnroyal@agilob

[kuhnroyal]

There doesn't seem to be a sample for this. What are you expecting to be serialized when you pass a map as parameter?

[agilob]

What are the consequences of encoding maps and not encoding maps?

[kuhnroyal]

Well, currently it throws an error. So we can:

1. not specifically handle it (that what the current changes do)
2. somehow encode it but this probably requires full deepObject support, I don't see this happening soon

So I am fine with just not handling it and let it run through the old encodeQueryParameter. Not sure what the output is but it won't throw.

[ahmednfwela]

using encodeQueryParameter returns a raw map that is then passed to dio, and dio handles it correctly as expected in the form of:
field[key]=value

[agilob]

Would be nice to have a test or sample to persist this behaviour

[kuhnroyal]

Please attach a spec snippet for this, we need to add this to the sample spec and add tests.

[agilob]

Since encoding map is a quite common scenario, should existing spec be updated to include maps?

[kuhnroyal]

Yea, I was thinking here: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048
@wing328 What do you think?

[wing328]

Yes, please update https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048 with more test cases.

[wing328]

As discussed, let's add test cases in a separate PR as we would like to include this PR in the upcoming v5.2.1 release this weekend.

[agilob]

Opened PR #10154

[ahmednfwela]

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master, 5.3.x, 6.0.x
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

CC: @jaumard@josh-burton@amondnet@sbu-WBT@kuhnroyal@agilob

[kuhnroyal]

There doesn't seem to be a sample for this. What are you expecting to be serialized when you pass a map as parameter?

[agilob]

What are the consequences of encoding maps and not encoding maps?

[kuhnroyal]

Well, currently it throws an error. So we can:

1. not specifically handle it (that what the current changes do)
2. somehow encode it but this probably requires full deepObject support, I don't see this happening soon

So I am fine with just not handling it and let it run through the old encodeQueryParameter. Not sure what the output is but it won't throw.

[ahmednfwela]

using encodeQueryParameter returns a raw map that is then passed to dio, and dio handles it correctly as expected in the form of:
field[key]=value

[agilob]

Would be nice to have a test or sample to persist this behaviour

[kuhnroyal]

Please attach a spec snippet for this, we need to add this to the sample spec and add tests.

[agilob]

Since encoding map is a quite common scenario, should existing spec be updated to include maps?

[kuhnroyal]

Yea, I was thinking here: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048
@wing328 What do you think?

[wing328]

Yes, please update https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048 with more test cases.

[wing328]

As discussed, let's add test cases in a separate PR as we would like to include this PR in the upcoming v5.2.1 release this weekend.

[agilob]

Opened PR #10154

[ahmednfwela]

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master, 5.3.x, 6.0.x
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

CC: @jaumard@josh-burton@amondnet@sbu-WBT@kuhnroyal@agilob

[kuhnroyal]

There doesn't seem to be a sample for this. What are you expecting to be serialized when you pass a map as parameter?

[agilob]

What are the consequences of encoding maps and not encoding maps?

[kuhnroyal]

Well, currently it throws an error. So we can:

1. not specifically handle it (that what the current changes do)
2. somehow encode it but this probably requires full deepObject support, I don't see this happening soon

So I am fine with just not handling it and let it run through the old encodeQueryParameter. Not sure what the output is but it won't throw.

[ahmednfwela]

using encodeQueryParameter returns a raw map that is then passed to dio, and dio handles it correctly as expected in the form of:
field[key]=value

[agilob]

Would be nice to have a test or sample to persist this behaviour

[kuhnroyal]

Please attach a spec snippet for this, we need to add this to the sample spec and add tests.

[agilob]

Since encoding map is a quite common scenario, should existing spec be updated to include maps?

[kuhnroyal]

Yea, I was thinking here: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048
@wing328 What do you think?

[wing328]

Yes, please update https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1048 with more test cases.

[wing328]

As discussed, let's add test cases in a separate PR as we would like to include this PR in the upcoming v5.2.1 release this weekend.

[agilob]

Opened PR #10154