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

Avoid creating object reference cycles in merge, clean up Sonar issues #1402

Merged
merged 1 commit into from
Mar 14, 2023
Merged
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
280 changes: 148 additions & 132 deletions core/src/main/java/io/smallrye/openapi/api/util/MergeUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@
import java.beans.PropertyDescriptor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.AbstractMap;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;

Expand All @@ -25,6 +28,7 @@
import org.eclipse.microprofile.openapi.models.tags.Tag;

import io.smallrye.openapi.api.models.ModelImpl;
import io.smallrye.openapi.runtime.OpenApiRuntimeException;

/**
* Used to merge OAI data models into a single one. The MP+OAI 1.0 spec
Expand Down Expand Up @@ -57,6 +61,14 @@ public static final OpenAPI merge(OpenAPI document1, OpenAPI document2) {
return mergeObjects(document1, document2);
}

static <T, P> boolean cycleDetected(String propertyName, T obj1, P prop1, T obj2, P prop2) {
if (prop1 == obj2 || prop2 == obj1) {
UtilLogging.logger.cylicReferenceAvoided(propertyName, obj1.getClass().getName());
return true;
}
return false;
}

/**
* Generic merge of two objects of the same type.
*
Expand All @@ -65,80 +77,73 @@ public static final OpenAPI merge(OpenAPI document1, OpenAPI document2) {
* @param <T> Type parameter
* @return Merged object
*/
@SuppressWarnings({ "rawtypes" })
public static <T> T mergeObjects(T object1, T object2) {
if (object1 == null && object2 != null) {
if (object1 == object2) {
return object1;
}
if (object1 == null) {
return object2;
}
if (object1 != null && object2 == null) {
if (object2 == null) {
return object1;
}
if (object1 == null && object2 == null) {
return null;
}

// It's uncommon, but in some cases (like Link Parameters or Examples) the values could
// be different types. In this case, just take the 2nd one (the override).
if (!object1.getClass().equals(object2.getClass())) {
return object2;
}

PropertyDescriptor[] descriptors = new PropertyDescriptor[0];
try {
descriptors = Introspector.getBeanInfo(object1.getClass()).getPropertyDescriptors();
Arrays.stream(Introspector.getBeanInfo(object1.getClass()).getPropertyDescriptors())
.filter(descriptor -> !EXCLUDED_PROPERTIES.contains(descriptor.getName()))
.filter(descriptor -> Objects.nonNull(descriptor.getWriteMethod()))
.forEach(descriptor -> {
try {
mergeProperty(object1, object2, descriptor);
} catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
throw new OpenApiRuntimeException(e);
}
});
} catch (IntrospectionException e) {
UtilLogging.logger.failedToIntrospectBeanInfo(object1.getClass(), e);
}

for (PropertyDescriptor descriptor : descriptors) {
if (EXCLUDED_PROPERTIES.contains(descriptor.getName())) {
continue;
}
Class ptype = descriptor.getPropertyType();
Method writeMethod = descriptor.getWriteMethod();
if (writeMethod != null) {
if (Constructible.class.isAssignableFrom(ptype)) {
try {
Object val1 = descriptor.getReadMethod().invoke(object1);
Object val2 = descriptor.getReadMethod().invoke(object2);
Object newValue = mergeObjects(val1, val2);
if (newValue != null) {
writeMethod.invoke(object1, newValue);
}
} catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
throw new RuntimeException(e);
}
} else if (Map.class.isAssignableFrom(ptype)) {
try {
Map values1 = (Map) descriptor.getReadMethod().invoke(object1);
Map values2 = (Map) descriptor.getReadMethod().invoke(object2);
Map newValues = mergeMaps(values1, values2);
writeMethod.invoke(object1, newValues);
} catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
throw new RuntimeException(e);
}
} else if (List.class.isAssignableFrom(ptype)) {
try {
List values1 = (List) descriptor.getReadMethod().invoke(object1);
List values2 = (List) descriptor.getReadMethod().invoke(object2);
List newValues = mergeLists(values1, values2).orElse(null);
writeMethod.invoke(object1, newValues);
} catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
throw new RuntimeException(e);
}
} else {
try {
Object newValue = descriptor.getReadMethod().invoke(object2);
if (newValue != null) {
writeMethod.invoke(object1, newValue);
}
} catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
throw new RuntimeException(e);
}
return object1;
}

