CircleOfNice / DoctrineRestDriver

GNU General Public License v3.0
153 stars 44 forks source link

The driver does not support UUIDs as ID #48

Closed nachomozo closed 6 years ago

nachomozo commented 7 years ago

What is this feature about (expected vs actual behaviour)?

The API I'm trying to consume uses UUIDs as resource ID. This produces several problems.

How can I reproduce it?

In first place if you try to configure you ID as follows:

/**
     * @var string
     *
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="UUID")
     * @ORM\Column(type="guid")
     */
    private $id;

And you try to perform a "persist" (POST) you get the following error:

An exception occurred while executing 'SELECT UUID()':

The given value for "tokens" is not of type array
500 Internal Server Error - DBALException
1 linked Exception: InvalidTypeException

I've tried to change de strategy to @ORM\GeneratedValue(strategy="AUTO") and it works but the returning ID is not the new resource real id, is a random integer. It seems to do some kind of type conversion.

But the biggest problem comes when you try to perform a "find" (GET by ID). In this case, at some point, it breaks the UUID from the first hyphen and sends the first part in the URL and the second part (with some characters of the first path) as a query.

For example, if I perform $em->getRepository('AppBundle\Entity\MyEntity')->find('461d2c96-0fac-11e7-8161-0242ac110002') I get the following error:

Execution failed for request: GET http://my-api-url/my-resource/461d2c96?d2c96-0fac-11e7-8161-0242ac110002 HTTP/1.1: HTTPCode 404

Does it take minutes, hours or days to fix?

I think is a type conversion problem, so maybe minutes.

Any additional information?

TobiasHauck commented 7 years ago

Thanks for reporting. I think it's a conversion problem, too. May you be so kind and search for the line where it fails? It's not that easy to reproduce and you got everything in place right now.

nachomozo commented 7 years ago

I have found the problem but it is a bit complex for my knowledge.

The problem occurs when the greenlion/PHP-SQL-Parser library is used to split the query in $tokens = $this->parser->parse($query); in DoctrineRestDriver/Transformers/MysqlToRequest.php line 84 . This interprets the UUID hyphens as operators and from this point everything fails.

The result of WHERE part of the query by numerical ID is:

Array
(
    [0] => Array
        (
            [expr_type] => colref
            [base_expr] => t0.id
            [no_quotes] => Array
                (
                    [delim] => .
                    [parts] => Array
                        (
                            [0] => t0
                            [1] => id
                        )

                )

            [sub_tree] => 
        )

    [1] => Array
        (
            [expr_type] => operator
            [base_expr] => =
            [sub_tree] => 
        )

    [2] => Array
        (
            [expr_type] => const
            [base_expr] => 22
            [sub_tree] => 
        )
)

However for an UUID it is:

Array
(
    [0] => Array
        (
            [expr_type] => colref
            [base_expr] => t0.id
            [no_quotes] => Array
                (
                    [delim] => .
                    [parts] => Array
                        (
                            [0] => t0
                            [1] => id
                        )

                )

            [sub_tree] => 
        )

    [1] => Array
        (
            [expr_type] => operator
            [base_expr] => =
            [sub_tree] => 
        )

    [2] => Array
        (
            [expr_type] => colref
            [base_expr] => 49cc9fc7
            [no_quotes] => Array
                (
                    [delim] => 
                    [parts] => Array
                        (
                            [0] => 49cc9fc7
                        )

                )

            [sub_tree] => 
        )

    [3] => Array
        (
            [expr_type] => operator
            [base_expr] => -
            [sub_tree] => 
        )

    [4] => Array
        (
            [expr_type] => colref
            [base_expr] => 145f
            [no_quotes] => Array
                (
                    [delim] => 
                    [parts] => Array
                        (
                            [0] => 145f
                        )

                )

            [sub_tree] => 
        )

    [5] => Array
        (
            [expr_type] => operator
            [base_expr] => -
            [sub_tree] => 
        )

    [6] => Array
        (
            [expr_type] => const
            [base_expr] => 11e7
            [sub_tree] => 
        )

    [7] => Array
        (
            [expr_type] => operator
            [base_expr] => -
            [sub_tree] => 
        )

    [8] => Array
        (
            [expr_type] => colref
            [base_expr] => b25d
            [no_quotes] => Array
                (
                    [delim] => 
                    [parts] => Array
                        (
                            [0] => b25d
                        )

                )

            [sub_tree] => 
        )

    [9] => Array
        (
            [expr_type] => operator
            [base_expr] => -
            [sub_tree] => 
        )

    [10] => Array
        (
            [expr_type] => colref
            [base_expr] => 0242ac110002
            [no_quotes] => Array
                (
                    [delim] => 
                    [parts] => Array
                        (
                            [0] => 0242ac110002
                        )

                )

            [sub_tree] => 
        )

)

I have tried to fix it but I have never used this library and I have not succeeded. Maybe you know a simple solution to this problem.

Thank you very much.

TobiasHauck commented 7 years ago

Thank you for this information, this helps a lot as I can create some unit tests. Got some ideas to fix it, but this is not an easy one :-D

TobiasHauck commented 7 years ago

Have you built the original query or is it an auto generated one by doctrine? I think the best way to get it work is to put quotes around the uuid value like

    WHERE t0.id = "49ccf..."

instead of

    WHERE t0.id = 49ccf...
TobiasHauck commented 7 years ago

This is the reason the parser library breaks (in general)

nachomozo commented 7 years ago

The query is auto generated by Doctrine.

What you proposed was the first thing I tried. The parser no longer takes the hyphens as operators but includes the quotes in the request so it fails as well.

TobiasHauck commented 7 years ago

Ah sh**. Ok this one is going to be an interesting one

TobiasHauck commented 7 years ago

Ok got news here.

Doctrine uses the SELECT UUID() statement to create a new UUID and then set this ID to the new instance. Technically the driver could check if this statement is fired and then create its own UUID. The problem is: The driver connects a CLIENT to a HOST. A client should never create a UUID for several reasons.

So what could be done is: Redirect the SELECT UUID() statement to a configured route returning a new UUID. Would this be an advantage for you?

TobiasHauck commented 6 years ago

We should close this issue because the driver is running on client side so the server has to handle ID creation. The use case where you send a request to a several route just to generate an UUID is very uncommon.

That doesn't mean we'd accept a pull request adding this feature.