Core: Can't use UUID

[polc]

Hello,

It's impossible right now to use UUID for entity identifier.

I don't know what's the purpose of this explode. : https://github.com/api-platform/core/blob/master/src/Bridge/Doctrine/Orm/ItemDataProvider.php#L71

[Simperfit]

How I see it and I have done it is to use both, one UID and one UUID, since the uid is incremented automatically you can set uuid and query on them.

WDYT ?

[theofidry]

@Simperfit there is two scenarios:

• having only a UUID as an ID
• having an auto-incremented ID ("private": used only in the application and never accessible to the user) and a UUID token ("public")

[Simperfit]

@theofidry then we may not support the first case, do we want to do so ?

@polc Do you get an invalid identifier exception on the first case ?

[theofidry]

@Simperfit I didn't check if we currently support the second case either.

But I think we should support both. Using the database ID is quite common because simple, but not wanting to do so is also a common use case so it would be sad if we were not able to support it.

[Simperfit]

We support the 2nd case because I use it in my project, uid + uuid and it's working properly.

[dunglas]

The purpose is supporting composed primary keys but - indeed - it's a problem for uuid and any other id containing a dash. I didn't think about that when I've made this change.

I think to 2 different solutions:

• always try to find the entity without exploding the key first (easy to implement but will slow down the app when a composed primary key is in use)
• use another special character as separator (I suggest a slash - but it will cause similar problems if identifiers contain slashes but it should be more rare and we can document it)

I would prefer switch to /. Wdyt?

[theofidry]

@dunglas can't we configure providers on a route basis? Like that we can have one provider for each case and a provider registry which will try to do everything (by using the other provides). By default we can use the registry, and for fine tuning the developer can go further and manually configure the providers as he see fit.

[dunglas]

A less common separator but still significant is a double dot "..". What do you think?

[dunglas]

We can but it is still a problem if one of the composed keys contains the special char we use as separator.

[dunglas]

Instead of a provider, I was thinking to a flag for the Resource annotation.

[Simperfit]

@dunglas I don't understand why you explode on anything ? why not transform the uid in an array since anyway you will just transform it as an array with one key, am i wrong ?

Edit : Ok i understand

[theofidry]

Could you give an example of what it looks like when using a composite key at the moment?

Instead of a provider, I was thinking to a flag for the Resource annotation.

I said provider because I saw the line @polc pointing a line there, no problem if it can be done elsewhere.

[Simperfit]

Using $identifierValues = (array) $id; all tests are green and uuid are working properly, I've added some tests to behat. Does the tests does not test the composite value ?

@theofidry https://github.com/api-platform/core/blob/master/features/composite.feature

Edit: How can we test that the composite key are working properly ?

[dunglas]

Because we lack some tests regarding composite identifiers support I bet.
Currently if you use a composite key both keys are joined using a dash when generating the URL and exploded when retrieving data through the data provider.

[Simperfit]

1/ Use a flag in the Resource, to know if its a composite id or not let the dash exist and explode only if it's a composite id ?
2/ Use '..' as a separator ?

I would go for 1/ because like you said if in the composite id there is the separator we will have strange behavior.

And add more test for the composite identifiers :)

[dunglas]

There is probably a better solution: inspect Doctrine metadata to detect if it's a composite key or not. If it is, explode.

[teohhanhui]

RESTful Web Services Cookbook suggests:

Use the comma (,) and semi-colon (;) characters to indicate nonhierarchical portions of the URI. The semicolon convention is used to identify matrix parameters:

http://www.example.org/co-ordinates;w=39.001409,z=-84.578201
http://www.example.org/axis;x=0,y=9

RFC 3986 has this to say:

Aside from dot-segments in hierarchical paths, a path segment is
considered opaque by the generic syntax. URI producing applications
often use the reserved characters allowed in a segment to delimit
scheme-specific or dereference-handler-specific subcomponents. For
example, the semicolon (";") and equals ("=") reserved characters are
often used to delimit parameters and parameter values applicable to
that segment. The comma (",") reserved character is often used for
similar purposes. For example, one URI producer might use a segment
such as "name;v=1.1" to indicate a reference to version 1.1 of
"name", whereas another might use a segment such as "name,1.1" to
indicate the same. Parameter types may be defined by scheme-specific
semantics, but in most cases the syntax of a parameter is specific to
the implementation of the URI's dereferencing algorithm.