@SuppressWarnings({ "rawtypes" })
static <T> void mergeProperty(T object1, T object2, PropertyDescriptor descriptor)
throws IllegalAccessException, IllegalArgumentException, InvocationTargetException {

Class<?> ptype = descriptor.getPropertyType();
Method writeMethod = descriptor.getWriteMethod();

if (Constructible.class.isAssignableFrom(ptype)) {
Object val1 = descriptor.getReadMethod().invoke(object1);
Object val2 = descriptor.getReadMethod().invoke(object2);
if (!cycleDetected(descriptor.getName(), object1, val1, object2, val2)) {
Object newValue = mergeObjects(val1, val2);
if (newValue != null) {
writeMethod.invoke(object1, newValue);
}
}
} else if (Map.class.isAssignableFrom(ptype)) {
Map values1 = (Map) descriptor.getReadMethod().invoke(object1);
Map values2 = (Map) descriptor.getReadMethod().invoke(object2);
Map newValues = mergeMaps(values1, values2);
writeMethod.invoke(object1, newValues);
} else if (List.class.isAssignableFrom(ptype)) {
List values1 = (List) descriptor.getReadMethod().invoke(object1);
List values2 = (List) descriptor.getReadMethod().invoke(object2);
List newValues = mergeLists(values1, values2);
writeMethod.invoke(object1, newValues);
} else {
Object newValue = descriptor.getReadMethod().invoke(object2);
if (newValue != null) {
writeMethod.invoke(object1, newValue);
}
}
return object1;
}

