Outdated update hook causes problems with certain characters

[laryn]

The outdated updated hook _1000 which is superceded by _1001 causes problems when trying to create config files and the source has a character like : in it. (e.g. from feeds_xpathparser, with a value like xpathparser:3).

I think we should be able to adjust the old/outdated code to just create a good config file in _1000 rather than create some kind of workaround, and then the logic in _1001 won't make it run again.

ConfigNameException: Invalid character in Config object name feeds_tamper.feed_name_feed.xpathparser:3. in Config::validateName() (line 562 of /var/www/html/core/includes/config.inc).

[laryn]

@herbdool Are you able to test/spot check this one?

[herbdool]

@laryn I'll take a look soon. By Monday I think.

[herbdool]

@laryn In update_1000 the feeds_tamper. files still exist, so in the next update hook it would run them again. Probably better to delete each file in the foreach loop. Then update_1001 wouldn't find any.

[laryn]

@herbdool I think update_1000 is what originally creates the feeds_tamper. files, and update_1001 was what converted them to the current format. I was running into errors where the update_1000 kept crashing because of invalid filenames when it tried to insert the source as part of the filename, so I'm proposing we just don't create the problematic files in the first place in update_1000.

[herbdool]

@laryn this is what happens when I look at the diff and start mixing up before and after. Yes, your PR wouldn't have feeds_tamper.* files so can ignore what I said. Then I would say lets' remove the body from update_1001 and put a comment that it was removed because update_1000 now covers both steps.

One other thing, right now in update_1000 it has unserialize($tamper->settings). In your PR it is adding the whole db row object to the config. Is unserialize handled automatically or is it actually storing the settings as a serialized string in your PR?

[laryn]

Hmm, good question!

[laryn]

Looks like it's safe to assume nobody is in between the two update hooks so it should be safe to gut _1001:

[laryn]

@herbdool Looking into this now and I think you're right. PR updated, including a minor tweak to save disabled status and to fix this warning in the adjusted code for _1000:

Warning: Undefined property: stdClass::$source in feeds_tamper_update_1000() (line 31 of /var/www/html/modules/contrib/feeds_tamper/feeds_tamper.install).

[herbdool]

I think it's good now. I added a link to this issue in the code comment.

I didn't test it. It's harder for me to find d7 sites with feeds now.

[laryn]

@herbdool Thanks. I think it's good, too. I'll merge and release and keep an eye on it in the next weeks -- I'll be using it once more shortly.