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

Regression on PATCH requests when dealing with OneToMany link #2287

Closed
ddewaele opened this issue Jul 12, 2023 · 4 comments
Closed

Regression on PATCH requests when dealing with OneToMany link #2287

ddewaele opened this issue Jul 12, 2023 · 4 comments
Assignees
Labels
type: regression A regression from a previous release

Comments

@ddewaele
Copy link

We noticed some regression in 4.1.1 on how on PATCH requests update OneToMany links on an entity.

Given an Invoice in the database containing only 1 invoiceLine, if I execute the following PATCH request to update this invoice (including 2 invoiceLines in the payload), the request will return a 200 but only 1 invoiceLine will be updated.

curl --location --request PATCH 'http://localhost:8080/invoices/19' \
--header 'Content-Type: application/json' \
--data-raw '{
    "reference":"124_update_patch",
    "invoiceLines": [
        {
            "name":"patch_prod1_update_patch",
            "quantity":10
        },
        {
            "name":"patch_prod2_update_patch",
            "quantity":20
        }
    ]
}'
@Entity
@Data
@Builder(setterPrefix = "with")
@NoArgsConstructor
@AllArgsConstructor
public class Invoice {

    @Id
    @GeneratedValue
    private Long id;

    private String reference;

    @OneToMany(fetch = LAZY, cascade = CascadeType.ALL, orphanRemoval = true)
    @JoinColumn(name = "invoice_id")
    private Set<InvoiceLine> invoiceLines = new HashSet<>();


}
@Entity
@Data
@Builder(setterPrefix = "with")
@NoArgsConstructor
@AllArgsConstructor
public class InvoiceLine {

    @Id
    @GeneratedValue
    private Long id;

    private String name;

    private Integer quantity;

}

This is due to the following commit d6d8c50 for issue #2261

This worked fine in 4.1.0 as the break didn't occur there.

I noticed in the issue the following comment :

We now completely skip handling additional values as they don't need to be merged anyway.

Is this the semantics on how a PATCH should work with OneToManyCollections ?

I was under the impression that the PATCH method should update the invoiceLines accordingly, and we should end up with 2 invoicelines after this PATCH.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 12, 2023
@mp911de mp911de added type: regression A regression from a previous release and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 17, 2023
@godzzo
Copy link

godzzo commented Jul 20, 2023

I was under the impression that the PATCH method should update the invoiceLines accordingly, and we should end up with 2 invoicelines after this PATCH.

same here 😐️

@pariaSadatHosseiny
Copy link

same for me!

@UglyBarnacle
Copy link

same for us. we recently tried to update to spring-boot version 2.7.14 but nothiced this regression, so we reverted back to our old version.
this bug seems to happen since version 2.7.13 - versions prior to this work. I created a sample reporitory to help with this issue.

https://github.com/UglyBarnacle/rest_patch_bug.git

odrotbohm added a commit that referenced this issue Aug 30, 2023
The removal of manual collection value appendance for primitives in the context of GH-2261 lead to complex object to be appended onto a collection not being deserialized at all. This deserialization is now reinstantiated with the value to be added looked up by reading the parent node into the collections element type using a JSON Pointer expression of /$propertyName/$index. This is done to make sure that @JsonDeserialize annotations on the property kick in for the deserialization of the individual elements.

We now also shortcut the entire merge algorithm for collections that are empty, contain primitives or enums.

Fixes GH-2287.
odrotbohm added a commit that referenced this issue Aug 30, 2023
The removal of manual collection value appendance for primitives in the context of GH-2261 lead to complex object to be appended onto a collection not being deserialized at all. This deserialization is now reinstantiated with the value to be added looked up by reading the parent node into the collections element type using a JSON Pointer expression of /$propertyName/$index. This is done to make sure that @JsonDeserialize annotations on the property kick in for the deserialization of the individual elements.

We now also shortcut the entire merge algorithm for collections that are empty, contain primitives or enums.

Fixes GH-2287.
odrotbohm added a commit that referenced this issue Aug 30, 2023
The removal of manual collection value appendance for primitives in the context of GH-2261 lead to complex object to be appended onto a collection not being deserialized at all. This deserialization is now reinstantiated with the value to be added looked up by reading the parent node into the collections element type using a JSON Pointer expression of /$propertyName/$index. This is done to make sure that @JsonDeserialize annotations on the property kick in for the deserialization of the individual elements.

We now also shortcut the entire merge algorithm for collections that are empty, contain primitives or enums.

Fixes GH-2287.
@odrotbohm odrotbohm added this to the 3.7.16 (2021.2.16) milestone Aug 30, 2023
@odrotbohm
Copy link
Member

This should be in place with the latest snapshots. The tests in the sample project provided by @UglyBarnacle are back to green with those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: regression A regression from a previous release
Projects
None yet
Development

No branches or pull requests

7 participants