Validation check for lazy model tagging (#546)

* Validation check for lazy model tagging

* test coverage fix

* name clarity, sonar fixen

aggregator/src/test/java/com/mastercard/test/flow/aggregator/SharedConstantTest.java

@@ -5,8 +5,10 @@
 
 import com.mastercard.test.flow.assrt.Order;
 import com.mastercard.test.flow.builder.Chain;
+import com.mastercard.test.flow.model.LazyModel;
 import com.mastercard.test.flow.report.Writer;
 import com.mastercard.test.flow.validation.check.ChainOverlapCheck;
+import com.mastercard.test.flow.validation.check.ReflectiveModelTaggingCheck;
 import com.mastercard.test.flow.validation.check.ResultTagCheck;
 
 /**
@@ -42,4 +44,14 @@ void resultTags() {
 		Assertions.assertEquals( Writer.SKIP_TAG, ResultTagCheck.SKIP_TAG );
 		Assertions.assertEquals( Writer.ERROR_TAG, ResultTagCheck.ERROR_TAG );
 	}
+
+	/**
+	 * We've got a validation check that exercises aspects of a particular model
+	 * implementation.
+	 */
+	@Test
+	void lazyModelStrings() {
+		Assertions.assertEquals( LazyModel.MODEL_TAGS_FIELD_NAME,
+				ReflectiveModelTaggingCheck.MODEL_TAGS_FIELD_NAME );
+	}
 }

doc/src/main/markdown/further.md