/**
Expand All @@ -150,15 +155,15 @@ public static <T> T mergeObjects(T object1, T object2) {
*/
@SuppressWarnings({ "rawtypes", "unchecked" })
private static Map mergeMaps(Map values1, Map values2) {
if (values1 == null && values2 == null) {
return null;
}
if (values1 != null && values2 == null) {
if (values1 == values2) {
return values1;
}
if (values1 == null && values2 != null) {
if (values1 == null) {
return values2;
}
if (values2 == null) {
return values1;
}

if (!(values1 instanceof ModelImpl)) {
values1 = new LinkedHashMap<>(values1);
Expand All @@ -167,48 +172,65 @@ private static Map mergeMaps(Map values1, Map values2) {
values2 = new LinkedHashMap<>(values2);
}

for (Object key : values2.keySet()) {
if (values1.containsKey(key)) {
Object pval1 = values1.get(key);
Object pval2 = values2.get(key);
if (pval1 instanceof Map) {
values1.put(key, mergeMaps((Map) pval1, (Map) pval2));
} else if (pval1 instanceof List) {
values1.put(key, mergeLists((List) pval1, (List) pval2).orElse(null));
} else if (pval1 instanceof Constructible) {
values1.put(key, mergeObjects(pval1, pval2));
} else {
values1.put(key, pval2);
}
} else {
Object pval2 = values2.get(key);
values1.put(key, pval2);
}
}
Map<Object, Object> targetValues = values1;
Set<Map.Entry<?, ?>> entrySet = values2.entrySet();

entrySet.stream()
.map(entry -> {
Object key = entry.getKey();
Object pval2 = entry.getValue();
Object value;

if (targetValues.containsKey(key)) {
Object pval1 = targetValues.get(key);

if (pval1 instanceof Map) {
value = mergeMaps((Map) pval1, (Map) pval2);
} else if (pval1 instanceof List) {
value = mergeLists((List) pval1, (List) pval2);
} else if (pval1 instanceof Constructible) {
value = mergeObjects(pval1, pval2);
} else {
value = pval2;
}
} else {
value = pval2;
}

return new AbstractMap.SimpleEntry<>(key, value);
})
.forEach(modifiedEntry -> targetValues.put(modifiedEntry.getKey(), modifiedEntry.getValue()));

if (values1 instanceof Constructible) {
if (values1 instanceof Reference) {
Reference ref1 = (Reference) values1;
Reference ref2 = (Reference) values2;
if (ref2.getRef() != null) {
ref1.setRef(ref2.getRef());
}
}
if (values1 instanceof Extensible) {
Extensible extensible1 = (Extensible) values1;
Extensible extensible2 = (Extensible) values2;
extensible1.setExtensions(mergeMaps(extensible1.getExtensions(), extensible2.getExtensions()));
}
if (values1 instanceof APIResponses) {
APIResponses responses1 = (APIResponses) values1;
APIResponses responses2 = (APIResponses) values2;
responses1.defaultValue(mergeObjects(responses1.getDefaultValue(), responses2.getDefaultValue()));
}
mergeConstructible(values1, values2);
}

return values1;
}

@SuppressWarnings({ "rawtypes", "unchecked" })
static void mergeConstructible(Map values1, Map values2) {
if (values1 instanceof Reference) {
Reference ref1 = (Reference) values1;
Reference ref2 = (Reference) values2;
if (ref2.getRef() != null) {
ref1.setRef(ref2.getRef());
}
}

if (values1 instanceof Extensible) {
Extensible extensible1 = (Extensible) values1;
Extensible extensible2 = (Extensible) values2;
extensible1.setExtensions(mergeMaps(extensible1.getExtensions(), extensible2.getExtensions()));
}

if (values1 instanceof APIResponses) {
APIResponses responses1 = (APIResponses) values1;
APIResponses responses2 = (APIResponses) values2;
responses1.defaultValue(mergeObjects(responses1.getDefaultValue(), responses2.getDefaultValue()));
}
}

/**
* Merges two Lists. Any values missing from List1 but present in List2 will be added. Depending on
* the type of list, further processing and de-duping may be required.
Expand All @@ -217,45 +239,42 @@ private static Map mergeMaps(Map values1, Map values2) {
* @param values2
*/
@SuppressWarnings({ "rawtypes", "unchecked" })
private static Optional<List> mergeLists(List values1, List values2) {
if (values1 == null && values2 == null) {
return Optional.empty();
}
if (values1 != null && values2 == null) {
return Optional.of(values1);
private static List mergeLists(List values1, List values2) {
if (Objects.equals(values1, values2)) {
// Do not merge if lists identical, both null, or both the same reference
return values1;
}
if ((values1 == null || values1.isEmpty()) && values2 != null) {
return Optional.of(values2);
if (values2 == null) {
return values1;
}
if (values1.equals(values2)) {
// Do not merge identical lists
return Optional.of(values1);
if ((values1 == null || values1.isEmpty())) {
return values2;
}

if (values1.get(0) instanceof String) {
return Optional.of(mergeStringLists(values1, values2));
return mergeStringLists(values1, values2);
}

if (values1.get(0) instanceof Tag) {
return Optional.of(mergeTagLists(values1, values2));
return mergeTagLists(values1, values2);
}

if (values1.get(0) instanceof Server) {
return Optional.of(mergeServerLists(values1, values2));
return mergeServerLists(values1, values2);
}

if (values1.get(0) instanceof SecurityRequirement) {
return Optional.of(mergeSecurityRequirementLists(values1, values2));
return mergeSecurityRequirementLists(values1, values2);
}

if (values1.get(0) instanceof Parameter) {
return Optional.of(mergeParameterLists(values1, values2));
return mergeParameterLists(values1, values2);
}

List merged = new ArrayList<>(values1.size() + values2.size());
merged.addAll(values1);
merged.addAll(values2);
return Optional.of(merged);
return merged;
}

/**
Expand Down Expand Up @@ -358,27 +377,24 @@ private static List<SecurityRequirement> mergeSecurityRequirementLists(List<Secu
* @param values2
*/
private static List<Parameter> mergeParameterLists(List<Parameter> values1, List<Parameter> values2) {
values1 = new ArrayList<>(values1);

for (Parameter value2 : values2) {
Parameter match = null;
for (Parameter value1 : values1) {
if (value1.getName() == null || !value1.getName().equals(value2.getName())) {
continue;
}
if (value1.getIn() == null || !value1.getIn().equals(value2.getIn())) {
continue;
}
List<Parameter> mutableValues = new ArrayList<>(values1);

values2.stream()
.filter(v -> Objects.nonNull(v.getName()))
.filter(v -> Objects.nonNull(v.getIn()))
.forEach(value2 -> {
Optional<Parameter> match = mutableValues.stream()
.filter(value1 -> Objects.equals(value1.getName(), value2.getName()))
.filter(value1 -> Objects.equals(value1.getIn(), value2.getIn()))
.findFirst();

if (match.isPresent()) {
mergeObjects(match.get(), value2);
} else {
mutableValues.add(value2);
}
});

match = value1;
break;
}
if (match == null) {
values1.add(value2);
} else {
mergeObjects(match, value2);
}
}
return values1;
return mutableValues;
}
}
Loading