Skip to content

Commit 611ea33

Browse files
alzimmermsftalzimmermsft
authored
Rewrite Internal Reflection Logic (Azure#36612)
Rewrite Internal Reflection Logic
1 parent a2d1db2 commit 611ea33

30 files changed

+1026
-652
lines changed

‎eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,8 +462,9 @@
462462
<!-- The casting problems do not exist in Azure use cases -->
463463
<Match>
464464
<Or>
465+
<Class name="com.azure.core.implementation.MethodHandleReflectiveInvoker"/>
466+
<Class name="com.azure.core.implementation.ReflectionUtilsMethodHandle"/>
465467
<Class name="com.azure.core.implementation.TypeUtil"/>
466-
<Class name="com.azure.core.implementation.ReflectionUtils"/>
467468
</Or>
468469
<Bug pattern="BC_UNCONFIRMED_CAST"/>
469470
</Match>

‎sdk/core/azure-core-serializer-json-gson/src/main/java/com/azure/core/serializer/json/gson/implementation/JsonSerializableTypeAdapter.java‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
package com.azure.core.serializer.json.gson.implementation;
55

6+
import com.azure.core.implementation.ReflectiveInvoker;
67
import com.azure.core.implementation.ReflectionUtils;
78
import com.azure.core.util.logging.ClientLogger;
89
import com.azure.json.JsonSerializable;
@@ -11,8 +12,6 @@
1112
import com.google.gson.stream.JsonWriter;
1213

1314
import java.io.IOException;
14-
import java.lang.invoke.MethodHandle;
15-
import java.lang.invoke.MethodHandles;
1615

1716
/**
1817
* Implementation of GSON's {@link TypeAdapter} that is capable of handling {@link JsonSerializable} types.
@@ -21,7 +20,7 @@ public class JsonSerializableTypeAdapter extends TypeAdapter<JsonSerializable<?>
2120
private static final ClientLogger LOGGER = new ClientLogger(JsonSerializableTypeAdapter.class);
2221

2322
private final Class<? extends JsonSerializable<?>> jsonSerializableType;
24-
private final MethodHandle readJson;
23+
private final ReflectiveInvoker readJson;
2524

2625
/**
2726
* Creates an instance of {@link JsonSerializableTypeAdapter}.
@@ -32,9 +31,8 @@ public class JsonSerializableTypeAdapter extends TypeAdapter<JsonSerializable<?>
3231
public JsonSerializableTypeAdapter(Class<? extends JsonSerializable<?>> jsonSerializableType) {
3332
this.jsonSerializableType = jsonSerializableType;
3433
try {
35-
MethodHandles.Lookup lookup = ReflectionUtils.getLookupToUse(jsonSerializableType);
36-
this.readJson = lookup.unreflect(jsonSerializableType.getDeclaredMethod("fromJson",
37-
com.azure.json.JsonReader.class));
34+
this.readJson = ReflectionUtils.getMethodInvoker(jsonSerializableType,
35+
jsonSerializableType.getDeclaredMethod("fromJson", com.azure.json.JsonReader.class));
3836
} catch (Exception e) {
3937
throw LOGGER.logExceptionAsError(new IllegalStateException(e));
4038
}
@@ -49,13 +47,11 @@ public void write(JsonWriter out, JsonSerializable<?> value) throws IOException
4947
public JsonSerializable<?> read(JsonReader in) throws IOException {
5048
try {
5149
return jsonSerializableType.cast(readJson.invokeWithArguments(new GsonJsonReader(in, null, true)));
52-
} catch (Throwable e) {
53-
if (e instanceof IOException) {
54-
throw (IOException) e;
55-
} else if (e instanceof Exception) {
56-
throw new IOException(e);
50+
} catch (Exception exception) {
51+
if (exception instanceof IOException) {
52+
throw (IOException) exception;
5753
} else {
58-
throw (Error) e;
54+
throw new IOException(exception);
5955
}
6056
}
6157
}

‎sdk/core/azure-core-serializer-json-jackson/src/main/java/com/azure/core/serializer/json/jackson/implementation/HeaderCollectionHandler.java‎

Lines changed: 28 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,12 @@
33

44
package com.azure.core.serializer.json.jackson.implementation;
55

6+
import com.azure.core.implementation.ReflectiveInvoker;
67
import com.azure.core.implementation.ReflectionUtils;
78
import com.azure.core.util.logging.ClientLogger;
9+
import com.azure.core.util.logging.LogLevel;
810

9-
import java.lang.invoke.MethodHandle;
10-
import java.lang.invoke.MethodHandles;
11-
import java.lang.invoke.MethodType;
1211
import java.lang.reflect.Field;
13-
import java.lang.reflect.Method;
1412
import java.security.PrivilegedAction;
1513
import java.util.HashMap;
1614
import java.util.Locale;
@@ -22,10 +20,10 @@
2220
*/
2321
final class HeaderCollectionHandler {
2422
private static final int CACHE_SIZE_LIMIT = 10000;
25-
private static final Map<Field, MethodHandle> FIELD_TO_SETTER_CACHE = new ConcurrentHashMap<>();
23+
private static final Map<Field, ReflectiveInvoker> FIELD_TO_SETTER_INVOKER_CACHE = new ConcurrentHashMap<>();
2624

2725
// Dummy constant that indicates no setter was found for the Field.
28-
private static final MethodHandle NO_SETTER_HANDLE = MethodHandles.identity(HeaderCollectionHandler.class);
26+
private static final ReflectiveInvoker NO_SETTER_REFLECTIVE_INVOKER = ReflectionUtils.createNoOpInvoker();
2927

3028
private final String prefix;
3129
private final int prefixLength;
@@ -86,24 +84,23 @@ private boolean usePublicSetter(Object deserializedHeaders, ClientLogger logger)
8684
final String clazzSimpleName = clazz.getSimpleName();
8785
final String fieldName = declaringField.getName();
8886

89-
MethodHandle setterHandler = getFromCache(declaringField, clazz, clazzSimpleName, fieldName, logger);
87+
ReflectiveInvoker
88+
setterReflectiveInvoker = getFromCache(declaringField, clazz, clazzSimpleName, fieldName, logger);
9089

91-
if (setterHandler == NO_SETTER_HANDLE) {
90+
if (setterReflectiveInvoker == NO_SETTER_REFLECTIVE_INVOKER) {
9291
return false;
9392
}
9493

9594
try {
96-
setterHandler.invokeWithArguments(deserializedHeaders, values);
97-
logger.verbose("Set header collection {} on class {} using MethodHandle.", fieldName, clazzSimpleName);
95+
setterReflectiveInvoker.invokeWithArguments(deserializedHeaders, values);
96+
logger.log(LogLevel.VERBOSE, () ->
97+
"Set header collection " + fieldName + " on class " + clazzSimpleName + " using reflection.");
9898

9999
return true;
100-
} catch (Throwable ex) {
101-
if (ex instanceof Error) {
102-
throw (Error) ex;
103-
}
104-
105-
logger.verbose("Failed to set header {} collection on class {} using MethodHandle.", fieldName,
106-
clazzSimpleName, ex);
100+
} catch (Exception ex) {
101+
logger.log(LogLevel.VERBOSE, () ->
102+
"Failed to set header " + fieldName + " collection on class " + clazzSimpleName + " using reflection.",
103+
ex);
107104
return false;
108105
}
109106
}
@@ -112,58 +109,27 @@ private static String getPotentialSetterName(String fieldName) {
112109
return "set" + fieldName.substring(0, 1).toUpperCase(Locale.ROOT) + fieldName.substring(1);
113110
}
114111

115-
private static MethodHandle getFromCache(Field key, Class<?> clazz, String clazzSimpleName,
112+
private static ReflectiveInvoker getFromCache(Field key, Class<?> clazz, String clazzSimpleName,
116113
String fieldName, ClientLogger logger) {
117-
if (FIELD_TO_SETTER_CACHE.size() >= CACHE_SIZE_LIMIT) {
118-
FIELD_TO_SETTER_CACHE.clear();
114+
if (FIELD_TO_SETTER_INVOKER_CACHE.size() >= CACHE_SIZE_LIMIT) {
115+
FIELD_TO_SETTER_INVOKER_CACHE.clear();
119116
}
120117

121-
return FIELD_TO_SETTER_CACHE.computeIfAbsent(key, field -> {
122-
MethodHandles.Lookup lookupToUse;
123-
try {
124-
lookupToUse = ReflectionUtils.getLookupToUse(clazz);
125-
} catch (Exception ex) {
126-
logger.verbose("Failed to retrieve MethodHandles.Lookup for field {}. Will attempt to make field accessible.", field, ex);
127-
128-
// In a previous implementation compute returned null here in an attempt to indicate that there is no
129-
// setter for the field. Unfortunately, null isn't a valid indicator to computeIfAbsent that a
130-
// computation has been performed and this cache would never effectively be a cache as compute would
131-
// always be performed when there was no setter for the field.
132-
//
133-
// Now the implementation returns a dummy constant when there is no setter for the field. This now
134-
// results in this case properly inserting into the cache and only running when a new type is seen or
135-
// the cache is cleared due to reaching capacity.
136-
return NO_SETTER_HANDLE;
137-
}
138-
118+
return FIELD_TO_SETTER_INVOKER_CACHE.computeIfAbsent(key, field -> {
139119
String setterName = getPotentialSetterName(fieldName);
140120

141121
try {
142-
MethodHandle handle = lookupToUse.findVirtual(clazz, setterName,
143-
MethodType.methodType(clazz, Map.class));
122+
ReflectiveInvoker reflectiveInvoker = ReflectionUtils.getMethodInvoker(clazz, clazz.getDeclaredMethod(setterName,
123+
Map.class));
144124

145-
logger.verbose("Using MethodHandle for setter {} on class {}.", setterName, clazzSimpleName);
125+
logger.log(LogLevel.VERBOSE, () ->
126+
"Using invoker for setter " + setterName + " on class " + clazzSimpleName + ".");
146127

147-
return handle;
148-
} catch (ReflectiveOperationException ex) {
149-
logger.verbose("Failed to retrieve MethodHandle for setter {} on class {}. "
150-
+ "Will attempt to make field accessible. "
151-
+ "Please consider adding public setter.", setterName,
152-
clazzSimpleName, ex);
153-
}
154-
155-
try {
156-
Method setterMethod = clazz.getDeclaredMethod(setterName, Map.class);
157-
MethodHandle handle = lookupToUse.unreflect(setterMethod);
158-
159-
logger.verbose("Using unreflected MethodHandle for setter {} on class {}.", setterName,
160-
clazzSimpleName);
161-
162-
return handle;
163-
} catch (ReflectiveOperationException ex) {
164-
logger.verbose("Failed to unreflect MethodHandle for setter {} on class {}."
165-
+ "Will attempt to make field accessible. "
166-
+ "Please consider adding public setter.", setterName, clazzSimpleName, ex);
128+
return reflectiveInvoker;
129+
} catch (Exception ex) {
130+
logger.log(LogLevel.VERBOSE, () ->
131+
"Failed to retrieve invoker for setter " + setterName + " on class " + clazzSimpleName
132+
+ ". Will attempt to make field accessible. Please consider adding public setter.", ex);
167133
}
168134

169135
// In a previous implementation compute returned null here in an attempt to indicate that there is no setter
@@ -174,7 +140,7 @@ private static MethodHandle getFromCache(Field key, Class<?> clazz, String clazz
174140
// Now the implementation returns a dummy constant when there is no setter for the field. This now results
175141
// in this case properly inserting into the cache and only running when a new type is seen or the cache is
176142
// cleared due to reaching capacity.
177-
return NO_SETTER_HANDLE;
143+
return NO_SETTER_REFLECTIVE_INVOKER;
178144
});
179145
}
180146
}

‎sdk/core/azure-core-serializer-json-jackson/src/main/java/com/azure/core/serializer/json/jackson/implementation/JacksonDatabind215.java‎

Lines changed: 27 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,12 @@
33

44
package com.azure.core.serializer.json.jackson.implementation;
55

6+
import com.azure.core.implementation.ReflectiveInvoker;
7+
import com.azure.core.implementation.ReflectionUtils;
68
import com.azure.core.util.logging.ClientLogger;
79
import com.fasterxml.jackson.core.JsonFactory;
810
import com.fasterxml.jackson.databind.ObjectMapper;
911

10-
import java.lang.invoke.MethodHandle;
11-
import java.lang.invoke.MethodHandles;
12-
1312
/**
1413
* Utility methods for Jackson Databind types when it's known that the version is 2.15+.
1514
*/
@@ -18,39 +17,38 @@ final class JacksonDatabind215 {
1817
private static final String STREAM_READ_CONSTRAINTS = "com.fasterxml.jackson.core.StreamReadConstraints";
1918
private static final String STREAM_READ_CONSTRAINTS_BUILDER = STREAM_READ_CONSTRAINTS + "$Builder";
2019

21-
private static final MethodHandle CREATE_STREAM_READ_CONSTRAINTS_BUILDER;
22-
private static final MethodHandle SET_MAX_STRING_LENGTH;
23-
private static final MethodHandle BUILD_STREAM_READ_CONSTRAINTS;
24-
private static final MethodHandle SET_STREAM_READ_CONSTRAINTS;
20+
private static final ReflectiveInvoker CREATE_STREAM_READ_CONSTRAINTS_BUILDER;
21+
private static final ReflectiveInvoker SET_MAX_STRING_LENGTH;
22+
private static final ReflectiveInvoker BUILD_STREAM_READ_CONSTRAINTS;
23+
private static final ReflectiveInvoker SET_STREAM_READ_CONSTRAINTS;
2524

2625
private static final boolean USE_JACKSON_215;
2726

2827
static {
29-
MethodHandles.Lookup publicLookup = MethodHandles.publicLookup();
3028
ClassLoader thisClassLoader = JacksonDatabind215.class.getClassLoader();
3129

32-
MethodHandle createStreamReadConstraintsBuilder = null;
33-
MethodHandle setMaxStringLength = null;
34-
MethodHandle buildStreamReadConstraints = null;
35-
MethodHandle setStreamReadConstraints = null;
30+
ReflectiveInvoker createStreamReadConstraintsBuilder = null;
31+
ReflectiveInvoker setMaxStringLength = null;
32+
ReflectiveInvoker buildStreamReadConstraints = null;
33+
ReflectiveInvoker setStreamReadConstraints = null;
3634
boolean useJackson215 = false;
3735
try {
3836
Class<?> streamReadConstraints = Class.forName(STREAM_READ_CONSTRAINTS, true, thisClassLoader);
3937
Class<?> streamReadConstraintsBuilder = Class.forName(STREAM_READ_CONSTRAINTS_BUILDER, true,
4038
thisClassLoader);
4139

42-
createStreamReadConstraintsBuilder = publicLookup.unreflect(streamReadConstraints
43-
.getDeclaredMethod("builder"));
44-
setMaxStringLength = publicLookup.unreflect(streamReadConstraintsBuilder
45-
.getDeclaredMethod("maxStringLength", int.class));
46-
buildStreamReadConstraints = publicLookup.unreflect(streamReadConstraintsBuilder
47-
.getDeclaredMethod("build"));
48-
setStreamReadConstraints = publicLookup.unreflect(JsonFactory.class.getDeclaredMethod(
49-
"setStreamReadConstraints", streamReadConstraints));
40+
createStreamReadConstraintsBuilder = ReflectionUtils.getMethodInvoker(streamReadConstraints,
41+
streamReadConstraints.getDeclaredMethod("builder"), false);
42+
setMaxStringLength = ReflectionUtils.getMethodInvoker(streamReadConstraintsBuilder,
43+
streamReadConstraintsBuilder.getDeclaredMethod("maxStringLength", int.class), false);
44+
buildStreamReadConstraints = ReflectionUtils.getMethodInvoker(streamReadConstraintsBuilder,
45+
streamReadConstraintsBuilder.getDeclaredMethod("build"), false);
46+
setStreamReadConstraints = ReflectionUtils.getMethodInvoker(JsonFactory.class,
47+
JsonFactory.class.getDeclaredMethod("setStreamReadConstraints", streamReadConstraints), false);
5048
useJackson215 = true;
5149
} catch (Throwable ex) {
5250
if (ex instanceof LinkageError) {
53-
LOGGER.info("Attempted to create MethodHandles for Jackson 2.15 features but failed. It's possible "
51+
LOGGER.info("Attempted to create invokers for Jackson 2.15 features but failed. It's possible "
5452
+ "that your application will run without error even with this failure. The Azure SDKs only set "
5553
+ "updated StreamReadConstraints to allow for larger payloads to be handled.");
5654
} else if (ex instanceof Error) {
@@ -79,18 +77,18 @@ static ObjectMapper mutateStreamReadConstraints(ObjectMapper objectMapper) {
7977
}
8078

8179
try {
82-
Object streamReadConstraintsBuilder = CREATE_STREAM_READ_CONSTRAINTS_BUILDER.invoke();
80+
Object streamReadConstraintsBuilder = CREATE_STREAM_READ_CONSTRAINTS_BUILDER.invokeStatic();
8381

84-
SET_MAX_STRING_LENGTH.invoke(streamReadConstraintsBuilder, 50 * 1024 * 1024);
85-
SET_STREAM_READ_CONSTRAINTS.invoke(objectMapper.tokenStreamFactory(),
86-
BUILD_STREAM_READ_CONSTRAINTS.invoke(streamReadConstraintsBuilder));
82+
SET_MAX_STRING_LENGTH.invokeWithArguments(streamReadConstraintsBuilder, 50 * 1024 * 1024);
83+
SET_STREAM_READ_CONSTRAINTS.invokeWithArguments(objectMapper.tokenStreamFactory(),
84+
BUILD_STREAM_READ_CONSTRAINTS.invokeWithArguments(streamReadConstraintsBuilder));
8785

8886
return objectMapper;
89-
} catch (Throwable throwable) {
90-
if (throwable instanceof Error) {
91-
throw (Error) throwable;
87+
} catch (Exception exception) {
88+
if (exception instanceof RuntimeException) {
89+
throw LOGGER.logExceptionAsError((RuntimeException) exception);
9290
} else {
93-
throw LOGGER.logExceptionAsError(new IllegalStateException(throwable));
91+
throw LOGGER.logExceptionAsError(new IllegalStateException(exception));
9492
}
9593
}
9694
}

‎sdk/core/azure-core-serializer-json-jackson/src/main/java/com/azure/core/serializer/json/jackson/implementation/JsonSerializableDeserializer.java‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
package com.azure.core.serializer.json.jackson.implementation;
55

6+
import com.azure.core.implementation.ReflectiveInvoker;
67
import com.azure.core.implementation.ReflectionUtils;
78
import com.azure.core.util.logging.ClientLogger;
89
import com.azure.json.JsonReader;
@@ -12,14 +13,12 @@
1213
import com.fasterxml.jackson.databind.JsonDeserializer;
1314

1415
import java.io.IOException;
15-
import java.lang.invoke.MethodHandle;
16-
import java.lang.invoke.MethodHandles;
1716

1817
public class JsonSerializableDeserializer extends JsonDeserializer<JsonSerializable<?>> {
1918
private static final ClientLogger LOGGER = new ClientLogger(JsonSerializableDeserializer.class);
2019

2120
private final Class<? extends JsonSerializable<?>> jsonSerializableType;
22-
private final MethodHandle readJson;
21+
private final ReflectiveInvoker readJson;
2322

2423
/**
2524
* Creates an instance of {@link JsonSerializableDeserializer}.
@@ -29,8 +28,8 @@ public class JsonSerializableDeserializer extends JsonDeserializer<JsonSerializa
2928
public JsonSerializableDeserializer(Class<? extends JsonSerializable<?>> jsonSerializableType) {
3029
this.jsonSerializableType = jsonSerializableType;
3130
try {
32-
MethodHandles.Lookup lookup = ReflectionUtils.getLookupToUse(jsonSerializableType);
33-
this.readJson = lookup.unreflect(jsonSerializableType.getDeclaredMethod("fromJson", JsonReader.class));
31+
this.readJson = ReflectionUtils.getMethodInvoker(jsonSerializableType,
32+
jsonSerializableType.getDeclaredMethod("fromJson", JsonReader.class));
3433
} catch (Exception e) {
3534
throw LOGGER.logExceptionAsError(new IllegalStateException(e));
3635
}
@@ -41,13 +40,11 @@ public JsonSerializable<?> deserialize(JsonParser p, DeserializationContext ctxt
4140
try {
4241
return jsonSerializableType.cast(readJson.invokeWithArguments(
4342
new JacksonJsonReader(p, null, null, false, null)));
44-
} catch (Throwable e) {
45-
if (e instanceof IOException) {
46-
throw (IOException) e;
47-
} else if (e instanceof Exception) {
48-
throw new IOException(e);
43+
} catch (Exception exception) {
44+
if (exception instanceof IOException) {
45+
throw (IOException) exception;
4946
} else {
50-
throw (Error) e;
47+
throw new IOException(exception);
5148
}
5249
}
5350
}

0 commit comments

Comments
 (0)