Skip to content
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.

Sign up for GitHub

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

Jump to bottom

reduce frequency of lastoriginupdate db writes #702

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft

reduce frequency of lastoriginupdate db writes #702

Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
8ffe49d
do not update last origin update if within 1 hour of last update
e-five256 Apr 11, 2024
a1e7a94
reduce throttle from one hour to one minute
e-five256 Apr 12, 2024
822b109
Merge branch 'main' into e5/lastoriginupdate-less
e-five256 Apr 12, 2024
98c998e
Merge remote-tracking branch 'origin' into e5/lastoriginupdate-less
e-five256 Apr 13, 2024
8bd3ef2
change to update last origin on create payloads
e-five256 Apr 13, 2024
94802b7
Merge remote-tracking branch 'origin' into e5/lastoriginupdate-less
e-five256 Apr 19, 2024
6224ea1
also update on announced creates
e-five256 Apr 20, 2024
5c83b29
use actor from top level ap payload
e-five256 Apr 20, 2024
dcdd3de
fix bad copy paste
e-five256 Apr 20, 2024
3ecaaaf
Merge branch 'main' into e5/lastoriginupdate-less
e-five256 Apr 20, 2024
e6e9137
Merge branch 'main' into e5/lastoriginupdate-less
e-five256 Apr 20, 2024
e04204f
Merge branch 'main' into e5/lastoriginupdate-less
e-five256 Apr 29, 2024
cdbc05a
Merge branch 'main' into e5/lastoriginupdate-less
nobodyatroot May 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 7 additions & 6 deletions src/MessageHandler/ActivityPub/Inbox/ActivityHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,6 @@ private function handle(?array $payload)
}

if ('Announce' === $payload['type']) {
$actor = $this->manager->findActorOrCreate($payload['actor']);
if ($actor instanceof Magazine) {
$actor->lastOriginUpdate = new \DateTime();
$this->entityManager->persist($actor);
$this->entityManager->flush();
}
// we check for an array here, because boosts are announces with an url (string) as the object
if (\is_array($payload['object'])) {
$payload = $payload['object'];
Expand All @@ -113,6 +107,13 @@ private function handle(?array $payload)
}
}
}
} elseif ('Create' === $payload['type']) {
$actor = $this->manager->findActorOrCreate($payload['actor']);
if ($actor instanceof Magazine && $actor->lastOriginUpdate < (new \DateTime())->modify('-3 hours')) {
BentiGorlich marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

This will not work, because the actor of a Create activity is never a magazine, but always a user. A magazine only announces what a user creates. If you get a plain Create activity you would have to check from which instance the user is from and then get the magazine the activity is posted to and then you can adjust the origin update.
However I think I removed that part (I initially had something that did the same) because you cannot be sure that you get create activities because you are subscribed to a magazine, because you will get create activities from users you are subscribed to, regardless of whether you're subscribed to the magazine they post it to. Announce activities on the other hand are only sent to people subscribed to magazines

$actor->lastOriginUpdate = new \DateTime();
$this->entityManager->persist($actor);
$this->entityManager->flush();
}
Copy link
Member

Choose a reason for hiding this comment

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

What actually needs checking is the announced type, so payloads like this will trigger the code:

{
  "type": "Announce",
  "object": {
    "type": "Create",
    ...
  },
  ...
}

So I'd say move it back to where it was before and change the condition to:

if ('Announce' === $payload['type']) {
    if (\is_array($payload['object'])) {
        if ('Create' === $payload['object']['type']) {

Copy link
Member Author

Choose a reason for hiding this comment

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

I would've thought that means it'd only update on comments that come in from external instances, but maybe I'm misunderstanding AP here. I assumed create = comes from owner instance, announce = comes from another instance, and owner is announcing it happened. Or maybe owner instances both create and announce create their own posts?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah a create from a magazine and an announced create from a magazine should refresh the field. Until now the plain create did not do that, but it should, you're right

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, that might actually explain some things I had seen. Looking at this one magazine, it says "This magazine is not receiving updates (last activity 29 day(s) ago). " but has posts from today, yesterday etc, continuing to get posts, but they're all posts from that instance the magazine is from

So if I understand correctly, I need both sections, what I have here which updates from the origin's creates, and then what you suggested which updates when the origin announces creates as well

Copy link
Member

Choose a reason for hiding this comment

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

Well you alsp just get the creates from the user directly which would not count towards origin uodates.

And yes both sections would be good indeed

Copy link
Member

Choose a reason for hiding this comment

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

In this conversation I forgot the point I made in my latest comment. Sorry for the confusion on my part

}

$this->logger->debug('Got activity message of type {type}: {message}', ['type' => $payload['type'], 'message' => json_encode($payload)]);
Expand Down