search

openfoodfacts / openfoodfacts-dart

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

Implement the new App uuid system #370

Closed teolemon closed 2 years ago

teolemon commented 2 years ago

What

For further details and potential questions, see: https://github.com/openfoodfacts/openfoodfacts-server/pull/6319

Part of

monsieurtanuki commented 2 years ago

@teolemon Basically it's about adding 3 optional String parameters to API queries, but will that concern all queries, or just the writes or just the reads?

M123-dev commented 2 years ago

I think we can edit the UserAgent to be used for that

teolemon commented 2 years ago
M123-dev commented 2 years ago

What apps can't send a UserAgent, the current UserAgent in this sdk is pretty similar, it's only lacking the uuid and is probably send different

https://github.com/openfoodfacts/openfoodfacts-dart/blob/master/lib/model/UserAgent.dart

  final String? name;
  final String? version;
  final String? system;
  final String? url;
  final String? comment;
stephanegigandet commented 2 years ago

I agree with @M123-dev , the info is the same as what we have in the UserAgent class, we can reuse it, so we don't need a separate SourceConfiguration class as proposed by @monsieurtanuki in his PR.

A few points:

  1. Apps should send both the User-Agent header (as we do today) and the extra app_name and app_version parameters (as GET or POST parameters, but they should not be mixed: either send all parameters as GET query string parameters, or all parameters as POST).

The reason we want both is that we can see them in different places. The User-Agent header is easy to find in nginx and Apache logs, while the more structured app_name and app_version are better for the OFF API internals.

  1. The UUID is kind of separate, we can deal with it somewhere else than in the User-Agent class (e.g. when we build the query parameters and add the User info). Maybe AbstractQueryConfiguration

  2. This user-agent and the app_name + app_version should be sent on every request: not only the save product, but also image uploads and crops/selection, and queries to read and search products as well.

  3. In addition, the user specific username, or UUID should be sent for save product, uploading image and image selection (anything that changes a product).

stephanegigandet commented 2 years ago
  1. For read queries / search queries, the SDK should allow apps (and apps users) to easily add (or not add) an user specific OFF username/password. This is to allow future features like accessing your scan history on the website, or other apps.
monsieurtanuki commented 2 years ago

Working on 1. and 3.

M123-dev commented 2 years ago

For the uuid, we can probably add it to the user class since it is somewhat related

monsieurtanuki commented 2 years ago

For the uuid, we can probably add it to the user class since it is somewhat related

Sounds good to me.

But: how do you compute the UUID? I cannot picture the meaning of UUID compared to just the userId (e.g. monsieurtanuki). Should we add the environment, hash them and present the result as a 36-character string? (cf. https://en.wikipedia.org/wiki/Universally_unique_identifier)

teolemon commented 2 years ago

https://github.com/openfoodfacts/smooth-app/blob/954449e437ce0a848bd8313eb6eed0f6a8770d20/packages/smooth_app/lib/cards/product_cards/question_card.dart#L397

teolemon commented 2 years ago

@monsieurtanuki here's what we do in Smoothie (for Robotoff only, I believe)

monsieurtanuki commented 2 years ago

Thank you @teolemon!

As off-dart is not restricted to Android and iOS, we need UUID also for MacOS, Linux, Web and Windows. Something like that:

if (Platform.isAndroid) {
    AndroidDeviceInfo androidInfo = await deviceInfo.androidInfo;
    deviceIdentifier = androidInfo.androidId;
  } else if (Platform.isIOS) {
    IosDeviceInfo iosInfo = await deviceInfo.iosInfo;
    deviceIdentifier = iosInfo.identifierForVendor;
  } else if (kIsWeb) {
    // The web doesnt have a device UID, so use a combination fingerprint as an example
    WebBrowserInfo webInfo = await deviceInfo.webBrowserInfo;
    deviceIdentifier = webInfo.vendor + webInfo.userAgent + webInfo.hardwareConcurrency.toString();
  } else if (Platform.isLinux) {
    LinuxDeviceInfo linuxInfo = await deviceInfo.linuxInfo;
    deviceIdentifier = linuxInfo.machineId;
  }
Or using platform_device_id: platfom source example
Windows BIOS UUID 99A4D301-53F5-11CB-8CA0-9CA39A9E1F01
Linux BIOS UUID 32a70060-2a39-437e-88e2-d68e6154de9f
Mac IOPlatformUUID 02662E79-E342-521C-98EA-D4C18B61FEF3
Android AndroidId 9774d56d682e549c
IOS IdentifierForVendor 9C287922-EE26-4501-94B5-DDE6F83E1475

(for web we only get the user agent).

Anyway the general idea is that we (as off-dart) compute the UUID and affect it automatically to the relevant requests, right? Should we match the "official" UUID format or do we just want something approximately unique, regardless of the string format?

monsieurtanuki commented 2 years ago

There is a couple of problems:

Shouldn't we create a static String? uuid field in configuration and let the users set (and store locally) its value, like they do with userAgent? Possibly with helpful comments and helper methods to generate a "decent" UUID?

jasmeet0817 commented 2 years ago

Sorry for being late to the party.

There is a couple of problems:

  • off-dart is a dart project, and all the "get device id" packages are flutter packages, not dart packages. They can't be used in dart (as far as I understand)
  • even if it worked, the "get device id" methods are Futures, which has a (manageable) impact on the code, like Uri getUri(...) becoming Future<Uri> getUri(...) async ... with the related strange await getUri(...) calls

Agreed, this is not ideal.

Shouldn't we create a static String? uuid field in configuration and let the users set (and store locally) its value,

I think the server should specifically either ask for a device-id or/and (a bunch of specific information like app-name, app-version, username etc) and the server should be responsible for generating the UUID based on that information. Reason being, if you let clients come up with their own mechanics for creating unique strings, two different clients could create the same UUID (which would be unique for them but not across devices). Clients are generally not responsible for creating their own identifiers (unless if they use something globally unique like device-id)

If the server specifically asked for device-id (and app-name+version), that would make this problem much easier, each client would send the device id (and other stuff) to the dart package which would forward it to the server.

monsieurtanuki commented 2 years ago

@jasmeet0817 As I said before, I don't quite understand the purpose of the uuid. If it's something like "device id", we cannot do that in dart but in flutter.

As I suggested, we can provide a dart method to generate UUID. The problem is that we cannot be sure that the user 1. uses this method 2. populates correctly the global uuid field 3. stores and reuses correctly its uuid generated once, instead of generating uuid on the fly when needed.

A solution would be either to convert off-dart to a flutter project (instead of a dart project), or to create a new project called flutter_openfoodfacts that basically embeds (dart) openfoodfacts but with additional features - like an automatic uuid computation from device id.

jasmeet0817 commented 2 years ago

hmm I see. Thanks for clarifying. To be honest I would just get the device_id in Smoothie directly, it's already so easy, we don't really need another layer on top.

@teolemon Basic question: Is this really needed in off-dart package ? Can the clients just pass deviceId to the APIs? we can just make it a requirement to always pass it in the APIs we care about.

monsieurtanuki commented 2 years ago

@teolemon I think we're done here; I guess the part about comment was more server-related. Feel free to reopen it if relevant.