Skip to content

Commit 0a93096

Browse files
raydacraydac
committed
refactoring
1 parent dfc585d commit 0a93096

File tree

4 files changed

+54
-163
lines changed

4 files changed

+54
-163
lines changed

‎jbbp/src/main/java/com/igormaznitsa/jbbp/io/AbstractMappedClassFieldObserver.java‎

Lines changed: 7 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -20,34 +20,24 @@
2020
import com.igormaznitsa.jbbp.exceptions.JBBPIllegalArgumentException;
2121
import com.igormaznitsa.jbbp.mapper.Bin;
2222
import com.igormaznitsa.jbbp.mapper.BinType;
23+
import com.igormaznitsa.jbbp.mapper.JBBPMapper;
24+
import com.igormaznitsa.jbbp.mapper.MappedFieldRecord;
2325
import com.igormaznitsa.jbbp.model.JBBPFieldInt;
2426
import com.igormaznitsa.jbbp.model.JBBPFieldLong;
2527
import com.igormaznitsa.jbbp.model.JBBPFieldShort;
2628
import com.igormaznitsa.jbbp.model.JBBPFieldString;
2729
import com.igormaznitsa.jbbp.utils.DslBinCustom;
2830
import com.igormaznitsa.jbbp.utils.JBBPUtils;
29-
import com.igormaznitsa.jbbp.utils.ReflectUtils;
3031
import java.lang.reflect.Array;
3132
import java.lang.reflect.Field;
32-
import java.lang.reflect.Modifier;
33-
import java.util.ArrayList;
34-
import java.util.Collections;
35-
import java.util.HashMap;
3633
import java.util.List;
37-
import java.util.Map;
3834

