Ori Liel has uploaded a new change for review.

Change subject: restapi: Fix possible version mismatch (#1206068)
......................................................................

restapi: Fix possible version mismatch (#1206068)

GET on the root of the API returns:

<api>
  .
  .
  <version>...</version>
  <full_version>...</full_version>
</api>

The first is taken from vdc_options-->VdcVersion.
The second is taken from vdc_options-->ProductRPMVersion.

Sometimes the second value is more accurate and there can
be a mismatch. This patch attempst to parse ProductRPMVersion
and use it for both fields. If parsing fails, VdcVersion is
used as fall-back.

Bug-Url: http://bugzilla.redhat.com/1206068
Change-Id: Id9d6e96e503f258df550201bbcac7cc44061ce78
Signed-off-by: Ori Liel <[email protected]>
---
M 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendApiResource.java
M 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/util/VersionHelper.java
M 
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendApiResourceTest.java
A 
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/util/VersionHelperTest.java
4 files changed, 117 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/06/39906/1

diff --git 
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendApiResource.java
 
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendApiResource.java
index fe349bf..de28b9f 100644
--- 
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendApiResource.java
+++ 
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendApiResource.java
@@ -46,6 +46,7 @@
 import org.ovirt.engine.api.model.StorageDomains;
 import org.ovirt.engine.api.model.Users;
 import org.ovirt.engine.api.model.VMs;
+import org.ovirt.engine.api.model.Version;
 import org.ovirt.engine.api.resource.ApiResource;
 import org.ovirt.engine.api.restapi.types.DateMapper;
 import org.ovirt.engine.api.restapi.util.ErrorMessageHelper;
@@ -269,8 +270,9 @@
     }
 
     private RSDL addSystemVersion(RSDL rsdl) {
-            
rsdl.setVersion(VersionHelper.parseVersion(getConfigurationValueDefault(String.class,
 ConfigurationValues.VdcVersion)));
-            return rsdl;
+        String productVersion = getConfigurationValueDefault(String.class, 
ConfigurationValues.ProductRPMVersion);
+        rsdl.setVersion(getVersion(productVersion));
+        return rsdl;
     }
 
     public synchronized RSDL getRSDL() throws ClassNotFoundException, 
IOException {
@@ -291,10 +293,19 @@
         
api.getProductInfo().setName(obrand.getMessage("obrand.backend.product"));
         
api.getProductInfo().setVendor(obrand.getMessage("obrand.backend.vendor"));
         api.getProductInfo().setFullVersion(productVersion);
-        
api.getProductInfo().setVersion(VersionHelper.parseVersion(getConfigurationValueDefault(String.class,
 ConfigurationValues.VdcVersion)));
+        api.getProductInfo().setVersion(getVersion(productVersion));
         return api;
     }
 
+    /**
+     * Try to parse 'ProductRPMVersion' to get the most accurate version. If 
parsing fails, fall back to the less
+     * accurate: 'VdcVersion" config value.
+     */
+    private Version getVersion(String rpmVersion) {
+        return VersionHelper.parseRpmVersion(rpmVersion) != null ? 
VersionHelper.parseRpmVersion(rpmVersion)
+                : 
VersionHelper.parseVersion(getConfigurationValueDefault(String.class, 
ConfigurationValues.VdcVersion));
+    }
+
     private HashMap<String, Integer> getSystemStatistics() {
         try {
             VdcQueryReturnValue result = 
runQuery(VdcQueryType.GetSystemStatistics,
diff --git 
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/util/VersionHelper.java
 
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/util/VersionHelper.java
index 9c1fe71..f7b91b7 100644
--- 
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/util/VersionHelper.java
+++ 
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/util/VersionHelper.java
@@ -1,8 +1,14 @@
 package org.ovirt.engine.api.restapi.util;
 
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 import org.ovirt.engine.api.model.Version;
 
 public class VersionHelper {
+
+    public static final String RPM_REG_EX = "^\\d+\\.\\d+\\.\\d+";
+
     /**
      * Parse String to SystemVersion
      *
@@ -29,4 +35,14 @@
         return v1.getMajor() != null && v1.getMajor().equals(v2.getMajor()) &&
                 v1.getMinor() != null && v1.getMinor().equals(v2.getMinor());
     }
+
+    public static Version parseRpmVersion(String rpmVersion) {
+        Pattern pattern = Pattern.compile(RPM_REG_EX);
+        Matcher matcher = pattern.matcher(rpmVersion);
+        if (matcher.find()) {
+            return parseVersion(matcher.group(0));
+        } else {
+            return null;
+        }
+    }
 }
diff --git 
a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendApiResourceTest.java
 
b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendApiResourceTest.java
index cf72733..434dfa3 100644
--- 
a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendApiResourceTest.java
+++ 
b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendApiResourceTest.java
@@ -30,6 +30,7 @@
 import org.ovirt.engine.api.restapi.util.SessionHelper;
 import org.ovirt.engine.core.common.interfaces.BackendLocal;
 import org.ovirt.engine.core.common.mode.ApplicationMode;
+import org.ovirt.engine.core.common.queries.ConfigurationValues;
 import org.ovirt.engine.core.common.queries.GetConfigurationValueParameters;
 import org.ovirt.engine.core.common.queries.GetSystemStatisticsQueryParameters;
 import org.ovirt.engine.core.common.queries.VdcQueryParametersBase;
@@ -282,6 +283,23 @@
         doTestGlusterOnlyGet();
     }
 
+    @Test
+    /**
+     * Tests that get() works when ProductRPMVersion is unusable
+     * (and then VdcVersion should be used).
+     */
+    public void testGetVersion() {
+        
expect(current.get(ApplicationMode.class)).andReturn(ApplicationMode.AllModes).anyTimes();
+        resource.setUriInfo(setUpUriInfo(URI_BASE + "/", relationships));
+        setUpGetSystemVersionExpectations("1.3e.33");
+        VdcQueryReturnValue queryResult = 
createMock(VdcQueryReturnValue.class);
+        expect(backend.runQuery(eq(VdcQueryType.GetConfigurationValue), 
getVdcVersionParam())).andReturn(queryResult);
+        expect(queryResult.getSucceeded()).andReturn(true).anyTimes();
+        
expect(queryResult.getReturnValue()).andReturn(SYSTEM_VERSION).anyTimes();
+        setUpGetSystemStatisticsExpectations();
+        verifyResponse(resource.get());
+    }
+
     protected void doTestGet(ApplicationMode appMode) {
         setupExpectations(appMode, relationships);
         verifyResponse(resource.get());
@@ -290,7 +308,7 @@
     private void setupExpectations(ApplicationMode appMode, String[] 
relationships) {
         
expect(current.get(ApplicationMode.class)).andReturn(appMode).anyTimes();
         resource.setUriInfo(setUpUriInfo(URI_BASE + "/", relationships));
-        setUpGetSystemVersionExpectations();
+        setUpGetSystemVersionExpectations(SYSTEM_VERSION);
         setUpGetSystemStatisticsExpectations();
     }
 
@@ -348,7 +366,6 @@
         assertEquals(MAJOR,    
api.getProductInfo().getVersion().getMajor().intValue());
         assertEquals(MINOR,    
api.getProductInfo().getVersion().getMinor().intValue());
         assertEquals(BUILD,    
api.getProductInfo().getVersion().getBuild().intValue());
-        assertEquals(REVISION, 
api.getProductInfo().getVersion().getRevision().intValue());
 
         assertNotNull(api.getSummary());
         assertEquals(TOTAL_VMS,              
api.getSummary().getVMs().getTotal());
@@ -381,7 +398,6 @@
         assertEquals(MAJOR, 
api.getProductInfo().getVersion().getMajor().intValue());
         assertEquals(MINOR, 
api.getProductInfo().getVersion().getMinor().intValue());
         assertEquals(BUILD, 
api.getProductInfo().getVersion().getBuild().intValue());
-        assertEquals(REVISION, 
api.getProductInfo().getVersion().getRevision().intValue());
 
         assertNotNull(api.getSummary());
         assertEquals(TOTAL_HOSTS, api.getSummary().getHosts().getTotal());
@@ -438,15 +454,13 @@
         return uriInfo;
     }
 
-    protected void setUpGetSystemVersionExpectations() {
+    protected void setUpGetSystemVersionExpectations(String rpmVersion) {
         VdcQueryReturnValue queryResult = 
createMock(VdcQueryReturnValue.class);
 
-        expect(backend.runQuery(eq(VdcQueryType.GetConfigurationValue), 
queryVdcVersionParams())).andReturn(queryResult);
-        expect(backend.runQuery(eq(VdcQueryType.GetConfigurationValue),
-                queryProductRPMVersionParams())).andReturn(queryResult);
+        expect(backend.runQuery(eq(VdcQueryType.GetConfigurationValue), 
getProductRPMVersionParam())).andReturn(queryResult);
 
         expect(queryResult.getSucceeded()).andReturn(true).anyTimes();
-        
expect(queryResult.getReturnValue()).andReturn(SYSTEM_VERSION).anyTimes();
+        expect(queryResult.getReturnValue()).andReturn(rpmVersion).anyTimes();
     }
 
     protected void setUpGetSystemStatisticsExpectations() {
@@ -460,16 +474,16 @@
         replayAll();
     }
 
-    protected VdcQueryParametersBase queryProductRPMVersionParams() {
+    protected VdcQueryParametersBase getProductRPMVersionParam() {
         return eqQueryParams(GetConfigurationValueParameters.class,
-                             new String[] { "SessionId"},
-                             new Object[] { getSessionId() });
+                new String[] { "SessionId", "ConfigValue" },
+                new Object[] { getSessionId(), 
ConfigurationValues.ProductRPMVersion });
     }
 
-    protected VdcQueryParametersBase queryVdcVersionParams() {
+    protected VdcQueryParametersBase getVdcVersionParam() {
         return eqQueryParams(GetConfigurationValueParameters.class,
-                             new String[] { "SessionId"},
-                             new Object[] { getSessionId() });
+                new String[] { "SessionId", "ConfigValue" },
+                new Object[] { getSessionId(), ConfigurationValues.VdcVersion 
});
     }
 
     protected VdcQueryParametersBase queryParams() {
diff --git 
a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/util/VersionHelperTest.java
 
b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/util/VersionHelperTest.java
new file mode 100644
index 0000000..678a17e
--- /dev/null
+++ 
b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/util/VersionHelperTest.java
@@ -0,0 +1,59 @@
+package org.ovirt.engine.api.restapi.util;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+import org.junit.Test;
+import org.ovirt.engine.api.model.Version;
+
+public class VersionHelperTest {
+
+    @Test
+    public void testRpmVersionParsable1() {
+        Version version = VersionHelper.parseRpmVersion("2.4.5");
+        assertNotNull(version);
+        Version version2 = new Version();
+        version2.setMajor(2);
+        version2.setMinor(4);
+        version2.setRevision(5);
+        assertTrue(VersionHelper.equals(version, version2));
+    }
+
+    @Test
+    public void testRpmVersionParsable2() {
+        Version version = VersionHelper.parseRpmVersion("2.4.5dsfas!d");
+        assertNotNull(version);
+        Version version2 = new Version();
+        version2.setMajor(2);
+        version2.setMinor(4);
+        version2.setRevision(5);
+        assertTrue(VersionHelper.equals(version, version2));
+    }
+
+    @Test
+    public void testRpmVersionParsable3() {
+        Version version = VersionHelper.parseRpmVersion("234.422.5dsfas!d");
+        assertNotNull(version);
+        Version version2 = new Version();
+        version2.setMajor(234);
+        version2.setMinor(422);
+        version2.setRevision(5);
+        assertTrue(VersionHelper.equals(version, version2));
+    }
+
+    @Test
+    public void testRpmVersionNotParsable1() {
+        assertNull(VersionHelper.parseRpmVersion("a2.4.5"));
+    }
+
+    @Test
+    public void testRpmVersionNotParsable2() {
+        assertNull(VersionHelper.parseRpmVersion("2.4..5"));
+    }
+
+    @Test
+    public void testRpmVersionNotParsable3() {
+        assertNull(VersionHelper.parseRpmVersion("2.f4.5"));
+    }
+}


-- 
To view, visit https://gerrit.ovirt.org/39906
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id9d6e96e503f258df550201bbcac7cc44061ce78
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ori Liel <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to