amazon-ion / ion-schema-rust

Rust implementation of Ion Schema
https://amazon-ion.github.io/ion-schema/sandbox
Apache License 2.0
12 stars 6 forks source link

Streamlines ISL model using version maker traits #191

Closed desaikd closed 1 year ago

desaikd commented 1 year ago

Issue #187

Description of changes:

This PR works on streamlining the ISL model using version maker traits.

List of changes:

Note for reviewers:


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

desaikd commented 1 year ago

Overall versioned ISL model gives an ease of use, maintainability and gives compile time checks for ISL version on types, constraints and schema.

  • Eventually, we really need to address the fact that some constraints have their own structs, and others are just an enum variant that's a tuple. If you're rewriting the model, this seems like an ideal time to fix it.

Previous ISL model had tuples for enum variants for all the constraints. I have created a struct for each constraint so that we have the same pattern throughout for all constraints be it a constraint thats same for both ISL versions or its different for different ISL versions. It gives us a nice distinction between how 2 different versions would work for a particular constraint. Also eventually if we decide to have single model that will be used for representation as well as validation then it makes sense to create different structs for all the constraints as each one will have its own validation.

  • I'd recommend something like this to eliminate some of the boilerplate for constraints. https://gist.github.com/popematt/11b1e632ed487d11ba50eda1bb43a78a (Note that this will not work exactly because it doesn't have the version it it.) As a bonus, a macro like this will mean that we also get a proper struct for each constraint.

Adding a macro to create the structs for constraints does sound like good idea. I could create an issue for it.

  • What other solutions did you consider? (Have you considered creating versioned builders for a versionless model?) What made you choose a versioned model over those other solutions? (I'd like to understand any tradeoffs, etc.)

I have chosen to go with versioned model using the version maker trait because that gives a better API to construct ISL model constraints/types and schema. This allows us to have compile time errors for when a user tries to mix two different versions of schema. I think this provides an ease of use for customers as well as ensures the different version constraints aren't mixed.