Fix RSS 1.0 parsing and add basic test framework for common_syndication_parser

What is the standard for this going to be object or array? I can submit a patch if nobody is willing I just need to know what the standard will be.

Using Feeds to eat an RSS feed cooked by Perl's XML::RSS, I have also choked on this bug. Attached is a patch that worked for me. UPDATE: I found that using '->' breaks Feeds when importing an RSS feed into a node object, I only get 1 node instead of the 15 I should've got. So, casting via the '(array)' method might be the better solution; ref. http://drupal.org/node/720116#comment-2691590 . UPDATE2: mn_rdf10_beta8.patch works like a charm.

#720116: Getting Cron Error "cannot use object of type stdClass as array..." and this issue are duplicates. #4 needs to be reconciled with #720116-1: Getting Cron Error "cannot use object of type stdClass as array..." and vice versa. This issue prevails in HEAD, hence setting issue to 6.x-1.x.

subscribe

sorry, i missed this issue and posted my patch (mn_rdf10_beta8.patch) mentioned in comment #4 and #5 over at #720116: Getting Cron Error "cannot use object of type stdClass as array...".

i've tried to make sure that the mn_rdf10_beta8.patch returns an identically structured array ($parsed_source) like the other "parsers" - i used _parser_common_syndication_RSS20_parse() as reference; the convention seems to be array only - but i might be wrong as I am new to Feeds.

The current function mixes object and array-casting, and the feeds.patch above casts to object rather than array.

This whole object/array-casting could be avoided if _parser_common_syndication_RDF10_item was slightly rewritten (which i am happy to do, if the project owners want me to).

Rolled a new patch, incorporating the old patch (#8) and simplifying _parser_common_syndication_RDF10_item-function and the individual $item-array. The individual item-array *will not* have an 'options'-key anymore. This does not seem to be needed further down the chain (i might be wrong, tho :| ) - the other parsers don't provide an 'options'-key in their items, anyway.

Sorry for unusual path in the patch - am only using feeds in mn-context; a patch -p0 should apply (can only verify right now against 6.x-1.0-alpha11 as that's mn_beta8) if you adjust the filename on the prompt accordingly.

#10 Looks very good. pounard: can you confirm that #10 addresses the error that you initially reported?

Sub.

I had the same problem on a feed from craigslist using Managing News 1.0-beta8 and applying this patch removed the error (there were no new items, so I can't say they parsed correctly).

I don't know enough about the parser to say that this is "ready to commit" but it seemed to solve the problem.

UI re-rolled the patch with the correct path info,

Removed the extra:
+ 'author' => array('dc:creator', 'dc:publisher'),
(there were two)

And additionally, the parse function in FeedsSyndicationParser.inc was expecting a
field called link, which it sets on the FeedsBatch object, and everwhere else it was a string, vs an object. Granted getLink() is never called anywhere, but just in case it does...

$parsed_source = array(
'title' => _parser_common_syndication_title((string)$rss_channel->title),
'description' => (string)$rss_channel->description,
- 'options' => (object)array('link' => (string)$rss_channel->link),
+ 'link' => (string)$rss_channel->link,
'items' => array(),
);

Edit: er.. whoops.. accidentally attached it twice.

Noticed one more thing: $item['author'] never gets explicitly set to $item['author_name'] which is what the mapper is looking for.

re-re-rolled patch with that fix.

Note that Im in strong disagreement with this bit:

if (empty($item['author_name'])) {
$item['author_name'] = $parsed_source['title'];
}

But Im leaving it there so as to not break existing functionality. But I for one certainly dont want titles in my author field, simply because its empty.

Setting status.

#17:

Need more time for thorough review, but this is what I find right away:

A Not sure whether switching to author_name without making sure that author_name will make the parser work - look at FeedsSyndicationParser::getMappingSources().
B I agree, let's drop using title as author (sic!) fallback. Is this happening anywhere else?

Setting this to critical, as I'd love to roll a fix into upcoming alpha14.

Yeah actually thats why I set it to author_name

Line 42 of FeedSyndicationParser:

'author_name' => array(
        'name' => t('Author name'),
        'description' => t('Name of the feed item\'s author.'),
      ),

The patch #17 works well for me - and I agree the title as fallback for author_name should go :) Could not find any other occurences of "author".

In trying to figure out whether there are any I made an overview of what is mapped how in the the common_syndication_parser.inc (because it is a little confusing in places) - please see here:

http://spreadsheets.google.com/ccc?key=0AsaFss74KTikdGU4YWhMdXhQbnBHazNQ...

Maybe this can help revising / harmonising the way the different formats are parsed / mapped and which fallbacks are used (e.g. currently time() is a fallback for timestamp in RSS2.0, but not in the others and so on)

#20: But shouldn't author_name be a string, not an array?

#20 is only referring to the FeedsSyndicationParser::getMappingSources()-definitions in profiles/managingnews/modules/contrib/feeds/plugins/FeedsSyndicationParser.inc; all occurences of 'author_name' in the common_syndication_parser.inc are strings.

#23: Thanks for clarification, I misread 'author_name' => array('dc:creator', 'dc:publisher'), - apologies for confusion.

@morningtime - i just checked - this patch has not made it into mn-beta9 ...

@alexb - do you need more input? - can i be of any assistance regarding this issue / patch?

frega - This is just waiting on a patch including #19 B . If you could provide one and make sure that this patch is passing all tests, that would be of great help!

alex_b: sorry this took so long, please find patch attached; tested it with several RSS1.0 feeds (mn_beta9 dies on "adding" a new RSS1 feed, after patch it doesn't).
couldn't find any info on how to run mn-tests, tho - if you give me a hint how to set that up, i'll be more than happy to do that - if patch in #19 passed, this should pass as well ... famous last words, i guess :) hope you guys have a great drupalcon.

Let's see if this one is acceptable. I will test later if I have time.

Patch applied cleanly for me on my managing news site; so far so good... but the emails only come every hour or two (on certain feeds). If none come in through the rest of the day, I'd say this patch fixes the problem for me.

I can confirm that patch in #29 allows me to import feed in #25. I've added tests for it, too. See attached patch.

For some reasons tests on feed from #25 fail though. I know this is weird, I'd love if somebody else here could have a look at it.

Hang on, I think this is a character encoding issue. Will report back in a bit.

Ok, this works now. It was really a character encoding issue that occurred when saving the RSS 1.0 feed. Will commit asap.

Committed. Thank you everybody. Final patch attached.