AMBARI-21693. Can't register multiple PATCH versions (ncole)

ambari-server/src/main/java/org/apache/ambari/server/controller/internal/VersionDefinitionResourceProvider.java

@@ -21,6 +21,7 @@ import java.io.InputStream;
 import java.io.UnsupportedEncodingException;
 import java.net.URI;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashSet;
@@ -71,6 +72,8 @@ import org.codehaus.jackson.node.ArrayNode;
 import org.codehaus.jackson.node.JsonNodeFactory;
 import org.codehaus.jackson.node.ObjectNode;
 
+import com.google.common.base.Function;
+import com.google.common.collect.Collections2;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Sets;
 import com.google.inject.Inject;
@@ -442,39 +445,67 @@ public class VersionDefinitionResourceProvider extends AbstractAuthorizedResourc
 
     boolean emptyCompatible = StringUtils.isBlank(holder.xml.release.compatibleWith);
 
-    for (RepositoryVersionEntity candidate : entities) {
-      String baseVersion = candidate.getVersion();
+    for (RepositoryVersionEntity version : entities) {
+      String baseVersion = version.getVersion();
       if (baseVersion.lastIndexOf('-') > -1) {
         baseVersion = baseVersion.substring(0,  baseVersion.lastIndexOf('-'));
       }
 
       if (emptyCompatible) {
         if (baseVersion.equals(holder.xml.release.version)) {
-          matching.add(candidate);
+          matching.add(version);
         }
       } else {
         if (baseVersion.matches(holder.xml.release.compatibleWith)) {
-          matching.add(candidate);
+          matching.add(version);
         }
       }
     }
 
+    RepositoryVersionEntity parent = null;
+
     if (matching.isEmpty()) {
       String format = "No versions matched pattern %s";
 
       throw new IllegalArgumentException(String.format(format,
           emptyCompatible ? holder.xml.release.version : holder.xml.release.compatibleWith));
     } else if (matching.size() > 1) {
-      Set<String> versions= new HashSet<>();
-      for (RepositoryVersionEntity match : matching) {
-        versions.add(match.getVersion());
-      }
 
-      throw new IllegalArgumentException(String.format("More than one repository matches patch %s: %s",
+      Function<RepositoryVersionEntity, String> function = new Function<RepositoryVersionEntity, String>() {
+        @Override
+        public String apply(RepositoryVersionEntity input) {
+          return input.getVersion();
+        }
+      };
+
+      Collection<String> versions = Collections2.transform(matching, function);
+
+      List<RepositoryVersionEntity> used = s_repoVersionDAO.findByServiceDesiredVersion(matching);
+
+      if (used.isEmpty()) {
+        throw new IllegalArgumentException(String.format("Could not determine which version " +
+          "to associate patch %s. Remove one of %s and try again.",
           entity.getVersion(), StringUtils.join(versions, ", ")));
+      } else if (used.size() > 1) {
+        Collection<String> usedVersions = Collections2.transform(used, function);
+
+        throw new IllegalArgumentException(String.format("Patch %s was found to match more " +
+            "than one repository in use: %s. Move all services to a common version and try again.",
+            entity.getVersion(), StringUtils.join(usedVersions, ", ")));
+      } else {
+        parent = used.get(0);
+        LOG.warn("Patch {} was found to match more than one repository in {}. " +
+            "Repository {} is in use and will be the parent.", entity.getVersion(),
+               StringUtils.join(versions, ", "), parent.getVersion());
+      }
+    } else {
+      parent = matching.get(0);
     }
 
-    RepositoryVersionEntity parent = matching.get(0);
+    if (null == parent) {
+      throw new IllegalArgumentException(String.format("Could not find any parent repository for %s.",
+          entity.getVersion()));
+    }
 
     entity.setParent(parent);
   }

ambari-server/src/main/java/org/apache/ambari/server/orm/dao/RepositoryVersionDAO.java

@@ -212,7 +212,6 @@ public class RepositoryVersionDAO extends CrudDAO<RepositoryVersionEntity, Long>
     return daoUtils.selectList(query);
   }
 
-
   /**
    * @param repositoryVersion
    * @return
@@ -225,4 +224,18 @@ public class RepositoryVersionDAO extends CrudDAO<RepositoryVersionEntity, Long>
 
     return daoUtils.selectOne(query);
   }
+
+  /**
+   * Retrieves the repo versions matching the provided ones that are currently being used
+   * for a service.
+   *
+   * @param matching  the list of repo versions
+   */
+  @RequiresSession
+  public List<RepositoryVersionEntity> findByServiceDesiredVersion(List<RepositoryVersionEntity> matching) {
+    TypedQuery<RepositoryVersionEntity> query = entityManagerProvider.get().
+          createNamedQuery("findByServiceDesiredVersion", RepositoryVersionEntity.class);
+
+    return daoUtils.selectList(query, matching);
+  }
 }

