Fix deserialization of recipe_id

[arroyoj]

While trying to run the latest devel version of Teifun2/nextcloud-cookbook-flutter@0507640, I found that the initial category screen failed to load with the following error:

Error: Deserializing '[{recipe_id: 2284, name: Waffles, keywords: Breakfast, category: Bre...' to 'BuiltList<RecipeStub>' failed due to: Deserializing '[recipe_id, 2284, name, Waffles, keywords, Breakfast, category, Brea...' to 'RecipeStub' failed due to: Deserializing '2284' to 'int' failed due to: type 'String' is not a subtype of type 'int' in type cast, null) }

I traced this back to this library, where the "recipe_id" is being specified as an int during deserialization here:

nextcloud_cookbook_dart_api/lib/src/model/recipe_stub.dart

Lines 159 to 167 in 5465b2f

	case r'recipe_id':
	final valueDes = serializers
	.deserialize(
	value,
	specifiedType: const FullType(int),
	)
	.toString();
	result.recipeId = valueDes;
	break;

I also found similar code in nextcloud_cookbook_dart_api/lib/src/model/recipe_stub_all_of.dart.

This PR changes the type to String in both files and fixes the error I was receiving above.

[Leptopoda]

What version of cookbook and NC are you running?

[Leptopoda]

They should be strings but i was facing issues on 0.10.2 with nc 26 thus changed the values.

nextcloud/cookbook#1590

I know that I only patched the issue myself but this repo only contains generated code changes shouldn't be made directly but rather to the api spec and then running the generator.

[arroyoj]

I'm on NC 25.0.6 with Cookbook 0.10.2. On my server, the recipe IDs are being returned as strings. Maybe something changed with NC 26? Unfortunately I can't update to NC 26 to test.

[Leptopoda]

Could you get the raw response of the api?

I'm wondering if this has to do with the raw recipe that's stored (I think the server will just return the raw data stored at id in the recipe.json)

Ideally the data should be passed through the filter (that#s already being done for the recipe itself but not for recipe_stub).
Look here: https://github.com/nextcloud/cookbook/pull/1477/files

Maybe your php is better than mine and you could fix it upstream :)
I'll think about what we can do for now.

[Leptopoda]

Can you try just ommiting the type and leave the .toString()?

this should work in both cases for now.

[arroyoj]

Here is an example of a raw recipe.json file for a recipe on my server:

{"nutrition":{"@type":"NutritionInformation"},"@context":"http:\/\/schema.org","@type":"Recipe","dateCreated":"2022-03-08T13:09:27+0000","printImage":true,"id":11887,"name":"Test Recipe","imageUrl":"\/apps\/cookbook\/webapp\/recipes\/11887\/image?size=full","recipeCategory":"","description":"This is a recipe for app testing","recipeIngredient":["## Ingredient Group","Ingredient 1","Ingredient 2","Ingredient 3"],"recipeInstructions":["Instruction step 1","Instruction step 2","Instruction step 3"],"tool":["Tool 1","Tool 2","Tool 3"],"recipeYield":4,"prepTime":"PT0H30M0S","cookTime":"PT1H0M0S","totalTime":"PT1H30M0S","keywords":"","image":"","url":"https:\/\/github.com\/nextcloud\/cookbook","dateModified":"2023-04-01T21:40:18+0000"}

In the raw file, the recipe ID appears to be an int.

When I manually call the "recipesInCategory" API, the corresponding JSON looks like this, with the recipe_id returned as a string:

{"recipe_id":"11887","name":"Test Recipe","keywords":null,"dateCreated":"2022-03-08 13:09:27","dateModified":"2023-04-01 21:40:18","category":null,"imageUrl":"\/apps\/cookbook\/webapp\/recipes\/11887\/image?size=thumb&t=1680385218","imagePlaceholderUrl":"\/apps\/cookbook\/webapp\/recipes\/11887\/image?size=thumb16"}

[Leptopoda]

Ok I still can't reproduce this even on 25.0.5
We would wat to get this fixed upstream :(

If you could adapt your PR as mentioned above, i.e:

       final valueDes = serializers
              .deserialize(value)
              .toString();

I think this should work.

[arroyoj]

I tried the change you suggested, removing the specifiedType, and got a different error:

Error: Deserializing '[{recipe_id: 2284, name: Waffles, keywords: Breakfast, category: Bre...' to 'BuiltList<RecipeStub>' failed due to: Deserializing '[recipe_id, 2284, name, Waffles, keywords, Breakfast, category, Brea...' to 'RecipeStub' failed due to: type 'String' is not a subtype of type 'List<Object?>' in type cast, null) }

