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

Support min and max length for a string property in CTO #643

Closed
ragi-dayananda opened this issue May 8, 2023 · 6 comments · Fixed by accordproject/concerto-metamodel#3
Closed
Assignees

Comments

@ragi-dayananda
Copy link
Contributor

Feature Request 🛍️

Currently, we have the option to use regex to enforce string length, but this can be an expensive operation. Considering that major data modeling tools support string length validators, it would be beneficial to have this functionality in Concerto as well.

Possible Solution

Solution 1: Introduce a new decorator to support this feature.

  • This could an easy option in terms of implementation.
  • Example -
concept Contact {
  @length(0,100)
   o String firstName
} 

Solution 2: Enhance CTO property definition to incorporate this feature.

  • I noticed the existence of the meta property range[1,10] to declare a valid range for numeric properties. We can leverage this meta property for strings as well, where it would represent the length of a string.
  • Example -
concept Contact {
   o String firstName range[0,100]
} 
  • If using the range meta property is not suitable for string properties, we can introduce a new meta property, perhaps called length to support this feature.

Context

We are in the process of replacing our data models with Concerto models. Some of the string properties in our data model have string length configurations/validators, and it would be great to have a similar setting in Concerto.

@dselman
Copy link
Contributor

dselman commented May 8, 2023

We should only use decorators for things that are external/opaque to Concerto. In this case we'd be using these facets to validate string properties so I think the meta model should be extended to support this. 'length' seems like a good name, and we could implement it similar to range for numbers, with an optional lower and upper bound, or we could add 'maxLength' and 'minLength' as explicit meta properties.

@mttrbrts
Copy link
Member

mttrbrts commented May 8, 2023

The range property modifier syntax includes an = so we should mirror that. e.g.

concept Contact {
   o String firstName length=[0,100]
}

We could also make lower and upper bounds optional (like with range), e.g.

concept Contact {
   o String firstName length=[1,] // only non-empty strings are allowed
   o String lastName length=[,100] // syntactic sugar for [0,100]
}

Also note that with range, the upper-bound is inclusive, i.e. range=[3,10] means 3 >= x <= 10

validate(identifier, value) {
if(value !== null) {
if(this.lowerBound !== null && value < this.lowerBound) {
this.reportError(identifier, `Value ${value} is outside lower bound ${this.lowerBound}`);
}
if(this.upperBound !== null && value > this.upperBound) {
this.reportError(identifier, `Value ${value} is outside upper bound ${this.upperBound}`);
}
}
}

@mttrbrts
Copy link
Member

As discussed on today's working group call, we should give this modifier precedence over the regex modifier if both are used.

Validating against length is the simpler, faster, and less-general operation.

@f5th-dimensional
Copy link

f5th-dimensional commented May 10, 2023

For translating these field constraints/modifiers into things such as OpenAPI, would using common labels such as those in JSON schema be practical? For example, as @dselman suggested, maxLength and minLength to describe character limits in a string, or minimum maximum to describe the range of possible values. Also, should distinguishing between inclusive and exclusive range values be considered, such as in the use of exclusiveMinimum and exclusiveMaximum?

-Steven

@mttrbrts
Copy link
Member

mttrbrts commented May 16, 2023

For translating these field constraints/modifiers into things such as OpenAPI, would using common labels such as those in JSON schema be practical?

@f5th-dimensional
Are you suggesting using the same keywords from OpenAPI in Concerto, or just declaring an equivalence when converting to/from OpenAPI.

@dselman
Copy link
Contributor

dselman commented May 19, 2023

Please ensure that the latest metamodel is publish to the public model repo: https://models.accordproject.org/concerto/[email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants