Merge pull request #1539 from aml-org/W-11437709

W-11437709: Move validations from Parsing to Validation stage
aml-org · Aug 10, 2022 · 1b0aa4f · 1b0aa4f
2 parents b4626e9 + 448903c
commit 1b0aa4f
Show file tree

Hide file tree

Showing 30 changed files with 386 additions and 616 deletions.
diff --git a/...rc/main/scala/amf/apicontract/internal/validation/definitions/ParserSideValidations.scala b/...rc/main/scala/amf/apicontract/internal/validation/definitions/ParserSideValidations.scala
@@ -392,36 +392,16 @@ object ParserSideValidations extends Validations {
  "Message header must be of type object"
  )
 
- val InvalidDirectiveApplication = validation(
- "invalid-directive-application",
- "Directive should not be applied in current location"
- )
-
  val InvalidDirectiveLocation = validation(
  "invalid-directive-location",
  "The location of the directive is invalid"
  )
 
- val DuplicatedField = validation(
- id = "duplicated-field",
- message = "Cannot exist two or more fields with same name"
- )
-
- val DuplicatedArgument = validation(
- id = "duplicated-argument",
- message = "Cannot exist two or more arguments with same name"
- )
-
  val DuplicatedDeclaration = validation(
  id = "duplicated-declaration",
  message = "Cannot exist two or more declarations with same name"
  )
 
