tuomur / python-odata

A simple library for read/write access to OData services
MIT License
79 stars 59 forks source link

_create_entities typeclass creation bug #8

Closed arjd closed 7 years ago

arjd commented 7 years ago

Hi,

First off, I have to say, your code is absolutely gorgeous.

On to the bug -- I'm trying to reflect a schema where all entity types have base types that are entity types themselves. I am relatively new to OData so I really have no idea how common this is. I assumed most OData schemas use Edm.* types or something like that.

It looks like all_types is first pulling enum types and then adding entity typeclass to all_types, but I am running into a TypeError when the code tries to create a typeclass with a base_type that has not been reflected yet. (TypeError: metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases)

entity_class = type(entity_name, (parent_entity_class,), object_dict) # type: EntityBase

line 89 in metadata.py

parent_entity_class should be pfs.ENTITY in my schema for a particular entity 'ACCESS_LEVEL', but pfs.ENTITY is not a typeclass in all_types yet, so in my run of this code it is None...hence the metaclass conflict. I think. Would this problem be resolved by first reflecting everything where entity_dict.get('name')? The choice of which entities to create first now does not seem explicit.

tuomur commented 7 years ago

Hi, and thanks :)

Any chance you could post the metadata xml? The EntityType subclassing currently relies on correct ordering in the metadata document itself.

arjd commented 7 years ago

unfortunately I cannot ): confidentiality etc.

is ordering of the metadata document part of the OData standard? do you know where documentation on that is? the service is working on full compliance but I assumed the metadata did not have to be so tightly ordered.

the base "pfs.ENTITY" shows up as the very last entity type in the single entity container in the data service if that helps.

tuomur commented 7 years ago

The metadata spec is here. It doesn't mention much about ordering so I guess my implementation is somewhat lacking. Our Microsoft WebApi endpoint seems to return entities ordered as base classes first, and that is what I've been using as reference.

arjd commented 7 years ago

Okay. It's not elegant but this seems to work:

    for schema in schemas:
        for entity_dict in sorted(schema.get('entities'),
                                  reverse=True, key=lambda d: 'base_type' not in d):

Then I found a second bug, the schema aliases itself and metadata.py does not account for this.

`

`
arjd commented 7 years ago

_parse_entity uses the bare namespace for the schema name; could use the alias instead.

Here's my XML for the ENTITY base type:

<EntitySet Name="ENTITY" EntityType="pfs.ENTITY">
<NavigationPropertyBinding Path="PROJECT" Target="PROJECT"/>
<NavigationPropertyBinding Path="LOCATION" Target="LOCATION"/>
</EntitySet>

_parse_entity creates a type 'PlatformForScience.ENTITY', should be 'pfs.ENTITY'

 def _parse_entity(self, xmlq, entity_element, schema_name):
        entity_name = entity_element.attrib['Name']

        entity_type_name = '.'.join([schema_name, entity_name]) # [alias_name, entity_name]  ??

        entity = {
            'name': entity_name,
            'type': entity_type_name,
            'properties': [],
            'navigation_properties': [],
        }
tuomur commented 7 years ago

Fixing the parent class lookup seems easy, but not so sure about the aliases.

The alias could be picked as the schema's name, but even the spec is not clear if the original namespace should remain usable as well. I mean, would there be mixed cases when some Entity is referred to with the full namespace and some other Entity with the aliased name.

tuomur commented 7 years ago

Pushed 02b0a1585b7704e2afb89361fab8e376fa5bea8a that may fix these. Try it? :)

arjd commented 7 years ago

It works! Depth of 3 lol.

tuomur commented 7 years ago

Merged to master in 405ab477f23ef2cea199918c970213810d9dd65d.