Server: Resolves #9931: Add task to delete events older than a week

[AdrienPoupa]

Revisiting https://github.com/laurent22/joplin/pull/9988/files as I had the same issue this week (my Joplin DB grew to 600mb while I only have a few notes...).

I'm using mostly the same code, but I tried to match other patterns found in the codebase (using a TTL attribute in the class, not requiring arguments in the event function).

I also handled Laurent's review to use fake timers for the test.

[github-actions]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[AdrienPoupa]

I have read the CLA Document and I hereby sign the CLA

[laurent22]

Thanks for looking into this. I think there's two issues:

• The TTL should be configurable via an env variable
• What's the impact of deleting old events - what service or function could be affected by it?

[laurent22]

7 * Day is clearer (import the Day constant)

[AdrienPoupa]

I was using TokenModel as a reference that has the exact same line. I changed it there as well.

[laurent22]

If possible please also add another for:

• When there's 0 event in the database
• When the TTL means no event would be deleted

[AdrienPoupa]

Sure, done.

[AdrienPoupa]

Hi Laurent, thanks for the review.

The TTL should be configurable via an env variable

What would we gain from this? The DeleteExpiredTokens task that I'm using as a reference does not have this logic.

If we need to implement it, do you have a preferred approach? I see the UserDeletionService uses the USER_DATA_AUTO_DELETE_AFTER_DAYS environment variable, I could use the same mechanism.

What's the impact of deleting old events - what service or function could be affected by it?

I don't know, sorry. It is the approach you suggested here however: #9931 (comment)

[laurent22]

What would we gain from this?

For audit purposes for example.

I don't know, sorry. It is the approach you suggested here however: #9931 (comment)

If you can check please post here

[AdrienPoupa]

I analyzed events and their usages, I found that we're creating events in TaskService's runTask just before and after the task is handled:

await this.models.event().create(EventType.TaskStarted, id.toString());

and

await this.models.event().create(EventType.TaskCompleted, id.toString());

In taskLastEvents, we retrieve the last events for a given task:

	public async taskLastEvents(id: TaskId): Promise<TaskEvents> {
		return {
			taskStarted: await this.models.event().lastEventByTypeAndName(EventType.TaskStarted, id.toString()),
			taskCompleted: await this.models.event().lastEventByTypeAndName(EventType.TaskCompleted, id.toString()),
		};
	}

This function, in turn, is used in the admin/tasks route to show task events (columns Last Run and Last Completion).

Those were the only usages I could find, so they're here for auditing/debugging purposes, they have no impact on the behaviour of the application.

[AdrienPoupa]

I added two environment variables to enable the deletion and control the cut off period, and improved the test coverage. Hopefully that should do it :)