The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.I believe that if there is a "guid" for an item then there is no need to check for duplicate URL. Changing the first two if statements as follows will eliminate one database query for every fetched item.
if (isset($feed_item->options->guid)) {
...
}
else if (isset($feed_item->options->original_url)) {
...
}
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 206217_unique_improvement.patch | 1.16 KB | aron novak |
Comments
Comment #1
alex_b commentedThere are a lot of feeds out there, where GUID changes, but the URL stays the same, that's why we switched to url OR guid duplicate checking with good results.
One thing that might make sense here is: check first for URL, if present, check for GUID, if GUID different, then update node existing node - together with saving revisions of a node this could be an elegant way of tracking changed articles on the web.
However:
- this should be an optional feature
- before we should implement this, we should do a survey on whether my assumptions - that changing GUIDs on the same URL stand for changed content - are correct. i got this impression while working on dupes for over a year now.
Comment #2
mustafau commentedComment #3
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.
Comment #4
shawn dearmond commentedSorry, I'd like to open this up for discussion again. I'm using 6.x-1.7-beta1, and this issue has come up for me. For the particular iCal feed that I'm grabbing, 98% of the events, for whatever reason, doesn't have original_url values. Only GUIDs.
However, $feed_item->options->original_url is still "set", it just happens to be blank. So, since '' == '', it's just recognizing most of the feeds as updates.
I would like to propose the following change:
Comment #5
aron novakThis is a really good point that make this process more robust. I'll post a patch here soon.
Comment #6
aron novakShawn DeArmond: Can you test the patch that it solves the issue as well?
in the function below i use the isset() check also, that's why i went this way.
Comment #7
shawn dearmond commentedPatch worked for me. Great idea about unsetting the variable.
Thanks!
Comment #8
aron novakShawn DeArmon: thanks for cooperating! The fix will be included in today's beta2 version! Stay tuned!