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