Unique targets (guid, url) fail if one is blank, and other is set. [#791274]

Okay, so this is probably edge case, but Im using the parser_ical to import event nodes, and the problem Im running into is that the only way to determine uniqueness is to use either the url or the guid.

I have several ical feeds, and some of which have both URL and GUID set, some have only URL, and some have only the GUID.

The problem is when one or the other is blank, the first node gets inserted into the database from the feed, and from that point forward the blank url or guid returns true for uniqueness resulting in only one node being created.

I've attached a crude patch that breaks before the query to show you how I solved this for the time being,

====Feature Brainstorm====
Ideally the way uniqueness is configured could be made more extensible so that ANDing and ORing would be possible to determine just what really is a unique feed item on a feed processor by feed processor basis (and additionally allow external modules a facility for defining their own unique fields) (ie, guid *and* url combined). Perhaps an additional column in feeds_node_item that contains a prehashed key (so we dont have to reinvent a query builder), and a serialized ruleset object stored in feeds_importer. One gotcha is that future changes to an exisiting feed importer's uniqueness rules would break the keys for existing feed_node_items that were imported by that feed importer. (Though since we have the nid of the existing feed_node_item we could rekey all the existing feed_node_items, maybe with a big fat warning that says "are you sure you want to change the uniqueness rules?"). I shouldnt try and solve stuff like this at this time of night.

Anyhow, thinking out loud here:

Check the patch below an let me know

Comment	File	Size	Author
#1	FeedsNodeProcessor.patch	870 bytes	Mixologic
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Mixologic

He/Him/His

English

Portland, OR

CreditAttribution: Mixologic commented 6 May 2010 at 08:24

File	Size
FeedsNodeProcessor.patch	870 bytes

whoops. Here's the patch.

Comment #2

alex_b CreditAttribution: alex_b commented 6 May 2010 at 19:37

Status:

Active

» Needs work

I think there's an existing issue for this. Maybe it's because it is very similar to #769084: Only imports the first feed item.

However, please use:

if (!empty($value)){
  $nid = db_result(db_query("...
}

Instead of a if - break pattern.

Comment #3

Argus CreditAttribution: Argus commented 30 June 2010 at 11:46

I'm not a coder but

<?php
    foreach ($this->uniqueTargets($source_item) as $target => $value) {
      switch ($target) {
        case 'url':
			if (!empty($value)){
          $nid = db_result(db_query("SELECT nid FROM {feeds_node_item} WHERE feed_nid = %d AND id = '%s' AND url = '%s'", $source->feed_nid, $source->id, $value));
		  }          
        case 'guid':
			if (!empty($value)){
          $nid = db_result(db_query("SELECT nid FROM {feeds_node_item} WHERE feed_nid = %d AND id = '%s' AND guid = '%s'", $source->feed_nid, $source->id, $value));
          }		  
      }
?>

doesn't work. It keeps on importing a small part of the feed over and over. Same thing happens if I don't use GUID as an Unique target.

Comment #4

alex_b CreditAttribution: alex_b commented 30 June 2010 at 14:01

Breaks are missing.

    foreach ($this->uniqueTargets($source_item) as $target => $value) {
      switch ($target) {
        case 'url':
            if (!empty($value)){
          $nid = db_result(db_query("SELECT nid FROM {feeds_node_item} WHERE feed_nid = %d AND id = '%s' AND url = '%s'", $source->feed_nid, $source->id, $value));
          }
          break;   
        case 'guid':
            if (!empty($value)){
          $nid = db_result(db_query("SELECT nid FROM {feeds_node_item} WHERE feed_nid = %d AND id = '%s' AND guid = '%s'", $source->feed_nid, $source->id, $value));
          }
          break;
      }