Item8740: Meta save, when calling beforeSaveHandler, ignores changes to topic text if also meta has changed

Someone have very recently changed something in trunk so CommentPlugin cannot save anymore.

When you hit the Save Comment button the page is refreshed but nothing is saved.

I do not remember any CommentPlugin updates recently so I suspect a core code change.

-- KennethLavrsen - 21 Mar 2010

After some detective work it turns out it is when RevCommentPlugin is enabled that CommentPlugin will not save.

These two have worked together for years in production.

-- KennethLavrsen - 22 Mar 2010

I believe the problem is the Meta.pm. Something has changed since 1.0.9.

It is the code line in RevCommentPlugin

$meta->put( 'REVCOMMENT', \%args );

that causes the text added by CommentPlugin to get removed. Not a plugin bug. That is a core bug

-- KennethLavrsen - 22 Mar 2010

Further debug reveals this.

There is absolutely nothing wrong with RevCommentPlugin or with CommentPlugin as I can track the data flow.

CommentPlugin is called before RevCommentPlugin.

CommentPlugin will add the comment to the $_[0] variable this is done correctly.

When RevCommentPlugin enters the beforeSaveHandler the $_[0] contains the correct comment added by CommentPlugin.

And before it leaves beforeSaveHandler, the $_[0] remains correct.

But! If the beforeSaveHandler called $meta->remove the resulting save ignores any changes done to $_[0]. This is very strange indeed.

It points again to a bug in core in Meta.pm.

-- KennethLavrsen - 22 Mar 2010

I tried this experiment.

Instead of the current RevCommentPlugin code I replaced the entire beforeSaveHandler with this sub

sub beforeSaveHandler {
### my ( $text, $topic, $web, $meta ) = @_;   # do not uncomment, use $_[0], $_[1]... instead
    my ( $topic, $web, $meta ) = @_[ 1 .. 3 ];
Foswiki::Func::writeDebug("RevComment entering\n$_[0]");
$meta->put( 'NONSENSE', { name => 'Maxage', title => 'Maxage', value =>'103' } );
return;

And that makes the CommentPlugin work again.

I then moved my $meta->put / return down line by line. And finally it turns out that it takes a $meta->remove to trigger the error.

-- KennethLavrsen - 22 Mar 2010

It seems it takes both a remove and put to trigger the error. If I replace the beforeSaveHandler in RevCommentPlugin by this simple sub it still makes CommentPlugin not save anything.

sub beforeSaveHandler {   
    my ( $text, $topic, $web, $meta ) = @_;
    $meta->remove('REVCOMMENT');
    $meta->put( 'REVCOMMENT', { comment_1 => 'ccc', minor_1 => '', ncomments => '1', rev_1 => '3', t_1 => '1269293340' } );
}

-- KennethLavrsen - 22 Mar 2010

The simple code above does actually work when you try again.

I have noticed that if the meta already in the topic is the same that I "put" again, the CommentPlugin saves its text. If the meta changes, the topic text gets reverted as of CommentPlugin had never altered it.

Here is a new code that always fails

sub beforeSaveHandler {   
    my ( $text, $topic, $web, $meta ) = @_;
    $meta->remove('REVCOMMENT');
    $meta->put( 'REVCOMMENT', { comment_1 => 'ccc', minor_1 => '', ncomments => '1', rev_1 => '3', t_1 => time() } );
}

-- KennethLavrsen - 22 Mar 2010

Looking more at Meta.pm.

The key is that the meta has changed in the beforeSaveHandler and this makes 'save' Meta.pm do something that makes it loose the content of $text.

The bug is introduced when the code was refactored from Store.pm to Meta.pm.

-- KennethLavrsen - 22 Mar 2010

I have it. Checking in fix.

It was the refactoring that was done wrong.

-- KennethLavrsen - 22 Mar 2010

My fix broke something else.

Fixing the fix. Peer review wanted.

-- KennethLavrsen - 23 Mar 2010

Peer-reviewing the fix, I must admit we're carrying legacy stuff for nothing.

Cleaned it up and made it more OO-ish. All unit tests pass, and all tests I made are OK. Kenneth, please try again your issue with my latest commit and let me know of the outcome.

-- OlivierRaginel - 23 Mar 2010

Note that Olivier checked in some code cleaning on this item to use more native OO then calling Foswiki::Meta from within Foswiki::Meta.

My fix was OK. (for my own selfish pride). We used this bug item because I asked for peer review.

-- KennethLavrsen - 23 Mar 2010

Commenting is disabled while not logged in