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

Validate that messages to vital messages and their return messages #451

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
285 changes: 158 additions & 127 deletions language/src/main/scala/com/reactific/riddl/language/AST.scala

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
package com.reactific.riddl.language.parsing

import com.reactific.riddl.language.AST.*
import Terminals.*
import Terminals.{Keywords, *}
import fastparse.*
import fastparse.ScalaWhitespace.*

Expand All @@ -17,21 +17,16 @@ private[parsing] trait ApplicationParser {
with ReferenceParser
with HandlerParser
with StatementParser
with TypeParser =>
with TypeParser
with CommonParser =>

private def applicationOptions[u: P]: P[Seq[ApplicationOption]] = {
options[u, ApplicationOption](StringIn(Options.technology).!) { case (loc, Options.technology, args) =>
ApplicationTechnologyOption(loc, args)
}
}

private def groupAliases[u: P]: P[String] = {
P(
StringIn(Keywords.group, "page", "pane", "dialog", "popup", "frame", "column", "window", "section", "tab").!
)
}

private def groupDefinitions[u:P]: P[Seq[GroupDefinition]] = {
private def groupDefinitions[u: P]: P[Seq[GroupDefinition]] = {
P {
undefined(Seq.empty[GroupDefinition]) | (group | appOutput | appInput).rep(0)
}
Expand All @@ -40,47 +35,56 @@ private[parsing] trait ApplicationParser {
private def group[u: P]: P[Group] = {
P(
location ~ groupAliases ~ identifier ~/ is ~ open ~
groupDefinitions ~
close ~ briefly ~ description
groupDefinitions ~
close ~ briefly ~ description
).map { case (loc, alias, id, elements, brief, description) =>
Group(loc, id, alias, elements, brief, description)
Group(loc, alias, id, elements, brief, description)
}
}

private def outputAliases[u: P]: P[String] = {
private def presentationAliases[u: P]: P[Unit] = {
StringIn(Keywords.presents, "shows", "displays", "writes", "emits")
}

private def outputDefinitions[u: P]: P[Seq[OutputDefinition]] = {
P(
StringIn(
Keywords.output,
"document",
"list",
"table",
"graph",
"animation",
"picture"
).!
)
is ~ open ~ appOutput.rep(1) ~ close
).?.map {
case Some(definitions) => definitions
case None => Seq.empty[OutputDefinition]
}
}

private def appOutput[u: P]: P[Output] = {
P(
location ~ outputAliases ~/ identifier ~ Keywords.presents ~/
messageRef ~ briefly ~ description
).map { case (loc, alias, id, putOut, brief, description) =>
Output(loc, id, alias, putOut, brief, description)
location ~ outputAliases ~/ identifier ~ presentationAliases ~/ typeRef ~
outputDefinitions ~ briefly ~ description
).map { case (loc, alias, id, putOut, outputs, brief, description) =>
Output(loc, alias, id, putOut, outputs, brief, description)
}
}

private def inputAliases[u: P]: P[String] = {
private def inputDefinitions[uP: P]: P[Seq[InputDefinition]] = {
P(
StringIn(Keywords.input, "form", "textblock", "button", "picklist").!
)
is ~ open ~
(undefined(Seq.empty[InputDefinition]) | appInput.rep(1))
~ close
).?.map {
case Some(definitions) => definitions
case None => Seq.empty[InputDefinition]
}
}

private def acquisitionAliases[u: P]: P[Unit] = {
StringIn(Keywords.acquires, "reads", "takes", "accepts", "admits")
}

private def appInput[u: P]: P[Input] = {
P(
location ~ inputAliases ~/ identifier ~ is ~ Keywords.acquires ~ messageRef ~ briefly ~ description
).map { case (loc, alias, id, putIn, brief, description) =>
Input(loc, id, alias, putIn, brief, description)
location ~ inputAliases ~/ identifier ~ acquisitionAliases ~ typeRef ~
inputDefinitions ~ briefly ~ description
).map { case (loc, alias, id, putIn, inputs, brief, description) =>
Input(loc, alias, id, putIn, inputs, brief, description)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,4 +226,31 @@ private[parsing] trait CommonParser extends NoWhiteSpaceParsers {
def term[u: P]: P[Term] = {
P(location ~ Keywords.term ~ identifier ~ is ~ briefly ~ description)./.map(tpl => (Term.apply _).tupled(tpl))
}

def groupAliases[u: P]: P[String] = {
P(
StringIn(Keywords.group, "page", "pane", "dialog", "popup", "frame", "column", "window", "section", "tab").!
)
}

def inputAliases[u: P]: P[String] = {
P(
StringIn(Keywords.input, "form", "text", "button", "picklist", "select").!
)
}

def outputAliases[u: P]: P[String] = {
P(
StringIn(
Keywords.output,
"document",
"list",
"table",
"graph",
"animation",
"picture"
).!
)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -156,18 +156,18 @@ private[parsing] trait ReferenceParser extends CommonParser {
}

def outputRef[u: P]: P[OutputRef] = {
P(location ~ Keywords.output ~ pathIdentifier)
.map(tpl => (OutputRef.apply _).tupled(tpl))
P(location ~ outputAliases ~ pathIdentifier)
.map { case (loc, _, pid) => OutputRef(loc, pid) }
}

def inputRef[u: P]: P[InputRef] = {
P(location ~ Keywords.input ~ pathIdentifier)
.map(tpl => (InputRef.apply _).tupled(tpl))
P(location ~ inputAliases ~ pathIdentifier)
.map { case (loc, _, pid) => InputRef(loc, pid) }
}

def groupRef[u:P]: P[GroupRef] = {
P(location ~ Keywords.group ~ pathIdentifier)
.map(tpl => (GroupRef.apply _).tupled(tpl))
def groupRef[u: P]: P[GroupRef] = {
P(location ~ groupAliases ~ pathIdentifier)
.map { case (loc, _, pid) => GroupRef(loc, pid) }
}

def authorRefs[u: P]: P[Seq[AuthorRef]] = {
Expand All @@ -183,11 +183,10 @@ private[parsing] trait ReferenceParser extends CommonParser {
)
}

def arbitraryInteractionRef[u:P]: P[Reference[Definition]] = {
P( processorRef | sagaRef | inputRef | outputRef | groupRef )
def arbitraryInteractionRef[u: P]: P[Reference[Definition]] = {
P(processorRef | sagaRef | inputRef | outputRef | groupRef)
}


def anyInteractionRef[u: P]: P[Reference[Definition]] = {
arbitraryInteractionRef | userRef
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,14 @@ trait BasicValidation {
}
}

def checkTypeRef(
ref: TypeRef,
topDef: Definition,
parents: Seq[Definition]
): Option[Type] = {
checkRef[Type](ref, topDef, parents)
}

def checkMessageRef(
ref: MessageRef,
topDef: Definition,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ case class ValidationPass(input: PassInput) extends Pass(input) with StreamingVa
case d: Domain =>
validateDomain(d, parentsAsSeq)
case s: Epic =>
validateStory(s, parentsAsSeq)
validateEpic(s, parentsAsSeq)
case a: Application =>
validateApplication(a, parentsAsSeq)
case r: Replica =>
Expand Down Expand Up @@ -524,7 +524,7 @@ case class ValidationPass(input: PassInput) extends Pass(input) with StreamingVa
checkDescription(replica)
}

private def validateStory(
private def validateEpic(
s: Epic,
parents: Seq[Definition]
): Unit = {
Expand Down Expand Up @@ -560,7 +560,7 @@ case class ValidationPass(input: PassInput) extends Pass(input) with StreamingVa
val parentsSeq = parents.toSeq
checkDefinition(parentsSeq, in)
// FIXME: validate this at the epic/case level so the restriction occurs only when sent to the backend
checkMessageRef(in.putIn, in, parentsSeq, Seq(RecordCase, CommandCase, QueryCase))
checkTypeRef(in.putIn, in, parentsSeq)
checkDescription(in)
}

Expand All @@ -570,8 +570,7 @@ case class ValidationPass(input: PassInput) extends Pass(input) with StreamingVa
): Unit = {
checkDefinition(parents, out)
// FIXME: validate this at the epic/case level so the restriction occurs only when received from the backend
checkMessageRef(out.putOut, out, parents, Seq(RecordCase, EventCase, ResultCase))

checkTypeRef(out.putOut, out, parents)
checkDescription(out)
}

Expand Down Expand Up @@ -646,6 +645,68 @@ case class ValidationPass(input: PassInput) extends Pass(input) with StreamingVa
checkDescription(uc)
}

@SuppressWarnings(Array("org.wartremover.warts.IterableOps"))
private def validateArbitraryInteraction(
origin: Option[Definition],
destination: Option[Definition],
parents: Seq[Definition]
): Unit = {
val maybeMessage: Option[Message] = origin match {
case Some(o) if o.isVital =>
destination match {
case Some(d) if d.isAppRelated =>
d match {
case output @ Output(loc, alias, id, putOut, _, _, _) =>
checkTypeRef(putOut, parents.head, parents.tail) match {
case Some(Type(_,_,typEx,_,_)) if typEx.isContainer =>
typEx match {
case ate: AggregateUseCaseTypeExpression if ate.usecase == EventCase || ate.usecase == ResultCase =>
None // events and results are permitted
case ty: TypeExpression => // everything else is not
Some(
error(s"${output.identify} showing ${putOut.format} of type ${ty.format} is invalid "+
s" because ${o.identify} is a vital definition which can only send Events and Results", loc
)
)
}
case _ => None //
}
case _ => None
}
case _ => None
}
case Some(o) if o.isAppRelated =>
destination match {
case Some(d) if d.isVital =>
o match {
case input @ Input(loc, alias, id, putIn, _, _, _) =>
checkTypeRef(putIn, parents.head, parents.tail) match {
case Some(Type(_, _, typEx, _, _)) if typEx.isContainer =>
typEx match {
case ate: AggregateUseCaseTypeExpression if ate.usecase == CommandCase || ate.usecase == QueryCase =>
None // commands and queries are permitted
case ty: TypeExpression => // everything else is not
Some(
error(s"${input.identify} sending ${putIn.format} of type ${ty.format} is invalid "+
s" because ${d.identify} is a vital definition which can only receive Commands and Queries")
)
}
case _ => None
}
case _ => None
}
case _ => None
}
case _ => None
}
maybeMessage match {
case Some(m: Message) =>
messages.add(m)
case None => ()
}
}

@SuppressWarnings(Array("org.wartremover.warts.IterableOps"))
private def validateInteraction(interaction: Interaction, parents: Seq[Definition]): Unit = {
checkDefinition(parents, interaction)
interaction match {
Expand All @@ -656,6 +717,9 @@ case class ValidationPass(input: PassInput) extends Pass(input) with StreamingVa
case ArbitraryInteraction(_, _, from, _, to, _, _) =>
checkRef[Definition](from, interaction, parents)
checkRef[Definition](to, interaction, parents)
val origin = resolution.refMap.definitionOf[Definition](from.pathId, parents.head)
val destination = resolution.refMap.definitionOf[Definition](to.pathId, parents.head)
validateArbitraryInteraction(origin, destination, parents)
case ShowOutputInteraction(_, _, from: OutputRef, _, to: UserRef, _, _) =>
checkRef[Output](from, interaction, parents)
checkRef[User](to, interaction, parents)
Expand Down
21 changes: 21 additions & 0 deletions testkit/src/test/input/issues/447.riddl
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
domain example is {
context TenantControl {
command CreateTenant { name: String }
event TenantCreated { name: String, result: String }
}
record Create { name: String }
user Admin is "fickle"
application App is {
page NewTenantPage {
input NameEntry acquires record example.Create
button Create acquires record example.Create
}
}
epic check_for_wrong_types_to_and_from_vitals {
case UserCreatesANewCompany {
user Admin wants to "create a new tenant" so that "the tenant can do stuff"
step from user Admin "presses" button App.NewTenantPage.Create,
step from button App.NewTenantPage.Create "sends" to context TenantControl
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,5 +96,13 @@ class ReportedIssuesTest extends ValidatingTest {
succeed
}
}
"447" in {
checkOne("447.riddl") {
case Left(messages) =>
fail(messages.format)
case Right(result) =>
succeed
}
}
}
}
Loading