Files
tinker_tickets/models/DependencyModel.php
T
jaredandClaude Sonnet 5 4fade1a9d3 Reject the semantic inverse of an existing ticket dependency (#51)
addDependency()'s "already exists" check only matched the exact
(ticket_id, depends_on_id, dependency_type) tuple. A user could add
"A blocks B" from ticket A's page, then separately add "B blocked_by
A" from ticket B's page — wouldCreateCycle() correctly found no cycle
(both normalize to the same precedence edge), so the insert was
allowed, creating two DB rows describing one real relationship (shown
twice on ticket B's page: once under Dependencies, once under
Dependents).

Added an inverse-relationship check before the insert: blocks/
blocked_by are inverses of each other, relates_to is its own inverse
(symmetric). duplicates has no defined inverse type in the schema, so
both directions remain independently insertable, which is correct —
"A duplicates B" and "B duplicates A" are distinct claims.

Verified against a local MariaDB instance: the exact repro from the
issue (A blocks B, then B blocked_by A) is now rejected, relates_to's
symmetric case is rejected in both directions, and duplicates in
either direction is unaffected.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
2026-09-08 12:23:04 -04:00

386 lines
14 KiB
PHP

<?php
/**
* DependencyModel - Manages ticket dependencies
*/
class DependencyModel
{
private $conn;
public function __construct($conn)
{
$this->conn = $conn;
}
/**
* Build the extra WHERE fragment (and bound params) that restricts the joined
* ticket alias `t` to tickets the requesting user may see. Reuses
* TicketModel::getVisibilityFilter so the rules stay in one place.
*
* @return array{sql:string,types:string,params:array}
*/
private function buildVisibilityClause($userId, array $userGroups, $isAdmin): array
{
if ($isAdmin) {
return ['sql' => '', 'types' => '', 'params' => []];
}
require_once dirname(__DIR__) . '/models/TicketModel.php';
$ticketModel = new TicketModel($this->conn);
$filter = $ticketModel->getVisibilityFilter([
'user_id' => (int)$userId,
'groups' => implode(',', $userGroups),
'is_admin' => false,
]);
if ($filter['sql'] === '1=1' || $filter['sql'] === '') {
return ['sql' => '', 'types' => '', 'params' => []];
}
return [
'sql' => ' AND ' . $filter['sql'],
'types' => $filter['types'],
'params' => $filter['params'],
];
}
/**
* Get all dependencies for a ticket
*
* The linked ticket's title/status/priority are only returned for tickets the
* requesting user is allowed to see (same rules as TicketModel::getVisibilityFilter).
* With the default (null user, non-admin) only public tickets are exposed.
*
* @param string $ticketId Ticket ID
* @param int|null $userId Requesting user's ID (null = anonymous)
* @param array $userGroups Requesting user's group names
* @param bool $isAdmin Whether the requesting user is an admin (bypasses filtering)
* @return array Dependencies grouped by type
*/
public function getDependencies($ticketId, $userId = null, array $userGroups = [], $isAdmin = false)
{
$visibility = $this->buildVisibilityClause($userId, $userGroups, $isAdmin);
$sql = "SELECT d.*, t.title, t.status, t.priority
FROM ticket_dependencies d
LEFT JOIN tickets t ON d.depends_on_id = t.ticket_id
WHERE d.ticket_id = ?" . $visibility['sql'] . "
ORDER BY d.dependency_type, d.created_at DESC";
$stmt = $this->conn->prepare($sql);
if (!$stmt) {
throw new Exception('Prepare failed: ' . $this->conn->error);
}
$types = 's' . $visibility['types'];
$stmt->bind_param($types, $ticketId, ...$visibility['params']);
if (!$stmt->execute()) {
throw new Exception('Execute failed: ' . $stmt->error);
}
$result = $stmt->get_result();
$dependencies = [
'blocks' => [],
'blocked_by' => [],
'relates_to' => [],
'duplicates' => []
];
while ($row = $result->fetch_assoc()) {
$dependencies[$row['dependency_type']][] = $row;
}
$stmt->close();
return $dependencies;
}
/**
* Get tickets that depend on this ticket
*
* The linked ticket's title/status/priority are only returned for tickets the
* requesting user is allowed to see (same rules as TicketModel::getVisibilityFilter).
* With the default (null user, non-admin) only public tickets are exposed.
*
* @param string $ticketId Ticket ID
* @param int|null $userId Requesting user's ID (null = anonymous)
* @param array $userGroups Requesting user's group names
* @param bool $isAdmin Whether the requesting user is an admin (bypasses filtering)
* @return array Dependent tickets
*/
public function getDependentTickets($ticketId, $userId = null, array $userGroups = [], $isAdmin = false)
{
$visibility = $this->buildVisibilityClause($userId, $userGroups, $isAdmin);
$sql = "SELECT d.*, t.title, t.status, t.priority
FROM ticket_dependencies d
LEFT JOIN tickets t ON d.ticket_id = t.ticket_id
WHERE d.depends_on_id = ?" . $visibility['sql'] . "
ORDER BY d.dependency_type, d.created_at DESC";
$stmt = $this->conn->prepare($sql);
if (!$stmt) {
throw new Exception('Prepare failed: ' . $this->conn->error);
}
$types = 's' . $visibility['types'];
$stmt->bind_param($types, $ticketId, ...$visibility['params']);
if (!$stmt->execute()) {
throw new Exception('Execute failed: ' . $stmt->error);
}
$result = $stmt->get_result();
$dependents = [];
while ($row = $result->fetch_assoc()) {
$dependents[] = $row;
}
$stmt->close();
return $dependents;
}
/**
* Add a dependency between tickets
*
* @param string $ticketId Source ticket ID
* @param string $dependsOnId Target ticket ID
* @param string $type Dependency type
* @param int $createdBy User ID who created the dependency
* @return array Result with success status
*/
public function addDependency($ticketId, $dependsOnId, $type = 'blocks', $createdBy = null)
{
// Validate dependency type
$validTypes = ['blocks', 'blocked_by', 'relates_to', 'duplicates'];
if (!in_array($type, $validTypes)) {
return ['success' => false, 'error' => 'Invalid dependency type'];
}
// Prevent self-reference
if ($ticketId === $dependsOnId) {
return ['success' => false, 'error' => 'A ticket cannot depend on itself'];
}
// Check if dependency already exists
$checkSql = "SELECT dependency_id FROM ticket_dependencies
WHERE ticket_id = ? AND depends_on_id = ? AND dependency_type = ?";
$checkStmt = $this->conn->prepare($checkSql);
$checkStmt->bind_param("sss", $ticketId, $dependsOnId, $type);
$checkStmt->execute();
$checkResult = $checkStmt->get_result();
if ($checkResult->num_rows > 0) {
$checkStmt->close();
return ['success' => false, 'error' => 'Dependency already exists'];
}
$checkStmt->close();
// Also check the semantic inverse: "A blocks B" and "B blocked_by A"
// describe the same relationship, so adding one from either ticket's
// page must be rejected as a duplicate of the other. relates_to is
// its own inverse (symmetric); duplicates has no defined inverse type.
$inverseTypes = ['blocks' => 'blocked_by', 'blocked_by' => 'blocks', 'relates_to' => 'relates_to'];
if (isset($inverseTypes[$type])) {
$inverseType = $inverseTypes[$type];
$checkInverseSql = "SELECT dependency_id FROM ticket_dependencies
WHERE ticket_id = ? AND depends_on_id = ? AND dependency_type = ?";
$checkInverseStmt = $this->conn->prepare($checkInverseSql);
$checkInverseStmt->bind_param("sss", $dependsOnId, $ticketId, $inverseType);
$checkInverseStmt->execute();
$inverseResult = $checkInverseStmt->get_result();
if ($inverseResult->num_rows > 0) {
$checkInverseStmt->close();
return ['success' => false, 'error' => 'This relationship already exists'];
}
$checkInverseStmt->close();
}
// Check for circular dependency
if ($this->wouldCreateCycle($ticketId, $dependsOnId, $type)) {
return ['success' => false, 'error' => 'This would create a circular dependency'];
}
// Insert the dependency
$sql = "INSERT INTO ticket_dependencies (ticket_id, depends_on_id, dependency_type, created_by)
VALUES (?, ?, ?, ?)";
$stmt = $this->conn->prepare($sql);
$stmt->bind_param("sssi", $ticketId, $dependsOnId, $type, $createdBy);
if ($stmt->execute()) {
$dependencyId = $stmt->insert_id;
$stmt->close();
return ['success' => true, 'dependency_id' => $dependencyId];
}
$error = $stmt->error;
$stmt->close();
return ['success' => false, 'error' => $error];
}
/**
* Remove a dependency
*
* @param int $dependencyId Dependency ID
* @return bool Success status
*/
public function removeDependency($dependencyId)
{
$sql = "DELETE FROM ticket_dependencies WHERE dependency_id = ?";
$stmt = $this->conn->prepare($sql);
$stmt->bind_param("i", $dependencyId);
$result = $stmt->execute();
$stmt->close();
return $result;
}
/**
* Remove dependency by ticket IDs and type
*
* @param string $ticketId Source ticket ID
* @param string $dependsOnId Target ticket ID
* @param string $type Dependency type
* @return bool Success status
*/
public function removeDependencyByTickets($ticketId, $dependsOnId, $type)
{
$sql = "DELETE FROM ticket_dependencies
WHERE ticket_id = ? AND depends_on_id = ? AND dependency_type = ?";
$stmt = $this->conn->prepare($sql);
$stmt->bind_param("sss", $ticketId, $dependsOnId, $type);
$result = $stmt->execute();
$stmt->close();
return $result;
}
/** Maximum depth for cycle detection to prevent DoS */
private const MAX_DEPENDENCY_DEPTH = 20;
/**
* Check if adding a dependency would create a cycle
*
* @param string $ticketId Source ticket ID
* @param string $dependsOnId Target ticket ID
* @param string $type Dependency type
* @return bool True if it would create a cycle
*/
private function wouldCreateCycle($ticketId, $dependsOnId, $type): bool
{
// Only blocking relationships impose an ordering that can form a cycle.
if (!in_array($type, ['blocks', 'blocked_by'])) {
return false;
}
// Normalize the new row to a precedence edge "from must finish before to":
// (t, d, 'blocks') => t blocks d => edge t -> d
// (t, d, 'blocked_by') => t blocked_by d => edge d -> t
if ($type === 'blocks') {
$from = $ticketId;
$to = $dependsOnId;
} else { // blocked_by
$from = $dependsOnId;
$to = $ticketId;
}
// Adding edge from->to creates a cycle iff a path to ->* from already exists.
$visited = [];
return $this->hasDependencyPath($to, $from, $visited, 0);
}
/**
* Check if there's a dependency path from source to target
*
* Uses iterative BFS approach with depth limit to prevent stack overflow
* and DoS attacks from deeply nested or circular dependencies.
*
* @param string $source Source ticket ID
* @param string $target Target ticket ID
* @param array $visited Already visited tickets (passed by reference for efficiency)
* @param int $depth Current recursion depth
* @return bool True if path exists
*/
private function hasDependencyPath($source, $target, array &$visited, int $depth): bool
{
// Depth limit to prevent DoS and stack overflow
if ($depth >= self::MAX_DEPENDENCY_DEPTH) {
error_log("Dependency cycle detection hit max depth ({$depth}) from {$source} to {$target}");
return false; // Assume no cycle to avoid blocking legitimate operations
}
if ($source === $target) {
return true;
}
if (in_array($source, $visited, true)) {
return false;
}
// Limit visited array size to prevent memory exhaustion
if (count($visited) > 100) {
error_log("Dependency cycle detection visited too many nodes from {$source} to {$target}");
return false;
}
$visited[] = $source;
// Walk the unified precedence graph forward from $source. Both directions
// of expression contribute an outgoing edge "$source must finish before X":
// blocks rows where ticket_id=$source -> X = depends_on_id
// blocked_by rows where depends_on_id=$source -> X = ticket_id
$sql = "SELECT depends_on_id AS next_id FROM ticket_dependencies
WHERE ticket_id = ? AND dependency_type = 'blocks'
UNION
SELECT ticket_id AS next_id FROM ticket_dependencies
WHERE depends_on_id = ? AND dependency_type = 'blocked_by'";
$stmt = $this->conn->prepare($sql);
$stmt->bind_param("ss", $source, $source);
$stmt->execute();
$result = $stmt->get_result();
while ($row = $result->fetch_assoc()) {
if ($this->hasDependencyPath($row['next_id'], $target, $visited, $depth + 1)) {
$stmt->close();
return true;
}
}
$stmt->close();
return false;
}
/**
* Get all dependencies for multiple tickets (batch)
*
* @param array $ticketIds Array of ticket IDs
* @return array Dependencies indexed by ticket ID
*/
public function getDependenciesBatch($ticketIds)
{
if (empty($ticketIds)) {
return [];
}
$placeholders = str_repeat('?,', count($ticketIds) - 1) . '?';
$sql = "SELECT d.*, t.title, t.status, t.priority
FROM ticket_dependencies d
JOIN tickets t ON d.depends_on_id = t.ticket_id
WHERE d.ticket_id IN ($placeholders)
ORDER BY d.ticket_id, d.dependency_type";
$stmt = $this->conn->prepare($sql);
$types = str_repeat('s', count($ticketIds));
$stmt->bind_param($types, ...$ticketIds);
$stmt->execute();
$result = $stmt->get_result();
$dependencies = [];
while ($row = $result->fetch_assoc()) {
$ticketId = $row['ticket_id'];
if (!isset($dependencies[$ticketId])) {
$dependencies[$ticketId] = [];
}
$dependencies[$ticketId][] = $row;
}
$stmt->close();
return $dependencies;
}
}