Merge pull request #91 from keboola/adamvyborny-PST-1966

Component should fail when there are set PKs for non-existing columns in manifest

Author: AdamVyborny

composer.json

@@ -49,7 +49,6 @@
         "phpcs": "phpcs --extensions=php src tests example",
         "phpcbf": "phpcbf --extensions=php src tests example",
         "build": [
-            "@phplint",
             "@phpcs",
             "@phpstan",
             "@tests"

src/Manifest/ManifestManager/Options/OutTable/Serializer/LegacyManifestNormalizer.php

@@ -7,6 +7,7 @@
 use Keboola\Component\Manifest\ManifestManager\Options\OptionsValidationException;
 use Keboola\Component\Manifest\ManifestManager\Options\OutTable\ManifestOptions;
 use Keboola\Component\Manifest\ManifestManager\Options\OutTable\ManifestOptionsSchema;
+use Keboola\Component\UserException;
 use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
 use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
 
@@ -181,6 +182,7 @@ private function setSchema(
     ): void {
         $schema = [];
 
+        $primaryKeysSet = [];
         foreach ($data['columns'] as $columnName) {
             $columnMetadata = $data['column_metadata'][$columnName] ?? [];
             $dataTypes = [];
@@ -198,6 +200,9 @@ private function setSchema(
             }
 
             $isPK = $primaryKey ?: (isset($data['primary_key']) && in_array($columnName, $data['primary_key']));
+            if ($isPK) {
+                $primaryKeysSet[] = $columnName;
+            }
             $schema[] = new ManifestOptionsSchema(
                 $columnName,
                 $dataTypes,
@@ -208,6 +213,13 @@ private function setSchema(
             );
         }
 
+        if (isset($data['primary_key']) && count($primaryKeysSet) !== count($data['primary_key'])) {
+            throw new UserException(sprintf(
+                'Primary keys do not match columns. Missing columns: %s',
+                implode(', ', array_diff($data['primary_key'], $primaryKeysSet)),
+            ));
+        }
+
         $manifestOptions->setSchema($schema);
     }
 

tests/Manifest/ManifestManager/Options/OutTableManifestOptionsTest.php

@@ -7,6 +7,7 @@
 use Keboola\Component\Manifest\ManifestManager\Options\OptionsValidationException;
 use Keboola\Component\Manifest\ManifestManager\Options\OutTable\ManifestOptions;
 use Keboola\Component\Manifest\ManifestManager\Options\OutTable\ManifestOptionsSchema;
+use Keboola\Component\UserException;
 use PHPUnit\Framework\TestCase;
 
 class OutTableManifestOptionsTest extends TestCase
@@ -182,6 +183,30 @@ public function validNamesProvider(): array
         ];
     }
 
+    public function testFromArrayWithNonExistingPrimaryKey(): void
+    {
+        $this->expectException(UserException::class);
+        $this->expectExceptionMessage('Primary keys do not match columns. Missing columns: number');
+
+        ManifestOptions::fromArray([
+            'destination' => 'my.table',
+            'columns' => ['id', 'number #', 'other_column'],
+            'incremental' => true,
+            'primary_key' => ['id', 'number'],
+        ]);
+
+        $this->expectException(UserException::class);
+        $this->expectExceptionMessage('Primary keys do not match columns. ' .
+            'Missing columns: non-existing-column-1, non-existing-column-2');
+
+        ManifestOptions::fromArray([
+            'destination' => 'my.table',
+            'columns' => ['id', 'number', 'other_column'],
+            'incremental' => true,
+            'primary_key' => ['id', 'non-existing-column-1', 'non-existing-column-2'],
+        ]);
+    }
+
     /**
      * @dataProvider provideInvalidOptions
      */