accordproject / concerto

Business schema language and runtime
https://concerto.accordproject.org
Apache License 2.0
109 stars 99 forks source link

Concerto scalar inline defaults #580

Open stefanblaginov opened 1 year ago

stefanblaginov commented 1 year ago

Feature Request 🛍️

💡 Proposed by @DS-AdamMilazzo here: https://github.com/accordproject/concerto/pull/546#issuecomment-1334137210.

We would like to be able to extend a scalar’s default property inside a class after its definition.

Example:

scalar SSN extends String regex=/\d{3}-\d{2}-\d{4}+/

concept Person identified by ssn {
    o SSN ssn default="999-99-9999"
}

This would be effectively the same as:

scalar SSN extends String default="999-99-9999" regex=/\d{3}-\d{2}-\d{4}+/

concept Person identified by ssn {
    o SSN ssn
}

or

concept Person identified by ssn {
    o String ssn default="999-99-9999" regex=/\d{3}-\d{2}-\d{4}+/
}

Design questions

Should the default meta property be part of the scalar type?

default is not part of the scalar type

If the default meta property is not part of the scalar type, but simply an add-on (similar to optional), then the inline overriding of the default would not be considered a violation of the type and the following would be possible:

scalar SSN extends String default="000-00-0000" regex=/\d{3}-\d{2}-\d{4}+/

concept Person identified by ssn {
    o SSN ssn default="999-99-9999"
}

Drawback of this approach: Boolean scalars become useless, as their only meta property currently is the default.

Which would effectively be the same as:

concept Person identified by ssn {
    o String ssn default="999-99-9999" regex=/\d{3}-\d{2}-\d{4}+/
}

default is part of the scalar type

If the default meta property is part of the scalar type, then the inline overriding of the default would be considered a violation of the type and the following would throw an error:

scalar SSN extends String default="000-00-0000" regex=/\d{3}-\d{2}-\d{4}+/

concept Person identified by ssn {
    o SSN ssn default="999-99-9999"
}

This would force the developer to define a new scalar with the desired default:

scalar SSN extends String default="000-00-0000" regex=/\d{3}-\d{2}-\d{4}+/

concept Person identified by ssn {
    o SSN ssn
}

scalar OtherSSN extends String default="999-99-9999" regex=/\d{3}-\d{2}-\d{4}+/

concept OtherPerson identified by ssn {
    o OtherSSN ssn
}

Drawback of this approach: The developer may need to define an entirely new scalar if he wants to have a different default.

One think to note when deciding on a path forward is that we can always implement the "default is part of the scalar type" approach and then later transition to the "default is not part of the scalar type" as a non-braking change, while the reverse is not possible.

What about range?

Although not in the scope of this issue, it may be useful to look forward to the question of extending the range meta property.

My opinion is that range should be part of the scalar type (otherwise scalars lose their purpose). What could be possible would be for the child range to always be more restrictive than the parent one. This, however, would mean that we would need to check this greater restrictiveness.

Example of a valid range extension:

scalar Age extends Integer range=[0,]

concept Person {
    o Age age
}

concept Adult {
    o Age age range=[18,]
}

Example of an invalid range extension:

scalar Age extends Integer range=[0,]

concept Person {
    o Age age
}

concept Unborn {
    o Age age range=[-1,]
}
stefanblaginov commented 1 year ago

@DS-AdamMilazzo what would the business case for this feature be (this would better inform us on what approach to take)? Additionally, how would you recommend that we prioritise this compared to other issues?

DS-AdamMilazzo commented 1 year ago

I don't have a business case. I just believe that if we take the word "type" to mean "a declaration that defines a set of allowable values" then properly speaking the default attribute is not part of the type but part of how the type is used. Therefore it belongs in a field declaration and not the type declaration. But I can see it going on the type definition too as a convenience even if it's strictly speaking it doesn't belong there. And anyway, it's already there on the type and I wouldn't say to remove it.

You didn't consider a third possibility: allow default on both the scalar type and the field declaration, and allow it to be overridden. I don't see why you consider this a violation of the type:

scalar SSN extends String default="000-00-0000" regex=/\d{3}-\d{2}-\d{4}+/

concept Person identified by ssn {
    o SSN ssn default="999-99-9999"
}

The type is "all the strings that match regex /\d{3}-\d{2}-\d{4}+/". If "999-99-9999" is a legal value, which it is, then it's a legal default value, and not a violation of the type. SSN ssn default="abc" would be a violation, on the other hand.

Anyway, I don't consider it to be high priority. Just normal priority.

I would leave range out of it for now.