Skip to content

Detect nullable marker parameters in constructor arguments. #2773

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-commons</artifactId>
<version>3.1.0-SNAPSHOT</version>
<version>3.1.x-GH-1947-SNAPSHOT</version>

<name>Spring Data Core</name>
<description>Core Spring concepts underpinning every Spring Data module.</description>
Expand Down
29 changes: 27 additions & 2 deletions src/main/java/org/springframework/data/mapping/Parameter.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public class Parameter<T, P extends PersistentProperty<P>> {

private final Lazy<Boolean> enclosingClassCache;
private final Lazy<Boolean> hasSpelExpression;
private boolean isNullableMarker;

/**
* Creates a new {@link Parameter} with the given name, {@link TypeInformation} as well as an array of
Expand All @@ -55,13 +56,28 @@ public class Parameter<T, P extends PersistentProperty<P>> {
*/
public Parameter(@Nullable String name, TypeInformation<T> type, Annotation[] annotations,
@Nullable PersistentEntity<T, P> entity) {
this(name, type, MergedAnnotations.from(annotations), entity, false);
}

/**
* Creates a new {@link Parameter} with the given name, {@link TypeInformation} as well as an array of
* {@link Annotation}s. Will inspect the annotations for an {@link Value} annotation to lookup a key or an SpEL
* expression to be evaluated.
*
* @param name the name of the parameter, can be {@literal null}
* @param type must not be {@literal null}
* @param annotations must not be {@literal null} but can be empty
* @param entity must not be {@literal null}.
*/
private Parameter(@Nullable String name, TypeInformation<T> type, MergedAnnotations annotations,
@Nullable PersistentEntity<T, P> entity, boolean isNullableMarker) {

Assert.notNull(type, "Type must not be null");
Assert.notNull(annotations, "Annotations must not be null");

this.name = name;
this.type = type;
this.annotations = MergedAnnotations.from(annotations);
this.annotations = annotations;
this.key = getValue(this.annotations);
this.entity = entity;

Expand All @@ -75,7 +91,8 @@ public Parameter(@Nullable String name, TypeInformation<T> type, Annotation[] an
return owningType.isMemberClass() && type.getType().equals(owningType.getEnclosingClass());
});

this.hasSpelExpression = Lazy.of(() -> StringUtils.hasText(getSpelExpression()));
this.hasSpelExpression = isNullableMarker ? Lazy.of(false) : Lazy.of(() -> StringUtils.hasText(getSpelExpression()));
this.isNullableMarker = isNullableMarker;
}

@Nullable
Expand All @@ -87,6 +104,10 @@ private static String getValue(MergedAnnotations annotations) {
.orElse(null);
}

public Parameter nullableMarker() {
return new Parameter(name, type, annotations, entity, true);
}

