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

Add support for API v3 Product WRITE requests to add / edit packaging components #617

Closed
Tracked by #515 ...
stephanegigandet opened this issue Nov 8, 2022 · 58 comments · Fixed by #640 or #666
Closed
Tracked by #515 ...

Comments

@stephanegigandet
Copy link
Contributor

The new API v3 to add/edit packaging components is almost finalized and implemented (but deployed only on the .dev server currently).

Server-side PR with the documentation and implementation: openfoodfacts/openfoodfacts-server#7614

There could still be minor changes (in particular, it is likely that we will use PATCH requests instead of POST requests). I will update this bug when it is merged, deployed on .net and then on .org
If you have feedback about the API, please let me know, we can still change things if needed.

Note that future v3 APIs will use the same model for reporting warnings and errors etc. so it would be good to have generic code for it. In particular, the API v3 for product READ is already implemented as well (with full v2 features support).

The API v3 product WRITE currently only supports packaging data, so we will need to keep the existing code for API v2 for some time.

@teolemon
Copy link
Member

teolemon commented Nov 8, 2022

curl -X POST https://off:off@world.openfoodfacts.dev/api/v3/product/12345678 -H "Content-Type: application/json" -d '{"product": { "packagings_add": [{"shape": "bottle"}]}, "fields": "updated"}'
{"errors":[],"product":{"packagings":[{"shape":"en:bottle"}]},"warnings":[{"field":{"default_value":"en","id":"tags_lc"},"impact":{"id":"warning"},"message":{"id":"missing_tags_lc"}},{"field":{"id":"number_of_units"},"impact":{"id":"field_ignored"},"message":{"id":"packaging_component_missing_number_of_units"}},{"field":{"id":"recycling","value":null},"impact":{"id":"field_ignored"},"message":{"id":"packaging_component_missing_recycling"}},{"field":{"id":"material","value":null},"impact":{"id":"field_ignored"},"message":{"id":"packaging_component_missing_material"}}]}

@teolemon
Copy link
Member

teolemon commented Nov 8, 2022

cc @monsieurtanuki @M123-dev @g123k

@monsieurtanuki monsieurtanuki self-assigned this Nov 9, 2022
@monsieurtanuki
Copy link
Contributor

@teolemon I've managed to send the following query to dev from off-dart:

      {
        "tags_lc": "fr",
        "product": {
          "packagings_add": [
            {
              "shape": "bottle",
              "material": "glass",
              "recycling": "recycler",
              "number_of_units": 1,
            }
          ],
        },
        "fields": "updated",
      }

In trial and error mode, I've got this reply from the server:

{
	"errors": [],
	"product": {
		"packagings": [{
			"material": "en:glass",
			"number_of_units": 1,
			"recycling": "fr:recycling",
			"shape": "en:bottle"
		}, {
			"material": "en:glass",
			"number_of_units": 1,
			"recycling": "fr:recyclage",
			"shape": "en:bottle"
		}, {
			"material": "en:glass",
			"number_of_units": 1,
			"recycling": "en:recycle",
			"shape": "en:bottle"
		}]
	},
	"status": "success",
	"warnings": []
}

@stephanegigandet Still, I have some questions:

  1. obviously I haven't always put a correct value for recycle and now I have duplicates.
    1. how can I say "consider that I give you everything in one shot and not something additional?"
    2. is it possible to remove packaging items from the server, in order to clean this mess?
  2. not sure how to play with languages: I've tried with "tags_lc": "fr" but could not see messages (e.g. errors or warnings) in French. How can I get localized messages?

@stephanegigandet
Copy link
Contributor Author

  1. how can I say "consider that I give you everything in one shot and not something additional?"
  2. is it possible to remove packaging items from the server, in order to clean this mess?

Instead of packagings_add, you can send the same structure in the packagings field, and it will replace the existing structure.

"packagings": [
            {
              "shape": "bottle",
              "material": "glass",
              "recycling": "recycler",
              "number_of_units": 1,
            }
          ],
  • not sure how to play with languages: I've tried with "tags_lc": "fr" but could not see messages (e.g. errors or warnings) in French. How can I get localized messages?

