fix: stop the Leantime sync halting silently on a bad row

CHANGELOG.md

@@ -8,6 +8,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+* [PR-325](https://github.com/itk-dev/economics/pull/325)
+  * Fixed the Leantime sync halting silently on a single bad row: `LeantimeApiService` and the sync message
+    handlers now catch `\Throwable`, and the upsert dispatch moved inside the same `try`. A skipped row logs
+    `Skipping <class> id <id>: <reason>` and the sync continues.
+  * Made the Leantime result mappers null-safe ahead of
+    [data-api#18](https://github.com/ITK-Leantime/data-api/pull/18): a deleted user is attributed to
+    `deleted-user-<userId>`, a missing name becomes `(no name)`, and rows with no `ticketId`/`projectId` are skipped.
+  * Fixed the `/deleted` request sending its timestamp as `deletedAfter` rather than `deleted`, which made every
+    delete-sync pull the entire unpaginated deletion history. Deletion entries with no id are now skipped and logged.
+  * Added `LeantimeApiServiceTest::testUpdateWithNullValues()`, covering the nullable payload plus probes that a
+    single unmappable row is logged and skipped rather than stopping the sync.
 * [PR-324](https://github.com/itk-dev/economics/pull/324)
   Added game center with snake
 * [PR-303](https://github.com/itk-dev/economics/pull/303)

Taskfile.yml

@@ -9,7 +9,7 @@ dotenv: [".env.local", ".env"]
 vars:
   # https://taskfile.dev/reference/templating/
   BASE_URL: "{{.TASK_BASE_URL | default .COMPOSE_SERVER_DOMAIN | default .COMPOSE_DOMAIN }}"
-  DOCKER_COMPOSE: '{{ .TASK_DOCKER_COMPOSE | default "itkdev-docker-compose" }}'
+  DOCKER_COMPOSE: '{{ .TASK_DOCKER_COMPOSE | default "docker compose" }}'
 
 tasks:
   default:
@@ -147,7 +147,7 @@ tasks:
     prompt: "This will reset fixture data. Continue?"
     desc: Load data fixtures.
     cmds:
-      - task composer -- fixtures:load
+      - task phpfpm -- bin/console doctrine:fixtures:load --no-interaction
 
   # ----------------------------------------------------------- Messenger ---
 

phpstan-baseline.neon

@@ -2056,7 +2056,7 @@ parameters:
 		-
 			message: '#^Access to an undefined property object\:\:\$name\.$#'
 			identifier: property.notFound
-			count: 4
+			count: 1
 			path: src/Service/LeantimeApiService.php
 
 		-
@@ -2113,12 +2113,6 @@ parameters:
 			count: 1
 			path: src/Service/LeantimeApiService.php
 
-		-
-			message: '#^Access to an undefined property object\:\:\$username\.$#'
-			identifier: property.notFound
-			count: 1
-			path: src/Service/LeantimeApiService.php
-
 		-
 			message: '#^Access to an undefined property object\:\:\$workDate\.$#'
 			identifier: property.notFound
@@ -2932,25 +2926,25 @@ parameters:
 		-
 			message: '#^Call to an undefined method object\:\:findAll\(\)\.$#'
 			identifier: method.notFound
-			count: 24
+			count: 32
 			path: tests/Integration/Service/LeantimeApiServiceTest.php
 
 		-
 			message: '#^Call to an undefined method object\:\:findOneBy\(\)\.$#'
 			identifier: method.notFound
-			count: 8
+			count: 17
 			path: tests/Integration/Service/LeantimeApiServiceTest.php
 
 		-
 			message: '#^Call to an undefined method object\:\:flush\(\)\.$#'
 			identifier: method.notFound
-			count: 3
+			count: 4
 			path: tests/Integration/Service/LeantimeApiServiceTest.php
 
 		-
 			message: '#^Call to an undefined method object\:\:persist\(\)\.$#'
 			identifier: method.notFound
-			count: 12
+			count: 13
 			path: tests/Integration/Service/LeantimeApiServiceTest.php
 
 		-
@@ -3028,31 +3022,31 @@ parameters:
 		-
 			message: '#^Parameter \#2 \$messageBus of class App\\Service\\LeantimeApiService constructor expects Symfony\\Component\\Messenger\\MessageBusInterface, object given\.$#'
 			identifier: argument.type
-			count: 2
+			count: 3
 			path: tests/Integration/Service/LeantimeApiServiceTest.php
 
 		-
 			message: '#^Parameter \#3 \$dataProviderRepository of class App\\Service\\LeantimeApiService constructor expects App\\Repository\\DataProviderRepository, object given\.$#'
 			identifier: argument.type
-			count: 2
+			count: 3
 			path: tests/Integration/Service/LeantimeApiServiceTest.php
 
 		-
 			message: '#^Parameter \#4 \$dataProviderId of method App\\Service\\LeantimeApiService\:\:updateAsJob\(\) expects int, int\|null given\.$#'
 			identifier: argument.type
-			count: 8
+			count: 12
 			path: tests/Integration/Service/LeantimeApiServiceTest.php
 
 		-
 			message: '#^Parameter \#4 \$entityManager of class App\\Service\\LeantimeApiService constructor expects Doctrine\\ORM\\EntityManagerInterface, object given\.$#'
 			identifier: argument.type
-			count: 2
+			count: 3
 			path: tests/Integration/Service/LeantimeApiServiceTest.php
 
 		-
 			message: '#^Parameter \#5 \$projectRepository of class App\\Service\\LeantimeApiService constructor expects App\\Repository\\ProjectRepository, object given\.$#'
 			identifier: argument.type
-			count: 2
+			count: 3
 			path: tests/Integration/Service/LeantimeApiServiceTest.php
 
 		-

src/MessageHandler/EntityRemovedFromDataProviderHandler.php

@@ -34,7 +34,7 @@ public function __invoke(EntityRemovedFromDataProviderMessage $message): void
                 Worklog::class => $this->dataProviderService->worklogRemovedFromDataProvider($message->dataProviderId, (int) $message->projectTrackerId, $message->deletedDate),
                 default => throw new NotSupportedException('classname not supported'),
             };
-        } catch (\Exception $e) {
+        } catch (\Throwable $e) {
             $this->logger->error($e->getMessage());
             throw new UnrecoverableMessageHandlingException($e->getMessage());
         }

src/MessageHandler/LeantimeDeleteHandler.php

@@ -27,7 +27,7 @@ public function __invoke(LeantimeDeleteMessage $message): void
                 $message->asyncJobQueue,
                 $message->deletedAfter,
             );
-        } catch (\Exception $e) {
+        } catch (\Throwable $e) {
             $this->logger->error($e->getMessage());
             throw new UnrecoverableMessageHandlingException($e->getMessage());
         }

