feat(output-mapping): AJDA-2946 store description only when table flag is system-managed

libs/output-mapping/README.md

@@ -8,6 +8,7 @@ Output mapping library for Keboola Runner and Workspaces. Processes component ou
     - Requires: `output-mapping-slice` feature flag, default CSV format (`,` delimiter, `"` enclosure), no custom `columns` mapping
     - Sliced files must have `columns` or `schema` specified in configuration
 - Workspace staging (Snowflake/BigQuery): Loads tables directly from workspace database objects (no file upload, no slicing)
+- Descriptions: Table and column descriptions (`description`, `schema[].description` or `KBC.description` metadata) are stored in the native Storage description field. A table created by the run always gets its description; on an existing table the description is only stored when the table's `isDescriptionSystemManaged` flag is set, so a description managed by the user is never overwritten.
 
 **Files:**
 - Uploads files as-is to Storage API File Storage (works with all staging types)

libs/output-mapping/composer.json

@@ -35,7 +35,7 @@
         "keboola/php-storage-names-sanitizer": "*@dev",
         "keboola/slicer": "*@dev",
         "keboola/staging-provider": "*@dev",
-        "keboola/storage-api-client": "^18.5",
+        "keboola/storage-api-client": "^18.9",
         "keboola/storage-api-php-client-branch-wrapper": "^7.0",
         "microsoft/azure-storage-blob": "^1.5",
         "psr/log": "^2.0|^3.0",

libs/output-mapping/phpunit.xml.dist

@@ -39,6 +39,7 @@
             <exclude>tests/Writer/StorageApiHeadlessWriterTest.php</exclude>
             <exclude>tests/Writer/StorageApiLocalTableWriterTest.php</exclude>
             <exclude>tests/Writer/StorageApiSlicedWriterTest.php</exclude>
+            <exclude>tests/Writer/TableDescriptionWriterTest.php</exclude>
         </testsuite>
         <testsuite name="workspace-writer-tests">
             <directory>tests/Writer/Workspace</directory>

libs/output-mapping/src/DeferredTasks/FailedLoadTableDecider.php

@@ -4,6 +4,7 @@
 
 namespace Keboola\OutputMapping\DeferredTasks;
 
+use Keboola\OutputMapping\Writer\Helper\DescriptionHelper;
 use Keboola\StorageApi\ClientException;
 use Keboola\StorageApiBranch\ClientWrapper;
 use Psr\Log\LoggerInterface;
@@ -29,6 +30,16 @@ public static function decideTableDelete(
             );
         }
 