tags_lc is the language that you use in shape, materials etc. (if you send something like "bouteille", if you send the taxonomy id en:bottle, then it's not used)

you can send the "lc" field to get localized warnings and errors messages, although as they are not translated yet, you will get English

@stephanegigandet
Copy link
Contributor Author

There is one version of the doc here: https://redocly.github.io/redoc/?url=https://raw.githubusercontent.com/openfoodfacts/openfoodfacts-server/packagings-api-2/docs/reference/api-v3.yml#operation/post-api-v3-product-barcode

But the OpenAPI file to documentation is not showing the product fields unfortunately. :-(

@monsieurtanuki
Copy link
Contributor

Thank you @stephanegigandet for your answers!

In the context of Smoothie:

  • I cannot quite picture the use-case of "adding" packaging, instead of merely "replacing" the whole packaging, therefore in a first approach I will only implement packagings (and not packagings_add) (for the record, I've just tried with packagings and indeed it replaces everything)
  • Regarding languages, I guess the typical parameter values would be the same for tags_lc and lc.
  • For the moment I see no other solution than creating a new method just to set a product packaging with this new modus operandi. The other fields will use the "old" method, until they are available too in v3.

@monsieurtanuki
Copy link
Contributor

Btw it's very convenient to be able to get the resulting Product without having to run an additional "get product" query :)

@stephanegigandet
Copy link
Contributor Author

  • I cannot quite picture the use-case of "adding" packaging, instead of merely "replacing" the whole packaging, therefore in a first approach I will only implement packagings (and not packagings_add) (for the record, I've just tried with packagings and indeed it replaces everything)

Yes, the "packagings_add" method is for things like Robotoff, which can detect that there is a plastic bottle, or some logo that says "Aluminium", or recycling instruction "Bottle to recycle".

  • Regarding languages, I guess the typical parameter values would be the same for tags_lc and lc.

Yes.

  • For the moment I see no other solution than creating a new method just to set a product packaging with this new modus operandi. The other fields will use the "old" method, until they are available too in v3.

Right, hopefully v3 product write will be extended to support everything from v2, but it will take some time.

@monsieurtanuki
Copy link
Contributor

@stephanegigandet What about READing packagings: will they all follow the new format, including history, regardless of having been written with an old format?
Please ping me when the API is available on .org.

@stephanegigandet
Copy link
Contributor Author

@stephanegigandet What about READing packagings: will they all follow the new format, including history, regardless of having been written with an old format?

We will convert the old format to the new format.

@stephanegigandet
Copy link
Contributor Author

@monsieurtanuki the new API is live on .net and .org

@monsieurtanuki
Copy link
Contributor

@stephanegigandet Just checking:

  • 'packaging' is not populated anymore and can be deprecated
  • 'packaging_tags' is not populated anymore and can be deprecated
  • 'packaging_text_languages' is still populated
  • 'packagings' is now supported (that's what the current issue is mainly about)

cf. https://fr.openfoodfacts.org/api/v2/product/3661344723290?fields=all&lc=en

		"packaging": "",
		"packaging_hierarchy": [],
		"packaging_lc": "fr",
		"packaging_old": "fr:carton",
		"packaging_old_before_taxonomization": "carton",
		"packaging_tags": [],
		"packaging_text": "1 opercule en métal à recycler\r\n1 pot en papier à recycler\r\n1 couvercle en papier à recycler",
		"packaging_text_fr": "1 opercule en métal à recycler\r\n1 pot en papier à recycler\r\n1 couvercle en papier à recycler",
		"packagings": [{
			"material": "en:metal",
			"number": "1",
			"recycling": "en:recycle",
			"shape": "en:seal"
		}, {
			"material": "en:paper",
			"number": "1",
			"recycling": "en:recycle",
			"shape": "en:pot"
		}, {
			"material": "en:paper",
			"number": "1",
			"recycling": "en:recycle",
			"shape": "en:lid"
		}],

@stephanegigandet
Copy link
Contributor Author

  • 'packaging' is not populated anymore and can be deprecated
  • 'packaging_tags' is not populated anymore and can be deprecated

At this point, packaging and packaging_tags can still be written, and they can still be read.
What's changed is that any new value written to it will not affect the packagings structure.

In the future, it's likely that will remove the write feature for packaging, and possibly use packaging_tags to return facets that would be derived from the new packagings structure.

So the best thing I think would be to:

  1. remove the ability to write "packaging"
  2. keep the ability to read "packaging_tags"
  • 'packaging_text_languages' is still populated

Yes. Anything written to it will be used as input for the new packagings structure.

  • 'packagings' is now supported (that's what the current issue is mainly about)

Yes.

@monsieurtanuki
Copy link
Contributor

@stephanegigandet Little problem: READing 'packagings' I can only read tags, even when providing 'tags_lc'.
In Smoothie I don't expect users to read tags. How can I easily get translations? Even if 'packaging_text_languages' is in the same order, it's a bit awkward. Do I need an extra look-up for translations?

cf. https://fr.openfoodfacts.org/api/v2/product/3661344723290?fields=packagings,packaging_text_languages&lc=fr&tags_lc=fr

'packagings':

[
  {"shape": "en:seal", "material": "en:metal", "recycling": "en:recycle"},
  {"shape": "en:pot", "material": "en:paper", "recycling": "en:recycle"},
  {"shape": "en:lid", "material": "en:paper", "recycling": "en:recycle"}
]

'packaging_text_fr':

1 opercule en métal à recycler
1 pot en papier à recycler
1 couvercle en papier à recycler

@stephanegigandet
Copy link
Contributor Author

@stephanegigandet Little problem: READing 'packagings' I can only read tags, even when providing 'tags_lc'.

@monsieurtanuki: good catch, I forgot to code this part. I'll add it.

This is what is specified in the OpenAPI file:

[
{
"number_of_units": 6,
"shape": {
"id": "en:bottle",
"lc_name": "bouteille"
},
"material": {
"id": "en:bottle",
"lc_name": "bouteille"
},
"recycling": {
"id": "en:bottle",
"lc_name": "bouteille"
},
"quantity_per_unit": "25 cl",
"quantity_per_unit_value": 25,
"quantity_per_unit_unit": "cl",
"weight_specified": 30,
"weight_measured": 32,
"weight_estimated": 26,
"weight": 30,
"weight_source_id": "specified"
}
]

@stephanegigandet
Copy link
Contributor Author

@monsieurtanuki here is the corresponding server side PR, could you review it?

openfoodfacts/openfoodfacts-server#7749

If you want to test, it's currently live on the .dev server:
https://world.openfoodfacts.dev/api/v3/product/3335880004112?fields=packagings&tags_lc=fr

Note that you have to use v3 to get the new format for packagings (so that apps using v2 don't break). The response format (the wrapper) has slightly changed to give more details about warnings, errors etc.

@monsieurtanuki
Copy link
Contributor

@stephanegigandet Now I can READ (id + lc_name) but I cannot WRITE anymore :)

How am I supposed to specify a new value for a shape? 'shape':{'lc_name':'une nouvelle forme'}?
If I do it's getting ugly, like {shape: {id: fr:HASH(0x561f0b58bf98), lc_name: HASH(0x561f0b58bf98)}.

@raphael0202
Copy link

raphael0202 commented Nov 22, 2022

I'm leaving my feedback here on potential bugs I've encountered when testing the current version of the API.

Overwriting packaging data

It seems there is an issue when trying to overwrite packaging data.
I'm using the following script to update this product (.net server):

import requests

username = "raphael0202"
password = "*********"
barcode = "3273220180259"
tld = "net"
auth = ("off", "off") if tld == "net" else None
url = f"https://world.openfoodfacts.{tld}/api/v3/product/{barcode}"

json = {
    "product": {
        "packagings": [{"material": {"id": "en:pp-polypropylene"}, "shape": {"id": "en:lid"}}]
    },
    "username": username,
    "password": password,
}
r = requests.patch(url=url, json=json, auth=auth)
r.raise_for_status()
response = r.json()
print(response["product"]["packagings"])

According to the documentation, the data is supposed to be overwritten, which isn't the case (packaging data is unchanged).

Adding new packaging item in local language

import requests

username = "raphael0202"
password = "Z4WcL2T9HZ8k2"
barcode = "3335880004990"
tld = "net"
auth = ("off", "off") if tld == "net" else None
url = f"https://world.openfoodfacts.{tld}/api/v3/product/{barcode}"

json = {
    "product": {
        "tags_lc": "fr",
        "packagings_add": [
            {
                "material": {"lc_name": "verre"},
                "shape": {"lc_name": "pot"},
                "recycling": {"lc_name": "à recycler"},
                "number_of_units": "1",
            }
        ],
    },
    "username": username,
    "password": password,
}
r = requests.patch(url=url, json=json, auth=auth)
r.raise_for_status()
response = r.json()
print(response["product"]["packagings"])


>>>[{'material': 'en:metal', 'shape': 'en:capsule'}, {'material': 'en:glass', 'shape': 'en:jar'}, {'material': 'en:HASH(0x559321f11308)', 'number_of_units': 1, 'recycling': 'en:HASH(0x55930cf56738)', 'shape': 'en:HASH(0x5592f7fd7dd8)'}]

The en:HASH(0x55930ac3f058) are obviously wrong.

@stephanegigandet
Copy link
Contributor Author

@monsieurtanuki @raphael0202
My mistake, I updated the OpenAPI with a new version (you can specify either "shape": {"id": "en:glass"} or "shape": {"lc_name": "verre"}), but I implemented the previous version... :-( I will fix it. (currently you can update with "shape": "en:glass" or with "shape" : "verre", but I'm going to change it)

@stephanegigandet
Copy link
Contributor Author

@monsieurtanuki @raphael0202 It's fixed in the PR and on the .dev server:

curl -k -X PATCH http://off:off@world.openfoodfacts.dev:8701/cgi/display.pl?api/v3/product/12345678977 -H "Content-Type: application/json" -d '{"product": { "packagings": [{"shape": {"lc_name": "bottle"}, "material": {"id": "en:plastic"}}]}, "fields": "updated"}'

{"code":"12345678977","errors":[],"product":{"packagings":[{"material":{"id":"en:plastic","lc_name":"Plastic"},"shape":{"id":"en:bottle","lc_name":"Bottle"}}]},"status":"success_with_warnings","warnings":[{"field":{"default_value":"en","id":"tags_lc"},"impact":{"id":"warning","lc_name":"Warning","name":"Warning"},"message":{"id":"missing_field","lc_name":"Missing field","name":"Missing field"}},{"field":{"id":"number_of_units"},"impact":{"id":"field_ignored","lc_name":"Field ignored","name":"Field ignored"},"message":{"id":"missing_field","lc_name":"Missing field","name":"Missing field"}},{"field":{"id":"recycling","value":null},"impact":{"id":"field_ignored","lc_name":"Field ignored","name":"Field ignored"},"message":{"id":"missing_field","lc_name":"Missing field","name":"Missing field"}}]}

@monsieurtanuki
Copy link
Contributor

@stephanegigandet I've tried this morning on dev:

  • the PATCH "update/replace" queries do work now, with id or lc_name
  • the "get product" GET query works for packagings, but the packaging_text_languages does not seem to be populated anymore
  • the "get product" POST query does NOT work
{
	"errors": [{
		"field": {
			"id": "api_method",
			"value": "POST"
		},
		"impact": {
			"id": "failure",
			"lc_name": "Failure",
			"name": "Failure"
		},
		"message": {
			"id": "invalid_api_method",
			"lc_name": "Invalid API method",
			"name": "Invalid API method"
		}
	}],
	"status": "failure",
	"warnings": []
}
  • the "get product" PATCH query does NOT work (but I'm not sure it was supposed to)
{
	"errors": [{
		"field": {
			"id": "product"
		},
		"impact": {
			"id": "failure",
			"lc_name": "Failure",
			"name": "Failure"
		},
		"message": {
			"id": "missing_field",
			"lc_name": "Missing field",
			"name": "Missing field"
		}
	}],
	"status": "failure",
	"warnings": []
}

@stephanegigandet
Copy link
Contributor Author

  • the "get product" POST query does NOT work

@monsieurtanuki: for v3 of the API, we decided to adopt the standard REST practices: the READ query must use GET, and the WRITE query must use PATCH.

Both the WRITE and READ requests have the same path: /api/v3/[barcode] , they differ by the method GET or PATCH.

@monsieurtanuki
Copy link
Contributor

@stephanegigandet I have no strong opinions regarding REST, but READ=GET goes against #414. That's possible to rollback, as long as we double-check that the user credentials are never in the URL.

Currently in off-dart:

  • POST
    • /cgi/product_jqm2.pl (saveProduct)
    • /api/vx/product/$barcode (getProduct)
    • get taxonomies
    • Robotoff postInsightAnnotation
    • extract ingredients
    • extract packaging
    • get autocomplete suggestions
    • login
    • cgi/nutrients.pl (get ordered localized nutrients)
    • cgi/product_image_crop.pl crop image
    • cgi/product_image_unselect.pl
  • Multipart
    • /cgi/product_image_upload.pl(addProductImage)
    • /cgi/user.pl (register)
    • /cgi/reset_password.pl
  • GET
    • Robotoff get random insight
    • Robotoff get product insights
    • Robotoff get questions for product
    • Robotoff get random questions

@stephanegigandet
Copy link
Contributor Author

Yes, it's a change from #414 , there was a discussion about methods and moving towards the REST best practices.

openfoodfacts/openfoodfacts-server#7667

openfoodfacts/openfoodfacts-server#7564

@monsieurtanuki
Copy link
Contributor

@stephanegigandet OK for using GET for /api/v3/product/$barcode/.

Should we also consider changing current POST (but READ) queries in off-dart into GET, for consistency and in application of REST best practices?

@monsieurtanuki
Copy link
Contributor

the deployment on .net is automated but it failed, I restarted it

That's one point. The other point is about the discrepancies in .org between fields=packagings and fields=all.

@stephanegigandet
Copy link
Contributor Author

fields=all returns fields as they are stored internally, I will rename it to fields=raw or something, and make fields=all obey the v3 rules if sent

@monsieurtanuki
Copy link
Contributor

Sounds perfect!

@stephanegigandet
Copy link
Contributor Author

@monsieurtanuki : the new version is live on .org and .net

@monsieurtanuki
Copy link
Contributor

@stephanegigandet I'm almost done with coding, but I think there's something missing in WRITE, at least in PROD.
How do I say which user did edit the packagings? It looks like whatever I'm doing, I get an "anonymous" user in history.
I mean, beyond the off:off authority for the test envs.

For this one, I was logged in as monsieurtanuki:

4 décembre 2022 à 13:08:25 CET - openfoodfacts-contributors - API v3 - voir

@stephanegigandet
Copy link
Contributor Author

@monsieurtanuki I think I forgot to take into account the user and password passed in the JSON body, I will add support for it, thanks for the report.

@stephanegigandet
Copy link
Contributor Author

@monsieurtanuki the authentification is fixed by this PR: openfoodfacts/openfoodfacts-server#7813

Please note that I renamed userid to user_id to be consistent with the field name used in v2.

If you want to test, it's live on https://world.openfoodfacts.dev

@monsieurtanuki
Copy link
Contributor

@stephanegigandet Thank you, it does work in DEV, now I see the user (https://world-fr.openfoodfacts.dev/produit/3330720237361/pate-a-tartiner-noisette-du-lot-et-garonne-cacao-lucien-georgelin):
Capture d’écran 2022-12-09 à 18 48 45

Not in PROD yet: please ping me when you roll out to NET and ORG.

@stephanegigandet
Copy link
Contributor Author

@monsieurtanuki it's now in prod

@monsieurtanuki
Copy link
Contributor

@stephanegigandet Unfortunately my computer power cable crashed this morning. I won't be able to PR before days.

If someone else feels like coding this issue, I won't be offended.

@stephanegigandet
Copy link
Contributor Author

Unfortunately my computer power cable crashed this morning.

@monsieurtanuki Sorry to hear that. :(

@monsieurtanuki
Copy link
Contributor

@stephanegigandet I have a new cable now: I'm back ;)

@stephanegigandet
Copy link
Contributor Author

stephanegigandet commented Dec 16, 2022

@monsieurtanuki Wonderful :)

@monsieurtanuki
Copy link
Contributor

@stephanegigandet Regarding api v3 and "get products" queries, in Smoothie we sometimes use "user" queries (e.g. "the products I contributed to"), and for that we use a different syntax.

Instead of /cgi/search.pl, we use

  • either /contributor/$userId.json (for contributor)
  • or /api/v2/search (for photographer, informer and to be completed)

But it doesn't look like we can apply the api_version=3 parameter, which makes it impossible to retrieve packagings for user related search (which does not) - for text search it's OK as we saw.

@stephanegigandet
Copy link
Contributor Author

@monsieurtanuki I also added recently a new field "packagings_complete" which takes values 0 or 1, to indicate if all the packaging components are listed: https://github.com/openfoodfacts/openfoodfacts-server/pull/7856/files

@stephanegigandet
Copy link
Contributor Author

@monsieurtanuki for the "user" queries, I will need to implement /api/v3/search but there won't be server deployments until the first week of January.

There is a possible workaround:

https://world.openfoodfacts.org/cgi/search.pl?action=process&tagtype_0=editors&tag_contains_0=contains&tag_0=stephane&page_size=20&json=1&fields=code,packagings&api_version=3

It works for editors, photographers, informers

It doesn't work for contributor

@monsieurtanuki
Copy link
Contributor

@stephanegigandet Actually I wouldn't call it a workaround but the solution!
I like this "search.pl / tagtype_x" syntax: doing so we reuse the same query search pattern as most queries.

Of course, if it could work for "contributor" that would solve it all. For the moment I will make it work in off-dart for editors, photographers and informers.

@monsieurtanuki
Copy link
Contributor

@stephanegigandet Actually there is probably a tag for "contributor": wouldn't it be creator?

And for "to be completed", I interpret that as "all the products I'm an informer for and with the state 'en:to-be-completed'".

Therefore I'm done here, just about to PR: I got the same counts with the old v2 (and "different" URI) and with the new v3 (all with the "standard" search.pl).

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