Moti Asayag has posted comments on this change.

Change subject: restapi: Implement custom properties using elements
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.ovirt.org/#/c/27316/1/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
File 
backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd:

Line 2550:       <xs:element name="name" type="xs:string" minOccurs="1" 
maxOccurs="1"/>
Line 2551:       <xs:element name="value" type="xs:string" minOccurs="1" 
maxOccurs="1"/>
Line 2552:     </xs:sequence>
Line 2553:   </xs:complexType>
Line 2554: 
shouldn't a type for the Property be created ? i.e. :
  <xs:element name="property" type="Property"/>

and the Properties element should be updated accordingly, and also be 
represented in plural form:
  <xs:complexType name="Properties">
    <xs:sequence>
      <xs:annotation>
        <xs:appinfo>
            <jaxb:property name="Properties"/>
        </xs:appinfo>
      </xs:annotation>
      <xs:element ref="property" minOccurs="1" maxOccurs="unbounded"/>
    </xs:sequence>
  </xs:complexType>

for 2 reasons:
 
1. Reusability of the "property" element (instead of defining name + type)

2. For approaching the collection within java as:  Properties.getProperties() 
vs Properties.getProperty() while this represents a collection and should have 
plural form.
Line 2555:   <xs:element name="properties" type="Properties"/>
Line 2556: 
Line 2557:   <xs:complexType name="Properties">
Line 2558:     <xs:sequence>


http://gerrit.ovirt.org/#/c/27316/1/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/utils/CustomPropertiesParser.java
File 
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/utils/CustomPropertiesParser.java:

Line 73:      *            The key:value pairs.
Line 74:      * @return A newly-created map containing the key:value pairs.
Line 75:      */
Line 76:     public static Map<String, String> toMap(Properties properties) {
Line 77:         Map<String, String> res = new HashMap<String, String>();
new HashMap<>() will do
Line 78:         for (Property property : properties.getProperty()) {
Line 79:             res.put(property.getName(), property.getValue());
Line 80:         }
Line 81:         return res;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I03173b72b4f9ab2906ca9a2989d6115efcb19264
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to