- val DuplicatedDirectiveApplication = validation(
- id = "duplicated-directive-application",
- message = "Directive can only be applied once per location"
- )
-
  val UnmatchedFieldInFieldSet = validation(
  id = "unmatched-field-in-field-set",
  message = "Cannot find property when resolving fieldSet"
@@ -446,7 +426,6 @@ object ParserSideValidations extends Validations {
  invalidExampleFieldWarning.id -> all(WARNING), // TODO: should be violation
  OasInvalidParameterSchema.id -> all(WARNING), // TODO: should be violation
  InvalidAllowedTargets.id -> all(WARNING), // TODO: should be violation
- InvalidDirectiveApplication.id -> all(VIOLATION),
  InvalidDirectiveLocation.id -> all(VIOLATION),
  InvalidPayload.id -> all(VIOLATION),
  ImplicitVersionParameterWithoutApiVersion.id -> all(WARNING), // TODO: should be violation
@@ -491,7 +470,6 @@ object ParserSideValidations extends Validations {
  SchemasDeprecated,
  InvalidDocumentationType,
  InvalidAllowedTargetsType,
- InvalidDirectiveApplication,
  InvalidDirectiveLocation,
  InvalidExtensionsType,
  ModuleNotFound,
@@ -509,9 +487,6 @@ object ParserSideValidations extends Validations {
  InvalidStatusCode,
  HeaderMustBeObject,
  InvalidModuleType,
- DuplicatedField,
- DuplicatedArgument,
- DuplicatedDeclaration,
- DuplicatedDirectiveApplication
+ DuplicatedDeclaration
  )
 }
diff --git a/...t/shared/src/main/scala/amf/apicontract/internal/validation/model/APIRawValidations.scala b/...t/shared/src/main/scala/amf/apicontract/internal/validation/model/APIRawValidations.scala
@@ -1183,6 +1183,36 @@ object APIRawValidations extends CommonValidationDefinitions {
  owlProperty = apiContract("path"),
  constraint = shape("invalidOutputTypeInEndpoint"),
  message = "Only an Output Type can be placed here"
+ ),
+ AMFValidation(
+ uri = amfParser("duplicated-directive-application"),
+ owlClass = doc("DomainElement"),
+ owlProperty = doc("customDomainProperties"),
+ constraint = shape("duplicatedDirectiveApplication")
+ ),
+ AMFValidation(
+ uri = amfParser("duplicated-field"),
+ owlClass = sh("NodeShape"),
+ owlProperty = sh("PropertyShape"),
+ constraint = shape("duplicatedField")
+ ),
+ AMFValidation(
+ uri = amfParser("duplicated-argument-field"),
+ owlClass = sh("NodeShape"),
+ owlProperty = shape("Request"),
+ constraint = shape("duplicatedArgumentField")
+ ),
+ AMFValidation(
+ uri = amfParser("duplicated-argument-directive"),
+ owlClass = doc("DomainProperty"),
+ owlProperty = shape("Request"),
+ constraint = shape("duplicatedArgumentDirective")
+ ),
+ AMFValidation (
+ uri = amfParser("invalid-directive-application"),
+ owlClass = doc("DomainElement"),
+ owlProperty = doc("customDomainProperties"),
+ constraint = shape("invalidDirectiveApplication")
  )
  )
  override def validations(): Seq[AMFValidation] = result

diff --git a/...ed/src/main/scala/amf/apicontract/internal/validation/shacl/APICustomShaclFunctions.scala b/...ed/src/main/scala/amf/apicontract/internal/validation/shacl/APICustomShaclFunctions.scala
@@ -10,20 +10,9 @@ import amf.apicontract.internal.metamodel.domain.security.{
  OpenIdConnectSettingsModel,
  SecuritySchemeModel
 }
-import amf.apicontract.internal.metamodel.domain.{
- CallbackModel,
- CorrelationIdModel,
- EndPointModel,
- ParameterModel,
- TemplatedLinkModel
-}
+import amf.apicontract.internal.metamodel.domain._
 import amf.apicontract.internal.validation.runtimeexpression.{AsyncExpressionValidator, Oas3ExpressionValidator}
-import amf.apicontract.internal.validation.shacl.graphql.{
- GraphQLAppliedDirective,
- GraphQLArgumentValidator,
- GraphQLEndpoint,
- GraphQLObject
-}
+import amf.apicontract.internal.validation.shacl.graphql._
 import amf.core.client.scala.model.domain._
 import amf.core.client.scala.model.domain.extensions.{CustomDomainProperty, DomainExtension, PropertyShape}
 import amf.core.internal.annotations.SynthesizedField
@@ -34,8 +23,9 @@ import amf.core.internal.metamodel.domain.extensions.{CustomDomainPropertyModel,
 import amf.core.internal.parser.domain.Annotations
 import amf.shapes.client.scala.model.domain._
 import amf.shapes.client.scala.model.domain.operations.AbstractParameter
+import amf.shapes.internal.annotations.DirectiveArguments
 import amf.shapes.internal.domain.metamodel._
-import amf.shapes.internal.domain.metamodel.operations.AbstractParameterModel
+import amf.shapes.internal.domain.metamodel.operations.{AbstractParameterModel, ShapeRequestModel}
 import amf.shapes.internal.validation.shacl.{BaseCustomShaclFunctions, ShapesCustomShaclFunctions}
 import amf.validation.internal.shacl.custom.CustomShaclValidator.{CustomShaclFunction, ValidationInfo}
 
@@ -562,6 +552,98 @@ object APICustomShaclFunctions extends BaseCustomShaclFunctions {
  )
  validationResults.foreach(info => validate(Some(info)))
  }
+ },
+ new CustomShaclFunction {
+ override val name: String = "duplicatedDirectiveApplication"
+ override def run(element: AmfObject, validate: Option[ValidationInfo] => Unit): Unit = {
+ element match {
+ case s: DomainElement =>
+ val directiveApplications = s.customDomainProperties
+ checkDuplicates(
+ directiveApplications,
+ validate,
+ ShapeModel.CustomDomainProperties,
+ { directiveName: String => s"Directive '$directiveName' can only be applied once per location" },
+ Some(s.annotations)
+ )
+ case _ => // ignore
+ }
+ }
+ },
+ new CustomShaclFunction {
+ override val name: String = "duplicatedField"
+ override def run(element: AmfObject, validate: Option[ValidationInfo] => Unit): Unit = {
+ val isDirectiveArgumentsShape = element.annotations.contains(classOf[DirectiveArguments])
+ element match {
+ case obj: NodeShape if !isDirectiveArgumentsShape =>
+ val fields = obj.properties ++ obj.operations
+ checkDuplicates(
+ fields,
+ validate,
+ NodeShapeModel.Properties,
+ { fieldName: String => s"Cannot exist two or more fields with name '$fieldName'" },
+ Some(obj.annotations)
+ )
+
+ case _ => // ignore
+ }
+ }
+ },
+ new CustomShaclFunction {
+ override val name: String = "duplicatedArgumentField"
+ override def run(element: AmfObject, validate: Option[ValidationInfo] => Unit): Unit = {
+ element match {
+ case obj: NodeShape =>
+ obj.operations.foreach({ op =>
+ val arguments = op.request.queryParameters
+ checkDuplicates(
+ arguments,
+ validate,
+ ShapeRequestModel.QueryParameters,
+ { argumentName: String => s"Cannot exist two or more arguments with name '$argumentName'" },
+ Some(op.annotations)
+ )
+ })
+ case _ => // ignore
+ }
+ }
+ },
+ new CustomShaclFunction {
+ override val name: String = "duplicatedArgumentDirective"
+ override def run(element: AmfObject, validate: Option[ValidationInfo] => Unit): Unit = {
+ element match {
+ case directive: CustomDomainProperty =>
+ val arguments = directive.schema.asInstanceOf[NodeShape].properties
+ checkDuplicates(
+ arguments,
+ validate,
+ NodeShapeModel.Properties,
+ { argumentName: String => s"Cannot exist two or more arguments with name '$argumentName'" },
+ Some(directive.annotations)
+ )
+ case _ => // ignore
+ }
+ }
+ },
+ new CustomShaclFunction {
+ override val name: String = "invalidDirectiveApplication"
+ override def run(element: AmfObject, validate: Option[ValidationInfo] => Unit): Unit = {
+ val isDirectiveArgument = element.annotations.contains(classOf[DirectiveArguments])
+ element match {
+ case directive: CustomDomainProperty =>
+ val arguments = directive.schema.asInstanceOf[NodeShape].properties
+ val results = arguments.flatMap({ arg =>
+ val directiveApplications = arg.customDomainProperties
+ GraphQLDirectiveLocationValidator(directiveApplications, arg, appliedToDirectiveArgument = true)
+ })
+ results.foreach(validate(_))
+ case elem: DomainElement if !isDirectiveArgument =>
+ val directiveApplications = elem.customDomainProperties
+ val results = GraphQLDirectiveLocationValidator(directiveApplications, elem)
+ results.foreach(validate(_))
+ case _ => // ignore
+ }
+ }
  }
  )
 

