AMBARI-12129 - Views: Use VARCHAR for DataStore entity String fields (tbeerbower)

ambari-server/src/main/java/org/apache/ambari/server/view/persistence/DataStoreImpl.java

@@ -25,7 +25,6 @@ import org.apache.ambari.view.PersistenceException;
 import org.eclipse.persistence.dynamic.DynamicClassLoader;
 import org.eclipse.persistence.dynamic.DynamicEntity;
 import org.eclipse.persistence.dynamic.DynamicType;
-import org.eclipse.persistence.internal.helper.DatabaseField;
 import org.eclipse.persistence.jpa.dynamic.JPADynamicHelper;
 import org.eclipse.persistence.jpa.dynamic.JPADynamicTypeBuilder;
 import org.eclipse.persistence.mappings.DirectToFieldMapping;
@@ -44,8 +43,6 @@ import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.ParameterizedType;
-import java.sql.Clob;
-import java.sql.Types;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -117,6 +114,11 @@ public class DataStoreImpl implements DataStore {
    */
   protected final static Logger LOG = LoggerFactory.getLogger(DataStoreImpl.class);
 
+  /**
+   * Max length of entity string field.
+   */
+  protected static final int MAX_ENTITY_STRING_FIELD_LENGTH = 4000;
+
   /**
    * Table / column name prefix.
    */
@@ -300,12 +302,9 @@ public class DataStoreImpl implements DataStore {
         if (isDirectMappingType(propertyType)) {
           DirectToFieldMapping mapping = typeBuilder.addDirectMapping(attributeName, propertyType, attributeName);
 
-          // explicitly set the type of string fields
+          // explicitly set the length of string fields
           if (String.class.isAssignableFrom(propertyType)) {
-            DatabaseField field = mapping.getField();
-
-            field.setSqlType(Types.CLOB);
-            field.setType(Clob.class);
+            mapping.getField().setLength(MAX_ENTITY_STRING_FIELD_LENGTH);
           }
         }
       }
@@ -427,6 +426,10 @@ public class DataStoreImpl implements DataStore {
                 value = persistEntity(value, em, persistSet);
               }
               if (value != null) {
+                if (String.class.isAssignableFrom(valueClass)) {
+                  // String values can not exceed MAX_ENTITY_STRING_FIELD_LENGTH
+                  checkStringValue(entity, fieldName, (String) value);
+                }
                 dynamicEntity.set(attributeName, value);
               }
             }
@@ -602,6 +605,19 @@ public class DataStoreImpl implements DataStore {
     return (Class<?>) parameterizedType.getActualTypeArguments()[0];
   }
 
