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

Fix validation in the case a newer release dropped the target entity of a required relation #216

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

NielsCW
Copy link
Contributor

@NielsCW NielsCW commented Apr 17, 2024

Add support to fix:

  • GET requests to existing entities (with required relation removed) result in http 200 instead of http 404
  • PUT requests to existing entities (with required relation removed) result in http 200 instead of http 400
  • PATCH requests to existing entities (with required relation removed) result in http 200 instead of http 400

Not supported in this PR:

  • POST request to create entity (with required relation removed) always fail with http 400

Side effects:

  • PATCH request to existing entities where required relation was not removed now result in http 400 database constraint violation instead of http 400 validation failed when using an invalid relation payload (because PATCH doesn't ignore relations unlike PUT)

@NielsCW NielsCW requested a review from a team as a code owner April 17, 2024 15:19
Copy link

sonarcloud bot commented Apr 17, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@@ -34,7 +35,7 @@ public class Order {
@JsonProperty(access = Access.READ_ONLY)
private UUID id;

@ManyToOne
@ManyToOne(fetch = FetchType.LAZY)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think cg-apps have FetchType.LAZY for @ManyToOne relations right now ? why are we changing this here ?

@vierbergenlars
Copy link
Member

This is an approach to try to solve some compatibility problems when running an older application on a newer codebase, but it doesn't cover all cases.

I think we should investigate a different approach, by actually having the database compatibility shims behave more closely to the old behavior of the database (being: actually keeping the old data around for the old application)

@NielsCW NielsCW marked this pull request as draft May 8, 2024 09:37
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.

None yet

3 participants