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

feat(decorators): validate decorators against model #916

Merged
merged 20 commits into from
Oct 16, 2024

Conversation

dselman
Copy link
Contributor

@dselman dselman commented Oct 2, 2024

Closes #391

Adds a new option to the model manager to validate the names and structure of decorators and their arguments:

  • Decorator must have been defined
  • Arguments must match expected types (String,Integer,Long,Double,Boolean)
  • Optional arguments are supported
  • Arguments that are a concept reference must be assignable

Changes

Documentation updates: accordproject/concerto-docs#61

Using this Model Manager configuration:

validatedModelManager = new ModelManager({
            strict: true, decoratorValidation: {
                missingDecorator: 'error',
                invalidDecorator: 'error'
            }
        });

This file will error, because it uses the undeclared decorator @Hide:

namespace [email protected]
concept Person {
   o String name
   @Hide
   o String ssn
   @Hide(false)
   o Integer age
}

While this file will not:

namespace [email protected]
import [email protected]

concept Hide extends Decorator {
    o Boolean hidden optional
}

concept Person {
   o String name
   @Hide
   o String ssn
   @Hide(false)
   o Integer age
}

Flags

  • Default behaviour should be unchanged (no validation)
  • DecoratorManager.decorateModels has a new option defaultNamespace to support decorating model managers with command sets when the model manager decoratorValidation option is ON. See below.
  • A new "system" model file has been created [email protected] that defines the abstract Decorator type.

The Decorator Command Set model has been updated to support an optional namespace property for DecoratorCommand.decorator (in addition to the existing decorator name). This is required so that the DecoratorManager can create imports for the decorators that are being added to a model file. If the namespace property is missing on a decorator command then the defaultNamespace option passed to DecoratorManager is used. If both are null and a decorator is applied, and decoratorValidation is ON then the model file will be flagged by the model manager as invalid.

Screenshots or Video

Related Issues

  • Issue #
  • Pull Request #

Author Checklist

  • Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
  • Vital features and changes captured in unit and/or integration tests
  • Commits messages follow AP format
  • Extend the documentation, if necessary
  • Merging to main from fork:branchname

@dselman dselman marked this pull request as draft October 2, 2024 19:29
@@ -762,8 +781,10 @@ class BaseModelManager {
fromAst(ast) {
this.clearModelFiles();
ast.models.forEach( model => {
const modelFile = new ModelFile( this, model );
this.addModelFile( modelFile, null, null, true );
if(model.namespace !== '[email protected]') { // excludes the Concerto namespace, already added
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a system utility (in concerto-util, I think), that identifies system namespaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't see that...

packages/concerto-core/lib/rootmodel.js Outdated Show resolved Hide resolved
packages/concerto-core/lib/rootmodel.js Outdated Show resolved Hide resolved
Signed-off-by: Dan Selman <[email protected]>
Signed-off-by: Dan Selman <[email protected]>
@dselman dselman marked this pull request as ready for review October 4, 2024 15:42
@dselman dselman requested a review from mttrbrts October 4, 2024 15:44
Signed-off-by: Dan Selman <[email protected]>
Signed-off-by: Dan Selman <[email protected]>
@dselman dselman requested a review from sanketshevkar October 7, 2024 18:37
dselman and others added 13 commits October 8, 2024 07:10
* fix(decorator): validate type references
* chore(deps): update concerto-metamodel to 3.10.1
* chore(npm): move to npm workspaces
* chore(concerto-util): remove axios and colors

Signed-off-by: Dan Selman <[email protected]>
Signed-off-by: GitHub <[email protected]>
Co-authored-by: dselman <[email protected]>
Signed-off-by: Dan Selman <[email protected]>
Signed-off-by: Dan Selman <[email protected]>
Signed-off-by: Dan Selman <[email protected]>
Signed-off-by: Dan Selman <[email protected]>
Signed-off-by: Dan Selman <[email protected]>
@dselman dselman merged commit bcdbd34 into main Oct 16, 2024
11 checks passed
@dselman dselman deleted the ds-validate-decorators branch October 16, 2024 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Decorator Validation
3 participants