Fix max read1 used for read2 (#26)

Author: simbig

CHANGELOG.md

@@ -9,11 +9,17 @@ See [GitHub releases](https://github.com/mll-lab/php-utils/releases).
 
 ## Unreleased
 
-## v2.2.1
+## v3.0.0
 
 ### Changed
 
-- All OverrideCycle total counts on DataSection must grow to the Reads-Sections maximum value by adding the N-tag
+- All `OverrideCycle` total counts on `DataSection` must grow to the `ReadsSection` maximum value by adding the N-tag
+- Throw when trying to use an empty `DataSection`
+- Breaking Change: class `OverrideCycles` requires class `DataSection` to calculate the max cycles
+
+### Fixed
+
+- Use method `maxRead2Cycles` for calculating `$fillUpToMax` for `read2`, not `maxRead1Cycles`
 
 ## v2.2.0
 

src/IlluminaSampleSheet/V2/BclConvert/DataSection.php

@@ -3,6 +3,7 @@
 namespace MLL\Utils\IlluminaSampleSheet\V2\BclConvert;
 
 use Illuminate\Support\Collection;
+use MLL\Utils\IlluminaSampleSheet\IlluminaSampleSheetException;
 use MLL\Utils\IlluminaSampleSheet\Section;
 use MLL\Utils\IlluminaSampleSheet\V2\ReadsSection;
 
@@ -23,12 +24,14 @@ public function addSample(BclSample $bclSample): void
 
     public function convertSectionToString(): string
     {
+        $this->assertNotEmpty();
+
         /** @var array<string> $samplePropertiesOfFirstSample */
         $samplePropertiesOfFirstSample = array_keys(get_object_vars($this->dataRows[0]));
         foreach ($this->dataRows as $sample) {
             $actualProperties = array_keys(get_object_vars($sample));
             if ($samplePropertiesOfFirstSample !== $actualProperties) {
-                throw new \Exception('All samples must have the same properties. Expected: ' . \Safe\json_encode($samplePropertiesOfFirstSample) . ', Actual: ' . \Safe\json_encode($actualProperties));
+                throw new IlluminaSampleSheetException('All samples must have the same properties. Expected: ' . \Safe\json_encode($samplePropertiesOfFirstSample) . ', Actual: ' . \Safe\json_encode($actualProperties));
             }
         }
 
@@ -114,4 +117,11 @@ public function maxIndex2Cycles(): ?int
 
         return $index2Cycles;
     }
+
+    public function assertNotEmpty(): void
+    {
+        if ($this->dataRows->isEmpty()) {
+            throw new IlluminaSampleSheetException('At least one sample must be added to the DataSection.');
+        }
+    }
 }

src/IlluminaSampleSheet/V2/BclConvert/OverrideCycle.php

@@ -2,6 +2,8 @@
 
 namespace MLL\Utils\IlluminaSampleSheet\V2\BclConvert;
 
+use MLL\Utils\IlluminaSampleSheet\IlluminaSampleSheetException;
+
 class OverrideCycle
 {
     /** @var array<CycleTypeWithCount> */
@@ -17,8 +19,9 @@ public function __construct(array $cycles)
     public function toString(int $fillUpToMax, ?bool $isSecondIndexAndForwardDirection): string
     {
         $countOfAllCycleTypes = $this->sumCountOfAllCycles();
-        assert($countOfAllCycleTypes <= $fillUpToMax, 'The sum of all cycle types must be less than or equal to the fill up to max value.');
-
+        if ($countOfAllCycleTypes > $fillUpToMax) {
+            throw new IlluminaSampleSheetException("The sum of all cycle types must be less than or equal to the fill up to max value. \$countOfAllCycleTypes: {$countOfAllCycleTypes} > \$fillUpToMax: {$fillUpToMax}");
+        }
         $rawOverrideCycle = implode('', array_map(
             fn (CycleTypeWithCount $cycle): string => $cycle->toString(),
             $this->cycles

src/IlluminaSampleSheet/V2/BclConvert/OverrideCycles.php

@@ -29,25 +29,13 @@ public function __construct(DataSection $dataSection, string $read1, string $ind
     public function toString(): string
     {
         $dataSection = $this->dataSection;
-
-        if ($this->index2 instanceof OverrideCycle) {
-            $maxIndex2Cycles = $dataSection->maxIndex2Cycles();
-            if ($maxIndex2Cycles === null) {
-                throw new IlluminaSampleSheetException('MaxIndex2Cycles is required when Index2 is set.');
-            }
-
-            $index2 = $this->index2->toString($maxIndex2Cycles, HeaderSection::isForwardIndexOrientation());
-        } else {
-            $index2 = null;
-        }
+        $dataSection->assertNotEmpty();
 
         return implode(';', array_filter([
             $this->read1->toString($dataSection->maxRead1Cycles(), null),
             $this->index1->toString($dataSection->maxIndex1Cycles(), null),
-            $index2,
-            $this->read2 instanceof OverrideCycle
-                ? $this->read2->toString($dataSection->maxRead1Cycles(), null)
-                : null,
+            $this->index2(),
+            $this->read2(),
         ]));
     }
 
@@ -70,4 +58,30 @@ public function makeOverrideCycle(string $cycleString): OverrideCycle
             )
         );
     }
+
+    private function index2(): ?string
+    {
+        if (! $this->index2 instanceof OverrideCycle) {
+            return null;
+        }
+        $maxIndex2Cycles = $this->dataSection->maxIndex2Cycles();
+        if ($maxIndex2Cycles === null) {
+            throw new IlluminaSampleSheetException('MaxIndex2Cycles is required when Index2 is set.');
+        }
+
+        return $this->index2->toString($maxIndex2Cycles, HeaderSection::isForwardIndexOrientation());
+    }
+
+    private function read2(): ?string
+    {
+        if (! $this->read2 instanceof OverrideCycle) {
+            return null;
+        }
+        $maxIndex2Cycles = $this->dataSection->maxRead2Cycles();
+        if ($maxIndex2Cycles === null) {
+            throw new IlluminaSampleSheetException('MaxRead2Cycles is required when Read2 is set.');
+        }
+
+        return $this->read2->toString($maxIndex2Cycles, null);
+    }
 }

tests/DnaSequenceTest.php

@@ -5,7 +5,7 @@
 use MLL\Utils\DnaSequence;
 use PHPUnit\Framework\TestCase;
 
-class DnaSequenceTest extends TestCase
+final class DnaSequenceTest extends TestCase
 {
     public function testReverse(): void
     {

tests/IlluminaSampleSheet/V1/DataSectionTest.php

@@ -8,7 +8,7 @@
 use MLL\Utils\IlluminaSampleSheet\V1\RowForDualIndexWithoutLane;
 use PHPUnit\Framework\TestCase;
 
-class DataSectionTest extends TestCase
+final class DataSectionTest extends TestCase
 {
     public function testDataSectionWithDualIndexWithLaneWorksNotWhenRowWithWrongTypeIsAdded(): void
     {

tests/IlluminaSampleSheet/V1/DualIndexTest.php

@@ -7,7 +7,7 @@
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 
-class DualIndexTest extends TestCase
+final class DualIndexTest extends TestCase
 {
     /** @dataProvider provideValidDualIndexes */
     #[DataProvider('provideValidDualIndexes')]

tests/IlluminaSampleSheet/V1/SampleSheetTest.php

@@ -13,7 +13,7 @@
 use MLL\Utils\IlluminaSampleSheet\V1\SettingsSection;
 use PHPUnit\Framework\TestCase;
 
-class SampleSheetTest extends TestCase
+final class SampleSheetTest extends TestCase
 {
     public function testDataSectionWithDualIndexWithLaneShouldReturnExpectedResult(): void
     {

tests/IlluminaSampleSheet/V2/DataSectionTest.php

@@ -0,0 +1,40 @@
+<?php declare(strict_types=1);
+
+namespace MLL\Utils\Tests\IlluminaSampleSheet\V2;
+
+use MLL\Utils\IlluminaSampleSheet\IlluminaSampleSheetException;
+use MLL\Utils\IlluminaSampleSheet\V2\BclConvert\BclSample;
+use MLL\Utils\IlluminaSampleSheet\V2\BclConvert\DataSection;
+use MLL\Utils\IlluminaSampleSheet\V2\BclConvert\OverrideCycles;
+use PHPUnit\Framework\TestCase;
+
+final class DataSectionTest extends TestCase
+{
+    public function testToString(): void
+    {
+        $dataSection = new DataSection();
+        $overrideCycles = new OverrideCycles($dataSection, 'Y130', 'I8', 'I10', 'Y100');
+        $dataSection->addSample(new BclSample(100, 'Sample1', 'Index1', $overrideCycles));
+
+        $overrideCycles = new OverrideCycles($dataSection, 'Y100', 'I11', 'I7', 'Y151');
+        $dataSection->addSample(new BclSample(101, 'Sample2', 'Index3', $overrideCycles));
+
+        $expected = <<<'CSV'
+[BCLConvert_Data]
+Lane,Sample_ID,Index,OverrideCycles
+100,Sample1,Index1,Y130;I8N3;I10;Y100N51
+101,Sample2,Index3,Y100N30;I11;N3I7;Y151
+
+CSV;
+        self::assertSame($expected, $dataSection->convertSectionToString());
+    }
+
+    public function testThrowsExceptionIfDataSectionIsEmpty(): void
+    {
+        $dataSection = new DataSection();
+
+        $this->expectException(IlluminaSampleSheetException::class);
+        $this->expectExceptionMessage('At least one sample must be added to the DataSection.');
+        $dataSection->convertSectionToString();
+    }
+}