🐛 Entity with composite PK of UUIDs isn't inserted properly

nevmerzhitsky commented 1 year ago

No duplicates 🥲.

[X] I have searched for a similar issue in our bug tracker and didn't find any solutions.

Database

PostgreSQL

What happened?

I use UUID as the PK of every table in my DB (via my custom mapper). The problem appears when I try to define an entity which is an association table (it represents the many-to-many relation). So it contains just two fields customer and telegramChat:

use Cycle\Annotated\Annotation as Cycle;

#[Cycle\Entity(mapper: \Cycle\ORM\Mapper\Mapper::class)]
#[Cycle\Table\Index(['telegram_chat_id'], unique: true)]
class CustomerTelegramChat
{
    #[Cycle\Column(type: 'uuid', name: 'customer_id', primary: true)]
    #[Cycle\Relation\BelongsTo(target: Customer::class, innerKey: 'customer_id')]
    private Customer $customer;

    #[Cycle\Column(type: 'uuid', name: 'telegram_chat_id', primary: true)]
    #[Cycle\Relation\BelongsTo(target: TelegramChat::class, innerKey: 'telegram_chat_id')]
    private TelegramChat $telegramChat;

...
}

use App\Entities\Traits\SoftDeleteTrait;
use App\Entities\Traits\TimestampsTraits;
use Cycle\Annotated\Annotation as Cycle;

#[Cycle\Entity]
class Customer
{
    use TimestampsTraits;
    use SoftDeleteTrait;

    #[Cycle\Column(type: 'uuid', primary: true)]
    private string $id;

...
}

use App\Entities\Traits\SoftDeleteTrait;
use App\Entities\Traits\TimestampsTraits;
use Cycle\Annotated\Annotation as Cycle;

#[Cycle\Entity]
class TelegramChat
{
    use TimestampsTraits;
    use SoftDeleteTrait;

    #[Cycle\Column(type: 'uuid', primary: true)]
    private string $id;

    #[Cycle\Column(type: 'bigInteger')]
    private int $telegramId;

    #[Cycle\Column(type: 'enum(private,group,supergroup,channel)')]
    private string $type;

    #[Cycle\Column(type: 'string', nullable: true)]
    private ?string $title;

    #[Cycle\Column(type: 'string', nullable: true)]
    private ?string $username;

    #[Cycle\Column(type: 'string', nullable: true)]
    private ?string $firstName;

    #[Cycle\Column(type: 'string', nullable: true)]
    private ?string $lastName;

...
}

The usage code looks like this:

        $chat = new TelegramChat(...);

        /** @var ?CustomerTelegramChat $link */
        $link = $this->orm->getRepository(CustomerTelegramChat::class)->select()
            ->where('telegram_chat_id', $chat->getId())
            ->fetchOne();

        if (!is_null($link)) {
            $customer = $link->getCustomer();
        } else {
            $customer = new Customer();
            $link = new CustomerTelegramChat($customer, $chat);
        }

        $em->persist($customer)->run();
        dump($customer);
        dump($link);
        $em->persist($link)->run();
        dump($link);

After the second run() call there is not a new row in the customer_telegram_chat table.

