Option to get related resource_uri with polymorphic resources #38

Merged
merged 4 commits into from Mar 29, 2013

Conversation

Projects
None yet
2 participants
Contributor

leo-naeka commented Nov 15, 2012

Currently, when using polymorphic resources, we have resource_uri's mapped according to the polymorphic resource's resource_name.
This is perfect and the good behaviour when we want to group multiple specific resources into a common "entity".
But in some cases (and especially when you use js frameworks like emberjs/data -our case- or backbone) it is useful to get the targetted object with its original (related) resource_uri. Finally, the polymorphic resource acts like a "transparent proxy".

Please, look at the tests and how I have implemented it.

If you're satisfied, I'll add some doc and more tests for this option.

Signed-off-by: Léo S. leo@naeka.fr

@mitar mitar commented on an outdated diff Nov 16, 2012

tastypie_mongoengine/fields.py
@@ -23,6 +23,14 @@ def get_api_name(self):
return self._resource._meta.api_name
return None
+ def get_related_resource(self, related_instance):
+ related_resource = super(ApiNameMixin, self).get_related_resource(related_instance)
+ type_map = getattr(related_resource._meta, 'polymorphic', {})
+ if type_map and getattr(related_resource._meta, 'prefer_related_resource_name', False):
+ resource = related_resource._get_resource_from_class(type_map, related_instance.__class__)
+ related_resource._meta.resource_name = resource._meta.resource_name
@mitar

mitar Nov 16, 2012

Owner

Hmm. Can related_resource object/class be shared among different instances of resources? In this case you are overriding here some value which could break something else?

Owner

mitar commented Nov 16, 2012

I am not sure if I understand the problem correctly. If I understand correctly, you want that ContactResource be polymorphic class, while IndividualResource and CompanyResource be specific classes, and that resource_uri when accessing through ContactResource, point to IndividualResource and CompanyResource?

Why do you want IndividualResource and CompanyResource resources to be available in the first place? Why not just use ContactResource and polymorphic feature of django-tastypie-mongoengine?

Cosmetic comment: I am not sure if ApiNameMixin is correct place for this code.

Contributor

leo-naeka commented Nov 16, 2012

You've understood correctly.

In my case, I don't care of the ContactResource, I do not expose it through the API. But I use it to dehydrate correctly references to either IndividualResource or CompanyResource in other resources.

My client should not care about the resource_type and get the correct resource (full or not) directly. It should only care of Individual and Company and doesn't know Contact, this is backend use of inheritance.

Owner

mitar commented Nov 16, 2012

But why you need then Contact in the first place? Why not just have Contact be abstract and have two documents and two resources? Or even not having it abstract, but simple having only IndividualResource and CompanyResource as it is defined now? Because you want to make a reference in ContactGroupResource to any of those?

But you could use GenericReferenceField on MongoEngine to get this, no?

Owner

mitar commented Nov 16, 2012

(Just brainstorming here, trying to understand the landscape of possible ways to address this.)

Contributor

leo-naeka commented Nov 16, 2012

On the mongoengine layer, using a ReferenceField on the base model instead of a GenericReferenceField ensures that the reference is one of the sub-models.

But even if consider two separated resources : IndividualResource and CompanyResource, when I want to transparently refer to it in another resource, how can I do that ?