diff --git a/...amf/apicontract/internal/validation/shacl/graphql/GraphQLDirectiveLocationValidator.scala b/...amf/apicontract/internal/validation/shacl/graphql/GraphQLDirectiveLocationValidator.scala
@@ -0,0 +1,98 @@
+package amf.apicontract.internal.validation.shacl.graphql
+import amf.apicontract.client.scala.model.domain.EndPoint
+import amf.apicontract.client.scala.model.domain.api.WebApi
+import amf.core.client.scala.model.domain.{AmfScalar, DataNode, DomainElement}
+import amf.core.client.scala.model.domain.extensions.{DomainExtension, PropertyShape}
+import amf.core.internal.metamodel.domain.DomainElementModel
+import amf.core.internal.metamodel.domain.common.{NameFieldSchema, NameFieldShacl}
+import amf.core.internal.metamodel.domain.extensions.PropertyShapeModel
+import amf.shapes.client.scala.model.domain.{NodeShape, ScalarShape, UnionShape}
+import amf.shapes.client.scala.model.domain.operations.{ShapeOperation, ShapeParameter}
+import amf.shapes.internal.domain.metamodel.operations.ShapeParameterModel
+import amf.validation.internal.shacl.custom.CustomShaclValidator.ValidationInfo
+
+object GraphQLDirectiveLocationValidator {
+ def apply(
+ directiveApplications: Seq[DomainExtension],
+ element: DomainElement,
+ appliedToDirectiveArgument: Boolean = false
+ ): Seq[Option[ValidationInfo]] = {
+ directiveApplications
+ .map(application => validateApplication(application, element, appliedToDirectiveArgument))
+ .collect { case Some(x) => Some(x) }
+ }
+
+ private def validateApplication(
+ directiveApplication: DomainExtension,
+ element: DomainElement,
+ appliedToDirectiveArgument: Boolean = false
+ ): Option[ValidationInfo] = {
+ val validDomains = directiveApplication.definedBy.domain.map(_.toString)
+ val currentDomains = element.meta.typeIris // maybe head?
+ val isAValidLocation = currentDomains.exists(validDomains.contains)
+
+ // when parsing directives we parse arguments as property shapes of a virtual node shape
+ lazy val isValidApplicationToDirectiveArgument = appliedToDirectiveArgument && element.meta
+ .isInstanceOf[PropertyShapeModel.type] && validDomains.intersect(ShapeParameterModel.typeIris).nonEmpty
+
+ if (!isAValidLocation && !isValidApplicationToDirectiveArgument) {
+ val message = buildErrorMessage(directiveApplication, element, appliedToDirectiveArgument)
+ Some(
+ ValidationInfo(DomainElementModel.CustomDomainProperties, Some(message), Some(directiveApplication.annotations))
+ )
+ } else None
+ }
+
+ private def buildErrorMessage(
+ directiveApplication: DomainExtension,
+ element: DomainElement,
+ appliedToDirectiveArgument: Boolean
+ ) = {
+ val kind = inferGraphQLKind(element, appliedToDirectiveArgument)
+ val nameOpt: Option[String] = extractName(element)
+
+ nameOpt match {
+ case Some(name) => s"Directive ${directiveApplication.name.value()} cannot be applied to $kind $name"
+ case _ => s"Directive ${directiveApplication.name.value()} cannot be applied to $kind"
+ }
+ }
+ private def extractName(element: DomainElement) = {
+ element.fields
+ .getValueAsOption(NameFieldSchema.Name)
+ .orElse(element.fields.getValueAsOption(NameFieldShacl.Name))
+ .flatMap { v =>
+ element match {
+ case _: WebApi => None
+ case _: EndPoint =>
+ Some(
+ v.value
+ .asInstanceOf[AmfScalar]
+ .toString
+ .stripPrefix("Query.")
+ .stripPrefix("Mutation.")
+ .stripPrefix("Subscription.")
+ )
+ case _ => Some(v.value.asInstanceOf[AmfScalar].toString)
+ }
+ }
+ .map(name => s"'$name'")
+ }
+ private def inferGraphQLKind(element: DomainElement, appliedToDirectiveArgument: Boolean): String = {
+ element match {
+ case _: PropertyShape if appliedToDirectiveArgument => "argument"
+ case _: PropertyShape => "field"
+ case _: ShapeOperation => "field"
+ case _: ShapeParameter => "argument"
+ case s: ScalarShape if s.values.nonEmpty => "enum"
+ case _: ScalarShape => "scalar"
+ case n: NodeShape if n.isAbstract.value() => "interface"
+ case n: NodeShape if n.isInputOnly.value() => "input object"
+ case _: NodeShape => "object"
+ case _: UnionShape => "union"
+ case _: DataNode => "value"
+ case _: EndPoint => "field"
+ case _: WebApi => "schema"
+ case _ => "type" // should be unreachable
+ }
+ }
+}
diff --git a/...red/src/test/resources/graphql/tck/apis/invalid/duplicate-arguments.api.federation.report b/...red/src/test/resources/graphql/tck/apis/invalid/duplicate-arguments.api.federation.report
@@ -1,22 +1,14 @@
 ModelId: file://amf-cli/shared/src/test/resources/graphql/tck/apis/invalid/duplicate-arguments.api.graphql
 Profile: GraphQL
 Conforms: false
