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

Add support for custom annotation on Type reference #132

Open
r0b0ji opened this issue Feb 20, 2024 · 3 comments
Open

Add support for custom annotation on Type reference #132

r0b0ji opened this issue Feb 20, 2024 · 3 comments
Labels
enhancement New feature or enhancement for the Ion Schema _specification_ requires new version Something that should be considered for next version of the Ion Schema Specification

Comments

@r0b0ji
Copy link

r0b0ji commented Feb 20, 2024

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: '[email protected]',
    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: amazon-ion/ion-schema-kotlin#303

@r0b0ji r0b0ji added the enhancement New feature or enhancement for the Ion Schema _specification_ label Feb 20, 2024
@r0b0ji
Copy link
Author

r0b0ji commented Feb 20, 2024

There is a workaround given in the related issue amazon-ion/ion-schema-kotlin#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
Copy link
Contributor

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.

@popematt popematt added the requires new version Something that should be considered for next version of the Ion Schema Specification label Feb 20, 2024
@r0b0ji
Copy link
Author

r0b0ji commented Feb 21, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or enhancement for the Ion Schema _specification_ requires new version Something that should be considered for next version of the Ion Schema Specification
Projects
None yet
Development

No branches or pull requests

2 participants