Skip to content

Model manager changes #88

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

Merged
merged 2 commits into from
Mar 12, 2015
Merged

Model manager changes #88

merged 2 commits into from
Mar 12, 2015

Conversation

csantero
Copy link
Collaborator

@csantero csantero commented Mar 6, 2015

Summary of changes:

  1. ModelManager now discovers everything it needs to know about a type's properties when that type is registered, rather than when the type is first (de)serialized. Closes ModelManager caches should be populated at registration time #82
  2. GetProperties() now returns a wrapper class for PropertyInfo called ModelProperty, which stores the PropertyInfo as well as the serialized json key and whether the property should be ignored by default in serialization. This class is abstract and has two child classes: FieldModelProperty and RelationshipModelProperty. The latter tracks the cardinality of the relationship as well as the related type.
  3. GetPropertyForJsonKey() returns a ModelProperty.
  4. GetJsonKeyForProperty is gone from IModelManager. Any code that was using this can use the field on ModelProperty instead.
  5. You can now override a property's json key by adding a [JsonProperty] attribute. Closes Allow overriding a property's serialization json key #87
  6. Attempting to filter by an unknown field will return a 400 error, rather than performing no filter.
  7. Models that use [UseAsId] to specify the id property no longer serialize the property both as id and as the property's formatted name. I feel that this is a bad practice due to not being compatible with a 1:1 mapping of json key to property, and would require special handling to make work given these new changes. Furthermore, ember data would probably squawk if you change a regular field and the server's response comes back with a new id. If a user really wants this they can accomplish it by adding a surrogate Id field in their model class, removing [UseAsId], and ensuring that the two are the same.

This PR should improve performance and make finding bugs in one's models easier, by moving several calculations to registration time that previously didn't happen until requests are made. It also puts the model manager in charge of some concerns that used to be in the formatter's domain, but really ought to be part of the registration process. Most significantly, the formatter no longer needs to figure out whether a property is for a field or a relationship every time that property has to be serialized.

@SphtKr Do you have any feedback before I merge? I have some more work I want to get in that depends on this, but I want to make sure this meets your approval first.

csantero added a commit that referenced this pull request Mar 12, 2015
@csantero csantero merged commit f7e0a30 into JSONAPIdotNET:0-4-0 Mar 12, 2015
@csantero csantero deleted the mm-caches branch March 12, 2015 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant