fix(backend) : add validation constraints and fix concurrent numbering
- Add Assert\Choice on ClientTicket type and status with typed constants - Add Assert\Url on GiteaConfiguration, BookStackConfiguration, TaskBookStackLink, ClientTicket - Fix concurrent task/ticket numbering: use pg_advisory_xact_lock instead of FOR UPDATE with MAX() - Wrap CreateTaskTool numbering in transaction - Harmonize repository contracts: both return max number, caller adds +1 Tickets: T-004, T-008, T-011, T-012 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -6,6 +6,7 @@ namespace App\Entity;
|
|||||||
|
|
||||||
use App\Repository\BookStackConfigurationRepository;
|
use App\Repository\BookStackConfigurationRepository;
|
||||||
use Doctrine\ORM\Mapping as ORM;
|
use Doctrine\ORM\Mapping as ORM;
|
||||||
|
use Symfony\Component\Validator\Constraints as Assert;
|
||||||
|
|
||||||
#[ORM\Entity(repositoryClass: BookStackConfigurationRepository::class)]
|
#[ORM\Entity(repositoryClass: BookStackConfigurationRepository::class)]
|
||||||
class BookStackConfiguration
|
class BookStackConfiguration
|
||||||
@@ -16,6 +17,7 @@ class BookStackConfiguration
|
|||||||
private ?int $id = null;
|
private ?int $id = null;
|
||||||
|
|
||||||
#[ORM\Column(length: 255, nullable: true)]
|
#[ORM\Column(length: 255, nullable: true)]
|
||||||
|
#[Assert\Url]
|
||||||
private ?string $url = null;
|
private ?string $url = null;
|
||||||
|
|
||||||
#[ORM\Column(type: 'text', nullable: true)]
|
#[ORM\Column(type: 'text', nullable: true)]
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ use Doctrine\Common\Collections\ArrayCollection;
|
|||||||
use Doctrine\Common\Collections\Collection;
|
use Doctrine\Common\Collections\Collection;
|
||||||
use Doctrine\ORM\Mapping as ORM;
|
use Doctrine\ORM\Mapping as ORM;
|
||||||
use Symfony\Component\Serializer\Attribute\Groups;
|
use Symfony\Component\Serializer\Attribute\Groups;
|
||||||
|
use Symfony\Component\Validator\Constraints as Assert;
|
||||||
|
|
||||||
#[ApiResource(
|
#[ApiResource(
|
||||||
operations: [
|
operations: [
|
||||||
@@ -54,6 +55,27 @@ use Symfony\Component\Serializer\Attribute\Groups;
|
|||||||
)]
|
)]
|
||||||
class ClientTicket
|
class ClientTicket
|
||||||
{
|
{
|
||||||
|
public const string TYPE_BUG = 'bug';
|
||||||
|
public const string TYPE_IMPROVEMENT = 'improvement';
|
||||||
|
public const string TYPE_OTHER = 'other';
|
||||||
|
|
||||||
|
public const array TYPES = [
|
||||||
|
self::TYPE_BUG,
|
||||||
|
self::TYPE_IMPROVEMENT,
|
||||||
|
self::TYPE_OTHER,
|
||||||
|
];
|
||||||
|
|
||||||
|
public const string STATUS_NEW = 'new';
|
||||||
|
public const string STATUS_IN_PROGRESS = 'in_progress';
|
||||||
|
public const string STATUS_DONE = 'done';
|
||||||
|
public const string STATUS_REJECTED = 'rejected';
|
||||||
|
|
||||||
|
public const array STATUSES = [
|
||||||
|
self::STATUS_NEW,
|
||||||
|
self::STATUS_IN_PROGRESS,
|
||||||
|
self::STATUS_DONE,
|
||||||
|
self::STATUS_REJECTED,
|
||||||
|
];
|
||||||
#[ORM\Id]
|
#[ORM\Id]
|
||||||
#[ORM\GeneratedValue]
|
#[ORM\GeneratedValue]
|
||||||
#[ORM\Column]
|
#[ORM\Column]
|
||||||
@@ -66,6 +88,7 @@ class ClientTicket
|
|||||||
|
|
||||||
#[ORM\Column(length: 20)]
|
#[ORM\Column(length: 20)]
|
||||||
#[Groups(['client_ticket:read', 'client_ticket:write', 'task:read'])]
|
#[Groups(['client_ticket:read', 'client_ticket:write', 'task:read'])]
|
||||||
|
#[Assert\Choice(choices: self::TYPES)]
|
||||||
private ?string $type = null;
|
private ?string $type = null;
|
||||||
|
|
||||||
#[ORM\Column(length: 255)]
|
#[ORM\Column(length: 255)]
|
||||||
@@ -78,10 +101,12 @@ class ClientTicket
|
|||||||
|
|
||||||
#[ORM\Column(length: 255, nullable: true)]
|
#[ORM\Column(length: 255, nullable: true)]
|
||||||
#[Groups(['client_ticket:read', 'client_ticket:write'])]
|
#[Groups(['client_ticket:read', 'client_ticket:write'])]
|
||||||
|
#[Assert\Url]
|
||||||
private ?string $url = null;
|
private ?string $url = null;
|
||||||
|
|
||||||
#[ORM\Column(length: 20)]
|
#[ORM\Column(length: 20)]
|
||||||
#[Groups(['client_ticket:read', 'client_ticket:write', 'task:read'])]
|
#[Groups(['client_ticket:read', 'client_ticket:write', 'task:read'])]
|
||||||
|
#[Assert\Choice(choices: self::STATUSES)]
|
||||||
private ?string $status = 'new';
|
private ?string $status = 'new';
|
||||||
|
|
||||||
#[ORM\Column(type: 'text', nullable: true)]
|
#[ORM\Column(type: 'text', nullable: true)]
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ namespace App\Entity;
|
|||||||
|
|
||||||
use App\Repository\GiteaConfigurationRepository;
|
use App\Repository\GiteaConfigurationRepository;
|
||||||
use Doctrine\ORM\Mapping as ORM;
|
use Doctrine\ORM\Mapping as ORM;
|
||||||
|
use Symfony\Component\Validator\Constraints as Assert;
|
||||||
|
|
||||||
#[ORM\Entity(repositoryClass: GiteaConfigurationRepository::class)]
|
#[ORM\Entity(repositoryClass: GiteaConfigurationRepository::class)]
|
||||||
class GiteaConfiguration
|
class GiteaConfiguration
|
||||||
@@ -16,6 +17,7 @@ class GiteaConfiguration
|
|||||||
private ?int $id = null;
|
private ?int $id = null;
|
||||||
|
|
||||||
#[ORM\Column(length: 255, nullable: true)]
|
#[ORM\Column(length: 255, nullable: true)]
|
||||||
|
#[Assert\Url]
|
||||||
private ?string $url = null;
|
private ?string $url = null;
|
||||||
|
|
||||||
#[ORM\Column(type: 'text', nullable: true)]
|
#[ORM\Column(type: 'text', nullable: true)]
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ namespace App\Entity;
|
|||||||
use App\Repository\TaskBookStackLinkRepository;
|
use App\Repository\TaskBookStackLinkRepository;
|
||||||
use DateTimeImmutable;
|
use DateTimeImmutable;
|
||||||
use Doctrine\ORM\Mapping as ORM;
|
use Doctrine\ORM\Mapping as ORM;
|
||||||
|
use Symfony\Component\Validator\Constraints as Assert;
|
||||||
|
|
||||||
#[ORM\Entity(repositoryClass: TaskBookStackLinkRepository::class)]
|
#[ORM\Entity(repositoryClass: TaskBookStackLinkRepository::class)]
|
||||||
#[ORM\UniqueConstraint(name: 'UNIQ_task_bookstack_link', columns: ['task_id', 'bookstack_id', 'bookstack_type'])]
|
#[ORM\UniqueConstraint(name: 'UNIQ_task_bookstack_link', columns: ['task_id', 'bookstack_id', 'bookstack_type'])]
|
||||||
@@ -31,6 +32,7 @@ class TaskBookStackLink
|
|||||||
private string $title;
|
private string $title;
|
||||||
|
|
||||||
#[ORM\Column(length: 500)]
|
#[ORM\Column(length: 500)]
|
||||||
|
#[Assert\Url]
|
||||||
private string $url;
|
private string $url;
|
||||||
|
|
||||||
#[ORM\Column]
|
#[ORM\Column]
|
||||||
|
|||||||
@@ -20,18 +20,26 @@ class ClientTicketRepository extends ServiceEntityRepository
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns the next ticket number for a project, using a row-level lock
|
* Returns the max ticket number for a project, using an advisory lock
|
||||||
* to prevent race conditions when creating tickets concurrently.
|
* to prevent race conditions when creating tickets concurrently.
|
||||||
*/
|
*/
|
||||||
public function findNextNumberForProjectForUpdate(Project $project): int
|
public function findMaxNumberByProjectForUpdate(Project $project): int
|
||||||
{
|
{
|
||||||
$conn = $this->getEntityManager()->getConnection();
|
$conn = $this->getEntityManager()->getConnection();
|
||||||
|
|
||||||
|
// Use PostgreSQL advisory lock instead of FOR UPDATE
|
||||||
|
// because FOR UPDATE is not allowed with aggregate functions in PostgreSQL.
|
||||||
|
// Offset by 1000000 to avoid collision with task locks on the same project ID.
|
||||||
|
$conn->executeStatement(
|
||||||
|
'SELECT pg_advisory_xact_lock(:lockKey)',
|
||||||
|
['lockKey' => $project->getId() + 1000000],
|
||||||
|
);
|
||||||
|
|
||||||
$result = $conn->fetchOne(
|
$result = $conn->fetchOne(
|
||||||
'SELECT COALESCE(MAX(number), 0) FROM client_ticket WHERE project_id = :project FOR UPDATE',
|
'SELECT COALESCE(MAX(number), 0) FROM client_ticket WHERE project_id = :project',
|
||||||
['project' => $project->getId()],
|
['project' => $project->getId()],
|
||||||
);
|
);
|
||||||
|
|
||||||
return ((int) $result) + 1;
|
return (int) $result;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -20,13 +20,20 @@ class TaskRepository extends ServiceEntityRepository
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns the max task number for a project, using a row-level lock
|
* Returns the max task number for a project, using an advisory lock
|
||||||
* to prevent race conditions when creating tasks concurrently.
|
* to prevent race conditions when creating tasks concurrently.
|
||||||
*/
|
*/
|
||||||
public function findMaxNumberByProjectForUpdate(Project $project): int
|
public function findMaxNumberByProjectForUpdate(Project $project): int
|
||||||
{
|
{
|
||||||
$conn = $this->getEntityManager()->getConnection();
|
$conn = $this->getEntityManager()->getConnection();
|
||||||
|
|
||||||
|
// Use PostgreSQL advisory lock (project ID as lock key) instead of FOR UPDATE
|
||||||
|
// because FOR UPDATE is not allowed with aggregate functions in PostgreSQL.
|
||||||
|
$conn->executeStatement(
|
||||||
|
'SELECT pg_advisory_xact_lock(:project)',
|
||||||
|
['project' => $project->getId()],
|
||||||
|
);
|
||||||
|
|
||||||
$result = $conn->fetchOne(
|
$result = $conn->fetchOne(
|
||||||
'SELECT COALESCE(MAX(number), 0) FROM task WHERE project_id = :project',
|
'SELECT COALESCE(MAX(number), 0) FROM task WHERE project_id = :project',
|
||||||
['project' => $project->getId()],
|
['project' => $project->getId()],
|
||||||
|
|||||||
@@ -53,7 +53,8 @@ final readonly class ClientTicketNumberProcessor implements ProcessorInterface
|
|||||||
|
|
||||||
$now = new DateTimeImmutable();
|
$now = new DateTimeImmutable();
|
||||||
|
|
||||||
$data->setNumber($this->clientTicketRepository->findNextNumberForProjectForUpdate($project));
|
$maxNumber = $this->clientTicketRepository->findMaxNumberByProjectForUpdate($project);
|
||||||
|
$data->setNumber($maxNumber + 1);
|
||||||
$data->setSubmittedBy($user);
|
$data->setSubmittedBy($user);
|
||||||
$data->setStatus('new');
|
$data->setStatus('new');
|
||||||
$data->setCreatedAt($now);
|
$data->setCreatedAt($now);
|
||||||
|
|||||||
Reference in New Issue
Block a user