dynamite errors trying to add cookbook support

[Leptopoda]

As discussed on Matrix I'm facing issues with integrating NC-CookBook support.

The original yaml specification: objects.yaml.txt openapi-cookbook.yaml.txt

The generated json one (using the openapi generator to generate itself I guess): openapi.json.txt

And the generated merged nextcloud.openapi.json with my touchups to remove the unsupported keys:

nextcloud.openapi.json.txt

All uploaded as txt as github only allows this and I think it's easier than creating giant codeblocks :)

[provokateurin]

I'll take a look in the next few days

[Leptopoda]

Just fyi the provided spec is a WIP and the patches are not final but I'm working on upstreaming them. Also the current server code is not compliant to the spec in the first place, see: see: https://github.com/nextcloud/cookbook/issues/1464

so don't be confused if you can't make anything that useful out of it. It'd be helpful enough if you can help me get a basic setup running and I can work from there :)

[provokateurin]

I put something together that works with little modifications. Still not working perfectly, but I can send a WIP branch. Biggest problem is that multiple status codes aren't supported yet, but I have plans for that.

[provokateurin]

https://github.com/provokateurin/nextcloud-neon/tree/feature/cookbook

[Leptopoda]

The return types are not that necessary anyways. It's just good to know in advance what to expect and makes error handling easier.

I'll have a look in the coming days. Thanks a lot :)

[Leptopoda]

Ok, as mentioned on Matrix I've had a quick glance at it and yesterday night played a bit with implementing it. My first complaints are:

1. things like allOf do not make use of inheritance.

Problem: currently the spec for the cookbook app implement something like interfaces. RecipeInformation and Recipe implements RecipeInformation. This is (probybly) because they also have something called RecipeStub with limited Information, that is used in lists or thumbnails for faster loading. Now your generated lib has something like:

class NextcloudCookbookCategory {
  NextcloudCookbookCategory(
    this._data, {
    this.cookbookCategoryInformation,
  });

  final dynamic _data;

  final NextcloudCookbookCategoryInformation? cookbookCategoryInformation;
}

Now I can't access the name property with category.name but would need to do something like category.cookbookCategoryInformation.name.

Solution: do something like

class NextcloudCookbookCategory implements NextcloudCookbookCategoryInformation{}

Now I can use the expected interface of a category.

2. Mutability

Problem: Currently it looks like all models are immutable. Changing a value in an edit screen or creating a new one is hard.

Solution: provide a way to easier change and maybe also construct an Object. This could be implemented using the copy_with_extension. Another idea would be to us the built_value package. This can handle the serialization (toJson and fromJson) and will aslo provide a builder method. A builder would provide a mutable instance of the object. The immutable instance could be 'transformed' into a mutable one and vice versa.

This would be a bigger effort but could make a lot of thing easier as I can use an onSaved callback on the formfield that will call the setter in the builder and finally calling mutableObject.build() in the form submit method. I think this is a cleaner approach than calling immutableObject.copyWith() with all properties propulated by the current texteditingcontroller.

2.1 const constructors

As all models are immutable they could have const construcotrs coudln't tehy?

2.2 equatable?

I might want to compare two objects to each other maybe just making sure to nat call updateRecipe() when the new one is just the same to the old one and nothing has been edited but there are other scenarios where this would be useful.

This could probably be easily implemented with the equatable package.

3. nested tags support

Problem: the cookbook api does have it's own tags. the most important differentiation would be the misc one and the rest. The misc part does contain information about the app (version, api version, config options). I quite like them not being in the same group as the rest. Also having an operationId would make naming a bit easier. Imaginq something like:

var NextcloudCookbookClient api = NextcloudCookbookClient();

final List<Recipe> recipes = api.recipes.getall();

await api.misc.changeConfig(config: new Confaig());

I quite like the separation.

Solution: Now there isn't any official nested tags support in neither openapi nor swagger but maybe we could name the tags something like cookbook.misc and the generator would know how to interpret them.

4. typedef

