doctrine-extensions / DoctrineExtensions

Doctrine2 behavioral extensions, Translatable, Sluggable, Tree-NestedSet, Timestampable, Loggable, Sortable
MIT License
4.05k stars 1.27k forks source link

[Tree] lft, lvl, rgt always set to zero in an entity using Ulid as primary key. #2701

Open groupecomplus opened 1 year ago

groupecomplus commented 1 year ago

Environment

Package

show

``` $ composer show --latest gedmo/doctrine-extensions name : gedmo/doctrine-extensions descrip. : Doctrine behavioral extensions keywords : Blameable, behaviors, doctrine, extensions, gedmo, loggable, nestedset, odm, orm, sluggable, sortable, timestampable, translatable, tree, uploadable versions : * v3.13.0 latest : v3.13.0 type : library license : MIT License (MIT) (OSI approved) https://spdx.org/licenses/MIT.html#licenseText homepage : http://gediminasm.org/ source : [git] https://github.com/doctrine-extensions/DoctrineExtensions.git 291d0c527d2dc9ee07b888c9a4e2a179893f08ab dist : [zip] https://api.github.com/repos/doctrine-extensions/DoctrineExtensions/zipball/291d0c527d2dc9ee07b888c9a4e2a179893f08ab 291d0c527d2dc9ee07b888c9a4e2a179893f08ab path : \vendor\gedmo\doctrine-extensions names : gedmo/doctrine-extensions support email : gediminas.morkevicius@gmail.com issues : https://github.com/doctrine-extensions/DoctrineExtensions/issues source : https://github.com/doctrine-extensions/DoctrineExtensions/tree/v3.13.0 wiki : https://github.com/Atlantic18/DoctrineExtensions/tree/main/doc autoload psr-4 Gedmo\ => src/ requires behat/transliterator ^1.2 doctrine/annotations ^1.13 || ^2.0 doctrine/collections ^1.2 || ^2.0 doctrine/common ^2.13 || ^3.0 doctrine/event-manager ^1.2 || ^2.0 doctrine/persistence ^2.2 || ^3.0 php ^7.2 || ^8.0 psr/cache ^1 || ^2 || ^3 symfony/cache ^4.4 || ^5.3 || ^6.0 symfony/deprecation-contracts ^2.1 || ^3.0 requires (dev) doctrine/cache ^1.11 || ^2.0 doctrine/dbal ^2.13.1 || ^3.2 doctrine/doctrine-bundle ^2.3 doctrine/mongodb-odm ^2.3 doctrine/orm ^2.10.2 friendsofphp/php-cs-fixer ^3.4.0 <3.10 nesbot/carbon ^2.55 phpstan/phpstan ^1.10.2 phpstan/phpstan-doctrine ^1.0 phpstan/phpstan-phpunit ^1.0 phpunit/phpunit ^8.5 || ^9.5 rector/rector ^0.15.20 symfony/console ^4.4 || ^5.3 || ^6.0 symfony/phpunit-bridge ^6.0 symfony/yaml ^4.4 || ^5.3 || ^6.0 suggests doctrine/mongodb-odm to use the extensions with the MongoDB ODM doctrine/orm to use the extensions with the ORM conflicts doctrine/dbal <2.13.1 || ^3.0 <3.2 doctrine/mongodb-odm <2.3 doctrine/orm <2.10.2 || 2.16.0 || 2.16.1 sebastian/comparator <2.0 ```

Doctrine packages

show

