Re: [ovirt-devel] Some ideas on oVirt Java SDK

30 Nov 2014

      ------=_Part_1859450_1427808343.1417346766163
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Hey Vojtech,

How are you?, please see my reply inline.

      On Friday, November 28, 2014 5:26 PM, Vojtech Szocs <vszocs@redhat.co=
m> wrote:
  =20

 Hi guys,

since the initial (small, working & well-tested) version of oVirtJS
JavaScript SDK is finished [*], I've started working on GWT wrapper
for oVirtJS.

While analyzing/reverse-engineering oVirt Java SDK, some thoughts
came to my mind, and I wanted to share them with you.

[*] TODO(vszocs) upload new patchset with all recent changes

First, the way XJC (JAXB binding compiler that generates Java beans
out of REST XSD schema) is invoked looks a bit weird to me, as Java
SDK's XsdCodegen does this:

=C2=A0 Runtime.getRuntime().exec(command)

Why not simply use existing Maven plugins to invoke XJC?
- either: https://github.com/highsource/maven-jaxb2-plugin

[MP] sdk was using jaxb to begin with, it was replaced with XJC just recent=
ly,
btw Juan, what was the motivation behind this?=20
(REST api uses jaxb as well so we used to have 1x1 mappings)
=C2=A0
- or: http://mojo.codehaus.org/jaxb2-maven-plugin/

[MP]=C2=A0same.

Second, and most importantly, what's the point of having "group"
entities? I'll give an example - api.xsd contains this:

=C2=A0 <xs:complexType name=3D"DataCenters">
=C2=A0 =C2=A0 <xs:complexContent>
=C2=A0 =C2=A0 =C2=A0 <xs:extension base=3D"BaseResources">
=C2=A0 =C2=A0 =C2=A0 =C2=A0 <xs:sequence>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 <xs:annotation>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 <xs:appinfo>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 <jaxb:property name=
=3D"DataCenters"/>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 </xs:appinfo>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 </xs:annotation>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 <xs:element ref=3D"data_center" minOccur=
s=3D"0" maxOccurs=3D"unbounded"/>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 </xs:sequence>
=C2=A0 =C2=A0 =C2=A0 </xs:extension>
=C2=A0 =C2=A0 </xs:complexContent>
=C2=A0 </xs:complexType>

(Same as above for Hosts, Clusters, VMs, etc.)

This results in following (IMHO rather meaningless) Java class
being generated by XJC:

public class DataCenters extends BaseResources {

=C2=A0 =C2=A0 @XmlElement(name =3D "data_center")
=C2=A0 =C2=A0 protected List<DataCenter> dataCenters;

=C2=A0 =C2=A0 public List<DataCenter> getDataCenters() {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (dataCenters =3D=3D null) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dataCenters =3D new ArrayList<Dat=
aCenter>();
=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
=C2=A0 =C2=A0 =C2=A0 =C2=A0 return this.dataCenters;
=C2=A0 =C2=A0 }

=C2=A0 =C2=A0 public boolean isSetDataCenters() {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 return ((this.dataCenters!=3D null)&&(!this.dat=
aCenters.isEmpty()));
=C2=A0 =C2=A0 }

=C2=A0 =C2=A0 public void unsetDataCenters() {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 this.dataCenters =3D null;
=C2=A0 =C2=A0 }

}

Instead, we could use @XmlElementWrapper as described in [1]
to avoid generating "group" entities altogether.

[1] https://github.com/dmak/jaxb-xew-plugin

The fact that Java SDK provides decorator for each specific
resource collection (like DataCenters), instead of having ONE
resource collection type, greatly complicates overall design
and code-gen aspect.

[MP]=C2=A0Well, i guess now is speaking JS constraints ghost, am i right?,
in any case, the reasons for having decorator per collection are:

1. compliance with REST API (all SDKs and REST api are sharing same well kn=
ow architecture)
2. "decorator" is a well known and commonly used java design pattern
3. having one resource type serving all collections would create a bottlene=
ck
(well it might depend on how you implementing it, but still in my view it's=
 less convenient/readable
than dedicated collection with own context, verbs and behavior),=C2=A0
after all the purpose of sdk is being java client serving application in "J=
ava" way(i.e type-safe + well bounded interface),=C2=A0while JS use-cases &=
 paradigms are totallydifferent,=C2=A0just consider:
