Hi,

I found out that, each time a pubsubhubub notification is sent by a hub to a subscriber, the subscriber issues a new subscribe request to the hub.

Why ? If you look into the import function of FeedsSource, you will see that at the end, it calls the save method. This save method calls, for each importer, its sourceSave method. When you're using pubsubhubbub with FeedsHTTPFetcher, it will therefore call the sourceSave method of FeedsHTTPFetcher, which will call the subscribe method to the hub, generating a subscribe request that is not needed.

I'm proposing the attached patch which fixes the bug...

Comments

guillaumev’s picture

StatusFileSize
new3.11 KB

After looking a little bit further, I found another small issue.

When a feed importer receives a content notification, it fetches the new content from the feed for which the notification was made. This is fine, however, Pubsubhubbub allows the hub to send the changed content directly in the notification to the subscriber, which avoids having the subscriber fetch the feed.

The new patch proposed detects if content was sent in the notification and, if content was sent in the notification, it directly parses and processes the content sent, without fetching the feed.

dasjo’s picture

just ran into the same issues for d7
see #1291942: Don't subscribe on push notifications

mgriego’s picture

Noticed this issue on our site. Kept wondering why I was seeing so many subscription requests/responses happening all the time.

I have only briefly looked at the patches here, and I think there are problems with each. I'll have to put together my own patch when I get some time, but here are my thoughts:

In the first patch, there is a problem with the way that ::exists works. In reality, when a PuSH subscription is saved, it's saved based on the return values from the hub. The hub has the option to *change* the topic/URL, so it's possible that looking up based on the URL might fail for legitimate subscriptions. The truth is that having the ->subscribe() call for each save acts as kind of a failsafe here. I think trying to match up based on URL will result in faulty logic *unless* you go the extra step of saving your previous URL so that you can compare the prior URL with the current URL during save time to see if they match, and, with the way that the Feeds classes work together, even this gets somewhat tricky. I haven't yet looked to figure out just *why* ->save() is being called during ->import(). Maybe that's not really necessary? If it is, I would lean towards adding a flag parameter to ->save() that denotes that this save occurred during import so that various method implementers are able to ignore it.

twistor’s picture

Issue summary: View changes
Status: Needs review » Closed (duplicate)

This should be fixed in 7.x first.

#1291942: Don't subscribe on push notifications