Add branch id to code quality reports (#286)

migrations/Version20230529072916.php

@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DoctrineMigrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+/**
+ * Auto-generated Migration: Please modify to your needs!
+ */
+final class Version20230529072916 extends AbstractMigration
+{
+    public function getDescription(): string
+    {
+        return '';
+    }
+
+    public function up(Schema $schema): void
+    {
+        // this up() migration is auto-generated, please modify it to your needs
+        $this->addSql('ALTER TABLE code_coverage_report ADD branch_id VARCHAR(255) DEFAULT NULL AFTER commit_hash');
+        $this->addSql('ALTER TABLE code_inspection_report ADD branch_id VARCHAR(255) DEFAULT NULL AFTER inspection_id');
+    }
+
+    public function down(Schema $schema): void
+    {
+        // this down() migration is auto-generated, please modify it to your needs
+        $this->addSql('ALTER TABLE code_coverage_report DROP branch_id');
+        $this->addSql('ALTER TABLE code_inspection_report DROP branch_id');
+    }
+}

resources/openapi/code-coverage.php

@@ -45,6 +45,14 @@
                 true,
                 schema: ['type' => 'string', 'pattern' => '^[a-zA-Z0-9]{6,255}$']
             ),
+            new Parameter(
+                'branchId',
+                'query',
+                'The identifier of the branch the report is for. Can either be branch name, MR/PR id, or something else unique to the branch. This ' .
+                'will be used to find the latest `identifier` within the branch group for a set of commit hashes.',
+                true,
+                schema: ['type' => 'string', 'minLength' => 1, 'maxLength' => 255]
+            ),
             new Parameter(
                 'basePath',
                 'query',

resources/openapi/code-inspection.php

@@ -58,6 +58,14 @@
                 true,
                 schema: ['type' => 'string', 'minLength' => 1, 'maxLength' => 50]
             ),
