Fix #4857 typescript-angular2 client deserialization failed due to change of property name
PR checklist
- [x] Read the contribution guidelines.
- [x] Ran the shell/batch script under
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates) - [x] Filed the PR against the correct branch: master for non-breaking changes and
2.3.0branch for breaking (non-backward compatible) changes.
Description of the PR
Fixes issue #4857 Deserialization failed due to change of property name for the typescript-angular2 client, re-using the solution in PR #4264:
- adds a property name map to each model, turning them from an interface into a class
- adds global objects tracking created classes and enums
- adds a class, ObjectSerializer, responsible for translating property names (and serializing/deserializing dates), and makes the generated api use this class at serialization/deserialization.
Breaks backward compatibility I think, because we change the models from interfaces to classes, which will cause issues for users reusing the models in their code and assigning object literals (because of the added constructor / properties). If that is undesirable, I'm happy to refactor the property name maps into a separate entity, leaving the models as interfaces.
Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/swagger-api/swagger-codegen/graphs/contributors.
Let me know if you need help fixing it.
Ref: https://github.com/swagger-api/swagger-codegen/wiki/FAQ#how-can-i-update-commits-that-are-not-linked-to-my-github-account
cc @Vrolijkx @taxpon @sebastianhaas @kenisteward @TiFu
Thanks for pointing that out! Somehow I hadn't added the e-mail address I use for commits to my GitHub account...
I will take at look at this on weekend.
I'm also having second thoughts about adding the property name maps directly in the models, because (if I understand this correctly) they are exposed for the users to reuse in their own code.
Since the property name maps are there just to enable the ObjectSerializer to function correctly, it seems to me that it would make sense to separate them from the models proper.
Any thoughts?
Here are the changes I would do. I'll revert the commits if you think we should leave the property name maps as they currently are, in the model class.
IMO the whole serializer should be hidden from the user. It is an implementation detail of the API module and the user should not rely on this functionality. This enables us to change it without breaking the code of users. The user should only interact with the model objects and the api objects.
@tifu I second this
On Mon, Jul 17, 2017, 6:14 PM Tino Fuhrmann [email protected] wrote:
IMO the whole serializer should be hidden from the user. It is an implementation detail of the API module and the user should not rely on this functionality. This enables us to change it without breaking the code of users. The user should only interact with the model objects and the api objects.
— You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/swagger-api/swagger-codegen/pull/6024#issuecomment-315900658, or mute the thread https://github.com/notifications/unsubscribe-auth/AMPLtftH9wFtE2Ds8dQtfqUN6uEe0PPyks5sO9ysgaJpZM4OUW9E .
Is this PR still being developed?