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

Support polymorphic relations for Ember Data #41

Open
bummzack opened this issue Jun 10, 2015 · 6 comments
Open

Support polymorphic relations for Ember Data #41

bummzack opened this issue Jun 10, 2015 · 6 comments

Comments

@bummzack
Copy link
Contributor

Ember Data supports polymorphic relations. Here's a presentation (slides) that explains some of the concepts (some of the issues discussed there are resolved nowadays).

To properly support polymorphic relations, the JSON output has to be type hinted. Eg. a has_many relation would change from:

shapes: [1, 3, 4]

to:

shapes: [
    { "id": 1, "type": "triangle" }, 
    { "id": 3, "type": "rectangle" }, 
    { "id": 4, "type": "triangle" }
]

Another issue is sideloading of records. Instead of having:

shapes: [ <shapes payload> ]

the sideloaded data has to be separated by polymorphic type. Eg.

triangles: [ <triangles payload> ], rectangles: [ <rectangles payload> ]
@colymba
Copy link
Owner

colymba commented Jun 10, 2015

Sounds like a great idea! Shouldn't be hard to serialize, but do you have an idea on how to detect those relation in SilverStripe?

@bummzack
Copy link
Contributor Author

Since somebody might not want "polymorphic" output, while still having polymorphism in SilverStripe, I'd say it should be configurable.

Say you have the following class hierarchy in SilverStripe:

Shape extends DataObject
Rectangle extends Shape
Triangle extends Shape

From what I've seen, it's enough to enable API access for all three classes by adding the following config:

Shape:
  api_access: true

So I would make the polymorphism just another flag in the config. Eg.

Shape:
  api_access: true
  polymorphic: true

I haven't looked into the code that much to see if this is sufficient.. I guess the serializer would have to check the relation, then see if the relation is polymorphic and switch to the polymorphic output mode instead.

@colymba
Copy link
Owner

colymba commented Jun 10, 2015

First, are you sure giving api access to the parent class gives access to all classes that extend it? I never really tested this and am a bit worried of the security issue here...

For the polymorphic relation | think it would be better to have either a 'polymorphic' property on the class with the value being an array with the relation name, or a 'polymorphic' Boolean for each relation.

Shape:
  polymorphic:
    - Owner
    - Other

Or

Shape:
  RelationName:
    polymorphic: true

I'll have to see what makes most sense when implementing....

@bummzack
Copy link
Contributor Author

Hm I haven't tested it extensively, but I just noticed that giving access to the base-class enables access on the subclasses. Why do you think it's an issue? Subclasses should inherit permissions from base-classes, unless explicitly declared otherwise.

Setting it on a relation might also be a good approach. Is probably more flexible.

@colymba
Copy link
Owner

colymba commented Jun 11, 2015

To me access should be explicitly declared, to avoid giving permissions without realising it. So it works a bit like controllers $allow_actions

@bummzack
Copy link
Contributor Author

My line of thought was that it would work the same way as permissions in SilverStripe. If your base-class has canView, canEdit etc. implemented, the permissions will carry over to subclasses. But since your API also allows "config-only" checks, it might actually be better to force permission-settings on every accessible class.

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