Skip Menu |

This queue is for tickets about the XML-FeedPP CPAN distribution.

Report information
The Basics
Id: 35061
Status: resolved
Priority: 0/
Queue: XML-FeedPP

People
Owner: MARKOV [...] cpan.org
Requestors: avbidder [...] fortytwo.ch
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 0.34
Fixed in: (no value)



Subject: Atom 1.0 feeds: uniq_item
As far as I can see, uniq_item should (at least for Atom 1.0 feeds, I haven't looked at any other spec) be unique by id and not by its link attribute (especially since the link attribute is optional in Atom 1.0 and id is not.) cheers -- vbi # HG changeset patch # User avbidder@fortytwo.ch # Date 1208258767 -7200 # Node ID 2f91438567b62d9a39e9fc6f717c1bca71900c31 # Parent f14f4473e5b3efc91d85a6b693dbc8062bbb8ed0 Atom 1.0: Articles should be unique by id, not by the link attribute. diff -r f14f4473e5b3 -r 2f91438567b6 lib/XML/FeedPP.pm --- a/lib/XML/FeedPP.pm Tue Apr 15 13:26:07 2008 +0200 +++ b/lib/XML/FeedPP.pm Tue Apr 15 13:26:07 2008 +0200 @@ -1429,8 +1429,8 @@ my $check = {}; my $uniq = []; foreach my $item (@$list) { - my $link = $item->link(); - push( @$uniq, $item ) unless $check->{$link}++; + my $id = $item->guid(); + push( @$uniq, $item ) unless $check->{$id}++; } @$list = @$uniq; } bi
From: avbidder [...] fortytwo.ch
Just a quick note: why is this important to me? Because I feel the behaviour as it is now violates the RFC. Updated patch which also fixes the tests, albeit probably not in an ideal way: diff -r a5546360ed69 lib/XML/FeedPP.pm --- a/lib/XML/FeedPP.pm Tue Apr 15 17:14:32 2008 +0200 +++ b/lib/XML/FeedPP.pm Tue Apr 15 17:33:05 2008 +0200 @@ -1429,8 +1429,8 @@ my $check = {}; my $uniq = []; foreach my $item (@$list) { - my $link = $item->link(); - push( @$uniq, $item ) unless $check->{$link}++; + my $id = $item->guid(); + push( @$uniq, $item ) unless $check->{$id}++; } @$list = @$uniq; } diff -r a5546360ed69 t/12_sort_item.t --- a/t/12_sort_item.t Tue Apr 15 17:14:32 2008 +0200 +++ b/t/12_sort_item.t Tue Apr 15 17:33:05 2008 +0200 @@ -39,7 +39,9 @@ is( $date2, $f->get_item(3)->pubDate(), "$mode sort #1" ); is( $date4, $f->get_item(1)->pubDate(), "$mode sort #2" ); $f->get_item(4)->link( $link3 ); + $f->get_item(4)->guid( $link3 ); $f->get_item(3)->link( $link3 ); + $f->get_item(3)->guid( $link3 ); $f->normalize(); is( 3, scalar $f->get_item(), "$mode count #3" ); $f->limit_item( 1 ); diff -r a5546360ed69 t/16_invalid_pubdate.t --- a/t/16_invalid_pubdate.t Tue Apr 15 17:14:32 2008 +0200 +++ b/t/16_invalid_pubdate.t Tue Apr 15 17:33:05 2008 +0200 @@ -57,6 +57,7 @@ <modified>$date114h</modified> <entry> <link rel="alternate" type="text/html" href="$url"/> + <id>$url</id> <issued>$date110h</issued> <modified>$date111h</modified> </entry>
Patch you sent would have a problem with RSS 1.0 (RDF) format which does not have guid. uniq_item() method's behavior could be its spec as getting uniqueness by URL. To get uniqueness by guid to follow Atom's spec, we should add similar code for XML::FeedPP::Atom* class? Thanks. On 金曜日 4月 18 08:59:45 2008, avbidder wrote: Show quoted text
> Just a quick note: why is this important to me? Because I feel the > behaviour as it is now violates the RFC. > > Updated patch which also fixes the tests, albeit probably not in an > ideal way: > > diff -r a5546360ed69 lib/XML/FeedPP.pm > --- a/lib/XML/FeedPP.pm Tue Apr 15 17:14:32 2008 +0200 > +++ b/lib/XML/FeedPP.pm Tue Apr 15 17:33:05 2008 +0200 > @@ -1429,8 +1429,8 @@ > my $check = {}; > my $uniq = []; > foreach my $item (@$list) { > - my $link = $item->link(); > - push( @$uniq, $item ) unless $check->{$link}++; > + my $id = $item->guid(); > + push( @$uniq, $item ) unless $check->{$id}++; > } > @$list = @$uniq; > }
agreed. Taken in 0.95