Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

101: Add support for immutable collection constructor creation #3108

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
5c23022
101: Add support for immutable collection constructor creation
epochcoder Mar 8, 2024
d79f125
Merge branch 'master' into pr/3108
harawata Nov 16, 2024
0497117
Added tests covering basics with simpler objects
harawata Nov 18, 2024
7fdfb4d
Merge branch 'master' into feature/101-support-constructor-collection…
epochcoder Nov 19, 2024
558c8a1
#101: Pass CollectionInConstructorTest
epochcoder Nov 19, 2024
a01b27e
#101: Remove experimental flag
epochcoder Nov 19, 2024
e163a91
#101: Keep track of pending creations that were set as property mappings
epochcoder Nov 20, 2024
bc53e84
#101: Add test which violates equals contract
epochcoder Nov 21, 2024
685bafb
isle -> aisle 😑
harawata Dec 26, 2024
ccc2071
Cleanup by maven plugins
harawata Dec 27, 2024
9ade243
Remove constructor verification process
harawata Dec 27, 2024
462d890
Merge branch 'master' into pr/3108
harawata Dec 28, 2024
be53709
Merge branch 'master' into pr/3108
harawata Dec 28, 2024
d1dca2a
Merge branch 'master' into feature/101-support-constructor-collection…
epochcoder Dec 30, 2024
f3a2432
Remove redundant containsKey invocations, etc.
harawata Dec 31, 2024
9886f1d
Merge branch 'master' into pr/3108
harawata Jan 1, 2025
212cbad
Revert the unnecessary change (for diff)
harawata Jan 1, 2025
1562473
Edited en doc a little bit and copied the doc changes to the other lo…
harawata Jan 1, 2025
9a92894
License year
harawata Jan 2, 2025
5916710
Remove unused variable
epochcoder Jan 2, 2025
fd69cf0
Merge branch 'master' into feature/101-support-constructor-collection…
epochcoder Jan 2, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@
<name>Tomáš Neuberg</name>
<email>[email protected]</email>
</contributor>
<contributor>
<name>Willie Scholtz</name>
<email>[email protected]</email>
</contributor>
</contributors>

<scm>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,8 @@ private void settingsElement(Properties props) {
booleanValueOf(props.getProperty("argNameBasedConstructorAutoMapping"), false));
configuration.setDefaultSqlProviderType(resolveClass(props.getProperty("defaultSqlProviderType")));
configuration.setNullableOnForEach(booleanValueOf(props.getProperty("nullableOnForEach"), false));
configuration.setExperimentalConstructorCollectionMapping(
booleanValueOf(props.getProperty("experimentalConstructorCollectionMapping"), false));
}

