SONAR-16138 Throw error with specific message when portfolio not found (#5999)

* SONAR-16138 Throw error with specific message when portfolio not found

2 files changed, 107 insertions, 10 deletions

diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v91/MigratePortfoliosToNewTables.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v91/MigratePortfoliosToNewTables.java
index 5e920b2276a..dc988604fbb 100644
--- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v91/MigratePortfoliosToNewTables.java
+++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v91/MigratePortfoliosToNewTables.java
@@ -26,6 +26,7 @@ import java.io.InputStreamReader;
 import java.io.StringReader;
 import java.sql.SQLException;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
@@ -52,6 +53,8 @@ import org.codehaus.staxmate.SMInputFactory;
 import org.codehaus.staxmate.in.SMHierarchicCursor;
 import org.codehaus.staxmate.in.SMInputCursor;
 import org.sonar.api.utils.System2;
+import org.sonar.api.utils.log.Logger;
+import org.sonar.api.utils.log.Loggers;
 import org.sonar.core.util.UuidFactory;
 import org.sonar.db.Database;
 import org.sonar.db.DatabaseUtils;
@@ -67,11 +70,13 @@ import org.xml.sax.helpers.DefaultHandler;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Optional.ofNullable;
+import static java.util.stream.Collectors.joining;
 import static javax.xml.XMLConstants.FEATURE_SECURE_PROCESSING;
 import static org.apache.commons.io.IOUtils.toInputStream;
 import static org.apache.commons.lang.StringUtils.trim;
 
 public class MigratePortfoliosToNewTables extends DataChange {
+  private static final Logger LOG = Loggers.get(MigratePortfoliosToNewTables.class);
   private static final String SELECT_DEFAULT_VISIBILITY = "select text_value from properties where prop_key = 'projects.default.visibility'";
   private static final String SELECT_UUID_VISIBILITY_BY_COMPONENT_KEY = "select c.uuid, c.private from components c where c.kee = ?";
   private static final String SELECT_PORTFOLIO_UUID_AND_SELECTION_MODE_BY_KEY = "select uuid,selection_mode from portfolios where kee = ?";
@@ -81,6 +86,11 @@ public class MigratePortfoliosToNewTables extends DataChange {
   private static final String VIEWS_DEF_KEY = "views.def";
   private static final String PLACEHOLDER = "PLACEHOLDER";
 
+  protected static final String PORTFOLIO_CONSISTENCY_ERROR = "Some issues were found in portfolio definition. Please verify "
+    + VIEWS_DEF_KEY + " consistency in internal_properties datatable";
+  protected static final String PORTFOLIO_ROOT_NOT_FOUND = "root with key: %s not found for portfolio with name: %s and key: %s";
+  protected static final String PORTFOLIO_PARENT_NOT_FOUND = "parent with key: %s not found for portfolio with name: %s and key: %s";
+
   private final UuidFactory uuidFactory;
   private final System2 system;
 
@@ -113,14 +123,8 @@ public class MigratePortfoliosToNewTables extends DataChange {
       Map<String, ViewXml.ViewDef> portfolioXmlMap = ViewXml.parse(xml);
       List<ViewXml.ViewDef> portfolios = new LinkedList<>(portfolioXmlMap.values());
 
-      Map<String, PortfolioDb> portfolioDbMap = new HashMap<>();
-      for (ViewXml.ViewDef portfolio : portfolios) {
-        PortfolioDb createdPortfolio = insertPortfolio(context, portfolio);
-        if (createdPortfolio.selectionMode == SelectionMode.MANUAL) {
-          insertPortfolioProjects(context, portfolio, createdPortfolio);
-        }
-        portfolioDbMap.put(createdPortfolio.kee, createdPortfolio);
-      }
+      Map<String, PortfolioDb> portfolioDbMap = buildPortfolioDbMap(context, portfolios);
+      verifyPortfoliosConsistency(portfolios, portfolioDbMap);
       // all portfolio has been created and new uuids assigned
       // update portfolio hierarchy parent/root
       insertReferences(context, portfolioXmlMap, portfolioDbMap);
@@ -130,6 +134,18 @@ public class MigratePortfoliosToNewTables extends DataChange {
     }
   }
 
+  private Map<String, PortfolioDb> buildPortfolioDbMap(Context context, List<ViewDef> portfolios) throws SQLException {
+    Map<String, PortfolioDb> portfolioDbMap = new HashMap<>();
+    for (ViewDef portfolio : portfolios) {
+      PortfolioDb createdPortfolio = insertPortfolio(context, portfolio);
+      if (createdPortfolio.selectionMode == SelectionMode.MANUAL) {
+        insertPortfolioProjects(context, portfolio, createdPortfolio);
+      }
+      portfolioDbMap.put(createdPortfolio.kee, createdPortfolio);
+    }
+    return portfolioDbMap;
+  }
+
   private PortfolioDb insertPortfolio(Context context, ViewXml.ViewDef portfolioFromXml) throws SQLException {
     long now = system.now();
     PortfolioDb portfolioDb = context.prepareSelect(SELECT_PORTFOLIO_UUID_AND_SELECTION_MODE_BY_KEY)
@@ -215,8 +231,7 @@ public class MigratePortfoliosToNewTables extends DataChange {
     return DatabaseUtils.executeLargeInputs(projectKeysToBeAdded, keys -> {
       try {
         String selectQuery = SELECT_PROJECT_UUIDS_BY_KEYS.replace(PLACEHOLDER,
-          keys.stream().map(key -> "'" + key + "'").collect(
-            Collectors.joining(",")));
+          keys.stream().map(key -> "'" + key + "'").collect(joining(",")));
         return context.prepareSelect(selectQuery)
           .list(r -> new ProjectDb(r.getString(1), r.getString(2)));
       } catch (SQLException e) {
@@ -225,6 +240,37 @@ public class MigratePortfoliosToNewTables extends DataChange {
     });
   }
 
+  private static void verifyPortfoliosConsistency(Collection<ViewDef> portfolios, Map<String, PortfolioDb> portfolioDbMap) {
+    List<String> errors = findConsistencyErrors(portfolios, portfolioDbMap);
+    if (!errors.isEmpty()) {
+      LOG.error(PORTFOLIO_CONSISTENCY_ERROR);
+      errors.forEach(LOG::error);
+      throw new IllegalStateException();
+    }
+  }
+
+  private static List<String> findConsistencyErrors(Collection<ViewDef> portfolios, Map<String, PortfolioDb> portfolioDbMap) {
+    return portfolios.stream()
+      .flatMap(portfolio -> findConsistencyErrors(portfolio, portfolioDbMap).stream())
+      .collect(Collectors.toList());
+  }
+
+  private static List<String> findConsistencyErrors(ViewDef portfolio, Map<String, PortfolioDb> portfolioDbMap) {
+    List<String> errors = new ArrayList<>();
+
+    String root = portfolio.root;
+    if (root != null && !portfolioDbMap.containsKey(root)) {
+      errors.add(String.format(PORTFOLIO_ROOT_NOT_FOUND, root, portfolio.name, portfolio.key));
+    }
+
+    String parent = portfolio.parent;
+    if (parent != null && !portfolioDbMap.containsKey(parent)) {
+      errors.add(String.format(PORTFOLIO_PARENT_NOT_FOUND, parent, portfolio.name, portfolio.key));
+    }
+
+    return errors;
+  }
+
   private void insertReferences(Context context, Map<String, ViewDef> portfolioXmlMap,
     Map<String, PortfolioDb> portfolioDbMap) throws SQLException {
     Upsert insertQuery = context.prepareUpsert("insert into portfolio_references(uuid, portfolio_uuid, reference_uuid, created_at) values (?, ?, ?, ?)");
diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v91/MigratePortfoliosToNewTablesTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v91/MigratePortfoliosToNewTablesTest.java
index 512acd9c4bc..73abaf0a50b 100644
--- a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v91/MigratePortfoliosToNewTablesTest.java
+++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v91/MigratePortfoliosToNewTablesTest.java
@@ -27,6 +27,8 @@ import org.junit.Test;
 import org.sonar.api.impl.utils.TestSystem2;
 import org.sonar.api.resources.Qualifiers;
 import org.sonar.api.utils.System2;
+import org.sonar.api.utils.log.LogTester;
+import org.sonar.api.utils.log.LoggerLevel;
 import org.sonar.core.util.UuidFactory;
 import org.sonar.core.util.UuidFactoryFast;
 import org.sonar.db.CoreDbTester;
@@ -34,7 +36,11 @@ import org.sonar.server.platform.db.migration.step.DataChange;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.Assertions.tuple;
+import static org.sonar.server.platform.db.migration.version.v91.MigratePortfoliosToNewTables.PORTFOLIO_CONSISTENCY_ERROR;
+import static org.sonar.server.platform.db.migration.version.v91.MigratePortfoliosToNewTables.PORTFOLIO_PARENT_NOT_FOUND;
+import static org.sonar.server.platform.db.migration.version.v91.MigratePortfoliosToNewTables.PORTFOLIO_ROOT_NOT_FOUND;
 
 public class MigratePortfoliosToNewTablesTest {
   static final int TEXT_VALUE_MAX_LENGTH = 4000;
@@ -43,6 +49,9 @@ public class MigratePortfoliosToNewTablesTest {
   @Rule
   public final CoreDbTester db = CoreDbTester.createForSchema(MigratePortfoliosToNewTablesTest.class, "schema.sql");
 
+  @Rule
+  public LogTester logTester = new LogTester();
+
   private final UuidFactory uuidFactory = UuidFactoryFast.getInstance();
   private final System2 system2 = new TestSystem2().setNow(NOW);
 
@@ -185,6 +194,26 @@ public class MigratePortfoliosToNewTablesTest {
     + "  </vw>"
     + "</views>";
 
+  private final String SIMPLE_XML_PORTFOLIOS_WRONG_HIERARCHY = "<views>"
+    + "  <vw key=\"port1\" def=\"false\">\n"
+    + "    <name><![CDATA[port1]]></name>\n"
+    + "    <desc><![CDATA[port1]]></desc>\n"
+    + "    <qualifier><![CDATA[VW]]></qualifier>\n"
+    + "  </vw>\n"
+    + "  <vw key=\"port2\" def=\"false\" root=\"NON_EXISTING_ROOT\" parent=\"port1\">\n"
+    + "    <name><![CDATA[port2]]></name>\n"
+    + "    <desc><![CDATA[port2]]></desc>\n"
+    + "  </vw>\n"
+    + "  <vw key=\"port3\" def=\"false\" root=\"port1\" parent=\"NON_EXISTING_PARENT\">\n"
+    + "    <name><![CDATA[port3]]></name>\n"
+    + "    <desc><![CDATA[port3]]></desc>\n"
+    + "  </vw>\n"
+    + "  <vw key=\"port4\" def=\"false\" root=\"port3\" parent=\"port3\">\n"
+    + "    <name><![CDATA[port4]]></name>\n"
+    + "    <desc><![CDATA[port4]]></desc>\n"
+    + "  </vw>\n"
+    + "</views>";
+
   @Test
   public void does_not_fail_when_nothing_to_migrate() throws SQLException {
     assertThatCode(underTest::execute)
@@ -384,6 +413,28 @@ public class MigratePortfoliosToNewTablesTest {
         tuple("port-tag-value", "NONE", null));
   }
 
+  @Test
+  public void migrate_xml_should_have_explicite_error_log_when_portfolio_hierarchy_nonexistent() {
+    //GIVEN
+    insertViewsDefInternalProperty(SIMPLE_XML_PORTFOLIOS_WRONG_HIERARCHY);
+    //WHEN, THEN
+    assertThatThrownBy(underTest::execute).isInstanceOf(IllegalStateException.class);
+    assertThat(logTester.logs(LoggerLevel.ERROR)).containsExactly(
+      PORTFOLIO_CONSISTENCY_ERROR,
+      String.format(PORTFOLIO_ROOT_NOT_FOUND, "NON_EXISTING_ROOT", "port2", "port2"),
+      String.format(PORTFOLIO_PARENT_NOT_FOUND, "NON_EXISTING_PARENT", "port3", "port3"));
+  }
+
+  @Test
+  public void migrate_xml_should_not_have_explicite_error_log() throws SQLException {
+    //GIVEN
+    insertViewsDefInternalProperty(SIMPLE_XML_PORTFOLIOS_HIERARCHY);
+    //WHEN
+    underTest.execute();
+    //THEN
+    assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty();
+  }
+
   private void insertProject(String uuid, String key, String qualifier) {
     db.executeInsert("PROJECTS",
       "UUID", uuid,