(Don't worry, also in brainstorming mode)

Owner

mitar commented Nov 16, 2012

You could always create your own GenericReferenceField subclass which would limit allowed documents (there is nothing at MongoDB level to prevent arbitrary links, this are all just MongoEngine checks anyway, I believe).

I never used GenericReferenceField field in resource. But it should probably be possible to do. And then you would just make a reference to any resource_uri you want.

Owner

mitar commented Nov 16, 2012

BTW, one general issue I have with this is: is it really callee who should define where does resource_uri point? Or it should be caller?

Owner

mitar commented Nov 16, 2012

One other thing; why do you need two resource endpoints, why is not enough to have only contacts and use polymorphic features of this package to get wanted resources? Your JS libraries do not support that?

Contributor

leo-naeka commented Nov 16, 2012

Yeah, those libraries may support that using adapters.
But that's not the expected behavior in my case: I need multiple endpoints for segmentation, my resources are quite different and should be acceeded through their own endpoint, never through a common one.

There is two ways to refers differenciated resources:

  • use the caller resource_uri, allowing grouping within a common endpoint
  • use the callee resource_uri, to refers to the original (related) endpoint

@mitar mitar commented on the diff Nov 16, 2012

tests/test_project/urls.py
@@ -8,6 +8,10 @@
v1_api.register(resources.PersonResource())
v1_api.register(resources.PersonObjectClassResource())
v1_api.register(resources.OnlySubtypePersonResource())
+v1_api.register(resources.IndividualResource())
+v1_api.register(resources.CompanyResource())
+v1_api.register(resources.ContactResource())
@mitar

mitar Nov 16, 2012

Owner

Do you have to register it?

@leo-naeka

leo-naeka Nov 16, 2012

Contributor

If you don't want to expose it through the API, no.
In this case, I register it because of the test https://github.com/mitar/django-tastypie-mongoengine/pull/38/files#L4R1103

Owner

mitar commented Jan 6, 2013

OK. Sorry for being late in getting back to this. :-( This life is just too busy.

OK, I understand your need but I believe the approach is not nice. You are overriding resource_name when you just want resource uri to point differently. I would be more for approach of overriding resource uri method.

I still believe this should be simply done with custom field class. It would much less hackish and just reusing existing code.

Owner

mitar commented Jan 6, 2013

OK. After some looking around, it seems this is really the most reasonable approach. :-) So please add more tests, documentation and I have just some cosmetic requests:

  • use additional mixin, don't reuse ApiNameMixin (you can make new mixin and create another intermediate mixin combinig those two, but currently name ApiNameMixin does not have anything with what get_related_resource is doing)
  • prefer_related_resource_name in fact works only for polymorphic resources, when they are referenced somewhere else, name should somehow relate this
  • name also uses something quite programmatic, "related_resource_name", I am not sure people will understand this, what will they understand is that resource URIs will point to those polymorphic resources directly, not to the parent, maybe name should be polymorphic_resource_uri(s) or prefer_polymorphic_resource_uri or something like this?
  • currently, polymorphic option do not require polymorphic resources listed in polymorphic dict to be registered into API, with this flag this is required (otherwise reverse fails when constructing URI); as you name field "prefer" I think there should be a check which falls back to normal resource_name if resource is not registered (I hope this can be checked easily/cheaply)
  • tests for all such options, with and without flag and so on

I checked code and it seems there will be no problems with overriding that resource name.

Thanks for all you effort!

@leo-naeka leo-naeka added a commit to Naeka/django-tastypie-mongoengine that referenced this pull request Jan 11, 2013

@leo-naeka leo-naeka According to wlanslovenija/django-tastypie-mongoengine#38:
Use an additional mixin for get_related_resource
Changed meta property name to prefer_polymorphic_resource_uri
dd82774
@leo-naeka leo-naeka Use an additional mixin for get_related_resource
Changed meta property name to prefer_polymorphic_resource_uri
b0a82de
Contributor

leo-naeka commented Jan 11, 2013

Regarding the registered check, I'm agree with you.
Unfortunately, the only way to check that is by checking in the Api registry. Neither the Api or the registry can be acceeded through the resource.
Anyway, we could use an inherited Api class which could store a is_registered boolean flag on the resource on register/unregister - cf. tastypie/api.py#L29-L62

Just tell me what's your opinion about that, for now I'll add some doc.

Owner

mitar commented Jan 11, 2013

I see two other options:

  • catch resolve exception and try not to use the polymorphic URI
  • instead of preferring, require polymorphic to be set for all resources

I like the first option a bit more. Because it is all about URIs, so if it cannot be found, the preferred one, we fall back to backup.

Owner

mitar commented Mar 28, 2013

Just to know: I do not get any notification when somebody just updates the pull request without any comment here. Sorry that I missed that you updated the pull request a while ago. :-( Please comment something here, like "updated", in the future.

Owner

mitar commented Mar 29, 2013

Sorry for delay.

@mitar mitar added a commit that referenced this pull request Mar 29, 2013

@mitar mitar Merge pull request #38 from leo-naeka/related_resource_name
Option to get related resource_uri with polymorphic resources
15ecfb0

@mitar mitar merged commit 15ecfb0 into wlanslovenija:master Mar 29, 2013

1 check passed

default The Travis build passed
Details
Owner

mitar commented Mar 29, 2013

And thanks for the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment