Hi Francesco, I looked at your changes and I think it is ok to merge them.
1.) Not sure what to deliver here on client side. I will have a look into it. 2.) Path is what leads to the property which is referenced in the key. For example: complexProperty1/complexProperty2 would be the path while the Name attribute would be the name of the property. Alias is then used inside the URI. 3.) A function import must have an attribute with a name which resolves to an unbound function but unbound functions can be overloaded using their parameters. Thus getFunction(parameters) delivers the function which matches with the parameters. Best Regards, Christian -----Original Message----- From: Francesco Chicchiriccò [mailto:ilgro...@apache.org] Sent: Mittwoch, 5. März 2014 16:28 To: dev@olingo.incubator.apache.org Subject: Re: [OLINGO-169] Work in progress Hi, as you might have noticed, I have completed the client-side implementation of common Edm interfaces in the olingo169 branch. Before merging such branch into master and close OLINGO-169, can you please take a look and see if everything looks fine? Of course all tests are succeeding, but I pushed some heavy refactoring that also affect server components and it would be nice if someone else can double-check. Finally, there are some more methods I have doubts about (and I have not implemented client-side): 1. EdmProperty#getMapping / EdmProperty#getMimeType / EdmParameter#getMapping - what kind of information can be provided here client-side? 2. EdmKeyPropertyRef#getPath - edm:PropertyRef element only allows Name and Alias attributes, so what is 'getPath' supposed to return? 3. Why EdmFunctionImport#getFunction(List<String> parameterNames) instead of EdmFunctionImport#getFunction() (e.g. without parameters)? Shall I open an issue for such changes (including the ones discussed below)? Regards. On 03/03/2014 13:44, Amend, Christian wrote: > Hi Francesco, > > I was about to start with metadata serialization on server side so I will > perform the changes on master and you can merge them when you are ready. > > About introducing an AbstractEdmEnumTypeImpl. Yes why not. > > Best Regards, > Christian > > > -----Original Message----- > From: Francesco Chicchiriccò [mailto:ilgro...@apache.org] > Sent: Montag, 3. März 2014 10:20 > To: dev@olingo.incubator.apache.org > Subject: Re: [OLINGO-169] Work in progress > > On 03/03/2014 09:52, Amend, Christian wrote: >> Hi Francesco, >> >> 1. yes. getMetadata should be deleted. > Fine: shall I take this on the olingo169 branch or instead can you make > it on master (and I will later merge)? > >> 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. > Same as above. > >> 3. Without arguments is fine by me. > Same as above. > >> 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. > +1 (even though I have already implemented client-side). > > What about > >> 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 implementations). > ? > > Regards. > >> -----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/
