Aggregator fails to parse author entries longer than 255 characters

To truncate the author field seems to be the right way for me because of the similarity to the title field.
Thanks in advance for the patch. :-)

Title was truncated as the spec states the maximum length is 100 characters. I suppose the key question is the likelihood of someone querying against author but in the case of the example above it is 'authors'. I am incline to say make it a text field because in reality the above example shows that in truncating would invalidate a search anyway.

Changing priority and tags with reference to #218004: Allow aggregator feed URLs longer than 255 characters

This is not a major bug.

Note to self read linked issue first ;)

Fixing tags per http://drupal.org/node/1517250

In this case I think we should truncate it. The URL is a special case.

Truncation is the approach I used to get a quick and dirty fix for myself under D7. I ran into this same scenario with a different "Nature" article. Scientific journals would seem more the culprits here. The only thing that might be a little cleaner is to try to avoid chopping a fraction of a name, and maybe inserting a "[more]" notice at the end to replace whatever is at the back of the field.

I'll write a test.

@joevansteen, the truncate_utf8() function takes care of that for us (adding an ellipsis).

Tests and fix.

Oopsie, broken tests patch above. Fixed here.

Adjusting another test that relies on the number of items in the sample feed.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Tests/FeedParserTest.phpundefined
@@ -38,6 +38,15 @@ function testRSS091Sample() {
+    $this->assertText('Long link feed item title');
+    $this->assertText('Long link feed item title');
...
+    $this->assertText('Long author feed item title');
+    $this->assertLinkByHref('http://example.com/long/author');
+    $this->assertText('Long author feed item title');

Oops, bad copypasta. Fixed in attached.

Upload failed apparently.

Simple fix, has tests. Let's get this in.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Tests/FeedParserTest.phpundefined
@@ -38,6 +38,15 @@ function testRSS091Sample() {
+    $this->assertLinkByHref('http://example.com/tomorrow/and/tomorrow/and/tomorrow/creeps/in/this/petty/pace/from/day/to/day/to/the/last/syllable/of/recorded/time/and/all/our/yesterdays/have/lighted/fools/the/way/to/dusty/death/out/out/brief/candle/life/is/but/a/walking/shadow/a/poor/player/that/struts/and/frets/his/hour/upon/the/stage/and/is/heard/no/more/it/is/a/tale/told/by/an/idiot/full/of/sound/and/fury/signifying/nothing');

+++ b/core/modules/aggregator/tests/aggregator_test_rss091.xmlundefined
@@ -26,5 +26,16 @@
+      <author>I wanted to get out and walk eastward toward the park through the soft twilight, but each time I tried to go I became entangled in some wild, strident argument which pulled me back, as if with ropes, into my chair. Yet high over the city our line of yellow windows must have contributed their share of human secrecy to the casual watcher in the darkening streets, and I was him too, looking up and wondering. I was within and without, simultaneously enchanted and repelled by the inexhaustible variety of life.</author>

o.0

Agreed with the fix, and thanks for the literature lesson in the process of the tests. ;)

Committed and pushed to 8.x. Moving to 7.x.

Backported to D7. Needs review.

Added test only version of patch.

#18: aggregator-1622974-18.patch queued for re-testing.

Backport looks good too, RTBC (Patch in #18, #19 is test only)

Thanks @design_dolphin. See #218004: Allow aggregator feed URLs longer than 255 characters for extensive discussion of why we are limiting the varchar, and why we used a text there. Also keep in mind that not all sites use MySQL for their storage.

My reasoning for truncation was that in each case, the author data is just a summary of the RSS item. The aggregator item still has a link to the actual item, so the full title, author, etc. information will be included there.

The current behavior when you have an author field over 255 characters isn't that the author is partially imported--it's that there's a fatal error that prevents any feed data from being imported at all. That's why this is a major bug. So let's commit #18 as a stopgap to resolve this major bug on production sites impacted by it, and then open a followup issue to discuss increasing the allowed length?

Please open a followup issue?

Reiterating, this issue is not changing any feature of Drupal 7. It's only stopping the whole feed import from breaking. Whether to lengthen the field is worth discussing, but it should not block this.

While I can definitely appreciate design_dolphin's point, xjm's correct that in this particular instance, I think truncating is the right thing to do, because:

- Aggregator items are ephemeral; reloading http://drupal.org/planet twice a week is going to come up with a completely different list of stuff. These are not intended to be reference pieces that stick around for a long time. AFAIK they don't even have their own URLs.
- Aggregator items are specifically intended to be a jumping off point to the "real" work that exists somewhere else, and will contain the full article, along with its full title and list of authors.
- Changing the data type of a field is not as easy as it sounds, due to cross-database concerns as well as query performance concerns.

If we were talking about something like Feeds module, whose primary purpose is to import this external content as actual site data, then I think the truncation argument makes a lot less sense. But that's not what we're talking abut here.

So, in the interest of closing down a major bug whose consequence is a database failure, Committed and pushed to 7.x. Thanks!

Now, to 6.x. :)

Thanks @design_dolphin, that followup looks good. I'll link it in this issue's summary so folks investigating this issue can find it easily.

Should this issue be backported to 6.x? The related 255-character title field did not get ported to 6.x.

To be honest, I want to make certain because the 6.x code is quite a bit different than it is in 7.x.

Actually, that's a good point. Can you test it and see if a feed with a super long author field causes the site to die with a horrible database failure like in the original post? I suspect not, since D6 just uses db_query() wrappers and not PDO. If not, I think we can move back to 7.x and fixed. Sorry, I was just following the tags here. :)

Yes, I believe this error only happens in 7.x because we enable the mysql strict mode, which isn't on in 6.x so MySQL should actually just anything that's too long. On the other side, it might break PostgreSQL, which 6.x kinda supports.

Adding this one to the release notes and CHANGELOG.txt.

Updated issue summary.