This is an automated email from the ASF dual-hosted git repository. mradhakrishnan pushed a commit to branch AMBARI-24711 in repository https://gitbox.apache.org/repos/asf/ambari.git
commit 4ae922209f7173f492aa4a58b9e9c5c3fc34437b Author: Madhuvanthi Radhakrishnan <[email protected]> AuthorDate: Wed Aug 30 11:37:24 2017 -0700 AMBARI-21849 : Clean up repo_version table during mpack delete, add create validation for mpacks (mradhakrishnan) --- .../ambari/server/api/services/AmbariMetaInfo.java | 4 +- .../controller/internal/MpackResourceProvider.java | 53 +++++++++++++++++- .../server/orm/dao/RepositoryVersionDAO.java | 15 ++++- .../internal/MpackResourceProviderTest.java | 65 ++++++++++++---------- 4 files changed, 104 insertions(+), 33 deletions(-) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java b/ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java index 6812bf2..6ae0e1c 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java @@ -1554,7 +1554,9 @@ public class AmbariMetaInfo { * @throws IOException */ public void removeMpack(MpackEntity mpackEntity, StackEntity stackEntity) throws IOException { - versionDefinitions.clear(); + if(versionDefinitions != null) { + versionDefinitions.clear(); + } boolean stackDelete = mpackManager.removeMpack(mpackEntity, stackEntity); if(stackDelete) { diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/MpackResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/MpackResourceProvider.java index 7366390..c4833d1 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/MpackResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/MpackResourceProvider.java @@ -18,8 +18,12 @@ package org.apache.ambari.server.controller.internal; import java.io.IOException; + import java.util.HashSet; import java.util.Set; +import java.net.URI; +import java.net.URL; +import java.util.ArrayList; import java.util.Arrays; import java.util.Map; import java.util.HashMap; @@ -47,11 +51,16 @@ import org.apache.ambari.server.controller.MpackRequest; import org.apache.ambari.server.controller.utilities.PredicateHelper; import org.apache.ambari.server.controller.utilities.PropertyHelper; import org.apache.ambari.server.orm.dao.MpackDAO; +import org.apache.ambari.server.orm.dao.RepositoryVersionDAO; import org.apache.ambari.server.orm.dao.StackDAO; import org.apache.ambari.server.orm.entities.MpackEntity; import org.apache.ambari.server.orm.entities.StackEntity; import org.apache.ambari.server.state.Packlet; +import org.apache.ambari.server.state.StackId; +import org.apache.commons.io.IOUtils; +import org.apache.commons.lang.Validate; + /** * ResourceProvider for Mpack instances */ @@ -88,6 +97,9 @@ public class MpackResourceProvider extends AbstractControllerResourceProvider { @Inject protected static StackDAO stackDAO; + @Inject + protected static RepositoryVersionDAO repositoryVersionDAO; + static { // properties PROPERTY_IDS.add(MPACK_ID); @@ -122,8 +134,10 @@ public class MpackResourceProvider extends AbstractControllerResourceProvider { Set<Resource> associatedResources = new HashSet<>(); try { MpackRequest mpackRequest = getRequest(request); - if (mpackRequest == null) + if (mpackRequest == null) { throw new BodyParseException("Please provide " + MPACK_NAME + " ," + MPACK_VERSION + " ," + MPACK_URI); + } + validateCreateRequest(mpackRequest); MpackResponse response = getManagementController().registerMpack(mpackRequest); if (response != null) { notifyCreate(Resource.Type.Mpack, request); @@ -145,8 +159,39 @@ public class MpackResourceProvider extends AbstractControllerResourceProvider { return null; } - public MpackRequest getRequest(Request request) { - MpackRequest mpackRequest = new MpackRequest(); + /*** + * Validates the request body for the required properties in order to create an Mpack resource. + * @param mpackRequest + */ + private void validateCreateRequest(MpackRequest mpackRequest) { + final String mpackName = mpackRequest.getMpackName(); + final String mpackUrl = mpackRequest.getMpackUri(); + final Long registryId = mpackRequest.getRegistryId(); + final String mpackVersion = mpackRequest.getMpackVersion(); + + if(registryId == null) { + Validate.isTrue(mpackUrl != null); + LOG.info("Received a createMpack request" + + ", mpackUrl=" + mpackUrl); + } else { + Validate.notNull(mpackName, "MpackName should not be null"); + Validate.notNull(mpackVersion, "MpackVersion should not be null"); + LOG.info("Received a createMpack request" + + ", mpackName=" + mpackName + + ", mpackVersion=" + mpackVersion + + ", registryId=" + registryId); + } + try { + URI uri = new URI(mpackUrl); + URL url = uri.toURL(); + String jsonString = IOUtils.toString(url); + }catch(Exception e){ + Validate.isTrue(e == null, e.getMessage() + " is an invalid mpack uri. Please check the download link for the mpack again."); + } + } + + public MpackRequest getRequest(Request request) throws AmbariException { + MpackRequest mpackRequest = new MpackRequest(); Set<Map<String, Object>> properties = request.getProperties(); for (Map propertyMap : properties) { //Mpack Download url is either given in the request body or is fetched using the registry id @@ -260,6 +305,7 @@ public class MpackResourceProvider extends AbstractControllerResourceProvider { MpackEntity mpackEntity = mpackDAO.findById(mpackId); StackEntity stackEntity = stackDAO.findByMpack(mpackId); + try { getManagementController().removeMpack(mpackEntity, stackEntity); if (mpackEntity != null) { @@ -267,6 +313,7 @@ public class MpackResourceProvider extends AbstractControllerResourceProvider { @Override public DeleteStatusMetaData invoke() throws AmbariException { if (stackEntity != null) { + repositoryVersionDAO.removeByStack(new StackId(stackEntity.getStackName() + "-" + stackEntity.getStackVersion())); stackDAO.removeByMpack(mpackId); notifyDelete(Resource.Type.Stack, predicate); } diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/RepositoryVersionDAO.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/RepositoryVersionDAO.java index eaccdb0..30b7adf 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/RepositoryVersionDAO.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/RepositoryVersionDAO.java @@ -255,8 +255,21 @@ public class RepositoryVersionDAO extends CrudDAO<RepositoryVersionEntity, Long> @RequiresSession public List<RepositoryVersionEntity> findByServiceDesiredVersion(List<RepositoryVersionEntity> matching) { TypedQuery<RepositoryVersionEntity> query = entityManagerProvider.get(). - createNamedQuery("findByServiceDesiredVersion", RepositoryVersionEntity.class); + createNamedQuery("findByServiceDesiredVersion", RepositoryVersionEntity.class); return daoUtils.selectList(query, matching); } + /** + * Removes the specified repoversion entry based on stackid. + * + * @param stackId + * + */ + @Transactional + public void removeByStack(StackId stackId) { + List<RepositoryVersionEntity> repoVersionDeleteCandidates = findByStack(stackId); + for(RepositoryVersionEntity repositoryVersionEntity : repoVersionDeleteCandidates) { + entityManagerProvider.get().remove(repositoryVersionEntity); + } + } } diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/MpackResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/MpackResourceProviderTest.java index d6638c6..0e38b08 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/MpackResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/MpackResourceProviderTest.java @@ -17,6 +17,20 @@ */ package org.apache.ambari.server.controller.internal; +import org.apache.commons.io.IOUtils; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; + +import java.net.URI; +import java.net.URL; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; import com.google.inject.Injector; @@ -110,10 +124,8 @@ public class MpackResourceProviderTest { replay(m_dao); ResourceProvider provider = AbstractControllerResourceProvider.getResourceProvider( - type, - PropertyHelper.getPropertyIds(type), - PropertyHelper.getKeyPropertyIds(type), - m_amc); + type + ); // create the request Request request = PropertyHelper.getReadRequest(); @@ -183,10 +195,7 @@ public class MpackResourceProviderTest { replay(m_dao,m_amc); ResourceProvider provider = AbstractControllerResourceProvider.getResourceProvider( - type, - PropertyHelper.getPropertyIds(type), - PropertyHelper.getKeyPropertyIds(type), - m_amc); + type); // create the request Request request = PropertyHelper.getReadRequest(); @@ -210,12 +219,13 @@ public class MpackResourceProviderTest { @Test public void testCreateResources() throws Exception { MpackRequest mpackRequest = new MpackRequest(); - mpackRequest.setMpackUri("abc.tar.gz"); + String mpackUri = Paths.get("src/test/resources/mpacks-v2/abc.tar.gz").toUri().toURL().toString(); + mpackRequest.setMpackUri(mpackUri); Request request = createMock(Request.class); - MpackResponse response = new MpackResponse(setupMpacks()); + MpackResponse response = new MpackResponse(setupMpack()); Set<Map<String, Object>> properties = new HashSet<>(); Map propertyMap = new HashMap(); - propertyMap.put(MpackResourceProvider.MPACK_URI,"abc.tar.gz"); + propertyMap.put(MpackResourceProvider.MPACK_URI,mpackUri); properties.add(propertyMap); // set expectations @@ -225,10 +235,8 @@ public class MpackResourceProviderTest { // end expectations ResourceProvider provider = AbstractControllerResourceProvider.getResourceProvider( - Resource.Type.Mpack, - PropertyHelper.getPropertyIds(Resource.Type.Mpack), - PropertyHelper.getKeyPropertyIds(Resource.Type.Mpack), - m_amc); + Resource.Type.Mpack + ); AbstractResourceProviderTest.TestObserver observer = new AbstractResourceProviderTest.TestObserver(); ((ObservableResourceProvider)provider).addObserver(observer); @@ -241,7 +249,7 @@ public class MpackResourceProviderTest { Assert.assertEquals((long)100,r.getPropertyValue(MpackResourceProvider.MPACK_ID)); Assert.assertEquals("testMpack",r.getPropertyValue(MpackResourceProvider.MPACK_NAME)); Assert.assertEquals("3.0",r.getPropertyValue(MpackResourceProvider.MPACK_VERSION)); - Assert.assertEquals("abc.tar.gz",r.getPropertyValue(MpackResourceProvider.MPACK_URI)); + Assert.assertEquals("../../../../../../../resources/mpacks-v2/abc.tar.gz",r.getPropertyValue(MpackResourceProvider.MPACK_URI)); } ResourceProviderEvent lastEvent = observer.getLastEvent(); Assert.assertNotNull(lastEvent); @@ -253,18 +261,19 @@ public class MpackResourceProviderTest { verify(m_amc,request); } - public Mpacks setupMpacks(){ - Mpacks mpacks = new Mpacks(); - mpacks.setMpackId((long)100); - mpacks.setPacklets(new ArrayList<Packlet>()); - mpacks.setPrerequisites(new HashMap<String, String>()); - mpacks.setRegistryId(new Long(100)); - mpacks.setVersion("3.0"); - mpacks.setMpacksUri("abc.tar.gz"); - mpacks.setDescription("Test mpacks"); - mpacks.setName("testMpack"); - - return mpacks; + + public Mpacks setupMpack() { + Mpacks mpack = new Mpacks(); + mpack.setMpackId((long)100); + mpack.setPacklets(new ArrayList<Packlet>()); + mpack.setPrerequisites(new HashMap<String, String>()); + mpack.setRegistryId(new Long(100)); + mpack.setVersion("3.0"); + mpack.setMpacksUri("../../../../../../../resources/mpacks-v2/abc.tar.gz"); + mpack.setDescription("Test mpack"); + mpack.setName("testMpack"); + + return mpack; } /**
