Item9563: pushTopicContext does not re-read preferences in afterSaveHandler

In afterSaveHander, the Preferences are still set as established prior to the save.

Initial Topic contains:
   * Set NOTIFY = FredUser

After save, Topic contains
   * Set NOTIFY = FredUser, AnotherUser

in the afterSaveHandler, NOTIFY is still set to FredUser. Calls to pushTopicContext, and/or popTopicContext to this or other topics leaves NOTIFY set to the value from prior to the save. There appears to be no combination that will refresh preferences.

-- GeorgeClark - 27 Aug 2010

One more for you Gilmar. Any thoughts on how to refresh the Topic level on the stack?

-- GeorgeClark - 29 Aug 2010

After topic preferences are parsed, there is currently no way to refresh the values (parse the topic again). I don't know if it's a bug or a feature request. I'd guess it's a bug. It would impact only at save time. We'd need to add a refresh call to the Foswiki::Prefs::BaseBackend API and refresh it automatically after the topic is saved.

I didn't get what you mean by refresh the Topic level on the stack...

-- GilmarSantosJr

I tried pushing the "saved" topic to the stack, hoping that this would pick up the changes, but I guess it uses the cached meta somewhere. Even after the pushTopicContext, the prefs were unchanged.

-- GeorgeClark - 31 Aug 2010

Right. The topic is only parsed at the first time it's loaded. Is the solution I pointed good enough? I think it's simple to implement... I could do it in the next weekend.

-- GilmarSantosJr - 01 Sep 2010

If you think it's safe for 1.1. At this point I've worked around the issue. Although doing it with the preferences API would probably be cleaner.

-- GeorgeClark - 01 Sep 2010

Since ThinPrefs was not released yet and there is only one backend and the impact is very limited, I think it's safe.

-- GilmarSantosJr - 01 Sep 2010

I just attached a proposed patch. I think it's enough, but I don't have the time to write unit tests properly. Please analyse if it solves your issue. If so, I'd be very glad if you could also write some unit tests (since you understand the need and use case of the patch).

The patch contains a minor enhancement to Foswiki::Prefs::Parser: since it gets the backend preferences object, that already contains the topic object and an accessor, it's ugly and unecessary to also give the topic object to the parser.

-- GilmarSantosJr - 01 Sep 2010

Thanks for the patch. I'll take a look through it and see if I can get some unit tests for it. Changing state to being worked on.

-- GeorgeClark - 03 Sep 2010

Hi Gilmar,

It's taken me long enough, but I've finally written a unit test that will let me test this patch. I had to make one change. The topicObject was undefined when set from $prefs->topicObject; So I had to set it before calling parse.

If I simply invalidate the meta preferences prior to the afterSave handler, it seems to work equally as well. However I still have issues, but I'm not sure if it might be related to the Unit Test environment.

• The Foswki::Func::getPreference Always returns null for my pref variable when called from the afterSaveHandler. However your fix, (or a simple invaldate) fixes the Meta::getPreference() call.
• I have to issue a pushTopicContext after the topic has been saved, otherwise I get the before-save setting instead of the after-save setting.

Attaching my changes including the unit test

-- GeorgeClark - 05 Sep 2011

From IRC log:

[22:18] <GilmarSantosJr> gac410: I owe you a feedback about Foswikitask:9563
[22:18] <gac410> I never did get anywhere with it - but probably not worth investing much time in it at this point.
[22:19] <gac410> After all that, iirc, the application really needed to read the topic directly anyway.
[22:22] <GilmarSantosJr>
[22:23] <GilmarSantosJr> I just read your patch
[22:24] <GilmarSantosJr> the "delete $this->{_preferences}" achieve the same effect and the new "refresh" method is not needed
[22:24] <GilmarSantosJr> so, it's better than mine

-- GilmarSantosJr - 10 Dec 2011

Actually this appears to have a one-line fix - much simpler. If we have to call the afterSaveHander, delete the preference cache prior to the call. Any requests for preferences load the new preferences.

     if ( $plugins->haveHandlerFor('afterSaveHandler') ) {
         my $text = $this->getEmbeddedStoreForm();
+        delete $this->{_preferences};  # Make sure handler has changed prefs
         my $error = $signal ? $signal->{-text} : undef;
         $plugins->dispatch( 'afterSaveHandler', $text, $this->{_topic},
             $this->{_web}, $error, $this );

So I'll probably commit this fix. Feedback welcomed.

-- GeorgeClark - 21 Jan 2012

Commenting is disabled while not logged in