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

----- Original Message ----- > From: "Juan Hernández" <jhernand(a)redhat.com&gt; > To: "Vojtech Szocs" <vszocs(a)redhat.com&gt; > Cc: devel(a)ovirt.org, "Michael Pasternak" <mishka8520(a)yahoo.com&gt; > Sent: Tuesday, December 2, 2014 10:02:36 AM > Subject: Re: [ovirt-devel] Some ideas on oVirt Java SDK > > On 12/01/2014 06:36 PM, Vojtech Szocs wrote: >> >> >> ----- Original Message ----- >>> From: "Juan Hernández" <jhernand(a)redhat.com&gt; >>> To: "Vojtech Szocs" <vszocs(a)redhat.com&gt; >>> Cc: devel(a)ovirt.org >>> Sent: Monday, December 1, 2014 4:24:45 PM >>> Subject: Re: [ovirt-devel] Some ideas on oVirt Java SDK >>> >>> On 12/01/2014 04:13 PM, Vojtech Szocs wrote: >>>> >>>> >>>> ----- Original Message ----- >>>>> From: "Juan Hernández" <jhernand(a)redhat.com&gt; >>>>> To: "Michael Pasternak" <mishka8520(a)yahoo.com&gt;, "Vojtech Szocs" >>>>> <vszocs(a)redhat.com&gt;, devel(a)ovirt.org >>>>> Sent: Monday, December 1, 2014 9:54:51 AM >>>>> Subject: Re: [ovirt-devel] Some ideas on oVirt Java SDK >>>>> >>>>> On 11/30/2014 12:26 PM, Michael Pasternak wrote: >>>>>> Hey Vojtech, >>>>>> >>>>>> How are you?, please see my reply inline. >>>>>> >>>>>> >>>>>> >>>>>> On Friday, November 28, 2014 5:26 PM, Vojtech Szocs >>>>>> <vszocs(a)redhat.com <mailto:vszocs@redhat.com>> wrote: >>>>>> >>>>>> >>>>>> 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: >>>>>> >>>>>> Runtime.getRuntime().exec(command) >>>>>> >>>>>> Why not simply use existing Maven plugins to invoke XJC? >>>>>> - either: https://github.com/highsource/maven-jaxb2-plugin >>>>>> <https://github.com/highsource/maven-jaxb2-plugin> >>>>>> >>>>>> >>>>>> [MP] sdk was using jaxb to begin with, it was replaced with XJC just >>>>>> recently, >>>>>> btw Juan, what was the motivation behind this? >>>>> >>>>> This didn't change, the use of "xjc" is there since commit 95a25a4, Nov >>>>> 12 2012. >>>>> >>>>> Note that using Maven for this isn't as simple as it may look. The >>>>> development model of the SDK is that the maven build does *not* generate >>>>> any code, it just builds what has been manually generated previously. >>>> >>>> To clarify, my question was meant for "ovirt-engine-sdk-java-codegen" >>>> project and its org.ovirt.engine.sdk.codegen.Main class that produces >>>> Java classes out of XSD as part of XsdCodegen.generate() method. >>>> >>>> But if XsdCodegen invokes XJC programatically, what is the purpose of: >>>> >>>> org.jvnet.jaxb2.maven2:maven-jaxb22-plugin:generate >>>> >>>> in "ovirt-engine-sdk-java-codegen" project's pom.xml? >>>> >>>> Is it related to what XsdCodegen is doing? >>>> >>> >>> The code generator invokes "xjc" directly in order to generate from the >>> XML schema the code that will eventually be part of the generated SDK. >>> In order to do its work it needs to parse the RSDL metadata, and for >>> that it uses JAXB and classes generated from the XML schema. Those >>> classes are generated as part of the build process of the code >>> generator. So the XML schema is converted into Java classes twice: once >>> for the internal use of the generator (during build time of the >>> generator), and another time for the generated SDK (during run time of >>> the generator). This is convenient in order to avoid dependencies >>> between the generator and the SDK. >> >> Thanks for clarification. >> >> IMHO calling XJC just to generate JAXB code that parses RSDL data >> is an overkill. In oVirtJS GWT wrapper project, I'm parsing RSDL >> as XML file directly. >> > > The "xjc" compiler isn't called directly to generate the code to parse > RSDL, it is called to generate the code of the SDK. Yes, that's what XsdCodegen is doing. > > The code that the generator uses internally is generated using the Maven > plugin (which in turn calls "xjc"). Yes, that's what maven-jaxb22-plugin is doing in ovirt-engine-sdk-java-codegen/pom.xml file. > > We could argue about the convenience of using JAXB or manually parsing > the RSDL, but it won't take us anywhere. If you find it convenient > parsing it directly just do it. > >> Conceptually, RSDL definition shouldn't even be inside api.xsd: >> >> <xs:element name="rsdl" type="RSDL"/> >> >> Above ^^ is unrelated to REST entity schema, it describes RSDL >> schema. It's inside api.xsd just to parse RSDL via JAXB. Entity >> schema and RSDL schema are two separate things. >> >> I suggest to split above into separate file, i.e. rsdl.xsd >> and reference shared types (like "Version") from api.xsd. >> This would be the proper separation these two schemas. >> > > The RSDL is just one resource offered by the RESTAPI, like a VM or a > host. It supports multiple formats (XML, JSON, etc) and it is consumed > by clients. So its entities, like any other entity, must be described in > the XML schema. We only have one XML schema, and we can split it as it > would break backwards compatibility. In my opinion, VM/host/etc. are resources representing business entities, while RSDL is a description (meta-data) of all operations (essentially links) supported on these entities. It makes sense to HTTP GET/POST/etc. a VM/host/etc. but it doesn't make sense to do that for RSDL. RSDL (as defined in api.xsd) isn't a proper resource from REST API point of view, in my opinion. In api.xsd I don't see any entity-specific type utilize RSDL type. Therefore I assume that RSDL in api.xsd just serves the purpose of having the convenience to utilize JAXB RI to parse RSDL XML file. Ideally, there would be two files: - api.xsd (without RSDL type) - rsdl.xsd -> containing <xs:element name="rsdl" type="RSDL"/> and related stuff During XJC execution both XSD files above would be passed, current behavior would be preserved. This is what I proposed in my previous email. However, I'm not REST API maintainer so this was just a suggestion that I wanted to elaborate on.

