Hi Francesco, 1. yes. getMetadata should be deleted.
2. We use the EdmEntitySetInfo to build the ServiceDocument. So we used this method to get the URI. Might also be a candidate to be deleted. 3. Without arguments is fine by me. I was thinking that the whole EdmServiceMetadata interface is unnecessary. We should provide access to all entity sets, function imports and singletons at the Edm interface directly. Best Regards, Christian -----Original Message----- From: Francesco Chicchiriccò [mailto:ilgro...@apache.org] Sent: Montag, 3. März 2014 08:50 To: dev@olingo.incubator.apache.org Subject: Re: [OLINGO-169] Work in progress Hi all, a quick update. I am currently implementing, as suggested below, the current set of commons-api Edm interfaces on client side, in the same way they are implemented on server side, e.g. on top of another whole set of classes: for example org.apache.olingo.odata4.server.core.edm.provider.EdmEnumImpl implements org.apache.olingo.odata4.commons.api.edm.EdmEnumType by delegating to an org.apache.olingo.odata4.server.api.edm.provider.EnumType server object which contains the actual information about the enum type; in the same fashion org.apache.olingo.odata4.client.core.edm.EdmEnumTypeImpl is implementing the same EdmEnumType above by delegating to an org.apache.olingo.odata4.client.api.edm.xml.EnumType client interface, whose concrete instances (different for V3 and V4) are actually constructed by the Jackson XML parser. At the end of this process I should be able to obtain what discussed below, hence I am fine. I am finding many similarities between server's EdmEnumImpl and client's EdmEnumTypeImpl: is it fine to you to introduce AbstractEdmEnumTypeImpl in commons-core? (and so on, for all other Edm interfaces implementions). Besides this, I will also remove usage of client-side EdmType and EdmSimpleType classes with the commons EdmType hierarchy. I am going to open two subtasks of OLINGO-169 in JIRA for these. Few questions I have after looking closely to Edm interfaces: 1. Does EdmServiceMetadata#getMetadata (which returns an InputStream containing the metadata document) make sense on client side? I'd say no, since client code actually *builds* an EdmServiceMetadata from an InputStream. Shall we remove such method from common interface? 2. What is EdmEntitySetInfo#getEntitySetUri expected to provide? Isn't the external URI of an EntitySet provided by the service document (and only for entity sets with 'IncludeInServiceDocument' set to true)? 3. Does Edm#getEntityContainer(FullQualifiedName) make sense? CSDL specification (at the beginning of chapter 13) says: "Each metadata document used to describe an OData service MUST define exactly one entity container." I would have expected Edm#getEntityContainer (e.g. without arguments). Regards. On 28/02/2014 09:09, Klevenz, Stephan wrote: > Sorry for responding late. > > I am not sure if there is still a problem with using edm interfaces from > commons. I can share [1] of OData 2.0 library which has a deserializer for > metadata and which returns an interface. > > Setters in the interface not necessary. They are required for > implementation only. The implementation in [1] is stax based and with that > I currently don not see issues with cyclic element dependencies. > > If there is a need to change the edm interface so that a client can better > deal with it, just make a proposal. We will then cross check for the > server case and if there is no issue ... just do it. > > Regards, > Stephan > > > [1] > https://git-wip-us.apache.org/repos/asf?p=incubator-olingo-odata2.git;a=blo > b;f=odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/ep/co > nsumer/XmlMetadataConsumer.java;h=e8677a1112aa319d9b12fa615b39015e63fce1d2; > hb=HEAD > > > On 26.02.14 17:05, "Challen He" <chall...@microsoft.com> wrote: > >> For (1), yes, I agree (the new work is to re-use ODataJClient parser, and >> then convert *"old" ODataJClient Edm interfaces* result --> *Olingo 4 >> common Edm interfaces* result) >> About (2), I think so, unifying Edm interfaces is one of the goals of >> moving to ASF, and the resulted Edm interfaces should be 'strong typed' >> rather than 'weak typed' model. (we can take a look at and prioritize all >> tasks for better plan & schedule) >> >> Thanks,-Challen >> >> -----Original Message----- >> From: Francesco Chicchiriccò [mailto:ilgro...@apache.org] >> Sent: 2014年2月26日 23:45 >> To: dev@olingo.incubator.apache.org >> Subject: Re: [OLINGO-169] Work in progress >> >> Hi Challen, >> thanks for your suggestions: I was actually more willing to check >> >> (1) whether my understanding, e.g. the concept difference between "old" >> ODataJClient Edm interfaces and Olingo 4 common Edm interfaces, is correct >> (2) if so, whether it is immediately necessary to change the client XML >> Metadata parser accordingly: being not a trivial task, I want to be sure >> it is needed before committing to it >> >> Hope this clarifies. >> Regards. >> >> On 26/02/2014 16:33, Challen He wrote: >>> Hi Francesco, >>> >>> I think cross/circle reference in Edm is inevitable: not only >>> EntityContainer but also EntityType itself may refer to EntityType, e.g. >>> entity type 'Employee' has a property of 'Manager' type, and 'Manager' >>> type itself contains a collection of 'Employee'. So the job of figuring >>> out their relationships will be taken by user /client / server code if >>> common doesn't do it. >>> >>> A 2-phased solution is: phase1 - parse CSDL xml doc into a set of >>> intermediate csdl objects (they only have weak reference like the >>> mentioned String field named 'entityType'), phase2 - use csdl objects to >>> first build all edm type objects (EdmComplexType, EdmEntityType, >>> EdmEnumType..., need to handle circle references among them), then these >>> edm type objects are used to build >>> EntitySet/Container/function/action/annotation term... >>> >>> I think we can start with the assumption of no circle reference and >>> later add support for it, but in the design it looks ok for all Edm >>> interface definitions to have 'strong type' references like >>> 'EdmEntitySet' -> 'EdmEntityType', or 'EdmEntityType' -> another >>> 'EdmEntityType' ... >>> >>> (though I am not 100% sure but personally I think entity interfaces >>> with getter only will work, considering that they will be created by >>> factory/builder) >>> >>> Thanks,-Challen >>> >>> -----Original Message----- >>> From: Francesco Chicchiriccò [mailto:ilgro...@apache.org] >>> Sent: 2014年2月26日 19:14 >>> To: dev@olingo.incubator.apache.org >>> Subject: Re: [OLINGO-169] Work in progress >>> >>> Hi, >>> I have spent some more time in comparing the commons-api Edm interfaces >>> and their client-api counterparts and I am going to try to explain >>> better my concerns. >>> >>> Let's take again the sample below: >>> >>> <EntitySet Name="Products" EntityType="ODataDemo.Product"> >>> <NavigationPropertyBinding >>> Path="ODataDemo.FeaturedProduct/Advertisement" Target="Advertisements"/> >>> <NavigationPropertyBinding Path="Categories" Target="Categories"/> >>> <NavigationPropertyBinding Path="Supplier" Target="Suppliers"/> >>> <NavigationPropertyBinding Path="ProductDetail" >>> Target="ProductDetails"/> >>> </EntitySet> >>> >>> Currently, the way odata4-client-core works will barely translate this >>> piece of information from XML to Java object(s), where an instance of >>> EntitySetImpl - implementation of EntitySet [9] - has a String field >>> named 'entityType' with value 'ODataDemo.Product'. >>> >>> If you take a look at EdmEntitySet [10] instead, this interface >>> supposes that an EdmEntitySetImpl will have an EdmEntityType field named >>> 'entityType', whose value is a reference to an EdmEntityTypeImpl >>> instance for ODataDemo.Product. >>> >>> This fact implies that the metadata XML parser will need, when >>> encountering the XML attribute >>> >>> EntityType="ODataDemo.Product" >>> >>> to look at EdmEntityTypeImpl instances built so far during parsing and >>> associate the one with name 'ODataDemo.Product' to the EdmEntitySetImpl >>> under construction. >>> All that where potentially ODataDemo.Product could be not defined in >>> the current metadata document but in one of referenced metadata >>> documents. >>> >>> As you can see, this is a *huge* difference: consider, for example, >>> that the instances to link to the one under construction could not even >>> be built yet - see the NavigationPropertyBinding's target above, where >>> "Categories" could be an EntitySet or a Singleton not yet encountered in >>> the XML document being parsed. >>> >>> Before starting this considerable change effort, I'd like to understand >>> if I am right in the finding above and also if you think it is worth to >>> change the way how the client metadata XML parser currently works. >>> If I am right, in fact, we need either to give up on making the effort >>> to use the same Edm interfaces client- and server-side or the change >>> described above must be implemented. >>> >>> Another minor issue I have with commons-api Edm interfaces is that they >>> mostly lack setter methods: is it fine to introduce them? It will make >>> the XML parser work a little less hard. >>> >>> Regards. >>> >>> On 25/02/2014 17:16, Amend, Christian wrote: >>>> 1. Yes using the EdmType hierarchy should work just fine. I Am not >>>> quite sure what you mean by parsing type expressions. Do you mean that >>>> you have a Java String and would like to have an EdmPrimitiveType for >>>> this? >>>> >>>> 2. I see your concern. We had a similar question with V2 when we tried >>>> to implement a client. We could delete these info objects but would >>>> then have to provide methods like getEntitySets() (returning a list of >>>> EdmEntitySets) at the EDM directly. Otherwise how would a client access >>>> the entity sets without knowing their name first. >>>> >>>> 2.) b+c) then lets move the V3 interfaces into commons. >>>> >>>> Best Regards, >>>> Christian >>>> >>>> -----Original Message----- >>>> From: Francesco Chicchiriccò [mailto:ilgro...@apache.org] >>>> Sent: Dienstag, 25. Februar 2014 16:10 >>>> To: dev@olingo.incubator.apache.org >>>> Subject: Re: [OLINGO-169] Work in progress >>>> >>>> On 25/02/2014 14:38, Amend, Christian wrote: >>>>> Hi Francesco, >>>>> >>>>> 1. I don`t quite understand where your concern is there. Could you >>>>> please clarify? >>>> Let's put it differently: is there already something for parsing type >>>> expressions? >>>> Moreover: is there already something to represent types? I guess that >>>> EdmType[8]'s hierarchy should fit the job, right? In this case I >>>> should be able to remove [1][2][3][4][5] and use EdmType descendants. >>>> >>>>> 2.) >>>>> a) The info interfaces are there because there was no way to get a >>>>> list of all entity sets inside the edm without knowing their name. >>>>> Also this information is needed for the service document. >>>> Ok: I am not sure whether the existing interfaces are suitable, from >>>> a client perspective; consider that client will instantiate the Edm >>>> interfaces' implementations by parsing the $metadata XML content. >>>> As an example, please see how to represent something like as >>>> >>>> <EntitySet Name="Products" EntityType="ODataDemo.Product"> >>>> <NavigationPropertyBinding >>>> Path="ODataDemo.FeaturedProduct/Advertisement" >>>> Target="Advertisements"/> >>>> <NavigationPropertyBinding Path="Categories" Target="Categories"/> >>>> <NavigationPropertyBinding Path="Supplier" Target="Suppliers"/> >>>> <NavigationPropertyBinding Path="ProductDetail" >>>> Target="ProductDetails"/> </EntitySet> >>>> >>>> With EntitySet [9] this will result in (please excuse the pseudo-JSON >>>> representation): >>>> >>>> EntitySetImpl { >>>> name: "Products", >>>> entityType: "ODataDemo.Product", >>>> includeInServiceDocument: true, >>>> navigationPropertyBindings: [ >>>> NavigationPropertyBindingImpl { >>>> path: "ODataDemo.FeaturedProduct/Advertisement", >>>> target: "Advertisements" >>>> }, >>>> ... >>>> ] >>>> } >>>> >>>> i.e. this is more or less the translation of the XML snippet above. >>>> >>>> With EdmEntitySet [10] instead the representation is much more >>>> involved, mostly because metadata is something that is to be built >>>> and then pushed out as XML, rather than parsed from XML. >>>> >>>> Not sure I actually succeeded in expressing my concerns... >>>> >>>>> b) No I think the interfaces in [6] should cover all aspects of the >>>>> EDM. If not this is functionality which has to be added. >>>>> c) If they can coexist without causing conflicts I would put the V3 >>>>> interfaces in the commons API. We might not support this on server >>>>> side from the beginning but a later refactoring would be cumbersome >>>>> IMHO. Although I am open for other opinions. >>>> The current client Edm interfaces [7] are available for V3 and V4 >>>> without conflicts, so there should be no problem on having them in >>>> odata4-commons-api, without providing server implementations for V3's. >>>> >>>> Regards. >>>> >>>>> -----Original Message----- >>>>> From: Francesco Chicchiriccò [mailto:ilgro...@apache.org] >>>>> Sent: Dienstag, 25. Februar 2014 14:06 >>>>> To: dev@olingo.incubator.apache.org >>>>> Subject: [OLINGO-169] Work in progress >>>>> >>>>> Hi all, >>>>> I have reached a fair stable point for merging ODataJClient's >>>>> Metadata parsing into olingo-odata4 (in the 'olingo169' GIT branch): >>>>> now metadata parsing works either for V3 and V4 (few unit tests have >>>>> been added in olingo-odata4-client-core ). >>>>> >>>>> At this point I have two main concerns before considering the work >>>>> for >>>>> OLINGO-169 completed (and consequently merge olingo169 into master): >>>>> >>>>> 1. types >>>>> >>>>> EdmType [1] is an interface (with concrete implementations available >>>>> for >>>>> V3 and V4) whose main purpose is parsing type expressions (like as >>>>> 'Collection(ODataDemo.Product)') into a meaningful representation, >>>>> e.g. >>>>> EdmSimpleType [2], EnumType [3], ComplexType [4] or EntityType [5] >>>>> and also to provide other useful information (is this a collection? >>>>> does it have a base type?). >>>>> >>>>> I am sure that this duplicates something else in odata-olingo4. but >>>>> I need some guidance about how to replace [1][2][3][4][5]. Also, I >>>>> suppose that a better location for such items is >>>>> olingo-odata4-commons-api rather than olingo-odata4-client-api (where >>>>> they reside ATM). >>>>> Who would like to volunteer? >>>>> >>>>> >>>>> 2. Edm interfaces >>>>> >>>>> olingo-odata4-commons-api holds in [6] the current set of Edm >>>>> interfaces implemented server-side whilst I have temporarily put the >>>>> set of Edm interfaces implemented client-side in >>>>> olingo-odata4-client-api's [7] (and subpackages). >>>>> >>>>> Again, I need to understand how to reconcile and consolidate these >>>>> two sets (into olingo-odata4-commons-api of course), but there are >>>>> several aspects I don't understand: >>>>> >>>>> (a) what is the purpose of *Info interfaces (like as >>>>> EdmEntitySetInfo)? what's the usage difference with EdmEntitySet? >>>>> (b) is the set from [6] still to be completed? >>>>> (c) would it be fine to put in [6] the merge with all >>>>> interfaces from [7], including V3's or is it better to keep V3's in >>>>> olingo-odata4-client-api? >>>>> >>>>> >>>>> Thanks for your support. >>>>> Regards. >>>>> >>>>> [1] >>>>> https://git-wip-us.apache.org/repos/asf?p=incubator-olingo-odata4.gi >>>>> t >>>>> ;a=blob;f=odata4-lib/odata4-client-api/src/main/java/org/apache/olin >>>>> g >>>>> o/odata4/client/api/edm/EdmType.java;h=5a480d72cbbc707696dd358e11746 >>>>> 8 >>>>> f0f87980fb;hb=olingo169 [2] >>>>> https://git-wip-us.apache.org/repos/asf?p=incubator-olingo-odata4.gi >>>>> t >>>>> ;a=blob;f=odata4-lib/odata4-client-api/src/main/java/org/apache/olin >>>>> g >>>>> o/odata4/client/api/data/EdmSimpleType.java;h=7d44c376e89b084c6e8563 >>>>> c >>>>> 63317012284278795;hb=olingo169 [3] >>>>> https://git-wip-us.apache.org/repos/asf?p=incubator-olingo-odata4.gi >>>>> t >>>>> ;a=blob;f=odata4-lib/odata4-client-api/src/main/java/org/apache/olin >>>>> g >>>>> o/odata4/client/api/edm/EnumType.java;h=614c5e1e85a731ae80de56899e7d >>>>> 9 >>>>> d82ae846bf0;hb=olingo169 [4] >>>>> https://git-wip-us.apache.org/repos/asf?p=incubator-olingo-odata4.gi >>>>> t >>>>> ;a=blob;f=odata4-lib/odata4-client-api/src/main/java/org/apache/olin >>>>> g >>>>> o/odata4/client/api/edm/ComplexType.java;h=929d1b83f4db4c290b0681b7d >>>>> 6 >>>>> 29b9d0545f33a5;hb=olingo169 [5] >>>>> https://git-wip-us.apache.org/repos/asf?p=incubator-olingo-odata4.gi >>>>> t >>>>> ;a=blob;f=odata4-lib/odata4-client-api/src/main/java/org/apache/olin >>>>> g >>>>> o/odata4/client/api/edm/EntityType.java;h=37ebc359f0ec36e439b3a8555e >>>>> 2 >>>>> 3ebaa84506c2e;hb=olingo169 [6] >>>>> https://git-wip-us.apache.org/repos/asf?p=incubator-olingo-odata4.gi >>>>> t >>>>> ;a=tree;f=odata4-lib/odata4-commons-api/src/main/java/org/apache/oli >>>>> n >>>>> go/odata4/commons/api/edm;h=5735ace09587887d86a9f09c9ffdcd624b97521f >>>>> ; >>>>> hb=olingo169 [7] >>>>> https://git-wip-us.apache.org/repos/asf?p=incubator-olingo-odata4.gi >>>>> t >>>>> ;a=tree;f=odata4-lib/odata4-client-api/src/main/java/org/apache/olin >>>>> g >>>>> o/odata4/client/api/edm;h=31d1e59d5b76ba321b93bd68bda675522f9d1a8c;h >>>>> b >>>>> =olingo169 >>>> [8] >>>> https://git-wip-us.apache.org/repos/asf?p=incubator-olingo-odata4.git >>>> ; >>>> a=blob;f=odata4-lib/odata4-commons-api/src/main/java/org/apache/oling >>>> o >>>> /odata4/commons/api/edm/EdmType.java;h=bee2fa5394774b4ea41a71ee433cf5 >>>> d >>>> 7366c961c;hb=olingo169 [9] >>>> https://git-wip-us.apache.org/repos/asf?p=incubator-olingo-odata4.git >>>> ; >>>> a=blob;f=odata4-lib/odata4-client-api/src/main/java/org/apache/olingo >>>> / >>>> odata4/client/api/edm/v4/EntitySet.java;h=703c0dfa3c23ea66ea88c8fb387 >>>> 5 >>>> f90962e3a800;hb=olingo169 [10] >>>> https://git-wip-us.apache.org/repos/asf?p=incubator-olingo-odata4.git >>>> ; >>>> a=blob;f=odata4-lib/odata4-commons-api/src/main/java/org/apache/oling >>>> o >>>> /odata4/commons/api/edm/EdmEntitySet.java;h=50dbe0590ff5aa32340eeee5d >>>> 3 >>>> 26844a92f0c91a;hb=olingo169 -- Francesco Chicchiriccò Tirasa - Open Source Excellence http://www.tirasa.net/ Involved at The Apache Software Foundation: member, Syncope PMC chair, Cocoon PMC, Olingo PPMC http://people.apache.org/~ilgrosso/