Skip to content

Commit

Permalink
Issue a warning when final field is overwritten #414
Browse files Browse the repository at this point in the history
  • Loading branch information
Ilya Kaznacheev committed Mar 23, 2022
1 parent 44c2bcf commit 2361c17
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 0 deletions.
25 changes: 25 additions & 0 deletions src/main/java/net/openhft/chronicle/wire/WireMarshaller.java
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,7 @@ abstract static class FieldAccess {

Comment commentAnnotation;
Boolean isLeaf;
boolean isFinalNoWarning;

FieldAccess(@NotNull Field field) {
this(field, null);
Expand All @@ -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) {
Expand Down Expand Up @@ -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);

Expand All @@ -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);
Expand All @@ -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, "Found 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;
Expand Down Expand Up @@ -1093,6 +1110,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();
Expand Down Expand Up @@ -1248,6 +1267,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();
Expand Down Expand Up @@ -1337,6 +1358,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();
Expand Down Expand Up @@ -1417,6 +1440,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();
Expand Down
72 changes: 72 additions & 0 deletions src/test/java/net/openhft/chronicle/wire/FinalFieldsTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* 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("Found final field map");
expectException("Found final field array");
expectException("Found final field intValue");
expectException("Found 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);
}

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);
}

@SuppressWarnings("unused")
private static class FinalFieldsClass extends SelfDescribingMarshallable {
final Map<CcyPair, String> map;
final String[] array;
final int intValue;
final double value;

public FinalFieldsClass(Map<CcyPair, String> map, String[] array, int intValue, double value) {
this.map = map;
this.array = array;
this.intValue = intValue;
this.value = value;
}
}
}

0 comments on commit 2361c17

Please sign in to comment.