> >>> >>>>> >>>>>> (REST api uses jaxb as well so we used to have 1x1 mappings) >>>>>> >>>>>> >>>>>> - or: http://mojo.codehaus.org/jaxb2-maven-plugin/ >>>>>> <http://mojo.codehaus.org/jaxb2-maven-plugin/> >>>>>> >>>>>> >>>>>> [MP] same. >>>>>> >>>>>> >>>>>> Second, and most importantly, what's the point of having "group" >>>>>> entities? I'll give an example - api.xsd contains this: >>>>>> >>>>>> <xs:complexType name="DataCenters"> >>>>>> <xs:complexContent> >>>>>> <xs:extension base="BaseResources"> >>>>>> <xs:sequence> >>>>>> <xs:annotation> >>>>>> <xs:appinfo> >>>>>> <jaxb:property name="DataCenters"/> >>>>>> </xs:appinfo> >>>>>> </xs:annotation> >>>>>> <xs:element ref="data_center" minOccurs="0" >>>>>> maxOccurs="unbounded"/> >>>>>> </xs:sequence> >>>>>> </xs:extension> >>>>>> </xs:complexContent> >>>>>> </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 { >>>>>> >>>>>> @XmlElement(name = "data_center") >>>>>> protected List<DataCenter> dataCenters; >>>>>> >>>>>> public List<DataCenter> getDataCenters() { >>>>>> if (dataCenters == null) { >>>>>> dataCenters = new ArrayList<DataCenter>(); >>>>>> } >>>>>> return this.dataCenters; >>>>>> } >>>>>> >>>>>> public boolean isSetDataCenters() { >>>>>> return ((this.dataCenters!= >>>>>> null)&&(!this.dataCenters.isEmpty())); >>>>>> } >>>>>> >>>>>> public void unsetDataCenters() { >>>>>> this.dataCenters = null; >>>>>> } >>>>>> >>>>>> } >>>>>> >>>>>> Instead, we could use @XmlElementWrapper as described in [1] >>>>>> to avoid generating "group" entities altogether. >>>>>> >>>>>> [1] https://github.com/dmak/jaxb-xew-plugin >>>>>> <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] Well, 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 >>>>>> know architecture) >>>>>> 2. "decorator" is a well known and commonly used java design pattern >>>>>> 3. having one resource type serving all collections would create a >>>>>> bottleneck >>>>>> (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), >>>>>> >>>>>> after all the purpose of sdk is being java client serving application >>>>>> in >>>>>> "Java" way >>>>>> (i.e type-safe + well bounded interface), while JS use-cases & >>>>>> paradigms >>>>>> are totally >>>>>> different, just consider: >>>>>> >>>>>> [1] java-sdk stile >>>>>> >>>>>> Disk snapshotDisk = >>>>>> api.getVms().get('my-vm').getSnapshots().get('my-snapshot').getDisks().get('my-disk') >>>>>> >>>>>> [2] JS style you propose >>>>>> >>>>>> Disk snapshotDisk = getCollections().get(new Params[] { Disk.class, >>>>>> 'my-vm', 'my-snapshot', 'my-disk'}) >>>>>> >>>>>> notice: >>>>>> ===== >>>>>> >>>>>> in [2] you have a bunch of parameters disconnected form any context >>>>>> where order >>>>>> is *important* (other way you heuristic guesses what user meaning by >>>>>> these params won't work), >>>>>> obviously it's fragile and error prone, >>>>>> >>>>>> while [1] is readable, well bounded, defending it's consumers from >>>>>> potentials errors >>>>>> (exactly what SDK should look like), >>>>>> >>>>>> hope it helps. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> On Friday, November 28, 2014 5:26 PM, Vojtech Szocs <vszocs(a)redhat.com&gt; >>>>>> wrote: >>>>>> >>>>>> >>>>>> 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: >>>>>> >>>>>> Runtime.getRuntime().exec(command) >>>>>> >>>>>> Why not simply use existing Maven plugins to invoke XJC? >>>>>> - either: https://github.com/highsource/maven-jaxb2-plugin >>>>>> - or: http://mojo.codehaus.org/jaxb2-maven-plugin/ >>>>>> >>>>>> Second, and most importantly, what's the point of having "group" >>>>>> entities? I'll give an example - api.xsd contains this: >>>>>> >>>>>> <xs:complexType name="DataCenters"> >>>>>> <xs:complexContent> >>>>>> <xs:extension base="BaseResources"> >>>>>> <xs:sequence> >>>>>> <xs:annotation> >>>>>> <xs:appinfo> >>>>>> <jaxb:property name="DataCenters"/> >>>>>> </xs:appinfo> >>>>>> </xs:annotation> >>>>>> <xs:element ref="data_center" minOccurs="0" >>>>>> maxOccurs="unbounded"/> >>>>>> </xs:sequence> >>>>>> </xs:extension> >>>>>> </xs:complexContent> >>>>>> </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 { >>>>>> >>>>>> @XmlElement(name = "data_center") >>>>>> protected List<DataCenter> dataCenters; >>>>>> >>>>>> public List<DataCenter> getDataCenters() { >>>>>> if (dataCenters == null) { >>>>>> dataCenters = new ArrayList<DataCenter>(); >>>>>> } >>>>>> return this.dataCenters; >>>>>> } >>>>>> >>>>>> public boolean isSetDataCenters() { >>>>>> return ((this.dataCenters!= >>>>>> null)&&(!this.dataCenters.isEmpty())); >>>>>> } >>>>>> >>>>>> public void unsetDataCenters() { >>>>>> this.dataCenters = null; >>>>>> } >>>>>> >>>>>> } >>>>>> >>>>>> 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. >>>>>> >>>>>> In oVirtJS GWT wrapper, we'll avoid above complication through >>>>>> single resource collection type (having common methods like >>>>>> get(id), list() etc) for all resources. >>>>>> >>>>>> Regards, >>>>>> Vojtech >>>