[1] java-sdk stile
Disk snapshotDisk =3D api.getVms().get('my-vm').getSnapshots().get('my-snap=
shot').getDisks().get('my-disk')
[2] JS style you propose
Disk snapshotDisk =3D getCollections().get(new Params[] { Disk.class, 'my-v=
m', 'my-snapshot', 'my-disk'})

notice:=C2=A0=3D=3D=3D=3D=3D
in [2] you have a bunch of parameters disconnected form any context where o=
rderis *important* (other way you heuristic guesses what user meaning by th=
ese params won't work),obviously it's fragile and error prone,
while [1] is readable, well bounded, defending it's consumers from potentia=
ls errors(exactly what SDK should look like),
hope it helps.

=20

     On Friday, November 28, 2014 5:26 PM, Vojtech Szocs <vszocs@redhat.com=
...
wrote:
  =20
...
          <xs:element ref=3D"data_center" minO=
ccurs=3D"0" maxOccurs=3D"unbounded"/><br class=3D"" style=3D"">  &n=
bsp;     </xs:sequence><br class=3D"" style=3D"">  &nb=
sp;   </xs:extension><br class=3D"" style=3D"">    <=
;/xs:complexContent><br class=3D"" style=3D"">  </xs:complexType=
><br class=3D"" style=3D""><br class=3D"" style=3D"">(Same as above for =
Hosts, Clusters, VMs, etc.)<br class=3D"" style=3D""><br class=3D"" style=
=3D"">This results in following (IMHO rather meaningless) Java class<br cla=
ss=3D"" style=3D"">being generated by XJC:<br class=3D"" style=3D""><br cla=
ss=3D"" style=3D"">public class DataCenters extends BaseResources {<br clas=
s=3D"" style=3D""><br class=3D"" style=3D"">    @XmlElement(name =
=3D "data_center")<br class=3D"" style=3D"">    protected List<=
;DataCenter> dataCenters;<br class=3D"" style=3D""><br class=3D"" style=
=3D"">    public List<DataCenter> getDataCenters() {<br cla=
ss=3D"" style=3D"">        if (dataCenters =3D=3D null)=
 {<br class=3D"" style=3D"">            dataC=
enters =3D new ArrayList<DataCenter>();<br class=3D"" style=3D"">&nbs=
...
<div class=3D"" style=3D"" id=3D"yui_3_16_0_1_1417345481659_14466"><font s=
ize=3D"4" class=3D"" style=3D"" id=3D"yui_3_16_0_1_1417345481659_14465">[MP=
] Well, i guess now is speaking JS constraints ghost, am i right?</fon=
t>,<br class=3D"" style=3D""></div><div class=3D"" style=3D"" id=3D"yui_3_1=
6_0_1_1417345481659_12513"><font size=3D"4" class=3D"" style=3D"" id=3D"yui=
_3_16_0_1_1417345481659_12514">in any case, the reasons for having decorato=
r per collection are:<br class=3D"" style=3D""><br class=3D"" style=3D""></=
font></div><div class=3D"" style=3D"" id=3D"yui_3_16_0_1_1417345481659_1251=
2"><font size=3D"4" class=3D"" style=3D"" id=3D"yui_3_16_0_1_1417345481659_=
12511">1. compliance with REST API (all SDKs and REST api are sharing same =
well know architecture)<br class=3D"" style=3D""></font></div><div class=3D=
"" style=3D"" id=3D"yui_3_16_0_1_1417345481659_12510"><font size=3D"4" clas=
s=3D"" style=3D"" id=3D"yui_3_16_0_1_1417345481659_12509">2. "decorator" is=
 a well known and commonly used java design pattern<br class=3D"" style=3D"=
"></font></div><div class=3D"" style=3D"" id=3D"yui_3_16_0_1_1417345481659_=
12508"><font size=3D"4" class=3D"" style=3D"" id=3D"yui_3_16_0_1_1417345481=
659_12507">3. having one resource type serving all collections would create=
 a bottleneck<br class=3D"" style=3D""></font></div><div class=3D"" style=
=3D"" id=3D"yui_3_16_0_1_1417345481659_12506"><font size=3D"4" class=3D"" s=
tyle=3D"" id=3D"yui_3_16_0_1_1417345481659_12505">(well it might depend on =
how you implementing it, but still in my view it's less convenient/readable=
<br class=3D"" style=3D"">than dedicated collection with own context, verbs=
 and behavior), </font></div><div class=3D"" style=3D"" id=3D"yui_3_16=
_0_1_1417345481659_12506"><font size=3D"4" class=3D"" style=3D""><br></font=
</div><font size=3D"4" class=3D"" id=3D"yui_3_16_0_1_1417345481659_12521" =
...
</div><div class=3D"" style=3D"" id=3D"yui_3_16_0_1_1417345481659_12506" d=
ir=3D"ltr"><font size=3D"4" class=3D"" style=3D"" id=3D"yui_3_16_0_1_141734=
5481659_16215">[1] java-sdk stile</font></div><div class=3D"" style=3D"" id=
=3D"yui_3_16_0_1_1417345481659_12506" dir=3D"ltr"><font size=3D"4" class=3D=
"" style=3D""><br></font></div><div class=3D"" style=3D"" id=3D"yui_3_16_0_=
1_1417345481659_12506" dir=3D"ltr"><font size=3D"4" class=3D"" style=3D"" i=
d=3D"yui_3_16_0_1_1417345481659_12981">Disk snapshotDisk =3D api.getVms().g=
et('my-vm').getSnapshots().get('my-snapshot').getDisks().get('my-disk')</fo=
nt></div><div class=3D"" style=3D"" id=3D"yui_3_16_0_1_1417345481659_12506"=
...
</div><div class=3D"" style=3D"" id=3D"yui_3_16_0_1_1417345481659_12506" d=
ir=3D"ltr"><font size=3D"4" class=3D"" style=3D"" id=3D"yui_3_16_0_1_141734=
5481659_13698">in [2] you have a bunch of parameters disconnected form any =
context where order</font></div><div class=3D"" style=3D"" id=3D"yui_3_16_0=
_1_1417345481659_12506" dir=3D"ltr"><font size=3D"4" class=3D"" style=3D"" =
id=3D"yui_3_16_0_1_1417345481659_13001">is *important* (other way you heuri=
stic guesses what user meaning by these params won't work),</font></div><di=
v class=3D"" style=3D"" id=3D"yui_3_16_0_1_1417345481659_12506" dir=3D"ltr"=
<font size=3D"4" class=3D"" style=3D"" id=3D"yui_3_16_0_1_1417345481659_14=
446">obviously it's fragile and error prone,</font></div><div class=3D"" st=
yle=3D"" id=3D"yui_3_16_0_1_1417345481659_12506" dir=3D"ltr"><font size=3D"=
4" class=3D"" style=3D""><br></font></div><div class=3D"" style=3D"" id=3D"=
yui_3_16_0_1_1417345481659_12506" dir=3D"ltr"><font size=3D"4" class=3D"" s=
tyle=3D"" id=3D"yui_3_16_0_1_1417345481659_13697">while [1] is readable, we=
ll bounded, defending it's consumers from potentials errors</font></div><di=
v class=3D"" style=3D"" id=3D"yui_3_16_0_1_1417345481659_12506" dir=3D"ltr"=
<font size=3D"4" class=3D"" style=3D"" id=3D"yui_3_16_0_1_1417345481659_18=
064">(exactly what SDK should look like),</font></div><div class=3D"" style=
=3D"" id=3D"yui_3_16_0_1_1417345481659_12506" dir=3D"ltr"><font size=3D"4" =
class=3D"" style=3D""><br></font></div><div class=3D"" style=3D"" id=3D"yui=
_3_16_0_1_1417345481659_12506" dir=3D"ltr"><font size=3D"4" class=3D"" styl=
e=3D"" id=3D"yui_3_16_0_1_1417345481659_17133">hope it helps.</font></div><=
Hi guys,