3935
/**
4036
* Abstract class to collect, order and process all fields in a mapped class.
4137
* since 1.1
4238
*/
4339
public abstract class AbstractMappedClassFieldObserver {
4440

45-
/**
46-
* Internal cache to keep outOrder of fields for classes for data output. It is
47-
* lazy initialized field.
48-
*/
49-
private static Map<Class<?>, Field[]> cachedClasses;
50-
5141
/**
5242
* Inside auxiliary method to read object field value.
5343
*
@@ -87,69 +77,7 @@ private static void assertFieldArray(final Field field) {
8777
protected void processObject(final Object obj, Field field, final Object customFieldProcessor) {
8878
JBBPUtils.assertNotNull(obj, "Object must not be null");
8979

90-
Field[] orderedFields = null;
91-
92-
final Map<Class<?>, Field[]> fieldz;
93-
if (cachedClasses == null) {
94-
fieldz = new HashMap<>();
95-
cachedClasses = fieldz;
96-
} else {
97-
fieldz = cachedClasses;
98-
synchronized (cachedClasses) {
99-
orderedFields = fieldz.get(obj.getClass());
100-
}
101-
}
102-
103-
if (orderedFields == null) {
104-
// find out the outOrder of fields and fields which should be serialized
105-
final List<Class<?>> listOfClassHierarchy = new ArrayList<>();
106-
final List<OrderedField> fields = new ArrayList<>();
107-
108-
Class<?> current = obj.getClass();
109-
while (current != java.lang.Object.class) {
110-
listOfClassHierarchy.add(current);
111-
current = current.getSuperclass();
112-
}
113-
114-
for (int i = listOfClassHierarchy.size() - 1; i >= 0; i--) {
115-
final Class<?> clazzToProcess = listOfClassHierarchy.get(i);
116-
final Bin clazzAnno = clazzToProcess.getAnnotation(Bin.class);
117-
118-
for (Field f : clazzToProcess.getDeclaredFields()) {
119-
if (!ReflectUtils.isPotentiallyAccessibleField(f)) {
120-
f = ReflectUtils.makeAccessible(f);
121-
}
122-
123-
final int modifiers = f.getModifiers();
124-
if (Modifier.isTransient(modifiers) || Modifier.isStatic(modifiers) || f.getName().indexOf('$') >= 0) {
125-
continue;
126-
}
127-
128-
Bin fieldAnno = f.getAnnotation(Bin.class);
129-
fieldAnno = fieldAnno == null ? clazzAnno : fieldAnno;
130-
if (fieldAnno == null) {
131-
continue;
132-
}
133-
134-
fields.add(new OrderedField(fieldAnno.outOrder(), f));
135-
}
136-
}
137-
138-
Collections.sort(fields);
139-
140-
orderedFields = new Field[fields.size()];
141-
for (int i = 0; i < fields.size(); i++) {
142-
orderedFields[i] = fields.get(i).field;
143-
}
144-
145-
synchronized (fieldz) {
146-
fieldz.put(obj.getClass(), orderedFields);
147-
}
148-
}
149-
150-
if (field != null && !ReflectUtils.isPotentiallyAccessibleField(field)) {
151-
field = ReflectUtils.makeAccessible(field);
152-
}
80+
final List<MappedFieldRecord> orderedFields = JBBPMapper.findAffectedFields(obj);
15381

15482
final Bin clazzAnno = obj.getClass().getAnnotation(Bin.class);
15583
final DslBinCustom clazzCustomAnno = obj.getClass().getAnnotation(DslBinCustom.class);
@@ -158,20 +86,14 @@ protected void processObject(final Object obj, Field field, final Object customF
15886

15987
this.onStructStart(obj, field, clazzAnno == null ? fieldAnno : clazzAnno);
16088

161-
for (final Field f : orderedFields) {
162-
Bin binAnno = f.getAnnotation(Bin.class);
163-
if (binAnno == null) {
164-
binAnno = f.getDeclaringClass().getAnnotation(Bin.class);
165-
if (binAnno == null) {
166-
throw new JBBPIllegalArgumentException("Can't find any Bin annotation to use for " + f + " field");
167-
}
168-
}
89+
for (final MappedFieldRecord rec : orderedFields) {
90+
Bin binAnno = rec.binAnnotation;
16991

17092
if (binAnno.custom() && customFieldProcessor == null) {
171-
throw new JBBPIllegalArgumentException("The Class '" + obj.getClass().getName() + "' contains the field '" + f.getName() + "\' which is a custom one, you must provide a JBBPCustomFieldWriter instance to save the field.");
93+
throw new JBBPIllegalArgumentException("The Class '" + obj.getClass().getName() + "' contains the field '" + rec.mappingField.getName() + "\' which is a custom one, you must provide a JBBPCustomFieldWriter instance to save the field.");
17294
}
17395

174-
processObjectField(obj, f, binAnno, customFieldProcessor);
96+
processObjectField(obj, rec.mappingField, binAnno, customFieldProcessor);
17597
}
17698

17799
this.onStructEnd(obj, field, clazzAnno == null ? fieldAnno : clazzAnno);
@@ -690,52 +612,4 @@ protected void onArrayEnd(final Object obj, final Field field, final Bin annotat
690612

691613
}
692614

693-
/**
694-
* Inside JBBPOut.Bin command creates cached list of fields of a saved class,
695-
* the method allows to reset the inside cache.
696-
*/
697-
public void resetInsideClassCache() {
698-
final Map<Class<?>, Field[]> fieldz = cachedClasses;
699-
if (fieldz != null) {
700-
synchronized (fieldz) {
701-
fieldz.clear();
702-
}
703-
}
704-
}
705-
706-
/**
707-
* An Auxiliary class to be used for class field ordering in save operations.
708-
*/
709-
private static final class OrderedField implements Comparable<OrderedField> {
710-
711-
final int order;
712-
final Field field;
713-
714-
OrderedField(final int order, final Field field) {
715-
this.order = order;
716-
this.field = field;
717-
}
718-
719-
@Override
720-
public boolean equals(final Object obj) {
721-
return obj != null && (obj == this || (obj instanceof OrderedField && this.field.equals(((OrderedField) obj).field)));
722-
}
723-
724-
@Override
725-
public int hashCode() {
726-
return this.order;
727-
}
728-
729-
@Override
730-
public int compareTo(final OrderedField o) {
731-
final int result;
732-
if (this.order == o.order) {
733-
result = this.field.getName().compareTo(o.field.getName());
734-
} else {
735-
result = this.order < o.order ? -1 : 1;
736-
}
737-
return result;
738-
}
739-
}
740-
741615
}