``` $ composer show --latest 'doctrine/*' Color legend: - patch or minor release available - update recommended - major release available - update possible - up to date version Direct dependencies required in composer.json: doctrine/doctrine-bundle 2.10.2 2.10.2 Symfony DoctrineBundle doctrine/doctrine-fixtures-bundle 3.4.4 3.4.4 Symfony DoctrineFixturesBundle doctrine/doctrine-migrations-bundle 3.2.4 3.2.4 Symfony DoctrineMigrationsBundle doctrine/orm 2.16.2 2.16.2 Object-Relational-Mapper for PHP Transitive dependencies not required in composer.json: doctrine/annotations 2.0.1 2.0.1 Docblock Annotations Parser doctrine/cache 2.2.0 2.2.0 PHP Doctrine Cache library is a popular cache implementation that supports many different d... doctrine/collections 2.1.3 2.1.3 PHP Doctrine Collections library that adds additional functionality on top of PHP arrays. doctrine/common 3.4.3 3.4.3 PHP Doctrine Common project is a library that provides additional functionality that other ... doctrine/data-fixtures 1.6.7 1.6.7 Data Fixtures for all Doctrine Object Managers doctrine/dbal 3.6.6 3.6.6 Powerful PHP database abstraction layer (DBAL) with many features for database schema intro... doctrine/deprecations v1.1.1 v1.1.1 A small layer on top of trigger_error(E_USER_DEPRECATED) or PSR-3 logging with options to d... doctrine/event-manager 2.0.0 2.0.0 The Doctrine Event Manager is a simple PHP event system that was built to be used with the ... doctrine/inflector 2.0.8 2.0.8 PHP Doctrine Inflector is a small library that can perform string manipulations with regard... doctrine/instantiator 2.0.0 2.0.0 A small, lightweight utility to instantiate objects in PHP without invoking their constructors doctrine/lexer 2.1.0 3.0.0 PHP Doctrine Lexer parser library that can be used in Top-Down, Recursive Descent Parsers. doctrine/migrations 3.6.0 3.6.0 PHP Doctrine Migrations project offer additional functionality on top of the database abstr... doctrine/persistence 3.2.0 3.2.0 The Doctrine Persistence project is a set of shared interfaces and functionality that the d... doctrine/sql-formatter 1.1.3 1.1.3 a PHP SQL highlighting library ```

PHP version

$ php -v
PHP 8.1.13 (cli) (built: Nov 22 2022 15:44:26) (NTS Visual C++ 2019 x64)
Copyright (c) The PHP Group
Zend Engine v4.1.13, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.13, Copyright (c), by Zend Technologies
    with Xdebug v3.2.0, Copyright (c) 2002-2022, by Derick Rethans

Subject

I have nested Tree entity that implements Translatable and uses Ulid as primary keys.
The Entity's repository is CategoryRepository , it extends NestedTreeRepository.

src/Entity/Category.php

```php |Category[] */ #[ORM\OneToMany(targetEntity: Category::class, mappedBy: 'parent')] #[ORM\OrderBy(['lft' => 'ASC'])] private Collection $children; #[Gedmo\Locale] private string $locale; public function __construct() { $this->formations = new ArrayCollection(); } public function getId(): ?Ulid { return $this->id; } public function getName(): ?string { return $this->name; } public function setName(string $name): static { $this->name = $name; return $this; } public function setTranslatableLocale(string $locale): void { $this->locale = $locale; } public function getRoot(): ?self { return $this->root; } public function setParent(?self $parent = null): void { $this->parent = $parent; } public function getParent(): ?self { return $this->parent; } } ```

I created a test fixture using the basic example found in the documentation

src/DataFixtures/CategoryFixtures.php

```php setName('Food'); $fruits = new Category(); $fruits->setName('Fruits'); $fruits->setParent($food); $vegetables = new Category(); $vegetables->setName('Vegetables'); $vegetables->setParent($food); $carrots = new Category(); $carrots->setName('Carrots'); $carrots->setParent($vegetables); $manager->persist($food); $manager->persist($fruits); $manager->persist($vegetables); $manager->persist($carrots); $manager->flush(); } } ```

After running the fixtures, I can see the following structure in DB:

SELECT RIGHT(HEX(id), 8) AS id, RIGHT(HEX(tree_root), 8) AS tree_root, RIGHT(HEX(parent_id), 8) AS parent_id, name, lft, lvl, rgt
FROM category;
id tree_root parent_id name lft lvl rgt
C80F8BC8 Food 0 0 0
C80F8BC9 C80F8BC8 Fruits 0 0 0
C80F8BCA C80F8BC8 Vegetables 0 0 0
C80F8BCB C80F8BCA Carrots 0 0 0

