AMBARI-5564. VersionUtils doesn't trim input strings. SchemaUpgradeHelper should log unexpected exceptions. (mpapirkovskyy)

ambari-server/src/main/java/org/apache/ambari/server/upgrade/SchemaUpgradeHelper.java

@@ -102,7 +102,7 @@ public class SchemaUpgradeHelper {
   protected String getAmbariServerVersion() {
     String versionFilePath = configuration.getServerVersionFilePath();
     try {
-      return FileUtils.readFileToString(new File(versionFilePath));
+      return FileUtils.readFileToString(new File(versionFilePath)).trim();
     } catch (IOException e) {
       e.printStackTrace();
     }
@@ -174,12 +174,6 @@ public class SchemaUpgradeHelper {
       for (UpgradeCatalog upgradeCatalog : upgradeCatalogs) {
         try {
           upgradeCatalog.upgradeSchema();
-        } catch (AmbariException e) {
-          LOG.error("Upgrade failed. ", e);
-          throw e;
-        } catch (SQLException e) {
-          LOG.error("Upgrade failed. ", e);
-          throw new AmbariException(e.getMessage(), e);
         } catch (Exception e) {
           LOG.error("Upgrade failed. ", e);
           throw new AmbariException(e.getMessage(), e);
@@ -195,12 +189,6 @@ public class SchemaUpgradeHelper {
       for (UpgradeCatalog upgradeCatalog : upgradeCatalogs) {
         try {
           upgradeCatalog.upgradeData();
-        } catch (AmbariException e) {
-          LOG.error("Upgrade failed. ", e);
-          throw e;
-        } catch (SQLException e) {
-          LOG.error("Upgrade failed. ", e);
-          throw new AmbariException(e.getMessage());
         } catch (Exception e) {
           LOG.error("Upgrade failed. ", e);
           throw new AmbariException(e.getMessage(), e);
@@ -215,32 +203,41 @@ public class SchemaUpgradeHelper {
    * @param args args[0] = target version to upgrade to.
    */
   public static void main(String[] args) throws Exception {
-    Injector injector = Guice.createInjector(new UpgradeHelperModule());
-    SchemaUpgradeHelper schemaUpgradeHelper = injector.getInstance(SchemaUpgradeHelper.class);
+    try {
+      Injector injector = Guice.createInjector(new UpgradeHelperModule());
+      SchemaUpgradeHelper schemaUpgradeHelper = injector.getInstance(SchemaUpgradeHelper.class);
 
-    String targetVersion = schemaUpgradeHelper.getAmbariServerVersion();
-    LOG.info("Upgrading schema to target version = " + targetVersion);
+      String targetVersion = schemaUpgradeHelper.getAmbariServerVersion();
+      LOG.info("Upgrading schema to target version = " + targetVersion);
 
-    UpgradeCatalog targetUpgradeCatalog = AbstractUpgradeCatalog
-      .getUpgradeCatalog(targetVersion);
+      UpgradeCatalog targetUpgradeCatalog = AbstractUpgradeCatalog
+        .getUpgradeCatalog(targetVersion);
 
-    LOG.debug("Target upgrade catalog. " + targetUpgradeCatalog);
+      LOG.debug("Target upgrade catalog. " + targetUpgradeCatalog);
 
-    // Read source version from DB
-    String sourceVersion = schemaUpgradeHelper.readSourceVersion();
-    LOG.info("Upgrading schema from source version = " + sourceVersion);
+      // Read source version from DB
+      String sourceVersion = schemaUpgradeHelper.readSourceVersion();
+      LOG.info("Upgrading schema from source version = " + sourceVersion);
 
-    List<UpgradeCatalog> upgradeCatalogs =
-      schemaUpgradeHelper.getUpgradePath(sourceVersion, targetVersion);
+      List<UpgradeCatalog> upgradeCatalogs =
+        schemaUpgradeHelper.getUpgradePath(sourceVersion, targetVersion);
 
-    schemaUpgradeHelper.executeUpgrade(upgradeCatalogs);
+      schemaUpgradeHelper.executeUpgrade(upgradeCatalogs);
 
-    schemaUpgradeHelper.startPersistenceService();
+      schemaUpgradeHelper.startPersistenceService();
 
-    schemaUpgradeHelper.executeDMLUpdates(upgradeCatalogs);
+      schemaUpgradeHelper.executeDMLUpdates(upgradeCatalogs);
 
-    LOG.info("Upgrade successful.");
+      LOG.info("Upgrade successful.");
 
-    schemaUpgradeHelper.stopPersistenceService();
+      schemaUpgradeHelper.stopPersistenceService();
+    } catch (Throwable e) {
+      if (e instanceof AmbariException) {
+        throw (AmbariException)e;
+      }else{
+        LOG.error("Unexpected error, upgrade failed", e);
+        throw new Exception("Unexpected error, upgrade failed", e);
+      }
+    }
   }
 }

ambari-server/src/main/java/org/apache/ambari/server/utils/VersionUtils.java

@@ -18,6 +18,7 @@
 package org.apache.ambari.server.utils;
 
 import org.apache.ambari.server.bootstrap.BootStrapImpl;
+import org.apache.commons.lang.StringUtils;
 
 /**
  * Provides various utility functions to be used for version handling.
@@ -36,6 +37,9 @@ public class VersionUtils {
    */
   public static int compareVersions(String version1, String version2, int maxLengthToCompare)
     throws IllegalArgumentException {
+    version1 = StringUtils.trim(version1);
+    version2 = StringUtils.trim(version2);
+
     if (version1 == null || version1.isEmpty()) {
       throw new IllegalArgumentException("version1 cannot be null or empty");
     }
@@ -46,9 +50,9 @@ public class VersionUtils {
       throw new IllegalArgumentException("maxLengthToCompare cannot be less than 0");
     }
 
-    
+
     if(BootStrapImpl.DEV_VERSION.equals(version1.trim())) return 0;
-    
+
     String[] version1Parts = version1.split("\\.");
     String[] version2Parts = version2.split("\\.");
 

ambari-server/src/test/java/org/apache/ambari/server/utils/TestVersionUtils.java

@@ -63,6 +63,7 @@ public class TestVersionUtils {
     Assert.assertEquals(0, VersionUtils.compareVersions("1.2.3", "1.2.3.4", 3));
     Assert.assertEquals(0, VersionUtils.compareVersions("1.2.3.6.7", "1.2.3.4", 3));
     Assert.assertEquals(1, VersionUtils.compareVersions("1.2.3.6.7", "1.2.3.4", 4));
+    Assert.assertEquals(1, VersionUtils.compareVersions("1.2.3.6.7\n", "1.2.3.4\n", 4)); //test version trimming
 
     Assert.assertEquals(1, VersionUtils.compareVersions("1.2.3.1", "1.2.3", true));
     Assert.assertEquals(1, VersionUtils.compareVersions("2.1.3", "1.2.3", true));