Posted by fax8 on February 17, 2010 at 1:48am
| Project: | Mollom |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
When a new release breaks existing configuration I would like to read on the release notes that "please review your module configurations as this update will break your configuration" ...
Next time, when a Mollom update leave all previously Mollom protected forms unprotected, I would like to read that on your release notes.
Thanks,
Fabio Varesano
Comments
#1
There is an upgrade path so no forms should be left unprotected -- did you run update.php?
#2
I run update.php but mollom doesn't recognize the guestbook, this night I got about 8 entries with spam
#3
Guestbook module will need to be updated to work with the latest version of the Mollom module. Best to create an issue in the guestbook module issue queue.
#4
I'm the maintainer of Guestbook module, but I do not know of any integration with Mollom. Is it possible that some other kind of guestbook was meant here?
#5
I'm using the last version from http://drupal.org/project/guestbook, and indeed there was integration with Mollom 1.10 but this integration is lost with the new version Mollom 1.11. I made a new issue under Guestbook module issue queue http://drupal.org/node/717040
#6
As far as I can see, the changes for Mollom module to support Guestbook module were never committed.
Since Mollom module now allows integration with any form, we can continue over in #717040: Mollom support
#7
@Dries sure, I *obviously* run update.php .. Unfortunately it seems that mollom_update_6105() has a bug.. See below...
@sun This is not related to the Guestbook module but all modules whose forms were protected by mollom. This is *critical* as I've been able to reproduce and track down the bug (see below). Consequences of this bug are that *all* forms previously protected by mollom are left unprotected after db update to 6.x-1.11
The reason for this mess (it actually took me 2 hours to troubleshoot this one.. ) is a bad bug in mollom_update_6105() ...
After the mollom_form table creation, implemented in
db_create_table($ret, 'mollom_form', $schema);For each protected form there is a call to mollom_form_save() which calls drupal_write_record() to add the form to be protected into mollom_form table.
Unfortunately there is a problem here: as the table has been just created the drupal_get_schema() call in drupal_write_record() will fail as the DB schema has been loaded before and cached.
This will result in leaving the mollom_form table empty as each insertion has failed. Consequence of this is that ALL forms will remain unprotected. This can really be a pain in the axx for big drupal sites using mollom as it could easily generate huge load of spam.
This bug is easily fixed by changing the db_create_table() code above to:
db_create_table($ret, 'mollom_form', $schema);drupal_get_schema('mollom_form', TRUE);
The call to drupal_get_schema('mollom_form', TRUE) will result in drupal recreating the schema for the mollom_form (and cache it) so that it subsequent calls to drupal_write_record() will succeed.
I would have liked to submit a patch for this.. unfortunately I'm not sure how to proceed..
Simply fixing mollom_update_6105() as suggested above won't work for already upgraded to 1.11 websites..
They'll be simply out of luck as the protected forms infos have been lost: not inserted in the mollom_form table and not anymore in the variables. (Thanks to variable_del() in mollom_update_6105())
Now, IMHO.. two questions arise:
- Has this upgrade path been ever tested? Looks no to me..
- Why the hell db_create_table doesn't add the newly created table schema to the cached one? (Or invalidate the cached one?)
p.s.: Can I work @ acquia/mollom now? ;-)
#8
Agreed. I had this problem with an XML sitemap upgrade and had to manually refresh the schema. Patches for both D7 and D6 for review.
#9
Hm. Technically, update.php should already do this before executing any updates...?
#10
It doesn't. :( Ran into this same thing with an XML sitemap upgrade wanting to use drupal_write_record() after a change to a table.
#11
I really wonder why the update worked for me then. :-/ But anyway, if this fixes the bug, we need to get it in asap.
#12
Committed to CVS HEAD. I can try to roll a bugfix release tonight (in 12 hours or so).
#13
Thanks fax8! Good debugging.
#14
After some thinking about this, I would say that this patches are kind of workarounds rather that fixes..
I would say that the bugged code is logically correct. The problem is that one would expect db_create_table() to invalidate/refresh cached schema and that subsequent calls to drupal_get_schema() would contain the newly created table.
I'm asking to modify db_create_table() to add a call to drupal_get_schema('table', TRUE) so that the cached table schema would be invalidated.
Please keep an eye on http://drupal.org/node/717902
#15
Hello,
there is still the problem of guestbook not pretected with mollom new version 612
#16
@sahuni: As mentioned earlier in this issue, Guestbook was never officially supported by Mollom module. Adding support for Mollom is being tackled in #717040: Mollom support currently.
#17
Yes thanks
I added a patch from http://drupal.org/node/717040 in guestbook module and it's ok for current version
#18
Automatically closed -- issue fixed for 2 weeks with no activity.