Refactor additional postprocessing

2026-08-28 17:01:16 +00:00 · 2026-08-11 10:43:22 +02:00
parent 140b7c8c04
commit 014077b514
45 changed files with 3875 additions and 338 deletions
@@ -154,6 +154,25 @@ class AdditionalProcessingNzbSplitRenameTest extends TestCase
        $this->assertSame('MLB.2026.Cincinnati.Reds.vs.Tampa.Bay.Rays.20.04.720pEN60fps', $result);
    }

+    public function test_additional_nfo_detection_accepts_alternative_and_scene_names(): void
+    {
+        $manager = $this->makeReleaseFileManager();
+
+        foreach (['file_id.diz', 'readme.txt', 'info.txt', '00-scene.nfo', 'group-release.nfo'] as $filename) {
+            $this->assertTrue($manager->isNfoFilename($filename), $filename.' should be treated as NFO data');
+        }
+    }
+
+    public function test_additional_nfo_normalization_converts_utf16_and_removes_utf8_bom(): void
+    {
+        $manager = $this->makeReleaseFileManager();
+        $normalize = new \ReflectionMethod($manager, 'normalizeNfoEncoding');
+        $utf16 = "\xFF\xFE".mb_convert_encoding('Scene NFO', 'UTF-16LE', 'UTF-8');
+
+        $this->assertSame('Scene NFO', $normalize->invoke($manager, $utf16));
+        $this->assertSame('Scene NFO', $normalize->invoke($manager, "\xEF\xBB\xBFScene NFO"));
+    }
+
    private function makeReleaseFileManager(?ReleaseUpdateService $updateService = null): ReleaseFileManager
    {
        $updateService ??= $this->createMock(ReleaseUpdateService::class);
@@ -7,13 +7,17 @@ namespace Tests\Feature;
 use App\Models\Release;
 use App\Services\AdditionalProcessing\AdditionalProcessingOrchestrator;
 use App\Services\AdditionalProcessing\ConsoleOutputService;
+use App\Services\AdditionalProcessing\DTO\ReleaseProcessingResult;
+use App\Services\AdditionalProcessing\Enums\ProcessingOutcome;
 use App\Services\AdditionalProcessing\ReleaseProcessor;
 use App\Services\AdditionalProcessing\State\ReleaseProcessingContext;
 use App\Services\TempWorkspaceService;
 use Illuminate\Contracts\Console\Kernel;
 use Illuminate\Database\Schema\Blueprint;
 use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Schema;
+use Mockery;
 use PDO;
 use Tests\TestCase;
 use Tests\Unit\AdditionalProcessing\CreatesProcessingConfiguration;
@@ -87,6 +91,8 @@ class AdditionalProcessingOrchestratorClaimTest extends TestCase

    public function test_handled_release_exception_clears_claim(): void
    {
+        Log::spy();
+
        DB::table('categories')->insert(['id' => 1, 'disablepreview' => 0]);
        DB::table('releases')->insert([
            'id' => 1,
@@ -121,15 +127,49 @@ class AdditionalProcessingOrchestratorClaimTest extends TestCase
            $output
        );

-        $orchestrator->start('', 'a');
+        $result = $orchestrator->start('', 'a');

        $this->assertSame(1, $processor->processCalls);
+        $this->assertSame([1], $result->claimedIds);
+        $this->assertSame(0, $result->successfulCount());
+        $this->assertSame(1, $result->unsuccessfulCount());
+        $this->assertTrue($result->hasOutcome(ProcessingOutcome::Failed));
+        $this->assertGreaterThan(0.0, $result->elapsedSeconds);
+        $this->assertGreaterThan(0.0, $result->averageReleaseSeconds());
+        $this->assertGreaterThan(0.0, $result->releasesPerSecond());
+        $this->assertGreaterThan(0, $result->peakMemoryBytes);
        $this->assertSame(1, $tempWorkspace->ensureMainTempPathCalls);
        $this->assertSame(1, $tempWorkspace->clearDirectoryCalls);
        $this->assertSame(1, $output->echoDescriptionCalls);
        $this->assertSame(1, $output->endOutputCalls);
        $this->assertNull(Release::query()->where('id', 1)->value('additional_pp_claimed_at'));
        $this->assertNull(Release::query()->where('id', 1)->value('additional_pp_claim_token'));
+
+        Log::shouldHaveReceived('info')
+            ->once()
+            ->with(
+                'Additional postprocessing run finished',
+                Mockery::on(static fn (array $context): bool => $context['picked'] === 1
+                    && $context['processed'] === 0
+                    && $context['failed'] === 1
+                    && $context['outcomes'] === ['failed' => 1]
+                    && array_key_exists('artifacts_created', $context)
+                    && array_key_exists('artifact_yield_percent', $context)
+                    && array_key_exists('release_files_added', $context)
+                    && array_key_exists('download_requests', $context)
+                    && array_key_exists('nntp_requests', $context)
+                    && array_key_exists('download_cache_hits', $context)
+                    && array_key_exists('database_statements', $context)
+                    && array_key_exists('database_milliseconds', $context)
+                    && array_key_exists('search_sync_requests', $context)
+                    && array_key_exists('search_sync_executions', $context)
+                    && array_key_exists('duplicate_message_ids', $context)
+                    && array_key_exists('unsupported_reasons', $context)
+                    && array_key_exists('releases_per_second', $context)
+                    && array_key_exists('average_release_seconds', $context)
+                    && array_key_exists('stage_seconds', $context)
+                    && array_key_exists('peak_memory_bytes', $context)),
+            );
    }

    public function test_temp_setup_failure_does_not_claim_releases(): void
@@ -168,9 +208,11 @@ class AdditionalProcessingOrchestratorClaimTest extends TestCase
            $output
        );

-        $orchestrator->start('', 'a');
+        $result = $orchestrator->start('', 'a');

        $this->assertSame(0, $processor->processCalls);
+        $this->assertSame(0, $result->claimedCount());
+        $this->assertStringContainsString('not writable', $result->setupFailure);
        $this->assertSame(1, $tempWorkspace->ensureMainTempPathCalls);
        $this->assertSame(0, $output->echoDescriptionCalls);
        $this->assertSame(0, $output->endOutputCalls);
@@ -246,7 +288,7 @@ class FailingAdditionalReleaseProcessor extends ReleaseProcessor

    public function __construct() {}

-    public function process(ReleaseProcessingContext $context, string $mainTmpPath): void
+    public function process(ReleaseProcessingContext $context, string $mainTmpPath): ReleaseProcessingResult
    {
        $this->processCalls++;

@@ -7,6 +7,7 @@ namespace Tests\Feature;
 use App\Facades\Search;
 use App\Models\Release;
 use App\Services\AdditionalProcessing\ReleaseFileManager;
+use App\Services\AdditionalProcessing\State\PersistenceMetricsCollector;
 use App\Services\AdditionalProcessing\State\ReleaseProcessingContext;
 use App\Services\CollectionCleanupService;
 use App\Services\NameFixing\NameFixingService;
@@ -14,6 +15,7 @@ use App\Services\NfoService;
 use App\Services\Nzb\NzbService;
 use App\Services\ReleaseImageService;
 use Illuminate\Contracts\Console\Kernel;
+use Illuminate\Database\QueryException;
 use Illuminate\Database\Schema\Blueprint;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\File;
@@ -129,6 +131,40 @@ class AdditionalProcessingReleaseFileManagerTest extends TestCase
        $this->assertNull(DB::table('releases')->where('id', 1)->value('additional_pp_claim_token'));
    }

+    public function test_finalization_flushes_rows_and_synchronizes_search_immediately(): void
+    {
+        DB::table('releases')->insert($this->releaseRow());
+
+        Search::shouldReceive('updateRelease')->once()->with(1);
+
+        $manager = $this->makeManagerWithImageService(new ReleaseImageService);
+        $context = new ReleaseProcessingContext(Release::query()->findOrFail(1));
+        $this->assertTrue($manager->addFileInfo([
+            'name' => 'Example.Movie.2026.mkv',
+            'size' => 2048,
+            'date' => 1_788_600_000,
+        ], $context, '\\.(?:par2|sfv|nzb)'));
+
+        $manager->finalizeRelease($context, false);
+
+        $this->assertSame(1, DB::table('release_files')->count());
+        $this->assertSame(1, DB::table('releases')->where('id', 1)->value('rarinnerfilecount'));
+        $this->assertSame([], $context->pendingReleaseFiles);
+    }
+
+    public function test_database_statements_are_measured_inside_an_active_release_scope(): void
+    {
+        DB::table('releases')->insert($this->releaseRow());
+        $collector = app(PersistenceMetricsCollector::class);
+
+        $collector->beginReleaseScope(1);
+        DB::table('releases')->where('id', 1)->value('guid');
+        $metrics = $collector->finishReleaseScope();
+
+        $this->assertSame(1, $metrics->databaseStatements);
+        $this->assertGreaterThanOrEqual(0.0, $metrics->databaseMilliseconds);
+    }
+
    public function test_queued_par_hashes_flush_with_release_files(): void
    {
        DB::table('releases')->insert($this->releaseRow());
@@ -156,6 +192,35 @@ class AdditionalProcessingReleaseFileManagerTest extends TestCase
        $this->assertSame(1, DB::table('par_hashes')->count());
    }

+    public function test_failed_finalization_rolls_back_buffered_rows_and_keeps_them_for_retry(): void
+    {
+        DB::table('releases')->insert($this->releaseRow());
+
+        Search::shouldReceive('updateRelease')->never();
+
+        $manager = $this->makeManager();
+        $context = new ReleaseProcessingContext(Release::query()->findOrFail(1));
+        $this->assertTrue($manager->addFileInfo([
+            'name' => 'Example.Movie.2026.mkv',
+            'size' => 1024,
+            'date' => 1_788_600_000,
+        ], $context, '\\.(?:par2|sfv|nzb)'));
+
+        DB::unprepared("CREATE TRIGGER fail_release_finalize BEFORE UPDATE ON releases BEGIN SELECT RAISE(ABORT, 'forced finalization failure'); END");
+
+        try {
+            $manager->finalizeRelease($context, false);
+            $this->fail('Finalization should have failed.');
+        } catch (QueryException $exception) {
+            $this->assertStringContainsString('forced finalization failure', $exception->getMessage());
+        } finally {
+            DB::unprepared('DROP TRIGGER IF EXISTS fail_release_finalize');
+        }
+
+        $this->assertSame(0, DB::table('release_files')->count());
+        $this->assertArrayHasKey('Example.Movie.2026.mkv', $context->pendingReleaseFiles);
+    }
+
    public function test_float_release_file_size_is_normalized_before_queueing(): void
    {
        DB::table('releases')->insert($this->releaseRow());
@@ -230,12 +295,16 @@ class AdditionalProcessingReleaseFileManagerTest extends TestCase
        return $this->makeManagerWithImageService(new ReleaseImageService, $nameFixing);
    }

+    /**
+     * @param  array<string, mixed>  $configOverrides
+     */
    private function makeManagerWithImageService(
        ReleaseImageService $imageService,
        ?NameFixingService $nameFixing = null,
+        array $configOverrides = [],
    ): ReleaseFileManager {
        return new ReleaseFileManager(
-            $this->makeConfig(),
+            $this->makeConfig($configOverrides),
            $imageService,
            new NfoService,
            new TestNzbService,
@@ -5,6 +5,11 @@ declare(strict_types=1);
 namespace Tests\Feature;

 use App\Services\AdditionalProcessing\AdditionalProcessingOrchestrator;
+use App\Services\AdditionalProcessing\DTO\AdditionalBatchResult;
+use App\Services\AdditionalProcessing\DTO\DownloadMetrics;
+use App\Services\AdditionalProcessing\DTO\PersistenceMetrics;
+use App\Services\AdditionalProcessing\DTO\ReleaseProcessingResult;
+use App\Services\AdditionalProcessing\Enums\ProcessingOutcome;
 use App\Services\Runners\PostProcessRunner;
 use Illuminate\Contracts\Console\Kernel;
 use Illuminate\Database\Schema\Blueprint;
@@ -186,7 +191,7 @@ class PostProcessRunnerAdditionalThreadsTest extends TestCase
            $mock->shouldReceive('start')
                ->once()
                ->with('', 'a', Mockery::type('string'), [])
-                ->andReturn(['claimed' => 1, 'processed' => 1, 'failed' => 0, 'claimed_ids' => [1]]);
+                ->andReturn($this->successfulBatch(1));
            $mock->shouldReceive('finish')->once();
        });

@@ -205,17 +210,17 @@ class PostProcessRunnerAdditionalThreadsTest extends TestCase
                ->once()
                ->ordered()
                ->with('', 'a', Mockery::type('string'), [])
-                ->andReturn(['claimed' => 1, 'processed' => 1, 'failed' => 0, 'claimed_ids' => [1]]);
+                ->andReturn($this->successfulBatch(1));
            $mock->shouldReceive('start')
                ->once()
                ->ordered()
                ->with('', 'a', Mockery::type('string'), [1])
-                ->andReturn(['claimed' => 1, 'processed' => 1, 'failed' => 0, 'claimed_ids' => [2]]);
+                ->andReturn($this->successfulBatch(2));
            $mock->shouldReceive('start')
                ->once()
                ->ordered()
                ->with('', 'a', Mockery::type('string'), [1, 2])
-                ->andReturn(['claimed' => 0, 'processed' => 0, 'failed' => 0, 'claimed_ids' => []]);
+                ->andReturn(AdditionalBatchResult::empty());
            $mock->shouldReceive('finish')->once();
        });