+  // make sure that a string field value doesn't exceed MAX_STRING_LENGTH
+  private static void checkStringValue(Object entity, String fieldName, String value) {
+    if (value.length() > MAX_ENTITY_STRING_FIELD_LENGTH) {
+
+      String msg = String.format("The value for the %s field of the %s entity can not exceed %d characters.  " +
+          "Given value = %s", fieldName, entity.getClass().getSimpleName(), MAX_ENTITY_STRING_FIELD_LENGTH, value);
+
+      LOG.error(msg);
+
+      throw new IllegalStateException(msg);
+    }
+  }
+
   // rollback the given transaction if it is active
   private static void rollbackTransaction(EntityTransaction transaction) {
     if (transaction != null && transaction.isActive()) {

ambari-server/src/test/java/org/apache/ambari/server/view/persistence/DataStoreImplTest.java

@@ -31,6 +31,7 @@ import org.apache.ambari.server.view.configuration.InstanceConfig;
 import org.apache.ambari.server.view.configuration.InstanceConfigTest;
 import org.apache.ambari.server.view.configuration.ViewConfig;
 import org.apache.ambari.server.view.configuration.ViewConfigTest;
+import org.apache.ambari.view.PersistenceException;
 import org.easymock.Capture;
 import org.eclipse.persistence.dynamic.DynamicClassLoader;
 import org.eclipse.persistence.dynamic.DynamicEntity;
@@ -157,22 +158,14 @@ public class DataStoreImplTest {
     expect(entityManagerFactory.createEntityManager()).andReturn(entityManager);
     expect(entityManager.getTransaction()).andReturn(transaction).anyTimes();
 
-    Capture<Class> entityClassCapture = new Capture<Class>();
-    expect(entityManager.find(capture(entityClassCapture), eq("bar"))).andReturn(null);
-
     Capture<Class> entityClassCapture2 = new Capture<Class>();
     expect(entityManager.find(capture(entityClassCapture2), eq(99))).andReturn(null);
 
-    Capture<DynamicEntity> entityCapture = new Capture<DynamicEntity>();
-    entityManager.persist(capture(entityCapture));
-
-    Capture<DynamicEntity> entityCapture2 = new Capture<DynamicEntity>();
-    entityManager.persist(capture(entityCapture2));
-
     entityManager.close();
 
     transaction.begin();
-    transaction.commit();
+    expect(transaction.isActive()).andReturn(true);
+    transaction.rollback();
 
     // replay mocks
     replay(entityManagerFactory, entityManager, jpaDynamicHelper, transaction, schemaManager);
@@ -185,16 +178,12 @@ public class DataStoreImplTest {
     }
     String longString = sb.toString();
 
-    dataStore.store(new TestEntity(99, longString, new TestSubEntity("bar")));
-
-    Assert.assertEquals(entityClassCapture.getValue(), typeCapture.getValue().getJavaClass());
-    Assert.assertEquals(entityClassCapture2.getValue(), typeCapture2.getValue().getJavaClass());
-
-    Assert.assertEquals("bar", entityCapture.getValue().get("DS_name"));
-
-    Assert.assertEquals(99, entityCapture2.getValue().get("DS_id"));
-    Assert.assertEquals(longString, entityCapture2.getValue().get("DS_name"));
-
+    try {
+      dataStore.store(new TestEntity(99, longString, new TestSubEntity("bar")));
+      Assert.fail("Expected PersistenceException.");
+    } catch (PersistenceException e) {
+      // expected
+    }
     // verify mocks
     verify(entityManagerFactory, entityManager, jpaDynamicHelper, transaction, schemaManager);
   }
@@ -273,15 +262,9 @@ public class DataStoreImplTest {
     expect(entityManagerFactory.createEntityManager()).andReturn(entityManager);
     expect(entityManager.getTransaction()).andReturn(transaction).anyTimes();
 
-    Capture<Class> entityClassCapture = new Capture<Class>();
-    expect(entityManager.find(capture(entityClassCapture), eq("bar"))).andReturn(null);
-
     Capture<Class> entityClassCapture2 = new Capture<Class>();
     expect(entityManager.find(capture(entityClassCapture2), eq(99))).andReturn(dynamicEntity);
 
-    Capture<DynamicEntity> entityCapture = new Capture<DynamicEntity>();
-    entityManager.persist(capture(entityCapture));
-
     entityManager.close();
 
     StringBuffer sb = new StringBuffer();
@@ -291,23 +274,22 @@ public class DataStoreImplTest {
     String longString = sb.toString();
 
     expect(dynamicEntity.set("DS_id", 99)).andReturn(dynamicEntity);
-    expect(dynamicEntity.set("DS_name", longString)).andReturn(dynamicEntity);
-
-    Capture<DynamicEntity> subEntityCapture = new Capture<DynamicEntity>();
-    expect(dynamicEntity.set(eq("DS_subEntity"), capture(subEntityCapture))).andReturn(dynamicEntity);
 
     transaction.begin();
-    transaction.commit();
+    expect(transaction.isActive()).andReturn(true).anyTimes();
+    transaction.rollback();
 
     // replay mocks
     replay(entityManagerFactory, entityManager, jpaDynamicHelper, transaction, schemaManager, dynamicEntity);
 
     DataStoreImpl dataStore = getDataStore(entityManagerFactory, jpaDynamicHelper, classLoader, schemaManager);
 
-    dataStore.store(new TestEntity(99, longString, new TestSubEntity("bar")));
-
-    Assert.assertEquals(entityClassCapture.getValue(), typeCapture.getValue().getJavaClass());
-    Assert.assertEquals(entityClassCapture2.getValue(), typeCapture2.getValue().getJavaClass());
+    try {
+      dataStore.store(new TestEntity(99, longString, new TestSubEntity("bar")));
+      Assert.fail();
+    } catch (PersistenceException e) {
+      // expected
+    }
 
     // verify mocks
     verify(entityManagerFactory, entityManager, jpaDynamicHelper, transaction, schemaManager, dynamicEntity);