Skip to content

Commit

Permalink
Use ObjectMapperShim in all core packages, deprecate direct ObjectMap…
Browse files Browse the repository at this point in the history 
…per usage (#24138)

* Use ObjectMapperShim in all core packages, deprecate direct ObjectMapper usage
  • Loading branch information
@lmolkova
lmolkova authored Sep 21, 2021
1 parent 3487515 commit e68f02a
Show file tree
Hide file tree
Showing 26 changed files with 579 additions and 237 deletions.
4 changes: 4 additions & 0 deletions eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -626,4 +626,8 @@ the main ServiceBusClientBuilder. -->
<!-- Checkstyle suppressions to keep HttpPipelinePolicy in implementation folder. -->
<suppress checks="com.azure.tools.checkstyle.checks.HttpPipelinePolicyCheck"
files="com.azure.communication.callingserver.implementation.RedirectPolicy.java"/>

<!-- Suppress VisibilityModifier in MemberConverterImplTests.-->
<suppress checks="VisibilityModifier"
files="com.azure.core.implementation.jackson.MemberNameConverterImplTests.java"/>
</suppressions>
7 changes: 7 additions & 0 deletions eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,13 @@
<Bug pattern="PZLA_PREFER_ZERO_LENGTH_ARRAYS"/>
</Match>

<!--False positive: https://github.com/spotbugs/spotbugs/issues/1219-->
<Match>
<Class name="com.azure.core.implementation.jackson.ObjectMapperShim"/>
<Method name="createJsonMapper"/>
<Bug pattern="BC_UNCONFIRMED_CAST_OF_RETURN_VALUE"/>
</Match>

<!-- Suppress non-null warning in the case that we change the code and it is possible for
KeyVaultCredentials.getAuthenticationCredentials to return null. -->
<Match>
Expand Down
3 changes: 3 additions & 0 deletions sdk/agrifood/azure-verticals-agrifood-farming/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
<properties>
<!-- Anomaly detector temporarily skipping code coverage until tests are added -->
<jacoco.skip.coverage.check>true</jacoco.skip.coverage.check>
<javaModulesSurefireArgLine>
--add-exports com.azure.core/com.azure.core.implementation.jackson=ALL-UNNAMED
</javaModulesSurefireArgLine>
</properties>

<dependencies>
Expand Down
3 changes: 3 additions & 0 deletions sdk/confidentialledger/azure-security-confidentialledger/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
<properties>
<!-- Anomaly detector temporarily skipping code coverage until tests are added -->
<jacoco.skip.coverage.check>true</jacoco.skip.coverage.check>
<javaModulesSurefireArgLine>
--add-exports com.azure.core/com.azure.core.implementation.jackson=ALL-UNNAMED
</javaModulesSurefireArgLine>
</properties>

<dependencies>
Expand Down
3 changes: 1 addition & 2 deletions ...rc/main/java/com/azure/core/management/implementation/serializer/AzureJacksonAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ public final class AzureJacksonAdapter extends JacksonAdapter {
* Creates an instance of the Azure flavored Jackson adapter.
*/
public AzureJacksonAdapter() {
super();
serializer().registerModule(ManagementErrorDeserializer.getModule(simpleMapper()));
super((outerMapper, innerMapper) -> outerMapper.registerModule(ManagementErrorDeserializer.getModule(innerMapper)));
}
}
12 changes: 12 additions & 0 deletions ...anagement/src/test/java/com/azure/core/management/exception/ManagementExceptionTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,18 @@ public void testSubclassDeserialization() throws IOException {
Assertions.assertEquals("ResourceGroupNotFound", managementError.getCode());
}

@Test
public void testCaseInsensitiveSubclassDeserialization() throws IOException {
final String errorBody = "{\"error\":{\"Code\":\"WepAppError\",\"MESSAGE\":\"Web app error.\",\"Details\":[{\"code\":\"e\"}],\"TaRgeT\":\"foo\"}}";

SerializerAdapter serializerAdapter = SerializerFactory.createDefaultManagementSerializerAdapter();
WebError webError = serializerAdapter.deserialize(errorBody, WebError.class, SerializerEncoding.JSON);
Assertions.assertEquals("WepAppError", webError.getCode());
Assertions.assertEquals("Web app error.", webError.getMessage());
Assertions.assertEquals(1, webError.getDetails().size());
Assertions.assertEquals("foo", webError.getTarget());
}

@Test
public void testDeserializationInResource() throws IOException {
final String virtualMachineJson = "{\"properties\":{\"instanceView\":{\"patchStatus\":{\"availablePatchSummary\":{\"error\":{}}}}}}";
Expand Down
189 changes: 6 additions & 183 deletions ...n-jackson/src/main/java/com/azure/core/serializer/json/jackson/JacksonJsonSerializer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,96 +3,35 @@

package com.azure.core.serializer.json.jackson;

import com.azure.core.util.CoreUtils;
import com.azure.core.implementation.jackson.ObjectMapperShim;
import com.azure.core.util.logging.ClientLogger;
import com.azure.core.util.serializer.JsonSerializer;
import com.azure.core.util.serializer.MemberNameConverter;
import com.azure.core.util.serializer.TypeReference;
import com.fasterxml.jackson.annotation.JsonGetter;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.cfg.MapperConfig;
import com.fasterxml.jackson.databind.introspect.AnnotatedClass;
import com.fasterxml.jackson.databind.introspect.AnnotatedClassResolver;
import com.fasterxml.jackson.databind.introspect.AnnotatedMethod;
import com.fasterxml.jackson.databind.introspect.VisibilityChecker;
import com.fasterxml.jackson.databind.type.TypeFactory;
import com.fasterxml.jackson.databind.util.BeanUtil;
import reactor.core.publisher.Mono;

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.UncheckedIOException;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.lang.reflect.Field;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;

/**
* Jackson based implementation of the {@link JsonSerializer} and {@link MemberNameConverter} interfaces.
*/
public final class JacksonJsonSerializer implements JsonSerializer, MemberNameConverter {
private static final String ACCESSOR_NAMING_STRATEGY =
"com.fasterxml.jackson.databind.introspect.AccessorNamingStrategy";
private static final String ACCESSOR_NAMING_STRATEGY_PROVIDER = ACCESSOR_NAMING_STRATEGY + ".Provider";
private static final MethodHandle GET_ACCESSOR_NAMING;
private static final MethodHandle FOR_POJO;
private static final MethodHandle FIND_NAME_FOR_IS_GETTER;
private static final MethodHandle FIND_NAME_FOR_REGULAR_GETTER;
private static final boolean USE_REFLECTION_FOR_MEMBER_NAME;

static {
MethodHandles.Lookup publicLookup = MethodHandles.publicLookup();

MethodHandle getAccessorNaming = null;
MethodHandle forPojo = null;
MethodHandle findNameForIsGetter = null;
MethodHandle findNameForRegularGetter = null;
boolean useReflectionForMemberName = false;

try {
Class<?> accessorNamingStrategyProviderClass = Class.forName(ACCESSOR_NAMING_STRATEGY_PROVIDER);
Class<?> accessorNamingStrategyClass = Class.forName(ACCESSOR_NAMING_STRATEGY);
getAccessorNaming = publicLookup.findVirtual(MapperConfig.class, "getAccessorNaming",
MethodType.methodType(accessorNamingStrategyProviderClass));
forPojo = publicLookup.findVirtual(accessorNamingStrategyProviderClass, "forPOJO",
MethodType.methodType(accessorNamingStrategyClass, MapperConfig.class, AnnotatedClass.class));
findNameForIsGetter = publicLookup.findVirtual(accessorNamingStrategyClass, "findNameForIsGetter",
MethodType.methodType(String.class, AnnotatedMethod.class, String.class));
findNameForRegularGetter = publicLookup.findVirtual(accessorNamingStrategyClass, "findNameForRegularGetter",
MethodType.methodType(String.class, AnnotatedMethod.class, String.class));
useReflectionForMemberName = true;
} catch (Throwable ex) {
new ClientLogger(JacksonJsonSerializer.class)
.verbose("Failed to retrieve MethodHandles used to get naming strategy. Falling back to BeanUtils.",
ex);
}

GET_ACCESSOR_NAMING = getAccessorNaming;
FOR_POJO = forPojo;
FIND_NAME_FOR_IS_GETTER = findNameForIsGetter;
FIND_NAME_FOR_REGULAR_GETTER = findNameForRegularGetter;
USE_REFLECTION_FOR_MEMBER_NAME = useReflectionForMemberName;
}

private final ClientLogger logger = new ClientLogger(JacksonJsonSerializer.class);

private final ObjectMapper mapper;
private final TypeFactory typeFactory;
private final ObjectMapperShim mapper;

/**
* Constructs a {@link JsonSerializer} using the passed Jackson serializer.
*
* @param mapper Configured Jackson serializer.
*/
JacksonJsonSerializer(ObjectMapper mapper) {
JacksonJsonSerializer(ObjectMapperShim mapper) {
this.mapper = mapper;
this.typeFactory = mapper.getTypeFactory();
}

@Override
Expand All @@ -102,7 +41,7 @@ public <T> T deserializeFromBytes(byte[] data, TypeReference<T> typeReference) {
}

try {
return mapper.readValue(data, typeFactory.constructType(typeReference.getJavaType()));
return mapper.readValue(data, typeReference.getJavaType());
} catch (IOException ex) {
throw logger.logExceptionAsError(new UncheckedIOException(ex));
}
Expand All @@ -115,7 +54,7 @@ public <T> T deserialize(InputStream stream, TypeReference<T> typeReference) {
}

try {
return mapper.readValue(stream, typeFactory.constructType(typeReference.getJavaType()));
return mapper.readValue(stream, typeReference.getJavaType());
} catch (IOException ex) {
throw logger.logExceptionAsError(new UncheckedIOException(ex));
}
Expand Down Expand Up @@ -159,124 +98,8 @@ public Mono<Void> serializeAsync(OutputStream stream, Object value) {
return Mono.fromRunnable(() -> serialize(stream, value));
}


@Override
public String convertMemberName(Member member) {
if (Modifier.isTransient(member.getModifiers())) {
return null;
}

VisibilityChecker<?> visibilityChecker = mapper.getVisibilityChecker();
if (member instanceof Field) {
Field f = (Field) member;

if (f.isAnnotationPresent(JsonIgnore.class) || !visibilityChecker.isFieldVisible(f)) {
if (f.isAnnotationPresent(JsonProperty.class)) {
logger.info("Field {} is annotated with JsonProperty but isn't accessible to "
+ "JacksonJsonSerializer.", f.getName());
}
return null;
}

if (f.isAnnotationPresent(JsonProperty.class)) {
String propertyName = f.getDeclaredAnnotation(JsonProperty.class).value();
return CoreUtils.isNullOrEmpty(propertyName) ? f.getName() : propertyName;
}

return f.getName();
}

if (member instanceof Method) {
Method m = (Method) member;

/*
* If the method isn't a getter, is annotated with JsonIgnore, or isn't visible to the ObjectMapper ignore
* it.
*/
if (!verifyGetter(m)
|| m.isAnnotationPresent(JsonIgnore.class)
|| !visibilityChecker.isGetterVisible(m)) {
if (m.isAnnotationPresent(JsonGetter.class) || m.isAnnotationPresent(JsonProperty.class)) {
logger.info("Method {} is annotated with either JsonGetter or JsonProperty but isn't accessible "
+ "to JacksonJsonSerializer.", m.getName());
}
return null;
}

String methodNameWithoutJavaBeans = removePrefix(m);

/*
* Prefer JsonGetter over JsonProperty as it is the more targeted annotation.
*/
if (m.isAnnotationPresent(JsonGetter.class)) {
String propertyName = m.getDeclaredAnnotation(JsonGetter.class).value();
return CoreUtils.isNullOrEmpty(propertyName) ? methodNameWithoutJavaBeans : propertyName;
}

if (m.isAnnotationPresent(JsonProperty.class)) {
String propertyName = m.getDeclaredAnnotation(JsonProperty.class).value();
return CoreUtils.isNullOrEmpty(propertyName) ? methodNameWithoutJavaBeans : propertyName;
}

// If no annotation is present default to the inferred name.
return methodNameWithoutJavaBeans;
}

return null;
}

/*
* Only consider methods that don't have parameters and aren't void as valid getter methods.
*/
private static boolean verifyGetter(Method method) {
Class<?> returnType = method.getReturnType();

return method.getParameterCount() == 0
&& returnType != void.class
&& returnType != Void.class;
}

private String removePrefix(Method method) {
MapperConfig<?> config = mapper.getSerializationConfig();

AnnotatedClass annotatedClass = AnnotatedClassResolver.resolve(config,
mapper.constructType(method.getDeclaringClass()), null);

AnnotatedMethod annotatedMethod = new AnnotatedMethod(null, method, null, null);
String annotatedMethodName = annotatedMethod.getName();

String name = null;
if (USE_REFLECTION_FOR_MEMBER_NAME) {
name = removePrefixWithReflection(config, annotatedClass, annotatedMethod, annotatedMethodName, logger);
}

if (name == null) {
name = removePrefixWithBeanUtils(annotatedMethod);
}

return name;
}

private static String removePrefixWithReflection(MapperConfig<?> config, AnnotatedClass annotatedClass,
AnnotatedMethod method, String methodName, ClientLogger logger) {
try {
Object accessorNamingStrategy = FOR_POJO.invoke(GET_ACCESSOR_NAMING.invoke(config), config, annotatedClass);


String name = (String) FIND_NAME_FOR_IS_GETTER.invoke(accessorNamingStrategy, method, methodName);
if (name == null) {
name = (String) FIND_NAME_FOR_REGULAR_GETTER.invoke(accessorNamingStrategy, method, methodName);
}

return name;
} catch (Throwable ex) {
logger.verbose("Failed to find member name with AccessorNamingStrategy, returning null.", ex);
return null;
}
}

@SuppressWarnings("deprecation")
private static String removePrefixWithBeanUtils(AnnotatedMethod annotatedMethod) {
return BeanUtil.okNameForGetter(annotatedMethod, false);
return mapper.convertMemberName(member);
}
}
13 changes: 8 additions & 5 deletions ...on/src/main/java/com/azure/core/serializer/json/jackson/JacksonJsonSerializerBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

package com.azure.core.serializer.json.jackson;

import com.azure.core.implementation.jackson.ObjectMapperShim;
import com.azure.core.util.serializer.JacksonAdapter;
import com.fasterxml.jackson.annotation.JsonAutoDetect;
import com.fasterxml.jackson.annotation.JsonInclude;
Expand All @@ -17,11 +18,13 @@ public final class JacksonJsonSerializerBuilder {
* Jackson uses. This configuration is reset here by mutating the inclusion scope and null handling to use the
* default Jackson values so that JacksonJsonSerializer has less friction when this default is used.
*/
private static final ObjectMapper DEFAULT_MAPPER = new JacksonAdapter().serializer()
.setSerializationInclusion(JsonInclude.Include.USE_DEFAULTS)
.setDefaultVisibility(JsonAutoDetect.Value.defaultVisibility());
private static final ObjectMapperShim DEFAULT_MAPPER = ObjectMapperShim
.createJsonMapper(ObjectMapperShim.createSimpleMapper(),
(mapper, innerMapper) -> mapper
.setSerializationInclusion(JsonInclude.Include.USE_DEFAULTS)
.setDefaultVisibility(JsonAutoDetect.Value.defaultVisibility()));

private ObjectMapper objectMapper;
private ObjectMapperShim objectMapper;

/**
* Constructs a new instance of {@link JacksonJsonSerializer} with the configurations set in this builder.
Expand All @@ -44,7 +47,7 @@ public JacksonJsonSerializer build() {
* @return The updated JacksonJsonSerializerBuilder class.
*/
public JacksonJsonSerializerBuilder serializer(ObjectMapper objectMapper) {
this.objectMapper = objectMapper;
this.objectMapper = new ObjectMapperShim(objectMapper);
return this;
}
}
3 changes: 2 additions & 1 deletion ...src/test/java/com/azure/core/serializer/json/jackson/JacksonMemberNameConverterTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

package com.azure.core.serializer.json.jackson;

import com.azure.core.implementation.jackson.ObjectMapperShim;
import com.azure.core.util.serializer.MemberNameConverter;
import com.fasterxml.jackson.annotation.JsonAutoDetect;
import com.fasterxml.jackson.annotation.JsonIgnore;
Expand Down Expand Up @@ -284,7 +285,7 @@ public <T> void classConversion(T object, JacksonJsonSerializer converter, Set<S
return null;
});

ObjectNode objectNode = ((ObjectMapper) field.get(converter)).valueToTree(object);
ObjectNode objectNode = ((ObjectMapperShim) field.get(converter)).valueToTree(object);

for (String name : actual) {
assertTrue(objectNode.has(name));
Expand Down
1 change: 1 addition & 0 deletions sdk/core/azure-core/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@
<javaModulesSurefireArgLine>
--add-exports com.azure.core/com.azure.core.implementation.http=ALL-UNNAMED
--add-exports com.azure.core/com.azure.core.implementation.serializer=ALL-UNNAMED
--add-exports com.azure.core/com.azure.core.implementation.jackson=ALL-UNNAMED

--add-opens com.azure.core/com.azure.core=ALL-UNNAMED
--add-opens com.azure.core/com.azure.core.credential=ALL-UNNAMED
Expand Down
Loading

0 comments on commit e68f02a

Please sign in to comment.