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

someOfs with many options are hard to create by hand #1508

Open
provokateurin opened this issue Jan 24, 2024 · 12 comments
Open

someOfs with many options are hard to create by hand #1508

provokateurin opened this issue Jan 24, 2024 · 12 comments
Assignees
Labels
package: dynamite refactoring Something that needs to be refactored

Comments

@provokateurin
Copy link
Member

provokateurin commented Jan 24, 2024

An anyOf like

typedef OcsGetCapabilitiesResponseApplicationJson_Ocs_Data_Capabilities = ({
is really hard to create by hand because you need to explicitly set every option. Not using records would solve this as all empty fields could be left out.

@provokateurin provokateurin added bug Something isn't working package: dynamite labels Jan 24, 2024
@Leptopoda
Copy link
Member

Should we really treat this as a bug?

@provokateurin
Copy link
Member Author

I didn't know a better label, you can also remove it

@provokateurin provokateurin added refactoring Something that needs to be refactored and removed bug Something isn't working labels Jan 24, 2024
@Leptopoda
Copy link
Member

Let's consider this for the next release

@Leptopoda Leptopoda added this to the Nextcloud package release milestone Jan 28, 2024
@provokateurin
Copy link
Member Author

provokateurin commented Jan 28, 2024

For the release that is coming up this week or for the release after that? If it is the latter then please add it after we have made the upcoming release.

@Leptopoda
Copy link
Member

I'll try to draft an idea today.
If it fits we can still pull it into the next that is coming up. But it's not a blocker.

@Leptopoda Leptopoda self-assigned this Jan 28, 2024
@Leptopoda
Copy link
Member

Ok My initial idea was to add a separate helper constructor to one of our extension methods that takes a dynamic value, switches over the runtimetype and assigns it to the right field(s).
But then it looks like we end up at the start, trying to build custom objects out of records.

Should we move back to classes (needing to wrap every object)?

@provokateurin
Copy link
Member Author

I was thinking of a extension that adds a constructor that has all the fields, but set to null by default. Then it is very easy to add a single value and we wouldn't have a major rework and breaking change.

@Leptopoda
Copy link
Member

There are two things to consider:

  1. the constructor/static method will be in the extension thus you'd need to call SomeTypte$Extension.fromValue()
  2. re introduce descriminator support for dynamite #1231

@provokateurin
Copy link
Member Author

I'd be fine with calling the extension.
Do you think it is easier to implement the discriminator with classes? Afaict it would also make the oneOf/anyOf validation easier, right?

@Leptopoda
Copy link
Member

Do you think it is easier to implement the discriminator with classes?

Let's say it the other way. I do not think that it is even possible for records (at least not without a lot of hacks).

@provokateurin
Copy link
Member Author

Ok, then let's go with classes again?

@provokateurin
Copy link
Member Author

It would also be great to have good error messages for oneOf, because currently the error message doesn't help in debugging the problem. In case something goes wrong the serialization errors of all sub types should be displayed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: dynamite refactoring Something that needs to be refactored
Projects
None yet
Development

No branches or pull requests

2 participants