Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

* [PR-334](https://github.com/itk-dev/economics/pull/334)
* Paginated the Leantime delete sync, following [data-api#21](https://github.com/ITK-Leantime/data-api/pull/21):
`/deleted` now serves one type per request with `start`/`limit`, so `delete()` queues a message per type and
`deleteAsJob()` pages through them the way `updateAsJob()` already does. The whole deletion history no longer
has to arrive in a single response — which is what the 300s `max_duration` in `config/packages/framework.yaml`
was sized for, though it stays as it is for the entity endpoints.
* The delete request now sends its timestamp as `deletedAfter`, the endpoint's new name for it, matching
`modifiedAfter` on the entity endpoints. The old `deleted` answers 400 rather than being ignored, so the key
cannot go missing unnoticed again.
* The delete cursor is the endpoint's new `deletionId`, not the deleted entity's `id`: deletions are ordered by
when they happened. It advances past a deletion that names no entity, since a skipped row still has to be paged
past, and a full page with no usable `deletionId` stops with an error rather than re-queueing itself.
* [PR-326](https://github.com/itk-dev/economics/pull/326)
* Stopped the pagination cursor in `updateAsJob()` looping on a page it cannot advance past. Skipping null ids
left the cursor where it started, so a full page of them re-queued the same page forever and starved the
Expand Down
6 changes: 3 additions & 3 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -2085,7 +2085,7 @@ parameters:
-
message: '#^Access to an undefined property object\:\:\$resultsCount\.$#'
identifier: property.notFound
count: 1
count: 2
path: src/Service/LeantimeApiService.php

-
Expand Down Expand Up @@ -2121,7 +2121,7 @@ parameters:
-
message: '#^Match expression does not handle remaining value\: string$#'
identifier: match.unhandled
count: 1
count: 2
path: src/Service/LeantimeApiService.php

-
Expand Down Expand Up @@ -2965,7 +2965,7 @@ parameters:
path: tests/Integration/Service/LeantimeApiServiceTest.php

-
message: '#^Parameter \#1 \$dataProviderId of method App\\Service\\LeantimeApiService\:\:deleteAsJob\(\) expects int, int\|null given\.$#'
message: '#^Parameter \#4 \$dataProviderId of method App\\Service\\LeantimeApiService\:\:deleteAsJob\(\) expects int, int\|null given\.$#'
identifier: argument.type
count: 1
path: tests/Integration/Service/LeantimeApiServiceTest.php
Expand Down
3 changes: 3 additions & 0 deletions src/Message/LeantimeDeleteMessage.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
readonly class LeantimeDeleteMessage
{
public function __construct(
public string $type,
public int $start,
public int $limit,
public int $dataProviderId,
public bool $asyncJobQueue,
public ?\DateTimeInterface $deletedAfter,
Expand Down
10 changes: 9 additions & 1 deletion src/MessageHandler/LeantimeDeleteHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,17 @@ public function __construct(
public function __invoke(LeantimeDeleteMessage $message): void
{
try {
$this->logger->info('Handling delete message. deletedAfter: '.$message->deletedAfter?->format('c'));
$this->logger->info(sprintf(
'Handling delete message. type: %s, start: %d, deletedAfter: %s',
$message->type,
$message->start,
$message->deletedAfter?->format('c') ?? 'none',
));

$this->leantimeApiService->deleteAsJob(
$message->type,
$message->start,
$message->limit,
$message->dataProviderId,
$message->asyncJobQueue,
$message->deletedAfter,
Expand Down
115 changes: 68 additions & 47 deletions src/Service/LeantimeApiService.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ class LeantimeApiService implements DataProviderInterface
public const TICKETS = 'tickets';
public const TIMESHEETS = 'timesheets';
public const WORKERS = 'workers';
/** The types the deleted endpoint tracks, children before the parents they hang off. */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can not be guaranteed when running async. Depending on the entity model consider using doctrines orphan removal to also delete children when parents are deleted.

Or scope deletes by projects so that all deletes within a project happens as one message/job.

@tuj tuj Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right that the queue gives no ordering guarantee, and the comment was wrong to imply the
constant provides one. What actually holds the order here is that this path is not async:
SyncDeletedCommand calls deleteAll(false, …), so both the delete messages and the
EntityRemovedFromDataProviderMessages get TransportNamesStamp('sync') and every handler runs
inline — including the next-page dispatch, so a type's pages all finish before the next type
starts. I've rewritten the comment to say that, and to record that fanning the four types out up
front only works under inline handling: switching this path to async would mean chaining the types
instead.

On orphan removal — it would take billing data with it. projectRemovedFromDataProvider() and
issueRemovedFromDataProvider() refuse to hard-delete while invoice-bound children exist and set
sourceDeletedDate instead, and worklogRemovedFromDataProvider() protects any worklog attached
to an invoiceEntry. A cascade from the parent would delete exactly the rows those checks exist to
keep.

On scoping by project — /deleted pages by type over deletionId and has no project dimension, so
grouping by project would mean holding the whole deletion history in memory, which is what the
pagination is here to avoid.

Digging into this did turn up a real bug, unrelated to pagination:
projectRemovedFromDataProvider() checks invoices, issues and worklogs but not versions, and
version.project_id has no ON DELETE, so a project whose milestone deletion was missed passes the
removable check and then hits an FK violation. Fixing that separately.

private const DELETED_TYPES = [self::TIMESHEETS, self::TICKETS, self::MILESTONES, self::PROJECTS];
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. The tracker id is
Expand Down Expand Up @@ -99,80 +101,99 @@ public function delete(bool $asyncJobQueue = false, ?\DateTimeInterface $deleted
$dataProviders = $this->getEnabledLeantimeDataProviders();

foreach ($dataProviders as $dataProvider) {
$this->messageBus->dispatch(
new LeantimeDeleteMessage($dataProvider->getId(), $asyncJobQueue, $deletedAfter),
[new TransportNamesStamp($asyncJobQueue ? $this::QUEUE_ASYNC : $this::QUEUE_SYNC)],
);
// One message per type, since the endpoint answers with a single type's page.
// The order is kept: children are removed before the parents they hang off.
foreach ($this::DELETED_TYPES as $type) {
$this->messageBus->dispatch(
new LeantimeDeleteMessage($type, 0, $this::LIMIT, $dataProvider->getId(), $asyncJobQueue, $deletedAfter),
[new TransportNamesStamp($asyncJobQueue ? $this::QUEUE_ASYNC : $this::QUEUE_SYNC)],
);
}
}
}

public function deleteAsJob(int $dataProviderId, bool $asyncJobQueue = false, ?\DateTimeInterface $deletedAfter = null): void
public function deleteAsJob(string $type, int $startId, int $limit, int $dataProviderId, bool $asyncJobQueue = false, ?\DateTimeInterface $deletedAfter = null): void
{
$dataProvider = $this->dataProviderRepository->find($dataProviderId);

if (null === $dataProvider) {
throw new NotFoundException("DataProvider with id: $dataProviderId not found");
}

$types = [
self::TIMESHEETS,
self::TICKETS,
self::MILESTONES,
self::PROJECTS,
];
$classname = match ($type) {
self::PROJECTS => Project::class,
self::MILESTONES => Version::class,
self::TICKETS => Issue::class,
self::TIMESHEETS => Worklog::class,
};

$params = [
'types' => $types,
// The plugin reads 'deleted'; anything else is discarded and the whole deletion
// history is returned, which /deleted does not paginate.
'deleted' => $deletedAfter?->getTimestamp(),
'type' => $type,
'start' => $startId,
'limit' => $limit,
// The plugin reads 'deletedAfter'; under the old 'deleted' it answers 400, and under
// any other key the timestamp is discarded and every deletion ever recorded is paged
// through.
'deletedAfter' => $deletedAfter?->getTimestamp(),
];

// Get data from Leantime.
$data = $this->fetchFromLeantime($dataProvider, 'deleted', $params);
$results = $data->results;

// Queue delete.
foreach ($types as $type) {
if (!isset($results->{$type})) {
continue;
$maxDeletionId = null;

foreach ($data->results as $result) {
// Tracked before anything below can skip the row: a deletion that cannot be acted
// on still has to be paged past. The cursor is deletionId, not id — the endpoint
// orders deletions by when they happened, not by the entity they refer to.
if (is_numeric($result->deletionId ?? null)) {
$deletionId = (int) $result->deletionId;
$maxDeletionId = null === $maxDeletionId ? $deletionId : max($maxDeletionId, $deletionId);
}

$classname = match ($type) {
self::PROJECTS => Project::class,
self::MILESTONES => Version::class,
self::TICKETS => Issue::class,
self::TIMESHEETS => Worklog::class,
};
// Nothing identifies the entity to remove.
if (null === $result->id) {
$this->logger->error(sprintf('Skipping deleted %s entry with no id', $type));

foreach ($results->{$type} as $result) {
// Nothing identifies the entity to remove.
if (null === $result->id) {
$this->logger->error(sprintf('Skipping deleted %s entry with no id', $type));
continue;
}

continue;
$projectTrackerId = $result->id;

// Now that the request actually filters by timestamp, an entry dropped here is
// dropped for good; before, every run re-read the full history and healed itself.
// So one bad entry must not take the rest of the run with it.
try {
$deletedDate = $this->getLeanDateTime($result->deletedDate);

$this->messageBus->dispatch(
new EntityRemovedFromDataProviderMessage($classname, $dataProviderId, $projectTrackerId, $deletedDate),
[new TransportNamesStamp($asyncJobQueue ? $this::QUEUE_ASYNC : $this::QUEUE_SYNC)],
);
} catch (HandlerFailedException|\DateMalformedStringException|\TypeError $e) {
if ($e instanceof HandlerFailedException) {
$this->rethrowUnlessRowLevel($e);
}

$projectTrackerId = $result->id;

// Now that the request actually filters by timestamp, an entry dropped here is
// dropped for good; before, every run re-read the full history and healed itself.
// So one bad entry must not take the rest of the run with it.
try {
$deletedDate = $this->getLeanDateTime($result->deletedDate);
$this->logger->error(sprintf('Skipping deleted %s id %s: %s', $type, $projectTrackerId, $e->getMessage()));
}
}

$this->messageBus->dispatch(
new EntityRemovedFromDataProviderMessage($classname, $dataProviderId, $projectTrackerId, $deletedDate),
[new TransportNamesStamp($asyncJobQueue ? $this::QUEUE_ASYNC : $this::QUEUE_SYNC)],
);
} catch (HandlerFailedException|\DateMalformedStringException|\TypeError $e) {
if ($e instanceof HandlerFailedException) {
$this->rethrowUnlessRowLevel($e);
}
// Queue next page.
if ($data->resultsCount === $limit) {
// A full page with nothing to advance on cannot be paged past. Stopping is visible in
// the log; continuing would re-read the same page until the queue starves.
if (null === $maxDeletionId || $maxDeletionId < $startId) {
$this->logger->error(sprintf('Stopping deleted %s sync at start %d: no usable deletionId on a full page, cursor cannot advance.', $type, $startId));

$this->logger->error(sprintf('Skipping deleted %s id %s: %s', $type, $projectTrackerId, $e->getMessage()));
}
return;
}

$this->messageBus->dispatch(
new LeantimeDeleteMessage($type, $maxDeletionId + 1, $limit, $dataProviderId, $asyncJobQueue, $deletedAfter),
[new TransportNamesStamp($asyncJobQueue ? $this::QUEUE_ASYNC : $this::QUEUE_SYNC)],
);
}
}

Expand Down
Loading
Loading