Introduce the activity table

[BentiGorlich]

• [x] Save activities in the database so that they can be called via URL
• [x] Add unit tests for the JSON used in outward federation
• [ ] Add functional tests for inbound federation
  • [ ] tests with our own JSON
  • [ ] tests with lemmy's JSON

Closes #772

[melroy89]

I think @BentiGorlich can better solve these conflicts, since he knows this code changes better. I will not touch this.

[github-actions[bot]]

This PR is stale because it has been open 40 days with no activity.

[BentiGorlich]

I am still working on this, so do not think that this is not going anywhere... The tests are getting there, but a lot of work is still todo :)

[BentiGorlich]

Left ToDos:

• add testing for undoing incoming dislikes
• add announcing of incoming reports to local magazines to all remote instances with moderators (although that might deserve it own issue and PR, what do you think @melroy89 ?)

[melroy89]

• add announcing of incoming reports to local magazines to all remote instances with moderators (although that might deserve it own issue and PR, what do you think @melroy89 ?)

That sounds indeed something for a separate PR. Especially if you want to limit the scope a bit of this PR in order to get it merged faster.

Personally I'm always in favor of smaller PRs, merging more, but smaller amount of changes (if possible). Like small iterations. Which also has the benefit from retrieving feedback earlier and more often, as well as leaner to adapt to new requests or changes in requirements. But I digress :D

[BentiGorlich]

Alright, then we'll do that in a different PR and I will make an issue about it

[BentiGorlich]

@melroy89 one maybe open todo could be to make the new tests thread safe, but I was really trying and did the setup the way I though should work with paratest, but I really really struggled and gave up in the end. Problem is that the CI duration is now up to 20 minutes again (from the usual 13 minutes). I don't know how much you know about parallelization, but maybe it would help if you take a look at it. Btw. the changes to the settings repo for example were only done to fix a parallelization issue

Time savings on my machine (even though paratest threw errors): 2m 8s -> 1m 1s (only 4 threads like GH Actions use)

[BentiGorlich]

And as a hint I get these type of errors when running the new AP tests with paratest:

1. App\Tests\Functional\ActivityPub\Inbox\FlagHandlerTest::testFlagRemoteEntryCommentInRemoteMagazine Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException: An exception occurred while executing a query: SQLSTATE[23503]: Foreign key violation: 7 ERROR: insert or update on table "report" violates foreign key constraint "fk_c42f778427ee0e60" DETAIL: Key (reporting_id)=(92) is not present in table "user".
2. App\Tests\Functional\ActivityPub\Inbox\DislikeHandlerTest::testUndoDislikeRemoteEntryInRemoteMagazine Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException: An exception occurred while executing a query: SQLSTATE[23503]: Foreign key violation: 7 ERROR: insert or update on table "moderator" violates foreign key constraint "fk_6a30b268a76ed395" DETAIL: Key (user_id)=(56) is not present in table "user".

My guess is that somewhere there could happen an implicit COMMIT call which breaks the transaction isolation of the tests, but I have no idea where this could happen

tested with:

php vendor/bin/paratest tests/Functional/ --group ActivityPub -p 4

[melroy89]

Are you aware of the setup function is not behaving the same as in phpunit due to the nature of paratest, the setup will be called multiple times for each process started by paratest. You could workaround this behavior, see: https://github.com/paratestphp/paratest?tab=readme-ov-file#initial-setup-for-all-tests

I'm not saying this is the root cause though, but important to know.

[BentiGorlich]

Yeah I know about that. The way I did the setUp methods everywhere should not be a problem I think...

[BentiGorlich]

I have finally marked this as ready for review. I have it running on gehirneimer.de It is of course a whole tone of code that is new. A lot of it are new snapshot tests, showing almost all of our federation jsons.

This issue is blocking basically all ActivityPub modifications, and it fixes some very important issues.

[melroy89]

• [x] Save activities in the database so that they can be called via URL

  • [x] Add unit tests for the JSON used in outward federation

  • [ ] Add functional tests for inbound federation

    • [x] tests with our own JSON
    • [ ] tests with lemmy's JSON

Closes #772

Where the lemmy tests also done?

[BentiGorlich]

Nope I didn't add any lemmy testing to the suite. But the mbin tests are a start and this PR is big enough and took long enough as it is...

[melroy89]

No doubt indeed, this pr is big enough.