-
Notifications
You must be signed in to change notification settings - Fork 30
Introduce the activity table #1173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
7b8a9f2 to
d07c5c3
Compare
c444eeb to
57ec00a
Compare
d07c5c3 to
fa6f302
Compare
cbcd898 to
49bdeaa
Compare
fa6f302 to
09a90b4
Compare
This comment was marked as outdated.
This comment was marked as outdated.
cf72318 to
7cd555b
Compare
49bdeaa to
5c5f342
Compare
ed923ae to
9b44612
Compare
ef0cc27 to
e108c22
Compare
c1aba40 to
eaa3390
Compare
e108c22 to
4d6b1ec
Compare
e05d4f2 to
6bae56c
Compare
4d6b1ec to
0ad5211
Compare
6bae56c to
7d164b1
Compare
0ad5211 to
3ce355a
Compare
7d164b1 to
c6e6807
Compare
3ce355a to
5729ac9
Compare
c6e6807 to
5d0bccd
Compare
5729ac9 to
1bc2da1
Compare
5d0bccd to
07d0603
Compare
1bc2da1 to
ec9516b
Compare
07d0603 to
7f7b1bf
Compare
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 |
|
Alright, then we'll do that in a different PR and I will make an issue about it |
|
@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). Time savings on my machine (even though paratest threw errors): 2m 8s -> 1m 1s (only 4 threads like GH Actions use) |
|
And as a hint I get these type of errors when running the new AP tests with paratest:
My guess is that somewhere there could happen an implicit tested with: php vendor/bin/paratest tests/Functional/ --group ActivityPub -p 4 |
|
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. |
|
Yeah I know about that. The way I did the |
186451e to
27dbc4e
Compare
Whenever an actor makes an activity (like, announce, etc.) we now save it in our db, so the url we pass to other activity pub servers is valid and can return the correct json. All the factory and wrapper classes now return an `Activity` entity that can be converted to json using the new `ActivityJsonBuilder` # Conflicts: # src/MessageHandler/ActivityPub/Inbox/FollowHandler.php # src/MessageHandler/ActivityPub/Outbox/AnnounceLikeHandler.php # src/MessageHandler/ActivityPub/Outbox/FollowHandler.php
- Remove the static nature of the `SettingsManager` and the `MentionManager` - If the environment is not in `test` then the `SettingsManager` will still use a static cache, but all static methods were replace with instance methods - All static methods from the `MentionManager` were replaced with instance methods - Remove some Doctrine cascade removes (removing a message thread with the entity manager resulted in all participating users being purged) - Handle pin announces in the message handler themselves instead of the pin event - pass the incoming activities through to the messages in specific cases - create `Activity` entities for remote payloads under specific circumstances (so we can announce the original payload) - improve webfinger exception message - Add functional ActivityPub testing. This is very complicated and requires a lot of code. Sadly it is also not thread-safe for a reason I could not find out, yet. In every setup method we create actors for different domains, so we can generate some activities, webfinger jsons, and everything we need to mock the activities needed to test the current part of the AP code. Each message handler is tested in its own test class
- Add tests for the `FlagHandler` - Add tests for undoing like (and fix double wrapping the activity)
27dbc4e to
7d73358
Compare
|
I have finally marked this as ready for review. I have it running on gehirneimer.de This issue is blocking basically all ActivityPub modifications, and it fixes some very important issues. |
Where the lemmy tests also done? |
|
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... |
|
No doubt indeed, this pr is big enough. |
Closes #772