Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_create_entities typeclass creation bug #8

Closed
arjd opened this issue May 2, 2017 · 9 comments
Closed

_create_entities typeclass creation bug #8

arjd opened this issue May 2, 2017 · 9 comments

Comments

@arjd
Copy link

arjd commented May 2, 2017

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
Copy link
Owner

tuomur commented May 2, 2017

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
Copy link
Author

arjd commented May 2, 2017

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
Copy link
Owner

tuomur commented May 2, 2017

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
Copy link
Author

arjd commented May 2, 2017

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.

<Schema xmlns="http://docs.oasis-open.org/odata/ns/edm" Namespace="PlatformForScience" Alias="pfs">

@arjd
Copy link
Author

arjd commented May 2, 2017

_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
Copy link
Owner

tuomur commented May 2, 2017

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
Copy link
Owner

tuomur commented May 2, 2017

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

@arjd
Copy link
Author

arjd commented May 2, 2017

It works! Depth of 3 lol.

@tuomur
Copy link
Owner

tuomur commented May 3, 2017

Merged to master in 405ab47.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants