diff --git a/libs/output-mapping/src/LoadTableTaskCreator.php b/libs/output-mapping/src/LoadTableTaskCreator.php index 798a67e2e..c2726dde7 100644 --- a/libs/output-mapping/src/LoadTableTaskCreator.php +++ b/libs/output-mapping/src/LoadTableTaskCreator.php @@ -40,11 +40,14 @@ public function create( $settings->hasNewNativeTypesFeature(), $settings->getTreatValuesAsNull(), ); + $defaultBranchTable = $storageSources->getDefaultBranchTable(); + $canCreateTypedTable = $defaultBranchTable === null || $defaultBranchTable->isTyped(); // some scenarios are not supported by the SAPI, so we need to take care of them manually here // - columns in config + headless CSV (SAPI always expect to have a header in CSV) // - sliced files - if ($settings->hasNativeTypesFeature() && + if ($canCreateTypedTable && + $settings->hasNativeTypesFeature() && !$storageSources->didTableExistBefore() && $source->hasColumns() && $source->hasColumnMetadata() ) { @@ -67,7 +70,8 @@ public function create( ); $this->tableCreator->createTableDefinition($source->getDestination()->getBucketId(), $tableDefinition); $loadTask = new LoadTableTask($source->getDestination(), $loadOptions, true); - } elseif ($settings->hasNewNativeTypesFeature() && + } elseif ($canCreateTypedTable && + $settings->hasNewNativeTypesFeature() && !$storageSources->didTableExistBefore() && $source->getSchema() ) { @@ -78,6 +82,25 @@ public function create( ); $this->tableCreator->createTableDefinition($source->getDestination()->getBucketId(), $tableDefinition); $loadTask = new LoadTableTask($source->getDestination(), $loadOptions, true); + } elseif (!$canCreateTypedTable && + $settings->hasNewNativeTypesFeature() && + !$storageSources->didTableExistBefore() && + $source->getSchema() + ) { + // Typed-table creation was suppressed because the production (default-branch) table is + // non-typed (AJDA-3014). The columns are still known from the schema, so create a plain + // non-typed table with those columns; otherwise the headless CSV would be imported with + // its first data row taken as the header (via the CreateAndLoadTableTask fallback). + $this->tableCreator->createTable( + $source->getDestination()->getBucketId(), + $source->getDestination()->getTableName(), + array_map( + fn (MappingFromConfigurationSchemaColumn $column) => $column->getName(), + $source->getSchema(), + ), + $loadOptions, + ); + $loadTask = new LoadTableTask($source->getDestination(), $loadOptions, true); } elseif (!$storageSources->didTableExistBefore() && $source->hasColumns()) { // tabulka neexistuje a známe sloupce z manifestu $this->tableCreator->createTable( diff --git a/libs/output-mapping/src/Mapping/MappingStorageSources.php b/libs/output-mapping/src/Mapping/MappingStorageSources.php index a157507cf..528e54443 100644 --- a/libs/output-mapping/src/Mapping/MappingStorageSources.php +++ b/libs/output-mapping/src/Mapping/MappingStorageSources.php @@ -9,8 +9,11 @@ class MappingStorageSources { - public function __construct(readonly private BucketInfo $bucket, readonly private ?TableInfo $table) - { + public function __construct( + readonly private BucketInfo $bucket, + readonly private ?TableInfo $table, + readonly private ?TableInfo $defaultBranchTable = null, + ) { } public function getBucket(): BucketInfo @@ -27,4 +30,9 @@ public function getTable(): ?TableInfo { return $this->table; } + + public function getDefaultBranchTable(): ?TableInfo + { + return $this->defaultBranchTable; + } } diff --git a/libs/output-mapping/src/Storage/StoragePreparer.php b/libs/output-mapping/src/Storage/StoragePreparer.php index 0ed588c65..fa22b627f 100644 --- a/libs/output-mapping/src/Storage/StoragePreparer.php +++ b/libs/output-mapping/src/Storage/StoragePreparer.php @@ -35,6 +35,15 @@ public function prepareStorageBucketAndTable( $destinationTableInfo = $this->getDestinationTableInfoIfExists( $processedSource->getDestination()->getTableId(), ); + $defaultBranchTableInfo = null; + if ($destinationTableInfo === null && + $this->clientWrapper->getClientOptionsReadOnly()->useBranchStorage() && + $this->clientWrapper->isDevelopmentBranch() + ) { + $defaultBranchTableInfo = $this->getDefaultBranchTableInfoIfExists( + $processedSource->getDestination()->getTableId(), + ); + } if ($destinationTableInfo !== null) { if ($this->hasNewNativeTypeFeature && $processedSource->getSchema()) { @@ -69,7 +78,7 @@ public function prepareStorageBucketAndTable( ); } - return new MappingStorageSources($destinationBucket, $destinationTableInfo); + return new MappingStorageSources($destinationBucket, $destinationTableInfo, $defaultBranchTableInfo); } private function getDestinationTableInfoIfExists(string $tableId): ?TableInfo @@ -84,4 +93,17 @@ private function getDestinationTableInfoIfExists(string $tableId): ?TableInfo return null; } + + private function getDefaultBranchTableInfoIfExists(string $tableId): ?TableInfo + { + try { + return new TableInfo($this->clientWrapper->getClientForDefaultBranch()->getTable($tableId)); + } catch (ClientException $e) { + if ($e->getCode() !== 404) { + throw $e; + } + } + + return null; + } } diff --git a/libs/output-mapping/tests/LoadTableTaskCreatorTest.php b/libs/output-mapping/tests/LoadTableTaskCreatorTest.php index b3a12ec69..ad19e3334 100644 --- a/libs/output-mapping/tests/LoadTableTaskCreatorTest.php +++ b/libs/output-mapping/tests/LoadTableTaskCreatorTest.php @@ -14,6 +14,7 @@ use Keboola\OutputMapping\Mapping\MappingFromRawConfigurationAndPhysicalDataWithManifest; use Keboola\OutputMapping\Mapping\MappingStorageSources; use Keboola\OutputMapping\Storage\BucketInfo; +use Keboola\OutputMapping\Storage\TableInfo; use Keboola\OutputMapping\Tests\AbstractTestCase; use Keboola\OutputMapping\Tests\Needs\NeedsEmptyOutputBucket; use Keboola\OutputMapping\Tests\Needs\NeedsTestTables; @@ -347,6 +348,142 @@ public function testLoadTaskTableNotExistsManifestNotExists(): void ); } + #[NeedsEmptyOutputBucket] + public function testNativeTypeCreationSuppressedWhenDefaultBranchTableNonTyped(): void + { + // The source has native types enabled and column metadata, so it would normally create a + // typed table (see testNativeTypeLoadTaskTableNotExists). Because the production + // (default-branch) table already exists and is NON-typed, typed creation must be + // suppressed and a plain non-typed table created instead (AJDA-3014). + $settings = self::createMock(OutputMappingSettings::class); + // The typed-creation branches are gated on $canCreateTypedTable first, which is false here + // and short-circuits, so hasNativeTypesFeature() is never queried. hasNewNativeTypesFeature() + // is queried in buildLoadOptions and again in the suppressed-schema branch (returns false + // here since this is the metadata path, so a plain non-typed table is created via columns). + $settings->expects(self::never())->method('hasNativeTypesFeature'); + $settings->expects(self::exactly(2))->method('hasNewNativeTypesFeature')->willReturn(false); + + $strategy = self::createMock(LocalTableStrategy::class); + $strategy->expects(self::once()) + ->method('prepareLoadTaskOptions') + ->willReturn([]); + + $source = self::createMock(MappingFromProcessedConfiguration::class); + $source->expects(self::once())->method('hasColumns')->willReturn(true); + $source->expects(self::exactly(3))->method('getDestination')->willReturn( + new MappingDestination($this->emptyOutputBucketId . '.destinationTable'), + ); + $source->expects(self::once())->method('getPrimaryKey')->willReturn([]); + $source->expects(self::exactly(2))->method('getColumns')->willReturn(['Id', 'Name']); + + $storageSources = self::createMock(MappingStorageSources::class); + $storageSources->expects(self::exactly(2))->method('didTableExistBefore')->willReturn(false); + $storageSources->expects(self::once())->method('getDefaultBranchTable')->willReturn( + new TableInfo([ + 'id' => $this->emptyOutputBucketId . '.destinationTable', + 'columns' => ['Id', 'Name'], + 'primaryKey' => [], + 'isTyped' => false, + ]), + ); + + $loadTableTaskCreator = new LoadTableTaskCreator( + $this->clientWrapper, + $this->testLogger, + ); + $loadTask = $loadTableTaskCreator->create( + strategy: $strategy, + source: $source, + storageSources: $storageSources, + settings: $settings, + ); + + self::assertInstanceOf(LoadTableTask::class, $loadTask); + self::assertTrue($loadTask->isUsingFreshlyCreatedTable()); + $storageTable = $this->clientWrapper->getTableAndFileStorageClient()->getTable( + $this->emptyOutputBucketId . '.destinationTable', + ); + + self::assertFalse($storageTable['isTyped']); + self::assertSame(['Id', 'Name'], $storageTable['columns']); + } + + #[NeedsEmptyOutputBucket] + public function testNativeTypeCreationAllowedWhenDefaultBranchTableTyped(): void + { + // Same setup as testNativeTypeLoadTaskTableNotExists, but with a TYPED production + // (default-branch) table present. The fix must not over-suppress: typed creation stays + // allowed when the production table is already typed (AJDA-3014). + $settings = self::createMock(OutputMappingSettings::class); + $settings->expects(self::once())->method('hasNativeTypesFeature')->willReturn(true); + $settings->expects(self::once())->method('hasBigqueryNativeTypesFeature')->willReturn(false); + + $strategy = self::createMock(LocalTableStrategy::class); + $strategy->expects(self::once()) + ->method('prepareLoadTaskOptions') + ->willReturn([]); + + $source = self::createMock(MappingFromProcessedConfiguration::class); + $source->expects(self::once())->method('hasColumns')->willReturn(true); + $source->expects(self::once())->method('hasColumnMetadata')->willReturn(true); + $source->expects(self::once())->method('hasMetadata')->willReturn(false); + $source->expects(self::exactly(3))->method('getDestination')->willReturn( + new MappingDestination($this->emptyOutputBucketId . '.destinationTable'), + ); + $source->expects(self::exactly(2))->method('getPrimaryKey')->willReturn([]); + $source->expects(self::once())->method('getColumnMetadata')->willReturn([ + new MappingColumnMetadata('col1', [ + [ + 'key' => 'KBC.datatype.basetype', + 'value' => 'STRING', + ], + ]), + new MappingColumnMetadata('col2', [ + [ + 'key' => 'KBC.datatype.basetype', + 'value' => 'INTEGER', + ], + ]), + ]); + + $storageSources = self::createMock(MappingStorageSources::class); + $storageSources->expects(self::exactly(2))->method('didTableExistBefore')->willReturn(false); + $storageSources->expects(self::once())->method('getDefaultBranchTable')->willReturn( + new TableInfo([ + 'id' => $this->emptyOutputBucketId . '.destinationTable', + 'columns' => ['col1', 'col2'], + 'primaryKey' => [], + 'isTyped' => true, + ]), + ); + $storageSources->expects(self::once())->method('getBucket')->willReturn( + new BucketInfo([ + 'id' => $this->emptyOutputBucketId, + 'backend' => 'Snowflake', + 'metadata' => [], + ]), + ); + + $loadTableTaskCreator = new LoadTableTaskCreator( + $this->clientWrapper, + $this->testLogger, + ); + $loadTask = $loadTableTaskCreator->create( + strategy: $strategy, + source: $source, + storageSources: $storageSources, + settings: $settings, + ); + + self::assertInstanceOf(LoadTableTask::class, $loadTask); + self::assertTrue($loadTask->isUsingFreshlyCreatedTable()); + $storageTable = $this->clientWrapper->getTableAndFileStorageClient()->getTable( + $this->emptyOutputBucketId . '.destinationTable', + ); + + self::assertTrue($storageTable['isTyped']); + } + /** * @dataProvider buildLoadOptionsDataProvider */ diff --git a/libs/output-mapping/tests/Mapping/MappingStorageSourcesTest.php b/libs/output-mapping/tests/Mapping/MappingStorageSourcesTest.php index 7798eb907..950645e0b 100644 --- a/libs/output-mapping/tests/Mapping/MappingStorageSourcesTest.php +++ b/libs/output-mapping/tests/Mapping/MappingStorageSourcesTest.php @@ -38,5 +38,10 @@ public function testBasic(): void self::assertEquals($bucketInfo, $source->getBucket()); self::assertFalse($source->didTableExistBefore()); self::assertNull($source->getTable()); + self::assertNull($source->getDefaultBranchTable()); + + $source = new MappingStorageSources($bucketInfo, null, $table); + self::assertFalse($source->didTableExistBefore()); + self::assertEquals($table, $source->getDefaultBranchTable()); } } diff --git a/libs/output-mapping/tests/Writer/StorageApiLocalTableWriterTest.php b/libs/output-mapping/tests/Writer/StorageApiLocalTableWriterTest.php index 314207e9e..6bbce32ed 100644 --- a/libs/output-mapping/tests/Writer/StorageApiLocalTableWriterTest.php +++ b/libs/output-mapping/tests/Writer/StorageApiLocalTableWriterTest.php @@ -319,6 +319,211 @@ public function testWriteTableOutputMappingRealDevMode(): void ); } + #[NeedsEmptyOutputBucket] + #[NeedsDevBranch] + public function testFirstDevBranchWritePreservesNonTypedProductionTable(): void + { + $root = $this->temp->getTmpFolder(); + $tableName = 'table'; + $tableId = $this->emptyOutputBucketId . '.' . $tableName; + $productionCsv = new CsvFile($root . '/production.csv'); + $productionCsv->writeRow(['Id', 'Name']); + $productionCsv->writeRow(['1', 'production']); + + $this->clientWrapper->getTableAndFileStorageClient()->createTableAsync( + $this->emptyOutputBucketId, + $tableName, + $productionCsv, + ); + self::assertFalse($this->clientWrapper->getTableAndFileStorageClient()->getTable($tableId)['isTyped']); + + $clientOptions = $this->clientWrapper->getClientOptionsReadOnly() + ->setBranchId($this->devBranchId) + ->setUseBranchStorage(true) + ; + $this->clientWrapper = new ClientWrapper($clientOptions); + + file_put_contents( + $root . '/upload/table.csv', + "\"2\",\"development\"\n", + ); + + $tableQueue = $this->getTableLoader()->uploadTables( + configuration: new OutputMappingSettings( + configuration: [ + 'mapping' => [ + [ + 'source' => 'table.csv', + 'destination' => $tableId, + 'schema' => [ + [ + 'name' => 'Id', + 'data_type' => [ + 'base' => [ + 'type' => 'INTEGER', + ], + ], + ], + [ + 'name' => 'Name', + 'data_type' => [ + 'base' => [ + 'type' => 'STRING', + ], + ], + ], + ], + ], + ], + ], + sourcePathPrefix: 'upload', + storageApiToken: $this->createTokenWithNewNativeTypesFeature(), + isFailedJob: false, + dataTypeSupport: OutputMappingSettings::DATA_TYPES_SUPPORT_AUTHORITATIVE, + ), + systemMetadata: new SystemMetadata(['componentId' => 'foo', 'branchId' => $this->devBranchId]), + ); + $tableQueue->waitForAll(); + + // The branch table must stay non-typed AND still have the correct columns and data: + // the suppressed-typing path routes through the "unknown columns" CreateAndLoadTableTask + // fallback, so we explicitly assert columns/data are correct, not just the isTyped flag. + $branchTable = $this->clientWrapper->getTableAndFileStorageClient()->getTable($tableId); + self::assertFalse($branchTable['isTyped']); + self::assertSame(['Id', 'Name'], $branchTable['columns']); + $this->assertTableRowsEquals($tableId, [ + '"id","name"', + '"2","development"', + ]); + } + + #[NeedsEmptyOutputBucket] + #[NeedsDevBranch] + public function testFirstDevBranchWriteCreatesTypedTableWhenProductionTableTyped(): void + { + // Counterpart to testFirstDevBranchWritePreservesNonTypedProductionTable: when the + // production table is already TYPED, the fix must NOT over-suppress — the branch table + // must still be created typed (AJDA-3014). + $root = $this->temp->getTmpFolder(); + $tableName = 'table'; + $tableId = $this->emptyOutputBucketId . '.' . $tableName; + + $this->clientWrapper->getTableAndFileStorageClient()->createTableDefinition( + $this->emptyOutputBucketId, + [ + 'name' => $tableName, + 'primaryKeysNames' => [], + 'columns' => [ + ['name' => 'Id', 'basetype' => 'INTEGER'], + ['name' => 'Name', 'basetype' => 'STRING'], + ], + ], + ); + self::assertTrue($this->clientWrapper->getTableAndFileStorageClient()->getTable($tableId)['isTyped']); + + $clientOptions = $this->clientWrapper->getClientOptionsReadOnly() + ->setBranchId($this->devBranchId) + ->setUseBranchStorage(true) + ; + $this->clientWrapper = new ClientWrapper($clientOptions); + + file_put_contents( + $root . '/upload/table.csv', + "\"2\",\"development\"\n", + ); + + $tableQueue = $this->getTableLoader()->uploadTables( + configuration: new OutputMappingSettings( + configuration: [ + 'mapping' => [ + [ + 'source' => 'table.csv', + 'destination' => $tableId, + 'schema' => [ + ['name' => 'Id', 'data_type' => ['base' => ['type' => 'INTEGER']]], + ['name' => 'Name', 'data_type' => ['base' => ['type' => 'STRING']]], + ], + ], + ], + ], + sourcePathPrefix: 'upload', + storageApiToken: $this->createTokenWithNewNativeTypesFeature(), + isFailedJob: false, + dataTypeSupport: OutputMappingSettings::DATA_TYPES_SUPPORT_AUTHORITATIVE, + ), + systemMetadata: new SystemMetadata(['componentId' => 'foo', 'branchId' => $this->devBranchId]), + ); + $tableQueue->waitForAll(); + + self::assertTrue($this->clientWrapper->getTableAndFileStorageClient()->getTable($tableId)['isTyped']); + } + + #[NeedsEmptyOutputBucket] + #[NeedsDevBranch] + public function testFirstDevBranchWriteCreatesTypedTableWhenProductionTableAbsent(): void + { + // A genuinely new table (no production counterpart) must still get automatic typed-table + // creation inside the branch — the fix only preserves the status of pre-existing + // production tables (AJDA-3014). This also exercises the 404 path of + // StoragePreparer::getDefaultBranchTableInfoIfExists. + $root = $this->temp->getTmpFolder(); + $tableName = 'table'; + $tableId = $this->emptyOutputBucketId . '.' . $tableName; + + self::assertFalse($this->clientWrapper->getTableAndFileStorageClient()->tableExists($tableId)); + + $clientOptions = $this->clientWrapper->getClientOptionsReadOnly() + ->setBranchId($this->devBranchId) + ->setUseBranchStorage(true) + ; + $this->clientWrapper = new ClientWrapper($clientOptions); + + file_put_contents( + $root . '/upload/table.csv', + "\"2\",\"development\"\n", + ); + + $tableQueue = $this->getTableLoader()->uploadTables( + configuration: new OutputMappingSettings( + configuration: [ + 'mapping' => [ + [ + 'source' => 'table.csv', + 'destination' => $tableId, + 'schema' => [ + ['name' => 'Id', 'data_type' => ['base' => ['type' => 'INTEGER']]], + ['name' => 'Name', 'data_type' => ['base' => ['type' => 'STRING']]], + ], + ], + ], + ], + sourcePathPrefix: 'upload', + storageApiToken: $this->createTokenWithNewNativeTypesFeature(), + isFailedJob: false, + dataTypeSupport: OutputMappingSettings::DATA_TYPES_SUPPORT_AUTHORITATIVE, + ), + systemMetadata: new SystemMetadata(['componentId' => 'foo', 'branchId' => $this->devBranchId]), + ); + $tableQueue->waitForAll(); + + self::assertTrue($this->clientWrapper->getTableAndFileStorageClient()->getTable($tableId)['isTyped']); + } + + private function createTokenWithNewNativeTypesFeature(): StorageApiToken + { + $realToken = $this->clientWrapper->getToken(); + $token = $this->createMock(StorageApiToken::class); + $token + ->method('hasFeature') + ->willReturnCallback( + static fn(string $feature): bool => $feature === OutputMappingSettings::NEW_NATIVE_TYPES_FEATURE + || $realToken->hasFeature($feature), + ) + ; + + return $token; + } + #[NeedsEmptyOutputBucket] public function testWriteTableOutputMappingExistingTable(): void {