‎jbbp/src/main/java/com/igormaznitsa/jbbp/mapper/JBBPMapper.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.lang.reflect.Field;
2828
import java.lang.reflect.Modifier;
2929
import java.util.ArrayList;
30+
import java.util.Collections;
3031
import java.util.List;
3132
import java.util.Map;
3233
import java.util.concurrent.ConcurrentHashMap;
@@ -268,7 +269,7 @@ public static <T> T map(final JBBPFieldStruct rootStructure, final T instance, f
268269
JBBPUtils.assertNotNull(instance, "The Mapping class instance must not be null");
269270

270271
// Don't use forEach() for Android compatibility!
271-
for (final MappedFieldRecord record : makeListOfRecords(instance)) {
272+
for (final MappedFieldRecord record : findAffectedFields(instance)) {
272273
processFieldOfMappedClass(
273274
record,
274275
rootStructure,
@@ -287,7 +288,7 @@ public static <T> T map(final JBBPFieldStruct rootStructure, final T instance, f
287288
* @return number of classes
288289
* @since 2.0.0
289290
*/
290-
public static int getClassCacheSize() {
291+
public static int getFieldCacheSize() {
291292
return CACHED_FIELDS.size();
292293
}
293294

@@ -296,11 +297,11 @@ public static int getClassCacheSize() {
296297
*
297298
* @since 2.0.0
298299
*/
299-
public static void clearClassCache() {
300+
public static void clearFieldCache() {
300301
CACHED_FIELDS.clear();
301302
}
302303

303-
private static List<MappedFieldRecord> makeListOfRecords(final Object instance) {
304+
public static List<MappedFieldRecord> findAffectedFields(final Object instance) {
304305
final Class<?> mappingClass = instance.getClass();
305306

306307
List<MappedFieldRecord> result = CACHED_FIELDS.get(mappingClass);
@@ -320,7 +321,7 @@ private static List<MappedFieldRecord> makeListOfRecords(final Object instance)
320321
for (final Class<?> processingClazz : listOfClassHierarchy) {
321322
for (Field mappingField : processingClazz.getDeclaredFields()) {
322323
final int modifiers = mappingField.getModifiers();
323-
if (Modifier.isTransient(modifiers) || Modifier.isStatic(modifiers)) {
324+
if (Modifier.isTransient(modifiers) || Modifier.isStatic(modifiers) || Modifier.isFinal(modifiers)) {
324325
continue;
325326
}
326327

@@ -343,6 +344,8 @@ private static List<MappedFieldRecord> makeListOfRecords(final Object instance)
343344
}
344345
}
345346

347+
Collections.sort(result);
348+
346349
CACHED_FIELDS.put(mappingClass, result);
347350
}
348351

‎jbbp/src/main/java/com/igormaznitsa/jbbp/mapper/MappedFieldRecord.java‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
import java.lang.reflect.Method;
2929
import java.lang.reflect.Modifier;
3030

31-
final class MappedFieldRecord {
31+
public final class MappedFieldRecord implements Comparable<MappedFieldRecord> {
3232
private static final Function<Class<?>, Object> STATIC_MAKE_CLASS_INSTANCE_INSTANTIATOR = (Class<?> klazz) -> {
3333
Class<?> currentClass = klazz;
3434
Object result = null;
@@ -143,15 +143,16 @@ final class MappedFieldRecord {
143143
}
144144
}
145145
};
146-
final Field mappingField;
147-
final Class<?> mappingClass;
148-
final Bin binAnnotation;
149-
final boolean bitWideField;
150-
final String fieldName;
151-
final String fieldPath;
152-
final JBBPBitNumber mappedBitNumber;
153-
final BinType fieldType;
154-
final FieldProcessor proc;
146+
147+
public final Field mappingField;
148+
public final Class<?> mappingClass;
149+
public final Bin binAnnotation;
150+
public final boolean bitWideField;
151+
public final String fieldName;
152+
public final String fieldPath;
153+
public final JBBPBitNumber mappedBitNumber;
154+
public final BinType fieldType;
155+
public final FieldProcessor proc;
155156

156157
MappedFieldRecord(final Field mappingField,
157158
final Class<?> mappingClass,
@@ -421,4 +422,17 @@ void apply(
421422
);
422423
}
423424

425+
@Override
426+
public int compareTo(final MappedFieldRecord o) {
427+
final int thisOrder = this.binAnnotation.outOrder();
428+
final int thatOrder = o.binAnnotation.outOrder();
429+
430+
final int result;
431+
if (thisOrder == thatOrder) {
432+
result = this.mappingField.getName().compareTo(o.mappingField.getName());
433+
} else {
434+
result = thisOrder < thatOrder ? -1 : 1;
435+
}
436+
return result;
437+
}
424438
}

‎jbbp/src/test/java/com/igormaznitsa/jbbp/mapper/JBBPMapperTest.java‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,29 @@
1616

1717
package com.igormaznitsa.jbbp.mapper;
1818

19+
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
20+
import static org.junit.jupiter.api.Assertions.assertEquals;
21+
import static org.junit.jupiter.api.Assertions.assertFalse;
22+
import static org.junit.jupiter.api.Assertions.assertNotNull;
23+
import static org.junit.jupiter.api.Assertions.assertNull;
24+
import static org.junit.jupiter.api.Assertions.assertSame;
25+
import static org.junit.jupiter.api.Assertions.assertThrows;
26+
import static org.junit.jupiter.api.Assertions.assertTrue;
27+
import static org.junit.jupiter.api.Assertions.fail;
28+
29+
1930
import com.igormaznitsa.jbbp.JBBPParser;
2031
import com.igormaznitsa.jbbp.TestUtils;
2132
import com.igormaznitsa.jbbp.exceptions.JBBPMapperException;
2233
import com.igormaznitsa.jbbp.io.JBBPBitNumber;
2334
import com.igormaznitsa.jbbp.io.JBBPOut;
2435
import com.igormaznitsa.jbbp.model.JBBPFieldInt;
25-
import org.junit.jupiter.api.Test;
26-
2736
import java.io.ByteArrayInputStream;
2837
import java.lang.reflect.Field;
2938
import java.security.AccessController;
3039
import java.security.PrivilegedAction;
3140
import java.util.Random;
32-
33-
import static org.junit.jupiter.api.Assertions.*;
41+
import org.junit.jupiter.api.Test;
3442

3543
public class JBBPMapperTest {
3644

@@ -904,7 +912,7 @@ class Parsed {
904912
}
905913

906914
@Test
907-
void testMap_Structure_WholeStream_LocalClassesNonDefaultConstructorsAndFinalFields() throws Exception {
915+
void testMap_Structure_IgnoringFinalFieldsInMapping() throws Exception {
908916
@Bin
909917
class Struct {
910918

@@ -932,18 +940,10 @@ class Parsed {
932940
rnd.nextBytes(array);
933941

934942
final Parsed parsed = JBBPParser.prepare("struct [_] { byte a; byte b; }").parse(new ByteArrayInputStream(array)).mapTo(new Parsed(null), aClass -> {
935-
if (aClass == Struct.class) {
936-
return new Struct((byte) 0, (byte) 0);
937-
}
943+
fail("Must not be called");
938944
return null;
939945
});
940-
assertEquals(array.length / 2, parsed.struct.length);
941-
942-
for (int i = 0; i < array.length; i += 2) {
943-
final Struct s = parsed.struct[i / 2];
944-
assertEquals(array[i], s.a);
945-
assertEquals(array[i + 1], s.b);
946-
}
946+
assertNull(parsed.struct);
947947
}
948948

949949
@Test

0 commit comments

Comments
 (0)