src/MessageHandler/LeantimeUpdateHandler.php

@@ -32,7 +32,7 @@ public function __invoke(LeantimeUpdateMessage $message): void
                 $message->modifiedAfter,
                 $message->disableModifiedAtCheck,
             );
-        } catch (\Exception $e) {
+        } catch (\Throwable $e) {
             $this->logger->error($e->getMessage());
             throw new UnrecoverableMessageHandlingException($e->getMessage());
         }

src/MessageHandler/UpsertIssueHandler.php

@@ -22,7 +22,7 @@ public function __invoke(UpsertIssueMessage $message): void
         try {
             $this->logger->info('Upserting issue: '.$message->issueData->name);
             $this->dataProviderService->upsertIssue($message->issueData);
-        } catch (\Exception $e) {
+        } catch (\Throwable $e) {
             $this->logger->error($e->getMessage());
             throw new UnrecoverableMessageHandlingException($e->getMessage());
         }

src/MessageHandler/UpsertProjectHandler.php

@@ -22,7 +22,7 @@ public function __invoke(UpsertProjectMessage $message): void
         try {
             $this->logger->info('Upserting project: '.$message->projectData->name);
             $this->dataProviderService->upsertProject($message->projectData);
-        } catch (\Exception $e) {
+        } catch (\Throwable $e) {
             $this->logger->error($e->getMessage());
             throw new UnrecoverableMessageHandlingException($e->getMessage());
         }

src/MessageHandler/UpsertVersionHandler.php

@@ -22,7 +22,7 @@ public function __invoke(UpsertVersionMessage $message): void
         try {
             $this->logger->info('Upserting version: '.$message->versionData->name);
             $this->dataProviderService->upsertVersion($message->versionData);
-        } catch (\Exception $e) {
+        } catch (\Throwable $e) {
             $this->logger->error($e->getMessage());
             throw new UnrecoverableMessageHandlingException($e->getMessage());
         }

src/MessageHandler/UpsertWorkerHandler.php

