Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serialization: Refactor Host constructor and tests #383

Conversation

Glutexo
Copy link
Collaborator

@Glutexo Glutexo commented Jul 31, 2019

The logic of creating a Host model from JSON dict has been scattered between the Host DB model constructor and the deserialization method. Moved everything to the deserializer without changing any logic.

Refactored the (de)serialization tests to not work around the bugs and quirks of the Host model constructor:

  • Invalid default values for account and display_name
  • Inability to provide all fields as keyword arguments (id, created_on, modified_on)

Added tests for the ability to fill an empty string into ansible_host. This is no longer done by a common trusted method _update_ansible_host. It is tested both on serialization and deserialization to verify that the value remains unchanged through the whole process.

Made the (de)serialization tests hopefully more straightforward and readable along the way.

This pull request is a step towards decoupling the logic from the database model. #309 Still some quirks remain like validation in the deserialization method.

@Glutexo Glutexo requested review from jhjaggars, dehort and Jason-RH July 31, 2019 12:11
@Glutexo Glutexo self-assigned this Jul 31, 2019
@Glutexo Glutexo force-pushed the host_constructor branch from 892ee6b to 9513d69 Compare July 31, 2019 12:38
@Glutexo Glutexo marked this pull request as ready for review July 31, 2019 14:13
@Glutexo Glutexo force-pushed the host_constructor branch from e7bef70 to 8cfe451 Compare July 31, 2019 14:20
The logic of creating a Host model from JSON dict has been scattered
between the Host DB model constructor and the deserialization method.
Moved everything to the deserializer without changing any logic.

Refactored the (de)serialization tests to not work around the bugs and
quirks of the Host model constructor:

* Invalid default values for account and display_name
* Inability to provide all fields as keyword arguments

Added tests for the ability to fill an empty string into ansible_host.
This is no longer done by a common trusted method.
@Glutexo
Copy link
Collaborator Author

Glutexo commented Aug 1, 2019

Rebased to fix conflicts.

@Glutexo Glutexo changed the title Refactor Host constructor and tests Serialization: Refactor Host constructor and tests Oct 11, 2019
@jharting jharting added the conflict needs to be resolved and rebased label Nov 1, 2019
@Glutexo Glutexo closed this Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflict needs to be resolved and rebased
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants