Repository: ambari Updated Branches: refs/heads/branch-2.1 b665fb511 -> 6c7443801
AMBARI-12129 - Views: Use VARCHAR for DataStore entity String fields (tbeerbower) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/6c744380 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/6c744380 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/6c744380 Branch: refs/heads/branch-2.1 Commit: 6c74438018ea46de2a0b4b682182398c5a2022f3 Parents: b665fb5 Author: tbeerbower <tbeerbo...@hortonworks.com> Authored: Wed Jun 24 15:41:13 2015 -0400 Committer: tbeerbower <tbeerbo...@hortonworks.com> Committed: Wed Jun 24 15:43:06 2015 -0400 ---------------------------------------------------------------------- .../server/view/persistence/DataStoreImpl.java | 32 +++++++++--- .../view/persistence/DataStoreImplTest.java | 52 +++++++------------- 2 files changed, 41 insertions(+), 43 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/6c744380/ambari-server/src/main/java/org/apache/ambari/server/view/persistence/DataStoreImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/view/persistence/DataStoreImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/view/persistence/DataStoreImpl.java index 69328e7..6d31a08 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/view/persistence/DataStoreImpl.java +++ b/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; @@ -118,6 +115,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. */ private static final String NAME_PREFIX = "DS_"; @@ -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()) { http://git-wip-us.apache.org/repos/asf/ambari/blob/6c744380/ambari-server/src/test/java/org/apache/ambari/server/view/persistence/DataStoreImplTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/view/persistence/DataStoreImplTest.java b/ambari-server/src/test/java/org/apache/ambari/server/view/persistence/DataStoreImplTest.java index d623a26..1b4758d 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/view/persistence/DataStoreImplTest.java +++ b/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);