@@ -22,7 +22,7 @@ public function __invoke(UpsertWorkerMessage $message): void
         try {
             $this->logger->info('Upserting worker: '.$message->workerData->email);
             $this->dataProviderService->upsertWorker($message->workerData);
-        } catch (\Exception $e) {
+        } catch (\Throwable $e) {
             $this->logger->error($e->getMessage());
             throw new UnrecoverableMessageHandlingException($e->getMessage());
         }

src/MessageHandler/UpsertWorklogHandler.php

@@ -22,7 +22,7 @@ public function __invoke(UpsertWorklogMessage $message): void
         try {
             $this->logger->info('Upserting worklog: '.$message->worklogData->projectTrackerId);
             $this->dataProviderService->upsertWorklog($message->worklogData);
-        } catch (\Exception $e) {
+        } catch (\Throwable $e) {
             $this->logger->error($e->getMessage());
             throw new UnrecoverableMessageHandlingException($e->getMessage());
         }

src/Model/DataProvider/DataProviderIssueData.php

@@ -12,8 +12,8 @@ public function __construct(
         public string $projectTrackerProjectId,
         public string $name,
         public array $epics,
-        public float $plannedHours,
-        public float $remainingHours,
+        public ?float $plannedHours,
+        public ?float $remainingHours,
         public ?string $worker,
         public IssueStatusEnum $status,
         public ?\DateTimeInterface $dueDate,

src/Model/DataProvider/DataProviderWorklogData.php

@@ -12,7 +12,7 @@ public function __construct(
         public \DateTimeInterface $startedDate,
         public string $username,
         public float $hours,
-        public string $kind,
+        public ?string $kind,
         public ?\DateTimeInterface $fetchTime,
         public ?\DateTimeInterface $sourceModifiedDate,
         public bool $disableModifiedAtCheck = false,

src/Service/DataProviderService.php

@@ -251,7 +251,7 @@ public function upsertWorklog(DataProviderWorklogData $upsertWorklogData): void
         $worklog->setStarted($upsertWorklogData->startedDate);
         $worklog->setProjectTrackerIssueId($upsertWorklogData->projectTrackerIssueId);
         $worklog->setTimeSpentSeconds($upsertWorklogData->hours * $this::SECONDS_IN_HOUR);
-        $worklog->setKind(BillableKindsEnum::tryFrom($upsertWorklogData->kind));
+        $worklog->setKind(null !== $upsertWorklogData->kind ? BillableKindsEnum::tryFrom($upsertWorklogData->kind) : null);
         $worklog->setProject($issue->getProject());
         $worklog->setIssue($issue);
         $worklog->setFetchDate($upsertWorklogData->fetchTime);

src/Service/LeantimeApiService.php

@@ -43,6 +43,9 @@ class LeantimeApiService implements DataProviderInterface
     public const TIMESHEETS = 'timesheets';
     public const WORKERS = 'workers';
     private const LIMIT = 100;
+    // Placeholder for a null name; the name columns are not nullable, and dropping the row would
+    // lose real data — for issues it would make their worklogs unstorable.
+    private const NAME_MISSING = '(no name)';
     private const QUEUE_ASYNC = 'async';
     private const QUEUE_SYNC = 'sync';
 
@@ -116,7 +119,9 @@ public function deleteAsJob(int $dataProviderId, bool $asyncJobQueue = false, ?\
 
         $params = [
             'types' => $types,
-            'deletedAfter' => $deletedAfter?->getTimestamp(),
+            // The plugin reads 'deleted'; anything else is discarded and the whole deletion
+            // history is returned, which /deleted does not paginate.
+            'deleted' => $deletedAfter?->getTimestamp(),
         ];
 
         // Get data from Leantime.
@@ -137,6 +142,13 @@ public function deleteAsJob(int $dataProviderId, bool $asyncJobQueue = false, ?\
             };
 
             foreach ($results->{$type} as $result) {
+                // Nothing identifies the entity to remove, and this loop has no other guard.
+                if (null === $result->id) {
+                    $this->logger->warning(sprintf('Skipping deleted %s entry with no id', $type));
+
+                    continue;
+                }
+
                 $projectTrackerId = $result->id;
                 $deletedDate = $this->getLeanDateTime($result->deletedDate);
 
@@ -208,6 +220,11 @@ public function updateAsJob(string $className, int $startId, int $limit, int $da
 
     private function dispatchUpsertMessage(string $className, object $data, int $dataProviderId, \DateTimeInterface $fetchDate, bool $asyncJobQueue = false, ?string $dataProviderUrl = null, bool $disableModifiedAtCheck = false): void
     {
+        // Catch \Throwable, not \Exception: a nullable source field mapped onto a non-nullable
+        // constructor argument raises a TypeError, which extends Error. Uncaught, it escapes the
+        // row loop in updateAsJob() before the next page is queued, halting the sync silently.
+        // The dispatch belongs inside the try for the same reason: on the sync transport the
+        // handler runs inline here, so its failures surface as part of this call.
         try {
             $message = match ($className) {
                 Project::class => new UpsertProjectMessage($this->getProjectUpsertFromResult($data, $dataProviderId, $fetchDate, $dataProviderUrl, $disableModifiedAtCheck)),
@@ -217,17 +234,15 @@ private function dispatchUpsertMessage(string $className, object $data, int $dat
                 Worker::class => new UpsertWorkerMessage($this->getWorkerUpsertFromResult($data, $dataProviderId, $fetchDate)),
                 default => null,
             };
-        } catch (\Exception $e) {
-            $this->logger->error($e->getMessage());
 
-            return;
-        }
-
-        if (null !== $message) {
-            $this->messageBus->dispatch(
-                $message,
-                [new TransportNamesStamp($asyncJobQueue ? $this::QUEUE_ASYNC : $this::QUEUE_SYNC)],
-            );
+            if (null !== $message) {
+                $this->messageBus->dispatch(
+                    $message,
+                    [new TransportNamesStamp($asyncJobQueue ? $this::QUEUE_ASYNC : $this::QUEUE_SYNC)],
+                );
+            }
+        } catch (\Throwable $e) {
+            $this->logger->error(sprintf('Skipping %s id %s: %s', $className, $data->id ?? '?', $e->getMessage()));
         }
     }
 
@@ -237,7 +252,7 @@ private function getProjectUpsertFromResult(object $result, int $dataProviderId,
 
         return new DataProviderProjectData(
             $dataProviderId,
-            $result->name,
+            $result->name ?? self::NAME_MISSING,
             $projectTrackerId,
             $this->linkToProject($projectTrackerId, $dataProviderUrl),
             $fetchDate,
@@ -248,9 +263,14 @@ private function getProjectUpsertFromResult(object $result, int $dataProviderId,
 
     private function getVersionUpsertFromResult(object $result, int $dataProviderId, \DateTimeInterface $fetchDate, bool $disableModifiedAtCheck = false): DataProviderVersionData
     {
+        // A version cannot exist without a project; Version::$project is not nullable.
+        if (null === $result->projectId) {
+            throw new NotAcceptableException('Version upsert not acceptable: projectId is null');
+        }
+
         return new DataProviderVersionData(
             $dataProviderId,
-            $result->name,
+            $result->name ?? self::NAME_MISSING,
             (string) $result->id,
             (string) $result->projectId,
             $fetchDate,
@@ -267,7 +287,7 @@ private function getIssueUpsertFromResult(object $result, int $dataProviderId, \
             $projectTrackerId,
             $dataProviderId,
             (string) $result->projectId,
-            $result->name,
+            $result->name ?? self::NAME_MISSING,
             $result->tags,
             $result->plannedHours,
             $result->remainingHours,
@@ -291,13 +311,20 @@ private function getWorklogUpsertFromResult(object $result, int $dataProviderId,
             throw new NotAcceptableException('Worklog upsert not acceptable: startedDate is null');
         }
 
+        // A worklog cannot exist without an issue; Worklog::$issue is not nullable.
+        if (null === $result->ticketId) {
+            throw new NotAcceptableException('Worklog upsert not acceptable: ticketId is null');
+        }
+
         return new DataProviderWorklogData(
             $result->id,
             $dataProviderId,
             (string) $result->ticketId,
             $result->description,
             $startedDate,
-            $result->username,
+            // A null username means the join found no user row, as Leantime never stores a null
+            // one. The hours are still real, so keep the worklog and name the departed user.
+            $result->username ?? 'deleted-user-'.($result->userId ?? 'unknown'),
             $result->hours,
             $result->kind,
             $fetchDate,
@@ -343,7 +370,7 @@ private function getLeanDateTime(?string $dateString): ?\DateTimeInterface
         return new \DateTime($dateString, new \DateTimeZone('UTC'));
     }
 
-    private function convertStatusToEnum(string $statusString): IssueStatusEnum
+    private function convertStatusToEnum(?string $statusString): IssueStatusEnum
     {
         return match ($statusString) {
             'NEW' => IssueStatusEnum::NEW,