diff --git a/database/factories/TaskFactory.php b/database/factories/TaskFactory.php index c504dca..002ed93 100644 --- a/database/factories/TaskFactory.php +++ b/database/factories/TaskFactory.php @@ -3,7 +3,7 @@ namespace Relaticle\Flowforge\Database\Factories; use Illuminate\Database\Eloquent\Factories\Factory; -use Relaticle\Flowforge\Services\Rank; +use Relaticle\Flowforge\Services\DecimalPosition; use Relaticle\Flowforge\Tests\Fixtures\Task; class TaskFactory extends Factory @@ -176,15 +176,15 @@ private function generateResearchTitle(): string private function generatePosition(): string { - // Generate positions in a realistic range - static $baseRank = null; + // Generate positions in a realistic range using decimal positions + static $position = null; - if ($baseRank === null) { - $baseRank = Rank::forEmptySequence(); + if ($position === null) { + $position = DecimalPosition::forEmptyColumn(); } else { - $baseRank = Rank::after($baseRank); + $position = DecimalPosition::after($position); } - return $baseRank->get(); + return $position; } } diff --git a/database/migrations/add_unique_position_constraint.php.stub b/database/migrations/add_unique_position_constraint.php.stub new file mode 100644 index 0000000..59031db --- /dev/null +++ b/database/migrations/add_unique_position_constraint.php.stub @@ -0,0 +1,35 @@ +unique(['status_column', 'position_column'], 'unique_position_per_column'); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('your_table_name', function (Blueprint $table) { + $table->dropUnique('unique_position_per_column'); + }); + } +}; diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 260b5e1..de458d6 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -2,7 +2,7 @@ includes: - phpstan-baseline.neon parameters: - level: 4 + level: 5 paths: - src - config diff --git a/resources/dist/flowforge.js b/resources/dist/flowforge.js index 90efd11..065cf2c 100644 --- a/resources/dist/flowforge.js +++ b/resources/dist/flowforge.js @@ -1 +1 @@ -function d({state:l}){return{state:l,isLoading:{},fullyLoaded:{},init(){this.$wire.$on("kanban-items-loaded",t=>{let{columnId:e,isFullyLoaded:i}=t;i&&(this.fullyLoaded[e]=!0)})},handleSortableEnd(t){let e=t.to.sortable.toArray(),i=t.item.getAttribute("x-sortable-item"),a=t.to.getAttribute("data-column-id"),o=t.item;this.setCardState(o,!0);let s=e.indexOf(i),r=s>0?e[s-1]:null,n=sthis.setCardState(o,!1)).catch(()=>this.setCardState(o,!1))},setCardState(t,e){t.style.opacity=e?"0.7":"",t.style.pointerEvents=e?"none":""},isLoadingColumn(t){return this.isLoading[t]||!1},isColumnFullyLoaded(t){return this.fullyLoaded[t]||!1},handleSmoothScroll(t){this.isLoadingColumn(t)||this.isColumnFullyLoaded(t)||(this.isLoading[t]=!0,this.$wire.loadMoreItems(t).then(()=>setTimeout(()=>this.isLoading[t]=!1,100)).catch(()=>this.isLoading[t]=!1))},handleColumnScroll(t,e){if(this.isColumnFullyLoaded(e))return;let{scrollTop:i,scrollHeight:a,clientHeight:o}=t.target;(i+o)/a>=.8&&!this.isLoadingColumn(e)&&this.handleSmoothScroll(e)}}}export{d as default}; +function d({state:l}){return{state:l,isLoading:{},fullyLoaded:{},init(){this.$wire.$on("kanban-items-loaded",t=>{let{columnId:e,isFullyLoaded:i}=t;i&&(this.fullyLoaded[e]=!0)})},handleSortableEnd(t){let e=t.to.sortable.toArray(),i=t.item.getAttribute("x-sortable-item"),a=t.to.getAttribute("data-column-id"),o=t.item;this.setCardState(o,!0);let s=e.indexOf(i),r=s>0?e[s-1]:null,n=sthis.setCardState(o,!1)).catch(()=>this.setCardState(o,!1))},setCardState(t,e){t.style.opacity=e?"0.7":"",t.style.pointerEvents=e?"none":""},isLoadingColumn(t){return this.isLoading[t]||!1},isColumnFullyLoaded(t){return this.fullyLoaded[t]||!1},handleSmoothScroll(t){this.isLoadingColumn(t)||this.isColumnFullyLoaded(t)||(this.isLoading[t]=!0,this.$wire.loadMoreItems(t).then(()=>setTimeout(()=>this.isLoading[t]=!1,100)).catch(()=>this.isLoading[t]=!1))},handleColumnScroll(t,e){if(this.isColumnFullyLoaded(e))return;let{scrollTop:i,scrollHeight:a,clientHeight:o}=t.target;(i+o)/a>=.8&&!this.isLoadingColumn(e)&&this.handleSmoothScroll(e)}}}export{d as default}; diff --git a/resources/js/flowforge.js b/resources/js/flowforge.js index a51357c..9fef938 100644 --- a/resources/js/flowforge.js +++ b/resources/js/flowforge.js @@ -25,7 +25,7 @@ export default function flowforge({state}) { const afterCardId = cardIndex > 0 ? newOrder[cardIndex - 1] : null; const beforeCardId = cardIndex < newOrder.length - 1 ? newOrder[cardIndex + 1] : null; - this.$wire.moveCard(cardId, targetColumn, beforeCardId, afterCardId) + this.$wire.moveCard(cardId, targetColumn, afterCardId, beforeCardId) .then(() => this.setCardState(cardElement, false)) .catch(() => this.setCardState(cardElement, false)); }, diff --git a/src/Commands/DiagnosePositionsCommand.php b/src/Commands/DiagnosePositionsCommand.php new file mode 100644 index 0000000..144e2ee --- /dev/null +++ b/src/Commands/DiagnosePositionsCommand.php @@ -0,0 +1,325 @@ +displayHeader(); + + // Get parameters (interactive or from options) + $model = $this->option('model') ?? text( + label: 'Model class (e.g., App\\Models\\Task)', + required: true, + validate: fn (string $value) => $this->validateModelClass($value) + ); + + $columnField = $this->option('column') ?? text( + label: 'Column identifier field (for grouping)', + placeholder: 'status', + required: true + ); + + $positionField = $this->option('position') ?? text( + label: 'Position field', + default: 'position', + required: true + ); + + // Validate model + if (! class_exists($model)) { + error("Model class '{$model}' does not exist"); + + return self::FAILURE; + } + + $modelInstance = new $model; + if (! $modelInstance instanceof Model) { + error("Class '{$model}' is not an Eloquent model"); + + return self::FAILURE; + } + + // Display configuration + info("Model: {$model}"); + info("Column Identifier: {$columnField}"); + info("Position Identifier: {$positionField}"); + $this->newLine(); + + // Run diagnostics + $issues = []; + + // 1. Check for small gaps (needs rebalancing) + $this->line('Checking position gaps...'); + $gapIssues = $this->checkGaps($modelInstance, $columnField, $positionField); + if (count($gapIssues) > 0) { + $issues = array_merge($issues, $gapIssues); + } + + // 2. Check for position inversions + $this->line('Scanning for position inversions...'); + $inversionIssues = $this->checkInversions($modelInstance, $columnField, $positionField); + if (count($inversionIssues) > 0) { + $issues = array_merge($issues, $inversionIssues); + } + + // 3. Check for duplicates + $this->line('Checking for duplicate positions...'); + $duplicateIssues = $this->checkDuplicates($modelInstance, $columnField, $positionField); + if (count($duplicateIssues) > 0) { + $issues = array_merge($issues, $duplicateIssues); + } + + // 4. Check for null positions + $this->line('Checking for null positions...'); + $nullIssue = $this->checkNullPositions($modelInstance, $positionField); + if ($nullIssue) { + $issues[] = $nullIssue; + } + + $this->newLine(); + + // Display results + if (empty($issues)) { + info('All checks passed! No issues detected.'); + + return self::SUCCESS; + } + + warning(sprintf('Found %d issue(s):', count($issues))); + $this->newLine(); + + foreach ($issues as $index => $issue) { + $this->displayIssue($index + 1, $issue); + } + + return self::FAILURE; + } + + private function displayHeader(): void + { + $this->newLine(); + $this->line('Flowforge Position Diagnostics'); + $this->line('==============================='); + $this->newLine(); + } + + private function validateModelClass(string $value): ?string + { + if (! class_exists($value)) { + return "Model class '{$value}' does not exist"; + } + + if (! is_subclass_of($value, Model::class)) { + return "Class '{$value}' is not an Eloquent model"; + } + + return null; + } + + private function checkGaps(Model $model, string $columnField, string $positionField): array + { + $issues = []; + $columns = $model->query()->distinct()->pluck($columnField)->map(fn ($value) => $value instanceof \BackedEnum ? $value->value : $value); + + foreach ($columns as $column) { + $positions = $model->query() + ->where($columnField, $column) + ->whereNotNull($positionField) + ->orderBy($positionField) + ->orderBy('id') + ->pluck($positionField); + + if ($positions->count() < 2) { + continue; + } + + $smallGaps = 0; + $minGap = null; + + for ($i = 0; $i < $positions->count() - 1; $i++) { + $current = DecimalPosition::normalize($positions[$i]); + $next = DecimalPosition::normalize($positions[$i + 1]); + $gap = DecimalPosition::gap($current, $next); + + if ($minGap === null || DecimalPosition::lessThan($gap, $minGap)) { + $minGap = $gap; + } + + if (DecimalPosition::needsRebalancing($current, $next)) { + $smallGaps++; + } + } + + if ($smallGaps > 0) { + $issues[] = [ + 'type' => 'small_gaps', + 'severity' => 'medium', + 'column' => $column, + 'count' => $smallGaps, + 'min_gap' => $minGap, + ]; + } + } + + if (empty($issues)) { + info(' No small gaps detected (no rebalancing needed)'); + } + + return $issues; + } + + private function checkInversions(Model $model, string $columnField, string $positionField): array + { + $issues = []; + $columns = $model->query()->distinct()->pluck($columnField)->map(fn ($value) => $value instanceof \BackedEnum ? $value->value : $value); + + foreach ($columns as $column) { + $records = $model->query() + ->where($columnField, $column) + ->whereNotNull($positionField) + ->orderBy('id') + ->get(); + + if ($records->count() < 2) { + continue; + } + + $inversions = []; + for ($i = 0; $i < $records->count() - 1; $i++) { + $current = $records[$i]; + $next = $records[$i + 1]; + + $currentPos = DecimalPosition::normalize($current->getAttribute($positionField)); + $nextPos = DecimalPosition::normalize($next->getAttribute($positionField)); + + // Check if positions are inverted (current >= next when they should be current < next) + if (DecimalPosition::compare($currentPos, $nextPos) >= 0) { + $inversions[] = [ + 'current_id' => $current->getKey(), + 'current_pos' => $currentPos, + 'next_id' => $next->getKey(), + 'next_pos' => $nextPos, + ]; + } + } + + if (count($inversions) > 0) { + $issues[] = [ + 'type' => 'inversion', + 'severity' => 'high', + 'column' => $column, + 'count' => count($inversions), + 'examples' => array_slice($inversions, 0, 3), // Show first 3 examples + ]; + } + } + + if (empty($issues)) { + info(' No position inversions detected'); + } + + return $issues; + } + + private function checkDuplicates(Model $model, string $columnField, string $positionField): array + { + $issues = []; + $columns = $model->query()->distinct()->pluck($columnField)->map(fn ($value) => $value instanceof \BackedEnum ? $value->value : $value); + + foreach ($columns as $column) { + $duplicates = DB::table($model->getTable()) + ->select($positionField, DB::raw('COUNT(*) as duplicate_count')) + ->where($columnField, $column) + ->whereNotNull($positionField) + ->groupBy($positionField) + ->havingRaw('COUNT(*) > 1') + ->get(); + + if ($duplicates->count() > 0) { + $issues[] = [ + 'type' => 'duplicate', + 'severity' => 'medium', + 'column' => $column, + 'count' => $duplicates->sum('duplicate_count'), + 'unique_positions' => $duplicates->count(), + ]; + } + } + + if (empty($issues)) { + info(' No duplicate positions detected'); + } + + return $issues; + } + + private function checkNullPositions(Model $model, string $positionField): ?array + { + $nullCount = $model->query()->whereNull($positionField)->count(); + + if ($nullCount === 0) { + info(' No null positions detected'); + + return null; + } + + return [ + 'type' => 'null', + 'severity' => 'low', + 'count' => $nullCount, + ]; + } + + private function displayIssue(int $number, array $issue): void + { + $this->line("Issue #{$number}: " . strtoupper($issue['type'])); + + if ($issue['type'] === 'small_gaps') { + warning(" Found {$issue['count']} position pair(s) with gap below " . DecimalPosition::MIN_GAP . " in column '{$issue['column']}'"); + $this->line(" Minimum gap found: {$issue['min_gap']}"); + $this->line(' This may cause precision issues. Consider running: php artisan flowforge:rebalance-positions'); + $this->newLine(); + } + + if ($issue['type'] === 'inversion') { + error(" Found {$issue['count']} inverted position pair(s) in column '{$issue['column']}':"); + foreach ($issue['examples'] as $example) { + $this->line(" - Record #{$example['current_id']} (pos: {$example['current_pos']}) >= Record #{$example['next_id']} (pos: {$example['next_pos']})"); + } + $this->newLine(); + } + + if ($issue['type'] === 'duplicate') { + warning(" Found {$issue['count']} duplicate positions in column '{$issue['column']}'"); + $this->line(" ({$issue['unique_positions']} unique position values with duplicates)"); + $this->newLine(); + } + + if ($issue['type'] === 'null') { + info(" Found {$issue['count']} records with null positions"); + $this->newLine(); + } + + info(' After fixing issues, run: php artisan flowforge:repair-positions'); + $this->newLine(); + } +} diff --git a/src/Commands/RebalancePositionsCommand.php b/src/Commands/RebalancePositionsCommand.php new file mode 100644 index 0000000..c5ec682 --- /dev/null +++ b/src/Commands/RebalancePositionsCommand.php @@ -0,0 +1,191 @@ +displayHeader(); + + // Get parameters + $model = $this->option('model') ?? text( + label: 'Model class (e.g., App\\Models\\Task)', + required: true, + validate: fn (string $value) => $this->validateModelClass($value) + ); + + $columnField = $this->option('column') ?? text( + label: 'Column identifier field (for grouping)', + placeholder: 'status', + required: true + ); + + $positionField = $this->option('position') ?? text( + label: 'Position field', + default: 'position', + required: true + ); + + // Validate model + if (! class_exists($model)) { + error("Model class '{$model}' does not exist"); + + return self::FAILURE; + } + + $modelInstance = new $model; + if (! $modelInstance instanceof Model) { + error("Class '{$model}' is not an Eloquent model"); + + return self::FAILURE; + } + + // Display configuration + info("Model: {$model}"); + info("Column Identifier: {$columnField}"); + info("Position Identifier: {$positionField}"); + $this->newLine(); + + $rebalancer = new PositionRebalancer; + $query = $model::query(); + + // Check which columns need rebalancing + $specificGroup = $this->option('group'); + + if ($specificGroup) { + // Rebalance specific group + $this->rebalanceGroup($rebalancer, $query, $columnField, $specificGroup, $positionField); + } else { + // Find and rebalance all groups needing it + $this->rebalanceAllNeeded($rebalancer, $query, $columnField, $positionField); + } + + return self::SUCCESS; + } + + private function displayHeader(): void + { + $this->newLine(); + $this->line('Flowforge Position Rebalancer'); + $this->line('============================='); + $this->newLine(); + } + + private function validateModelClass(string $value): ?string + { + if (! class_exists($value)) { + return "Model class '{$value}' does not exist"; + } + + if (! is_subclass_of($value, Model::class)) { + return "Class '{$value}' is not an Eloquent model"; + } + + return null; + } + + private function rebalanceGroup( + PositionRebalancer $rebalancer, + $query, + string $columnField, + string $groupId, + string $positionField + ): void { + // Get current stats + $stats = $rebalancer->getGapStatistics($query, $columnField, $groupId, $positionField); + + $this->line("Column '{$groupId}':"); + $this->line(" Records: {$stats['count']}"); + + if ($stats['min_gap'] !== null) { + $this->line(" Min gap: {$stats['min_gap']}"); + $this->line(" Max gap: {$stats['max_gap']}"); + $this->line(" Small gaps: {$stats['small_gaps']}"); + } + + $this->newLine(); + + if ($this->option('dry-run')) { + info("Dry run - would rebalance {$stats['count']} records in column '{$groupId}'"); + + return; + } + + if (! confirm("Rebalance {$stats['count']} records in column '{$groupId}'?", true)) { + info('Operation cancelled.'); + + return; + } + + $count = $rebalancer->rebalanceColumn($query, $columnField, $groupId, $positionField); + info("Rebalanced {$count} records in column '{$groupId}'"); + } + + private function rebalanceAllNeeded( + PositionRebalancer $rebalancer, + $query, + string $columnField, + string $positionField + ): void { + $columnsNeedingRebalancing = $rebalancer->findColumnsNeedingRebalancing( + $query, + $columnField, + $positionField + ); + + if ($columnsNeedingRebalancing->isEmpty()) { + info('No columns need rebalancing. All gap sizes are healthy.'); + + return; + } + + $this->line(sprintf('Found %d column(s) needing rebalancing:', $columnsNeedingRebalancing->count())); + $this->newLine(); + + foreach ($columnsNeedingRebalancing as $columnId) { + $stats = $rebalancer->getGapStatistics($query, $columnField, (string) $columnId, $positionField); + $this->line(" - {$columnId}: {$stats['count']} records, {$stats['small_gaps']} small gaps"); + } + + $this->newLine(); + + if ($this->option('dry-run')) { + info('Dry run - no changes applied'); + + return; + } + + if (! confirm('Rebalance all columns listed above?', true)) { + info('Operation cancelled.'); + + return; + } + + $results = $rebalancer->rebalanceAll($query, $columnField, $positionField); + + $this->newLine(); + info('Rebalancing complete:'); + foreach ($results as $columnId => $count) { + $this->line(" - {$columnId}: {$count} records rebalanced"); + } + } +} diff --git a/src/Commands/RepairPositionsCommand.php b/src/Commands/RepairPositionsCommand.php index a9f08a6..1dbf6d5 100644 --- a/src/Commands/RepairPositionsCommand.php +++ b/src/Commands/RepairPositionsCommand.php @@ -7,7 +7,7 @@ use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Facades\DB; -use Relaticle\Flowforge\Services\Rank; +use Relaticle\Flowforge\Services\DecimalPosition; use function Laravel\Prompts\confirm; use function Laravel\Prompts\info; @@ -325,15 +325,17 @@ private function getDuplicatePositions(Builder $query, string $positionField): a private function generatePositions(iterable $records, string $strategy): array { $positions = []; - $lastRank = null; + $lastPosition = null; foreach ($records as $record) { $positionValue = $record instanceof Model ? $record->getAttribute('position') : $record->position ?? null; if ($strategy === 'regenerate' || is_null($positionValue)) { - $rank = $lastRank ? Rank::after($lastRank) : Rank::forEmptySequence(); + $newPosition = $lastPosition !== null + ? DecimalPosition::after($lastPosition) + : DecimalPosition::forEmptyColumn(); $recordId = $record instanceof Model ? $record->getKey() : $record->id; - $positions[$recordId] = $rank->get(); - $lastRank = $rank; + $positions[$recordId] = $newPosition; + $lastPosition = $newPosition; } } diff --git a/src/Concerns/HasBoardRecords.php b/src/Concerns/HasBoardRecords.php index 9188f2b..e5dba02 100644 --- a/src/Concerns/HasBoardRecords.php +++ b/src/Concerns/HasBoardRecords.php @@ -84,7 +84,8 @@ public function getBoardRecords(string $columnId): Collection $positionField = $this->getPositionIdentifierAttribute(); if ($positionField && $this->modelHasColumn($queryClone->getModel(), $positionField)) { - $queryClone->orderBy($positionField, 'asc'); + $queryClone->orderBy($positionField, 'asc') + ->orderBy('id', 'asc'); // Tie-breaker for deterministic order } return $queryClone->limit($limit)->get(); @@ -175,6 +176,7 @@ public function getRecordsBeforePosition(string $columnId, string $position, int ->where($statusField, $columnId) ->where($positionField, '<', $position) ->orderBy($positionField, 'desc') + ->orderBy('id', 'desc') // Tie-breaker for deterministic order ->limit($limit) ->get(); } @@ -197,6 +199,7 @@ public function getRecordsAfterPosition(string $columnId, string $position, int ->where($statusField, $columnId) ->where($positionField, '>', $position) ->orderBy($positionField, 'asc') + ->orderBy('id', 'asc') // Tie-breaker for deterministic order ->limit($limit) ->get(); } @@ -218,6 +221,7 @@ public function getLastPositionInColumn(string $columnId): ?string $record = (clone $query) ->where($statusField, $columnId) ->orderBy($positionField, 'desc') + ->orderBy('id', 'desc') // Tie-breaker for deterministic order ->first(); return $record?->getAttribute($positionField); diff --git a/src/Concerns/HasCardSchema.php b/src/Concerns/HasCardSchema.php index 793dfe5..20f8b5d 100644 --- a/src/Concerns/HasCardSchema.php +++ b/src/Concerns/HasCardSchema.php @@ -32,6 +32,7 @@ public function getCardSchema(Model $record): ?Schema } $livewire = $this->getLivewire(); + /** @phpstan-ignore argument.type (Filament Schema expects HasSchemas&Livewire\Component but getLivewire returns HasBoard) */ $schema = Schema::make($livewire)->record($record); return $this->evaluate($this->cardSchemaBuilder, ['schema' => $schema]); @@ -42,6 +43,7 @@ public function getCardSchema(Model $record): ?Schema */ protected function resolveDefaultClosureDependencyForEvaluationByName(string $parameterName): array { + /** @phpstan-ignore argument.type (Filament Schema expects HasSchemas&Livewire\Component but getLivewire returns HasBoard) */ return match ($parameterName) { 'schema' => [Schema::make($this->getLivewire())], default => parent::resolveDefaultClosureDependencyForEvaluationByName($parameterName), diff --git a/src/Concerns/InteractsWithBoard.php b/src/Concerns/InteractsWithBoard.php index cb93bd9..eed9e6b 100644 --- a/src/Concerns/InteractsWithBoard.php +++ b/src/Concerns/InteractsWithBoard.php @@ -8,10 +8,14 @@ use Filament\Actions\ActionGroup; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\QueryException; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Log; use InvalidArgumentException; use Relaticle\Flowforge\Board; -use Relaticle\Flowforge\Services\Rank; +use Relaticle\Flowforge\Exceptions\MaxRetriesExceededException; +use Relaticle\Flowforge\Services\DecimalPosition; +use Relaticle\Flowforge\Services\PositionRebalancer; use Throwable; trait InteractsWithBoard @@ -73,7 +77,7 @@ protected function makeBoard(): Board } /** - * Move card to new position using Rank-based positioning. + * Move card to new position using decimal-based positioning. * * @throws Throwable */ @@ -95,20 +99,8 @@ public function moveCard( throw new InvalidArgumentException("Card not found: {$cardId}"); } - // Calculate new position using Rank service - $newPosition = $this->calculatePositionBetweenCards($afterCardId, $beforeCardId, $targetColumnId); - - // Use transaction for data consistency - DB::transaction(function () use ($card, $board, $targetColumnId, $newPosition) { - $columnIdentifier = $board->getColumnIdentifierAttribute(); - $columnValue = $this->resolveStatusValue($card, $columnIdentifier, $targetColumnId); - $positionIdentifier = $board->getPositionIdentifierAttribute(); - - $card->update([ - $columnIdentifier => $columnValue, - $positionIdentifier => $newPosition, - ]); - }); + // Calculate and update position with automatic retry on conflicts + $newPosition = $this->calculateAndUpdatePositionWithRetry($card, $targetColumnId, $afterCardId, $beforeCardId); // Emit success event after successful transaction $this->dispatch('kanban-card-moved', [ @@ -118,6 +110,198 @@ public function moveCard( ]); } + /** + * Calculate position and update card within transaction with pessimistic locking. + * This prevents race conditions when multiple users drag cards simultaneously. + */ + protected function calculateAndUpdatePosition( + Model $card, + string $targetColumnId, + ?string $afterCardId, + ?string $beforeCardId + ): string { + $newPosition = ''; + + DB::transaction(function () use ($card, $targetColumnId, $afterCardId, $beforeCardId, &$newPosition) { + $board = $this->getBoard(); + $query = $board->getQuery(); + $positionField = $board->getPositionIdentifierAttribute(); + $columnField = $board->getColumnIdentifierAttribute(); + + // LOCK reference cards for reading to prevent stale data + $afterCard = $afterCardId + ? (clone $query)->where('id', $afterCardId)->lockForUpdate()->first() + : null; + + $beforeCard = $beforeCardId + ? (clone $query)->where('id', $beforeCardId)->lockForUpdate()->first() + : null; + + // Get positions from locked cards + $afterPos = $afterCard?->getAttribute($positionField); + $beforePos = $beforeCard?->getAttribute($positionField); + + // Calculate position INSIDE transaction with locked data + $newPosition = $this->calculateDecimalPosition($afterPos, $beforePos, $targetColumnId); + + // Check if rebalancing is needed after this insert + if ($afterPos !== null && $beforePos !== null) { + $afterPosStr = DecimalPosition::normalize($afterPos); + $beforePosStr = DecimalPosition::normalize($beforePos); + + if (DecimalPosition::needsRebalancing($afterPosStr, $beforePosStr)) { + // Rebalance the column - this redistributes positions evenly + $this->rebalanceColumn($targetColumnId); + + // Recalculate position after rebalancing + $afterCard = $afterCardId + ? (clone $query)->where('id', $afterCardId)->lockForUpdate()->first() + : null; + $beforeCard = $beforeCardId + ? (clone $query)->where('id', $beforeCardId)->lockForUpdate()->first() + : null; + + $afterPos = $afterCard?->getAttribute($positionField); + $beforePos = $beforeCard?->getAttribute($positionField); + $newPosition = $this->calculateDecimalPosition($afterPos, $beforePos, $targetColumnId); + } + } + + // Update card position + $columnValue = $this->resolveStatusValue($card, $columnField, $targetColumnId); + + $card->update([ + $columnField => $columnValue, + $positionField => $newPosition, + ]); + }); + + return $newPosition; + } + + /** + * Calculate position using DecimalPosition service. + * + * @param mixed $afterPos Position of card above (null for top) + * @param mixed $beforePos Position of card below (null for bottom) + * @param string $columnId Target column ID + */ + protected function calculateDecimalPosition(mixed $afterPos, mixed $beforePos, string $columnId): string + { + // Handle empty column case + if ($afterPos === null && $beforePos === null) { + return $this->getBoardPositionInColumn($columnId, 'bottom'); + } + + // Normalize positions to strings for BCMath + $afterPosStr = $afterPos !== null ? DecimalPosition::normalize($afterPos) : null; + $beforePosStr = $beforePos !== null ? DecimalPosition::normalize($beforePos) : null; + + return DecimalPosition::calculate($afterPosStr, $beforePosStr); + } + + /** + * Rebalance all positions in a column, redistributing them evenly. + * Called automatically when gap between positions falls below MIN_GAP. + */ + protected function rebalanceColumn(string $columnId): void + { + $board = $this->getBoard(); + $query = $board->getQuery(); + + if (! $query) { + return; + } + + $rebalancer = new PositionRebalancer; + $count = $rebalancer->rebalanceColumn( + $query, + $board->getColumnIdentifierAttribute(), + $columnId, + $board->getPositionIdentifierAttribute() + ); + + Log::info('Flowforge: Auto-rebalanced column due to small gap', [ + 'column' => $columnId, + 'records' => $count, + ]); + } + + /** + * Calculate and update position with automatic retry on conflicts. + * Wraps calculateAndUpdatePosition() with retry logic to handle rare duplicate position conflicts. + */ + protected function calculateAndUpdatePositionWithRetry( + Model $card, + string $targetColumnId, + ?string $afterCardId, + ?string $beforeCardId, + int $maxAttempts = 3 + ): string { + $baseDelay = 50; // milliseconds + $lastException = null; + + for ($attempt = 1; $attempt <= $maxAttempts; $attempt++) { + try { + return $this->calculateAndUpdatePosition( + $card, + $targetColumnId, + $afterCardId, + $beforeCardId + ); + } catch (QueryException $e) { + // Check if this is a unique constraint violation + if (! $this->isDuplicatePositionError($e)) { + throw $e; // Not a duplicate, rethrow + } + + $lastException = $e; + + // Log the conflict for monitoring + Log::info('Position conflict detected, retrying', [ + 'card_id' => $card->getKey(), + 'target_column' => $targetColumnId, + 'attempt' => $attempt, + 'max_attempts' => $maxAttempts, + ]); + + // Max retries reached? + if ($attempt >= $maxAttempts) { + throw new MaxRetriesExceededException( + "Failed to move card after {$maxAttempts} attempts due to position conflicts", + previous: $e + ); + } + + // Exponential backoff: 50ms, 100ms, 200ms + $delay = $baseDelay * pow(2, $attempt - 1); + usleep($delay * 1000); + + // Refresh reference cards before retry (they may have moved) + continue; + } + } + + // Should never reach here + throw $lastException ?? new \RuntimeException('Unexpected retry loop exit'); + } + + /** + * Check if a QueryException is due to unique constraint violation on positions. + */ + protected function isDuplicatePositionError(QueryException $e): bool + { + $errorCode = $e->errorInfo[1] ?? null; + + // SQLite: SQLITE_CONSTRAINT (19) + // MySQL: ER_DUP_ENTRY (1062) + // PostgreSQL: unique_violation (23505) + + return in_array($errorCode, [19, 1062, 23505]) || + str_contains($e->getMessage(), 'unique_position_per_column') || + str_contains($e->getMessage(), 'UNIQUE constraint failed'); + } + public function loadMoreItems(string $columnId, ?int $count = null): void { $count = $count ?? $this->getBoard()->getCardsPerColumn(); @@ -203,7 +387,7 @@ protected function calculatePositionBetweenCards( $query = $this->getBoard()->getQuery(); if (! $query) { - return Rank::forEmptySequence()->get(); + return DecimalPosition::forEmptyColumn(); } $positionField = $this->getBoard()->getPositionIdentifierAttribute(); @@ -214,19 +398,11 @@ protected function calculatePositionBetweenCards( $afterCard = $afterCardId ? (clone $query)->find($afterCardId) : null; $afterPos = $afterCard?->getAttribute($positionField); - if ($beforePos && $afterPos && is_string($beforePos) && is_string($afterPos)) { - return Rank::betweenRanks(Rank::fromString($beforePos), Rank::fromString($afterPos))->get(); - } - - if ($beforePos && is_string($beforePos)) { - return Rank::after(Rank::fromString($beforePos))->get(); - } - - if ($afterPos && is_string($afterPos)) { - return Rank::before(Rank::fromString($afterPos))->get(); - } + // Normalize positions + $afterPosStr = $afterPos !== null ? DecimalPosition::normalize($afterPos) : null; + $beforePosStr = $beforePos !== null ? DecimalPosition::normalize($beforePos) : null; - return Rank::forEmptySequence()->get(); + return DecimalPosition::calculate($afterPosStr, $beforePosStr); } /** @@ -330,7 +506,7 @@ public function getBoardPositionInColumn(string $columnId, string $position = 't { $query = $this->getBoard()->getQuery(); if (! $query) { - return Rank::forEmptySequence()->get(); + return DecimalPosition::forEmptyColumn(); } $board = $this->getBoard(); @@ -343,31 +519,33 @@ public function getBoardPositionInColumn(string $columnId, string $position = 't $firstRecord = $queryClone ->whereNotNull($positionField) ->orderBy($positionField, 'asc') + ->orderBy('id', 'asc') // Tie-breaker for deterministic order ->first(); if ($firstRecord) { $firstPosition = $firstRecord->getAttribute($positionField); - if (is_string($firstPosition)) { - return Rank::before(Rank::fromString($firstPosition))->get(); + if ($firstPosition !== null) { + return DecimalPosition::before(DecimalPosition::normalize($firstPosition)); } } - return Rank::forEmptySequence()->get(); + return DecimalPosition::forEmptyColumn(); } // Get last valid position (ignore null positions) $lastRecord = $queryClone ->whereNotNull($positionField) ->orderBy($positionField, 'desc') + ->orderBy('id', 'desc') // Tie-breaker for deterministic order ->first(); if ($lastRecord) { $lastPosition = $lastRecord->getAttribute($positionField); - if (is_string($lastPosition)) { - return Rank::after(Rank::fromString($lastPosition))->get(); + if ($lastPosition !== null) { + return DecimalPosition::after(DecimalPosition::normalize($lastPosition)); } } - return Rank::forEmptySequence()->get(); + return DecimalPosition::forEmptyColumn(); } } diff --git a/src/Exceptions/InvalidChars.php b/src/Exceptions/InvalidChars.php deleted file mode 100644 index 7893b40..0000000 --- a/src/Exceptions/InvalidChars.php +++ /dev/null @@ -1,17 +0,0 @@ - $chars - */ - public static function forInputRankWithInvalidChars(string $rank, array $chars): self - { - return new self('Rank provided contains an invalid Char. Rank Provided: ' . $rank . ' - Invalid char: ' . implode(', ', $chars)); - } -} diff --git a/src/Exceptions/LastCharCantBeEqualToMinChar.php b/src/Exceptions/LastCharCantBeEqualToMinChar.php deleted file mode 100644 index 70c04b8..0000000 --- a/src/Exceptions/LastCharCantBeEqualToMinChar.php +++ /dev/null @@ -1,22 +0,0 @@ -get() . ') is greater than or equals to Next (' . $next->get() . ')'); - } -} diff --git a/src/FlowforgeServiceProvider.php b/src/FlowforgeServiceProvider.php index 9e9138d..8f51960 100644 --- a/src/FlowforgeServiceProvider.php +++ b/src/FlowforgeServiceProvider.php @@ -8,8 +8,9 @@ use Filament\Support\Facades\FilamentIcon; use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\Blade; -use Illuminate\Support\Facades\DB; +use Relaticle\Flowforge\Commands\DiagnosePositionsCommand; use Relaticle\Flowforge\Commands\MakeKanbanBoardCommand; +use Relaticle\Flowforge\Commands\RebalancePositionsCommand; use Relaticle\Flowforge\Commands\RepairPositionsCommand; use Spatie\LaravelPackageTools\Commands\InstallCommand; use Spatie\LaravelPackageTools\Package; @@ -114,7 +115,9 @@ protected function getAssets(): array protected function getCommands(): array { return [ + DiagnosePositionsCommand::class, MakeKanbanBoardCommand::class, + RebalancePositionsCommand::class, RepairPositionsCommand::class, ]; } @@ -152,16 +155,10 @@ protected function getScriptData(): array */ private function registerBlueprintMacros(): void { + // DECIMAL(20,10) for position - 10 integer digits + 10 decimal places + // Supports ~33 bisections before precision loss, with 65535 gap Blueprint::macro('flowforgePositionColumn', function (string $name = 'position') { - $driver = DB::connection()->getDriverName(); - $column = $this->string($name)->nullable(); - - return match ($driver) { - 'pgsql' => $column->collation('C'), - 'mysql' => $column->collation('utf8mb4_bin'), - 'sqlsrv' => $column->collation('Latin1_General_BIN2'), - default => $column, // No collation needed - BINARY by default, e.g. SQLite - }; + return $this->decimal($name, 20, 10)->nullable(); }); } } diff --git a/src/Services/DecimalPosition.php b/src/Services/DecimalPosition.php new file mode 100644 index 0000000..04a9a82 --- /dev/null +++ b/src/Services/DecimalPosition.php @@ -0,0 +1,302 @@ += before (invalid bounds) + */ + public static function between(string $after, string $before): string + { + if (bccomp($after, $before, self::SCALE) >= 0) { + throw new InvalidArgumentException( + "Invalid bounds: after ({$after}) must be less than before ({$before})" + ); + } + + // Calculate the exact midpoint + $sum = bcadd($after, $before, self::SCALE); + $midpoint = bcdiv($sum, '2', self::SCALE); + + // Calculate the gap between positions + $gap = bcsub($before, $after, self::SCALE); + + // Jitter range: ±(gap * JITTER_FACTOR / 2) + // This gives us ±5% of the gap on each side of midpoint + $jitterRange = bcmul($gap, self::JITTER_FACTOR, self::SCALE); + $halfJitter = bcdiv($jitterRange, '2', self::SCALE); + + // Generate cryptographically secure random jitter + $jitter = self::generateJitter($halfJitter); + + return bcadd($midpoint, $jitter, self::SCALE); + } + + /** + * Calculate exact midpoint between two positions WITHOUT jitter. + * + * Use this for deterministic testing or when exact midpoint is required. + * For production use, prefer between() which adds jitter for concurrent safety. + * + * @param string $after Lower bound position + * @param string $before Upper bound position + * @return string Exact midpoint between bounds + * + * @throws InvalidArgumentException When after >= before (invalid bounds) + */ + public static function betweenExact(string $after, string $before): string + { + if (bccomp($after, $before, self::SCALE) >= 0) { + throw new InvalidArgumentException( + "Invalid bounds: after ({$after}) must be less than before ({$before})" + ); + } + + $sum = bcadd($after, $before, self::SCALE); + + return bcdiv($sum, '2', self::SCALE); + } + + /** + * Calculate position based on adjacent card positions. + * + * @param string|null $afterPos Position of card above (null = insert at top) + * @param string|null $beforePos Position of card below (null = insert at bottom) + */ + public static function calculate(?string $afterPos, ?string $beforePos): string + { + // Empty column - return default position + if ($afterPos === null && $beforePos === null) { + return self::forEmptyColumn(); + } + + // Insert between two cards - midpoint (never fails!) + if ($afterPos !== null && $beforePos !== null) { + return self::between($afterPos, $beforePos); + } + + // Insert at top - subtract gap + if ($beforePos !== null) { + return self::before($beforePos); + } + + // Insert at bottom - add gap + return self::after($afterPos); + } + + /** + * Check if gap between positions is too small and needs rebalancing. + */ + public static function needsRebalancing(string $afterPos, string $beforePos): bool + { + $gap = bcsub($beforePos, $afterPos, self::SCALE); + + return bccomp($gap, self::MIN_GAP, self::SCALE) < 0; + } + + /** + * Generate sequential positions for rebalancing a column. + * + * @param int $count Number of positions to generate + * @return array Array of evenly-spaced positions + */ + public static function generateSequence(int $count): array + { + $positions = []; + for ($i = 1; $i <= $count; $i++) { + $positions[] = bcmul((string) $i, self::DEFAULT_GAP, self::SCALE); + } + + return $positions; + } + + /** + * Normalize a position string to ensure consistent format. + * Converts numeric values to properly scaled decimal strings. + */ + public static function normalize(string | int | float $position): string + { + return bcadd((string) $position, '0', self::SCALE); + } + + /** + * Compare two positions. + * + * @return int -1 if $a < $b, 0 if equal, 1 if $a > $b + */ + public static function compare(string $a, string $b): int + { + return bccomp($a, $b, self::SCALE); + } + + /** + * Check if position A is less than position B. + */ + public static function lessThan(string $a, string $b): bool + { + return self::compare($a, $b) < 0; + } + + /** + * Check if position A is greater than position B. + */ + public static function greaterThan(string $a, string $b): bool + { + return self::compare($a, $b) > 0; + } + + /** + * Get the gap between two positions. + */ + public static function gap(string $lower, string $upper): string + { + return bcsub($upper, $lower, self::SCALE); + } + + /** + * Generate N positions between two bounds with independent jitter. + * + * Useful for bulk insertions that need multiple unique positions + * within a given range. Each position gets independent jitter. + * + * @param string $after Lower bound position + * @param string $before Upper bound position + * @param int $count Number of positions to generate + * @return array Array of unique positions + */ + public static function generateBetween(string $after, string $before, int $count): array + { + if ($count < 1) { + return []; + } + + $positions = []; + $gap = bcsub($before, $after, self::SCALE); + $step = bcdiv($gap, (string) ($count + 1), self::SCALE); + + for ($i = 1; $i <= $count; $i++) { + $basePosition = bcadd($after, bcmul($step, (string) $i, self::SCALE), self::SCALE); + + // Add jitter to each position (5% of step size) + $jitterRange = bcmul($step, '0.05', self::SCALE); + $jitter = self::generateJitter($jitterRange); + + $positions[] = bcadd($basePosition, $jitter, self::SCALE); + } + + return $positions; + } + + /** + * Generate cryptographically secure random jitter in range [-$maxOffset, +$maxOffset]. + * + * Uses random_bytes() for cryptographic randomness, then scales to the + * desired range using BCMath for precision. + * + * @param string $maxOffset Maximum absolute offset (positive number) + * @return string Random value in [-maxOffset, +maxOffset] + */ + private static function generateJitter(string $maxOffset): string + { + // If maxOffset is zero or very small, return zero + if (bccomp($maxOffset, '0', self::SCALE) <= 0) { + return '0.0000000000'; + } + + // Get 8 random bytes and convert to unsigned 64-bit string + // PHP's unpack('P') returns signed int for values >= 2^63, + // so we manually convert bytes to an unsigned decimal string + $bytes = random_bytes(8); + $randomUnsigned = '0'; + for ($i = 7; $i >= 0; $i--) { + $randomUnsigned = bcmul($randomUnsigned, '256', 0); + $randomUnsigned = bcadd($randomUnsigned, (string) ord($bytes[$i]), 0); + } + + // Normalize to [0, 1] range using 2^64 - 1 as max + $maxUint64 = '18446744073709551615'; // 2^64 - 1 + $normalized = bcdiv($randomUnsigned, $maxUint64, self::SCALE); + + // Scale to [-1, 1] range + $scaled = bcsub(bcmul($normalized, '2', self::SCALE), '1', self::SCALE); + + // Apply to max offset: result in [-maxOffset, +maxOffset] + return bcmul($scaled, $maxOffset, self::SCALE); + } +} diff --git a/src/Services/PositionRebalancer.php b/src/Services/PositionRebalancer.php new file mode 100644 index 0000000..9533d0e --- /dev/null +++ b/src/Services/PositionRebalancer.php @@ -0,0 +1,237 @@ +where($columnField, $columnId) + ->whereNotNull($positionField) + ->orderBy($positionField) + ->orderBy('id') // Tie-breaker for deterministic order + ->get(); + + if ($records->isEmpty()) { + return 0; + } + + $positions = DecimalPosition::generateSequence($records->count()); + + DB::transaction(function () use ($records, $positions, $positionField) { + foreach ($records as $index => $record) { + /** @var Model $record */ + $record->update([$positionField => $positions[$index]]); + } + }); + + Log::info('Flowforge: Rebalanced column positions', [ + 'column' => $columnId, + 'count' => $records->count(), + ]); + + return $records->count(); + } + + /** + * Check if a column needs rebalancing by scanning for small gaps. + * + * @param Builder $query The base query for the model + * @param string $columnField The field identifying the column + * @param string $columnId The column value to check + * @param string $positionField The field storing positions + */ + public function needsRebalancing( + Builder $query, + string $columnField, + string $columnId, + string $positionField + ): bool { + $positions = (clone $query) + ->where($columnField, $columnId) + ->whereNotNull($positionField) + ->orderBy($positionField) + ->pluck($positionField); + + return $this->hasSmallGaps($positions); + } + + /** + * Find columns that need rebalancing. + * + * @param Builder $query The base query for the model + * @param string $columnField The field identifying the column + * @param string $positionField The field storing positions + * @return Collection Column IDs that need rebalancing + */ + public function findColumnsNeedingRebalancing( + Builder $query, + string $columnField, + string $positionField + ): Collection { + $columns = (clone $query) + ->select($columnField) + ->distinct() + ->pluck($columnField); + + return $columns->filter(function ($columnId) use ($query, $columnField, $positionField) { + return $this->needsRebalancing($query, $columnField, (string) $columnId, $positionField); + })->values(); + } + + /** + * Rebalance all columns that need it. + * + * @param Builder $query The base query for the model + * @param string $columnField The field identifying the column + * @param string $positionField The field storing positions + * @return array Map of column ID to records rebalanced + */ + public function rebalanceAll( + Builder $query, + string $columnField, + string $positionField + ): array { + $results = []; + + $columnsNeedingRebalancing = $this->findColumnsNeedingRebalancing( + $query, + $columnField, + $positionField + ); + + foreach ($columnsNeedingRebalancing as $columnId) { + $results[(string) $columnId] = $this->rebalanceColumn( + $query, + $columnField, + (string) $columnId, + $positionField + ); + } + + return $results; + } + + /** + * Get gap statistics for a column. + * + * @param Builder $query The base query for the model + * @param string $columnField The field identifying the column + * @param string $columnId The column value to analyze + * @param string $positionField The field storing positions + * @return array{count: int, min_gap: string|null, max_gap: string|null, avg_gap: string|null, small_gaps: int} + */ + public function getGapStatistics( + Builder $query, + string $columnField, + string $columnId, + string $positionField + ): array { + $positions = (clone $query) + ->where($columnField, $columnId) + ->whereNotNull($positionField) + ->orderBy($positionField) + ->pluck($positionField) + ->map(fn ($p) => DecimalPosition::normalize($p)) + ->values(); + + if ($positions->count() < 2) { + return [ + 'count' => $positions->count(), + 'min_gap' => null, + 'max_gap' => null, + 'avg_gap' => null, + 'small_gaps' => 0, + ]; + } + + $gaps = []; + $smallGapCount = 0; + + for ($i = 1; $i < $positions->count(); $i++) { + $gap = DecimalPosition::gap($positions[$i - 1], $positions[$i]); + $gaps[] = $gap; + + if (bccomp($gap, DecimalPosition::MIN_GAP, DecimalPosition::SCALE) < 0) { + $smallGapCount++; + } + } + + // Calculate min/max/avg using bcmath + $minGap = $gaps[0]; + $maxGap = $gaps[0]; + $totalGap = '0'; + + foreach ($gaps as $gap) { + if (bccomp($gap, $minGap, DecimalPosition::SCALE) < 0) { + $minGap = $gap; + } + if (bccomp($gap, $maxGap, DecimalPosition::SCALE) > 0) { + $maxGap = $gap; + } + $totalGap = bcadd($totalGap, $gap, DecimalPosition::SCALE); + } + + $avgGap = bcdiv($totalGap, (string) count($gaps), DecimalPosition::SCALE); + + return [ + 'count' => $positions->count(), + 'min_gap' => $minGap, + 'max_gap' => $maxGap, + 'avg_gap' => $avgGap, + 'small_gaps' => $smallGapCount, + ]; + } + + /** + * Check if a collection of positions has any gaps below MIN_GAP. + * + * @param Collection $positions + */ + private function hasSmallGaps(Collection $positions): bool + { + if ($positions->count() < 2) { + return false; + } + + $normalized = $positions + ->map(fn ($p) => DecimalPosition::normalize($p)) + ->values(); + + for ($i = 1; $i < $normalized->count(); $i++) { + if (DecimalPosition::needsRebalancing($normalized[$i - 1], $normalized[$i])) { + return true; + } + } + + return false; + } +} diff --git a/src/Services/Rank.php b/src/Services/Rank.php deleted file mode 100644 index 0faec0a..0000000 --- a/src/Services/Rank.php +++ /dev/null @@ -1,194 +0,0 @@ -rank = $rank; - } - - /** - * @param non-empty-string $rank - */ - private static function rankValidator(string $rank): void - { - if (strlen($rank) > self::MAX_RANK_LEN) { - throw MaxRankLength::forInputRank($rank, self::MAX_RANK_LEN); - } - - $invalidChars = array_filter( - str_split($rank), - static function ($char) { - return ord($char) < ord(self::MIN_CHAR) || ord($char) > ord(self::MAX_CHAR); - } - ); - - if ($invalidChars !== []) { - throw InvalidChars::forInputRankWithInvalidChars($rank, array_values($invalidChars)); - } - - $lastChar = substr($rank, -1); - if ($lastChar === self::MIN_CHAR) { - throw LastCharCantBeEqualToMinChar::forRank($rank, self::MIN_CHAR); - } - } - - /** - * @return non-empty-string - */ - public function get(): string - { - return $this->rank; - } - - /** - * @param non-empty-string $rank - */ - public static function fromString(string $rank): self - { - return new self($rank); - } - - public static function forEmptySequence(): self - { - return self::fromString(self::mid(self::MIN_CHAR, self::MAX_CHAR)); - } - - public static function after(self $prevRank): self - { - $char = substr($prevRank->get(), -1); - - if (ord($char) + 1 >= ord(self::MAX_CHAR)) { - return self::fromString( - $prevRank->get() . chr(ord(self::MIN_CHAR) + 1) - ); - } - - $return = substr($prevRank->get(), 0, -1) . chr(ord($char) + 1); - - Assert::stringNotEmpty($return); - - return self::fromString($return); - } - - public static function before(self $nextRank): self - { - $char = substr($nextRank->get(), -1); - - if (ord($char) - 1 <= ord(self::MIN_CHAR)) { - $return = substr($nextRank->get(), 0, -1) . chr(ord($char) - 1) . chr(ord(self::MAX_CHAR) - 1); - - Assert::stringNotEmpty($return); - - return self::fromString($return); - } - - $return = substr($nextRank->get(), 0, -1) . chr(ord($char) - 1); - - Assert::stringNotEmpty($return); - - return self::fromString($return); - } - - public static function betweenRanks(self $prevRank, self $nextRank): self - { - if (strcmp($prevRank->get(), $nextRank->get()) >= 0) { - throw PrevGreaterThanOrEquals::betweenRanks($prevRank, $nextRank); - } - - $rank = ''; - $i = 0; - while ($i <= self::MAX_RANK_LEN) { - $prevChar = $prevRank->getChar($i, self::MIN_CHAR); - $nextChar = $nextRank->getChar($i, self::MAX_CHAR); - $i++; - - $midChar = self::mid($prevChar, $nextChar); - if (in_array($midChar, [$prevChar, $nextChar])) { - $rank .= $prevChar; - - continue; - } - - $rank .= $midChar; - - break; - } - - Assert::stringNotEmpty($rank); - - return self::fromString($rank); - } - - /** - * @param 0|positive-int $i - * @param non-empty-string $defaultChar - * @return non-empty-string - */ - private function getChar(int $i, string $defaultChar): string - { - $return = $this->rank[$i] ?? $defaultChar; - - Assert::stringNotEmpty($return); - - return $return; - } - - /** - * @param non-empty-string $prev - * @param non-empty-string $next - * @return non-empty-string - * - * @psalm-pure - */ - private static function mid(string $prev, string $next): string - { - if (ord($prev) >= ord($next)) { - return $prev; - } - - $return = chr((int) ((ord($prev) + ord($next)) / 2)); - - Assert::stringNotEmpty($return); - - return $return; - } -} diff --git a/tests/Datasets/TaskMovements.php b/tests/Datasets/TaskMovements.php deleted file mode 100644 index 823b6ca..0000000 --- a/tests/Datasets/TaskMovements.php +++ /dev/null @@ -1,128 +0,0 @@ - ['todo', 'in_progress'], - 'todo_to_completed' => ['todo', 'completed'], - 'in_progress_to_completed' => ['in_progress', 'completed'], - 'in_progress_to_todo' => ['in_progress', 'todo'], - 'completed_to_todo' => ['completed', 'todo'], - 'completed_to_in_progress' => ['completed', 'in_progress'], -]); - -dataset('rapid_move_sequences', [ - 'indecisive_user' => [['in_progress', 'completed', 'todo']], - 'complex_workflow' => [['completed', 'in_progress', 'todo', 'completed']], - 'back_and_forth' => [['in_progress', 'todo', 'in_progress', 'completed']], - 'same_column_moves' => [['todo', 'todo', 'todo', 'in_progress']], -]); - -dataset('board_sizes', [ - 'small_team' => 25, - 'medium_team' => 50, - 'large_team' => 100, - 'enterprise' => 250, -]); - -dataset('performance_benchmarks', [ - 'small_board' => [10, 0.1], // 10 cards: < 100ms - 'medium_board' => [50, 0.2], // 50 cards: < 200ms - 'large_board' => [100, 0.3], // 100 cards: < 300ms - 'huge_board' => [250, 0.5], // 250 cards: < 500ms -]); - -dataset('reordering_patterns', [ - 'simple_reorder' => [['after', 'before', 'after']], - 'all_before' => [['before', 'before', 'before']], - 'all_after' => [['after', 'after', 'after']], - 'complex_reorder' => [['after', 'before', 'after', 'before', 'after']], -]); - -dataset('cascade_depths', [ - 'light_usage' => 5, - 'normal_usage' => 10, - 'heavy_usage' => 15, - 'extreme_usage' => 25, -]); - -dataset('team_collaboration_scenarios', [ - 'daily_standup' => [ - ['status' => 'in_progress'], // Dev 1 starts task - ['status' => 'in_progress'], // Dev 2 starts task - ['status' => 'completed'], // Dev 3 finishes task - ['status' => 'todo'], // PM moves task back - ], - 'sprint_planning' => [ - ['status' => 'todo'], // Reprioritize - ['status' => 'todo'], // Reprioritize - ['status' => 'in_progress'], // Start urgent task - ['status' => 'completed'], // Complete quick win - ], - 'crisis_response' => [ - ['status' => 'in_progress'], // All hands on critical bug - ['status' => 'in_progress'], - ['status' => 'in_progress'], - ['status' => 'in_progress'], - ], -]); - -dataset('stress_operation_counts', [ - 'light_load' => 50, - 'medium_load' => 100, - 'heavy_load' => 200, -]); - -dataset('edge_case_scenarios', [ - 'move_all_to_first', - 'circular_moves', - 'mass_revert', -]); - -dataset('position_corruption_types', [ - 'null_positions', - 'duplicate_positions', - 'invalid_positions', -]); - -// Production board state factory -function createProductionBoardState(): array -{ - $rank = Rank::forEmptySequence(); - $tasks = []; - - // Generate tasks with proper Rank positions - $taskData = [ - ['Fix critical security vulnerability', 'todo', 'high'], - ['Implement user authentication', 'todo', 'high'], - ['Add payment processing', 'todo', 'high'], - ['Build user dashboard', 'todo', 'medium'], - ['Implement search functionality', 'todo', 'medium'], - ['Add email notifications', 'todo', 'medium'], - ['Add dark mode theme', 'todo', 'low'], - ['Implement keyboard shortcuts', 'todo', 'low'], - ['Optimize database queries', 'in_progress', 'high'], - ['Refactor API endpoints', 'in_progress', 'medium'], - ['Update documentation', 'in_progress', 'low'], - ['Set up CI/CD pipeline', 'completed', 'high'], - ['Implement logging system', 'completed', 'medium'], - ['Create deployment scripts', 'completed', 'medium'], - ]; - - foreach ($taskData as $index => [$title, $status, $priority]) { - if ($index > 0) { - $rank = Rank::after($rank); - } - - $tasks[] = [ - 'title' => $title, - 'status' => $status, - 'order_position' => $rank->get(), - 'priority' => $priority, - ]; - } - - return $tasks; -} - -dataset('production_board_states', fn () => [createProductionBoardState()]); diff --git a/tests/Feature/BlueprintMacroTest.php b/tests/Feature/BlueprintMacroTest.php index b1960e6..87d0916 100644 --- a/tests/Feature/BlueprintMacroTest.php +++ b/tests/Feature/BlueprintMacroTest.php @@ -2,93 +2,34 @@ use Illuminate\Database\Schema\Blueprint; use Illuminate\Database\Schema\ColumnDefinition; -use Illuminate\Support\Facades\DB; - -test('flowforgePositionColumn macro creates position column with correct collation for MySQL', function () { - // Mock MySQL connection - DB::shouldReceive('connection->getDriverName') - ->once() - ->andReturn('mysql'); +test('flowforgePositionColumn macro creates decimal position column', function () { $blueprint = new Blueprint('test_table'); $column = $blueprint->flowforgePositionColumn(); expect($column)->toBeInstanceOf(ColumnDefinition::class) - ->and($column->get('type'))->toBe('string') + ->and($column->get('type'))->toBe('decimal') ->and($column->get('nullable'))->toBeTrue() - ->and($column->get('collation'))->toBe('utf8mb4_bin'); -}); - -test('flowforgePositionColumn macro creates position column with correct collation for PostgreSQL', function () { - // Mock PostgreSQL connection - DB::shouldReceive('connection->getDriverName') - ->once() - ->andReturn('pgsql'); - - $blueprint = new Blueprint('test_table'); - $column = $blueprint->flowforgePositionColumn(); - - expect($column)->toBeInstanceOf(ColumnDefinition::class) - ->and($column->get('type'))->toBe('string') - ->and($column->get('nullable'))->toBeTrue() - ->and($column->get('collation'))->toBe('C'); + ->and($column->get('total'))->toBe(20) + ->and($column->get('places'))->toBe(10); }); test('flowforgePositionColumn macro accepts custom column name', function () { - // Mock MySQL connection - DB::shouldReceive('connection->getDriverName') - ->once() - ->andReturn('mysql'); - $blueprint = new Blueprint('test_table'); $column = $blueprint->flowforgePositionColumn('sort_order'); expect($column)->toBeInstanceOf(ColumnDefinition::class) ->and($column->get('name'))->toBe('sort_order') - ->and($column->get('collation'))->toBe('utf8mb4_bin'); + ->and($column->get('type'))->toBe('decimal') + ->and($column->get('nullable'))->toBeTrue(); }); -test('flowforgePositionColumn macro creates position column with correct collation for SQL Server', function () { - // Mock SQL Server connection - DB::shouldReceive('connection->getDriverName') - ->once() - ->andReturn('sqlsrv'); - +test('flowforgePositionColumn macro creates column with correct precision for 33+ bisections', function () { $blueprint = new Blueprint('test_table'); $column = $blueprint->flowforgePositionColumn(); - expect($column)->toBeInstanceOf(ColumnDefinition::class) - ->and($column->get('type'))->toBe('string') - ->and($column->get('nullable'))->toBeTrue() - ->and($column->get('collation'))->toBe('Latin1_General_BIN2'); -}); - -test('flowforgePositionColumn macro works with SQLite (no collation needed)', function () { - // Mock SQLite connection - DB::shouldReceive('connection->getDriverName') - ->once() - ->andReturn('sqlite'); - - $blueprint = new Blueprint('test_table'); - $column = $blueprint->flowforgePositionColumn(); - - expect($column)->toBeInstanceOf(ColumnDefinition::class) - ->and($column->get('type'))->toBe('string') - ->and($column->get('nullable'))->toBeTrue() - ->and($column->get('collation'))->toBeNull(); // SQLite uses BINARY by default -}); - -test('flowforgePositionColumn macro works with unsupported database driver', function () { - // Mock unsupported driver - DB::shouldReceive('connection->getDriverName') - ->once() - ->andReturn('unknown_driver'); - - $blueprint = new Blueprint('test_table'); - $column = $blueprint->flowforgePositionColumn(); - - expect($column)->toBeInstanceOf(ColumnDefinition::class) - ->and($column->get('type'))->toBe('string') - ->and($column->get('nullable'))->toBeTrue() - ->and($column->get('collation'))->toBeNull(); // Graceful fallback + // DECIMAL(20,10) = 10 integer digits + 10 decimal places + // This supports approximately 33 bisections before hitting MIN_GAP + expect($column->get('total'))->toBe(20) + ->and($column->get('places'))->toBe(10); }); diff --git a/tests/Feature/CardMovementIntegrationTest.php b/tests/Feature/CardMovementIntegrationTest.php new file mode 100644 index 0000000..a7ddc26 --- /dev/null +++ b/tests/Feature/CardMovementIntegrationTest.php @@ -0,0 +1,222 @@ +toBe(DecimalPosition::DEFAULT_GAP); + }); + + test('card at top of column gets position before first card', function () { + // Create existing cards + Task::create(['title' => 'First', 'status' => 'todo', 'order_position' => '65535.0000000000']); + Task::create(['title' => 'Second', 'status' => 'todo', 'order_position' => '131070.0000000000']); + + // Calculate position at top (before first card) + $firstPosition = '65535.0000000000'; + $newPosition = DecimalPosition::calculate(null, $firstPosition); + + expect(bccomp($newPosition, $firstPosition, 10))->toBeLessThan(0); + }); + + test('card at bottom of column gets position after last card', function () { + // Create existing cards + Task::create(['title' => 'First', 'status' => 'todo', 'order_position' => '65535.0000000000']); + Task::create(['title' => 'Last', 'status' => 'todo', 'order_position' => '131070.0000000000']); + + // Calculate position at bottom (after last card) + $lastPosition = '131070.0000000000'; + $newPosition = DecimalPosition::calculate($lastPosition, null); + + expect(bccomp($newPosition, $lastPosition, 10))->toBeGreaterThan(0); + }); + + test('card between two cards gets position in middle', function () { + // Create reference cards + Task::create(['title' => 'First', 'status' => 'todo', 'order_position' => '1000.0000000000']); + Task::create(['title' => 'Third', 'status' => 'todo', 'order_position' => '2000.0000000000']); + + // Calculate position between + $newPosition = DecimalPosition::calculate('1000.0000000000', '2000.0000000000'); + + expect(bccomp($newPosition, '1000.0000000000', 10))->toBeGreaterThan(0) + ->and(bccomp($newPosition, '2000.0000000000', 10))->toBeLessThan(0); + }); +}); + +describe('card movement scenarios', function () { + beforeEach(function () { + // Create a column with 5 cards + Task::create(['title' => 'Card 1', 'status' => 'todo', 'order_position' => '65535.0000000000']); + Task::create(['title' => 'Card 2', 'status' => 'todo', 'order_position' => '131070.0000000000']); + Task::create(['title' => 'Card 3', 'status' => 'todo', 'order_position' => '196605.0000000000']); + Task::create(['title' => 'Card 4', 'status' => 'todo', 'order_position' => '262140.0000000000']); + Task::create(['title' => 'Card 5', 'status' => 'todo', 'order_position' => '327675.0000000000']); + }); + + test('move card from position 5 to position 2', function () { + $card5 = Task::where('title', 'Card 5')->first(); + $card1Position = '65535.0000000000'; + $card2Position = '131070.0000000000'; + + // Calculate new position between Card 1 and Card 2 + $newPosition = DecimalPosition::between($card1Position, $card2Position); + $card5->update(['order_position' => $newPosition]); + + // Verify order + $ordered = Task::where('status', 'todo') + ->orderBy('order_position') + ->pluck('title') + ->toArray(); + + expect($ordered)->toBe(['Card 1', 'Card 5', 'Card 2', 'Card 3', 'Card 4']); + }); + + test('move card from position 1 to end', function () { + $card1 = Task::where('title', 'Card 1')->first(); + $card5Position = '327675.0000000000'; + + // Calculate new position after Card 5 + $newPosition = DecimalPosition::after($card5Position); + $card1->update(['order_position' => $newPosition]); + + // Verify order + $ordered = Task::where('status', 'todo') + ->orderBy('order_position') + ->pluck('title') + ->toArray(); + + expect($ordered)->toBe(['Card 2', 'Card 3', 'Card 4', 'Card 5', 'Card 1']); + }); + + test('move card from middle to top', function () { + $card3 = Task::where('title', 'Card 3')->first(); + $card1Position = '65535.0000000000'; + + // Calculate new position before Card 1 + $newPosition = DecimalPosition::before($card1Position); + $card3->update(['order_position' => $newPosition]); + + // Verify order + $ordered = Task::where('status', 'todo') + ->orderBy('order_position') + ->pluck('title') + ->toArray(); + + expect($ordered)->toBe(['Card 3', 'Card 1', 'Card 2', 'Card 4', 'Card 5']); + }); + + test('move card to different column', function () { + $card3 = Task::where('title', 'Card 3')->first(); + + // Move to empty "in_progress" column + $newPosition = DecimalPosition::forEmptyColumn(); + $card3->update([ + 'status' => 'in_progress', + 'order_position' => $newPosition, + ]); + + // Verify card moved + expect($card3->refresh()->status)->toBe('in_progress') + ->and(DecimalPosition::normalize($card3->order_position))->toBe(DecimalPosition::normalize(DecimalPosition::DEFAULT_GAP)); + + // Verify original column order + $todoOrdered = Task::where('status', 'todo') + ->orderBy('order_position') + ->pluck('title') + ->toArray(); + + expect($todoOrdered)->toBe(['Card 1', 'Card 2', 'Card 4', 'Card 5']); + + // Verify new column + $inProgressOrdered = Task::where('status', 'in_progress') + ->orderBy('order_position') + ->pluck('title') + ->toArray(); + + expect($inProgressOrdered)->toBe(['Card 3']); + }); + + test('multiple moves maintain correct order', function () { + // Perform series of moves + $card5 = Task::where('title', 'Card 5')->first(); + $card1 = Task::where('title', 'Card 1')->first(); + $card3 = Task::where('title', 'Card 3')->first(); + + // Move Card 5 to position 2 + $newPos = DecimalPosition::between('65535.0000000000', '131070.0000000000'); + $card5->update(['order_position' => $newPos]); + + // Move Card 1 to position 4 + $card4Pos = DecimalPosition::normalize(Task::where('title', 'Card 4')->first()->order_position); + $originalCard5Pos = '327675.0000000000'; // Card 5's old position is now unused + $newPos2 = DecimalPosition::between($card4Pos, $originalCard5Pos); + $card1->update(['order_position' => $newPos2]); + + // Verify final order + $ordered = Task::where('status', 'todo') + ->orderBy('order_position') + ->pluck('title') + ->toArray(); + + // Card 5 moved between 1,2 → becomes position 2 + // Card 1 moved after 4 → becomes position 5 + // Order should be: 5, 2, 3, 4, 1 + expect($ordered)->toBe(['Card 5', 'Card 2', 'Card 3', 'Card 4', 'Card 1']); + }); +}); + +describe('edge cases', function () { + test('handles many consecutive insertions at same position', function () { + // Create two reference cards + $card1 = Task::create(['title' => 'Anchor 1', 'status' => 'todo', 'order_position' => '1000.0000000000']); + $card2 = Task::create(['title' => 'Anchor 2', 'status' => 'todo', 'order_position' => '2000.0000000000']); + + // Insert 30 cards between them + for ($i = 0; $i < 30; $i++) { + $pos = DecimalPosition::between('1000.0000000000', '2000.0000000000'); + Task::create([ + 'title' => "Insert {$i}", + 'status' => 'todo', + 'order_position' => $pos, + ]); + } + + // All should be unique and between bounds + $middleCards = Task::where('status', 'todo') + ->where('title', 'like', 'Insert%') + ->pluck('order_position') + ->map(fn ($p) => DecimalPosition::normalize($p)) + ->toArray(); + + expect(array_unique($middleCards))->toHaveCount(30); + + foreach ($middleCards as $pos) { + expect(bccomp($pos, '1000.0000000000', 10))->toBeGreaterThan(0) + ->and(bccomp($pos, '2000.0000000000', 10))->toBeLessThan(0); + } + }); + + test('handles negative positions correctly', function () { + // Create a card at position 0 + $card1 = Task::create(['title' => 'Zero', 'status' => 'todo', 'order_position' => '0.0000000000']); + + // Insert before it (should get negative position) + $negativePos = DecimalPosition::before('0.0000000000'); + $card2 = Task::create(['title' => 'Negative', 'status' => 'todo', 'order_position' => $negativePos]); + + // Verify order + $ordered = Task::where('status', 'todo') + ->orderBy('order_position') + ->pluck('title') + ->toArray(); + + expect($ordered)->toBe(['Negative', 'Zero']); + expect(bccomp(DecimalPosition::normalize($card2->order_position), '0', 10))->toBeLessThan(0); + }); +}); diff --git a/tests/Feature/ColumnColorTest.php b/tests/Feature/ColumnColorTest.php deleted file mode 100644 index 3579fe2..0000000 --- a/tests/Feature/ColumnColorTest.php +++ /dev/null @@ -1,120 +0,0 @@ -color('primary'); - - expect($column->getColor())->toBe('primary'); - }); - - it('can set tailwind colors on columns by name', function () { - $colors = ['red', 'blue', 'green', 'amber', 'purple', 'pink', 'gray']; - - foreach ($colors as $color) { - $column = Column::make('test')->color($color); - expect($column->getColor())->toBe($color); - } - }); - - it('can set Color constants directly', function () { - $column = Column::make('test') - ->color(Color::Red); - - $color = $column->getColor(); - expect($color)->toBeArray() - ->and($color)->toHaveKey(50) - ->and($color)->toHaveKey(500) - ->and($color)->toHaveKey(900); - }); - - it('can set hex colors on columns', function () { - $column = Column::make('test') - ->color('#ff0000'); - - expect($column->getColor())->toBe('#ff0000'); - }); - - it('can set default color on columns', function () { - $column = Column::make('test') - ->defaultColor('gray'); - - expect($column->getColor())->toBe('gray'); - }); - - it('uses default color when no color is set', function () { - $column = Column::make('test') - ->defaultColor('info'); - - expect($column->getColor())->toBe('info'); - }); - - it('prefers explicit color over default color', function () { - $column = Column::make('test') - ->defaultColor('gray') - ->color('primary'); - - expect($column->getColor())->toBe('primary'); - }); -}); - -describe('ColorResolver', function () { - it('resolves semantic colors', function () { - expect(ColorResolver::resolve('primary'))->toBe('primary') - ->and(ColorResolver::resolve('danger'))->toBe('danger') - ->and(ColorResolver::resolve('success'))->toBe('success') - ->and(ColorResolver::isSemantic('primary'))->toBeTrue() - ->and(ColorResolver::isSemantic('danger'))->toBeTrue(); - }); - - it('resolves Tailwind color names', function () { - $redColor = ColorResolver::resolve('red'); - expect($redColor)->toBeArray() - ->and($redColor)->toHaveKey(500) - ->and($redColor[500])->toContain('oklch'); - - $blueColor = ColorResolver::resolve('blue'); - expect($blueColor)->toBeArray() - ->and($blueColor)->toHaveKey(500); - }); - - it('resolves Color constants', function () { - $color = ColorResolver::resolve(Color::Green); - expect($color)->toBeArray() - ->and($color)->toBe(Color::Green) - ->and($color)->toHaveKey(500); - }); - - it('resolves hex colors', function () { - $color = ColorResolver::resolve('#ff0000'); - expect($color)->toBeArray() - ->and($color)->toHaveKey(500); - }); - - it('handles invalid colors gracefully', function () { - expect(ColorResolver::resolve('invalid-color'))->toBeNull() - ->and(ColorResolver::resolve('not-a-color'))->toBeNull() - ->and(ColorResolver::resolve('#gggggg'))->toBeNull() - ->and(ColorResolver::resolve(''))->toBeNull() - ->and(ColorResolver::resolve(null))->toBeNull(); - }); - - it('is case-insensitive for Tailwind colors', function () { - expect(ColorResolver::resolve('RED'))->toBeArray() - ->and(ColorResolver::resolve('Red'))->toBeArray() - ->and(ColorResolver::resolve('red'))->toBeArray(); - }); - - it('correctly identifies semantic vs non-semantic colors', function () { - expect(ColorResolver::isSemantic('primary'))->toBeTrue() - ->and(ColorResolver::isSemantic(Color::Red))->toBeFalse() - ->and(ColorResolver::isSemantic('#ff0000'))->toBeFalse() - ->and(ColorResolver::isSemantic('red'))->toBeFalse(); - }); -}); diff --git a/tests/Feature/ConcurrentPositionInsertionTest.php b/tests/Feature/ConcurrentPositionInsertionTest.php new file mode 100644 index 0000000..0a9f920 --- /dev/null +++ b/tests/Feature/ConcurrentPositionInsertionTest.php @@ -0,0 +1,191 @@ + 'Reference Card A', + 'status' => 'todo', + 'order_position' => '1000.0000000000', + ]); + + Task::create([ + 'title' => 'Reference Card B', + 'status' => 'todo', + 'order_position' => '2000.0000000000', + ]); +}); + +describe('concurrent position insertions', function () { + test('50 insertions at same position produce unique positions', function () { + $afterPos = '1000.0000000000'; + $beforePos = '2000.0000000000'; + $insertedPositions = []; + $failedInserts = 0; + + // Simulate 50 concurrent insertions using the jitter mechanism + for ($i = 0; $i < 50; $i++) { + $position = DecimalPosition::between($afterPos, $beforePos); + + try { + $task = Task::create([ + 'title' => "Concurrent Card {$i}", + 'status' => 'todo', + 'order_position' => $position, + ]); + $insertedPositions[] = $task->order_position; + } catch (\Illuminate\Database\QueryException $e) { + // If we hit a duplicate (extremely rare), count it + $failedInserts++; + } + } + + // All positions should be unique + expect(array_unique($insertedPositions))->toHaveCount(count($insertedPositions)) + ->and($failedInserts)->toBe(0); + + // All positions should be strictly between bounds + foreach ($insertedPositions as $position) { + $posStr = DecimalPosition::normalize($position); + expect(bccomp($posStr, $afterPos, 10))->toBeGreaterThan(0) + ->and(bccomp($posStr, $beforePos, 10))->toBeLessThan(0); + } + + // ORDER BY should give consistent results + $orderedTasks = Task::where('status', 'todo') + ->whereNotIn('id', [1, 2]) // Exclude reference cards + ->orderBy('order_position') + ->get(); + + expect($orderedTasks)->toHaveCount(50); + + // Verify ordering is consistent + $previousPosition = '0'; + foreach ($orderedTasks as $task) { + $posStr = DecimalPosition::normalize($task->order_position); + expect(bccomp($posStr, $previousPosition, 10)) + ->toBeGreaterThan(0); + $previousPosition = $posStr; + } + }); + + test('rapid successive insertions by same user dont collide', function () { + $afterPos = '1000.0000000000'; + $beforePos = '2000.0000000000'; + $insertedCount = 0; + + // Rapid fire 50 insertions without any delay + for ($i = 0; $i < 50; $i++) { + $position = DecimalPosition::between($afterPos, $beforePos); + + try { + Task::create([ + 'title' => "Rapid Card {$i}", + 'status' => 'todo', + 'order_position' => $position, + ]); + $insertedCount++; + } catch (\Illuminate\Database\QueryException) { + // Unique constraint violation - should never happen + } + } + + expect($insertedCount)->toBe(50); + + // Verify all positions are unique + $positions = Task::where('status', 'todo') + ->whereNotIn('id', [1, 2]) + ->pluck('order_position') + ->toArray(); + + expect(array_unique($positions))->toHaveCount(50); + }); + + test('unique constraint actually prevents duplicate positions', function () { + // Insert a card with a specific position + Task::create([ + 'title' => 'First Card', + 'status' => 'todo', + 'order_position' => '1500.0000000000', + ]); + + // Try to insert another card with the exact same position + expect(fn () => Task::create([ + 'title' => 'Duplicate Card', + 'status' => 'todo', + 'order_position' => '1500.0000000000', + ]))->toThrow(\Illuminate\Database\QueryException::class); + }); + + test('positions remain sortable after many insertions', function () { + $afterPos = '1000.0000000000'; + $beforePos = '2000.0000000000'; + + // Insert 100 cards + for ($i = 0; $i < 100; $i++) { + $position = DecimalPosition::between($afterPos, $beforePos); + Task::create([ + 'title' => "Card {$i}", + 'status' => 'todo', + 'order_position' => $position, + ]); + } + + // Verify ORDER BY works correctly + $tasks = Task::where('status', 'todo') + ->orderBy('order_position') + ->get(); + + expect($tasks)->toHaveCount(102); // 100 + 2 reference cards + + // Verify strict ordering + $previousPosition = '-999999'; + foreach ($tasks as $task) { + $posStr = DecimalPosition::normalize($task->order_position); + expect(bccomp($posStr, $previousPosition, 10)) + ->toBeGreaterThan(0); + $previousPosition = $posStr; + } + }); +}); + +describe('position collision statistics', function () { + test('jitter produces statistically unique positions', function () { + $afterPos = '1000.0000000000'; + $beforePos = '2000.0000000000'; + $positions = []; + + // Generate 1000 positions without inserting + for ($i = 0; $i < 1000; $i++) { + $positions[] = DecimalPosition::between($afterPos, $beforePos); + } + + $uniquePositions = array_unique($positions); + + // With cryptographic jitter, we should have 100% unique positions + expect(count($uniquePositions))->toBe(1000); + + // Calculate position distribution around midpoint + $midpoint = DecimalPosition::betweenExact($afterPos, $beforePos); + $belowMidpoint = 0; + $aboveMidpoint = 0; + + foreach ($positions as $position) { + if (bccomp($position, $midpoint, 10) < 0) { + $belowMidpoint++; + } else { + $aboveMidpoint++; + } + } + + // Distribution should be roughly 50/50 (within reasonable variance) + expect($belowMidpoint)->toBeGreaterThan(400) + ->and($belowMidpoint)->toBeLessThan(600) + ->and($aboveMidpoint)->toBeGreaterThan(400) + ->and($aboveMidpoint)->toBeLessThan(600); + }); +}); diff --git a/tests/Feature/DragDropFunctionalityTest.php b/tests/Feature/DragDropFunctionalityTest.php deleted file mode 100644 index b20c056..0000000 --- a/tests/Feature/DragDropFunctionalityTest.php +++ /dev/null @@ -1,511 +0,0 @@ -users = User::factory()->count(6)->create([ - 'team' => 'Development', - ]); - - $this->projects = Project::factory()->count(3)->create([ - 'owner_id' => $this->users->random()->id, - ]); - - // Create tasks across different projects with realistic distribution - $this->tasks = collect(); - - // Project 1: Active development project with mixed priority tasks - $project1Tasks = Task::factory()->count(8)->create([ - 'project_id' => $this->projects->get(0)->id, - 'created_by' => $this->users->random()->id, - 'status' => 'todo', - ]); - $this->tasks = $this->tasks->merge($project1Tasks); - - // Project 2: Tasks in progress - $project2Tasks = Task::factory()->count(3)->create([ - 'project_id' => $this->projects->get(1)->id, - 'created_by' => $this->users->random()->id, - 'status' => 'in_progress', - 'assigned_to' => $this->users->random()->id, - ]); - $this->tasks = $this->tasks->merge($project2Tasks); - - // Project 3: Completed tasks - $project3Tasks = Task::factory()->count(3)->create([ - 'project_id' => $this->projects->get(2)->id, - 'created_by' => $this->users->random()->id, - 'status' => 'completed', - 'assigned_to' => $this->users->random()->id, - 'completed_at' => now()->subDays(rand(1, 30)), - ]); - $this->tasks = $this->tasks->merge($project3Tasks); - - $this->board = Livewire::test(TestBoard::class); - }); - - describe('Core Movement Functionality', function () { - it('executes basic card movements between columns', function (string $fromStatus, string $toStatus) { - $task = Task::where('status', $fromStatus)->first(); - expect($task)->not()->toBeNull(); - - $originalPosition = $task->order_position; - $originalAssignee = $task->assigned_to; - - $this->board->call('moveCard', (string) $task->id, $toStatus); - - $task->refresh(); - expect($task->status)->toBe($toStatus) - ->and($task->order_position)->not()->toBeNull() - ->and($task->assigned_to)->toBe($originalAssignee); // Assignee should not change - - // Position should be valid regardless of column change - expect($task->order_position)->toBeString()->not()->toBeEmpty(); - })->with('workflow_progressions'); - - it('handles rapid sequential movements without data corruption', function (array $moveSequence) { - $task = Task::where('status', 'todo')->first(); - expect($task)->not()->toBeNull(); - - $originalProjectId = $task->project_id; - $originalCreatedBy = $task->created_by; - - foreach ($moveSequence as $status) { - $this->board->call('moveCard', (string) $task->id, $status); - } - - $task->refresh(); - expect($task->status)->toBe(end($moveSequence)) - ->and($task->order_position)->not()->toBeNull()->toBeString() - ->and($task->project_id)->toBe($originalProjectId) - ->and($task->created_by)->toBe($originalCreatedBy); - })->with('rapid_move_sequences'); - - it('maintains all related data integrity during moves', function () { - $task = Task::with(['project', 'assignedUser', 'creator'])->where('status', 'todo')->first(); - - $originalTitle = $task->title; - $originalPriority = $task->priority; - $originalDescription = $task->description; - $originalLabels = $task->labels; - $originalDueDate = $task->due_date; - - $this->board->call('moveCard', (string) $task->id, 'in_progress'); - - $task->refresh(); - expect($task->title)->toBe($originalTitle) - ->and($task->priority)->toBe($originalPriority) - ->and($task->description)->toBe($originalDescription) - ->and($task->labels)->toEqual($originalLabels) - ->and($task->due_date?->format('Y-m-d'))->toBe($originalDueDate?->format('Y-m-d')) - ->and($task->status)->toBe('in_progress'); - }); - }); - - describe('Position-Based Drag & Drop with Real Data', function () { - it('handles complex multi-project positioning', function () { - // Test positioning across different projects in same column - $todoTasks = Task::where('status', 'todo')->with('project')->orderBy('order_position')->get(); - expect($todoTasks)->toHaveCount(8); - - $sourceCard = $todoTasks->first(); - $targetCard = $todoTasks->skip(3)->first(); // Fourth card, possibly different project - - // Move card maintaining project relationships - $this->board->call('moveCard', (string) $sourceCard->id, 'todo', null, (string) $targetCard->id); - - $sourceCard->refresh(); - - // beforeCardId actually places the card AFTER the specified card (based on implementation) - expect(strcmp($sourceCard->order_position, $targetCard->order_position))->toBeGreaterThan(0); - - // Project relationship should remain intact - expect($sourceCard->project_id)->not()->toBeNull(); - }); - - it('maintains proper ordering with mixed projects and priorities', function () { - // Create specific ordering scenario - $highPriorityTask = Task::factory()->create([ - 'status' => 'todo', - 'priority' => 'high', - 'project_id' => $this->projects->first()->id, - ]); - - $mediumPriorityTask = Task::factory()->create([ - 'status' => 'todo', - 'priority' => 'medium', - 'project_id' => $this->projects->last()->id, - ]); - - // Position high priority task before medium priority - $this->board->call('moveCard', (string) $highPriorityTask->id, 'todo', null, (string) $mediumPriorityTask->id); - - $highPriorityTask->refresh(); - $mediumPriorityTask->refresh(); - - expect(strcmp($highPriorityTask->order_position, $mediumPriorityTask->order_position))->toBeGreaterThan(0); - }); - }); - - describe('Production Workflow Scenarios', function () { - it('simulates realistic sprint planning with team assignments', function () { - $developer = $this->users->where('team', 'Development')->first(); - - // Sprint planning: Assign high priority tasks to developer - $sprintTasks = Task::where('status', 'todo') - ->where('priority', 'high') - ->take(3) - ->get(); - - foreach ($sprintTasks as $task) { - // Update assignment and move to in_progress - $task->update(['assigned_to' => $developer->id]); - $this->board->call('moveCard', (string) $task->id, 'in_progress'); - } - - // Verify sprint setup - $inProgressTasks = Task::where('status', 'in_progress')->get(); - expect($inProgressTasks->count())->toBeGreaterThanOrEqual(3); // At least the tasks we just moved - - foreach ($sprintTasks as $task) { - $task->refresh(); - expect($task->status)->toBe('in_progress') - ->and($task->assigned_to)->toBe($developer->id); - } - }); - - it('handles task completion with timestamps and metrics', function () { - $inProgressTask = Task::where('status', 'in_progress')->first(); - $inProgressTask->update([ - 'estimated_hours' => 8, - 'actual_hours' => null, - ]); - - $completionTime = now(); - - // Complete the task - $this->board->call('moveCard', (string) $inProgressTask->id, 'completed'); - - $inProgressTask->refresh(); - expect($inProgressTask->status)->toBe('completed') - ->and($inProgressTask->order_position)->not()->toBeNull(); - - // In real world, completion would update metrics - $inProgressTask->update([ - 'completed_at' => $completionTime, - 'actual_hours' => 10, - ]); - - expect($inProgressTask->completed_at)->not()->toBeNull(); - }); - - it('maintains referential integrity during bulk operations', function () { - // Record initial state with all relationships - $initialState = Task::with(['project', 'assignedUser', 'creator'])->get(); - $initialProjectIds = $initialState->pluck('project_id')->filter()->unique(); - $initialUserIds = $initialState->pluck('assigned_to')->filter()->unique(); - - // Perform bulk moves - $tasks = Task::all(); - for ($i = 0; $i < 25; $i++) { - $task = $tasks->random(); - $newStatus = collect(['todo', 'in_progress', 'completed'])->random(); - $this->board->call('moveCard', (string) $task->id, $newStatus); - } - - // Verify relationships are maintained - $finalState = Task::with(['project', 'assignedUser', 'creator'])->get(); - expect($finalState->count())->toBe($initialState->count()); - - // Project assignments should remain unchanged - $finalProjectIds = $finalState->pluck('project_id')->filter()->unique(); - expect($finalProjectIds->sort()->values()->toArray()) - ->toEqual($initialProjectIds->sort()->values()->toArray()); - - // User assignments should remain unchanged - $finalUserIds = $finalState->pluck('assigned_to')->filter()->unique(); - expect($finalUserIds->sort()->values()->toArray()) - ->toEqual($initialUserIds->sort()->values()->toArray()); - }); - }); - - describe('Real-World Performance & Scale Testing', function () { - it('handles large team boards with multiple projects', function (int $additionalTasks) { - // Add more tasks to simulate large team environment - $projects = $this->projects; - $users = $this->users; - - Task::factory()->count($additionalTasks)->create([ - 'project_id' => $projects->random()->id, - 'assigned_to' => $users->random()->id, - 'created_by' => $users->random()->id, - ]); - - $totalTasks = Task::count(); - expect($totalTasks)->toBeGreaterThan($additionalTasks); - - // Test move performance on large board - $testCard = Task::inRandomOrder()->first(); - $newStatus = collect(['todo', 'in_progress', 'completed'])->random(); - - $startTime = microtime(true); - $this->board->call('moveCard', (string) $testCard->id, $newStatus); - $duration = microtime(true) - $startTime; - - expect($duration)->toBeLessThan(0.5); // Should complete within 500ms - - $testCard->refresh(); - expect($testCard->status)->toBe($newStatus); - })->with([ - 'small_team' => 25, - 'medium_team' => 75, - 'large_team' => 150, - ]); - - it('validates database constraints under stress', function () { - $tasks = Task::all(); - - // Perform focused stress operations - for ($i = 0; $i < 20; $i++) { - $task = $tasks->random(); - $newStatus = collect(['todo', 'in_progress', 'completed'])->random(); - - $this->board->call('moveCard', (string) $task->id, $newStatus); - - // Validate database integrity after each move - $task->refresh(); - expect($task->project_id)->not()->toBeNull() - ->and($task->created_by)->not()->toBeNull(); - } - - // Final integrity check - no orphaned data - $orphanedTasks = Task::whereNull('project_id')->count(); - expect($orphanedTasks)->toBe(0); - }); - }); - - describe('Error Handling & Recovery', function () { - it('handles concurrent modifications gracefully', function () { - $task = Task::first(); - - // Simulate concurrent modification (e.g., another user updates the task) - $task->update(['title' => 'Modified by another user']); - - // Move operation should still work - $this->board->call('moveCard', (string) $task->id, 'in_progress'); - - $task->refresh(); - expect($task->status)->toBe('in_progress') - ->and($task->title)->toBe('Modified by another user'); - }); - - it('maintains foreign key constraints during moves', function () { - $task = Task::with('project')->first(); - $originalProject = $task->project; - - // Move task through all statuses - $this->board->call('moveCard', (string) $task->id, 'in_progress'); - $this->board->call('moveCard', (string) $task->id, 'completed'); - $this->board->call('moveCard', (string) $task->id, 'todo'); - - $task->refresh(); - expect($task->project_id)->toBe($originalProject->id); - - // Verify project relationship still works - expect($task->project->name)->toBe($originalProject->name); - }); - - it('handles invalid references without corrupting data', function () { - expect(fn () => $this->board->call('moveCard', 'nonexistent-id', 'todo')) - ->toThrow(InvalidArgumentException::class); - - // Verify no data corruption occurred - $taskCount = Task::count(); - $userCount = User::count(); - $projectCount = Project::count(); - - expect($taskCount)->toBe($this->tasks->count()) - ->and($userCount)->toBe($this->users->count()) - ->and($projectCount)->toBe($this->projects->count()); - }); - }); - - describe('Advanced Position Management', function () { - it('prevents position collisions in high-frequency scenarios', function () { - // Simulate rapid task creation and movement (like importing tasks) - $newTasks = Task::factory()->count(10)->create([ - 'status' => 'todo', - 'project_id' => $this->projects->first()->id, - ]); - - // Move all new tasks rapidly - foreach ($newTasks as $task) { - $this->board->call('moveCard', (string) $task->id, 'in_progress'); - $this->board->call('moveCard', (string) $task->id, 'todo'); - } - - // Verify no position duplicates - $positions = Task::where('status', 'todo') - ->whereNotNull('order_position') - ->pluck('order_position') - ->toArray(); - - expect(array_unique($positions))->toHaveCount(count($positions)); - }); - - it('maintains ordering consistency with controlled reordering', function () { - // Get current todo tasks to work with - $existingTodos = Task::where('status', 'todo')->count(); - - // Create just a few additional tasks for controlled testing - $newTasks = Task::factory()->count(3)->create([ - 'status' => 'todo', - 'priority' => 'medium', - 'project_id' => $this->projects->first()->id, - ]); - - // Perform simple reordering - move one task at a time - $taskToMove = $newTasks->first(); - $targetTask = $newTasks->last(); - - // Move first task after last task - $this->board->call('moveCard', (string) $taskToMove->id, 'todo', null, (string) $targetTask->id); - - // Verify no duplicate positions in todo column - $todoPositions = Task::where('status', 'todo') - ->whereNotNull('order_position') - ->pluck('order_position') - ->toArray(); - - // Main test: ensure no position collisions - expect(array_unique($todoPositions))->toHaveCount(count($todoPositions)); - - // Verify specific task positioning worked - $taskToMove->refresh(); - $targetTask->refresh(); - expect($taskToMove->order_position)->not()->toBe($targetTask->order_position); - }); - }); - - describe('Team Collaboration Stress Testing', function () { - it('simulates realistic daily workflow with multiple team members', function () { - // Morning standup: Multiple developers pick up work - $developers = $this->users->where('team', 'Development'); - $backlogTasks = Task::where('status', 'todo')->where('priority', 'high')->get(); - - foreach ($backlogTasks->take(3) as $index => $task) { - $developer = $developers->get($index % $developers->count()); - $task->update(['assigned_to' => $developer->id]); - $this->board->call('moveCard', (string) $task->id, 'in_progress'); - } - - // Verify assignments and status changes - $activeWork = Task::where('status', 'in_progress')->get(); - expect($activeWork->count())->toBeGreaterThanOrEqual(3); - - // Mid-day: Some tasks completed, new ones started - $completableTasks = $activeWork->take(2); - foreach ($completableTasks as $task) { - $this->board->call('moveCard', (string) $task->id, 'completed'); - $task->update(['completed_at' => now()]); - } - - // End of day: Verify team productivity - $completedToday = Task::where('status', 'completed') - ->whereNotNull('completed_at') - ->get(); - - expect($completedToday->count())->toBeGreaterThanOrEqual(5); - }); - - it('handles project-based task isolation correctly', function () { - $project1 = $this->projects->first(); - $project2 = $this->projects->last(); - - // Move tasks from project 1 - $project1Tasks = Task::where('project_id', $project1->id)->get(); - foreach ($project1Tasks as $task) { - $this->board->call('moveCard', (string) $task->id, 'in_progress'); - } - - // Move tasks from project 2 - $project2Tasks = Task::where('project_id', $project2->id)->get(); - foreach ($project2Tasks as $task) { - $this->board->call('moveCard', (string) $task->id, 'completed'); - } - - // Verify project isolation is maintained - $project1TasksAfter = Task::where('project_id', $project1->id)->get(); - $project2TasksAfter = Task::where('project_id', $project2->id)->get(); - - expect($project1TasksAfter->every(fn ($task) => $task->status === 'in_progress'))->toBeTrue(); - expect($project2TasksAfter->every(fn ($task) => $task->status === 'completed'))->toBeTrue(); - }); - }); - - describe('Production Data Integrity & Constraints', function () { - it('validates all foreign key relationships remain intact', function () { - $allTasks = Task::with(['project', 'assignedUser', 'creator'])->get(); - - // Perform controlled moves (reduced for performance) - for ($i = 0; $i < 25; $i++) { - $task = $allTasks->random(); - $newStatus = collect(['todo', 'in_progress', 'completed'])->random(); - $this->board->call('moveCard', (string) $task->id, $newStatus); - } - - // Comprehensive relationship validation - $finalTasks = Task::with(['project', 'assignedUser', 'creator'])->get(); - - foreach ($finalTasks as $task) { - // Core kanban fields should be valid - expect($task->status)->toBeIn(['todo', 'in_progress', 'completed']) - ->and($task->order_position)->not()->toBeNull(); - - // Relationships should be resolvable (no broken foreign keys) - if ($task->project_id) { - expect($task->project)->not()->toBeNull(); - } - if ($task->assigned_to) { - expect($task->assignedUser)->not()->toBeNull(); - } - if ($task->created_by) { - expect($task->creator)->not()->toBeNull(); - } - } - }); - - it('handles database constraints during edge case operations', function () { - // Test with tasks that have complex constraint scenarios - $constrainedTask = Task::factory()->create([ - 'status' => 'todo', - 'assigned_to' => $this->users->first()->id, - 'project_id' => $this->projects->first()->id, - 'due_date' => now()->addDays(7), - 'labels' => ['critical', 'security', 'hotfix'], - ]); - - // Multiple rapid moves with constrained data - for ($i = 0; $i < 10; $i++) { - $status = collect(['todo', 'in_progress', 'completed'])->random(); - $this->board->call('moveCard', (string) $constrainedTask->id, $status); - } - - $constrainedTask->refresh(); - - // All constraints should still be satisfied - expect($constrainedTask->assignedUser)->not()->toBeNull() - ->and($constrainedTask->project)->not()->toBeNull() - ->and($constrainedTask->labels)->toBeArray() - ->and($constrainedTask->due_date)->not()->toBeNull(); - }); - }); -}); diff --git a/tests/Feature/PerformanceTest.php b/tests/Feature/PerformanceTest.php new file mode 100644 index 0000000..107639a --- /dev/null +++ b/tests/Feature/PerformanceTest.php @@ -0,0 +1,148 @@ +toBeLessThan(0.5); // Allow some margin + }); + + test('10,000 exact midpoint calculations complete in < 50ms', function () { + $start = microtime(true); + + for ($i = 0; $i < 10_000; $i++) { + DecimalPosition::betweenExact('1000.0000000000', '2000.0000000000'); + } + + $elapsed = microtime(true) - $start; + + expect($elapsed)->toBeLessThan(0.1); + }); + + test('10,000 normalize operations complete in < 50ms', function () { + $values = ['1000', '1000.5', 1000, 1000.5, '-500']; + $start = microtime(true); + + for ($i = 0; $i < 10_000; $i++) { + DecimalPosition::normalize($values[$i % 5]); + } + + $elapsed = microtime(true) - $start; + + expect($elapsed)->toBeLessThan(0.1); + }); +}); + +describe('PositionRebalancer performance', function () { + test('rebalancing 100 cards completes in < 2 seconds', function () { + // Create 100 cards with positions + for ($i = 0; $i < 100; $i++) { + Task::create([ + 'title' => "Card {$i}", + 'status' => 'todo', + 'order_position' => DecimalPosition::normalize($i * 100), + ]); + } + + $rebalancer = new PositionRebalancer; + + $start = microtime(true); + $count = $rebalancer->rebalanceColumn( + Task::query(), + 'status', + 'todo', + 'order_position' + ); + $elapsed = microtime(true) - $start; + + expect($count)->toBe(100) + ->and($elapsed)->toBeLessThan(2.0); + }); + + test('gap statistics on 100 cards completes in < 500ms', function () { + // Create 100 cards with positions + for ($i = 0; $i < 100; $i++) { + Task::create([ + 'title' => "Card {$i}", + 'status' => 'todo', + 'order_position' => DecimalPosition::normalize($i * 100), + ]); + } + + $rebalancer = new PositionRebalancer; + + $start = microtime(true); + $stats = $rebalancer->getGapStatistics( + Task::query(), + 'status', + 'todo', + 'order_position' + ); + $elapsed = microtime(true) - $start; + + expect($stats['count'])->toBe(100) + ->and($elapsed)->toBeLessThan(0.5); + }); +}); + +describe('sequence generation performance', function () { + test('generating 1000 sequential positions completes in < 50ms', function () { + $start = microtime(true); + + $positions = DecimalPosition::generateSequence(1000); + + $elapsed = microtime(true) - $start; + + expect($positions)->toHaveCount(1000) + ->and($elapsed)->toBeLessThan(0.1); + }); + + test('generating 100 between positions completes in < 50ms', function () { + $start = microtime(true); + + $positions = DecimalPosition::generateBetween('1000', '2000', 100); + + $elapsed = microtime(true) - $start; + + expect($positions)->toHaveCount(100) + ->and($elapsed)->toBeLessThan(0.1); + }); +}); + +describe('comparison performance', function () { + test('10,000 comparisons complete in < 50ms', function () { + $positions = []; + for ($i = 0; $i < 100; $i++) { + $positions[] = DecimalPosition::normalize($i * 1000); + } + + $start = microtime(true); + + $count = 0; + for ($i = 0; $i < 10_000; $i++) { + $a = $positions[$i % 100]; + $b = $positions[($i + 1) % 100]; + DecimalPosition::compare($a, $b); + DecimalPosition::lessThan($a, $b); + DecimalPosition::greaterThan($a, $b); + $count++; + } + + $elapsed = microtime(true) - $start; + + expect($count)->toBe(10_000) + ->and($elapsed)->toBeLessThan(0.1); + }); +}); diff --git a/tests/Feature/PositionRebalancingTest.php b/tests/Feature/PositionRebalancingTest.php new file mode 100644 index 0000000..b0b4bb0 --- /dev/null +++ b/tests/Feature/PositionRebalancingTest.php @@ -0,0 +1,235 @@ + 'Task 1', 'status' => 'todo', 'order_position' => '1000.0000000000']); + Task::create(['title' => 'Task 2', 'status' => 'todo', 'order_position' => '2000.0000000000']); + Task::create(['title' => 'Task 3', 'status' => 'todo', 'order_position' => '3000.0000000000']); +}); + +describe('PositionRebalancer::needsRebalancing()', function () { + test('detects gap below MIN_GAP', function () { + // Create tasks with very small gap + Task::create(['title' => 'Close 1', 'status' => 'in_progress', 'order_position' => '1000.0000000000']); + Task::create(['title' => 'Close 2', 'status' => 'in_progress', 'order_position' => '1000.00005']); // Gap < MIN_GAP + + $rebalancer = new PositionRebalancer; + + expect($rebalancer->needsRebalancing( + Task::query(), + 'status', + 'in_progress', + 'order_position' + ))->toBeTrue(); + }); + + test('returns false when gaps are healthy', function () { + $rebalancer = new PositionRebalancer; + + expect($rebalancer->needsRebalancing( + Task::query(), + 'status', + 'todo', + 'order_position' + ))->toBeFalse(); + }); + + test('returns false for empty column', function () { + $rebalancer = new PositionRebalancer; + + expect($rebalancer->needsRebalancing( + Task::query(), + 'status', + 'done', // No tasks in this column + 'order_position' + ))->toBeFalse(); + }); + + test('returns false for single item column', function () { + Task::create(['title' => 'Alone', 'status' => 'review', 'order_position' => '1000.0000000000']); + + $rebalancer = new PositionRebalancer; + + expect($rebalancer->needsRebalancing( + Task::query(), + 'status', + 'review', + 'order_position' + ))->toBeFalse(); + }); +}); + +describe('PositionRebalancer::rebalanceColumn()', function () { + test('redistributes positions evenly', function () { + $rebalancer = new PositionRebalancer; + + $count = $rebalancer->rebalanceColumn( + Task::query(), + 'status', + 'todo', + 'order_position' + ); + + expect($count)->toBe(3); + + // Verify positions are evenly spaced + $tasks = Task::where('status', 'todo')->orderBy('order_position')->get(); + + expect(DecimalPosition::normalize($tasks[0]->order_position))->toBe('65535.0000000000') + ->and(DecimalPosition::normalize($tasks[1]->order_position))->toBe('131070.0000000000') + ->and(DecimalPosition::normalize($tasks[2]->order_position))->toBe('196605.0000000000'); + }); + + test('maintains original order after rebalancing', function () { + // Create tasks with irregular positions + Task::create(['title' => 'A', 'status' => 'testing', 'order_position' => '100.0000000000']); + Task::create(['title' => 'B', 'status' => 'testing', 'order_position' => '100.0001000000']); + Task::create(['title' => 'C', 'status' => 'testing', 'order_position' => '100.0001500000']); + Task::create(['title' => 'D', 'status' => 'testing', 'order_position' => '100.0001600000']); + + // Get original order + $originalOrder = Task::where('status', 'testing') + ->orderBy('order_position') + ->pluck('title') + ->toArray(); + + // Rebalance + $rebalancer = new PositionRebalancer; + $rebalancer->rebalanceColumn(Task::query(), 'status', 'testing', 'order_position'); + + // Get new order + $newOrder = Task::where('status', 'testing') + ->orderBy('order_position') + ->pluck('title') + ->toArray(); + + expect($newOrder)->toBe($originalOrder); + }); + + test('returns zero for empty column', function () { + $rebalancer = new PositionRebalancer; + + $count = $rebalancer->rebalanceColumn( + Task::query(), + 'status', + 'nonexistent', + 'order_position' + ); + + expect($count)->toBe(0); + }); +}); + +describe('PositionRebalancer::findColumnsNeedingRebalancing()', function () { + test('identifies columns with small gaps', function () { + // Create column with healthy gaps + Task::create(['title' => 'Healthy 1', 'status' => 'done', 'order_position' => '1000.0000000000']); + Task::create(['title' => 'Healthy 2', 'status' => 'done', 'order_position' => '2000.0000000000']); + + // Create column with small gaps + Task::create(['title' => 'Cramped 1', 'status' => 'blocked', 'order_position' => '1000.0000000000']); + Task::create(['title' => 'Cramped 2', 'status' => 'blocked', 'order_position' => '1000.00005']); // Gap < MIN_GAP + + $rebalancer = new PositionRebalancer; + + $needsRebalancing = $rebalancer->findColumnsNeedingRebalancing( + Task::query(), + 'status', + 'order_position' + ); + + expect($needsRebalancing)->toContain('blocked') + ->and($needsRebalancing)->not->toContain('done') + ->and($needsRebalancing)->not->toContain('todo'); + }); +}); + +describe('PositionRebalancer::rebalanceAll()', function () { + test('processes all columns needing rebalancing', function () { + // Create multiple columns needing rebalancing + Task::create(['title' => 'Col1 A', 'status' => 'blocked', 'order_position' => '1000.0000000000']); + Task::create(['title' => 'Col1 B', 'status' => 'blocked', 'order_position' => '1000.00005']); + + Task::create(['title' => 'Col2 A', 'status' => 'review', 'order_position' => '2000.0000000000']); + Task::create(['title' => 'Col2 B', 'status' => 'review', 'order_position' => '2000.00003']); + + $rebalancer = new PositionRebalancer; + + $results = $rebalancer->rebalanceAll( + Task::query(), + 'status', + 'order_position' + ); + + expect($results)->toHaveKey('blocked') + ->and($results)->toHaveKey('review') + ->and($results['blocked'])->toBe(2) + ->and($results['review'])->toBe(2); + + // Verify gaps are now healthy + expect($rebalancer->needsRebalancing(Task::query(), 'status', 'blocked', 'order_position'))->toBeFalse() + ->and($rebalancer->needsRebalancing(Task::query(), 'status', 'review', 'order_position'))->toBeFalse(); + }); +}); + +describe('PositionRebalancer::getGapStatistics()', function () { + test('returns correct statistics for column', function () { + $rebalancer = new PositionRebalancer; + + $stats = $rebalancer->getGapStatistics( + Task::query(), + 'status', + 'todo', + 'order_position' + ); + + expect($stats['count'])->toBe(3) + ->and($stats['min_gap'])->toBe('1000.0000000000') + ->and($stats['max_gap'])->toBe('1000.0000000000') + ->and($stats['avg_gap'])->toBe('1000.0000000000') + ->and($stats['small_gaps'])->toBe(0); + }); + + test('returns nulls for single item column', function () { + Task::create(['title' => 'Solo', 'status' => 'solo_column', 'order_position' => '1000.0000000000']); + + $rebalancer = new PositionRebalancer; + + $stats = $rebalancer->getGapStatistics( + Task::query(), + 'status', + 'solo_column', + 'order_position' + ); + + expect($stats['count'])->toBe(1) + ->and($stats['min_gap'])->toBeNull() + ->and($stats['max_gap'])->toBeNull() + ->and($stats['avg_gap'])->toBeNull() + ->and($stats['small_gaps'])->toBe(0); + }); + + test('counts small gaps correctly', function () { + Task::create(['title' => 'A', 'status' => 'cramped', 'order_position' => '1000.0000000000']); + Task::create(['title' => 'B', 'status' => 'cramped', 'order_position' => '1000.00005']); // Small gap + Task::create(['title' => 'C', 'status' => 'cramped', 'order_position' => '2000.0000000000']); // Large gap + + $rebalancer = new PositionRebalancer; + + $stats = $rebalancer->getGapStatistics( + Task::query(), + 'status', + 'cramped', + 'order_position' + ); + + expect($stats['count'])->toBe(3) + ->and($stats['small_gaps'])->toBe(1); + }); +}); diff --git a/tests/Feature/RepairPositionsCommandTest.php b/tests/Feature/RepairPositionsCommandTest.php deleted file mode 100644 index 2d3e4e4..0000000 --- a/tests/Feature/RepairPositionsCommandTest.php +++ /dev/null @@ -1,298 +0,0 @@ -id(); - $table->string('title'); - $table->string('status'); - $table->string('position')->nullable(); - $table->integer('team_id')->nullable(); - $table->timestamps(); - }); -}); - -afterEach(function () { - Schema::dropIfExists('test_tasks'); -}); - -test('repair command shows help and usage', function () { - $result = Artisan::call('flowforge:repair-positions', ['--help' => true]); - - expect($result)->toBe(0); - expect(Artisan::output()) - ->toContain('Interactive command to repair and regenerate position fields') - ->toContain('--dry-run') - ->toContain('--ids') - ->toContain('--where'); -}); - -test('repair command handles non-existent model class', function () { - // We can't easily test interactive prompts, but we can test the validation logic - $command = new \Relaticle\Flowforge\Commands\RepairPositionsCommand; - $reflection = new ReflectionClass($command); - $method = $reflection->getMethod('validateModelClass'); - $method->setAccessible(true); - - $result = $method->invoke($command, 'NonExistentModel'); - expect($result)->toContain('does not exist'); - - $result = $method->invoke($command, 'stdClass'); - expect($result)->toContain('is not an Eloquent model'); - - $result = $method->invoke($command, 'TestTask'); - expect($result)->toBeNull(); -}); - -test('repair command analyzes positions correctly', function () { - // Create test data with various position states - DB::table('test_tasks')->insert([ - ['id' => 1, 'title' => 'Task 1', 'status' => 'todo', 'position' => 'a0', 'team_id' => 1], - ['id' => 2, 'title' => 'Task 2', 'status' => 'todo', 'position' => 'a1', 'team_id' => 1], - ['id' => 3, 'title' => 'Task 3', 'status' => 'todo', 'position' => null, 'team_id' => 1], // Missing position - ['id' => 4, 'title' => 'Task 4', 'status' => 'in_progress', 'position' => 'a0', 'team_id' => 1], // Duplicate position - ['id' => 5, 'title' => 'Task 5', 'status' => 'in_progress', 'position' => 'a0', 'team_id' => 1], // Duplicate position - ['id' => 6, 'title' => 'Task 6', 'status' => 'done', 'position' => 'a2', 'team_id' => 2], - ]); - - $command = new \Relaticle\Flowforge\Commands\RepairPositionsCommand; - $reflection = new ReflectionClass($command); - $method = $reflection->getMethod('analyzePositions'); - $method->setAccessible(true); - - $analysis = $method->invoke($command, 'TestTask', 'status', 'position'); - - expect($analysis['total'])->toBe(6); - expect($analysis['null_positions'])->toBe(1); - expect($analysis['duplicates'])->toBe(1); // One duplicate position ('a0') - expect($analysis['groups'])->toEqual([ - 'todo' => 3, - 'in_progress' => 2, - 'done' => 1, - ]); -}); - -test('repair command analyzes positions with filtering', function () { - // Create test data - DB::table('test_tasks')->insert([ - ['id' => 1, 'title' => 'Task 1', 'status' => 'todo', 'position' => 'a0', 'team_id' => 1], - ['id' => 2, 'title' => 'Task 2', 'status' => 'todo', 'position' => null, 'team_id' => 1], - ['id' => 3, 'title' => 'Task 3', 'status' => 'todo', 'position' => 'a0', 'team_id' => 2], // Different team - ['id' => 4, 'title' => 'Task 4', 'status' => 'done', 'position' => 'a1', 'team_id' => 1], - ]); - - $command = new \Relaticle\Flowforge\Commands\RepairPositionsCommand; - $reflection = new ReflectionClass($command); - - // Test ID filtering - $applyFiltersMethod = $reflection->getMethod('applyFilters'); - $applyFiltersMethod->setAccessible(true); - - $analyzeMethod = $reflection->getMethod('analyzePositions'); - $analyzeMethod->setAccessible(true); - - // Mock command options for ID filtering - $command->expects($this->any()) - ->method('option') - ->willReturnMap([ - ['ids', '1,2'], - ['where', null], - ]); - - $baseQuery = (new TestTask)->newQuery(); - $filteredQuery = $applyFiltersMethod->invoke($command, $baseQuery); - - $analysis = $analyzeMethod->invoke($command, 'TestTask', 'status', 'position', $filteredQuery); - - expect($analysis['total'])->toBe(2); - expect($analysis['null_positions'])->toBe(1); -})->skip('Mocking command options is complex in this context'); - -test('repair command generates positions correctly', function () { - // Create test records using Eloquent Collection - $records = new \Illuminate\Database\Eloquent\Collection([ - (object) ['id' => 1, 'position' => null], - (object) ['id' => 2, 'position' => null], - (object) ['id' => 3, 'position' => null], - ]); - - $command = new \Relaticle\Flowforge\Commands\RepairPositionsCommand; - $reflection = new ReflectionClass($command); - $method = $reflection->getMethod('generatePositions'); - $method->setAccessible(true); - - $positions = $method->invoke($command, $records, 'regenerate'); - - expect($positions)->toHaveCount(3); - expect($positions)->toHaveKeys([1, 2, 3]); - - // Check that positions are valid fractional ranks - $positionValues = array_values($positions); - expect($positionValues[0])->toBeString()->not()->toBeEmpty(); // Valid rank format - expect($positionValues[1] > $positionValues[0])->toBeTrue(); // Ascending order - expect($positionValues[2] > $positionValues[1])->toBeTrue(); // Ascending order - - // Debug: Let's see what the actual format is - // dump($positionValues); // Uncomment to see actual values -}); - -test('repair command finds duplicate positions correctly', function () { - // Create test data with duplicates - DB::table('test_tasks')->insert([ - ['id' => 1, 'title' => 'Task 1', 'status' => 'todo', 'position' => 'a0'], - ['id' => 2, 'title' => 'Task 2', 'status' => 'todo', 'position' => 'a1'], - ['id' => 3, 'title' => 'Task 3', 'status' => 'todo', 'position' => 'a0'], // Duplicate - ['id' => 4, 'title' => 'Task 4', 'status' => 'todo', 'position' => 'a2'], - ['id' => 5, 'title' => 'Task 5', 'status' => 'todo', 'position' => 'a2'], // Duplicate - ]); - - $command = new \Relaticle\Flowforge\Commands\RepairPositionsCommand; - $reflection = new ReflectionClass($command); - $method = $reflection->getMethod('getDuplicatePositions'); - $method->setAccessible(true); - - $query = (new TestTask)->where('status', 'todo'); - $duplicates = $method->invoke($command, $query, 'position'); - - expect($duplicates)->toHaveCount(2); - expect($duplicates)->toContain('a0'); - expect($duplicates)->toContain('a2'); -}); - -test('repair command applies changes correctly', function () { - // Create test data - DB::table('test_tasks')->insert([ - ['id' => 1, 'title' => 'Task 1', 'status' => 'todo', 'position' => null], - ['id' => 2, 'title' => 'Task 2', 'status' => 'todo', 'position' => null], - ]); - - $changes = [ - 'todo' => [ - 1 => 'a0', - 2 => 'a1', - ], - ]; - - $command = new \Relaticle\Flowforge\Commands\RepairPositionsCommand; - $reflection = new ReflectionClass($command); - $method = $reflection->getMethod('applyChanges'); - $method->setAccessible(true); - - $method->invoke($command, 'TestTask', 'position', $changes); - - // Verify changes were applied - $task1 = DB::table('test_tasks')->where('id', 1)->first(); - $task2 = DB::table('test_tasks')->where('id', 2)->first(); - - expect($task1->position)->toBe('a0'); - expect($task2->position)->toBe('a1'); -}); - -test('repair command handles empty results gracefully', function () { - $command = new \Relaticle\Flowforge\Commands\RepairPositionsCommand; - $reflection = new ReflectionClass($command); - - $analyzeMethod = $reflection->getMethod('analyzePositions'); - $analyzeMethod->setAccessible(true); - - $analysis = $analyzeMethod->invoke($command, 'TestTask', 'status', 'position'); - - expect($analysis['total'])->toBe(0); - expect($analysis['null_positions'])->toBe(0); - expect($analysis['duplicates'])->toBe(0); - expect($analysis['groups'])->toBeEmpty(); -}); - -test('repair command validates model fields correctly', function () { - $command = new \Relaticle\Flowforge\Commands\RepairPositionsCommand; - $reflection = new ReflectionClass($command); - $method = $reflection->getMethod('validateFields'); - $method->setAccessible(true); - - $model = new TestTask; - - // This should return true (just shows warning) since our test model has fillable fields - $result = $method->invoke($command, $model, 'status', 'position'); - expect($result)->toBeTrue(); -}); - -test('repair command calculates changes for different strategies', function () { - // Create test data with mixed states - DB::table('test_tasks')->insert([ - ['id' => 1, 'title' => 'Task 1', 'status' => 'todo', 'position' => 'a0'], - ['id' => 2, 'title' => 'Task 2', 'status' => 'todo', 'position' => null], // Missing - ['id' => 3, 'title' => 'Task 3', 'status' => 'todo', 'position' => 'a0'], // Duplicate - ['id' => 4, 'title' => 'Task 4', 'status' => 'done', 'position' => 'a1'], - ]); - - $command = new \Relaticle\Flowforge\Commands\RepairPositionsCommand; - $reflection = new ReflectionClass($command); - $method = $reflection->getMethod('calculateChanges'); - $method->setAccessible(true); - - // Test fix_missing strategy - $changes = $method->invoke($command, 'TestTask', 'status', 'position', 'fix_missing'); - expect($changes['todo'])->toHaveCount(1); // Only the missing position record - expect($changes['todo'])->toHaveKey(2); // ID 2 has missing position - - // Test regenerate strategy - $changes = $method->invoke($command, 'TestTask', 'status', 'position', 'regenerate'); - expect($changes['todo'])->toHaveCount(3); // All todo records - expect($changes['done'])->toHaveCount(1); // All done records -}); - -test('repair command handles enum conversion correctly', function () { - // Create a mock enum-like object - $mockEnum = new class - { - public $value = 'todo'; - - public function __toString(): string - { - return $this->value; - } - }; - - // Test the conversion logic - $stringKey = is_object($mockEnum) && method_exists($mockEnum, 'value') ? $mockEnum->value : (string) $mockEnum; - expect($stringKey)->toBe('todo'); - - // Test with regular string - $regularString = 'in_progress'; - $stringKey = is_object($regularString) && method_exists($regularString, 'value') ? $regularString->value : (string) $regularString; - expect($stringKey)->toBe('in_progress'); -}); - -test('repair command filter parsing works correctly', function () { - $command = new \Relaticle\Flowforge\Commands\RepairPositionsCommand; - $reflection = new ReflectionClass($command); - - // Test WHERE clause parsing - $testCases = [ - 'team_id=5' => ['team_id', '=', '5'], - 'priority>3' => ['priority', '>', '3'], - 'status!=done' => ['status', '!=', 'done'], - 'count<=10' => ['count', '<=', '10'], - ]; - - foreach ($testCases as $where => $expected) { - if (preg_match('/^(\w+)\s*([=<>!]+)\s*(.+)$/', $where, $matches)) { - [, $column, $operator, $value] = $matches; - expect([$column, $operator, $value])->toBe($expected); - } - } -}); diff --git a/tests/Feature/RetryMechanismTest.php b/tests/Feature/RetryMechanismTest.php new file mode 100644 index 0000000..8d39997 --- /dev/null +++ b/tests/Feature/RetryMechanismTest.php @@ -0,0 +1,151 @@ +setAccessible(true); + $reflection->setValue($exception, ['23000', 19, 'UNIQUE constraint failed']); + + expect($helper->isDuplicatePositionError($exception))->toBeTrue(); + }); + + test('detects MySQL ER_DUP_ENTRY error', function () { + $helper = new RetryMechanismTestHelper; + + // MySQL duplicate entry error + $exception = new QueryException( + 'mysql', + 'INSERT INTO tasks ...', + [], + new PDOException("SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '100.5-todo' for key 'unique_position_per_column'") + ); + + $reflection = new ReflectionProperty($exception, 'errorInfo'); + $reflection->setAccessible(true); + $reflection->setValue($exception, ['23000', 1062, 'Duplicate entry']); + + expect($helper->isDuplicatePositionError($exception))->toBeTrue(); + }); + + test('detects PostgreSQL unique_violation error', function () { + $helper = new RetryMechanismTestHelper; + + // PostgreSQL unique violation error + $exception = new QueryException( + 'pgsql', + 'INSERT INTO tasks ...', + [], + new PDOException('SQLSTATE[23505]: Unique violation: duplicate key value violates unique constraint "unique_position_per_column"') + ); + + $reflection = new ReflectionProperty($exception, 'errorInfo'); + $reflection->setAccessible(true); + $reflection->setValue($exception, ['23505', 23505, 'duplicate key value']); + + expect($helper->isDuplicatePositionError($exception))->toBeTrue(); + }); + + test('detects error by message containing unique_position_per_column', function () { + $helper = new RetryMechanismTestHelper; + + // Generic error with constraint name in message + $exception = new QueryException( + 'sqlite', + 'INSERT INTO tasks ...', + [], + new PDOException('UNIQUE constraint failed: unique_position_per_column') + ); + + $reflection = new ReflectionProperty($exception, 'errorInfo'); + $reflection->setAccessible(true); + $reflection->setValue($exception, ['23000', 999, 'unknown error']); // Unknown error code + + expect($helper->isDuplicatePositionError($exception))->toBeTrue(); + }); + + test('returns false for non-duplicate errors', function () { + $helper = new RetryMechanismTestHelper; + + // Foreign key constraint error (not duplicate) + $exception = new QueryException( + 'mysql', + 'INSERT INTO tasks ...', + [], + new PDOException('SQLSTATE[23000]: Integrity constraint violation: 1452 Cannot add or update a child row: a foreign key constraint fails') + ); + + $reflection = new ReflectionProperty($exception, 'errorInfo'); + $reflection->setAccessible(true); + $reflection->setValue($exception, ['23000', 1452, 'foreign key constraint fails']); + + expect($helper->isDuplicatePositionError($exception))->toBeFalse(); + }); +}); + +describe('database unique constraint behavior', function () { + beforeEach(function () { + Task::create(['title' => 'Existing', 'status' => 'todo', 'order_position' => '1500.0000000000']); + }); + + test('unique constraint throws QueryException on duplicate', function () { + expect(fn () => Task::create([ + 'title' => 'Duplicate', + 'status' => 'todo', + 'order_position' => '1500.0000000000', + ]))->toThrow(QueryException::class); + }); + + test('same position in different columns is allowed', function () { + // Same position, different status (column) + $task = Task::create([ + 'title' => 'Different Column', + 'status' => 'in_progress', // Different column + 'order_position' => '1500.0000000000', // Same position + ]); + + expect($task->exists)->toBeTrue(); + }); + + test('null positions are allowed', function () { + // NULL is special in unique constraints - multiple NULLs are allowed + Task::create(['title' => 'Null 1', 'status' => 'todo', 'order_position' => null]); + $task2 = Task::create(['title' => 'Null 2', 'status' => 'todo', 'order_position' => null]); + + expect($task2->exists)->toBeTrue(); + }); +}); diff --git a/tests/Unit/DecimalPositionTest.php b/tests/Unit/DecimalPositionTest.php new file mode 100644 index 0000000..f14c277 --- /dev/null +++ b/tests/Unit/DecimalPositionTest.php @@ -0,0 +1,237 @@ +toBeGreaterThan(0) + ->and(bccomp($result, $before, 10))->toBeLessThan(0); + } + }); + + test('produces different results on successive calls (jitter verification)', function () { + $after = '1000.0000000000'; + $before = '2000.0000000000'; + + $results = []; + for ($i = 0; $i < 100; $i++) { + $results[] = DecimalPosition::between($after, $before); + } + + // All 100 results should be unique (cryptographic jitter) + $unique = array_unique($results); + expect(count($unique))->toBe(100); + }); + + test('throws InvalidArgumentException when after >= before', function () { + $after = '2000.0000000000'; + $before = '1000.0000000000'; + + DecimalPosition::between($after, $before); + })->throws(InvalidArgumentException::class, 'Invalid bounds: after (2000.0000000000) must be less than before (1000.0000000000)'); + + test('throws InvalidArgumentException when after equals before', function () { + $position = '1500.0000000000'; + + DecimalPosition::between($position, $position); + })->throws(InvalidArgumentException::class); +}); + +describe('betweenExact() deterministic', function () { + test('returns exact midpoint', function () { + expect(DecimalPosition::betweenExact('1000', '2000'))->toBe('1500.0000000000') + ->and(DecimalPosition::betweenExact('0', '100'))->toBe('50.0000000000') + ->and(DecimalPosition::betweenExact('100', '101'))->toBe('100.5000000000'); + }); + + test('returns consistent results (deterministic)', function () { + $results = []; + for ($i = 0; $i < 100; $i++) { + $results[] = DecimalPosition::betweenExact('1000', '2000'); + } + + // All 100 results should be identical + expect(array_unique($results))->toHaveCount(1) + ->and($results[0])->toBe('1500.0000000000'); + }); + + test('throws InvalidArgumentException when after >= before', function () { + DecimalPosition::betweenExact('2000', '1000'); + })->throws(InvalidArgumentException::class); +}); + +describe('needsRebalancing()', function () { + test('returns false for large gaps', function () { + expect(DecimalPosition::needsRebalancing('1000', '2000'))->toBeFalse() + ->and(DecimalPosition::needsRebalancing('0', '65535'))->toBeFalse(); + }); + + test('returns true when gap equals MIN_GAP', function () { + $after = '1000.0000000000'; + $before = bcadd($after, DecimalPosition::MIN_GAP, 10); + + expect(DecimalPosition::needsRebalancing($after, $before))->toBeFalse(); + }); + + test('returns true when gap is below MIN_GAP', function () { + $after = '1000.0000000000'; + $before = '1000.00009'; // Gap of 0.00009 < 0.0001 + + expect(DecimalPosition::needsRebalancing($after, $before))->toBeTrue(); + }); + + test('returns true for extremely small gaps', function () { + $after = '1000.0000000000'; + $before = '1000.0000000001'; + + expect(DecimalPosition::needsRebalancing($after, $before))->toBeTrue(); + }); +}); + +describe('generateSequence()', function () { + test('produces evenly spaced positions', function () { + $positions = DecimalPosition::generateSequence(5); + + expect($positions)->toHaveCount(5) + ->and($positions[0])->toBe('65535.0000000000') + ->and($positions[1])->toBe('131070.0000000000') + ->and($positions[2])->toBe('196605.0000000000') + ->and($positions[3])->toBe('262140.0000000000') + ->and($positions[4])->toBe('327675.0000000000'); + }); + + test('returns empty array for zero count', function () { + expect(DecimalPosition::generateSequence(0))->toBe([]); + }); + + test('returns single position for count of 1', function () { + $positions = DecimalPosition::generateSequence(1); + + expect($positions)->toHaveCount(1) + ->and($positions[0])->toBe('65535.0000000000'); + }); +}); + +describe('generateBetween()', function () { + test('produces N unique positions within bounds', function () { + $after = '1000.0000000000'; + $before = '2000.0000000000'; + $count = 10; + + $positions = DecimalPosition::generateBetween($after, $before, $count); + + expect($positions)->toHaveCount($count); + + // All positions should be unique + expect(array_unique($positions))->toHaveCount($count); + + // All positions should be strictly between bounds + foreach ($positions as $position) { + expect(bccomp($position, $after, 10))->toBeGreaterThan(0) + ->and(bccomp($position, $before, 10))->toBeLessThan(0); + } + }); + + test('returns empty array for zero count', function () { + expect(DecimalPosition::generateBetween('1000', '2000', 0))->toBe([]); + }); + + test('returns single position for count of 1', function () { + $positions = DecimalPosition::generateBetween('1000', '2000', 1); + + expect($positions)->toHaveCount(1); + }); +}); + +describe('normalize()', function () { + test('handles various input formats', function () { + expect(DecimalPosition::normalize('1000'))->toBe('1000.0000000000') + ->and(DecimalPosition::normalize('1000.5'))->toBe('1000.5000000000') + ->and(DecimalPosition::normalize(1000))->toBe('1000.0000000000') + ->and(DecimalPosition::normalize(1000.5))->toBe('1000.5000000000') + ->and(DecimalPosition::normalize('0'))->toBe('0.0000000000') + ->and(DecimalPosition::normalize('-1000'))->toBe('-1000.0000000000'); + }); +}); + +describe('comparison methods', function () { + test('compare() returns correct values', function () { + expect(DecimalPosition::compare('1000', '2000'))->toBe(-1) + ->and(DecimalPosition::compare('2000', '1000'))->toBe(1) + ->and(DecimalPosition::compare('1000', '1000'))->toBe(0); + }); + + test('lessThan() works correctly', function () { + expect(DecimalPosition::lessThan('1000', '2000'))->toBeTrue() + ->and(DecimalPosition::lessThan('2000', '1000'))->toBeFalse() + ->and(DecimalPosition::lessThan('1000', '1000'))->toBeFalse(); + }); + + test('greaterThan() works correctly', function () { + expect(DecimalPosition::greaterThan('2000', '1000'))->toBeTrue() + ->and(DecimalPosition::greaterThan('1000', '2000'))->toBeFalse() + ->and(DecimalPosition::greaterThan('1000', '1000'))->toBeFalse(); + }); +}); + +describe('gap()', function () { + test('calculates gap between positions', function () { + expect(DecimalPosition::gap('1000', '2000'))->toBe('1000.0000000000') + ->and(DecimalPosition::gap('0', '65535'))->toBe('65535.0000000000') + ->and(DecimalPosition::gap('100', '100.5'))->toBe('0.5000000000'); + }); +}); + +describe('forEmptyColumn()', function () { + test('returns DEFAULT_GAP', function () { + expect(DecimalPosition::forEmptyColumn())->toBe(DecimalPosition::DEFAULT_GAP); + }); +}); + +describe('after() and before()', function () { + test('after() adds DEFAULT_GAP', function () { + expect(DecimalPosition::after('1000'))->toBe('66535.0000000000') + ->and(DecimalPosition::after('0'))->toBe('65535.0000000000'); + }); + + test('before() subtracts DEFAULT_GAP', function () { + expect(DecimalPosition::before('100000'))->toBe('34465.0000000000') + ->and(DecimalPosition::before('65535'))->toBe('0.0000000000'); + }); + + test('before() can produce negative positions', function () { + expect(DecimalPosition::before('0'))->toBe('-65535.0000000000'); + }); +}); + +describe('calculate()', function () { + test('returns forEmptyColumn when both positions are null', function () { + expect(DecimalPosition::calculate(null, null))->toBe(DecimalPosition::DEFAULT_GAP); + }); + + test('returns after() when only afterPos is provided', function () { + $result = DecimalPosition::calculate('1000', null); + expect($result)->toBe('66535.0000000000'); + }); + + test('returns before() when only beforePos is provided', function () { + $result = DecimalPosition::calculate(null, '100000'); + expect($result)->toBe('34465.0000000000'); + }); + + test('returns between() when both positions are provided', function () { + $result = DecimalPosition::calculate('1000', '2000'); + + // Should be between bounds (with jitter) + expect(bccomp($result, '1000', 10))->toBeGreaterThan(0) + ->and(bccomp($result, '2000', 10))->toBeLessThan(0); + }); +}); diff --git a/tests/Unit/RankServiceTest.php b/tests/Unit/RankServiceTest.php deleted file mode 100644 index 858367e..0000000 --- a/tests/Unit/RankServiceTest.php +++ /dev/null @@ -1,327 +0,0 @@ -get())->toBeString() - ->and(strlen($rank->get()))->toBeGreaterThan(0); - }); - - it('handles single card scenario', function () { - $cardA = Rank::forEmptySequence(); - - expect($cardA->get())->toBeString(); - }); - - describe('Two Cards Scenario (A, B)', function () { - beforeEach(function () { - $this->cardA = Rank::forEmptySequence(); - $this->cardB = Rank::after($this->cardA); - }); - - it('creates two cards in sequence', function () { - expect($this->cardA->get())->toBeLessThan($this->cardB->get()); - }); - - it('moves A after B (A->B)', function () { - $newA = Rank::after($this->cardB); - - expect($this->cardB->get())->toBeLessThan($newA->get()); - }); - - it('moves B before A (B->A)', function () { - $newB = Rank::before($this->cardA); - - expect($newB->get())->toBeLessThan($this->cardA->get()); - }); - }); - - describe('Three Cards Scenario (A, B, C)', function () { - beforeEach(function () { - $this->cardA = Rank::forEmptySequence(); - $this->cardB = Rank::after($this->cardA); - $this->cardC = Rank::after($this->cardB); - }); - - it('creates three cards in sequence', function () { - expect($this->cardA->get())->toBeLessThan($this->cardB->get()) - ->and($this->cardB->get())->toBeLessThan($this->cardC->get()); - }); - - it('moves A between B and C', function () { - $newA = Rank::betweenRanks($this->cardB, $this->cardC); - - expect($this->cardB->get())->toBeLessThan($newA->get()) - ->and($newA->get())->toBeLessThan($this->cardC->get()); - }); - - it('moves C between A and B', function () { - $newC = Rank::betweenRanks($this->cardA, $this->cardB); - - expect($this->cardA->get())->toBeLessThan($newC->get()) - ->and($newC->get())->toBeLessThan($this->cardB->get()); - }); - - it('moves B to the end', function () { - $newB = Rank::after($this->cardC); - - expect($this->cardC->get())->toBeLessThan($newB->get()); - }); - - it('moves B to the beginning', function () { - $newB = Rank::before($this->cardA); - - expect($newB->get())->toBeLessThan($this->cardA->get()); - }); - }); - - describe('Four Cards Scenario (A, B, C, D)', function () { - beforeEach(function () { - $this->cardA = Rank::forEmptySequence(); - $this->cardB = Rank::after($this->cardA); - $this->cardC = Rank::after($this->cardB); - $this->cardD = Rank::after($this->cardC); - - $this->originalOrder = [ - 'A' => $this->cardA->get(), - 'B' => $this->cardB->get(), - 'C' => $this->cardC->get(), - 'D' => $this->cardD->get(), - ]; - }); - - it('creates four cards in sequence', function () { - expect($this->cardA->get())->toBeLessThan($this->cardB->get()) - ->and($this->cardB->get())->toBeLessThan($this->cardC->get()) - ->and($this->cardC->get())->toBeLessThan($this->cardD->get()); - }); - - it('moves A after B (A->B)', function () { - $newA = Rank::betweenRanks($this->cardB, $this->cardC); - - expect($this->cardB->get())->toBeLessThan($newA->get()) - ->and($newA->get())->toBeLessThan($this->cardC->get()); - }); - - it('moves B after A (B->A)', function () { - $newB = Rank::before($this->cardA); - - expect($newB->get())->toBeLessThan($this->cardA->get()); - }); - - it('moves A after B then B after A (multiple swaps)', function () { - // A->B - $newA = Rank::betweenRanks($this->cardB, $this->cardC); - expect($this->cardB->get())->toBeLessThan($newA->get()) - ->and($newA->get())->toBeLessThan($this->cardC->get()); - - // B->A (using original A position) - $newB = Rank::before($this->cardA); - expect($newB->get())->toBeLessThan($this->cardA->get()); - }); - - it('moves A to end', function () { - $newA = Rank::after($this->cardD); - - expect($this->cardD->get())->toBeLessThan($newA->get()); - }); - - it('moves D to beginning', function () { - $newD = Rank::before($this->cardA); - - expect($newD->get())->toBeLessThan($this->cardA->get()); - }); - - it('moves B between C and D', function () { - $newB = Rank::betweenRanks($this->cardC, $this->cardD); - - expect($this->cardC->get())->toBeLessThan($newB->get()) - ->and($newB->get())->toBeLessThan($this->cardD->get()); - }); - - it('moves C between A and B', function () { - $newC = Rank::betweenRanks($this->cardA, $this->cardB); - - expect($this->cardA->get())->toBeLessThan($newC->get()) - ->and($newC->get())->toBeLessThan($this->cardB->get()); - }); - - describe('Complex Movement Sequences', function () { - it('performs multiple random movements', function () { - $cards = [ - 'A' => $this->cardA, - 'B' => $this->cardB, - 'C' => $this->cardC, - 'D' => $this->cardD, - ]; - - // Move A after C - $cards['A'] = Rank::betweenRanks($this->cardC, $this->cardD); - - // Move B to beginning - $cards['B'] = Rank::before($this->cardA); - - // Move D between original A and new A positions - $cards['D'] = Rank::betweenRanks($this->cardA, $cards['A']); - - // All cards should be unique - $ranks = array_map(fn ($card) => $card->get(), $cards); - expect(array_unique($ranks))->toHaveCount(4); - - // Check that ordering is valid for each moved card - expect($cards['B']->get())->toBeLessThan($this->cardA->get()); - expect($this->cardC->get())->toBeLessThan($cards['A']->get()) - ->and($cards['A']->get())->toBeLessThan($this->cardD->get()); - expect($this->cardA->get())->toBeLessThan($cards['D']->get()) - ->and($cards['D']->get())->toBeLessThan($cards['A']->get()); - }); - }); - }); - - describe('Stress Testing', function () { - it('handles 100 sequential insertions', function () { - $cards = [Rank::forEmptySequence()]; - - for ($i = 1; $i < 100; $i++) { - $cards[] = Rank::after(end($cards)); - } - - // Verify all are unique and in order - $ranks = array_map(fn ($card) => $card->get(), $cards); - expect(array_unique($ranks))->toHaveCount(100); - - $sortedRanks = $ranks; - sort($sortedRanks); - expect($ranks)->toBe($sortedRanks); - }); - - it('handles many insertions with cascading positions', function () { - $first = Rank::forEmptySequence(); - $last = Rank::after($first); - $insertions = [$first]; - - // Create a cascading series of insertions - for ($i = 0; $i < 10; $i++) { - $newRank = Rank::betweenRanks($insertions[count($insertions) - 1], $last); - $insertions[] = $newRank; - } - - // Verify all are unique and properly ordered - $ranks = array_map(fn ($card) => $card->get(), $insertions); - expect(array_unique($ranks))->toHaveCount(11); - - // Check ordering - for ($i = 0; $i < count($ranks) - 1; $i++) { - expect($ranks[$i])->toBeLessThan($ranks[$i + 1]); - } - }); - }); - - describe('Kanban Board Simulation', function () { - beforeEach(function () { - // Simulate 3 columns with tasks - $this->todoColumn = [ - Rank::forEmptySequence(), - Rank::after(Rank::forEmptySequence()), - ]; - - $this->inProgressColumn = [ - Rank::fromString('m1'), - Rank::fromString('m2'), - ]; - - $this->doneColumn = [ - Rank::fromString('x1'), - Rank::fromString('x2'), - ]; - }); - - it('moves task from todo to in-progress', function () { - $taskToMove = $this->todoColumn[0]; - - // Move to end of in-progress column - $newRank = Rank::after(end($this->inProgressColumn)); - - expect(end($this->inProgressColumn)->get())->toBeLessThan($newRank->get()); - }); - - it('moves task to beginning of column', function () { - $taskToMove = $this->doneColumn[1]; - - // Move to beginning of todo column - $newRank = Rank::before($this->todoColumn[0]); - - expect($newRank->get())->toBeLessThan($this->todoColumn[0]->get()); - }); - - it('reorders within same column', function () { - // Move second task in todo to first position - $newRank = Rank::before($this->todoColumn[0]); - - expect($newRank->get())->toBeLessThan($this->todoColumn[0]->get()); - }); - }); - - describe('Edge Cases and Error Handling', function () { - it('throws exception when prev >= next in betweenRanks', function () { - $first = Rank::forEmptySequence(); - $second = Rank::after($first); - - expect(fn () => Rank::betweenRanks($second, $first)) - ->toThrow(Relaticle\Flowforge\Exceptions\PrevGreaterThanOrEquals::class); - }); - - it('throws exception for invalid characters', function () { - expect(fn () => Rank::fromString('invalid!')) - ->toThrow(Relaticle\Flowforge\Exceptions\InvalidChars::class); - }); - - it('throws exception for rank ending with MIN_CHAR', function () { - expect(fn () => Rank::fromString('a0')) - ->toThrow(Relaticle\Flowforge\Exceptions\LastCharCantBeEqualToMinChar::class); - }); - }); - - describe('Real-world Performance Characteristics', function () { - it('maintains reasonable rank lengths under normal usage', function () { - $cards = [Rank::forEmptySequence()]; - - // Simulate typical usage - adding cards and occasional reordering - for ($i = 0; $i < 20; $i++) { - $cards[] = Rank::after(end($cards)); - - // Occasionally insert between existing cards (keep array sorted) - if ($i % 5 === 0 && count($cards) > 2) { - // Sort cards to ensure proper ordering before insertion - usort($cards, fn ($a, $b) => strcmp($a->get(), $b->get())); - $randomIndex = random_int(0, count($cards) - 2); - $newCard = Rank::betweenRanks($cards[$randomIndex], $cards[$randomIndex + 1]); - $cards[] = $newCard; - } - } - - // Most ranks should be reasonable length - $longRanks = array_filter($cards, fn ($card) => strlen($card->get()) > 10); - expect($longRanks)->toHaveCount(0, 'Ranks should stay reasonable length under normal usage'); - }); - - it('generates lexicographically ordered ranks', function () { - $first = Rank::forEmptySequence(); - $second = Rank::after($first); - $between = Rank::betweenRanks($first, $second); - - // Test PHP string comparison matches our ordering - expect(strcmp($first->get(), $between->get()))->toBeLessThan(0) - ->and(strcmp($between->get(), $second->get()))->toBeLessThan(0); - }); - }); -}); diff --git a/tests/database/migrations/2024_01_01_000000_create_tasks_table.php b/tests/database/migrations/2024_01_01_000000_create_tasks_table.php index 675d91a..9a8aa62 100644 --- a/tests/database/migrations/2024_01_01_000000_create_tasks_table.php +++ b/tests/database/migrations/2024_01_01_000000_create_tasks_table.php @@ -2,7 +2,6 @@ use Illuminate\Database\Migrations\Migration; use Illuminate\Database\Schema\Blueprint; -use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Schema; return new class extends Migration @@ -14,23 +13,16 @@ public function up(): void $table->string('title'); $table->string('status')->default('todo'); - $this->definePositionColumn($table); + // DECIMAL(20,10) for position - 10 integer digits + 10 decimal places + // Supports ~33 bisections before precision loss, with 65535 gap + $table->decimal('order_position', 20, 10)->nullable(); + + // Unique constraint per column to detect position collisions + // Combined with jitter, this enables retry logic for concurrent safety + $table->unique(['status', 'order_position'], 'unique_position_per_column'); $table->string('priority')->default('medium'); $table->timestamps(); }); } - - private function definePositionColumn(Blueprint $table): void - { - $driver = DB::connection()->getDriverName(); - - $positionColumn = $table->string('order_position', 64)->nullable(); - - match ($driver) { - 'pgsql' => $positionColumn->collation('C'), - 'mysql' => $positionColumn->collation('utf8mb4_bin'), - default => null, - }; - } };