Piotr Kliczewski has uploaded a new change for review.

Change subject: rest: Validation of the protocol values
......................................................................

rest: Validation of the protocol values

We did not check protocol values provided so update of the host with
wrong protocol string failed on backend whereas it should fail during
initial validation.

Change-Id: I666b5469bcaf2bd00f793ad4320c3f37505d49b0
Signed-off-by: pkliczewski <piotr.kliczew...@gmail.com>
Bug-Url: https://bugzilla.redhat.com/1170125
---
A 
backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/HostProtocol.java
M 
backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
M 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendCapabilitiesResource.java
M 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/HostValidator.java
M 
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/HostMapper.java
M 
backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/HostMapperTest.java
6 files changed, 81 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/98/35898/1

diff --git 
a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/HostProtocol.java
 
b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/HostProtocol.java
new file mode 100644
index 0000000..7a17c6d
--- /dev/null
+++ 
b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/HostProtocol.java
@@ -0,0 +1,19 @@
+package org.ovirt.engine.api.model;
+
+public enum HostProtocol {
+    XML,
+    STOMP,
+    AMQP;
+
+    public String value() {
+        return name().toLowerCase();
+    }
+
+    public static HostProtocol fromValue(String value) {
+        try {
+            return valueOf(value.toUpperCase());
+        } catch (IllegalArgumentException e) {
+            return null;
+        }
+    }
+}
diff --git 
a/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
 
b/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
index a025336..c0ecf13 100644
--- 
a/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
+++ 
b/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
@@ -613,6 +613,7 @@
           <xs:element ref="creation_states" minOccurs="0"/>
           <xs:element ref="power_management_states" minOccurs="0"/>
           <xs:element ref="host_states" minOccurs="0"/>
+          <xs:element ref="host_protocols" minOccurs="0"/>
           <xs:element ref="host_non_operational_details" minOccurs="0"/>
           <xs:element ref="network_states" minOccurs="0"/>
           <xs:element ref="storage_domain_states" minOccurs="0"/>
@@ -1819,6 +1820,20 @@
     </xs:sequence>
   </xs:complexType>
 
+  <xs:element name="host_protocols" type="HostProtocols"/>
+
+  <xs:complexType name="HostProtocols">
+    <xs:sequence>
+      <xs:element name="host_protocol" type="xs:string" minOccurs="0" 
maxOccurs="unbounded">
+        <xs:annotation>
+          <xs:appinfo>
+            <jaxb:property name="HostProtocols"/>
+          </xs:appinfo>
+        </xs:annotation>
+      </xs:element>
+    </xs:sequence>
+  </xs:complexType>
+
   <xs:element name="host_non_operational_details" 
type="HostNonOperationalDetails"/>
 
   <xs:complexType name="HostNonOperationalDetails">
diff --git 
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendCapabilitiesResource.java
 
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendCapabilitiesResource.java
index 13d091e..b8a9d21 100644
--- 
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendCapabilitiesResource.java
+++ 
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendCapabilitiesResource.java
@@ -44,6 +44,8 @@
 import org.ovirt.engine.api.model.HookStatus;
 import org.ovirt.engine.api.model.HostNICStates;
 import org.ovirt.engine.api.model.HostNonOperationalDetails;
+import org.ovirt.engine.api.model.HostProtocol;
+import org.ovirt.engine.api.model.HostProtocols;
 import org.ovirt.engine.api.model.HostStates;
 import org.ovirt.engine.api.model.HostStatus;
 import org.ovirt.engine.api.model.IpVersions;
@@ -246,6 +248,7 @@
         addStorageDomaintStates(version, StorageDomainStatus.values());
         addPowerManagementStateses(version, PowerManagementStatus.values());
         addHostStates(version, HostStatus.values());
+        addHostProtocols(version, HostProtocol.values());
         addHostNonOperationalDetails(version, NonOperationalReason.values());
         addNetworkStates(version, NetworkStatus.values());
         addTemplateStates(version, TemplateStatus.values());
@@ -742,6 +745,13 @@
         }
     }
 
