amazon-ion / ion-schema

The Ion Schema Specification. This specification is licensed under the Apache 2.0 License.
https://amazon-ion.github.io/ion-schema/
Apache License 2.0
13 stars 10 forks source link

Add support for custom annotation on Type reference #132

Open r0b0ji opened 7 months ago

r0b0ji commented 7 months ago

I have a usecase to extend Type to add some custom logic.

For example, a new Type may have some fields which is mandatory but when comparing two instances of that type I'd like to ignore few fields. We can achieve this by annotating few fields with our custom annotation say compare.ignore and then parse the schema and have a list of fields to be ignored in Type and then while comparing we remove those ignored fields from both left and right before comparing.

public class MyCustomType {
    private final Type type;
    private final Set<String> comparatorIgnoredFields;
    ...

}

A corresponding schema may look something like:

type::{
    name: 'my_custom_type@1.0',
    type: struct,
    fields: {
       a: {
            type: int,
            occurs: 1
        },
        b:{
            type: string,
            occurs: 1
        },
        c:'compare_ignore':: {
            type: timestamp,
            occurs: 1
        }
    }
}

But this fails as ISL doesn't allow any annotation other than some default annotations [1].

ISL itself is Ion doc and as any Ion doc it could support any custom annotation and have it behave as passthru and .getIsl() on type can return the type IonValue as it is along with any annotation and let the client handle that however they want.

[1] https://github.com/amazon-ion/ion-schema-kotlin/blob/bbab678c12fdd07bcb9b5d632fb7364a04541a73/ion-schema/src/main/kotlin/com/amazon/ionschema/internal/TypeReference.kt#L58

A related issue: https://github.com/amazon-ion/ion-schema-kotlin/issues/303

r0b0ji commented 7 months ago

There is a workaround given in the related issue https://github.com/amazon-ion/ion-schema-kotlin/issues/303 but I opened this issue to discuss the possibility of supporting this in ISL or if not then why it is not a good idea.

popematt commented 7 months ago

Thanks for suggesting this feature. From an implementation perspective, it is certainly possible to allow other annotations on type references. I think the next step here is to determine the answers to at least these questions to help figure out how this fits with the rest of the Ion Schema Language.

  1. What use cases are served by this? Specifically, why is it better in those cases to annotate a type reference rather than adding a field to a type definition or a field definition?
  2. How do these extra annotations interact with the annotations that are already built into Ion Schema Language? (Does order matter?)
  3. If we proceed, we cannot allow completely arbitrary annotations. (See https://amazon-ion.github.io/ion-schema/rfcs/ion_schema_2_0/open_content.html —allowing unrestricted annotations has the same problems as allowing unrestricted fields.) What subset of annotations should be reserved for system use? It cannot be the same as the reserved words for field names because ISL 2.0 already has a $null_or annotation.
r0b0ji commented 7 months ago

Thanks for the response.

  1. The usecase mentioned was one of the usecase. But it can also be used to create custom/user defined validators. After looking at the workaround, I can see that this can be achieved by adding a field to type definition. The annotation support looks like a syntactic sugar to that. Also for primitive types, it will be easier to add annotation than to add a field to type definition.
  2. In Ion, generally the order of annotation doesn't matter. Most common way would be to check if a field has annotation like .hasTypeAnnotation("my_custom_annotation"). Similarly, I'd expect ISL shouldn't have any order dependent logic either. For internal built-in annotations, ISL should also just check if hasTypeAnnotation("any_isl_annotation") or the annotations contains the required annotation.
  3. I'd also prefer not to allow completely arbitrary annotations. Any annotations which type creator intends to add to type can be declared before as allowed annotation, something like user_reserved_field_annotations or similar.