+        // A description passed in the create-table-definition payload makes Storage write a KBC.description
+        // row under the "storage" provider at create time, i.e. before anything is loaded. It therefore says
+        // nothing about the table having been used and must not stop the drop of a failed empty table. This
+        // is intentionally outside the isTyped branch above - the non-typed create-table-definition payload
+        // (TableDefinitionFromColumns) can carry a description too.
+        $metadata = array_filter(
+            $metadata,
+            fn($m) => $m['key'] !== DescriptionHelper::DESCRIPTION_METADATA_KEY || $m['provider'] !== 'storage',
+        );
+
         if ($task->isUsingFreshlyCreatedTable() && // most important
             ($tableInfo['rowsCount'] === 0 || $tableInfo['rowsCount'] === null) && // seems both are possible 🙄
             (count($metadata) === 0) // at this point there should be no metadata, they're set after load

libs/output-mapping/src/DeferredTasks/LoadTableQueue.php

@@ -6,6 +6,9 @@
 
 use Keboola\InputMapping\Table\Result\TableInfo;
 use Keboola\OutputMapping\Exception\InvalidOutputException;
+use Keboola\OutputMapping\Storage\TableDescription;
+use Keboola\OutputMapping\Storage\TableDescriptionModifier;
+use Keboola\OutputMapping\Storage\TableInfo as StorageTableInfo;
 use Keboola\OutputMapping\Table\Result;
 use Keboola\OutputMapping\Table\Result\Metrics;
 use Keboola\StorageApi\ClientException;
@@ -20,16 +23,26 @@ class LoadTableQueue
 
     /** @var LoadTableTaskInterface[] */
     private array $loadTableTasks;
+
+    /** @var array<string, TableDescription> table id => descriptions of a table created by this run */
+    private array $createdTableDescriptions;
+
     private Result $tableResult;
 
     /**
      * @param LoadTableTaskInterface[] $loadTableTasks
+     * @param array<string, TableDescription> $createdTableDescriptions
      */
-    public function __construct(ClientWrapper $clientWrapper, LoggerInterface $logger, array $loadTableTasks)
-    {
+    public function __construct(
+        ClientWrapper $clientWrapper,
+        LoggerInterface $logger,
+        array $loadTableTasks,
+        array $createdTableDescriptions = [],
+    ) {
         $this->clientWrapper = $clientWrapper;
         $this->logger = $logger;
         $this->loadTableTasks = $loadTableTasks;
+        $this->createdTableDescriptions = $createdTableDescriptions;
         $this->tableResult = new Result();
     }
 
@@ -66,14 +79,20 @@ public function waitForAll(): array
             $jobResult = $this->clientWrapper->getBranchClient()->waitForJob($jobId);
 
             if ($jobResult['status'] === 'error') {
+                $destinationTableName = $task->getDestinationTableName();
                 $errors[] = sprintf(
                     'Failed to load table "%s": %s',
-                    $task->getDestinationTableName(),
+                    $destinationTableName,
                     $jobResult['error']['message'],
                 );
+                // The load failed, so there is no table to describe - it is about to be dropped below or it
+                // keeps the description it already had. Dropping the pending description here keeps it out of
+                // the "was not stored" warning, which is meant for descriptions lost by mistake.
+                unset($this->createdTableDescriptions[$destinationTableName]);
+
                 if (FailedLoadTableDecider::decideTableDelete($this->logger, $this->clientWrapper, $task)) {
                     $this->clientWrapper->getTableAndFileStorageClient()->dropTable(
-                        $task->getDestinationTableName(),
+                        $destinationTableName,
                         ['force' => true],
                     );
                 }
@@ -107,17 +126,32 @@ public function waitForAll(): array
                         $jobResults[] = $jobResult;
                         break;
                     case 'tableCreate':
-                        $this->tableResult->addTable(
-                            new TableInfo($this->clientWrapper->getTableAndFileStorageClient()->getTable(
-                                $jobResult['results']['id'],
-                            )),
+                        $tableData = $this->clientWrapper->getTableAndFileStorageClient()->getTable(
+                            $jobResult['results']['id'],
                         );
+                        $this->tableResult->addTable(new TableInfo($tableData));
+                        // Only a table created by the load job itself (CreateAndLoadTableTask) can have a
+                        // pending description - every other table is created through a table definition,
+                        // which carries the description in its create payload.
+                        try {
+                            $this->applyCreatedTableDescriptions($task, $tableData);
+                        } catch (InvalidOutputException $e) {
+                            $errors[] = $e->getMessage();
+                        }
                         $jobResults[] = $jobResult;
                         break;
                 }
             }
         }
 
+        if ($this->createdTableDescriptions !== []) {
+            // Must never happen - a description left here would be silently thrown away.
+            $this->logger->warning(sprintf(
+                'Description of table(s) "%s" was not stored.',
+                implode('", "', array_keys($this->createdTableDescriptions)),
+            ));
+        }
+
         $this->tableResult->setMetrics(new Metrics($jobResults));
 
         if ($errors) {
@@ -126,6 +160,25 @@ public function waitForAll(): array
         return $jobIds;
     }
 
+    /**
+     * @param array<mixed> $tableData table detail as returned by Storage after a successful load
+     */
+    private function applyCreatedTableDescriptions(LoadTableTaskInterface $task, array $tableData): void
+    {
+        $tableId = $task->getDestinationTableName();
+        $descriptions = $this->createdTableDescriptions[$tableId] ?? null;
+        if ($descriptions === null) {
+            return;
+        }
+
+        // Several sources may be mapped to the same destination table, but the descriptions of a table only
+        // need to be stored once.
+        unset($this->createdTableDescriptions[$tableId]);
+
+        $descriptionModifier = new TableDescriptionModifier($this->clientWrapper, $this->logger);
+        $descriptionModifier->updateDescriptions(new StorageTableInfo($tableData), $descriptions);
+    }
+
     public function getTaskCount(): int
     {
         return count($this->loadTableTasks);

libs/output-mapping/src/DeferredTasks/Metadata/ColumnsMetadata.php

@@ -35,7 +35,15 @@ public function apply(Metadata $apiClient, int $bulkSize = 100): void
                     ];
                 }
 
