Field mapping for edges

[SammyK]

As we think about how we're going to support "non-root node thingies" in #401, it'd be nice to also include support for auto-casting of nodes returned from an edge.

We could reference the edge just as we do with fields and define how to cast each node from that edge.

class GraphEvent extends GraphNode
{
    protected static $graphObjectMap = [
        // Normal field
        'cover' => '\Facebook\GraphNodes\GraphCoverPhoto',
        // Edge with all of the same subtypes returned
        'photos' => '\Facebook\GraphNodes\GraphPhoto',
    ];

    // . . .
}

The SDK already has support for casting edge subtypes so it would just require getting the $graphObjectMap feature to support it.

[devmsh]

👍

When I reviewed the edge support for the first time, basically for the user.picture edge, I confused!

Based on the official docs, user/picture must return an edge that contain a list of ProfilePictureSource nodes, but in fact, it return only one ProfilePictureSource object and it did not include any paging metadata!

Now, the SDK uses GraphPicture edge as a node subclass name nested of ProfilePictureSource and auto cast it to a single node nested of edge because isCastableAsGraphEdge function in GraphNodeFactory class will return false!

mmm, this way we auto cast me?fields=photos, but the most common scenario is to call me/photos, and this still have no auto casting option. So, I suggest to add the auto casting functionality for getGraphEdge also.

for example:

$graphEdgeMap = array(
    'photos' => '\Facebook\GraphNodes\GraphPhoto',
    'events' => '\Facebook\GraphNodes\GraphEvent',
);

where the key of the array equal to the edge name that extracted from the Graph quiery {id}/{edge}?

[SammyK]

Based on the official docs, user/picture must return an edge that contain a list of ProfilePictureSource nodes

Yeah, the /{object}/picture is a special edge. It really should be listed in the "fields" list like significant_other which also returns one Graph node. I think the Graph API people moved it to an edge so that URL's to people's profile photos could be easily generated.

because isCastableAsGraphEdge function in GraphNodeFactory class will return false!

Yeah, isCastableAsGraphEdge() is supposed to distinguish a single node vs an edge despite what the docs call it. But the only discrepancy I know of is for pictures. :)

mmm, this way we auto cast me?fields=photos, but the most common scenario is to call me/photos, and this still have no auto casting option.

I think that's fine. We can document that if you want to get a list back as a specific subtype you just used nested request syntax. I actually prefer nested requests as they give me a lot more control over what I'm getting back and it's these types of situations I want to control the edge subtypes. That's why I wrote the Facebook Query Builder. :)

where the key of the array equal to the edge name that extracted from the Graph quiery {id}/{edge}?

"Yes" to casting by referencing using the edge/field name. But I wouldn't try and extract parts of the URL to guess what subtypes to cast to. One of the worst things about v3 of the PHP SDK was all the guessing it did. It was a nightmare. If you're using nested request syntax, the edges will be returned as a field in the response.

Direct Edge Response

GET /848252565231486/photos

{
  "data": [
    {
      "id": "10152749540391510"
      . . .
    },
    . . .
    ]
}

Nested Request Edge Response

GET /848252565231486?fields=photos

{
  "photos": {
    "data": [
      {
        "id": "10152749540391510"
        . . . 
      },
      . . .
      ]
    }
}

[yguedidi]

I'm 👎 on this "just" because one of my proposal refactoring for 5.0 is to remove that static map :)

[SammyK]

@yguedidi Is it just because it's a static? If it were a constant would you be open to supporting it? I wanted to make it a constant but constants don't support arrays. :)

Statics are usually used incorrectly about 90% of the time, but this is a situation that really warrants statics since the map is just immutable config.

[yguedidi]

@SammyK not really because it's a static, but more about how it's used. Once I got some time, probably later today, I'll open the issue about this with more details ;)

[SammyK]

Sounds good! :)

[devmsh]

Can't wait :)

[yguedidi]

@SammyK @devmsh issue opened: #437 ;)

[ghost]

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.

[gfosco]

Closing this because #437. Re-open if you think it necessary.