@@ -229,6 +234,154 @@ class PostProcessRunnerAdditionalThreadsTest extends TestCase
        $this->assertSame(0, $status);
    }

+    public function test_worker_guid_command_emits_machine_readable_batch_profiles(): void
+    {
+        $batchResult = $this->successfulBatch(1)->withPerformance(2.0, 1024);
+        $this->mock(AdditionalProcessingOrchestrator::class, function (MockInterface $mock) use ($batchResult): void {
+            $mock->shouldReceive('start')->once()->andReturn($batchResult);
+            $mock->shouldReceive('finish')->once();
+        });
+
+        $status = Artisan::call('postprocess:guid', [
+            'type' => 'additional',
+            'guid' => 'a',
+            '--profile' => true,
+        ]);
+        $profile = json_decode(trim(Artisan::output()), true, flags: JSON_THROW_ON_ERROR);
+
+        $this->assertSame(0, $status);
+        $this->assertSame('additional-postprocessing-profile', $profile['event']);
+        $this->assertSame('v2', $profile['pipeline']);
+        $this->assertSame(1, $profile['attempted']);
+        $this->assertSame(1800, $profile['releases_per_hour']);
+        $this->assertSame(1024, $profile['peak_memory_bytes']);
+    }
+
+    public function test_targeted_additional_command_reports_a_successful_typed_outcome(): void
+    {
+        DB::table('categories')->insert(['id' => 1, 'disablepreview' => 0]);
+        DB::table('releases')->insert($this->releaseRow(1, 'a'));
+
+        $this->mock(AdditionalProcessingOrchestrator::class, function (MockInterface $mock): void {
+            $mock->shouldReceive('processSingleGuid')
+                ->once()
+                ->with('a-guid-1')
+                ->andReturn(new ReleaseProcessingResult(
+                    1,
+                    'a-guid-1',
+                    ProcessingOutcome::Completed,
+                    artifactsCreated: true,
+                    releaseFilesAdded: 2,
+                    elapsedSeconds: 1.2345,
+                    stageDurations: ['nzb-parsing' => 0.5],
+                    downloadMetrics: new DownloadMetrics(2, 1, 1, 100, 100),
+                    persistenceMetrics: new PersistenceMetrics(4, 3.5, 2, 1),
+                ));
+            $mock->shouldReceive('finish')->once();
+        });
+
+        $status = Artisan::call('releases:additional', ['--id' => 1, '--profile' => true]);
+        $output = Artisan::output();
+
+        $this->assertSame(0, $status);
+        $this->assertStringContainsString('outcome completed', $output);
+        $this->assertStringContainsString('Artifacts created', $output);
+        $this->assertStringContainsString('Release files added', $output);
+        $this->assertStringContainsString('Stage profile', $output);
+        $this->assertStringContainsString('Download profile', $output);
+        $this->assertStringContainsString('Persistence profile', $output);
+    }
+
+    public function test_targeted_additional_command_fails_for_an_unsuccessful_typed_outcome(): void
+    {
+        DB::table('categories')->insert(['id' => 1, 'disablepreview' => 0]);
+        DB::table('releases')->insert($this->releaseRow(1, 'a'));
+
+        $this->mock(AdditionalProcessingOrchestrator::class, function (MockInterface $mock): void {
+            $mock->shouldReceive('processSingleGuid')
+                ->once()
+                ->with('a-guid-1')
+                ->andReturn(new ReleaseProcessingResult(
+                    1,
+                    'a-guid-1',
+                    ProcessingOutcome::TimedOut,
+                    reason: 'The release exceeded the post-processing timeout.',
+                    elapsedSeconds: 2.3456,
+                ));
+            $mock->shouldReceive('finish')->once();
+        });
+
+        $status = Artisan::call('releases:additional', ['--id' => 1]);
+        $output = Artisan::output();
+
+        $this->assertSame(3, $status);
+        $this->assertStringContainsString('outcome timed-out', $output);
+        $this->assertStringContainsString('Reason', $output);
+        $this->assertStringContainsString('The release exceeded the post-processing timeout.', $output);
+    }
+
+    public function test_additional_diagnostics_reports_stale_claims_and_preflight_warnings_as_json(): void
+    {
+        config(['nntmux.tmp_unrar_path' => '']);
+        DB::table('settings')->upsert([
+            ['name' => 'postthreads', 'value' => '33'],
+            ['name' => 'maxaddprocessed', 'value' => '25'],
+            ['name' => 'maxpptimeoutcount', 'value' => '3'],
+        ], ['name'], ['value']);
+        DB::table('categories')->insert(['id' => 1, 'disablepreview' => 0]);
+        DB::table('releases')->insert([
+            ...$this->releaseRow(1, 'a'),
+            'additional_pp_claimed_at' => now()->subHour(),
+            'additional_pp_claim_token' => 'stale-claim',
+        ]);
+
+        $status = Artisan::call('nntmux:additional-diagnose', ['--json' => true]);
+        $report = json_decode(Artisan::output(), true, flags: JSON_THROW_ON_ERROR);
+        $warningCodes = array_column($report['warnings'], 'code');
+
+        $this->assertSame(0, $status);
+        $this->assertSame(1, $report['backlog']['total']);
+        $this->assertSame(1, $report['backlog']['available']);
+        $this->assertSame(1, $report['backlog']['stale_claims']);
+        $this->assertSame(825, $report['capacity']['max_in_flight']);
+        $this->assertSame('v2', $report['settings']['pipeline']);
+        $this->assertContains('stale-claims', $warningCodes);
+        $this->assertContains('claim-ttl', $warningCodes);
+        $this->assertContains('missing-index', $warningCodes);
+        $this->assertContains('unwritable-temp-path', $warningCodes);
+        $this->assertContains('oversized-capacity', $warningCodes);
+    }
+
+    public function test_additional_diagnostics_recognizes_the_covering_claim_index(): void
+    {
+        Schema::table('releases', function (Blueprint $table): void {
+            $table->index([
+                'passwordstatus',
+                'haspreview',
+                'nzbstatus',
+                'leftguid',
+                'additional_pp_claimed_at',
+                'postdate',
+            ], 'ix_releases_add_pp_claim_queue');
+        });
+
+        $status = Artisan::call('nntmux:additional-diagnose', ['--json' => true]);
+        $report = json_decode(Artisan::output(), true, flags: JSON_THROW_ON_ERROR);
+
+        $this->assertSame(0, $status);
+        $this->assertFalse($report['indexes']['missing']);
+        $this->assertSame('ix_releases_add_pp_claim_queue', $report['indexes']['covering_index']);
+        $this->assertNotContains('missing-index', array_column($report['warnings'], 'code'));
+    }
+
+    private function successfulBatch(int $releaseId): AdditionalBatchResult
+    {
+        return new AdditionalBatchResult(
+            [$releaseId],
+            [new ReleaseProcessingResult($releaseId, 'a-guid-'.$releaseId, ProcessingOutcome::Completed)],
+        );
+    }
+
    /**
     * @return array<string, mixed>
     */