/**
* Returns the name of the parameter.
*
Expand Down Expand Up @@ -143,6 +164,10 @@ public boolean hasSpelExpression() {
return this.hasSpelExpression.get();
}

public boolean isNullableMarker() {
return isNullableMarker;
}

@Override
public boolean equals(Object o) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ static <P extends PersistentProperty<P>, T> Object[] extractInvocationArguments(

int index = 0;
for (Parameter<?, P> parameter : constructor.getParameters()) {
params[index++] = provider.getParameterValue(parameter);
params[index++] = !parameter.isNullableMarker() ? provider.getParameterValue(parameter) : null;
}

return params;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.springframework.data.mapping.model;

import kotlin.jvm.JvmClassMappingKt;
import kotlin.jvm.internal.DefaultConstructorMarker;
import kotlin.reflect.KFunction;
import kotlin.reflect.full.KClasses;
import kotlin.reflect.jvm.ReflectJvmMapping;
Expand Down Expand Up @@ -195,10 +196,24 @@ <T, P extends PersistentProperty<P>> PreferredConstructor<T, P> discover(TypeInf
}

Constructor<T> javaConstructor = ReflectJvmMapping.getJavaConstructor(primaryConstructor);

return javaConstructor != null ? buildPreferredConstructor(javaConstructor, type, entity) : null;
});
}

@Override
<T, P extends PersistentProperty<P>> Parameter buildParameter(PersistentEntity<T, P> owingEntity, String name,
TypeInformation<?> type, Annotation[] annotations) {

if (name != null) {
return super.buildParameter(owingEntity, name, type, annotations);
}

if (ClassUtils.isAssignable(DefaultConstructorMarker.class, type.getType())) {
return super.buildParameter(owingEntity, "$defaultConstructorMarker", type, annotations).nullableMarker();
}

throw new IllegalStateException("oh no - cannot build parameter for " + name);
}
};

private static final ParameterNameDiscoverer PARAMETER_NAME_DISCOVERER = new DefaultParameterNameDiscoverer();
Expand All @@ -224,8 +239,13 @@ private static Discoverers findDiscoverer(Class<?> type) {
abstract <T, P extends PersistentProperty<P>> PreferredConstructor<T, P> discover(TypeInformation<T> type,
@Nullable PersistentEntity<T, P> entity);

<T, P extends PersistentProperty<P>> Parameter buildParameter(PersistentEntity<T, P> owingEntity, String name,
TypeInformation<?> type, Annotation[] annotations) {
return new Parameter(name, type, annotations, owingEntity);
}

@SuppressWarnings({ "unchecked", "rawtypes" })
private static <T, P extends PersistentProperty<P>> PreferredConstructor<T, P> buildPreferredConstructor(
<T, P extends PersistentProperty<P>> PreferredConstructor<T, P> buildPreferredConstructor(
Constructor<?> constructor, TypeInformation<T> typeInformation, @Nullable PersistentEntity<T, P> entity) {

if (constructor.getParameterCount() == 0) {
Expand All @@ -244,7 +264,7 @@ private static <T, P extends PersistentProperty<P>> PreferredConstructor<T, P> b
TypeInformation<?> type = parameterTypes.get(i);
Annotation[] annotations = parameterAnnotations[i];

parameters[i] = new Parameter(name, type, annotations, entity);
parameters[i] = buildParameter(entity, name, type, annotations);
}

return new PreferredConstructor<>((Constructor<T>) constructor, parameters);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,33 @@ void entityInstantiatorShouldFailForAbstractClass() {
.createInstance(new BasicPersistentEntity<>(TypeInformation.of(AbstractDto.class)), provider));
}

@Test // GH-1947
void skipsValueRetrievalForKotlinDefaultConstructorMarker() {

doReturn(TheTypeConsumingAnInlineClass.class).when(entity).getType();
doReturn(PreferredConstructorDiscoverer.discover(TheTypeConsumingAnInlineClass.class))//
.when(entity).getInstanceCreatorMetadata();

ParameterValueProvider<P> valueProvider = new ParameterValueProvider<>() {

public Object getParameterValue(Parameter parameter) {

if (parameter.getName() == null || "$defaultConstructorMarker".equals(parameter.getName())) {
throw new RuntimeException("Stop, Hammer time! You can't touch this! Break id down!");
}
return parameter.getName();
}
};

Object instance = new ClassGeneratingEntityInstantiator().createInstance(entity, valueProvider);
assertThat(instance).isInstanceOf(TheTypeConsumingAnInlineClass.class).satisfies(it -> {
assertThat(it.toString()) //
.contains("inlineTypeParam=TheActualInlineClass(id=inlineTypeParam)") //
.contains("someStringParam=someStringParam") //
.contains("id=id");
});
}

private void prepareMocks(Class<?> type) {

doReturn(type).when(entity).getType();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright 2023 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.springframework.data.mapping.model

inline class TheActualInlineClass(val id: String)
data class TheTypeConsumingAnInlineClass(val inlineTypeParam: TheActualInlineClass, val someStringParam: String, val id: String)