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.

Sign up for GitHub

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

Jump to bottom

Patch method broken for JPA repositories #269

Closed
remmeier opened this issue Dec 19, 2016 · 5 comments
Closed

Patch method broken for JPA repositories #269

remmeier opened this issue Dec 19, 2016 · 5 comments
Assignees
@remmeier
Labels
bug

Comments

@remmeier
Copy link
Contributor

  • recent change in resourceUpsert to use repository.find and patch existing resources leads to problem with detached entities (that PR was ok, just JPA needs to be updated as well).
  • KatharsisClient currently pushes always, that flag needs to be disabled as it was hiding the problem.
@remmeier remmeier self-assigned this Dec 19, 2016
@remmeier
Copy link
Contributor Author

it is a bit of an annoying problem. The reason the detaching is necessary is because IncludeLookupSetter further modifies the resources before serializing and returning them to the client. This lookup of inclusions should not be done on the POJOs, but on a structure representing the JSON API response document. Which would imply a little bit of cleanup in the serialization part (which is overdue).

@Ramblurr
Copy link
Contributor

Ramblurr commented Dec 19, 2016 •
edited
Loading

KatharsisClient currently pushes always, that flag needs to be disabled as it was hiding the problem.

What do you mean by that?

@remmeier
Copy link
Contributor Author

sorry, I meant POST instead of PUSH. The flag is also wrongly named "pushAlways" instead of "postAlways". Until a couple of weeks ago there was only I "save" method on the resource repository. In the V2 version there is now a "create" and "save". So it becomes possible to distinguish "POST" and "PATCH". Before that it just did a POST. This thing now just needs to be enabled.

The general issue is a bit more complicated due to the flaws in the current serialization handling. I looked into all that and think I will have a solution tomorrow that does a first cleanup in this area by having a proper representation of a JSON API document and moving the logic out of the serializers.

remmeier added a commit that referenced this issue Dec 19, 2016
#269 serializer refactoring
11029de 
introduced Resource, Document, ResourceId, Relationship classes to hold
a JSON API request/response.

Moved all the logic out of the serializers to those classes.

Updated test cases (work in progress)
@chb0github chb0github mentioned this issue Dec 21, 2016
Merged
@masterspambot
Copy link
Member

@remmeier will this be solved by #271? If so, close this issue as duplicate.

chb0github pushed a commit that referenced this issue Jan 5, 2017
@chb0github
#269 Serialization refactoring (#271)
53a8962 
* Update changelog

* Space in POM version was causing error

* Update changelog

* [maven-release-plugin] prepare release katharsis-build-2.8.2

* [maven-release-plugin] prepare for next development iteration

* #269 serializer refactoring

introduced Resource, Document, ResourceId, Relationship classes to hold
a JSON API request/response.

Moved all the logic out of the serializers to those classes.

Updated test cases (work in progress)

* updated test cases

* testing fixes

* trying to fix travis

* serializer/deserializer

* object <-> document conversion implemented

* testing

* mapper testing + inclusion cleanup

* inclusion refactoring & testing

* working backend, client pending

* client migrated to new serialization api

* - more robust resource field lookup (in the context of polymorphism)
- annotation based repository support (proper test cases in this area
seem to be non-existing)

* various updates based on reviews

* refactored JsonAnySetter support

* minor code update

* further cleanups:

- removed unused class ResourceDigest
- inheritance serialization fix
- fixed ClientProxyFactory contract (removed unused url)
- catch cleanup in ResourceAttributesBridge
@chb0github
Copy link
Contributor

@remmeier ? Should we close this or no?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@remmeier remmeier

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Ramblurr @masterspambot @chb0github @remmeier