Skip to content

Commit

Permalink
Retain default LocalVariableTableParameterNameDiscoverer with warn lo…
Browse files Browse the repository at this point in the history
…g entries

For a transition period, LocalVariableTableParameterNameDiscoverer logs a warning for each successful resolution attempt now, suggesting that -parameters was missed.

See gh-29531
See gh-29559
  • Loading branch information
jhoeller committed Nov 23, 2022
1 parent ed5ab77 commit fe5bd67
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@

/**
* Default implementation of the {@link ParameterNameDiscoverer} strategy interface,
* delegating to the Java 8 standard reflection mechanism.
* delegating to the Java 8 standard reflection mechanism, with a deprecated fallback
* to {@link LocalVariableTableParameterNameDiscoverer}.
*
* <p>If a Kotlin reflection implementation is present,
* {@link KotlinReflectionParameterNameDiscoverer} is added first in the list and
Expand All @@ -35,11 +36,20 @@
*/
public class DefaultParameterNameDiscoverer extends PrioritizedParameterNameDiscoverer {

@SuppressWarnings("removal")
public DefaultParameterNameDiscoverer() {
if (KotlinDetector.isKotlinReflectPresent()) {
addDiscoverer(new KotlinReflectionParameterNameDiscoverer());
}

// Recommended approach on Java 8+: compilation with -parameters.
addDiscoverer(new StandardReflectionParameterNameDiscoverer());

// Deprecated fallback to class file parsing for -debug symbols.
// Does not work on native images without class file resources.
if (!NativeDetector.inNativeImage()) {
addDiscoverer(new LocalVariableTableParameterNameDiscoverer());
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
* @deprecated as of 6.0.1, in favor of {@link StandardReflectionParameterNameDiscoverer}
* (with the "-parameters" compiler flag)
*/
@Deprecated(since = "6.0.1")
@Deprecated(since = "6.0.1", forRemoval = true)
public class LocalVariableTableParameterNameDiscoverer implements ParameterNameDiscoverer {

private static final Log logger = LogFactory.getLog(LocalVariableTableParameterNameDiscoverer.class);
Expand Down Expand Up @@ -85,7 +85,12 @@ public String[] getParameterNames(Constructor<?> ctor) {
private String[] doGetParameterNames(Executable executable) {
Class<?> declaringClass = executable.getDeclaringClass();
Map<Executable, String[]> map = this.parameterNamesCache.computeIfAbsent(declaringClass, this::inspectClass);
return (map != NO_DEBUG_INFO_MAP ? map.get(executable) : null);
String[] names = (map != NO_DEBUG_INFO_MAP ? map.get(executable) : null);
if (names != null && logger.isWarnEnabled()) {
logger.warn("Using deprecated '-debug' fallback for parameter name resolution. " +
"Compile the affected code with '-parameters' instead: " + executable);
}
return names;
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2022 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 @@ -339,11 +339,7 @@ void registerAdapters(ReactiveAdapterRegistry registry) {

/**
* {@code BlockHoundIntegration} for spring-core classes.
* <p>Explicitly allow the following:
* <ul>
* <li>Reading class info via {@link LocalVariableTableParameterNameDiscoverer}.
* <li>Locking within {@link ConcurrentReferenceHashMap}.
* </ul>
* Explicitly allows locking within {@link ConcurrentReferenceHashMap}.
* @since 5.2.4
*/
public static class SpringCoreBlockHoundIntegration implements BlockHoundIntegration {
Expand All @@ -352,9 +348,6 @@ public static class SpringCoreBlockHoundIntegration implements BlockHoundIntegra
public void applyTo(BlockHound.Builder builder) {
// Avoid hard references potentially anywhere in spring-core (no need for structural dependency)

builder.allowBlockingCallsInside(
"org.springframework.core.LocalVariableTableParameterNameDiscoverer", "inspectClass");

String className = "org.springframework.util.ConcurrentReferenceHashMap$Segment";
builder.allowBlockingCallsInside(className, "doTask");
builder.allowBlockingCallsInside(className, "clear");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2022 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 @@ -34,7 +34,8 @@
*/
class LocalVariableTableParameterNameDiscovererTests {

private final LocalVariableTableParameterNameDiscoverer discoverer = new LocalVariableTableParameterNameDiscoverer();
@SuppressWarnings("removal")
private final ParameterNameDiscoverer discoverer = new LocalVariableTableParameterNameDiscoverer();


@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package org.springframework.core;

import java.lang.reflect.Method;
import java.util.Map;
import java.util.concurrent.CompletableFuture;

Expand All @@ -28,7 +27,6 @@
import reactor.core.scheduler.ReactorBlockHoundIntegration;
import reactor.core.scheduler.Schedulers;

import org.springframework.tests.sample.objects.TestObject;
import org.springframework.util.ConcurrentReferenceHashMap;

import static org.assertj.core.api.Assertions.assertThat;
Expand All @@ -52,11 +50,10 @@
@DisabledOnJre(value= {JRE.JAVA_18, JRE.JAVA_19}, disabledReason = "BlockHound is not compatible with Java 18+")
class SpringCoreBlockHoundIntegrationTests {


@BeforeAll
static void setUp() {
static void setup() {
BlockHound.builder()
.with(new ReactorBlockHoundIntegration()) // Reactor non-blocking thread predicate
.with(new ReactorBlockHoundIntegration()) // Reactor non-blocking thread predicate
.with(new ReactiveAdapterRegistry.SpringCoreBlockHoundIntegration())
.install();
}
Expand All @@ -68,15 +65,6 @@ void blockHoundIsInstalled() {
.hasMessageContaining("Blocking call!");
}

@Test
void localVariableTableParameterNameDiscoverer() {
testNonBlockingTask(() -> {
Method setName = TestObject.class.getMethod("setName", String.class);
String[] names = new LocalVariableTableParameterNameDiscoverer().getParameterNames(setName);
assertThat(names).containsExactly("name");
});
}

@Test
void concurrentReferenceHashMap() {
int size = 10000;
Expand Down

0 comments on commit fe5bd67

Please sign in to comment.