ambari-server/src/main/java/org/apache/ambari/server/orm/entities/RepositoryVersionEntity.java

@@ -73,13 +73,24 @@ import com.google.inject.Provider;
     initialValue = 0
     )
 @NamedQueries({
-    @NamedQuery(name = "repositoryVersionByDisplayName", query = "SELECT repoversion FROM RepositoryVersionEntity repoversion WHERE repoversion.displayName=:displayname"),
-    @NamedQuery(name = "repositoryVersionByStack", query = "SELECT repoversion FROM RepositoryVersionEntity repoversion WHERE repoversion.stack.stackName=:stackName AND repoversion.stack.stackVersion=:stackVersion"),
-    @NamedQuery(name = "repositoryVersionByVersion", query = "SELECT repoversion FROM RepositoryVersionEntity repoversion WHERE repoversion.version=:version"),
-    @NamedQuery(name = "repositoryVersionByStackNameAndVersion", query = "SELECT repoversion FROM RepositoryVersionEntity repoversion WHERE repoversion.stack.stackName=:stackName AND repoversion.version=:version"),
-    @NamedQuery(name = "repositoryVersionsFromDefinition", query = "SELECT repoversion FROM RepositoryVersionEntity repoversion WHERE repoversion.versionXsd IS NOT NULL")
-
-})
+    @NamedQuery(
+        name = "repositoryVersionByDisplayName",
+        query = "SELECT repoversion FROM RepositoryVersionEntity repoversion WHERE repoversion.displayName=:displayname"),
+    @NamedQuery(
+        name = "repositoryVersionByStack",
+        query = "SELECT repoversion FROM RepositoryVersionEntity repoversion WHERE repoversion.stack.stackName=:stackName AND repoversion.stack.stackVersion=:stackVersion"),
+    @NamedQuery(
+        name = "repositoryVersionByStackNameAndVersion",
+        query = "SELECT repoversion FROM RepositoryVersionEntity repoversion WHERE repoversion.stack.stackName=:stackName AND repoversion.version=:version"),
+    @NamedQuery(
+        name = "repositoryVersionsFromDefinition",
+        query = "SELECT repoversion FROM RepositoryVersionEntity repoversion WHERE repoversion.versionXsd IS NOT NULL"),
+    @NamedQuery(
+        name = "findRepositoryByVersion",
+        query = "SELECT repositoryVersion FROM RepositoryVersionEntity repositoryVersion WHERE repositoryVersion.version = :version ORDER BY repositoryVersion.id DESC"),
+    @NamedQuery(
+        name = "findByServiceDesiredVersion",
+        query = "SELECT DISTINCT sd.desiredRepositoryVersion from ServiceDesiredStateEntity sd WHERE sd.desiredRepositoryVersion IN ?1") })
 @StaticallyInject
 public class RepositoryVersionEntity {
   private static final Logger LOG = LoggerFactory.getLogger(RepositoryVersionEntity.class);

ambari-server/src/test/java/org/apache/ambari/server/controller/internal/VersionDefinitionResourceProviderTest.java

@@ -28,6 +28,7 @@ import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.H2DatabaseCleaner;
 import org.apache.ambari.server.api.services.AmbariMetaInfo;
 import org.apache.ambari.server.controller.ResourceProviderFactory;
@@ -47,6 +48,8 @@ import org.apache.ambari.server.orm.entities.RepositoryVersionEntity;
 import org.apache.ambari.server.orm.entities.StackEntity;
 import org.apache.ambari.server.security.TestAuthenticationFactory;
 import org.apache.ambari.server.stack.StackManager;
+import org.apache.ambari.server.state.Cluster;
+import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.state.repository.VersionDefinitionXml;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.io.IOUtils;
@@ -67,6 +70,8 @@ import com.google.inject.Injector;
 public class VersionDefinitionResourceProviderTest {
   private Injector injector;
 
+  private RepositoryVersionEntity parentEntity = null;
+
   @Before
   public void before() throws Exception {
     injector = Guice.createInjector(new InMemoryDefaultTestModule());
@@ -78,13 +83,14 @@ public class VersionDefinitionResourceProviderTest {
     StackDAO stackDao = injector.getInstance(StackDAO.class);
     StackEntity stack = stackDao.find("HDP", "2.2.0");
 
-    RepositoryVersionEntity entity = new RepositoryVersionEntity();
-    entity.setStack(stack);
-    entity.setDisplayName("2.2.0.0");
-    entity.setVersion("2.3.4.4-1234");
+    parentEntity = new RepositoryVersionEntity();
+    parentEntity.setStack(stack);
+    parentEntity.setDisplayName("2.2.0.0");
+    parentEntity.setVersion("2.3.4.4-1234");
 
     RepositoryVersionDAO dao = injector.getInstance(RepositoryVersionDAO.class);
-    dao.create(entity);
+    dao.create(parentEntity);
+
   }
 
   @After
@@ -546,4 +552,158 @@ public class VersionDefinitionResourceProviderTest {
     validation = (Set<String>) res.getPropertyValue("VersionDefinition/validation");
     Assert.assertEquals(1, validation.size());
   }
+
+  @Test
+  public void testCreatePatchNoParentVersions() throws Exception {
+    Authentication authentication = TestAuthenticationFactory.createAdministrator();
+    SecurityContextHolder.getContext().setAuthentication(authentication);
+
+    File file = new File("src/test/resources/version_definition_resource_provider.xml");
+
+    final ResourceProvider versionProvider = new VersionDefinitionResourceProvider();
+
+    final Set<Map<String, Object>> propertySet = new LinkedHashSet<>();
+    final Map<String, Object> properties = new LinkedHashMap<>();
+    properties.put(VersionDefinitionResourceProvider.VERSION_DEF_DEFINITION_URL,
+        file.toURI().toURL().toString());
+    propertySet.add(properties);
+
+    RepositoryVersionDAO dao = injector.getInstance(RepositoryVersionDAO.class);
+    dao.remove(dao.findByDisplayName("2.2.0.0"));
+
+    Map<String, String> info = Collections.singletonMap(Request.DIRECTIVE_DRY_RUN, "true");
+
+    final Request createRequest = PropertyHelper.getCreateRequest(propertySet, info);
+    try {
+      versionProvider.createResources(createRequest);
+      fail("Expected exception creating a resource with no parent");
+    } catch (IllegalArgumentException expected) {
+      Assert.assertTrue(expected.getMessage().contains("there are no repositories for"));
+    }
+  }
+
+  @Test
+  public void testCreatePatchManyParentVersionsNoneUsed() throws Exception {
+    Authentication authentication = TestAuthenticationFactory.createAdministrator();
+    SecurityContextHolder.getContext().setAuthentication(authentication);
+
+    File file = new File("src/test/resources/version_definition_resource_provider.xml");
+
+    final ResourceProvider versionProvider = new VersionDefinitionResourceProvider();
+
+    final Set<Map<String, Object>> propertySet = new LinkedHashSet<>();
+    final Map<String, Object> properties = new LinkedHashMap<>();
+    properties.put(VersionDefinitionResourceProvider.VERSION_DEF_DEFINITION_URL,
+        file.toURI().toURL().toString());
+    propertySet.add(properties);
+
+    RepositoryVersionDAO dao = injector.getInstance(RepositoryVersionDAO.class);
+    RepositoryVersionEntity entity = new RepositoryVersionEntity();
+    entity.setStack(parentEntity.getStack());
+    entity.setDisplayName("2.2.1.0");
+    entity.setVersion("2.3.4.5-1234");
+    dao.create(entity);
+
+    Map<String, String> info = Collections.singletonMap(Request.DIRECTIVE_DRY_RUN, "true");
+
+    final Request createRequest = PropertyHelper.getCreateRequest(propertySet, info);
+
+    try {
+      versionProvider.createResources(createRequest);
+      fail("Expected exception creating a resource with no parent");
+    } catch (IllegalArgumentException expected) {
+      Assert.assertTrue(expected.getMessage().contains("Could not determine which version"));
+    }
+  }
+
+  @Test
+  public void testCreatePatchManyParentVersionsOneUsed() throws Exception {
+    Authentication authentication = TestAuthenticationFactory.createAdministrator();
+    SecurityContextHolder.getContext().setAuthentication(authentication);
+
+    File file = new File("src/test/resources/version_definition_resource_provider.xml");
+
+    final ResourceProvider versionProvider = new VersionDefinitionResourceProvider();
+
+    final Set<Map<String, Object>> propertySet = new LinkedHashSet<>();
+    final Map<String, Object> properties = new LinkedHashMap<>();
+    properties.put(VersionDefinitionResourceProvider.VERSION_DEF_DEFINITION_URL,
+        file.toURI().toURL().toString());
+    propertySet.add(properties);
+
+    RepositoryVersionDAO dao = injector.getInstance(RepositoryVersionDAO.class);
+    RepositoryVersionEntity entity = new RepositoryVersionEntity();
+    entity.setStack(parentEntity.getStack());
+    entity.setDisplayName("2.2.1.0");
+    entity.setVersion("2.3.4.5-1234");
+    dao.create(entity);
+
+    makeService("c1", "HDFS", parentEntity);
+    makeService("c1", "ZOOKEEPER", parentEntity);
+
+    Map<String, String> info = Collections.singletonMap(Request.DIRECTIVE_DRY_RUN, "true");
+
+    final Request createRequest = PropertyHelper.getCreateRequest(propertySet, info);
+
+    try {
+      versionProvider.createResources(createRequest);
+    } catch (IllegalArgumentException unexpected) {
+      // !!! better not
+    }
+  }
+
+  @Test
+  public void testCreatePatchManyParentVersionsManyUsed() throws Exception {
+    Authentication authentication = TestAuthenticationFactory.createAdministrator();
+    SecurityContextHolder.getContext().setAuthentication(authentication);
+
+    File file = new File("src/test/resources/version_definition_resource_provider.xml");
+
+    final ResourceProvider versionProvider = new VersionDefinitionResourceProvider();
+
+    final Set<Map<String, Object>> propertySet = new LinkedHashSet<>();
+    final Map<String, Object> properties = new LinkedHashMap<>();
+    properties.put(VersionDefinitionResourceProvider.VERSION_DEF_DEFINITION_URL,
+        file.toURI().toURL().toString());
+    propertySet.add(properties);
+
+    RepositoryVersionDAO dao = injector.getInstance(RepositoryVersionDAO.class);
+    RepositoryVersionEntity entity = new RepositoryVersionEntity();
+    entity.setStack(parentEntity.getStack());
+    entity.setDisplayName("2.2.1.0");
+    entity.setVersion("2.3.4.5-1234");
+    dao.create(entity);
+
+    makeService("c1", "HDFS", parentEntity);
+    makeService("c1", "ZOOKEEPER", entity);
+
+    Map<String, String> info = Collections.singletonMap(Request.DIRECTIVE_DRY_RUN, "true");
+
+    final Request createRequest = PropertyHelper.getCreateRequest(propertySet, info);
+
+    try {
+      versionProvider.createResources(createRequest);
+      fail("expected exception creating resources");
+    } catch (IllegalArgumentException expected) {
+      Assert.assertTrue(expected.getMessage().contains("Move all services to a common version and try again."));
+    }
+  }
+
+  /**
+   * Helper to create services that are tested with parent repo checks
+   */
+  private void makeService(String clusterName, String serviceName, RepositoryVersionEntity serviceRepo) throws Exception {
+    Clusters clusters = injector.getInstance(Clusters.class);
+
+    Cluster cluster;
+    try {
+      cluster = clusters.getCluster("c1");
+    } catch (AmbariException e) {
+      clusters.addCluster("c1", parentEntity.getStackId());
+      cluster = clusters.getCluster("c1");
+    }
+
+    cluster.addService(serviceName, serviceRepo);
+  }
+
 }