-                $columnsMetadata[$mappingColumnMetadata->getColumnName()] = $columnMetadata;
+                // a column may end up with no metadata at all, e.g. when its only entry was the description,
+                // which is stored in the native Storage description field instead
+                if ($columnMetadata) {
+                    $columnsMetadata[$mappingColumnMetadata->getColumnName()] = $columnMetadata;
+                }
+            }
+
+            if (!$columnsMetadata) {
+                continue;
             }
 
             $options = new TableMetadataUpdateOptions(

libs/output-mapping/src/LoadTableTaskCreator.php

@@ -4,18 +4,19 @@
 
 namespace Keboola\OutputMapping;
 
-use Keboola\OutputMapping\DeferredTasks\LoadTableTaskInterface;
 use Keboola\OutputMapping\DeferredTasks\TableWriter\CreateAndLoadTableTask;
 use Keboola\OutputMapping\DeferredTasks\TableWriter\LoadTableTask;
 use Keboola\OutputMapping\Mapping\MappingFromConfigurationSchemaColumn;
 use Keboola\OutputMapping\Mapping\MappingFromProcessedConfiguration;
 use Keboola\OutputMapping\Mapping\MappingStorageSources;
 use Keboola\OutputMapping\Storage\NativeTypeDecisionHelper;
 use Keboola\OutputMapping\Storage\TableCreator;
+use Keboola\OutputMapping\Storage\TableDescription;
 use Keboola\OutputMapping\Writer\Table\StrategyInterface;
 use Keboola\OutputMapping\Writer\Table\TableDefinition\TableDefinitionFactory;
 use Keboola\OutputMapping\Writer\Table\TableDefinition\TableDefinitionFromColumns;
 use Keboola\OutputMapping\Writer\Table\TableDefinitionFromSchema\TableDefinitionFromSchema;
+use Keboola\OutputMapping\Writer\Table\TableDefinitionInterface;
 use Keboola\StorageApiBranch\ClientWrapper;
 use Psr\Log\LoggerInterface;
 
@@ -28,12 +29,19 @@ public function __construct(readonly ClientWrapper $clientWrapper, readonly Logg
         $this->tableCreator = new TableCreator($clientWrapper);
     }
 
+    /**
+     * @param TableDescription|null $descriptions descriptions produced by output mapping for the destination
+     *     table; they are embedded in the create-table-definition payload whenever the table is created here.
+     *     Descriptions of a table which existed before are handled by StoragePreparer, where the
+     *     `isDescriptionSystemManaged` flag of the table is known.
+     */
     public function create(
         StrategyInterface $strategy,
         MappingFromProcessedConfiguration $source,
         MappingStorageSources $storageSources,
         OutputMappingSettings $settings,
-    ): LoadTableTaskInterface {
+        ?TableDescription $descriptions = null,
+    ): LoadTableTaskResult {
         $loadOptions = $this->buildLoadOptions(
             $source,
             $strategy,
@@ -66,8 +74,8 @@ public function create(
                 $source->getPrimaryKey(),
                 $source->getColumnMetadata(),
             );
-            $this->tableCreator->createTableDefinition($source->getDestination()->getBucketId(), $tableDefinition);
-            $loadTask = new LoadTableTask($source->getDestination(), $loadOptions, true);
+            $this->createTableDefinition($source, $tableDefinition, $descriptions);
+            return new LoadTableTaskResult(new LoadTableTask($source->getDestination(), $loadOptions, true));
         } elseif ($settings->hasNewNativeTypesFeature() &&
             !$storageSources->didTableExistBefore() &&
             $source->getSchema()
@@ -77,20 +85,20 @@ public function create(
                 $source->getSchema(),
                 $storageSources->getBucket()->backend,
             );
-            $this->tableCreator->createTableDefinition($source->getDestination()->getBucketId(), $tableDefinition);
-            $loadTask = new LoadTableTask($source->getDestination(), $loadOptions, true);
+            $this->createTableDefinition($source, $tableDefinition, $descriptions);
+            return new LoadTableTaskResult(new LoadTableTask($source->getDestination(), $loadOptions, true));
         } elseif (!$storageSources->didTableExistBefore() && $source->hasColumns()) {
             // tabulka neexistuje a známe sloupce z manifestu - vytváříme ji přes table definition bez typů
             $tableDefinition = new TableDefinitionFromColumns(
                 $source->getDestination()->getTableName(),
                 $source->getColumns(),
                 $source->getPrimaryKey(),
             );
-            $this->tableCreator->createTableDefinition($source->getDestination()->getBucketId(), $tableDefinition);
-            $loadTask = new LoadTableTask($source->getDestination(), $loadOptions, true);
+            $this->createTableDefinition($source, $tableDefinition, $descriptions);
+            return new LoadTableTaskResult(new LoadTableTask($source->getDestination(), $loadOptions, true));
         } elseif ($storageSources->didTableExistBefore()) {
             // tabulka existuje takže nahráváme data
-            $loadTask = new LoadTableTask($source->getDestination(), $loadOptions, false);
+            return new LoadTableTaskResult(new LoadTableTask($source->getDestination(), $loadOptions, false));
         } else {
             // tabulka nemá manifest a tím nemá známé columns
             if ($settings->getTreatValuesAsNull() !== null) {
@@ -100,9 +108,30 @@ public function create(
                     $source->getDestination()->getTableName(),
                 ));
             }
-            $loadTask = new CreateAndLoadTableTask($source->getDestination(), $loadOptions, true);
+            // The table is created by the load job itself (Client::queueTableCreate), there is no create
+            // payload the descriptions could be embedded into - they have to be stored after the load.
+            return new LoadTableTaskResult(
+                new CreateAndLoadTableTask($source->getDestination(), $loadOptions, true),
+                $descriptions !== null && !$descriptions->isEmpty() ? $descriptions : null,
+            );
         }
-        return $loadTask;
+    }
+
+    private function createTableDefinition(
+        MappingFromProcessedConfiguration $source,
+        TableDefinitionInterface $tableDefinition,
+        ?TableDescription $descriptions,
+    ): void {
+        // The table is brand new, therefore its description is always system-managed (Storage default) and
+        // there is nothing to diff against.
+        if ($descriptions !== null && !$descriptions->isEmpty()) {
+            $tableDefinition->setDescriptions($descriptions, $this->logger);
+        }
+
+        $this->tableCreator->createTableDefinition(
+            $source->getDestination()->getBucketId(),
+            $tableDefinition,
+        );
     }
 
     public function buildLoadOptions(

libs/output-mapping/src/LoadTableTaskResult.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Keboola\OutputMapping;
+
+use Keboola\OutputMapping\DeferredTasks\LoadTableTaskInterface;
+use Keboola\OutputMapping\Storage\TableDescription;
+
+/**
+ * Outcome of LoadTableTaskCreator::create() - the load task itself plus the descriptions which the creator
+ * could not hand over to Storage yet.
+ */
+class LoadTableTaskResult
+{
+    /**
+     * @param TableDescription|null $descriptionsNotEmbeddedInCreatePayload descriptions which could not be
+     *     part of a create-table-definition payload, because the table is created by the load job itself;
+     *     null when there is nothing left to do - the descriptions were embedded in the create payload, they
+     *     were already applied to a table which existed before, or there is no description at all
+     */
+    public function __construct(
+        private readonly LoadTableTaskInterface $loadTableTask,
+        private readonly ?TableDescription $descriptionsNotEmbeddedInCreatePayload = null,
+    ) {
+    }
+
+    public function getLoadTableTask(): LoadTableTaskInterface
+    {
+        return $this->loadTableTask;
+    }
+
+    public function getDescriptionsNotEmbeddedInCreatePayload(): ?TableDescription
+    {
+        return $this->descriptionsNotEmbeddedInCreatePayload;
+    }
+}

libs/output-mapping/src/Mapping/MappingFromConfigurationSchemaColumn.php

@@ -4,6 +4,8 @@
 
 namespace Keboola\OutputMapping\Mapping;
 
+use Keboola\OutputMapping\Writer\Helper\DescriptionHelper;
+
 class MappingFromConfigurationSchemaColumn
 {
     public function __construct(private readonly array $mapping)
@@ -43,12 +45,32 @@ public function hasMetadata(): bool
         return !empty($this->getMetadata());
     }
 
+    /**
+     * Column metadata without the description, which is stored in the native Storage description field
+     * instead - see getDescription().
+     */
     public function getMetadata(): array
     {
-        $metadata = $this->mapping['metadata'] ?? [];
+        return DescriptionHelper::removeDescriptionFromMetadataMap(
+            $this->mapping['metadata'] ?? [],
+            'schema.metadata',
+        );
+    }
+
+    /**
+     * Description of the column, stored in the native Storage description field. The configuration allows only
+     * one of the two sources to be used at a time. An empty description is treated as no description, so that
+     * it never clears a description stored in Storage.
+     */
+    public function getDescription(): ?string
+    {
         if (isset($this->mapping['description'])) {
-            $metadata['KBC.description'] = $this->mapping['description'];
+            return DescriptionHelper::normalizeDescription($this->mapping['description']);
         }
-        return $metadata;
+
+        return DescriptionHelper::getDescriptionFromMetadataMap(
+            $this->mapping['metadata'] ?? [],
+            'schema.metadata',
+        );
     }
 }