You can notice that lft, lvl and rgt are set to 0 for all rows, instead of reflecting the expected structure.

groupecomplus commented 1 year ago

I tried to change the primary key type from Ulid to Uuid, the result is the same.

id tree_root parent_id name lft lvl rgt
C180 Food 0 0 0
F3C9 C180 Fruits 0 0 0
9A12 C180 Vegetables 0 0 0
C3D4 9A12 Carrots 0 0 0
// src/Entity/Category.php

class Category implements Translatable
{
    #[ORM\Id]
    #[ORM\Column(type: UuidType::NAME, unique: true)]
    #[ORM\GeneratedValue(strategy: 'CUSTOM')]
    #[ORM\CustomIdGenerator(class: 'doctrine.uuid_generator')]
    private ?Uuid $id = null;
kl3sk commented 9 months ago

Hello,

I have the same configuration, is there any solution. Or we need to switch back to int as primary key ?

My root field is always empty, think this is related.

groupecomplus commented 9 months ago

@kl3sk I still couldn't find a solution, nor succeed to patch the issue. For now I kept an int PK.
Nobody from the team replied yet either.

kl3sk commented 9 months ago

I just find a solution that someone shared, It works for my first tests.

Let me know for you.

https://github.com/doctrine-extensions/DoctrineExtensions/issues/2216#issuecomment-926767989

mbabker commented 9 months ago

OK, I set up a test case for this, see https://github.com/doctrine-extensions/DoctrineExtensions/compare/main...mbabker:DoctrineExtensions:issue-2701 for that.

I couldn't reproduce the issue with the lft/rgt values not being set right (this could very well be an SQLite versus another database platform issue), output below:

$ vendor/bin/phpunit -c tests/ tests/Gedmo/Tree/Issue/Issue2701Test.php 
PHPUnit 9.6.17 by Sebastian Bergmann and contributors.

1 "Food lft/rgt"
2 1
3 8
1 "Fruits lft/rgt"
2 2
3 3
1 "Vegetables lft/rgt"
2 4
3 7
1 "Carrots lft/rgt"
2 5
3 6

But, the test fails:

1) Gedmo\Tests\Tree\Issue\Issue2701Test::testGetNextSiblingsWithoutIdentifierMethod
Failed asserting that Array &0 (
    0 => 'index [0], missing on tree root: 01HQM5N5P9FCKDHJ69XZX1DVH0'
) is true.

tests/Gedmo/Tree/Issue/Issue2701Test.php:90

Dumping some info from the query logger during that test, I see this query setting info on the root node:

  16 => array:2 [
    "message" => "Executing statement: {sql} (parameters: {params}, types: {types})"
    "context" => array:3 [
      "sql" => "UPDATE Category SET tree_root = ?, lvl = 0, lft = 1, rgt = 2 WHERE id = ?"
      "params" => array:2 [
        1 => Symfony\Component\Uid\Ulid {#360
          #uid: "01HQM6B6QA4M459E41B3EHYG1E"
          toBase58: "1C8fpHddgcTvCYdfyA4Wt5"
          toRfc4122: "018de865-9aea-2508-54b8-8158dd1f402e"
          time: "2024-02-27 02:29:49.418 UTC"
        }
        2 => Symfony\Component\Uid\Ulid {#360}
      ]
      "types" => array:2 [
        1 => 2
        2 => 2
      ]
    ]
  ]

The interesting bit here is the types array. Because none of the setParameter() calls are specifying types, under the hood everything ends up in the ORM's ParameterTypeInferer and it only knows how to deal with a subset of built-in types, so, it defaults to the DBAL's ParameterType::STRING (2). Adding the ulid type to the relevant setParameter() calls does make the test pass.

So, the extension needs to become a lot smarter when it comes to determining ID field types. How that happens, I can't say I've got a clue since I don't use UID-based PKs myself. https://github.com/doctrine-extensions/DoctrineExtensions/issues/2216#issuecomment-926767989 might be an OK workaround in an application, but it's not going to be a great library-level solution (and it's only going to fix one specific use case).

github-actions[bot] commented 3 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.