@@ -89,7 +89,7 @@ One of the benefits of selective execution (not having to build every flow in or
 
 The two main `Model` implementations provided in this framework are:
  * [`EagerModel`][EagerModel]: Builds flows instances in the constructor.
- * [`LazyModel`][LazyModel]: A collection of `EagerModel` instances, which are constructed as flows are requested.
+ * [`LazyModel`][LazyModel!]: A collection of `EagerModel` instances, which are constructed as flows are requested.
 
 If a flow in one `EagerModel` type is derived from a flow in another `EagerModel`, then you can simply declare the dependency model as a constructor parameter.
 The `LazyModel` that both models are registered to will handle satisfying the dependency.
@@ -102,7 +102,7 @@ You can see this behaviour in action in the example system model:
 <!-- code_link_start -->
 
 [EagerModel]: ../../../../model/src/main/java/com/mastercard/test/flow/model/EagerModel.java
-[LazyModel]: ../../../../model/src/main/java/com/mastercard/test/flow/model/LazyModel.java
+[LazyModel!]: ../../../../model/src/main/java/com/mastercard/test/flow/model/LazyModel.java
 [Implicit]: ../../../../example/app-model/src/main/java/com/mastercard/test/flow/example/app/model/Implicit.java
 [Deferred]: ../../../../example/app-model/src/main/java/com/mastercard/test/flow/example/app/model/Deferred.java
 [ExampleSystem]: ../../../../example/app-model/src/main/java/com/mastercard/test/flow/example/app/model/ExampleSystem.java

model/README.md

@@ -17,20 +17,20 @@ This module provides an API by which Flows can be grouped and collected into a c
 
 The three concrete classes are:
  * [`EagerModel`][EagerModel] - this will build all of its constituent flows immediately upon model construction
- * [`LazyModel`][LazyModel] - this combines multiple `EagerModel` implementations, and constructs them as required as Flows are requested.
+ * [`LazyModel`][LazyModel!] - this combines multiple `EagerModel` implementations, and constructs them as required as Flows are requested.
  * [`CombineModel`][CombineModel] - combines multiple child `Model` instances.
 
 The point of structuring Flow construction into separate models is to improve testing performance and iteration time.
 
 For example, let's assume that the system under test has two separate functions, called `foo` and `bar`.
 Our system model will thus contains a bunch Flow instances, some tagged with `foo`, and some tagged with `bar`.
-Let's also assume that those flow instances are defined in two separate EagerModels, one for `foo` flows and one for `bar` flows, and those two models are combined in a LazyModel. 
+Let's also assume that those flow instances are defined in two separate `EagerModel`s, one for `foo` flows and one for `bar` flows, and those two models are combined in a `LazyModel`.
 Thus when we're iterating on changing the `foo` behaviour we can supply the `foo` tag to the assert component and avoid building all the `bar` flows that we don't even want to run.
 
 <!-- code_link_start -->
 
 [EagerModel]: src/main/java/com/mastercard/test/flow/model/EagerModel.java
-[LazyModel]: src/main/java/com/mastercard/test/flow/model/LazyModel.java
+[LazyModel!]: src/main/java/com/mastercard/test/flow/model/LazyModel.java
 [CombineModel]: src/main/java/com/mastercard/test/flow/model/CombineModel.java
 
 <!-- code_link_end -->

model/src/main/java/com/mastercard/test/flow/model/EagerModel.java

@@ -2,6 +2,7 @@
 package com.mastercard.test.flow.model;
 
 import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.LinkedHashSet;
@@ -58,12 +59,24 @@ public TaggedGroup tags() {
 	 */
 	public static TaggedGroup typeTags( Class<? extends EagerModel> type ) {
 		try {
-			Field f = type.getDeclaredField( "MODEL_TAGS" );
+			Field f = type.getDeclaredField( LazyModel.MODEL_TAGS_FIELD_NAME );
+
+			int mod = f.getModifiers();
+			if( !(Modifier.isPublic( mod )
+					&& Modifier.isStatic( mod )
+					&& Modifier.isFinal( mod )
+					&& TaggedGroup.class.isAssignableFrom( f.getType() )) ) {
+				throw new IllegalArgumentException( String.format(
+						"%s must have a `public static final %s %s` field",
+						type, TaggedGroup.class.getSimpleName(), LazyModel.MODEL_TAGS_FIELD_NAME ) );
+			}
+
 			return (TaggedGroup) f.get( null );
 		}
 		catch( NoSuchFieldException | IllegalAccessException e ) {
-			throw new IllegalArgumentException(
-					type + " must have a `public static TaggedGroup MODEL_TAGS` field", e );
+			throw new IllegalArgumentException( String.format(
+					"%s must have a `public static final %s %s` field",
+					type, TaggedGroup.class.getSimpleName(), LazyModel.MODEL_TAGS_FIELD_NAME ), e );
 		}
 	}
 

model/src/main/java/com/mastercard/test/flow/model/LazyModel.java

@@ -39,6 +39,12 @@
  */
 public class LazyModel extends TitledModel {
 
+	/**
+	 * The name of the <code>public static final TaggedGroup</code> field that
+	 * submodel classes are assumed to have
+	 */
+	public static final String MODEL_TAGS_FIELD_NAME = "MODEL_TAGS";
+
 	private static final Comparator<Class<?>> cc = Comparator.comparing( Class::getName );
 	private final Map<Class<? extends EagerModel>, TaggedGroup> types = new TreeMap<>( cc );
 	private final Map<Class<? extends EagerModel>, Model> instances = new TreeMap<>( cc );

model/src/test/java/com/mastercard/test/flow/model/EagerModelTest.java

@@ -210,8 +210,41 @@ void listener() {
 	void typeTags() {
 		assertEquals( "∩[c]⋃[a, b, c, d, e]",
 				EagerModel.typeTags( TaggedModel.class ).toString() );
+	}
 
-		assertThrows( IllegalArgumentException.class,
+	/**
+	 * Trying to extract the tags of a model that simply doesn't have the expected
+	 * <code>MODEL_TAGS</code> field
+	 */
+	@Test
+	void untagged() {
+		IllegalArgumentException e = assertThrows( IllegalArgumentException.class,
 				() -> EagerModel.typeTags( LumpyModel.class ) );
+		assertEquals( ""
+				+ "class com.mastercard.test.flow.model.EagerModelTest$LumpyModel "
+				+ "must have a `public static final TaggedGroup MODEL_TAGS` field",
+				e.getMessage() );
+	}
+
+	private static class BadlyTagged extends EagerModel {
+		public static TaggedGroup MODEL_TAGS = new TaggedGroup( "should be final" );
+
+		public BadlyTagged() {
+			super( MODEL_TAGS );
+		}
+	}
+
+	/**
+	 * Trying to extract the tags of a model where the <code>MODEL_TAGS</code> field
+	 * exists, but is not of the expected form
+	 */
+	@Test
+	void badlyTagged() {
+		IllegalArgumentException e = assertThrows( IllegalArgumentException.class,
+				() -> EagerModel.typeTags( BadlyTagged.class ) );
+		assertEquals( ""
+				+ "class com.mastercard.test.flow.model.EagerModelTest$BadlyTagged "
+				+ "must have a `public static final TaggedGroup MODEL_TAGS` field",
+				e.getMessage() );
 	}
 }

validation/validation-core/pom.xml

@@ -19,13 +19,13 @@
 		</dependency>
 
 		<dependency>
-			<!-- Context/residue serialisation -->
+			<!-- Context/residue serialisation for inheritance health metric -->
 			<groupId>com.fasterxml.jackson.core</groupId>
 			<artifactId>jackson-databind</artifactId>
 		</dependency>
 
 		<dependency>
-			<!-- text diffs -->
+			<!-- text diffs for inheritance health metric -->
 			<groupId>io.github.java-diff-utils</groupId>
 			<artifactId>java-diff-utils</artifactId>
 		</dependency>

validation/validation-core/src/main/java/com/mastercard/test/flow/validation/AbstractValidator.java

@@ -22,6 +22,7 @@
 import com.mastercard.test.flow.validation.check.InteractionIdentityCheck;
 import com.mastercard.test.flow.validation.check.MessageSharingCheck;
 import com.mastercard.test.flow.validation.check.ModelTaggingCheck;
+import com.mastercard.test.flow.validation.check.ReflectiveModelTaggingCheck;
 import com.mastercard.test.flow.validation.check.ResultTagCheck;
 import com.mastercard.test.flow.validation.check.TraceUniquenessCheck;
 
@@ -48,6 +49,7 @@ public static final Validation[] defaultChecks() {
 				new InteractionIdentityCheck(),
 				new MessageSharingCheck(),
 				new ModelTaggingCheck(),
+				new ReflectiveModelTaggingCheck(),
 				new ResultTagCheck(),
 				new TraceUniquenessCheck(),
 		};

validation/validation-core/src/main/java/com/mastercard/test/flow/validation/check/ReflectiveModelTaggingCheck.java

@@ -0,0 +1,73 @@
+package com.mastercard.test.flow.validation.check;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Stream;
+
+import com.mastercard.test.flow.Model;
+import com.mastercard.test.flow.util.TaggedGroup;
+import com.mastercard.test.flow.validation.Check;
+import com.mastercard.test.flow.validation.Validation;
+import com.mastercard.test.flow.validation.Violation;
+
+/**
+ * Checks that model implementations that offer reflective access to model
+ * tagging information do so in an accurate manner
+ */
+public class ReflectiveModelTaggingCheck implements Validation {
+
+	/**
+	 * The name of the <code>public static final TaggedGroup</code> field that
+	 * lazily-constructed model types are assumed to have
+	 */
+	public static final String MODEL_TAGS_FIELD_NAME = "MODEL_TAGS";
+
+	@Override
+	public String name() {
+		return "Reflective model tagging";
+	}
+
+	@Override
+	public String explanation() {
+		return "Models that offer reflective tagging information do so accurately";
+	}
+
+	@Override
+	public Stream<Check> checks( Model model ) {
+		return buildChecks( model, new ArrayList<>() ).stream();
+	}
+
+	private List<Check> buildChecks( Model model, List<Check> checks ) {
+		Optional<Field> mf = Stream.of( model.getClass().getDeclaredFields() )
+				.filter( f -> Modifier.isPublic( f.getModifiers() ) )
+				.filter( f -> Modifier.isStatic( f.getModifiers() ) )
+				.filter( f -> Modifier.isFinal( f.getModifiers() ) )
+				.filter( f -> TaggedGroup.class.isAssignableFrom( f.getType() ) )
+				.filter( f -> MODEL_TAGS_FIELD_NAME.equals( f.getName() ) )
+				.findFirst();
+
+		if( mf.isPresent() ) {
+			try {
+				TaggedGroup reflective = (TaggedGroup) mf.get().get( null );
+				TaggedGroup method = model.tags();
+
+				checks.add( new Check( this, model.title(), () -> reflective != method
+						? new Violation( this, String.format(
+								"%s.tags() should just return %s",
+								model.getClass().getName(), MODEL_TAGS_FIELD_NAME ) )
+						: null ) );
+			}
+			catch( Exception e ) {
+				throw new IllegalStateException( "Failed to access tags for " + model.getClass(), e );
+			}
+		}
+
+		model.subModels().forEach( child -> buildChecks( child, checks ) );
+
+		return checks;
+	}
+
+}

validation/validation-core/src/test/java/com/mastercard/test/flow/validation/AbstractValidatorTest.java

@@ -44,6 +44,7 @@ void with() {
 				+ "Interaction Identity\n"
 				+ "Message sharing\n"
 				+ "Model tagging\n"
+				+ "Reflective model tagging\n"
 				+ "Result tag misuse\n"
 				+ "Trace uniqueness",
 				tv.validations()