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

Decide on HTTP method for API v3 to create and/or update products #7667

Closed
stephanegigandet opened this issue Nov 8, 2022 · 13 comments
Closed
Labels
API v3 API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…)

Comments

@stephanegigandet
Copy link
Contributor

stephanegigandet commented Nov 8, 2022

The new API v3 (a first version will soon be deployed: #7614 ) will use the HTTP method to determine what to do.

A GET request on the path /api/v3/[barcode] will return the product.

For writing, we will use the same path /api/v3/[barcode], we could use any other method: POST, PUT or PATCH. (see link about REST apis from @alexgarel ):

alexgarel 4 days ago
On one end, I like the fact that we update and create with same interface, but I would prefer it to be explicit (at least in v3 api).

REST would say to use a POST / PUT for this, see https://restfulapi.net/rest-put-vs-post/

But we could have an "eventually_create" parameter that could be true/false, default false to specify we are ready to creation

Member
Author
@stephanegigandet stephanegigandet yesterday
I was looking into that, but PUT is intended to replace an existing document entirely, which is not what we do.

When we don't know the barcode, we could use POST on /product
When we know the barcode, ideally we should use PATCH on /product/[barcode] https://datatracker.ietf.org/doc/rfc5789/?include_text=1

I'm thinking PATCH may be a clearer way to convey what the API actually does: we never replace the product content completely with the product passed to the API, we always merge the request data with any existing data. And if the product does not exist yet, we create it.

Any thoughts?

@stephanegigandet stephanegigandet added the API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) label Nov 8, 2022
@stephanegigandet
Copy link
Contributor Author

From https://restfulapi.net/rest-put-vs-post/ : "PUT replaces the resource in its entirety. Use PATCH if request updates part of the resource."

@alexgarel
Copy link
Member

I think REST is about using a precise grammar, so I'm +1 for patch :-)

I really think respecting REST as much as possible really comes with a lot of benefits, like tools compatibility, consistency, etc.

@olivier5741
Copy link
Contributor

Totally different perspective but you could introduce the concept of product draft :)

POST /product/draft -> create a draft for non existing product
POST /product/43335279/draft -> create a draft for existing product

PUT or PATCH /product/43335279/draft/e387cev -> modify the draft

POST /product/43335279/publish/e387cev -> publish the draft as product data

GET /product/43335279 -> get the new product :)

Advantages :

  • you already have this process for producer products with OFF internal validation
  • versioning is first class citizen
  • people can contribute to official backed products, the producer can thereafter validate the data

(other aspects, since you need a lot of read flexibility, I would really consider graphQL but I realised alternative food tech sector is quite reticent to it :D. It plays really well with mongoDB, Mongo Atlas decided to solely use it ...)

@john-gom
Copy link
Contributor

john-gom commented Nov 9, 2022

I would personally avoid PUT as that implies a complete replacement of the object, but it is generally better to get people to only supply the data that they want to change (e.g. they might supply an old value for a field they aren't interested in and overwrite someone else's change).

PATCH or POST are both fine in my opinion. For patch you are basically required to define your own semantics for this, which fits in with the OpenAPI approach.

Regarding the GET operation, has any investigation been done into using GraphQL? This allows the caller to specify the specific fields and object depth they need, rather than getting everything or having to make repeated calls to get "deep" data structures. Note that GraphQL does also have scope for mutation methods, although I'm not personally a fan of their syntax for these and the POST / PATCH approach already solves the problem of only supplying the fields you want to change.

@stephanegigandet
Copy link
Contributor Author

Totally different perspective but you could introduce the concept of product draft :)

I don't really understand the intent for the drafts, for what purpose would they be used? A product might have several different unpublished drafts? That sounds like a lot of added complexity.

@stephanegigandet
Copy link
Contributor Author

Regarding GraphQL, that looks interesting indeed. The ability to request specific fields sounds like an extension of the "fields=[comma separated list of fields] system we have. But it would certainly be a lot of work, so maybe for api v4. ;)

@stephanegigandet
Copy link
Contributor Author

Thanks for all the feedback. It seems everyone is ok with PATCH, so I will change the POST requests to PATCH.

@olivier5741
Copy link
Contributor

Totally different perspective but you could introduce the concept of product draft :)

I don't really understand the intent for the drafts, for what purpose would they be used? A product might have several different unpublished drafts? That sounds like a lot of added complexity.

Maybe draft is not the right concept/name ... PATCH is good choice but what if you want someone else to validate the suggested patch (which would improve data quality) ? I also wonder what grammar you would use for PATCH, how do you delete an existing field for instance ?

@stephanegigandet
Copy link
Contributor Author

stephanegigandet commented Nov 10, 2022

I also wonder what grammar you would use for PATCH, how do you delete an existing field for instance ?

We are going to use the existing grammar: if you send a field with a non empty value, we update this field and replace its value with the one you sent. If you send a field with an empty / undef value, we delete it.

@olivier5741
Copy link
Contributor

We are going to use the existing grammar: if you send a field with a non empty value, we update this field and replace its value with the one you sent. If you send a field with an empty / undef value, we delete it.

Sounds good :) (github API does PATCH the same way). In practice it might be ok most or all the time but for consistency it might be good to prevent and remove all null and empty values from the database (found some over here ). (Undefined is not a valid JSON value)

@github-actions
Copy link
Contributor

This issue is stale because it has been open 90 days with no activity.

@github-actions github-actions bot added the ⏰ Stale This issue hasn't seen activity in a while. You can try documenting more to unblock it. label Feb 10, 2023
@alexgarel
Copy link
Member

We can close isn't it @stephanegigandet ?

@github-actions github-actions bot removed the ⏰ Stale This issue hasn't seen activity in a while. You can try documenting more to unblock it. label Feb 15, 2023
@stephanegigandet
Copy link
Contributor Author

Yes, API v3 uses PATCH for updating products.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API v3 API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…)
Projects
Archived in project
Development

No branches or pull requests

5 participants