Re: [OLINGO-169] Work in progress

Francesco Chicchiriccò Wed, 05 Mar 2014 07:29:40 -0800

Hi,

as you might have noticed, I have completed the client-sideimplementation of common Edm interfaces in the olingo169 branch.

Before merging such branch into master and close OLINGO-169, can youplease take a look and see if everything looks fine? Of course all testsare succeeding, but I pushed some heavy refactoring that also affectserver 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 notimplemented client-side):

1. EdmProperty#getMapping / EdmProperty#getMimeType /EdmParameter#getMapping - what kind of information can be provided hereclient-side?

2. EdmKeyPropertyRef#getPath - edm:PropertyRef element only allowsName 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/

Re: [OLINGO-169] Work in progress

Reply via email to