From 410f6a7934bbc955b3acab05cd0c88ee3edf3d37 Mon Sep 17 00:00:00 2001 From: sandeep Date: Fri, 20 Jan 2017 16:18:09 +0530 Subject: [PATCH] Incorporated review comments --- .../falcon/persistence/ExtensionJobsBean.java | 2 +- .../extensions/jdbc/ExtensionMetaStore.java | 45 +++++++------------ .../extensions/store/ExtensionStore.java | 12 ----- .../jdbc/ExtensionMetaStoreTest.java | 6 ++- .../resource/AbstractExtensionManager.java | 4 +- .../resource/proxy/ExtensionManagerProxy.java | 21 ++------- 6 files changed, 25 insertions(+), 65 deletions(-) diff --git a/common/src/main/java/org/apache/falcon/persistence/ExtensionJobsBean.java b/common/src/main/java/org/apache/falcon/persistence/ExtensionJobsBean.java index b6ac79d5f..acb5cf4b6 100644 --- a/common/src/main/java/org/apache/falcon/persistence/ExtensionJobsBean.java +++ b/common/src/main/java/org/apache/falcon/persistence/ExtensionJobsBean.java @@ -45,7 +45,7 @@ @NamedQuery(name = PersistenceConstants.GET_ALL_EXTENSION_JOBS, query = "select OBJECT(a) from ExtensionJobsBean a "), @NamedQuery(name = PersistenceConstants.DELETE_EXTENSION_JOB, query = "delete from ExtensionJobsBean a where a.jobName = :jobName "), @NamedQuery(name = PersistenceConstants.GET_EXTENSION_JOB, query = "select OBJECT(a) from ExtensionJobsBean a where a.jobName = :jobName"), - @NamedQuery(name = PersistenceConstants.GET_JOBS_FOR_AN_EXTENSION, query = "select OBJECT(a) from ExtensionJobsBean a where a.extensionName = :extensionName") + @NamedQuery(name = PersistenceConstants.GET_JOBS_FOR_AN_EXTENSION, query = "select a.jobName from ExtensionJobsBean a where a.extensionName = :extensionName") }) //RESUME CHECKSTYLE CHECK LineLengthCheck public class ExtensionJobsBean { diff --git a/extensions/src/main/java/org/apache/falcon/extensions/jdbc/ExtensionMetaStore.java b/extensions/src/main/java/org/apache/falcon/extensions/jdbc/ExtensionMetaStore.java index 5f6288031..18c854076 100644 --- a/extensions/src/main/java/org/apache/falcon/extensions/jdbc/ExtensionMetaStore.java +++ b/extensions/src/main/java/org/apache/falcon/extensions/jdbc/ExtensionMetaStore.java @@ -27,6 +27,7 @@ import javax.persistence.EntityManager; import javax.persistence.Query; +import java.util.ArrayList; import java.util.Date; import java.util.List; @@ -74,10 +75,7 @@ public Boolean checkIfExtensionExists(String extensionName) { } finally { commitAndCloseTransaction(entityManager); } - if (resultSize > 0){ - return true; - } - return false; + return resultSize > 0; } public Boolean checkIfExtensionJobExists(String jobName) { @@ -91,10 +89,7 @@ public Boolean checkIfExtensionJobExists(String jobName) { } finally { commitAndCloseTransaction(entityManager); } - if (resultSize > 0){ - return true; - } - return false; + return resultSize > 0; } public List getAllExtensions() { @@ -102,7 +97,7 @@ public List getAllExtensions() { beginTransaction(entityManager); Query q = entityManager.createNamedQuery(PersistenceConstants.GET_ALL_EXTENSIONS); try { - return (List)q.getResultList(); + return (List) q.getResultList(); } finally { commitAndCloseTransaction(entityManager); } @@ -113,7 +108,7 @@ public void deleteExtensionsOfType(ExtensionType extensionType) { beginTransaction(entityManager); Query q = entityManager.createNamedQuery(PersistenceConstants.DELETE_EXTENSIONS_OF_TYPE); q.setParameter(EXTENSION_TYPE, extensionType); - try{ + try { q.executeUpdate(); } finally { commitAndCloseTransaction(entityManager); @@ -128,7 +123,7 @@ public ExtensionBean getDetail(String extensionName) { try { List resultList = q.getResultList(); if (!resultList.isEmpty()) { - return (ExtensionBean)resultList.get(0); + return (ExtensionBean) resultList.get(0); } else { return null; } @@ -137,36 +132,26 @@ public ExtensionBean getDetail(String extensionName) { } } - public List getJobsForAnExtension(String extensionName) { + public List getJobsForAnExtension(String extensionName) { EntityManager entityManager = getEntityManager(); beginTransaction(entityManager); Query query = entityManager.createNamedQuery(PersistenceConstants.GET_JOBS_FOR_AN_EXTENSION); query.setParameter(EXTENSION_NAME, extensionName); + List jobNames = new ArrayList<>(); try { - return (List)query.getResultList(); + jobNames.addAll((List) query.getResultList()); } finally { commitAndCloseTransaction(entityManager); } + return jobNames; } - public List getJobsForAnExtension(String extensionName) { - EntityManager entityManager = getEntityManager(); - beginTransaction(entityManager); - Query query = entityManager.createNamedQuery(PersistenceConstants.GET_JOBS_FOR_AN_EXTENSION); - query.setParameter(EXTENSION_NAME, extensionName); - try { - return (List)query.getResultList(); - } finally { - commitAndCloseTransaction(entityManager); - } - } - - public void deleteExtension(String extensionName){ + public void deleteExtension(String extensionName) { EntityManager entityManager = getEntityManager(); beginTransaction(entityManager); Query q = entityManager.createNamedQuery(PersistenceConstants.DELETE_EXTENSION); q.setParameter(EXTENSION_NAME, extensionName); - try{ + try { q.executeUpdate(); } finally { commitAndCloseTransaction(entityManager); @@ -177,7 +162,7 @@ public void storeExtensionJob(String jobName, String extensionName, List byte[] config) { ExtensionMetaStore metaStore = ExtensionStore.getMetaStore(); boolean alreadySubmitted = false; - if (metaStore.getExtensionJobDetails(jobName) != null){ + if (metaStore.getExtensionJobDetails(jobName) != null) { alreadySubmitted = true; } ExtensionJobsBean extensionJobsBean = new ExtensionJobsBean(); @@ -207,7 +192,7 @@ public void deleteExtensionJob(String jobName) { beginTransaction(entityManager); Query query = entityManager.createNamedQuery(PersistenceConstants.DELETE_EXTENSION_JOB); query.setParameter(JOB_NAME, jobName); - try{ + try { query.executeUpdate(); } finally { commitAndCloseTransaction(entityManager); @@ -249,7 +234,7 @@ public ExtensionJobsBean getExtensionJobDetails(String jobName) { } } - public List getAllExtensionJobs() { + List getAllExtensionJobs() { EntityManager entityManager = getEntityManager(); beginTransaction(entityManager); Query q = entityManager.createNamedQuery(PersistenceConstants.GET_ALL_EXTENSION_JOBS); diff --git a/extensions/src/main/java/org/apache/falcon/extensions/store/ExtensionStore.java b/extensions/src/main/java/org/apache/falcon/extensions/store/ExtensionStore.java index c3b4feb49..c50d6dea5 100644 --- a/extensions/src/main/java/org/apache/falcon/extensions/store/ExtensionStore.java +++ b/extensions/src/main/java/org/apache/falcon/extensions/store/ExtensionStore.java @@ -28,7 +28,6 @@ import org.apache.falcon.extensions.jdbc.ExtensionMetaStore; import org.apache.falcon.hadoop.HadoopClientFactory; import org.apache.falcon.persistence.ExtensionBean; -import org.apache.falcon.persistence.ExtensionJobsBean; import org.apache.falcon.util.StartupProperties; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileStatus; @@ -383,17 +382,6 @@ public boolean isExtensionStoreInitialized() { return (storePath != null); } - public List getJobsForAnExtension(final String extensionName) throws FalconException { - List extensionJobs = metaStore.getJobsForAnExtension(extensionName); - List extensionJobNames = new ArrayList<>(); - if (null != extensionJobs && !extensionJobs.isEmpty()) { - for (ExtensionJobsBean extensionJobsBean : extensionJobs) { - extensionJobNames.add(extensionJobsBean.getJobName()); - } - } - return extensionJobNames; - } - public String updateExtensionStatus(final String extensionName, String currentUser, ExtensionStatus status) throws FalconException { validateStatusChange(extensionName, currentUser); diff --git a/extensions/src/test/java/org/apache/falcon/extensions/jdbc/ExtensionMetaStoreTest.java b/extensions/src/test/java/org/apache/falcon/extensions/jdbc/ExtensionMetaStoreTest.java index e3327e8a1..4c494457e 100644 --- a/extensions/src/test/java/org/apache/falcon/extensions/jdbc/ExtensionMetaStoreTest.java +++ b/extensions/src/test/java/org/apache/falcon/extensions/jdbc/ExtensionMetaStoreTest.java @@ -45,7 +45,7 @@ public class ExtensionMetaStoreTest extends AbstractTestExtensionStore { private static ExtensionMetaStore stateStore; @BeforeClass - public void setup() throws Exception{ + public void setup() throws Exception { initExtensionStore(); this.dfsCluster = EmbeddedCluster.newCluster("testCluster"); this.conf = dfsCluster.getConf(); @@ -58,7 +58,7 @@ public void init() { } @Test - public void testExtension(){ + public void testExtension() { //insert stateStore.storeExtensionBean("test1", "test_location", ExtensionType.TRUSTED, "test_description", "falconUser"); @@ -86,6 +86,8 @@ public void testExtensionJob() { //storing again to check for entity manager merge to let submission go forward. stateStore.storeExtensionJob("job1", "test2", feeds, processes, config); + Assert.assertEquals(stateStore.getJobsForAnExtension("test2").size(), 1); + Assert.assertEquals(stateStore.getJobsForAnExtension("test2").get(0), "job1"); Assert.assertEquals(stateStore.getAllExtensionJobs().size(), 1); Assert.assertEquals(stateStore.getExtensionJobDetails("job1").getFeeds().get(0), "testFeed"); stateStore.deleteExtensionJob("job1"); diff --git a/prism/src/main/java/org/apache/falcon/resource/AbstractExtensionManager.java b/prism/src/main/java/org/apache/falcon/resource/AbstractExtensionManager.java index dd10e53a4..c7da6e351 100644 --- a/prism/src/main/java/org/apache/falcon/resource/AbstractExtensionManager.java +++ b/prism/src/main/java/org/apache/falcon/resource/AbstractExtensionManager.java @@ -113,7 +113,7 @@ public APIResult deleteExtensionMetadata(String extensionName) { } private void canDeleteExtension(String extensionName) throws FalconException { - ExtensionStore metaStore = ExtensionStore.get(); + ExtensionMetaStore metaStore = ExtensionStore.getMetaStore(); List extensionJobs = metaStore.getJobsForAnExtension(extensionName); if (!extensionJobs.isEmpty()) { LOG.error("Extension:{} cannot be unregistered as {} are instances of the extension", extensionName, @@ -190,7 +190,7 @@ protected String enableExtension(String extensionName, String currentUser) { private JSONObject buildExtensionDetailResult(final String extensionName) throws FalconException { ExtensionMetaStore metaStore = ExtensionStore.getMetaStore(); - if (!metaStore.checkIfExtensionExists(extensionName)){ + if (!metaStore.checkIfExtensionExists(extensionName)) { throw new ValidationException("No extension resources found for " + extensionName); } diff --git a/prism/src/main/java/org/apache/falcon/resource/proxy/ExtensionManagerProxy.java b/prism/src/main/java/org/apache/falcon/resource/proxy/ExtensionManagerProxy.java index d2700b18c..efb548936 100644 --- a/prism/src/main/java/org/apache/falcon/resource/proxy/ExtensionManagerProxy.java +++ b/prism/src/main/java/org/apache/falcon/resource/proxy/ExtensionManagerProxy.java @@ -64,7 +64,6 @@ import javax.xml.bind.JAXBException; import java.util.Collections; import java.util.ArrayList; -import java.util.HashMap; import java.util.HashSet; import java.util.Arrays; import java.util.Set; @@ -85,7 +84,6 @@ public class ExtensionManagerProxy extends AbstractExtensionManager { public static final Logger LOG = LoggerFactory.getLogger(ExtensionManagerProxy.class); - private static final String TAG_PREFIX_EXTENSION_NAME = "_falcon_extension_name="; private static final String ASCENDING_SORT_ORDER = "asc"; private static final String DESCENDING_SORT_ORDER = "desc"; private Extension extension = new Extension(); @@ -96,6 +94,7 @@ public class ExtensionManagerProxy extends AbstractExtensionManager { private EntityProxyUtil entityProxyUtil = new EntityProxyUtil(); private static final String EXTENSION_PROPERTY_JSON_SUFFIX = "-properties.json"; + //SUSPEND CHECKSTYLE CHECK ParameterNumberCheck @GET @Path("list/{extension-name}") @@ -107,7 +106,7 @@ public ExtensionJobList getExtensionJobs( checkIfExtensionServiceIsEnabled(); checkIfExtensionExists(extensionName); try { - List jobNames = ExtensionStore.get().getJobsForAnExtension(extensionName); + List jobNames = ExtensionStore.getMetaStore().getJobsForAnExtension(extensionName); switch (sortOrder.toLowerCase()) { case DESCENDING_SORT_ORDER: Collections.sort(jobNames, Collections.reverseOrder(String.CASE_INSENSITIVE_ORDER)); @@ -116,7 +115,7 @@ public ExtensionJobList getExtensionJobs( Collections.sort(jobNames, String.CASE_INSENSITIVE_ORDER); } return new ExtensionJobList(jobNames.size(), jobNames); - } catch (FalconException e) { + } catch (Throwable e) { LOG.error("Failed to get extension job list of " + extensionName + ": ", e); throw FalconWebException.newAPIException(e, Response.Status.INTERNAL_SERVER_ERROR); } @@ -758,18 +757,6 @@ private List generateEntities(String extensionName, InputStream configSt return entities; } - private Map> groupEntitiesByJob(List entities) { - Map> groupedEntities = new HashMap<>(); - for (Entity entity : entities) { - String jobName = getJobNameFromTag(entity.getTags()); - if (!groupedEntities.containsKey(jobName)) { - groupedEntities.put(jobName, new ArrayList()); - } - groupedEntities.get(jobName).add(entity); - } - return groupedEntities; - } - private static void checkIfExtensionServiceIsEnabled() { if (!Services.get().isRegistered(ExtensionService.SERVICE_NAME)) { LOG.error(ExtensionService.SERVICE_NAME + " is not enabled."); @@ -777,6 +764,4 @@ private static void checkIfExtensionServiceIsEnabled() { ExtensionService.SERVICE_NAME + " is not enabled.", Response.Status.NOT_FOUND); } } - - }