+            new Parameter(
+                'branchId',
+                'query',
+                'The identifier of the branch the report is for. Can either be branch name, MR/PR id, or something else unique to the branch. This ' .
+                'will be used to find the latest `identifier` within the branch group for a set of commit hashes.',
+                true,
+                schema: ['type' => 'string', 'minLength' => 1, 'maxLength' => 255]
+            ),
             new Parameter(
                 'basePath',
                 'query',

src/Controller/Api/Report/UploadCodeCoverageController.php

@@ -37,16 +37,24 @@ public function __invoke(UploadCodeCoverageRequest $request, string $repositoryN
             throw new NotFoundHttpException();
         }
 
-        $format = $request->getFormat();
+        $format   = $request->getFormat();
         $basePath = $request->getBasePath();
-        $data = $request->getData();
+        $branchId = $request->getBranchId();
+        $data     = $request->getData();
 
         $this->logger?->info(
-            'CodeCoverageReport: {name}, {basePath}, {hash}, {id}, {format}, body size: {size}',
-            ['name' => $repositoryName, 'hash' => $commitHash, 'basePath' => $basePath, 'format' => $format, 'size' => strlen($data)]
+            'CodeCoverageReport: {name}, {basePath}, {hash}, {branchId}, {format}, body size: {size}',
+            [
+                'name'     => $repositoryName,
+                'hash'     => $commitHash,
+                'branchId' => $branchId,
+                'basePath' => $basePath,
+                'format'   => $format,
+                'size'     => strlen($data)
+            ]
         );
 
-        $report = $this->reportFactory->parse($repository, $commitHash, $format, $basePath, $data);
+        $report = $this->reportFactory->parse($repository, $commitHash, $branchId, $format, $basePath, $data);
 
         $this->logger?->info(
             'CodeCoverageReport: {name}, creating report with {count} files.',

src/Controller/Api/Report/UploadCodeInspectionController.php

@@ -38,21 +38,23 @@ public function __invoke(UploadCodeInspectionRequest $request, string $repositor
         }
 
         $this->logger?->info(
-            'CodeInspectionReport: {name}, {basePath}, {hash}, {id}, {format}, body size: {size}',
+            'CodeInspectionReport: {name}, {basePath}, {hash}, {id}, {branchId}, {format}, body size: {size}',
             [
-                'name' => $repositoryName,
-                'hash' => $commitHash,
+                'name'     => $repositoryName,
+                'hash'     => $commitHash,
                 'basePath' => $request->getBasePath(),
-                'id' => $request->getIdentifier(),
-                'format' => $request->getFormat(),
-                'size' => strlen($request->getData())
+                'id'       => $request->getIdentifier(),
+                'branchId' => $request->getBranchId(),
+                'format'   => $request->getFormat(),
+                'size'     => strlen($request->getData())
             ]
         );
 
         $report = $this->reportFactory->parse(
             $repository,
             $commitHash,
             $request->getIdentifier(),
+            $request->getBranchId(),
             $request->getFormat(),
             $request->getBasePath(),
             $request->getData()

src/Entity/Report/CodeCoverageReport.php

@@ -24,6 +24,9 @@ class CodeCoverageReport
     #[ORM\Column(length: 255)]
     private string $commitHash;
 
+    #[ORM\Column(length: 255, nullable: true)]
+    private ?string $branchId = null;
+
     #[ORM\Column]
     private int $createTimestamp;
 
@@ -64,6 +67,18 @@ public function setCommitHash(string $commitHash): self
         return $this;
     }
 
+    public function getBranchId(): ?string
+    {
+        return $this->branchId;
+    }
+
+    public function setBranchId(?string $branchId): self
+    {
+        $this->branchId = $branchId;
+
+        return $this;
+    }
+
     public function getCreateTimestamp(): int
     {
         return $this->createTimestamp;

src/Entity/Report/CodeInspectionReport.php

@@ -26,6 +26,9 @@ class CodeInspectionReport
     #[ORM\Column(length: 50)]
     private ?string $inspectionId = null;
 
+    #[ORM\Column(length: 255, nullable: true)]
+    private ?string $branchId = null;
+
     #[ORM\ManyToOne(targetEntity: Repository::class)]
     #[ORM\JoinColumn(nullable: false)]
     private ?Repository $repository = null;
@@ -78,6 +81,18 @@ public function setInspectionId(?string $inspectionId): self
         return $this;
     }
 
+    public function getBranchId(): ?string
+    {
+        return $this->branchId;
+    }
+
+    public function setBranchId(?string $branchId): self
+    {
+        $this->branchId = $branchId;
+
+        return $this;
+    }
+
     public function getRepository(): ?Repository
     {
         return $this->repository;

src/Repository/Report/CodeInspectionReportRepository.php

@@ -3,6 +3,7 @@
 
 namespace DR\Review\Repository\Report;
 
+use Doctrine\ORM\Query\ResultSetMappingBuilder;
 use Doctrine\Persistence\ManagerRegistry;
 use DR\Review\Doctrine\EntityRepository\ServiceEntityRepository;
 use DR\Review\Entity\Report\CodeInspectionReport;
@@ -24,24 +25,92 @@ public function __construct(ManagerRegistry $registry)
     }
 
     /**
+     * For the given set of revisions find all inspection reports and the branchId if available
+     *
      * @param Revision[] $revisions
      *
-     * @return CodeInspectionReport[]
+     * @return array<array{0: string, 1: string}>
      */
-    public function findByRevisions(Repository $repository, array $revisions): array
+    public function findBranchIds(Repository $repository, array $revisions): array
     {
-        // TODO optimize this to select with group by and then order by
-        $hashes  = array_values(array_map(static fn(Revision $rev): string => (string)$rev->getCommitHash(), $revisions));
-        $reports = $this->findBy(['repository' => $repository, 'commitHash' => $hashes], ['createTimestamp' => 'DESC'], 50);
+        $hashes = array_values(array_map(static fn(Revision $rev): string => (string)$rev->getCommitHash(), $revisions));
+        /** @var array<array{inspectionId: string, branchId: string}> $rows */
+        $rows = $this->getEntityManager()
+            ->createQueryBuilder()
+            ->select(['r.inspectionId', 'r.branchId'])
+            ->from($this->getEntityName(), 'r')
+            ->where('r.commitHash IN (:hashes)')
+            ->andWhere('r.repository = :repositoryId')
+            ->andWhere('r.branchId IS NOT NULL')
+            ->setParameter('hashes', $hashes)
+            ->setParameter('repositoryId', $repository->getId())
+            ->groupBy('r.inspectionId')
+            ->getQuery()
+            ->getArrayResult();
 
         $result = [];
-        foreach ($reports as $report) {
-            if (isset($result[$report->getInspectionId()]) === false) {
-                $result[$report->getInspectionId()] = $report;
-            }
+        foreach ($rows as $row) {
+            $result[] = [$row['inspectionId'], $row['branchId']];
+        }
+
+        return $result;
+    }
+
+    /**
+     * @param Revision[]                         $revisions
+     * @param array<array{0: string, 1: string}> $branchIds
+     *
+     * @return CodeInspectionReport[]
+     */
+    public function findByRevisions(Repository $repository, array $revisions, array $branchIds): array
+    {
+        $conn   = $this->getEntityManager()->getConnection();
+        $qb     = $conn->createQueryBuilder();
+        $params = [
+            'hashes'       => array_values(array_map(static fn(Revision $rev): string => (string)$rev->getCommitHash(), $revisions)),
+            'repositoryId' => $repository->getId()
+        ];
+
+        $filterExpr = [$qb->expr()->in('commit_hash', ':hashes')];
+        foreach ($branchIds as $index => [$inspectionId, $branchId]) {
+            $filterExpr[]                    = (string)$qb->expr()->and(
+                $qb->expr()->eq('inspection_id', ':inspectionId' . $index),
+                $qb->expr()->eq('branch_id', ':branchId' . $index),
+            );
+            $params['inspectionId' . $index] = $inspectionId;
+            $params['branchId' . $index]     = $branchId;
         }
 
-        return array_values($result);
+        // find most recent report for each inspectionId given commit hashes and branchIds
+        $subQuery = $conn->createQueryBuilder()
+            ->select('inspection_id', 'MAX(create_timestamp) AS create_timestamp')
+            ->from('code_inspection_report')
+            ->where($qb->expr()->or(...$filterExpr))
+            ->andWhere('repository_id = :repositoryId')
+            ->groupBy('inspection_id');
+
+        // inner join to get full entity data
+        $query = $conn->createQueryBuilder()
+            ->select('report.*')
+            ->from('code_inspection_report', 'report')
+            ->innerJoin(
+                'report',
+                sprintf('(%s)', $subQuery->getSQL()),
+                'filter',
+                (string)$qb->expr()->and(
+                    $qb->expr()->eq('report.inspection_id', 'filter.inspection_id'),
+                    $qb->expr()->eq('report.create_timestamp', 'filter.create_timestamp')
+                )
+            );
+
+        // create ResultSetMapping to transform to Entity
+        $rsm = new ResultSetMappingBuilder($this->getEntityManager());
+        $rsm->addRootEntityFromClassMetadata(CodeInspectionReport::class, 'report');
+
+        /** @var CodeInspectionReport[] $result */
+        $result = $this->getEntityManager()->createNativeQuery($query->getSQL(), $rsm)->setParameters($params)->getResult();
+
+        return $result;
     }
 
     public function cleanUp(int $beforeTimestamp): int

src/Request/Report/UploadCodeCoverageRequest.php

@@ -9,6 +9,11 @@
 
 class UploadCodeCoverageRequest extends AbstractValidatedRequest
 {
+    public function getBranchId(): ?string
+    {
+        return $this->request->query->get('branchId');
+    }
+
     public function getBasePath(): string
     {
         return $this->request->query->get('basePath', '');
@@ -30,6 +35,7 @@ protected function getValidationRules(): ?ValidationRules
             [
                 'query' => [
                     'basePath' => 'string',
+                    'branchId' => 'string|min:1|max:255',
                     'format'   => 'string|in:' . CoberturaParser::FORMAT
                 ]
             ]

src/Request/Report/UploadCodeInspectionRequest.php

@@ -16,6 +16,11 @@ public function getIdentifier(): string
         return $this->request->query->get('identifier', '');
     }
 
+    public function getBranchId(): ?string
+    {
+        return $this->request->query->get('branchId');
+    }
+
     public function getBasePath(): string
     {
         return $this->request->query->get('basePath', '');
@@ -37,6 +42,7 @@ protected function getValidationRules(): ?ValidationRules
             [
                 'query' => [
                     'identifier' => 'required|string|min:1|max:50',
+                    'branchId'   => 'string|min:1|max:255',
                     'basePath'   => 'string',
                     'format'     => 'string|in:' . CheckStyleIssueParser::FORMAT . ',' . GitlabIssueParser::FORMAT . ',' . JunitIssueParser::FORMAT
                 ]

src/Service/Report/CodeInspection/CodeInspectionReportFactory.php

@@ -16,6 +16,7 @@ public function parse(
         Repository $repository,
         string $commitHash,
         string $inspectionId,
+        ?string $branchId,
         string $format,
         string $basePath,
         string $data
@@ -25,6 +26,7 @@ public function parse(
         $report = new CodeInspectionReport();
         $report->setRepository($repository);
         $report->setInspectionId($inspectionId);
+        $report->setBranchId($branchId);
         $report->setCommitHash($commitHash);
         foreach ($issues as $issue) {
             $issue->setReport($report);

src/Service/Report/Coverage/CodeCoverageReportFactory.php

@@ -12,13 +12,20 @@ public function __construct(private readonly CodeCoverageParserProvider $parserP
     {
     }
 
-    public function parse(Repository $repository, string $commitHash, string $format, string $basePath, string $data): CodeCoverageReport
-    {
+    public function parse(
+        Repository $repository,
+        string $commitHash,
+        ?string $branchId,
+        string $format,
+        string $basePath,
+        string $data
+    ): CodeCoverageReport {
         $files = $this->parserProvider->getParser($format)->parse($basePath, $data);
 
         $report = new CodeCoverageReport();
         $report->setRepository($repository);
         $report->setCommitHash($commitHash);
+        $report->setBranchId($branchId);
         foreach ($files as $file) {
             $file->setReport($report);
             $report->getFiles()->add($file);

src/ViewModelProvider/CodeQualityViewModelProvider.php

@@ -31,7 +31,8 @@ public function getCodeQualityViewModel(CodeReview $review, string $filePath): C
         $revisions  = $this->revisionService->getRevisions($review);
 
         // find code inspections
-        $inspectionReports = $this->reportRepository->findByRevisions($repository, $revisions);
+        $branchIds         = $this->reportRepository->findBranchIds($repository, $revisions);
+        $inspectionReports = $this->reportRepository->findByRevisions($repository, $revisions, $branchIds);
         $issues            = [];
         if (count($inspectionReports) > 0) {
             $issues = $this->issueRepository->findBy(['report' => $inspectionReports, 'file' => $filePath]);

src/ViewModelProvider/ReviewSummaryViewModelProvider.php

@@ -28,8 +28,10 @@ public function __construct(
      */
     public function getSummaryViewModel(CodeReview $review, array $revisions, DirectoryTreeNode $fileTree): ReviewSummaryViewModel
     {
-        $reports = $this->reportRepository->findByRevisions(Assert::notNull($review->getRepository()), $revisions);
-        $issues  = [];
+        $repository = Assert::notNull($review->getRepository());
+        $branchIds  = $this->reportRepository->findBranchIds($repository, $revisions);
+        $reports    = $this->reportRepository->findByRevisions($repository, $revisions, $branchIds);
+        $issues     = [];
         if (count($reports) > 0) {
             $filePaths = array_map(static fn(DiffFile $file) => $file->getPathname(), $fileTree->getFilesRecursive());
             $issues    = $this->issueRepository->findBy(['report' => $reports, 'file' => $filePaths], ['file' => 'ASC']);