-Number of results: 2
+Number of results: 1
 
 Level: Violation
 
-- Constraint: http://a.ml/vocabularies/amf/parser#duplicated-argument
+- Constraint: http://a.ml/vocabularies/amf/parser#duplicated-argument-field
  Message: Cannot exist two or more arguments with name 'unit'
  Severity: Violation
- Target: file://amf-cli/shared/src/test/resources/graphql/tck/apis/invalid/duplicate-arguments.api.graphql#/declares/shape/Starship/supportedOperation/length/expects/request/parameter/parameter/unit
- Property: 
- Range: [(12,11)-(12,29)]
- Location: 
-
-- Constraint: http://a.ml/vocabularies/amf/parser#duplicated-argument
- Message: Cannot exist two or more arguments with name 'unit'
- Severity: Violation
- Target: file://amf-cli/shared/src/test/resources/graphql/tck/apis/invalid/duplicate-arguments.api.graphql#/declares/shape/Starship/supportedOperation/length/expects/request/parameter/parameter/unit_1
- Property: 
- Range: [(12,31)-(12,48)]
+ Target: file://amf-cli/shared/src/test/resources/graphql/tck/apis/invalid/duplicate-arguments.api.graphql#/declares/shape/Starship
+ Property: http://a.ml/vocabularies/shapes#parameter
+ Range: [(12,4)-(12,56)]
  Location: 
diff --git a/amf-cli/shared/src/test/resources/graphql/tck/apis/invalid/duplicate-arguments.api.report b/amf-cli/shared/src/test/resources/graphql/tck/apis/invalid/duplicate-arguments.api.report
@@ -1,22 +1,14 @@
 ModelId: file://amf-cli/shared/src/test/resources/graphql/tck/apis/invalid/duplicate-arguments.api.graphql
 Profile: GraphQL
 Conforms: false
-Number of results: 2
+Number of results: 1
 
 Level: Violation
 
-- Constraint: http://a.ml/vocabularies/amf/parser#duplicated-argument
+- Constraint: http://a.ml/vocabularies/amf/parser#duplicated-argument-field
  Message: Cannot exist two or more arguments with name 'unit'
  Severity: Violation
- Target: file://amf-cli/shared/src/test/resources/graphql/tck/apis/invalid/duplicate-arguments.api.graphql#/declares/shape/Starship/supportedOperation/length/expects/request/parameter/parameter/unit
- Property: 
- Range: [(12,11)-(12,29)]
- Location: 
-
-- Constraint: http://a.ml/vocabularies/amf/parser#duplicated-argument
- Message: Cannot exist two or more arguments with name 'unit'
- Severity: Violation
- Target: file://amf-cli/shared/src/test/resources/graphql/tck/apis/invalid/duplicate-arguments.api.graphql#/declares/shape/Starship/supportedOperation/length/expects/request/parameter/parameter/unit_1
- Property: 
- Range: [(12,31)-(12,48)]
+ Target: file://amf-cli/shared/src/test/resources/graphql/tck/apis/invalid/duplicate-arguments.api.graphql#/declares/shape/Starship
+ Property: http://a.ml/vocabularies/shapes#parameter
+ Range: [(12,4)-(12,56)]
  Location: