From 2a8f15f07c2b7c2b87edf9f37288b246d75c0d8a Mon Sep 17 00:00:00 2001 From: Ryan McNally Date: Wed, 26 Oct 2022 09:31:28 +0100 Subject: [PATCH] Dependency inclusion (#127) * simplified api * Added validation --- .../flow/validation/AbstractValidator.java | 2 + .../test/flow/validation/Violation.java | 18 +- .../validation/check/ChainOverlapCheck.java | 2 +- .../check/DependencyChronologyCheck.java | 9 +- .../check/DependencyInclusionCheck.java | 51 +++++ .../flow/validation/check/FlowPairCheck.java | 2 +- .../check/InteractionIdentityCheck.java | 2 +- .../validation/check/MessageSharingCheck.java | 7 +- .../flow/validation/check/ResultTagCheck.java | 2 +- .../validation/AbstractValidatorTest.java | 5 +- .../check/DependencyInclusionCheckTest.java | 193 ++++++++++++++++++ .../check/DependencyLoopCheckTest.java | 2 +- .../flow/validation/junit4/ExampleTest.java | 4 +- .../flow/validation/junit5/ExampleTest.java | 4 +- 14 files changed, 279 insertions(+), 24 deletions(-) create mode 100644 validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/DependencyInclusionCheck.java create mode 100644 validation/validation-core/src/test/java/com/mastercard/test/flow/validation/check/DependencyInclusionCheckTest.java diff --git a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/AbstractValidator.java b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/AbstractValidator.java index b875c4180a..ca4af8a1b8 100644 --- a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/AbstractValidator.java +++ b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/AbstractValidator.java @@ -12,6 +12,7 @@ import com.mastercard.test.flow.Model; import com.mastercard.test.flow.validation.check.ChainOverlapCheck; import com.mastercard.test.flow.validation.check.DependencyChronologyCheck; +import com.mastercard.test.flow.validation.check.DependencyInclusionCheck; import com.mastercard.test.flow.validation.check.DependencyLoopCheck; import com.mastercard.test.flow.validation.check.FlowIdentityCheck; import com.mastercard.test.flow.validation.check.InteractionIdentityCheck; @@ -38,6 +39,7 @@ public static final Validation[] defaultChecks() { new ChainOverlapCheck(), new DependencyChronologyCheck(), new DependencyLoopCheck(), + new DependencyInclusionCheck(), new FlowIdentityCheck(), new InteractionIdentityCheck(), new MessageSharingCheck(), diff --git a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/Violation.java b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/Violation.java index 5609e670b5..8acf223c65 100644 --- a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/Violation.java +++ b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/Violation.java @@ -26,12 +26,22 @@ public class Violation { private final String actual; /** + * Constructs a new {@link Violation} that signals a general failure + * + * @param validation The {@link Validation} that has been violated + * @param details A human-readable description of the violation + */ + public Violation( Validation validation, String details ) { + this( validation, details, null, null ); + } + + /** + * Constructs a new {@link Violation} that signals an expected-equality mismatch + * * @param validation The {@link Validation} that has been violated * @param details A human-readable description of the violation - * @param expected Expected half of a comparison test, or null to - * just signal a general test failure - * @param actual Actual half of a comparison test, or nullto - * just signal a general test failure + * @param expected Expected half of an equality test + * @param actual Actual half of an equality test */ public Violation( Validation validation, String details, String expected, String actual ) { this.validation = validation; diff --git a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/ChainOverlapCheck.java b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/ChainOverlapCheck.java index ad361381a4..0de145ed5c 100644 --- a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/ChainOverlapCheck.java +++ b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/ChainOverlapCheck.java @@ -40,7 +40,7 @@ public Stream checks( Model model ) { .collect( Collectors.toCollection( TreeSet::new ) ); if( chains.size() > 1 ) { - return new Violation( this, "Overlapping chains " + chains, null, null ) + return new Violation( this, "Overlapping chains " + chains ) .offender( flow ); } diff --git a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/DependencyChronologyCheck.java b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/DependencyChronologyCheck.java index 8ac6ea6670..b1e382ad97 100644 --- a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/DependencyChronologyCheck.java +++ b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/DependencyChronologyCheck.java @@ -48,11 +48,10 @@ public Stream checks( Model model ) { + "\nto\n" + "%s", delorean.source().getMessage().map( Message::assertable ).orElse( "???" ), - delorean.sink().getMessage().map( Message::assertable ).orElse( "???" ) ), - null, null ) - .offender( flow, - delorean.source().getInteraction().orElse( null ), - delorean.sink().getInteraction().orElse( null ) ); + delorean.sink().getMessage().map( Message::assertable ).orElse( "???" ) ) ) + .offender( flow, + delorean.source().getInteraction().orElse( null ), + delorean.sink().getInteraction().orElse( null ) ); } return null; } ) ); diff --git a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/DependencyInclusionCheck.java b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/DependencyInclusionCheck.java new file mode 100644 index 0000000000..7c6cdc7060 --- /dev/null +++ b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/DependencyInclusionCheck.java @@ -0,0 +1,51 @@ +package com.mastercard.test.flow.validation.check; + +import static java.util.stream.Collectors.toSet; + +import java.util.Set; +import java.util.stream.Stream; + +import com.mastercard.test.flow.Dependency; +import com.mastercard.test.flow.Flow; +import com.mastercard.test.flow.Model; +import com.mastercard.test.flow.validation.Check; +import com.mastercard.test.flow.validation.Validation; +import com.mastercard.test.flow.validation.Violation; + +/** + * Checks that the dependency flows are actually part of the system model + */ +public class DependencyInclusionCheck implements Validation { + + @Override + public String name() { + return "Dependency inclusion"; + } + + @Override + public String explanation() { + return "Dependency sources must be included in the system model"; + } + + @Override + public Stream checks( Model model ) { + Set allFlows = model.flows().collect( toSet() ); + return model.flows() + .flatMap( Flow::dependencies ) + .map( d -> new Check( this, + name( d ), + () -> d.source().getFlow() + .filter( src -> !allFlows.contains( src ) ) + .map( src -> new Violation( this, + String.format( "Dependency source '%s' not presented in system model", + src.meta().id() ) ) + .offender( d.sink().flow() ) ) + .orElse( null ) ) ); + } + + private static String name( Dependency d ) { + return String.format( "%s → %s", + d.source().getFlow().map( f -> f.meta().id() ).orElse( null ), + d.sink().getFlow().map( f -> f.meta().id() ).orElse( null ) ); + } +} diff --git a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/FlowPairCheck.java b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/FlowPairCheck.java index b51eeb0e12..6cd53662f4 100644 --- a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/FlowPairCheck.java +++ b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/FlowPairCheck.java @@ -51,7 +51,7 @@ public Stream checks( Model model ) { checks.add( new Check( this, left.meta().id() + " x " + right.meta().id(), () -> violation( left, right ) - .map( v -> new Violation( this, v, null, null ) + .map( v -> new Violation( this, v ) .offender( left ) .offender( right ) ) .orElse( null ) ) ); diff --git a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/InteractionIdentityCheck.java b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/InteractionIdentityCheck.java index 91d102c70c..0e43ffe711 100644 --- a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/InteractionIdentityCheck.java +++ b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/InteractionIdentityCheck.java @@ -38,7 +38,7 @@ public Stream checks( Model model ) { String id = String.format( "%s->%s %s", ntr.requester().name(), ntr.responder().name(), ntr.tags() ); if( ids.containsKey( id ) ) { - return new Violation( this, "Shared interaction ID", null, null ) + return new Violation( this, "Shared interaction ID" ) .offender( flow, ids.get( id ), ntr ); } ids.put( id, ntr ); diff --git a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/MessageSharingCheck.java b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/MessageSharingCheck.java index 1180633823..b1adf0580f 100644 --- a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/MessageSharingCheck.java +++ b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/MessageSharingCheck.java @@ -50,10 +50,9 @@ public Stream checks( Model model ) { MessageOwner current = new MessageOwner( flow, tx.source() ); MessageOwner previous = messageIdentities.get( objectId ); if( previous != null ) { - return new Violation( this, "Shared message:\n" + tx.message().assertable(), - null, null ) - .offender( previous.flow, previous.interaction ) - .offender( current.flow, current.interaction ); + return new Violation( this, "Shared message:\n" + tx.message().assertable() ) + .offender( previous.flow, previous.interaction ) + .offender( current.flow, current.interaction ); } messageIdentities.put( objectId, current ); } diff --git a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/ResultTagCheck.java b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/ResultTagCheck.java index 510eb6233b..88469aba3f 100644 --- a/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/ResultTagCheck.java +++ b/validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/ResultTagCheck.java @@ -54,7 +54,7 @@ public Stream checks( Model model ) { .collect( Collectors.joining( ", " ) ); if( !misused.isEmpty() ) { - return new Violation( this, "Use of reserved tags: " + misused, null, null ) + return new Violation( this, "Use of reserved tags: " + misused ) .offender( flow ); } diff --git a/validation/validation-core/src/test/java/com/mastercard/test/flow/validation/AbstractValidatorTest.java b/validation/validation-core/src/test/java/com/mastercard/test/flow/validation/AbstractValidatorTest.java index 3e59f67ce7..7c3621b428 100644 --- a/validation/validation-core/src/test/java/com/mastercard/test/flow/validation/AbstractValidatorTest.java +++ b/validation/validation-core/src/test/java/com/mastercard/test/flow/validation/AbstractValidatorTest.java @@ -31,6 +31,7 @@ void with() { Assertions.assertEquals( "" + "Chain overlap\n" + "Dependency chronology\n" + + "Dependency inclusion\n" + "Dependency loop\n" + "Flow Identity\n" + "Interaction Identity\n" @@ -62,9 +63,9 @@ void acceptance() { .accepting( v -> v.details().contains( "minor" ) ); Assertions.assertFalse( tv.accepted( - new Violation( null, "gadzooks! this is a major problem!", null, null ) ) ); + new Violation( null, "gadzooks! this is a major problem!" ) ) ); Assertions.assertTrue( tv.accepted( - new Violation( null, "meh, this is a minor problem", null, null ) ) ); + new Violation( null, "meh, this is a minor problem" ) ) ); } } diff --git a/validation/validation-core/src/test/java/com/mastercard/test/flow/validation/check/DependencyInclusionCheckTest.java b/validation/validation-core/src/test/java/com/mastercard/test/flow/validation/check/DependencyInclusionCheckTest.java new file mode 100644 index 0000000000..df432200e0 --- /dev/null +++ b/validation/validation-core/src/test/java/com/mastercard/test/flow/validation/check/DependencyInclusionCheckTest.java @@ -0,0 +1,193 @@ +package com.mastercard.test.flow.validation.check; + +import static java.util.Collections.emptySet; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import com.mastercard.test.flow.Dependency; +import com.mastercard.test.flow.FieldAddress; +import com.mastercard.test.flow.Flow; +import com.mastercard.test.flow.Metadata; +import com.mastercard.test.flow.Model; + +/** + * Exercises {@link DependencyInclusionCheck} + */ +class DependencyInclusionCheckTest extends AbstractValidationTest { + + /***/ + DependencyInclusionCheckTest() { + super( new DependencyInclusionCheck(), + "Dependency inclusion", + "Dependency sources must be included in the system model" ); + } + + /** + * No flows + */ + @Test + void empty() { + test( mdl( flws().values() ) ); + } + + /** + * A single flow + */ + @Test + void lonesome() { + test( mdl( flws( "lonely" ).values() ) ); + } + + /** + * Two flows, but no dependencies + */ + @Test + void noDependencies() { + test( mdl( flws( "abc", "def" ).values() ) ); + } + + /** + * Two flows, a dependency, the sink is included in the model + */ + @Test + void included() { + test( mdl( flws( "abc", "def abc" ).values() ), + "abc [] → def [] : pass" ); + } + + /** + * Two flows, a dependency, the sink is not included in the model + */ + @Test + void excluded() { + Map flows = flws( "abc", "def abc" ); + flows.remove( "abc" ); + test( mdl( flows.values() ), + " details: Dependency source 'abc []' not presented in system model\n" + + " expected: null\n" + + " actual: null\n" + + "offenders: def []\n" + + "null" ); + } + + /** + * One valid dependency, one violation + */ + @Test + void mixed() { + Map flows = flws( + "abc", "def abc", + "ghi", "jkl ghi" ); + flows.remove( "ghi" ); + test( mdl( flows.values() ), + "abc [] → def [] : pass", + " details: Dependency source 'ghi []' not presented in system model\n" + + " expected: null\n" + + " actual: null\n" + + "offenders: jkl []\n" + + "null" ); + } + + /** + * Multiple valid dependencies + */ + @Test + void multiIncluded() { + test( mdl( flws( "abc", "def abc", "ghi abc def", "jkl abc def ghi" ).values() ), + "abc [] → def [] : pass", + "abc [] → ghi [] : pass", + "def [] → ghi [] : pass", + "abc [] → jkl [] : pass", + "def [] → jkl [] : pass", + "ghi [] → jkl [] : pass" ); + } + + /** + * Multiple invalid dependencies + */ + @Test + void multiExcluded() { + Map flows = flws( "abc", "def abc", "ghi abc def", "jkl abc def ghi" ); + flows.remove( "def" ); + test( mdl( flows.values() ), + "abc [] → ghi [] : pass", + " details: Dependency source 'def []' not presented in system model\n" + + " expected: null\n" + + " actual: null\n" + + "offenders: ghi []\n" + + "null", + "abc [] → jkl [] : pass", + " details: Dependency source 'def []' not presented in system model\n" + + " expected: null\n" + + " actual: null\n" + + "offenders: jkl []\n" + + "null", + "ghi [] → jkl [] : pass" ); + } + + /** + * @param flows A set of strings, each specifying one flow. The strings are + * space-separated lists. The first element is the flow name, the + * following elements are the names of dependency flows + * @return A model, with the specified flows and dependency structure + */ + private static Map flws( String... flows ) { + Map names = new HashMap<>(); + + for( String flow : flows ) { + String[] tkns = flow.split( " " ); + + Flow flw = Mockito.mock( Flow.class ); + Metadata mtdt = Mockito.mock( Metadata.class ); + + when( mtdt.description() ).thenReturn( tkns[0] ); + when( mtdt.tags() ).thenReturn( emptySet() ); + when( mtdt.id() ).thenCallRealMethod(); + when( flw.meta() ).thenReturn( mtdt ); + + List deps = new ArrayList<>(); + for( int i = 1; i < tkns.length; i++ ) { + String depName = tkns[i]; + + FieldAddress src = mock( FieldAddress.class ); + when( src.getFlow() ).thenReturn( Optional.ofNullable( names.get( depName ) ) ); + when( src.flow() ).thenAnswer( a -> names.get( depName ) ); + + FieldAddress snk = mock( FieldAddress.class ); + when( snk.getFlow() ).thenReturn( Optional.of( flw ) ); + when( snk.flow() ).thenReturn( flw ); + + Dependency dep = mock( Dependency.class ); + when( dep.source() ).thenReturn( src ); + when( dep.sink() ).thenReturn( snk ); + + deps.add( dep ); + } + when( flw.dependencies() ).thenAnswer( a -> deps.stream() ); + + names.put( tkns[0], flw ); + } + + return names; + } + + /** + * @param flows Some {@link Flow}s + * @return a {@link Model} that produces the supplied {@link Flow}s + */ + private static Model mdl( Collection flows ) { + Model mdl = Mockito.mock( Model.class ); + when( mdl.flows() ).thenAnswer( a -> flows.stream() ); + return mdl; + } +} diff --git a/validation/validation-core/src/test/java/com/mastercard/test/flow/validation/check/DependencyLoopCheckTest.java b/validation/validation-core/src/test/java/com/mastercard/test/flow/validation/check/DependencyLoopCheckTest.java index 4bb694ff8d..6e9422c3da 100644 --- a/validation/validation-core/src/test/java/com/mastercard/test/flow/validation/check/DependencyLoopCheckTest.java +++ b/validation/validation-core/src/test/java/com/mastercard/test/flow/validation/check/DependencyLoopCheckTest.java @@ -175,7 +175,7 @@ void lollipop() { } /** - * @param flows A set of string, each specifying one flow. The strings are + * @param flows A set of strings, each specifying one flow. The strings are * space-separated lists. The first element is the flow name, the * following elements are the names of dependency flows * @return A model, with the specified flows and dependency structure diff --git a/validation/validation-junit4/src/test/java/com/mastercard/test/flow/validation/junit4/ExampleTest.java b/validation/validation-junit4/src/test/java/com/mastercard/test/flow/validation/junit4/ExampleTest.java index b455901f5b..db6a7c89f9 100644 --- a/validation/validation-junit4/src/test/java/com/mastercard/test/flow/validation/junit4/ExampleTest.java +++ b/validation/validation-junit4/src/test/java/com/mastercard/test/flow/validation/junit4/ExampleTest.java @@ -139,7 +139,7 @@ public String explanation() { public Stream checks( Model model ) { return Stream.of( new Check( this, "fail", - () -> new Violation( this, "fail", null, null ) ), + () -> new Violation( this, "fail" ) ), new Check( this, "cmp fail", () -> new Violation( this, "fail", "expect", "actual" ) ) ); } @@ -163,7 +163,7 @@ public String explanation() { public Stream checks( Model model ) { return Stream.of( new Check( this, "accept", - () -> new Violation( this, "accept", null, null ) ) ); + () -> new Violation( this, "accept" ) ) ); } }; } diff --git a/validation/validation-junit5/src/test/java/com/mastercard/test/flow/validation/junit5/ExampleTest.java b/validation/validation-junit5/src/test/java/com/mastercard/test/flow/validation/junit5/ExampleTest.java index 937ab24a2b..aa37dc9bd3 100644 --- a/validation/validation-junit5/src/test/java/com/mastercard/test/flow/validation/junit5/ExampleTest.java +++ b/validation/validation-junit5/src/test/java/com/mastercard/test/flow/validation/junit5/ExampleTest.java @@ -116,7 +116,7 @@ public String explanation() { public Stream checks( Model model ) { return Stream.of( new Check( this, "fail", - () -> new Violation( this, "fail", null, null ) ), + () -> new Violation( this, "fail" ) ), new Check( this, "cmp fail", () -> new Violation( this, "fail", "expect", "actual" ) ) ); } @@ -140,7 +140,7 @@ public String explanation() { public Stream checks( Model model ) { return Stream.of( new Check( this, "accept", - () -> new Violation( this, "accept", null, null ) ) ); + () -> new Violation( this, "accept" ) ) ); } }; }