openfoodfacts / openfoodfacts-dart

Open Food Facts API Wrapper
https://pub.dev/packages/openfoodfacts
Apache License 2.0
167 stars 67 forks source link

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

Closed stephanegigandet closed 1 year ago

stephanegigandet commented 2 years ago

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: https://github.com/openfoodfacts/openfoodfacts-server/pull/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 commented 2 years ago

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 commented 2 years ago

cc @monsieurtanuki @M123-dev @g123k

monsieurtanuki commented 2 years ago

@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 commented 2 years ago
  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 commented 2 years ago

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 commented 2 years ago

Thank you @stephanegigandet for your answers!

In the context of Smoothie:

monsieurtanuki commented 2 years ago

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

stephanegigandet commented 2 years ago
  • 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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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

monsieurtanuki commented 1 year ago

@stephanegigandet Just checking:

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 commented 1 year ago
  • '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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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

https://github.com/openfoodfacts/openfoodfacts-server/pull/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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@stephanegigandet I've tried this morning on dev:

stephanegigandet commented 1 year ago
  • 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 commented 1 year ago

@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:

stephanegigandet commented 1 year ago

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

https://github.com/openfoodfacts/openfoodfacts-server/issues/7667

https://github.com/openfoodfacts/openfoodfacts-server/issues/7564

monsieurtanuki commented 1 year ago

@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 commented 1 year ago

@stephanegigandet Please ping me when WRITE packagings and READ api/v3/product are available in .net or .org.

monsieurtanuki commented 1 year ago

@stephanegigandet MAYDAY MAYDAY!!! It looks like the POST "get data" queries are now rejected in TEST env. The same queries run as GET do work. If you do the same in PROD, all "get data" queries will fail, as currently we use mainly POST (even for mere "get data" queries).

  HandshakeException: Handshake error in client (OS Error: 
    CERTIFICATE_VERIFY_FAILED: application verification failure(handshake.cc:393))
  dart:io  _RawSecureSocket._tryFilter
stephanegigandet commented 1 year ago

@monsieurtanuki that looks like a SSL certificate related error, can you tell me exactly which query you are sending?

I'm getting the expected results on .net:

v2: curl -X POST https://off:off@world.openfoodfacts.net/api/v2/product/0073416000469 -d 'fields=code,product_name'

{"code":"0073416000469","product":{"code":"0073416000469","product_name":"Lightly salted organic brown rice puffed grain cakes, lightly salted"},"status":1,"status_verbose":"product found"}

v3: curl -X POST https://off:off@world.openfoodfacts.net/api/v3/product/0073416000469 -d 'fields=code,product_name'

{"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":[]}

stephanegigandet commented 1 year ago

@stephanegigandet Please ping me when WRITE packagings and READ api/v3/product are available in .net or .org.

@monsieurtanuki both are live since yesterday

monsieurtanuki commented 1 year ago

that looks like a SSL certificate related error, can you tell me exactly which query you are sending?

@stephanegigandet That doesn't seem to happen anymore, which is good news! It happened for instance on cgi/nutrients.pl.

monsieurtanuki commented 1 year ago

@stephanegigandet Side effect when you use /cgi/search.pl for all product fields:

If I run a barcode query on a product, I can decide if it's V2 or V3. It looks like when I use /cgi/search.pl it's always V2.

A solution would be to be robust in off-dart and say "try to decode the packagings with the V3 format, else try with the V2 format and turn it into a V3"? But that's not very elegant (and we loose the localization).

monsieurtanuki commented 1 year ago

@stephanegigandet I've double-checked in the off-dart code, and with the json code generating tool we're currently using we cannot say "consider the packagingS field to be either a String or a Map".

In short, all queries that use all as (meta) field will fail if I implement how to "decode" the new packagings field. Or that would still work if I keep the V2 String format. But then we miss the localizations and that won't work in V3.

I don't know in which direction we should go...

monsieurtanuki commented 1 year ago

Possible solutions are

Without that, there would be discrepancies for packagings results between the barcode get product queries (either V2 or V3, you can decide) and the text search get product method (it's always V2, therefore no localization).

Here I'm blocked. What can the server side do?

stephanegigandet commented 1 year ago

@monsieurtanuki I had not thought of that. I think the easiest is to add an API version parameter to /cgi/search.pl &api_version=3

I'll implement that soon.

monsieurtanuki commented 1 year ago

@stephanegigandet A quicker fix for the moment would be to remove packagings from V2 - not that many people had the chance to use it, I presume. Not in contradiction with an api_version for search.pl. Meanwhile, I'm blocked.

stephanegigandet commented 1 year ago

@monsieurtanuki the fix has been deployed: https://world.openfoodfacts.org/cgi/search.pl?action=process&code=8715700407760&fields=packagings&json=1&api_version=3

monsieurtanuki commented 1 year ago

@stephanegigandet Thank you, working on it!

monsieurtanuki commented 1 year ago

@stephanegigandet It looks like it's been deployed on .org, but not on .net, am I right?

monsieurtanuki commented 1 year ago

@stephanegigandet Things are getting a little stranger...

For barcode 7613034246882, if I try to retrieve only the packagingS, I use https://fr.openfoodfacts.org/cgi/search.pl?lc=en&tags_lc=en&fields=packagings&page=1&page_size=1&code=7613034246882&search_terms=&json=1&api_version=3&action=process and this is what I get as packagings:

[{
    "material": {
        "id": "en:cardboard",
        "lc_name": "Cardboard"
    },
    "shape": {
        "id": "en:sleeve",
        "lc_name": "Sleeve"
    }
}, {
    "material": {
        "id": "en:plastic",
        "lc_name": "Plastic"
    },
    "number_of_units": 1,
    "shape": {
        "id": "en:container",
        "lc_name": "Container"
    }
}, {
    "shape": {
        "id": "en:bag",
        "lc_name": "Bag"
    }
}, {
    "shape": {
        "id": "en:box",
        "lc_name": "Box"
    }
}, {
    "shape": {
        "id": "en:unknown",
        "lc_name": "Unknown"
    }
}]

BUT if I get ALL fields (https://fr.openfoodfacts.org/cgi/search.pl?lc=en&tags_lc=en&fields=all&page=1&page_size=1&code=7613034246882&search_terms=&json=1&api_version=3&action=process), this is what I get for packagings:

[{
    "material": "en:cardboard",
    "shape": "en:sleeve"
}, {
    "material": "en:plastic",
    "number_of_units": 1,
    "shape": "en:container"
}, {
    "shape": "en:bag"
}, {
    "shape": "en:box"
}, {
    "shape": "en:unknown"
}]

Which is NOT "legal" in api_version 3. This was a "Am I crazy or what?" moment, and this time I was NOT crazy.

stephanegigandet commented 1 year ago

@stephanegigandet It looks like it's been deployed on .org, but not on .net, am I right?

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

monsieurtanuki commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Sounds perfect!

stephanegigandet commented 1 year ago

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

monsieurtanuki commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@monsieurtanuki the authentification is fixed by this PR: https://github.com/openfoodfacts/openfoodfacts-server/pull/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 commented 1 year ago

@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 commented 1 year ago

@monsieurtanuki it's now in prod

monsieurtanuki commented 1 year ago

@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.