diff --git a/.github/workflows/qa.yml b/.github/workflows/qa.yml index 0d3ff49..bfbdf5b 100644 --- a/.github/workflows/qa.yml +++ b/.github/workflows/qa.yml @@ -130,7 +130,7 @@ jobs: id: stan run: | set +e - vendor/bin/phpstan analyse --error-format=json --no-progress > phpstan.json 2>&1 + vendor/bin/phpstan analyse --error-format=json --no-progress > phpstan.json EXIT_CODE=$? # Also run for GitHub format @@ -181,11 +181,16 @@ jobs: id: psalm run: | set +e - vendor/bin/psalm --output-format=json --show-info=false > psalm.json 2>&1 + vendor/bin/psalm --output-format=json --show-info=false > psalm.json EXIT_CODE=$? # Generate SARIF for GitHub Security - vendor/bin/psalm --output-format=sarif --show-info=false > psalm.sarif 2>&1 || true + vendor/bin/psalm --output-format=sarif --show-info=false > psalm.sarif || true + + # Sanitize SARIF: Ensure all location coordinates are >= 1 + if [ -f psalm.sarif ]; then + jq 'walk(if type == "object" then with_entries(if (.key | test("^(start|end)(Line|Column)$")) and (.value < 1) then .value = 1 else . end) else . end)' psalm.sarif > psalm.sarif.tmp && mv psalm.sarif.tmp psalm.sarif + fi ERRORS=$(jq 'length' psalm.json 2>/dev/null || echo "0") echo "errors=${ERRORS}" >> $GITHUB_OUTPUT @@ -200,7 +205,7 @@ jobs: - name: Upload Psalm SARIF if: always() - uses: github/codeql-action/upload-sarif@v3 + uses: github/codeql-action/upload-sarif@v4 with: sarif_file: psalm.sarif category: psalm @@ -242,7 +247,7 @@ jobs: id: fmt run: | set +e - OUTPUT=$(vendor/bin/pint --test --format=json 2>&1) + OUTPUT=$(vendor/bin/pint --test --format=json) EXIT_CODE=$? echo "$OUTPUT" > pint.json @@ -292,7 +297,7 @@ jobs: id: audit run: | set +e - composer audit --format=json > audit.json 2>&1 + composer audit --format=json > audit.json EXIT_CODE=$? VULNS=$(jq '.advisories | to_entries | map(.value | length) | add // 0' audit.json 2>/dev/null || echo "0") diff --git a/src/Core/Config/ConfigVersioning.php b/src/Core/Config/ConfigVersioning.php index 325b9ee..624781b 100644 --- a/src/Core/Config/ConfigVersioning.php +++ b/src/Core/Config/ConfigVersioning.php @@ -295,16 +295,28 @@ public function deleteVersion(int $versionId): void */ protected function pruneOldVersions(int $profileId): void { - $versions = ConfigVersion::where('profile_id', $profileId) + // Find the oldest version we want to keep (the Nth newest) + $cutoff = ConfigVersion::where('profile_id', $profileId) ->orderByDesc('created_at') - ->get(); + ->orderByDesc('id') + ->skip($this->maxVersions - 1) + ->take(1) + ->first(); - if ($versions->count() > $this->maxVersions) { - $toDelete = $versions->slice($this->maxVersions); - foreach ($toDelete as $version) { - $version->delete(); - } + if ($cutoff === null) { + return; } + + // Delete versions older than the cutoff + ConfigVersion::where('profile_id', $profileId) + ->where(function ($query) use ($cutoff) { + $query->where('created_at', '<', $cutoff->created_at) + ->orWhere(function ($q) use ($cutoff) { + $q->where('created_at', $cutoff->created_at) + ->where('id', '<', $cutoff->id); + }); + }) + ->delete(); } /** diff --git a/src/Core/Config/Tests/Feature/ConfigVersioningPerformanceTest.php b/src/Core/Config/Tests/Feature/ConfigVersioningPerformanceTest.php new file mode 100644 index 0000000..dc92969 --- /dev/null +++ b/src/Core/Config/Tests/Feature/ConfigVersioningPerformanceTest.php @@ -0,0 +1,77 @@ +loadMigrationsFrom(__DIR__.'/../../Migrations'); + } + + public function test_pruning_performance() + { + // 1. Setup dependencies + $mockExporter = Mockery::mock(ConfigExporter::class); + $mockExporter->shouldReceive('exportJson') + ->andReturn(json_encode(['values' => []])); + + $configService = app(ConfigService::class); + + $versioning = new ConfigVersioning($configService, $mockExporter); + + // set max versions to a small number to force pruning + Config::set('core.config.max_versions', 50); + // Re-instantiate to pick up config or use setter if available + $versioning->setMaxVersions(50); + + // 2. Create a profile + $profile = ConfigProfile::ensureSystem(); + + // 3. Seed many versions (e.g., 500) + // We insert them directly to avoid triggering pruning during setup + $now = now(); + $versions = []; + for ($i = 0; $i < 500; $i++) { + $versions[] = [ + 'profile_id' => $profile->id, + 'workspace_id' => null, + 'label' => "Version $i", + 'snapshot' => '{}', + 'author' => 'tester', + 'created_at' => $now->subMinutes(500 - $i), // Oldest first + ]; + } + ConfigVersion::insert($versions); + + $initialCount = ConfigVersion::count(); + $this->assertEquals(500, $initialCount); + + // 4. Measure pruning time + $start = microtime(true); + + // createVersion triggers pruneOldVersions + $versioning->createVersion(null, 'New Version'); + + $duration = microtime(true) - $start; + + // 5. Verify + $finalCount = ConfigVersion::count(); + $this->assertEquals(50, $finalCount, 'Should have pruned down to 50 versions'); + + echo "\nPerformance: Pruning took ".number_format($duration * 1000, 2)."ms\n"; + } +}