I've added a few dump() calls into DatabaseMapper::queueCreate()


    public function queueCreate(object $entity, Node $node, State $state): CommandInterface
    {
        $values = $state->getData();
        dump($state, $values);

        // sync the state
        $state->setStatus(Node::SCHEDULED_INSERT);

        foreach ($this->primaryKeys as $key) {
            dump($key);
            if (!isset($values[$key])) {
                foreach ($this->nextPrimaryKey() ?? [] as $pk => $value) {
                    $state->register($pk, $value);
                }
                break;
            }
        }
...

So the total debug log looks like this:

Cycle\ORM\Heap\State {#6823 // vendor/cycle/orm/src/Mapper/DatabaseMapper.php:111
  -transactionData: []
  -relationStatus: []
  -storage: []
  -state: 3
  -data: array:2 [
    "updatedAt" => null
    "deletedAt" => null
  ]
  -transactionRaw: []
  -relations: []
  #waitingFields: []
}
array:2 [ // vendor/cycle/orm/src/Mapper/DatabaseMapper.php:111
  "updatedAt" => null
  "deletedAt" => null
]
"id" // vendor/cycle/orm/src/Mapper/DatabaseMapper.php:117
App\Entities\Customer {#6251 // app/TelegramBot/Listeners/UpdateListener.php:41
  -id: Ramsey\Uuid\Lazy\LazyUuidFromString {#6421
    -unwrapped: null
    -uuid: "42543529-f8c4-4836-9bc2-a89bdce5f66c"
    uuid: "42543529-f8c4-4836-9bc2-a89bdce5f66c"
  }
  -createdAt: DateTimeImmutable @1683280564 {#6822
    date: 2023-05-05 09:56:04.849477 UTC (+00:00)
  }
  -updatedAt: DateTimeImmutable @1683280564 {#6822}
  +deletedAt: null
}
App\Entities\CustomerTelegramChat {#6245 // app/TelegramBot/Listeners/UpdateListener.php:42
  -customer: App\Entities\Customer {#6251
    -id: Ramsey\Uuid\Lazy\LazyUuidFromString {#6421
      -unwrapped: null
      -uuid: "42543529-f8c4-4836-9bc2-a89bdce5f66c"
      uuid: "42543529-f8c4-4836-9bc2-a89bdce5f66c"
    }
    -createdAt: DateTimeImmutable @1683280564 {#6822
      date: 2023-05-05 09:56:04.849477 UTC (+00:00)
    }
    -updatedAt: DateTimeImmutable @1683280564 {#6822}
    +deletedAt: null
  }
  -telegramChat: App\Entities\TelegramChat Cycle ORM Proxy {#6302
    +deletedAt: null
    +__cycle_orm_rel_map: Cycle\ORM\RelationMap {#6299
      -innerRelations: []
      -dependencies: []
      -slaves: []
      -embedded: []
    }
    +__cycle_orm_relation_props: Cycle\ORM\Mapper\Proxy\Hydrator\PropertyMap {#6500
      -class: "App\Entities\TelegramChat Cycle ORM Proxy"
      -properties: []
    }
    +__cycle_orm_rel_data: []
    -id: "e17df7a0-0487-4e31-b198-666b105c9736"
    -telegramId: 76953481
    -type: "private"
    -title: null
    -username: "nevmerzhitsky"
    -firstName: "Sergey"
    -lastName: "Nevmerzhitsky"
    -createdAt: DateTimeImmutable @1683237412 {#6323
      date: 2023-05-04 21:56:52.0 UTC (+00:00)
    }
    -updatedAt: DateTimeImmutable @1683237412 {#6326
      date: 2023-05-04 21:56:52.0 UTC (+00:00)
    }
    deletedAt: null
  }
}
Cycle\ORM\Heap\State {#6372 // vendor/cycle/orm/src/Mapper/DatabaseMapper.php:111
  -transactionData: []
  -relationStatus: array:2 [
    "customer" => 3
    "telegramChat" => 3
  ]
  -storage: []
  -state: 3
  -data: []
  -transactionRaw: []
  -relations: array:2 [
    "customer" => App\Entities\Customer {#6251
      -id: Ramsey\Uuid\Lazy\LazyUuidFromString {#6421
        -unwrapped: null
        -uuid: "42543529-f8c4-4836-9bc2-a89bdce5f66c"
        uuid: "42543529-f8c4-4836-9bc2-a89bdce5f66c"
      }
      -createdAt: DateTimeImmutable @1683280564 {#6822
        date: 2023-05-05 09:56:04.849477 UTC (+00:00)
      }
      -updatedAt: DateTimeImmutable @1683280564 {#6822}
      +deletedAt: null
    }
    "telegramChat" => App\Entities\TelegramChat Cycle ORM Proxy {#6302
      +deletedAt: null
      +__cycle_orm_rel_map: Cycle\ORM\RelationMap {#6299
        -innerRelations: []
        -dependencies: []
        -slaves: []
        -embedded: []
      }
      +__cycle_orm_relation_props: Cycle\ORM\Mapper\Proxy\Hydrator\PropertyMap {#6500
        -class: "App\Entities\TelegramChat Cycle ORM Proxy"
        -properties: []
      }
      +__cycle_orm_rel_data: []
      -id: "e17df7a0-0487-4e31-b198-666b105c9736"
      -telegramId: 76953481
      -type: "private"
      -title: null
      -username: "nevmerzhitsky"
      -firstName: "Sergey"
      -lastName: "Nevmerzhitsky"
      -createdAt: DateTimeImmutable @1683237412 {#6323
        date: 2023-05-04 21:56:52.0 UTC (+00:00)
      }
      -updatedAt: DateTimeImmutable @1683237412 {#6326
        date: 2023-05-04 21:56:52.0 UTC (+00:00)
      }
      deletedAt: null
    }
  ]
  #waitingFields: []
}
[] // vendor/cycle/orm/src/Mapper/DatabaseMapper.php:111
"customer" // vendor/cycle/orm/src/Mapper/DatabaseMapper.php:117
App\Entities\CustomerTelegramChat {#6245 // app/TelegramBot/Listeners/UpdateListener.php:46
  -customer: App\Entities\Customer {#6251
    -id: Ramsey\Uuid\Lazy\LazyUuidFromString {#6421
      -unwrapped: null
      -uuid: "42543529-f8c4-4836-9bc2-a89bdce5f66c"
      uuid: "42543529-f8c4-4836-9bc2-a89bdce5f66c"
    }
    -createdAt: DateTimeImmutable @1683280564 {#6822
      date: 2023-05-05 09:56:04.849477 UTC (+00:00)
    }
    -updatedAt: DateTimeImmutable @1683280564 {#6822}
    +deletedAt: null
  }
  -telegramChat: App\Entities\TelegramChat Cycle ORM Proxy {#6302
    +deletedAt: null
    +__cycle_orm_rel_map: Cycle\ORM\RelationMap {#6299
      -innerRelations: []
      -dependencies: []
      -slaves: []
      -embedded: []
    }
    +__cycle_orm_relation_props: Cycle\ORM\Mapper\Proxy\Hydrator\PropertyMap {#6500
      -class: "App\Entities\TelegramChat Cycle ORM Proxy"
      -properties: []
    }
    +__cycle_orm_rel_data: []
    -id: "e17df7a0-0487-4e31-b198-666b105c9736"
    -telegramId: 76953481
    -type: "private"
    -title: null
    -username: "nevmerzhitsky"
    -firstName: "Sergey"
    -lastName: "Nevmerzhitsky"
    -createdAt: DateTimeImmutable @1683237412 {#6323
      date: 2023-05-04 21:56:52.0 UTC (+00:00)
    }
    -updatedAt: DateTimeImmutable @1683237412 {#6326
      date: 2023-05-04 21:56:52.0 UTC (+00:00)
    }
    deletedAt: null
  }
}

The last entry (marked by app/TelegramBot/Listeners/UpdateListener.php:46) is the last dump() call. So it looks fully proper to be inserted in the table, but it's not happened and no errors were reported.

I've experimented a lot with this problem and I've tried to use my custom Mapper which generates UUID PK primary keys (which I use for all my other entities) as mentioned for the 1.x version of the cycle-database: https://cycle-orm.dev/docs/advanced-uuid/1.x/en#mapper

use Cycle\ORM\Exception\MapperException;
use Cycle\ORM\Mapper\Mapper;
use Exception;
use Ramsey\Uuid\Uuid;

class UuidMapper extends Mapper
{
    public function nextPrimaryKey(): ?array
    {
        try {
            return collect($this->primaryKeys)
                ->mapWithKeys(function ($attribute) {
                    $value = Uuid::uuid4()->toString();
                    return [$attribute => $value];
                })
                ->toArray();
        } catch (Exception $e) {
            throw new MapperException($e->getMessage(), $e->getCode(), $e);
        }
    }
}

The big problem here is that the mapper always tries to override the real value of customer[_id] and telegram_chat[_id] before the insert and so it broke the proper FK of CustomerTelegramChat. It was the reason why I switched the entity to the default mapper. I think this is a bug that the nextPrimaryKey() method has no access to actual field values to be able to check that the null must be returned instead of a new UUID generation (this may fix unwanted overriding of the real values).

Also, I've checked docs about composite PKs in the cycle-database 2.x version: https://cycle-orm.dev/docs/advanced-composite-pk/2.x/en#declaration-via-annotations. But I'm not sure how to mix the #[Column] and the #[BelongsTo] annotations together in the case of the composite key. I want my entity to support methods like setCustomer() and getCustomer() with eager load, not just setCustomerId(), etc.

Also, I've checked docs about UUID in the cycle-database 2.x version: https://cycle-orm.dev/docs/entity-behaviors-uuid/2.x/en#version-4-random. But I cannot apply this solution because #[UuidX] annotations cannot be used to the composite PK because these annotations are not repeatable so only one can be declared per class (it's a very strange limitation).

Sorry, I'm totally bloated up by the situation. Can you provide a guide on how to make a many-to-many table with composite UUIDs PK in cycle-database 2.x? Should I implement my own mapper with nextPrimaryKey() to use UUID or not?

Version

PHP 8.2.5
laravel/framework                 10.9.0
cycle/annotated                   v3.2.1
cycle/database                    2.4.1
cycle/entity-behavior             1.1.1
cycle/entity-behavior-uuid        1.0.0
cycle/migrations                  v4.0.1
cycle/orm                         v2.3.1
cycle/schema-builder              v2.3.0
cycle/schema-migrations-generator 2.1.0

nevmerzhitsky commented 1 year ago

This annotated version work as expected:

class CustomerTelegramChat
{
    #[Cycle\Column(type: 'uuid', primary: true)]
    private string $customerId;

    #[Cycle\Column(type: 'uuid', primary: true)]
    private string $telegramChatId;

...
}

but as I mentioned I need also to add the BelongsTo annotation else working with the entity is annoying.

With this info I can suggest that my issue is not about UUID at all, but about mixing the Column and BelongsTo annotations together for association tables. It's required to add the Column annotation to mark the field as the primary key.

msmakouz commented 1 year ago

@nevmerzhitsky Hi, you can't use relation and column attributes on the same property. In this case, need to add a standalone field for pk:

#[Cycle\Entity]
#[Cycle\Table\Index(['telegram_chat_id'], unique: true)]
class CustomerTelegramChat
{
    #[Cycle\Column(type: 'primary')] // or uuid
    private int $id;

    #[Cycle\Relation\BelongsTo(target: Customer::class, innerKey: 'customer_id')]
    private Customer $customer;

    #[Cycle\Relation\BelongsTo(target: TelegramChat::class, innerKey: 'telegram_chat_id')]
    private TelegramChat $telegramChat;
}

Or don't add a pk property to the class and a field to the database, but add Column attributes to the entity that will refer to the customer_id and telegram_chat_id fields in the database, but in PHP 8.2 there may be problems with data hydration from the database. During hydration, these fields will be created dynamically (in this case, you can add private fields customer_id and telegram_chat_id without getters):

#[Cycle\Entity]
#[Cycle\Table\Index(['telegram_chat_id'], unique: true)]
#[Cycle\Column(type: 'uuid', name: 'customer_id', primary: true)]
#[Cycle\Column(type: 'uuid', name: 'telegram_chat_id', primary: true)]
class CustomerTelegramChat
{    
    #[Cycle\Relation\BelongsTo(target: Customer::class, innerKey: 'customer_id')]
    public Customer $customer;

    #[Cycle\Relation\BelongsTo(target: TelegramChat::class, innerKey: 'telegram_chat_id')]
    public TelegramChat $telegramChat;
}

nevmerzhitsky commented 8 months ago

In this case, need to add a standalone field for pk:

So, do you think ORMs (as a product) mustn't support link tables with multicolumn PK? Or is this only the cycle-orm limitation, which may be fixed in 3.x or later?

roxblnfk commented 8 months ago

Hello @nevmerzhitsky !

Cycle ORM supports relations using multiple columns - just list linked entities field names in arrays.

#[BelogsTo(..., innerKey: ['field1', 'field2'], outerKey: ['field1', 'field2'])]

@msmakouz talks about not using the same field simultaneously for the relationship entity and for mapping the table column.

By the way, about UuidX attributes:

Also, I've checked docs about UUID in the cycle-database 2.x version: https://cycle-orm.dev/docs/entity-behaviors-uuid/2.x/en#version-4-random. But I cannot apply this solution because #[UuidX] annotations cannot be used to the composite PK because these annotations are not repeatable so only one can be declared per class (it's a very strange limitation).

The IS_REPEATABLE flag has been added to UUID attributes (issue).

nevmerzhitsky commented 5 months ago

Or don't add a pk property to the class and a field to the database, but add Column attributes

msmakouz talks about not using the same field simultaneously for the relationship entity and for mapping the table column.

Oh damn guys, it took me a few hours to wrap my head around this solution. 😄 Maybe it's a signal to cover this usecase in the documentation?

My final code of the entity that works excellent:

<?php

namespace App\Entities;

use Cycle\Annotated\Annotation as Cycle;

#[Cycle\Entity]
#[Cycle\Table\Index(['telegram_chat_id'], unique: true)]
#[Cycle\Column(type: 'uuid', name: 'customer_id', primary: true)]
#[Cycle\Column(type: 'uuid', name: 'telegram_chat_id', primary: true)]
class CustomerTelegramChat
{
    #[Cycle\Relation\BelongsTo(target: Customer::class, innerKey: 'customer_id')]
    private Customer $customer;

    #[Cycle\Relation\BelongsTo(target: TelegramChat::class, innerKey: 'telegram_chat_id')]
    private TelegramChat $telegramChat;

    public function __construct(Customer $customer, TelegramChat $telegramChat)
    {
        $this->setCustomer($customer);
        $this->setTelegramChat($telegramChat);
    }

    public function getCustomer(): Customer
    {
        return $this->customer;
    }

    public function setCustomer(Customer $value): static
    {
        $this->customer = $value;

        return $this;
    }

    public function getTelegramChat(): TelegramChat
    {
        return $this->telegramChat;
    }

    public function setTelegramChat(TelegramChat $value): static
    {
        $this->telegramChat = $value;

        return $this;
    }
}

cycle / orm