CB-16465 Lazy load safe toString() methods

Added tests for Entity classes to make sure they implement toString() correctly.
hortonworks · Apr 11, 2022 · 60694a3 · 60694a3
1 parent cd9bc4c
commit 60694a3
Show file tree

Hide file tree

Showing 95 changed files with 1,174 additions and 27 deletions.
diff --git a/autoscale/src/main/java/com/sequenceiq/periscope/domain/BaseAlert.java b/autoscale/src/main/java/com/sequenceiq/periscope/domain/BaseAlert.java
@@ -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 +
+                '}';
+    }
 }
diff --git a/autoscale/src/main/java/com/sequenceiq/periscope/domain/Cluster.java b/autoscale/src/main/java/com/sequenceiq/periscope/domain/Cluster.java
@@ -326,6 +326,17 @@ public Boolean isStopStartScalingEnabled() {
     public void setStopStartScalingEnabled(Boolean stopStartScalingEnabled) {
         this.stopStartScalingEnabled = stopStartScalingEnabled;
     }
+
+    @Override
+    public String toString() {
+        return "Cluster{" +
+                "id=" + id +
+                ", stackCrn='" + stackCrn + '\'' +
+                ", stackName='" + stackName + '\'' +
+                ", cloudPlatform='" + cloudPlatform + '\'' +
+                ", stackType=" + stackType +
+                '}';
+    }
 }
 
 
diff --git a/autoscale/src/main/java/com/sequenceiq/periscope/domain/ClusterManager.java b/autoscale/src/main/java/com/sequenceiq/periscope/domain/ClusterManager.java
@@ -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 +
+                '}';
+    }
 }
diff --git a/autoscale/src/main/java/com/sequenceiq/periscope/domain/ClusterPertain.java b/autoscale/src/main/java/com/sequenceiq/periscope/domain/ClusterPertain.java
@@ -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 + '\'' +
+                '}';
+    }
 }
diff --git a/autoscale/src/main/java/com/sequenceiq/periscope/domain/History.java b/autoscale/src/main/java/com/sequenceiq/periscope/domain/History.java
@@ -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 + '\'' +
+                '}';
+    }
 }
diff --git a/autoscale/src/main/java/com/sequenceiq/periscope/domain/ScalingPolicy.java b/autoscale/src/main/java/com/sequenceiq/periscope/domain/ScalingPolicy.java
@@ -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 + '\'' +
+                '}';
+    }
 }
diff --git a/autoscale/src/main/java/com/sequenceiq/periscope/domain/SecurityConfig.java b/autoscale/src/main/java/com/sequenceiq/periscope/domain/SecurityConfig.java
@@ -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 {
@@ -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) +
+                '}';
+    }
 }
diff --git a/autoscale/src/main/java/com/sequenceiq/periscope/domain/Subscription.java b/autoscale/src/main/java/com/sequenceiq/periscope/domain/Subscription.java
@@ -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 + '\'' +
+                '}';
+    }
 }
diff --git a/autoscale/src/main/java/com/sequenceiq/periscope/domain/TimeAlert.java b/autoscale/src/main/java/com/sequenceiq/periscope/domain/TimeAlert.java
@@ -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 + '\'' +
+                '}';
+    }
 }
diff --git a/autoscale/src/test/java/com/sequenceiq/periscope/PeriscopeEntityToStringTest.java b/autoscale/src/test/java/com/sequenceiq/periscope/PeriscopeEntityToStringTest.java
@@ -0,0 +1,7 @@
+package com.sequenceiq.periscope;
+
+import com.sequenceiq.cloudbreak.AbstractEntityToStringTest;
+
+class PeriscopeEntityToStringTest extends AbstractEntityToStringTest {
+
+}
diff --git a/common/build.gradle b/common/build.gradle
@@ -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

diff --git a/common/src/main/java/com/sequenceiq/cloudbreak/common/anonymizer/AnonymizerUtil.java b/common/src/main/java/com/sequenceiq/cloudbreak/common/anonymizer/AnonymizerUtil.java
@@ -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) {

diff --git a/common/src/main/java/com/sequenceiq/cloudbreak/util/DatabaseUtil.java b/common/src/main/java/com/sequenceiq/cloudbreak/util/DatabaseUtil.java
@@ -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() {
     }
 
@@ -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());
+    }
+
+    public static <T> String lazyLoadSafeToString(T o, Function<T, Object> toStringFunction) {
+        return lazyLoadSafeToString(o, () -> toStringFunction.apply(o));
+    }
+
+    public static String lazyLoadSafeToString(Object o, Supplier<Object> toStringSupplier) {
+        if (o == null) {
+            return null;
+        } else if (isLazyLoadInitialized(o)) {
+            return String.valueOf(toStringSupplier.get());
+        } else {
+            return UNINITIALIZED_TO_STRING;
+        }
+    }
 }
diff --git a/common/src/test/java/com/sequenceiq/cloudbreak/AbstractEntityToStringTest.java b/common/src/test/java/com/sequenceiq/cloudbreak/AbstractEntityToStringTest.java
@@ -0,0 +1,132 @@
+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.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() %s", entityClass.getName(), toString.getDeclaringClass());
+                    }
+                }));
+    }
+
+    @Test
+    void testLazyLoadSafeToString() {
+        assertSoftly(softly ->
+                entityClasses.forEach(entityClass -> {
+                    Object entityInstance = createEntityInstance(entityClass);
+                    setLazyLoadedFields(entityClass, entityInstance);
+                    try {
+                        entityInstance.toString();
+                    } catch (UninitializedToString e) {
+                        softly.fail("Lazy loaded field %s may throw UninitializedException, please wrap in DatabaseUtil.lazyLoadSafeToString()", e.getField());
+                    }
+                }));
+    }
+
+    /**
+     * 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 UninitializedToString(field));
+        return hibernateProxy;
+    }
+
+    static class UninitializedToString extends RuntimeException {
+        private final Field field;
+
+        UninitializedToString(Field field) {
+            this.field = field;
+        }
+
+        public Field getField() {
+            return field;
+        }
+    }
+}