dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.3k stars 9.97k forks source link

JsonPath for private properties #51514

Open denis-tsv opened 11 months ago

denis-tsv commented 11 months ago

Is there an existing issue for this?

Is your feature request related to a problem? Please describe the problem.

I created class with private property setter (DDD style) and use it for JsonPath. I received error "The property at path 'Name' could not be updated."

public class Product
{
    public string Name { get; private set; }
}

[ApiController]
[Route("[controller]")]
public class WeatherForecastController : ControllerBase
{
    [HttpPatch]
    public void Patch([FromBody] JsonPatchDocument<Product> p)
    {
        var prod = new Product();
        p.ApplyTo(prod);
    }
}

Describe the solution you'd like

I'd like to update property value when property setter is private. DDD approach and RichModel is very popular now, a lot of teams creates encapsulated model classes with private setters. Other JsonPath implementations allows to initialize properties with private setters. PocoAdapter class can be fixed

Additional context

No response

captainsafia commented 11 months ago

@denis-tsv Thanks for filing this issue!

I don't think the principles of DDD and JsonPatch play super well here. What other implementations of JsonPatch have you seen that support this pattern?

ghost commented 11 months ago

Hi @denis-tsv. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

denis-tsv commented 11 months ago

JsonPatch allows to initialize properties with private setters

captainsafia commented 11 months ago

Ah, interesting design decision. Admittedly, I'm not inclined to change the behavior here at the moment. It would be a breaking change, and in a rather surprising direction since we're giving JSON patch visibility into properties it would otherwise not have.

I'll move this to the backlog for now with the option to consider if there is overwhelming interest in this.

ghost commented 11 months ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.