OF-796: Update version handling

Use numeric comparison for the various version components

src/java/org/jivesoftware/openfire/container/PluginManager.java

@@ -297,11 +297,9 @@ public class PluginManager {
                 // required to run.
                 Element minServerVersion = (Element)pluginXML.selectSingleNode("/plugin/minServerVersion");
                 if (minServerVersion != null) {
-                    String requiredVersion = minServerVersion.getTextTrim();
-                    Version version = XMPPServer.getInstance().getServerInfo().getVersion();
-                    String hasVersion = version.getMajor() + "." + version.getMinor() + "." +
-                        version.getMicro();
-                    if (hasVersion.compareTo(requiredVersion) < 0) {
+                    Version requiredVersion = new Version(minServerVersion.getTextTrim());
+                    Version currentVersion = XMPPServer.getInstance().getServerInfo().getVersion();
+                    if (requiredVersion.isNewerThan(currentVersion)) {
                         String msg = "Ignoring plugin " + pluginDir.getName() + ": requires " +
                             "server version " + requiredVersion;
                         Log.warn(msg);

src/java/org/jivesoftware/openfire/update/UpdateManager.java

@@ -54,6 +54,7 @@ import org.jivesoftware.openfire.container.Plugin;
 import org.jivesoftware.util.JiveConstants;
 import org.jivesoftware.util.JiveGlobals;
 import org.jivesoftware.util.LocaleUtils;
+import org.jivesoftware.util.Version;
 import org.jivesoftware.util.XMLWriter;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -337,10 +338,11 @@ public class UpdateManager extends BasicModule {
             }
         }
         // Remove plugins that require a newer server version
-        String serverVersion = XMPPServer.getInstance().getServerInfo().getVersion().getVersionString();
+        Version currentServerVersion = XMPPServer.getInstance().getServerInfo().getVersion();
         for (Iterator<AvailablePlugin> it=plugins.iterator(); it.hasNext();) {
             AvailablePlugin plugin = it.next();
-            if (serverVersion.compareTo(plugin.getMinServerVersion()) < 0) {
+            Version pluginMinServerVersion = new Version(plugin.getMinServerVersion());
+            if (pluginMinServerVersion.isNewerThan(currentServerVersion)) {
                 it.remove();
             }
         }
@@ -618,21 +620,24 @@ public class UpdateManager extends BasicModule {
         // Reset list of plugins that need to be updated
         pluginUpdates = new ArrayList<Update>();
         XMPPServer server = XMPPServer.getInstance();
+        Version currentServerVersion = XMPPServer.getInstance().getServerInfo().getVersion();
         // Compare local plugins versions with latest ones
         for (Plugin plugin : server.getPluginManager().getPlugins()) {
             String pluginName = server.getPluginManager().getName(plugin);
             AvailablePlugin latestPlugin = availablePlugins.get(pluginName);
-            String currentVersion = server.getPluginManager().getVersion(plugin);
-            if (latestPlugin != null &&
-                    latestPlugin.getLatestVersion().compareTo(currentVersion) > 0) {
-                // Check if the update can run in the current version of the server
-                String serverVersion =
-                        XMPPServer.getInstance().getServerInfo().getVersion().getVersionString();
-                if (serverVersion.compareTo(latestPlugin.getMinServerVersion()) >= 0) {
-                    Update update = new Update(pluginName, latestPlugin.getLatestVersion(),
-                            latestPlugin.getChangelog(), latestPlugin.getURL());
-                    pluginUpdates.add(update);
-                }
+
+            if (latestPlugin != null) {
+				Version currentPluginVersion = new Version(server.getPluginManager().getVersion(plugin));
+				Version latestPluginVersion = new Version(latestPlugin.getLatestVersion());
+            	if (latestPluginVersion.isNewerThan(currentPluginVersion)) {
+					// Check if the update can run in the current version of the server
+					Version pluginMinServerVersion = new Version(latestPlugin.getMinServerVersion());
+					if (!pluginMinServerVersion.isNewerThan(currentServerVersion)) {
+						Update update = new Update(pluginName, latestPlugin.getLatestVersion(),
+								latestPlugin.getChangelog(), latestPlugin.getURL());
+						pluginUpdates.add(update);
+					}
+				}
             }
         }
     }
@@ -789,14 +794,13 @@ public class UpdateManager extends BasicModule {
         // Parse info and recreate update information (if still required)
         Element openfire = xmlResponse.getRootElement().element("openfire");
         if (openfire != null) {
-            String latestVersion = openfire.attributeValue("latest");
+            Version latestVersion = new Version(openfire.attributeValue("latest"));
             String changelog = openfire.attributeValue("changelog");
             String url = openfire.attributeValue("url");
             // Check if current server version is correct
-            String serverVersion =
-                    XMPPServer.getInstance().getServerInfo().getVersion().getVersionString();
-            if (serverVersion.compareTo(latestVersion) < 0) {
-                serverUpdate = new Update("Openfire", latestVersion, changelog, url);
+            Version curentServerVersion = XMPPServer.getInstance().getServerInfo().getVersion();
+            if (latestVersion.isNewerThan(curentServerVersion)) {
+                serverUpdate = new Update("Openfire", latestVersion.getVersionString(), changelog, url);
             }
         }
     }

src/java/org/jivesoftware/util/Version.java

@@ -19,12 +19,14 @@
 
 package org.jivesoftware.util;
 
+import java.util.StringTokenizer;
+
 /**
  * Holds version information for Openfire.
  *
  * @author Iain Shigeoka
  */
-public class Version {
+public class Version implements Comparable<Version> {
 
     /**
      * The major number (ie 1.x.x).
@@ -70,23 +72,30 @@ public class Version {
         this.micro = micro;
         this.status = status;
         this.statusVersion = statusVersion;
-        if (status != null) {
-            if (status == ReleaseStatus.Release) {
-                versionString = major + "." + minor + "." + micro;
-            }
-            else {
-                if (statusVersion >= 0) {
-                    versionString = major + "." + minor + "." + micro + " " + status.toString() +
-                            " " + statusVersion;
-                }
-                else {
-                    versionString = major + "." + minor + "." + micro + " " + status.toString();
-                }
-            }
-        }
-        else {
-            versionString = major + "." + minor + "." + micro;
-        }
+    }
+    
+    /**
+     * Create a new version from a simple version string (e.g. "3.9.3")
+     * 
+     * @param source the version string
+     */
+    public Version(String source) {
+    	// initialize the defaults
+    	major = minor = micro = 0;
+    	status = ReleaseStatus.Release;
+    	statusVersion = -1;
+    	
+    	if (source != null) {
+        	StringTokenizer parser = new StringTokenizer(source, ".");
+    		try {
+    			major = parser.hasMoreTokens() ? Integer.parseInt(parser.nextToken()) : 0;
+    			minor = parser.hasMoreTokens() ? Integer.parseInt(parser.nextToken()) : 0;
+    			micro = parser.hasMoreTokens() ? Integer.parseInt(parser.nextToken()) : 0;
+    		}
+    		catch (NumberFormatException nfe) {
+    			// ignore bad version
+    		}
+    	}  	
     }
 
     /**
@@ -96,6 +105,26 @@ public class Version {
      * @return The version as a string
      */
     public String getVersionString() {
+    	if (versionString == null) {
+            if (status != null) {
+                if (status == ReleaseStatus.Release) {
+                    versionString = major + "." + minor + "." + micro;
+                }
+                else {
+                    if (statusVersion >= 0) {
+                        versionString = major + "." + minor + "." + micro + " " + status.toString() +
+                                " " + statusVersion;
+                    }
+                    else {
+                        versionString = major + "." + minor + "." + micro + " " + status.toString();
+                    }
+                }
+            }
+            else {
+                versionString = major + "." + minor + "." + micro;
+            }
+    		
+    	}
         return versionString;
     }
 
@@ -136,7 +165,7 @@ public class Version {
     }
 
     /**
-     * Obtain the status relase number for this product. For example, if
+     * Obtain the status release number for this product. For example, if
      * the release status is <strong>alpha</strong> the release may be <strong>5</strong>
      * resulting in a release status of <strong>Alpha 5</strong>.
      *
@@ -164,4 +193,22 @@ public class Version {
             return status;
         }
     }
+    
+    /**
+     * Convenience method for comparing versions
+     * 
+     * @param otherVersion a verion to comapr against
+     */
+    public boolean isNewerThan(Version otherVersion) {
+    	return this.compareTo(otherVersion) > 0;
+    }
+
+	@Override
+	public int compareTo(Version that) {
+		
+		long thisVersion = (this.getMicro()*10) + (this.getMinor()*1000) + (this.getMajor()*100000);
+		long thatVersion = (that.getMicro()*10) + (that.getMinor()*1000) + (that.getMajor()*100000);
+		
+		return thisVersion == thatVersion ? 0 : thisVersion > thatVersion ? 1 : -1;
+	}
 }

src/test/java/org/jivesoftware/util/VersionTest.java

+package org.jivesoftware.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+
+import org.jivesoftware.util.Version.ReleaseStatus;
+import org.junit.Test;
+
+public class VersionTest {
+
+	@Test
+	public void testVersionWithInitializingConstructor() {
+		Version test = new Version(3, 2, 1, ReleaseStatus.Beta, 4);
+		
+		assertEquals(3, test.getMajor());
+		assertEquals(2, test.getMinor());
+		assertEquals(1, test.getMicro());
+		
+		assertEquals(ReleaseStatus.Beta, test.getStatus());
+		assertEquals(4, test.getStatusVersion());
+		
+		assertEquals("3.2.1 Beta 4", test.getVersionString());
+	}
+
+	@Test
+	public void testVersionWithRegularStringConstructor() {
+		Version test = new Version("1.2.3");
+		
+		assertEquals(1, test.getMajor());
+		assertEquals(2, test.getMinor());
+		assertEquals(3, test.getMicro());
+		
+		assertEquals(ReleaseStatus.Release, test.getStatus());
+		assertEquals(-1, test.getStatusVersion());
+		
+		assertEquals("1.2.3", test.getVersionString());
+	}
+
+	@Test
+	public void testVersionWithNullStringConstructor() {
+		Version test = new Version(null);
+		
+		assertEquals(0, test.getMajor());
+		assertEquals(0, test.getMinor());
+		assertEquals(0, test.getMicro());
+		
+		assertEquals(ReleaseStatus.Release, test.getStatus());
+		assertEquals(-1, test.getStatusVersion());
+		
+		assertEquals("0.0.0", test.getVersionString());
+	}
+
+	@Test
+	public void testVersionComparisons() {
+		
+		Version test123 = new Version("1.2.3");
+		Version test321 = new Version("3.2.1");
+		Version test322 = new Version("3.2.2");
+		Version test333 = new Version("3.3.3");
+		Version test300 = new Version("3.0.0");
+		Version test3100 = new Version("3.10.0");
+		Version test29999 = new Version("2.99.99");
+		
+		assertEquals(-1, test123.compareTo(test321));
+		assertEquals(0, test123.compareTo(test123));
+		assertEquals(1, test321.compareTo(test123));
+		
+		assertTrue(test322.isNewerThan(test321));
+		assertFalse(test322.isNewerThan(test333));
+		assertFalse(test300.isNewerThan(test321));
+		assertTrue(test3100.isNewerThan(test333));
+		assertTrue(test3100.isNewerThan(test29999));
+		assertTrue(test300.isNewerThan(test29999));
+		
+	}
+
+}