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

`SchemaSystem::load_schema` should not wrap the returned `Schema` with an `Rc` #150

Closed sadderchris closed 1 year ago

sadderchris commented 1 year ago

If I load a schema with a schema system like so

    // Assuming I have a `main.isl` file somewhere
    let authority = MapDocumentAuthority::new([("main.isl", include_str!("main.isl"))]);
    let mut schema_system = SchemaSystem::new(vec![Box::new(authority)]);

    let schema: Rc<Schema> = schema_system.load_schema("main.isl").expect("loaded schema");

I get a schema back that's wrapped in an Rc. Why?

Returning the schema wrapped in an Rc might be convenient for some use cases, but it entirely prevents the user from managing the lifetime of the schema themself. What if I want an Arc<Schema> instead because I want to share this across threads? Or maybe I don't need any indirection at all. There doesn't seem to be any way to accomplish this with the current API definition.

Can the loaded schema just be returned unwrapped?

desaikd commented 1 year ago

Thanks for explaining the issue @sadderchris! Rc<Schema> is used for our caching logic if a same schema id is asked again to load its loaded from this cache of the SchemaSystem. We already have an issue to return a wrapped version of Rc<Schema> but we can shift to Arc<Schema> instead for this wrapper version. I think this will solve your issue and hide this implementation detail from public facing API.

Reference issue: https://github.com/amazon-ion/ion-schema-rust/issues/36