From f8b500fbca3e10a8def8024a208af0ffd5cf6a5a Mon Sep 17 00:00:00 2001 From: Lukas Eder Date: Mon, 14 Dec 2020 11:14:45 +0100 Subject: [PATCH] [jOOQ/jOOQ#11118] Nested records produced by DefaultRecordMapper do not pass through RecordListener lifecycle --- .../org/jooq/impl/DefaultRecordMapper.java | 158 ++++++++++-------- jOOQ/src/main/java/org/jooq/impl/Tools.java | 2 +- 2 files changed, 88 insertions(+), 72 deletions(-) diff --git a/jOOQ/src/main/java/org/jooq/impl/DefaultRecordMapper.java b/jOOQ/src/main/java/org/jooq/impl/DefaultRecordMapper.java index be7425765e..9f48308511 100644 --- a/jOOQ/src/main/java/org/jooq/impl/DefaultRecordMapper.java +++ b/jOOQ/src/main/java/org/jooq/impl/DefaultRecordMapper.java @@ -41,6 +41,7 @@ import static java.lang.Boolean.FALSE; import static java.lang.Boolean.TRUE; import static org.jooq.impl.DSL.field; import static org.jooq.impl.DSL.name; +import static org.jooq.impl.Tools.EMPTY_FIELD; import static org.jooq.impl.Tools.getAnnotatedGetter; import static org.jooq.impl.Tools.getAnnotatedMembers; import static org.jooq.impl.Tools.getAnnotatedSetters; @@ -49,6 +50,9 @@ import static org.jooq.impl.Tools.getMatchingMembers; import static org.jooq.impl.Tools.getMatchingSetters; import static org.jooq.impl.Tools.getPropertyName; import static org.jooq.impl.Tools.hasColumnAnnotations; +import static org.jooq.impl.Tools.newRecord; +import static org.jooq.impl.Tools.recordType; +import static org.jooq.impl.Tools.row0; import static org.jooq.tools.reflect.Reflect.accessible; import java.beans.ConstructorProperties; @@ -716,14 +720,12 @@ public class DefaultRecordMapper implements RecordMapper { - private final Callable constructor; - private final boolean useAnnotations; - private final List[] members; - private final List[] methods; - private Map>> nestedMappers; - private Map>> nestedMappedFields; - private Map> nestedIndexLookup; - private final E instance; + private final Callable constructor; + private final boolean useAnnotations; + private final List[] members; + private final List[] methods; + private final Map nestedMappingInfos; + private final E instance; MutablePOJOMapper(Callable constructor, E instance) { this.constructor = constructor; @@ -731,6 +733,9 @@ public class DefaultRecordMapper implements RecordMapper(); + + Map>> nestedMappedFields = null; for (int i = 0; i < fields.length; i++) { Field field = fields[i]; @@ -750,22 +755,19 @@ public class DefaultRecordMapper implements RecordMapper -1) { String prefix = name.substring(0, dot); - if (nestedMappedFields == null) { - this.nestedMappers = new HashMap<>(); - this.nestedMappedFields = new HashMap<>(); - this.nestedIndexLookup = new HashMap<>(); - } + if (nestedMappedFields == null) + nestedMappedFields = new HashMap<>(); List> f = nestedMappedFields.get(prefix); if (f == null) nestedMappedFields.put(prefix, f = new ArrayList<>()); - List indexes = nestedIndexLookup.get(prefix); - if (indexes == null) - nestedIndexLookup.put(prefix, indexes = new ArrayList<>()); + NestedMappingInfo nestedMappingInfo = nestedMappingInfos.get(prefix); + if (nestedMappingInfo == null) + nestedMappingInfos.put(prefix, nestedMappingInfo = new NestedMappingInfo()); f.add(field(name(name.substring(prefix.length() + 1)), field.getDataType())); - indexes.add(i); + nestedMappingInfo.indexLookup.add(i); members[i] = Collections.emptyList(); methods[i] = Collections.emptyList(); @@ -782,23 +784,20 @@ public class DefaultRecordMapper implements RecordMapper>> entry : nestedMappedFields.entrySet()) { String prefix = entry.getKey(); - List> list = new ArrayList<>(); - for (java.lang.reflect.Field member : getMatchingMembers(configuration, type, prefix, true)) { - list.add(configuration + NestedMappingInfo nestedMappingInfo = nestedMappingInfos.get(prefix); + nestedMappingInfo.row = Tools.row0(entry.getValue()); + nestedMappingInfo.recordDelegate = newRecord(true, recordType(nestedMappingInfo.row.size()), nestedMappingInfo.row, configuration); + + for (java.lang.reflect.Field member : getMatchingMembers(configuration, type, prefix, true)) + nestedMappingInfo.mappers.add(configuration .recordMapperProvider() - .provide(new FieldsImpl<>(entry.getValue()), member.getType()) - ); - } + .provide((RecordType) nestedMappingInfo.row.fields, member.getType())); - for (Method method : getMatchingSetters(configuration, type, prefix, true)) { - list.add(configuration + for (Method method : getMatchingSetters(configuration, type, prefix, true)) + nestedMappingInfo.mappers.add(configuration .recordMapperProvider() - .provide(new FieldsImpl<>(entry.getValue()), method.getParameterTypes()[0]) - ); - } - - nestedMappers.put(prefix, list); + .provide((RecordType) nestedMappingInfo.row.fields, method.getParameterTypes()[0])); } } } @@ -816,7 +815,6 @@ public class DefaultRecordMapper implements RecordMapper implements RecordMapper>> entry : nestedMappers.entrySet()) { - String prefix = entry.getKey(); + for (final Entry entry : nestedMappingInfos.entrySet()) { + final String prefix = entry.getKey(); - for (RecordMapper mapper : entry.getValue()) { - RecordImplN rec = new RecordImplN(nestedMappedFields.get(prefix)); + for (final RecordMapper mapper : entry.getValue().mappers) { + entry.getValue().recordDelegate.operate(new RecordOperation() { + @Override + public AbstractRecord operate(AbstractRecord rec) throws Exception { + List indexes = entry.getValue().indexLookup; + for (int index = 0; index < indexes.size(); index++) + rec.set(index, record.get(indexes.get(index))); - List indexes = nestedIndexLookup.get(prefix); - for (int index = 0; index < indexes.size(); index++) - rec.set(index, record.get(indexes.get(index))); + Object value = mapper.map(rec); + for (java.lang.reflect.Field member : getMatchingMembers(configuration, type, prefix, true)) { - Object value = mapper.map(rec); + // [#935] Avoid setting final fields + if ((member.getModifiers() & Modifier.FINAL) == 0) + map(value, result, member); + } - for (java.lang.reflect.Field member : getMatchingMembers(configuration, type, prefix, true)) { + for (Method method : getMatchingSetters(configuration, type, prefix, true)) + method.invoke(result, value); - // [#935] Avoid setting final fields - if ((member.getModifiers() & Modifier.FINAL) == 0) - map(value, result, member); + return rec; } - - for (Method method : getMatchingSetters(configuration, type, prefix, true)) - method.invoke(result, value); - } + }); } } @@ -875,7 +875,6 @@ public class DefaultRecordMapper implements RecordMapper mType = member.getType(); @@ -952,24 +951,23 @@ public class DefaultRecordMapper implements RecordMapper { - final Constructor constructor; - final Class[] parameterTypes; - private final boolean nested; - private final int[] nonNestedIndexLookup; - private final List[] nestedIndexLookup; - private final List>[] nestedMappedFields; - private final RecordMapper[] nestedMappers; + final Constructor constructor; + final Class[] parameterTypes; + private final boolean nested; + private final int[] nonNestedIndexLookup; + private final NestedMappingInfo[] nestedMappingInfo; ImmutablePOJOMapper(Constructor constructor, Class[] parameterTypes, boolean supportsNesting) { + int size = prefixes().size(); + this.constructor = accessible(constructor); this.parameterTypes = parameterTypes; - this.nestedMappedFields = new List[prefixes().size()]; - this.nestedMappers = new RecordMapper[prefixes().size()]; - this.nestedIndexLookup = new List[prefixes().size()]; - this.nonNestedIndexLookup = new int[prefixes().size()]; + this.nestedMappingInfo = new NestedMappingInfo[size]; + this.nonNestedIndexLookup = new int[size]; int i = -1; boolean hasNestedFields = false; + List>[] nestedMappedFields = new List[size]; if (supportsNesting) { @@ -987,22 +985,24 @@ public class DefaultRecordMapper implements RecordMapper(); - if (nestedIndexLookup[i] == null) - nestedIndexLookup[i] = new ArrayList<>(); + if (nestedMappingInfo[i] == null) + nestedMappingInfo[i] = new NestedMappingInfo(); nestedMappedFields[i].add(field( name(fields[j].getName().substring(prefix.length() + 1)), fields[j].getDataType() )); - nestedIndexLookup[i].add(j); + nestedMappingInfo[i].indexLookup.add(j); } } if (nestedMappedFields[i] != null) { - nestedMappers[i] = configuration + nestedMappingInfo[i].row = row0(nestedMappedFields[i].toArray(EMPTY_FIELD)); + nestedMappingInfo[i].recordDelegate = newRecord(true, recordType(nestedMappingInfo[i].row.size()), nestedMappingInfo[i].row, configuration); + nestedMappingInfo[i].mappers.add(configuration .recordMapperProvider() - .provide((RecordType) new FieldsImpl<>(nestedMappedFields[i]), parameterTypes[i]); + .provide((RecordType) nestedMappingInfo[i].row.fields, parameterTypes[i])); } } } @@ -1037,16 +1037,20 @@ public class DefaultRecordMapper implements RecordMapper indexLookup = nestedMappingInfo[i].indexLookup; - for (int index = 0; index < nestedIndexLookup[i].size(); index++) - rec.set(index, record.get(nestedIndexLookup[i].get(index))); - - converted[i] = nestedMappers[i].map(rec); + converted[i] = nestedMappingInfo[i].mappers.get(0).map(nestedMappingInfo[i].recordDelegate.operate(new RecordOperation() { + @Override + public AbstractRecord operate(AbstractRecord rec) { + for (int j = 0; j < indexLookup.size(); j++) + rec.set(j, record.get(indexLookup.get(j))); + return rec; + } + })); } } @@ -1155,4 +1159,16 @@ public class DefaultRecordMapper implements RecordMapper> mappers; + AbstractRow row; + final List indexLookup; + RecordDelegate recordDelegate; + + NestedMappingInfo() { + mappers = new ArrayList<>(); + indexLookup = new ArrayList<>(); + } + } } diff --git a/jOOQ/src/main/java/org/jooq/impl/Tools.java b/jOOQ/src/main/java/org/jooq/impl/Tools.java index 883c0bd1fc..0624d44e8d 100644 --- a/jOOQ/src/main/java/org/jooq/impl/Tools.java +++ b/jOOQ/src/main/java/org/jooq/impl/Tools.java @@ -958,7 +958,7 @@ final class Tools { return row0(new FieldsImpl<>(fields)); } - static final Class recordType(int length) { + static final Class recordType(int length) { switch (length) {