But, while looking into the upstream code, I think I figured out why the recipe_stub is behaving differently from the recipe API. Not sure how to fix it, though. I'll add a comment with my findings on nextcloud/cookbook#1590.

[arroyoj]

FYI, I think I may have figured out why we are having different results when calling the API and getting a recipe stub. I'm on php-7.4. Are you on php-8.1 or greater?

Prior to php-8.1, it looks like the default behavior of the underlying PDO mysql database driver was to return string versions of numeric values from the database, unless native prepared statements were used. I couldn't figure out how to test it, but my guess is Nextcloud has not been using native prepared statements but just the default emulated prepared statements. That would explain why I'm getting string versions of the numbers, since those recipe stub API calls are returning values straight from the database.

Starting in php-8.1, it looks like the default behavior of PDO-mysql was changed to return native numeric types from the database for all prepared statements: php/php-src@c18b1ae. See also this warning from the 8.1 Upgrade notes: https://github.com/php/php-src/blob/3a76f795f8543aca37df6bb8b63a860d9701e62d/UPGRADING#L130-L134. Assuming you are on 8.1 or later, that's probably why you are getting integer values returned from the Cookbook API.

I guess the question is, assuming upstream isn't prepared to fix the API and only return string values for "recipe_id", should we have a workaround for accepting either string or int for now? php-8.0 is still supported for Nextcloud 26, so presumably there would be users who might see different recipe_id types until everyone is forced to migrate to php-8.1+. Would you be OK with an approach using a try/catch to first try one type and then try the other type if there is an error? I'm not sure what the best approach is here.

[Leptopoda]

That makes total sense (I'm on 8.1 rn)
It might be good to also append the information to the upstream issue.

I'm totally for it to fix this. It's not acceptable to limit the use on newer versions and we should also support as many old platforms as possible.

Would you be OK with an approach using a try/catch

Nope. I'm not sure either as my long term goal is to move to the nextcloud dart lib (as linked in the readme) and I'm working a lot with the dev over there to move forward on this front. This project uses 100% code generation so we can't rely on hacks (otherwise we could have sticked with the old backend).

I thinks it's best to just properly document the api and change the spec to use a oneOf type:
https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/

Making it oneOf<int, String> (not valid openapi syntax). Then you would need to run the generator script (under ./tools/...).
NOTE: the generator script might be a bit broken I haven't tested it on other machines yet. ALso you would need to have the openapi-generator installed.

Finally we'd need to change the flutter app itself but a simple .toString() might be enough.
Another Idea would be to omit the type altogether leaving a JsonObject wich would definitely work but wouldn't be as precise.

I can start working on this next week so If you have the time and want to tackle it I can assign you to the issue.

[arroyoj]

Glad that explains the differences we were seeing. I'll update the upstream issue with this info as well.

At this point, I think you have a better handle on what is needed to fix this then I do, so I don't think it makes sense to assign it to me. I'm happy to help with testing though.

[Leptopoda]

could you please try this?

You'll also need to patch the flutter app so I attached one for you :)
0001-fix-api-update-lib.patch.txt

[arroyoj]

Thanks for the patch for the app. I applied it and tested out the app, and the categories loaded without any error! As far as I can tell, the app is fully working with the changes that you made to the api package and the app. Hopefully we can eventually get a fix into upstream so that the api output matches the spec regardless of php version, but this is great. Thanks for the fix!

[Leptopoda]

Could you also give recipe edit/creation and deletion a try?

They should return the id of the changed recipe and I'm not sure they are the right format :)

[arroyoj]

As far as the patch is concerned, I was able to invoke recipe edit/creation and deletion and the return values seemed to be handled properly. However, while testing out editing/creating, I discovered that ingredients, instructions, and tools are not being saved, though other values like title and serving are. Seemed like it might be the ones that are arrays instead of simple values. Anyway, that's not related to this patch/issue, since reverting the patch did not change that behavior. I can file a separate issue about that, unless you're already working on it.

[Leptopoda]

Could you please open an issue on the main repo.

It's always good having it documented somewhere. I'll have a look later.
I don't think this is related to the api and must be an issue in the UI code