Skip to content

DefaultEntityRepository: inconsistent data handling #519

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

Closed
maurei opened this issue Jun 7, 2019 · 0 comments
Closed

DefaultEntityRepository: inconsistent data handling #519

maurei opened this issue Jun 7, 2019 · 0 comments

Comments

@maurei
Copy link
Member

maurei commented Jun 7, 2019

This issue is related to #502 in the sense that it's also about cleaning up the default repository layer

Issue:
in CreateAsync the values that are stored in the database are a direct result of the values of the properties of the to-be-created. This makes sense, because in the service layer, the following call is made: await _repository.CreateAsync(entity), and intuitively, any changes that are made on entity in eg a custom service will be included by the repository create action.

The UpdateAsync method is called in a very similar way: as await _repository.UpdateAsync(id, entity) and therefore one would expect roughly the same idea. However, the entity parameter that is being passed is not used at all, and therefore, any manual changes (in a custom service) that are done on this object do not affect the database at all.

This is a problem:

  • This is confusing because it suggests differently, as explained with the CreateAsync approach.
  • It is also internally inconsistent: in the UpdateAsync method, all to-be-updated values and relationships are retrieved from IJsonApiContext and pushed to the database, without using this entity parameter, whereas in CreateAsync the values on the actualy entity parameter dictate what is pushed to the database.
  • Any changes done on the entity parameter through resource hooks in the case of UpdateAsync are lost. This beats the purpose of being able to use business logic.

I think UpdateAsync should be changed to also work on the actual entity parameter. It is still better to first get the current state of the entity (oldEntity), make changes on this and then SaveChanges, because this will result in more efficient queries, but the values to put on oldEntity should be retrieved from the entity parameter, not IJsonApiContext.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant