Issue a warning when final field is overwritten #414

src/main/java/net/openhft/chronicle/wire/WireMarshaller.java

@@ -450,6 +450,7 @@ abstract static class FieldAccess {
 
         Comment commentAnnotation;
         Boolean isLeaf;
+        boolean isFinalNoWarning;
 
         FieldAccess(@NotNull Field field) {
             this(field, null);
@@ -461,6 +462,10 @@ abstract static class FieldAccess {
             offset = unsafeObjectFieldOffset(field);
             key = field::getName;
             this.isLeaf = isLeaf;
+
+            if ((field.getModifiers() & Modifier.FINAL) != 0)
+                isFinalNoWarning = true;
+
             try {
                 commentAnnotation = field.getAnnotation(Comment.class);
             } catch (NullPointerException ignore) {
@@ -613,6 +618,8 @@ protected boolean sameValue(Object o, Object o2) throws IllegalAccessException {
         }
 
         protected void copy(Object from, Object to) throws IllegalAccessException {
+            triggerFinalWarning();
+
             ObjectUtils.requireNonNull(from);
             ObjectUtils.requireNonNull(to);
 
@@ -622,6 +629,8 @@ protected void copy(Object from, Object to) throws IllegalAccessException {
         protected abstract void getValue(Object o, ValueOut write, Object previous) throws IllegalAccessException;
 
         protected void readValue(Object o, Object defaults, ValueIn read, boolean overwrite) throws IllegalAccessException {
+            triggerFinalWarning();
+
             if (!read.isPresent()) {
                 if (overwrite && defaults != null)
                     copy(Objects.requireNonNull(defaults), o);
@@ -641,6 +650,14 @@ protected void readValue(Object o, Object defaults, ValueIn read, boolean overwr
             }
         }
 
+        protected void triggerFinalWarning() {
+            if (isFinalNoWarning) {
+                Jvm.warn().on(WireMarshaller.class, "Overwriting final field " + field.getName() + " in " +
+                        field.getDeclaringClass() + ", final fields cannot be updated safely and will be ignored in future releases");
+                isFinalNoWarning = false;
+            }
+        }
+
         protected abstract void setValue(Object o, ValueIn read, boolean overwrite) throws IllegalAccessException;
 
         public abstract void getAsBytes(Object o, Bytes<?> bytes) throws IllegalAccessException;
@@ -1094,6 +1111,8 @@ protected void getValue(final Object o, final ValueOut write, final Object previ
 
         @Override
         protected void readValue(Object o, Object defaults, ValueIn read, boolean overwrite) throws IllegalAccessException {
+            triggerFinalWarning();
+
             EnumSet coll = (EnumSet) field.get(o);
             if (coll == null) {
                 coll = enumSetSupplier.get();
@@ -1249,6 +1268,8 @@ protected void copy(Object from, Object to) throws IllegalAccessException {
 
         @Override
         protected void readValue(Object o, Object defaults, ValueIn read, boolean overwrite) throws IllegalAccessException {
+            triggerFinalWarning();
+
             Collection coll = (Collection) field.get(o);
             if (coll == null) {
                 coll = collectionSupplier.get();
@@ -1338,6 +1359,8 @@ protected void getValue(Object o, @NotNull ValueOut write, Object previous) thro
 
         @Override
         protected void readValue(Object o, Object defaults, ValueIn read, boolean overwrite) throws IllegalAccessException {
+            triggerFinalWarning();
+
             Collection coll = (Collection) field.get(o);
             if (coll == null) {
                 coll = collectionSupplier.get();
@@ -1418,6 +1441,8 @@ protected void copy(Object from, Object to) throws IllegalAccessException {
 
         @Override
         protected void readValue(Object o, Object defaults, ValueIn read, boolean overwrite) throws IllegalAccessException {
+            triggerFinalWarning();
+
             Map map = (Map) field.get(o);
             if (map == null) {
                 map = collectionSupplier.get();

src/main/java/net/openhft/chronicle/wire/Wires.java

@@ -380,6 +380,9 @@ public static void writeKey(@NotNull Object marshallable, Bytes<?> bytes) {
 
     @NotNull
     public static <T extends Marshallable> T deepCopy(@NotNull T marshallable) {
+        if (Enum.class.isAssignableFrom(marshallable.getClass()))
+            return marshallable;
+
         Wire wire = acquireBinaryWire();
         @NotNull T t = (T) ObjectUtils.newInstance(marshallable.getClass());
         boolean useSelfDescribing = t.usesSelfDescribingMessage() || !(t instanceof BytesMarshallable);

src/test/java/net/openhft/chronicle/wire/FinalFieldsTest.java

@@ -0,0 +1,86 @@
+/*
+ * Copyright 2016-2020 chronicle.software
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package net.openhft.chronicle.wire;
+
+import net.openhft.chronicle.bytes.Bytes;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+
+public class FinalFieldsTest extends WireTestCommon {
+    @SuppressWarnings("rawtypes")
+    @Test
+    public void testCopy() {
+        expectException("Overwriting final field map");
+        expectException("Overwriting final field array");
+        expectException("Overwriting final field intValue");
+        expectException("Overwriting final field value");
+
+        Bytes bytesFrom = Bytes.allocateElasticOnHeap(64);
+        Wire wireFrom = WireType.BINARY.apply(bytesFrom);
+        Bytes bytesTo = Bytes.allocateElasticOnHeap(64);
+        Wire wireTo = WireType.JSON.apply(bytesTo);
+
+        FinalFieldsClass a = create();
+
+        wireFrom.getValueOut().marshallable(a);
+
+        wireFrom.copyTo(wireTo);
+        FinalFieldsClass b = wireTo.getValueIn().object(FinalFieldsClass.class);
+
+        assertEquals(a, b);
+    }
+
+    @Test
+    public void testCopyEnum() {
+        //expectException("Overwriting final field name");
+
+        assertEquals(EnumValue.A, EnumValue.A.deepCopy());
+    }
+
+    private FinalFieldsClass create() {
+        Map<CcyPair, String> map = new HashMap<>();
+        map.put(CcyPair.EURUSD, "eurusd");
+        return new FinalFieldsClass(map, new String[]{"hello", "there"}, 11, 123.4, EnumValue.A);
+    }
+
+    @SuppressWarnings("unused")
+    private static class FinalFieldsClass extends SelfDescribingMarshallable {
+        final Map<CcyPair, String> map;
+        final String[] array;
+        final int intValue;
+        final double value;
+        EnumValue enumValue;
+
+        public FinalFieldsClass(Map<CcyPair, String> map, String[] array, int intValue, double value,
+                                EnumValue enumValue) {
+            this.map = map;
+            this.array = array;
+            this.intValue = intValue;
+            this.value = value;
+            this.enumValue = enumValue;
+        }
+    }
+
+    private enum EnumValue implements Marshallable {
+        A;
+    }
+}

src/test/java/net/openhft/chronicle/wire/FloatDtoTest.java

@@ -29,7 +29,7 @@ static class Key extends SelfDescribingMarshallable implements KeyedMarshallable
     static class Value extends Key implements Marshallable {
 
         @SuppressWarnings("unused")
-        final float myFloat;
+        float myFloat;
 
         Value(int uiid,
               float myFloat) {

src/test/java/net/openhft/chronicle/wire/ForwardAndBackwardCompatibilityMarshallableTest.java

@@ -158,7 +158,7 @@ public MDTO1 one(int one) {
 
     public static class MDTO2 extends SelfDescribingMarshallable implements Demarshallable {
 
-        final StringBuilder three = new StringBuilder();
+        StringBuilder three = new StringBuilder();
         int one;
         int two;
 

src/test/java/net/openhft/chronicle/wire/MyTypes.java

@@ -20,7 +20,7 @@
 import org.jetbrains.annotations.NotNull;
 
 class MyTypes extends SelfDescribingMarshallable {
-    final StringBuilder text = new StringBuilder();
+    StringBuilder text = new StringBuilder();
     boolean flag;
     byte b;
     short s;

src/test/java/net/openhft/chronicle/wire/NestedMapsTest.java

@@ -255,9 +255,9 @@ public void testMapReadAndWrite() {
     }
 
     static class Mapped extends SelfDescribingMarshallable {
-        final Set<String> words = new LinkedHashSet<>();
-        final List<Integer> numbers = new ArrayList<>();
-        final Map<String, String> map1 = new LinkedHashMap<>();
-        final Map<String, Double> map2 = new LinkedHashMap<>();
+        Set<String> words = new LinkedHashSet<>();
+        List<Integer> numbers = new ArrayList<>();
+        Map<String, String> map1 = new LinkedHashMap<>();
+        Map<String, Double> map2 = new LinkedHashMap<>();
     }
 }

src/test/java/net/openhft/chronicle/wire/ObjectWithTreeMap.java

@@ -3,5 +3,5 @@
 import java.util.TreeMap;
 
 class ObjectWithTreeMap extends SelfDescribingMarshallable {
-    final TreeMap<String, String> map = new TreeMap<>();
+    TreeMap<String, String> map = new TreeMap<>();
 }

src/test/java/net/openhft/chronicle/wire/OverrideAValueTest.java

@@ -76,6 +76,6 @@ static class SubClass extends ParentClass {
     }
 
     static class ParentHolder extends SelfDescribingMarshallable {
-        final ParentClass object = new ParentClass();
+        ParentClass object = new ParentClass();
     }
 }

src/test/java/net/openhft/chronicle/wire/bytesmarshallable/NestedGenericTest.java

@@ -39,7 +39,7 @@ public void testDefined() {
     }
 
     private static class ValueHolder<V> {
-        private final V defaultValue;
+        private V defaultValue;
 
         public ValueHolder(V defaultValue) {
             this.defaultValue = defaultValue;
@@ -55,7 +55,7 @@ public boolean equals(Object o) {
     }
 
     private static class ValueHolderDef {
-        private final A defaultValue;
+        private A defaultValue;
 
         public ValueHolderDef(A defaultValue) {
             this.defaultValue = defaultValue;

src/test/java/net/openhft/chronicle/wire/dynenum/WireDynamicEnumTest.java

@@ -144,6 +144,8 @@ public void deserialize() {
 
     @Test
     public void deserialize2() {
+        expectException("Overwriting final field name in class java.lang.Enum");
+
         String text = "push: ONE\n" +
                 "...\n" +
                 "unwraps: {\n" +
@@ -234,8 +236,8 @@ enum WDENums implements WDEI, DynamicEnum {
         ONE("One", 1),
         TWO("Two", 2);
 
-        private final String nice;
-        private final int value;
+        private String nice;
+        private int value;
 
         WDENums(String nice, int value) {
             this.nice = nice;
@@ -278,9 +280,9 @@ static class WDENum2 extends SelfDescribingMarshallable implements WDEI, Dynamic
         static final WDENum2 ONE = new WDENum2("One", 1);
         static final WDENum2 TWO = new WDENum2("Two", 2);
 
-        private final String name;
-        private final String nice;
-        private final int value;
+        private String name;
+        private String nice;
+        private int value;
 
         WDENum2(String nice, int value) {
             this.name = nice.toUpperCase();

src/test/java/net/openhft/chronicle/wire/marshallable/TwoArrays.java

@@ -8,8 +8,8 @@
 import net.openhft.chronicle.wire.SelfDescribingMarshallable;
 
 public class TwoArrays extends SelfDescribingMarshallable implements Closeable {
-    final IntArrayValues ia;
-    final LongArrayValues la;
+    IntArrayValues ia;
+    LongArrayValues la;
     transient boolean closed;
 
     public TwoArrays(int isize, long lsize) {

src/test/java/net/openhft/chronicle/wire/method/VanillaMethodReaderTest.java

@@ -415,7 +415,7 @@ static class NestedUnknown extends SelfDescribingMarshallable {
     }
 
     static class MRT1 extends SelfDescribingMarshallable implements MRTInterface {
-        final String field1;
+        String field1;
         String value = "a";
 
         MRT1(String field1) {
@@ -424,7 +424,7 @@ static class MRT1 extends SelfDescribingMarshallable implements MRTInterface {
     }
 
     static class MRT2 extends MRT1 {
-        final String field2;
+        String field2;
 
         MRT2(String field1, String field2) {
             super(field1);