+    private void addHostProtocols(VersionCaps version, HostProtocol[] values) {
+        version.setHostProtocols(new HostProtocols());
+        for (HostProtocol protocol: values) {
+            
version.getHostProtocols().getHostProtocols().add(protocol.value());
+        }
+    }
+
     private void addHostNonOperationalDetails(VersionCaps version, 
NonOperationalReason[] values) {
         version.setHostNonOperationalDetails(new HostNonOperationalDetails());
         for (NonOperationalReason reason : values) {
diff --git 
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/HostValidator.java
 
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/HostValidator.java
index 27817df..ee1d682 100644
--- 
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/HostValidator.java
+++ 
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/HostValidator.java
@@ -5,6 +5,7 @@
 import org.ovirt.engine.api.model.Host;
 import org.ovirt.engine.api.model.PmProxy;
 import org.ovirt.engine.api.model.PmProxyType;
+import org.ovirt.engine.api.model.HostProtocol;
 
 @ValidatedClass(clazz = Host.class)
 public class HostValidator implements Validator<Host> {
@@ -22,5 +23,8 @@
         if (host.isSetSsh()) {
             sshValidator.validateEnums(host.getSsh());
         }
+        if (host.isSetProtocol()) {
+            validateEnum(HostProtocol.class, host.getProtocol(), true);
+        }
     }
 }
diff --git 
a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/HostMapper.java
 
b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/HostMapper.java
index a884eb4..5f118e5 100644
--- 
a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/HostMapper.java
+++ 
b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/HostMapper.java
@@ -18,6 +18,7 @@
 import org.ovirt.engine.api.model.Hook;
 import org.ovirt.engine.api.model.Hooks;
 import org.ovirt.engine.api.model.Host;
+import org.ovirt.engine.api.model.HostProtocol;
 import org.ovirt.engine.api.model.HostStatus;
 import org.ovirt.engine.api.model.HostType;
 import org.ovirt.engine.api.model.HostedEngine;
@@ -84,7 +85,7 @@
             entity.setPort(DEFAULT_VDSM_PORT);
         }
         if (model.isSetProtocol()) {
-            entity.setProtocol(VdsProtocol.fromValue(model.getProtocol()));
+            map(model.getProtocol(), entity);
         }
         if (model.isSetSsh()) {
             map(model.getSsh(), entity);
@@ -355,7 +356,8 @@
         if (entity.getPort() > 0) {
             model.setPort(entity.getPort());
         }
-        model.setProtocol(map(entity.getProtocol(), (String) null));
+        HostProtocol protocol = map(entity.getProtocol(), null);
+        model.setProtocol(protocol != null ? protocol.value() : null);
         HostStatus status = map(entity.getStatus(), null);
         model.setStatus(StatusUtils.create(status));
         if (status==HostStatus.NON_OPERATIONAL) {
@@ -823,26 +825,47 @@
         return result;
     }
 
-    @Mapping(from = 
org.ovirt.engine.core.common.businessentities.VdsProtocol.class, to = 
String.class)
-    public static String 
map(org.ovirt.engine.core.common.businessentities.VdsProtocol protocol, String 
template) {
-        String result = null;
+    @Mapping(from = VdsProtocol.class, to = HostProtocol.class)
+    public static HostProtocol map(VdsProtocol protocol, HostProtocol 
template) {
+        HostProtocol result = null;
         if (protocol != null) {
             switch (protocol) {
                 case STOMP:
-                    result = VdsProtocol.STOMP.value();
+                    result =  HostProtocol.STOMP;
                     break;
                 case AMQP:
-                    result = VdsProtocol.AMQP.value();
+                    result = HostProtocol.AMQP;
                     break;
                 case XML:
                 default:
-                    result = VdsProtocol.XML.value();
+                    result = HostProtocol.XML;
                     break;
             }
         }
         return result;
     }
 
+    @Mapping(from = String.class, to = VdsStatic.class)
+    public static VdsStatic map(String protocol, VdsStatic template) {
+        VdsStatic entity = template != null ? template : new VdsStatic();
+        HostProtocol hostProtocol = HostProtocol.fromValue(protocol);
+        VdsProtocol result = null;
+        switch (hostProtocol) {
+            case STOMP:
+                result =  VdsProtocol.STOMP;
+                break;
+            case AMQP:
+                result = VdsProtocol.AMQP;
+                break;
+            case XML:
+            default:
+                result = VdsProtocol.XML;
+                break;
+        }
+        entity.setProtocol(result);
+        return entity;
+    }
+
     @Mapping(from = VdsSpmStatus.class, to = SpmState.class)
     public static SpmState map(VdsSpmStatus entityStatus, SpmState template) {
         switch (entityStatus) {
diff --git 
a/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/HostMapperTest.java
 
b/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/HostMapperTest.java
index 4e8e83f..abd4990 100644
--- 
a/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/HostMapperTest.java
+++ 
b/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/HostMapperTest.java
@@ -6,6 +6,7 @@
 import org.ovirt.engine.api.model.Agent;
 import org.ovirt.engine.api.model.Agents;
 import org.ovirt.engine.api.model.Host;
+import org.ovirt.engine.api.model.HostProtocol;
 import org.ovirt.engine.api.model.HostedEngine;
 import org.ovirt.engine.api.model.PowerManagement;
 import org.ovirt.engine.api.model.SSH;
@@ -26,6 +27,7 @@
         while (from.getPort() == 0) {
             from.setPort(MappingTestHelper.rand(65535));
         }
+        
from.setProtocol(MappingTestHelper.shuffle(HostProtocol.class).value());
         from.getSpm().setPriority(3);
         return from;
     }


-- 
To view, visit http://gerrit.ovirt.org/35898
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I666b5469bcaf2bd00f793ad4320c3f37505d49b0
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.5
Gerrit-Owner: Piotr Kliczewski <piotr.kliczew...@gmail.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to