diff --git a/doc/src/main/markdown/further.md b/doc/src/main/markdown/further.md index 8af9c770a6..f7aa13ba69 100644 --- a/doc/src/main/markdown/further.md +++ b/doc/src/main/markdown/further.md @@ -245,12 +245,12 @@ Consider the following worked example: [flow.Unpredictable]: ../../../../api/src/main/java/com/mastercard/test/flow/Unpredictable.java -[AbstractMessage.masking(Unpredictable,UnaryOperator)]: ../../../../message/message-core/src/main/java/com/mastercard/test/flow/msg/AbstractMessage.java#L46-L53,46-53 +[AbstractMessage.masking(Unpredictable,UnaryOperator)]: ../../../../message/message-core/src/main/java/com/mastercard/test/flow/msg/AbstractMessage.java#L48-L55,48-55 [AbstractFlocessor.masking(Unpredictable...)]: ../../../../assert/assert-core/src/main/java/com/mastercard/test/flow/assrt/AbstractFlocessor.java#L202-L209,202-209 [mask.BenSys]: ../../test/java/com/mastercard/test/flow/doc/mask/BenSys.java [mask.DieSys]: ../../test/java/com/mastercard/test/flow/doc/mask/DieSys.java [mask.Unpredictables]: ../../test/java/com/mastercard/test/flow/doc/mask/Unpredictables.java -[AbstractMessage.masking(Unpredictable,UnaryOperator)]: ../../../../message/message-core/src/main/java/com/mastercard/test/flow/msg/AbstractMessage.java#L46-L53,46-53 +[AbstractMessage.masking(Unpredictable,UnaryOperator)]: ../../../../message/message-core/src/main/java/com/mastercard/test/flow/msg/AbstractMessage.java#L48-L55,48-55 [Rolling?d\+]: ../../test/java/com/mastercard/test/flow/doc/mask/Rolling.java#L30,30 [msg.Mask]: ../../../../message/message-core/src/main/java/com/mastercard/test/flow/msg/Mask.java [msg.Mask.andThen(Consumer)]: ../../../../message/message-core/src/main/java/com/mastercard/test/flow/msg/Mask.java#L290-L292,290-292 diff --git a/message/message-core/src/main/java/com/mastercard/test/flow/msg/AbstractMessage.java b/message/message-core/src/main/java/com/mastercard/test/flow/msg/AbstractMessage.java index afcd5693c2..f227f9f237 100644 --- a/message/message-core/src/main/java/com/mastercard/test/flow/msg/AbstractMessage.java +++ b/message/message-core/src/main/java/com/mastercard/test/flow/msg/AbstractMessage.java @@ -5,6 +5,7 @@ import java.lang.reflect.Array; import java.math.BigDecimal; import java.math.BigInteger; +import java.sql.Timestamp; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -13,6 +14,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.UUID; import java.util.function.UnaryOperator; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -111,12 +113,33 @@ public T set( String field, Object value ) { return self(); } + @Override + public final Object get( String field ) { + Object value = access( field ); + if( value == null ) { + return null; + } + return defensiveCopiers.getOrDefault( + value.getClass(), + o -> o ) + .apply( value ); + } + + /** + * Field accessor + * + * @param field The field address + * @return The field value + */ + protected abstract Object access( String field ); + private static final Set> immutableTypes = Stream.of( Boolean.class, Byte.class, Short.class, Character.class, Integer.class, Float.class, Long.class, Double.class, String.class, - BigDecimal.class, BigInteger.class ) + BigDecimal.class, BigInteger.class, + UUID.class ) .collect( toCollection( HashSet::new ) ); /** @@ -141,6 +164,15 @@ protected Object validateValueType( String field, Object value ) { } return copy; } + if( defensiveCopiers.containsKey( value.getClass() ) ) { + Object copy = defensiveCopiers.get( value.getClass() ).apply( value ); + if( copy == value ) { + throw new IllegalStateException( String.format( + "Ineffective defensive copy for field '%s', type %s", + field, value.getClass() ) ); + } + return copy; + } if( value != DELETE && !value.getClass().isPrimitive() && !value.getClass().isEnum() @@ -151,6 +183,9 @@ protected Object validateValueType( String field, Object value ) { + " AbstractMessage.registerImmutable( " + value.getClass().getSimpleName() + ".class )\n" + "to suppress this error.\n" + + "Alternatively you can use\n" + + " AbstractMessage.registerDefensiveCopier( type, function )\n" + + "to make defensive copies of you mutable types.\n" + "You can also override\n" + " AbstractMessage.validateValueType( field, value )\n" + "in your subclass implementation to do validation more suitable\n" @@ -173,6 +208,22 @@ public static void registerImmutable( Class type ) { immutableTypes.add( type ); } + private static final Map, UnaryOperator> defensiveCopiers = new HashMap<>(); + + /** + * @param value type + * @param type value type + * @param copier function to make a defensive copy of a value + */ + @SuppressWarnings("unchecked") + public static void registerDefensiveCopier( Class type, UnaryOperator copier ) { + defensiveCopiers.put( type, (UnaryOperator) copier ); + } + + static { + registerDefensiveCopier( Timestamp.class, t -> new Timestamp( t.getTime() ) ); + } + /** * @return this */ diff --git a/message/message-core/src/test/java/com/mastercard/test/flow/msg/AbstractMessageTest.java b/message/message-core/src/test/java/com/mastercard/test/flow/msg/AbstractMessageTest.java index a9e91ee6c4..d6e187fffd 100644 --- a/message/message-core/src/test/java/com/mastercard/test/flow/msg/AbstractMessageTest.java +++ b/message/message-core/src/test/java/com/mastercard/test/flow/msg/AbstractMessageTest.java @@ -6,9 +6,15 @@ import java.lang.reflect.Array; import java.lang.reflect.Field; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.sql.Timestamp; import java.util.ArrayList; import java.util.Collections; +import java.util.Map; import java.util.Set; +import java.util.UUID; +import java.util.function.UnaryOperator; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -79,8 +85,12 @@ public Set fields() { } @Override - public Object get( String field ) { - throw new UnsupportedOperationException(); + protected Object access( String field ) { + return updates.stream() + .filter( u -> u.field().equals( field ) ) + .findAny() + .map( Update::value ) + .orElse( null ); } @Override @@ -162,7 +172,7 @@ void copyMasksTo() { void mutableValues() throws Exception { // pitest runs the test multiple times in the same classloader, so we have to - // clear the static immuatble type set back to default + // clear the static immutable type set back to default Field f = AbstractMessage.class.getDeclaredField( "immutableTypes" ); f.setAccessible( true ); ((Set>) f.get( null )).remove( ArrayList.class ); @@ -177,6 +187,9 @@ void mutableValues() throws Exception { + "If you're sure that this type is immutable, then you can call\n" + " AbstractMessage.registerImmutable( EmptyList.class )\n" + "to suppress this error.\n" + + "Alternatively you can use\n" + + " AbstractMessage.registerDefensiveCopier( type, function )\n" + + "to make defensive copies of you mutable types.\n" + "You can also override\n" + " AbstractMessage.validateValueType( field, value )\n" + "in your subclass implementation to do validation more suitable\n" @@ -191,6 +204,109 @@ void mutableValues() throws Exception { msg.set( "field", new ArrayList<>() ); } + /** + * Runs through the default allowed types and shows they pass the mutability + * filter + */ + @Test + void validTypes() { + ConcreteMessage msg = new ConcreteMessage( "msg" ); + msg.set( "boolean", true ); + msg.set( "byte", (byte) 1 ); + msg.set( "short", (short) 2 ); + msg.set( "character", 'a' ); + msg.set( "integer", 4 ); + msg.set( "float", 4.0f ); + msg.set( "long", 8L ); + msg.set( "double", 8.0 ); + msg.set( "string", "bcd" ); + msg.set( "bigdecimal", new BigDecimal( "1.234" ) ); + msg.set( "biginteger", new BigInteger( "5678" ) ); + msg.set( "uuid", new UUID( 12345L, 67890L ) ); + msg.set( "timestamp", new Timestamp( 1234567890L ) ); + msg.set( "null", null ); + + assertEquals( true, msg.get( "boolean" ) ); + assertEquals( (byte) 1, msg.get( "byte" ) ); + assertEquals( (short) 2, msg.get( "short" ) ); + assertEquals( 'a', msg.get( "character" ) ); + assertEquals( 4, msg.get( "integer" ) ); + assertEquals( 4.0f, msg.get( "float" ) ); + assertEquals( 8L, msg.get( "long" ) ); + assertEquals( 8.0, msg.get( "double" ) ); + assertEquals( "bcd", msg.get( "string" ) ); + assertEquals( new BigDecimal( "1.234" ), msg.get( "bigdecimal" ) ); + assertEquals( new BigInteger( "5678" ), msg.get( "biginteger" ) ); + assertEquals( new UUID( 12345L, 67890L ), msg.get( "uuid" ) ); + assertEquals( new Timestamp( 1234567890L ), msg.get( "timestamp" ) ); + assertEquals( null, msg.get( "null" ) ); + assertEquals( null, msg.get( "no such field" ) ); + } + + /** + * Demonstrates the mechanism that allows mutable field types to be used via + * defensive copies + * + * @throws Exception if something goes wrong with our reflection + */ + @SuppressWarnings("unchecked") + @Test + void defensiveCopy() throws Exception { + + // pitest runs the test multiple times in the same classloader, so we have to + // clear the static copier map set back to default + Field f = AbstractMessage.class.getDeclaredField( "defensiveCopiers" ); + f.setAccessible( true ); + ((Map, UnaryOperator>) f.get( null )).remove( StringBuilder.class ); + + ConcreteMessage msg = new ConcreteMessage( "msg" ); + + assertThrows( IllegalArgumentException.class, + () -> msg.set( "field", new StringBuilder( "buff" ) ) ); + + AbstractMessage.registerDefensiveCopier( StringBuilder.class, + b -> new StringBuilder( "copy of [" ).append( b.toString() ).append( "]" ) ); + + msg.set( "field", new StringBuilder( "buff" ) ); + + assertEquals( "" + + "msg\n" + + " field=copy of [buff]", + msg.asHuman(), + "The message holds a defensive copy of the supplied value" ); + + assertEquals( "copy of [copy of [buff]]", msg.get( "field" ).toString(), + "When queried, the message returns a defensive copy of the held value" ); + } + + /** + * Demonstrates that we reject ineffective defensive copiers + * + * @throws Exception if something goes wrong with our reflection + */ + @SuppressWarnings("unchecked") + @Test + void badDefensiveCopy() throws Exception { + + // pitest runs the test multiple times in the same classloader, so we have to + // clear the static copier map set back to default + Field f = AbstractMessage.class.getDeclaredField( "defensiveCopiers" ); + f.setAccessible( true ); + ((Map, UnaryOperator>) f.get( null )).remove( StringBuilder.class ); + + ConcreteMessage msg = new ConcreteMessage( "msg" ); + + AbstractMessage.registerDefensiveCopier( StringBuilder.class, + b -> b ); + + IllegalStateException ise = assertThrows( IllegalStateException.class, + () -> msg.set( "field", new StringBuilder( "buff" ) ) ); + + assertEquals( + "Ineffective defensive copy for field 'field', type class java.lang.StringBuilder", + ise.getMessage() ); + } + /** * Shows that setting arrays takes a defensive copy */ @@ -214,6 +330,9 @@ void arrayDefense() { + "If you're sure that this type is immutable, then you can call\n" + " AbstractMessage.registerImmutable( Object.class )\n" + "to suppress this error.\n" + + "Alternatively you can use\n" + + " AbstractMessage.registerDefensiveCopier( type, function )\n" + + "to make defensive copies of you mutable types.\n" + "You can also override\n" + " AbstractMessage.validateValueType( field, value )\n" + "in your subclass implementation to do validation more suitable\n" diff --git a/message/message-http/src/main/java/com/mastercard/test/flow/msg/http/HttpMsg.java b/message/message-http/src/main/java/com/mastercard/test/flow/msg/http/HttpMsg.java index 075dc4fe9f..84e025a16e 100644 --- a/message/message-http/src/main/java/com/mastercard/test/flow/msg/http/HttpMsg.java +++ b/message/message-http/src/main/java/com/mastercard/test/flow/msg/http/HttpMsg.java @@ -124,7 +124,7 @@ else if( isHttpField( field ) ) { } @Override - public Object get( String field ) { + protected Object access( String field ) { if( BODY.equals( field ) ) { return body.orElse( null ); } diff --git a/message/message-json/src/main/java/com/mastercard/test/flow/msg/json/Json.java b/message/message-json/src/main/java/com/mastercard/test/flow/msg/json/Json.java index 9537d901de..6c7d563d7b 100644 --- a/message/message-json/src/main/java/com/mastercard/test/flow/msg/json/Json.java +++ b/message/message-json/src/main/java/com/mastercard/test/flow/msg/json/Json.java @@ -151,7 +151,7 @@ else if( value == EMPTY_LIST ) { } @Override - public Object get( String field ) { + protected Object access( String field ) { AtomicReference result = new AtomicReference<>(); traverse( data(), field, false, ( map, key ) -> result.set( map.get( key ) ), diff --git a/message/message-sql/src/main/java/com/mastercard/test/flow/msg/sql/Query.java b/message/message-sql/src/main/java/com/mastercard/test/flow/msg/sql/Query.java index bda0217e56..4bac3256d2 100644 --- a/message/message-sql/src/main/java/com/mastercard/test/flow/msg/sql/Query.java +++ b/message/message-sql/src/main/java/com/mastercard/test/flow/msg/sql/Query.java @@ -98,7 +98,7 @@ public Set fields() { } @Override - public Object get( String field ) { + protected Object access( String field ) { return data().get( field ); } diff --git a/message/message-sql/src/main/java/com/mastercard/test/flow/msg/sql/Result.java b/message/message-sql/src/main/java/com/mastercard/test/flow/msg/sql/Result.java index bc0c693d8a..eac120cd0e 100644 --- a/message/message-sql/src/main/java/com/mastercard/test/flow/msg/sql/Result.java +++ b/message/message-sql/src/main/java/com/mastercard/test/flow/msg/sql/Result.java @@ -173,7 +173,7 @@ public Set fields() { } @Override - public Object get( String field ) { + protected Object access( String field ) { ResultSetStructure data = data(); if( COLUMNS.equals( field ) ) { return new ArrayList<>( data.columns ); diff --git a/message/message-text/src/main/java/com/mastercard/test/flow/msg/txt/Text.java b/message/message-text/src/main/java/com/mastercard/test/flow/msg/txt/Text.java index 477615a842..30c86f8c3c 100644 --- a/message/message-text/src/main/java/com/mastercard/test/flow/msg/txt/Text.java +++ b/message/message-text/src/main/java/com/mastercard/test/flow/msg/txt/Text.java @@ -82,7 +82,7 @@ protected String asHuman() { } @Override - public Object get( String field ) { + protected Object access( String field ) { Matcher m = Pattern.compile( field ).matcher( build() ); if( m.find() ) { return m.groupCount() == 0 ? m.group( 0 ) : m.group( 1 ); diff --git a/message/message-web/src/main/java/com/mastercard/test/flow/msg/web/WebSequence.java b/message/message-web/src/main/java/com/mastercard/test/flow/msg/web/WebSequence.java index 466eaeb608..7950e86d88 100644 --- a/message/message-web/src/main/java/com/mastercard/test/flow/msg/web/WebSequence.java +++ b/message/message-web/src/main/java/com/mastercard/test/flow/msg/web/WebSequence.java @@ -139,7 +139,7 @@ public WebSequence operation( String name, } @Override - public Object get( String field ) { + protected Object access( String field ) { return parameters().get( field ); } diff --git a/message/message-xml/src/main/java/com/mastercard/test/flow/msg/xml/XML.java b/message/message-xml/src/main/java/com/mastercard/test/flow/msg/xml/XML.java index b8f6b977c9..2b47a40e1b 100644 --- a/message/message-xml/src/main/java/com/mastercard/test/flow/msg/xml/XML.java +++ b/message/message-xml/src/main/java/com/mastercard/test/flow/msg/xml/XML.java @@ -265,7 +265,7 @@ else if( value instanceof List ) { } @Override - public Object get( String field ) { + protected Object access( String field ) { AtomicReference result = new AtomicReference<>(); traverse( data(), field, false, false, ( map, key ) -> result.set( map.get( key ) ),