The goal of neon is to become a single lib for all nextcloud apps. The problem is that most app devs would probably only implement one app though. so for me the only important types would be NextcloudCookbook*. It might be a good idea to export a file with typedefs shortening the names. An appp dev could then only import the needed typedefs like

import 'package:nextcloud/cookbook_types.dart';

wich in turn would be like

typedef Recipe = NextcloudCookbookRecipe;
typedef RecipeStub = NextcloudCookbookRecipeStub;
typedef Category = NextcloudCookbookCategory;
typedef Version = NextcloudCookbookVersion;
typedef APIVersion = NextcloudCookbookAPIVersion;

This would make the code a lot more readable and there shouldn't be any naming conflict per api anyways. Now if I need to pass a Category as a parameter to a function I don't need to import the entire nextcloud package with all its types making intellisense more usefull.

5. Code documentation

use the example, description and whatnot to to generate doc comments. At a glance this might seem unnecessary but welcomming new devs to a project would be much easier as they could just hover over code to get all needed information.

6. Differentiate image return type

The cookbook api will return images as jpeg or svg where the jpeg is used normally and the svg is only a standart fallback image when none is set. Now for svg support one would probably use flutter_svg so we would need to differentiate between an svg or an jpeg so we could use Image.memory() or SvgPicture.memory() correctly.

The raw binary data is not of much use here.

Yes I can try to parse the Uint8List into a String and if it is possible/makes sense us the svg else the normal image but I don't think this is nice code when we could just wrap it in a class that specifies the type.

abstract class BinaryImage {
  final Uint8List data;
}

class SVGImage extends BinaryImage {}

would probably be enough so we could do a typecheck on it

if(data is SVGImage)

7. The not yet touched multiple respones :upside_down_face:

---

I just mentioned all things that bothered me compared to the generator I used before and I obviously understand that not all is suited for this project. I just wrote down all my concerns I would nee to work around not meaning that it would be wrong.

The biggest Issue and blocker are section 2 and section 6 as they are really needed to make it work.

I also understand that something (¿unnecessarily?) complex like the problem I have in section 1 wouldn't even occur if we migrate the cookbook spec to one generated by your future script. Just things I found and wanted to let you know about. It might become useful another time anyways.

---

Also seeing the patch you made to dynamite It gives me confidence to start implement something easy like section 5 and I might take a shot at it after my exams.

[provokateurin]

1. On my TODO list, I know it's a little harder to implement
2. We can do that, I just hadn't had the usecase for that yet
3. I agree with you, let's do it like you suggested (also for all the other APIs)
4. Good idea, let's do that
5. This depends on the spec, except for examples everything else is implemented. I haven't bothered to do it until now, because the automatic spec generation will fix most of it.
6. SVG data should be returned as a string, but I agree with your suggestion (multiple response types are also not implemented yet, but will come together will multiple response codes)
7. On my TODO list

[Leptopoda]

Ok good to see that we don't fundamentally disagree :)

6. SVG data should be returned as a string, but I agree with your suggestion (multiple response types are also not implemented yet, but will come together will multiple response codes)

The svg could still be returned as a Uint8List as the most common svg lib does support it. I only need a way to differentiate wether it's an svg in the first place.

I also might give the typedf thing a try in the coming days

[provokateurin]

The differentiation will already happen in the lib when multiple response types are implemented

[Leptopoda]

Ok I've had some time yesterday to learn generators on an old project of mine and I feel confident implementing the typedef thing.

As you have a fairly opionated code style I'd like to know how you want this to be? We could annotate the classes with a custom annotation and then have a second builder handle the generation.

We could also try to do it in the same pass as the main builder but Ideally the typedefs would be in separate files per app so we don't can keep intellisense clean and this would just bloat the main builder.

[provokateurin]

I think a single generator would be nice, but it's probably not possible in thise case because a generator can only produce a single file which would defeat the purpose of typedef.

[provokateurin]

@Leptopoda do we want to keep this issue open?

[Leptopoda]

Yes and no.

I also thought of closing this as with my current knowledge of dynamite it should be trivial to do it myself.

On the other hand my list above is still kinda valid. I'll open separate issues for them tomorrow and then close this one (I hope I don't forget)