[teohhanhui]

So perhaps we can go with something like

/dummies/dummyId=1,dummyUuid=41B29566-144B-11E6-A148-3E1D05DEFE78

or simply

/dummies/1,41B29566-144B-11E6-A148-3E1D05DEFE78 (but in this case we might have difficulty in ensuring the order remains stable...)

[soyuka]

There is probably a better solution: inspect Doctrine metadata to detect if it's a composite key or not. If it is, explode.

+1 on this, it would be from far the cleanest and durable solution!

[dunglas]

I like this proposal from @teohhanhui: /dummies/1,41B29566-144B-11E6-A148-3E1D05DEFE78
The URL looks cleaner than the other one with explicit keys.

[teohhanhui]

But how do we tell which is which? We have the same problem with the current explode-based implementation. We're relying on a specific order of the properties, which should not matter (IMO).

[dunglas]

The current problem isn't the order. It is that ids are exploded if they contain a - (even if no composed keys are used). The order is the natural order of Doctrine (the order where ids are defined). It will - indeed - be broken if the developper change the order of id definitions. But with your solution it will be broken if the developper change the name of one of the keys.

Both solutions have drawbacks. I prefer the one with the cleanest exposed URL.

[teohhanhui]

The order is the natural order of Doctrine

No. Looking at the code, it's the order of properties as contained in the PropertyNameCollection. What's passed to Doctrine is an associative array with the field names as keys.

But with your solution it will be broken if the developper change the name of one of the keys.

That is only to be expected. The other solution fares worse in this regard. If you change the order of properties, the identifiers might get swapped (dangerous!).

[dunglas]

No. Looking at the code, it's the order of properties as contained in the PropertyNameCollection. What's passed to Doctrine is an associative array with the field names as keys.

Yes but this collection is populated using the Reflection API. It is in the "natural order" of properties (in the order or properties in the entity).

[dunglas]

Anyway, if there is a consensus for /dummies/dummyId=1,dummyUuid=41B29566-144B-11E6-A148-3E1D05DEFE78, go for this one. As I never use composed keys, I don't care much :-)

[teohhanhui]

Indeed, I don't use composite keys too, but I'd prefer the one with least surprise.

[Simperfit]

@teohhanhui IIUC this will be use for composite keys ? instead of 1-1 we would use name=1,othername=1 ?

[soyuka]

yes, like id_one=1,id_two=2, hopefully there won't be any string identifier containing a coma :stuck_out_tongue_winking_eye:

Still, with only one identifier, the current behavior won't change.

I'm up for both but I prefer 1,41B29566-144B-11E6-A148-3E1D05DEFE78 because it's closer to the parameter without composite identifiers.

[teohhanhui]

The comma is a reserved character. We should have already URL-encoded the identifier value beforehand.

[teohhanhui]

Unfortunately it looks like if we URL-encode beforehand it'll get double-encoded...

[Simperfit]

I'm going for 1,1 or uid=1,uid2=1 ?

Anyway If i try with 1,1 it's working but not for the list because in the return the @id is bad IIUC.

   {
                  "@id": "\/composite_relations\/1-1",
                  "@type": "CompositeRelation",
                  "id": "1,1",
                  "value": "somefoobardummy"
  }

The @idshould be 1,1 ?

ping @teohhanhui @dunglas

[theofidry]

I'm going for 1,1 or uid=1,uid2=1 ?

uid=1,uid2=1

[Simperfit]

I've updated the PR but I don't like the modification I have made, it feels like a Hack,

It is one ? ping @teohhanhui @dunglas @theofidry

[teohhanhui]

I'm leaning towards semi-colon ; as the separator now. The comma , can be kept for expressing a list of values (not that we should support that now... not sure if it even makes sense), what do you think?

e.g. uid=1;uid2=1 or the hypothetical uid=1;uid2=1,2,3

[theofidry]

makes sense to me 👍

[dunglas]

👍

[soyuka]

+1 on semi-colon

[Simperfit]

I've updated the PR according to your comments