[Python] Correct sanitize_for_serialization in python generator for list in oneOf schema elements

[karge-itestra]

The method did not consider the objects created for oneOf schemata. If one of the cases was a list, to_dict would return it instead of something that has an items() method.

This should be reviewed by members of the python technical committee:
@cbornet @tomplus @krjakbrjak @fa0311 @multani

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/configs/*.yaml
  ./bin/utils/export_docs_generators.sh
  

  (For Windows users, please run the script in Git BASH)
  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*.
  IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
• File the PR against the correct branch: master (upcoming 7.6.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

[wing328]

thanks for the PR

please follow step 3 to regenerate the samples so that CI can verify the change

[karge-itestra]

thanks for the PR

You´re welcome :)

please follow step 3 to regenerate the samples so that CI can verify the change

Done, compiling took a long time but it and the sample generation lead to the desired result. I am not sure what should happen in the export_docs_generators.sh, it did not change anything in the repo

[wing328]

I wonder if you can add a test or 2 in

openapi-generator/samples/openapi3/client/petstore/python/tests/test_api_client.py

Line 184 in 7d59c0e

def test_sanitize_for_serialization_decimal(self):

[karge-itestra]

I wonder if you can add a test or 2 in

openapi-generator/samples/openapi3/client/petstore/python/tests/test_api_client.py

Line 184 in 7d59c0e

def test_sanitize_for_serialization_decimal(self):

The problem I see there is that the pet store example schema doesn´t contain a oneOf part. Adding that or creating a new test exceeds my disposable time. If you provide a stub or two (maybe modeled after the referenced issue #18106), I can implement it.

[denys999]

@wing328 @karge-itestra can we merge?

[denys999]

it is already 7.12.0

[denys999]

@wing328 @karge-itestra
Can you also include something similar for model_oneof.mustache?

        # handle list
        if isinstance(self.actual_instance, list):
            return [item.to_dict() for item in self.actual_instance]

[karge-itestra]

@denys999 I have never worked with Mustache, but I can take a look. I would have liked to merge much earlier, but @wing328 asked for tests that I didn´t have the time to design, because the schemas that the tests used lacked cases for testing the behavior, and adding them is much more effort than only writing a testcase.

[ChristophMatthies]

Hi everyone, thanks for taking a look at this!
@denys999 we'd be very happy to have the fix for the model_oneof.mustache that you proposed.

Any updates on this or some way that I can help out?

[wing328]

@ChristophMatthies did you test the fix locally using customized templates (e.g. -t via cli)?

[karge-itestra]

If that helps you @wing328 , I did test that on a real-world usecase which I cannot disclose. Probably updating the API Specs for the tests or adding a new one that includes the case from the issue would be the easiest way to achieve convincing coverage.

[wing328]

👌

will get it merged and see if anyone from the community can contribute a test or 2 later