private void environmentsElement(XNode context) throws Exception {
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
/*
* Copyright 2009-2024 the original author or authors.
*
* 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
*
* https://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 org.apache.ibatis.executor.resultset;

import java.lang.reflect.Constructor;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;

import org.apache.ibatis.executor.ExecutorException;
import org.apache.ibatis.mapping.ResultMap;
import org.apache.ibatis.mapping.ResultMapping;
import org.apache.ibatis.reflection.ReflectionException;
import org.apache.ibatis.reflection.factory.ObjectFactory;

/**
* Represents an object that is still to be created once all nested results with collection values have been gathered
*
* @author Willie Scholtz
*/
final class PendingConstructorCreation {

private final Class<?> resultType;
private final List<Class<?>> constructorArgTypes;
private final List<Object> constructorArgs;
private final Map<Integer, PendingCreationMetaInfo> linkedCollectionMetaInfo;
private final Map<String, Collection<Object>> linkedCollectionsByResultMapId;
private final Map<String, List<PendingConstructorCreation>> linkedCreationsByResultMapId;

PendingConstructorCreation(Class<?> resultType, List<Class<?>> types, List<Object> args) {
// since all our keys are based on result map id, we know we will never go over args size
final int maxSize = types.size();
this.linkedCollectionMetaInfo = new HashMap<>(maxSize);
this.linkedCollectionsByResultMapId = new HashMap<>(maxSize);
this.linkedCreationsByResultMapId = new HashMap<>(maxSize);
this.resultType = resultType;
this.constructorArgTypes = types;
this.constructorArgs = args;
}

@SuppressWarnings("unchecked")
Collection<Object> initializeCollectionForResultMapping(ObjectFactory objectFactory, ResultMap resultMap,
ResultMapping constructorMapping, Integer index) {
final Class<?> parameterType = constructorMapping.getJavaType();
if (!objectFactory.isCollection(parameterType)) {
throw new ExecutorException(
"Cannot add a collection result to non-collection based resultMapping: " + constructorMapping);
}

final String resultMapId = constructorMapping.getNestedResultMapId();
return linkedCollectionsByResultMapId.computeIfAbsent(resultMapId, (k) -> {
// this will allow us to verify the types of the collection before creating the final object
linkedCollectionMetaInfo.put(index, new PendingCreationMetaInfo(resultMap.getType(), resultMapId));

// will be checked before we finally create the object) as we cannot reliably do that here
return (Collection<Object>) objectFactory.create(parameterType);
});
}

void linkCreation(ResultMap nestedResultMap, PendingConstructorCreation pcc) {
final String resultMapId = nestedResultMap.getId();
final List<PendingConstructorCreation> pendingConstructorCreations = linkedCreationsByResultMapId
.computeIfAbsent(resultMapId, (k) -> new ArrayList<>());

if (pendingConstructorCreations.contains(pcc)) {
throw new ExecutorException("Cannot link inner pcc with same value, MyBatis programming error!");
}

pendingConstructorCreations.add(pcc);
}

void linkCollectionValue(ResultMapping constructorMapping, Object value) {
// not necessary to add null results to the collection (is this a config flag?)
if (value == null) {
return;
}

final String resultMapId = constructorMapping.getNestedResultMapId();
if (!linkedCollectionsByResultMapId.containsKey(resultMapId)) {
throw new ExecutorException("Cannot link collection value for resultMapping: " + constructorMapping
+ ", resultMap has not been seen/initialized yet! Internal error");
}

linkedCollectionsByResultMapId.get(resultMapId).add(value);
}

/**
* Verifies preconditions before we can actually create the result object, this is more of a sanity check to ensure
* all the mappings are as we expect them to be.
*
* @param objectFactory
* the object factory
*/
private void verifyCanCreate(ObjectFactory objectFactory) {
// before we create, we need to get the constructor to be used and verify our types match
// since we added to the collection completely unchecked
final Constructor<?> resolvedConstructor = resolveConstructor(resultType, constructorArgTypes);
final Type[] genericParameterTypes = resolvedConstructor.getGenericParameterTypes();
for (int i = 0; i < genericParameterTypes.length; i++) {
if (!linkedCollectionMetaInfo.containsKey(i)) {
continue;
}

final PendingCreationMetaInfo creationMetaInfo = linkedCollectionMetaInfo.get(i);
final Class<?> resolvedItemType = checkResolvedItemType(creationMetaInfo, genericParameterTypes[i]);

// ensure we have an empty collection if there are linked creations for this arg
final String resultMapId = creationMetaInfo.getResultMapId();
if (linkedCreationsByResultMapId.containsKey(resultMapId)) {
final Object emptyCollection = constructorArgs.get(i);
if (emptyCollection == null || !objectFactory.isCollection(emptyCollection.getClass())) {
throw new ExecutorException(
"Expected empty collection for '" + resolvedItemType + "', this is a MyBatis internal error!");
}
} else {
final Object linkedCollection = constructorArgs.get(i);
if (!linkedCollectionsByResultMapId.containsKey(resultMapId)) {
throw new ExecutorException("Expected linked collection for resultMap '" + resultMapId
+ "', not found! this is a MyBatis internal error!");
}

// comparing memory locations here (we rely on that fact)
if (linkedCollection != linkedCollectionsByResultMapId.get(resultMapId)) {
throw new ExecutorException("Expected linked collection in creation to be the same as arg for resultMap '"
+ resultMapId + "', not equal! this is a MyBatis internal error!");
}
}
}
}

private static <T> Constructor<T> resolveConstructor(Class<T> type, List<Class<?>> constructorArgTypes) {
try {
if (constructorArgTypes == null) {
return type.getDeclaredConstructor();
}

return type.getDeclaredConstructor(constructorArgTypes.toArray(new Class[0]));
} catch (Exception e) {
String argTypes = Optional.ofNullable(constructorArgTypes).orElseGet(Collections::emptyList).stream()
.map(Class::getSimpleName).collect(Collectors.joining(","));
throw new ReflectionException(
"Error resolving constructor for " + type + " with invalid types (" + argTypes + ") . Cause: " + e, e);
}
}

private static Class<?> checkResolvedItemType(PendingCreationMetaInfo creationMetaInfo, Type genericParameterTypes) {
final ParameterizedType genericParameterType = (ParameterizedType) genericParameterTypes;
final Class<?> expectedType = (Class<?>) genericParameterType.getActualTypeArguments()[0];
final Class<?> resolvedItemType = creationMetaInfo.getArgumentType();

if (!expectedType.isAssignableFrom(resolvedItemType)) {
throw new ReflectionException(
"Expected type '" + resolvedItemType + "', while the actual type of the collection was '" + expectedType
+ "', ensure your resultMap matches the type of the collection you are trying to inject");
}

return resolvedItemType;
}

@Override
public String toString() {
return "PendingConstructorCreation(" + this.hashCode() + "){" + "resultType=" + resultType + '}';
}

/**
* Recursively creates the final result of this creation.
*
* @param objectFactory
* the object factory
* @param verifyCreate
* should we verify this object can be created, should only be needed once
*
* @return the new immutable result
*/
Object create(ObjectFactory objectFactory, boolean verifyCreate) {
if (verifyCreate) {
verifyCanCreate(objectFactory);
}

final List<Object> newArguments = new ArrayList<>(constructorArgs.size());
for (int i = 0; i < constructorArgs.size(); i++) {
final PendingCreationMetaInfo creationMetaInfo = linkedCollectionMetaInfo.get(i);
final Object existingArg = constructorArgs.get(i);

if (creationMetaInfo == null) {
// we are not aware of this argument wrt pending creations
newArguments.add(existingArg);
continue;
}

// time to finally build this collection
final String resultMapId = creationMetaInfo.getResultMapId();
if (linkedCreationsByResultMapId.containsKey(resultMapId)) {
@SuppressWarnings("unchecked")
final Collection<Object> emptyCollection = (Collection<Object>) existingArg;
final List<PendingConstructorCreation> linkedCreations = linkedCreationsByResultMapId.get(resultMapId);

for (PendingConstructorCreation linkedCreation : linkedCreations) {
emptyCollection.add(linkedCreation.create(objectFactory, verifyCreate));
}

newArguments.add(emptyCollection);
continue;
}

// handle the base collection (it was built inline already)
newArguments.add(existingArg);
}

return objectFactory.create(resultType, constructorArgTypes, newArguments);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright 2009-2024 the original author or authors.
*
* 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
*
* https://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 org.apache.ibatis.executor.resultset;

/**
* @author Willie Scholtz
*/
final class PendingCreationMetaInfo {
private final Class<?> argumentType;
private final String resultMapId;

PendingCreationMetaInfo(Class<?> argumentType, String resultMapId) {
this.argumentType = argumentType;
this.resultMapId = resultMapId;
}

Class<?> getArgumentType() {
return argumentType;
}

String getResultMapId() {
return resultMapId;
}

@Override
public String toString() {
return "PendingCreationMetaInfo{" + "argumentType=" + argumentType + ", resultMapId='" + resultMapId + '\'' + '}';
}
}
16 changes: 15 additions & 1 deletion src/main/java/org/apache/ibatis/mapping/ResultMap.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2023 the original author or authors.
* Copyright 2009-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -46,6 +46,7 @@ public class ResultMap {
private Set<String> mappedColumns;
private Set<String> mappedProperties;
private Discriminator discriminator;
private boolean hasResultMapsUsingConstructorCollection;
private boolean hasNestedResultMaps;
private boolean hasNestedQueries;
private Boolean autoMapping;
Expand Down Expand Up @@ -111,6 +112,15 @@ public ResultMap build() {
}
if (resultMapping.getFlags().contains(ResultFlag.CONSTRUCTOR)) {
resultMap.constructorResultMappings.add(resultMapping);

// #101
if (resultMap.configuration.isExperimentalConstructorCollectionMappingEnabled()) {
Class<?> javaType = resultMapping.getJavaType();
resultMap.hasResultMapsUsingConstructorCollection = resultMap.hasResultMapsUsingConstructorCollection
|| (resultMapping.getNestedQueryId() == null && javaType != null
&& resultMap.configuration.getObjectFactory().isCollection(javaType));
}

if (resultMapping.getProperty() != null) {
constructorArgNames.add(resultMapping.getProperty());
}
Expand Down Expand Up @@ -210,6 +220,10 @@ public String getId() {
return id;
}

public boolean hasResultMapsUsingConstructorCollection() {
return hasResultMapsUsingConstructorCollection;
}

public boolean hasNestedResultMaps() {
return hasNestedResultMaps;
}
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/org/apache/ibatis/session/Configuration.java
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ public class Configuration {
protected boolean shrinkWhitespacesInSql;
protected boolean nullableOnForEach;
protected boolean argNameBasedConstructorAutoMapping;
protected boolean experimentalConstructorCollectionMapping;

protected String logPrefix;
protected Class<? extends Log> logImpl;
Expand Down Expand Up @@ -369,6 +370,14 @@ public boolean isSafeRowBoundsEnabled() {
return safeRowBoundsEnabled;
}

public void setExperimentalConstructorCollectionMapping(boolean experimentalConstructorCollectionMapping) {
this.experimentalConstructorCollectionMapping = experimentalConstructorCollectionMapping;
}

public boolean isExperimentalConstructorCollectionMappingEnabled() {
return experimentalConstructorCollectionMapping;
}

public void setSafeRowBoundsEnabled(boolean safeRowBoundsEnabled) {
this.safeRowBoundsEnabled = safeRowBoundsEnabled;
}
Expand Down
Loading