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

CB-16465 Lazy load safe toString() methods #12485

Closed
wants to merge 1 commit 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
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,16 @@ public void setAlertCrn(String alertCrn) {
}

public abstract Map<String, String> getTelemetryParameters();

@Override
public String toString() {
return getClass().getSimpleName() + "{" +
"id=" + id +
", cluster=" + cluster +
", name='" + name + '\'' +
", description='" + description + '\'' +
", alertCrn='" + alertCrn + '\'' +
", scalingPolicy=" + scalingPolicy +
'}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,17 @@ public UpdateFailedDetails getUpdateFailedDetails() {
public void setUpdateFailedDetails(UpdateFailedDetails updateFailedDetails) {
this.updateFailedDetails = updateFailedDetails;
}

@Override
public String toString() {
return "Cluster{" +
"id=" + id +
", stackCrn='" + stackCrn + '\'' +
", stackName='" + stackName + '\'' +
", cloudPlatform='" + cloudPlatform + '\'' +
", stackType=" + stackType +
'}';
}
}


Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,14 @@ public ClusterManagerVariant getVariant() {
public void setVariant(ClusterManagerVariant variant) {
this.variant = variant;
}

@Override
public String toString() {
return "ClusterManager{" +
"id=" + id +
", host='" + host + '\'' +
", port='" + port + '\'' +
", variant=" + variant +
'}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,15 @@ public String getUserCrn() {
public void setUserCrn(String userCrn) {
this.userCrn = userCrn;
}

@Override
public String toString() {
return "ClusterPertain{" +
"id=" + id +
", tenant='" + tenant + '\'' +
", workspaceId=" + workspaceId +
", userId='" + userId + '\'' +
", userCrn='" + userCrn + '\'' +
'}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -218,4 +218,13 @@ public String getHostGroup() {
public void setHostGroup(String hostGroup) {
this.hostGroup = hostGroup;
}

@Override
public String toString() {
return "History{" +
"id=" + id +
", clusterId=" + clusterId +
", stackCrn='" + stackCrn + '\'' +
'}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,16 @@ public long getAlertId() {
public Cluster getCluster() {
return alert.getCluster();
}

@Override
public String toString() {
return "ScalingPolicy{" +
"id=" + id +
", name='" + name + '\'' +
", adjustmentType=" + adjustmentType +
", scalingAdjustment=" + scalingAdjustment +
", alert=" + alert +
", hostGroup='" + hostGroup + '\'' +
'}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import javax.persistence.OneToOne;
import javax.persistence.SequenceGenerator;

import com.sequenceiq.cloudbreak.util.DatabaseUtil;

@Entity
@NamedQueries(@NamedQuery(name = "SecurityConfig.findByClusterId", query = "SELECT s FROM SecurityConfig s WHERE s.cluster.id= :id"))
public class SecurityConfig implements Clustered {
Expand Down Expand Up @@ -84,4 +86,12 @@ public void update(SecurityConfig updatedConfig) {
clientKey = updatedConfig.clientKey;
serverCert = updatedConfig.serverCert;
}

@Override
public String toString() {
return "SecurityConfig{" +
"id=" + id +
", cluster=" + DatabaseUtil.lazyLoadSafeToString(cluster, Cluster::getId) +
'}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,13 @@ public String getEndpoint() {
public void setEndpoint(String endpoint) {
this.endpoint = endpoint;
}

@Override
public String toString() {
return "Subscription{" +
"id=" + id +
", clientId='" + clientId + '\'' +
", endpoint='" + endpoint + '\'' +
'}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,13 @@ public Map<String, String> getTelemetryParameters() {
parameters.put(ADJUSTMENT_TYPE, "" + getScalingPolicy().getAdjustmentType());
return parameters;
}

@Override
public String toString() {
return "TimeAlert{" +
"cluster=" + cluster +
", timeZone='" + timeZone + '\'' +
", cron='" + cron + '\'' +
'}';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.sequenceiq.periscope;

import com.sequenceiq.cloudbreak.AbstractEntityToStringTest;

class PeriscopeEntityToStringTest extends AbstractEntityToStringTest {

}
2 changes: 1 addition & 1 deletion common/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ dependencies {
implementation group: 'net.sf.json-lib', name: 'json-lib', version: '2.4', classifier: 'jdk15'
api group: 'net.jcip', name: 'jcip-annotations', version: '1.0'
api group: 'com.github.spotbugs', name: 'spotbugs-annotations', version: '4.4.2'
api group: 'org.reflections', name: 'reflections', version: '0.9.11'
api group: 'org.reflections', name: 'reflections', version: reflectionsVersion

implementation group: 'com.github.ben-manes.caffeine', name: 'caffeine', version: caffeineVersion
implementation group: 'org.glassfish.jersey.core', name: 'jersey-client', version: jerseyCoreVersion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ public class AnonymizerUtil {
private AnonymizerUtil() {
}

public static String anonymize(Object object) {
return object == null ? null : anonymize(object.toString());
}

public static String anonymize(String content) {
String ret = content;
if (ret != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,18 @@
import java.sql.Connection;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.function.Function;
import java.util.function.Supplier;

import org.hibernate.Hibernate;
import org.postgresql.Driver;
import org.springframework.jdbc.datasource.SimpleDriverDataSource;

public class DatabaseUtil {
public static final String DEFAULT_SCHEMA_NAME = "public";

public static final String UNINITIALIZED_TO_STRING = "<uninitialized>";

private DatabaseUtil() {
}

Expand All @@ -24,4 +29,26 @@ public static void createSchemaIfNeeded(String dbType, String dbAddress, String
}
}
}

public static <T> boolean isLazyLoadInitialized(T o) {
return Hibernate.isInitialized(o);
}

public static <T> String lazyLoadSafeToString(T o) {
return lazyLoadSafeToString(o, Function.identity());
}
Bajzathd marked this conversation as resolved.
Show resolved Hide resolved

public static <T> String lazyLoadSafeToString(T o, Function<T, Object> toStringFunction) {
return lazyLoadSafeToString(o, () -> toStringFunction.apply(o));
}

public static <T> String lazyLoadSafeToString(Object o, Supplier<T> toStringSupplier) {
if (o == null) {
return null;
} else if (isLazyLoadInitialized(o)) {
return String.valueOf(toStringSupplier.get());
} else {
return UNINITIALIZED_TO_STRING;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
package com.sequenceiq.cloudbreak;

import static org.assertj.core.api.SoftAssertions.assertSoftly;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.withSettings;

import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import javax.persistence.Entity;
import javax.persistence.FetchType;
import javax.persistence.ManyToOne;
import javax.persistence.OneToMany;
import javax.persistence.OneToOne;

import org.hibernate.LazyInitializationException;
import org.hibernate.proxy.HibernateProxy;
import org.hibernate.proxy.LazyInitializer;
import org.junit.jupiter.api.Test;
import org.reflections.Reflections;
import org.reflections.scanners.SubTypesScanner;
import org.reflections.scanners.TypeAnnotationsScanner;
import org.springframework.util.ReflectionUtils;

public abstract class AbstractEntityToStringTest {

private final Set<Class<?>> entityClasses = getEntityClasses();

Set<Class<?>> getEntityClasses() {
return new Reflections(getClass().getPackageName(), new TypeAnnotationsScanner(), new SubTypesScanner())
.getTypesAnnotatedWith(Entity.class, true)
.stream()
.filter(clazz -> !Modifier.isAbstract(clazz.getModifiers()))
.collect(Collectors.toSet());
}

@Test
void testImplementedToString() {
assertSoftly(softly ->
entityClasses.forEach(entityClass -> {
Method toString = ReflectionUtils.findMethod(entityClass, "toString");
if (Object.class.equals(toString.getDeclaringClass())) {
softly.fail("Class %s does not implement toString()", entityClass.getName());
}
}));
}

@Test
void testLazyLoadSafeToString() {
assertSoftly(softly ->
entityClasses.forEach(entityClass -> {
Object entityInstance = createEntityInstance(entityClass);
setLazyLoadedFields(entityClass, entityInstance);
try {
entityInstance.toString();
} catch (UninitializedToStringException e) {
softly.fail("Lazy loaded field %s may throw %s, please wrap its call in toString() in DatabaseUtil.lazyLoadSafeToString()",
e.getField(), LazyInitializationException.class.getName());
}
}));
}

/**
* Set circular entity references, so if they are referenced in each other's toString() methods, the test will fail with a {@link StackOverflowError}
*/
@Test
void testCircularEntityReferencesInToString() {
Map<? extends Class<?>, Object> entityInstancesByClass = entityClasses.stream()
.map(this::createEntityInstance)
.collect(Collectors.toMap(Object::getClass, Function.identity()));

entityInstancesByClass.values().forEach(entity -> Stream.of(entity.getClass().getDeclaredFields())
.filter(field -> entityInstancesByClass.containsKey(field.getType()))
.forEach(field -> {
ReflectionUtils.makeAccessible(field);
ReflectionUtils.setField(field, entity, entityInstancesByClass.get(field.getType()));
}));

assertSoftly(softly -> entityInstancesByClass.values()
.forEach(entityInstance -> softly.assertThatCode(entityInstance::toString).doesNotThrowAnyException()));
}

private Object createEntityInstance(Class<?> entityClass) {
try {
return entityClass.getDeclaredConstructor().newInstance();
} catch (Exception e) {
throw new RuntimeException("Failed to verify toString() implementation of entity " + entityClass.getName(), e);
}
}

private void setLazyLoadedFields(Class<?> entityClass, Object newInstance) {
Set<Field> lazyLoadedFields = Stream.of(entityClass.getDeclaredFields())
.filter(this::isLazyLoaded)
.collect(Collectors.toSet());
lazyLoadedFields.forEach(field -> {
ReflectionUtils.makeAccessible(field);
ReflectionUtils.setField(field, newInstance, createProxyObject(field.getType(), field));
});
}

private boolean isLazyLoaded(Field field) {
return (field.isAnnotationPresent(OneToMany.class) && FetchType.LAZY.equals(field.getAnnotation(OneToMany.class).fetch()))
|| (field.isAnnotationPresent(OneToOne.class) && FetchType.LAZY.equals(field.getAnnotation(OneToOne.class).fetch()))
|| (field.isAnnotationPresent(ManyToOne.class) && FetchType.LAZY.equals(field.getAnnotation(ManyToOne.class).fetch()));
}

private <T> T createProxyObject(Class<T> type, Field field) {
LazyInitializer lazyInitializer = mock(LazyInitializer.class);
when(lazyInitializer.isUninitialized()).thenReturn(true);
T hibernateProxy = mock(type, withSettings().extraInterfaces(HibernateProxy.class));
when(((HibernateProxy) hibernateProxy).getHibernateLazyInitializer()).thenReturn(lazyInitializer);
when(hibernateProxy.toString()).thenThrow(new UninitializedToStringException(field));
return hibernateProxy;
}

protected static class UninitializedToStringException extends RuntimeException {
private final Field field;

public UninitializedToStringException(Field field) {
this.field = field;
}

public Field getField() {
return field;
}
}
}
Loading