Item1854: Foswiki::Prefs refactor has broken part of the published API

Can't locate object method "getTopicPreferencesValue" via package "Foswiki::Prefs"

this function is also used in the compatibility TWiki::Prefs module..

I'm not sure how serious an issue this is, as it appears to only be used in one published contrib, but its still a problem that I think needs to be resolved before we release 1.1

-- SvenDowideit - 26 Jul 2009

I implemented the same API that existed after Crawford's refactor. But if that method is used it should be available. I'm with very restricted internet access these days. Hope to solve it this week.

-- GilmarSantosJr - 27 Jul 2009

That function is documented as "used only in the protections mechanisms", and should never be called outside the core.

I see two applications of it; in WebPermissionsPlugin, where it is used to mirror the function of the core in code which badly needs rewritten anyway.

The second application is in LocalTimePlugin; I can't see any good reason for it to be called there.

So changing this to be a bug against the relevant plugins.

-- CrawfordCurrie - 05 Aug 2009

I'm working on the LocalTimePlugin right now. I can work on this if someone can explain what the issue is. The plugin can optionally invoke $session->{prefs}->getTopicPreferencesValue( 'TIMEZONE', $web, $topic ); but that seems to work okay and without error in Foswiki 1.0.9. Is this a Foswiki 1.1 issue? What makes this issue "urgent"?

As part of a general overhaul of LocalTimePlugin, I'd like to mark that particular feature of the plugin as officially deprecated to get the clock started on eventual removal. I'm not real clear on how much of the general Foswiki policy on deprecation applies to extensions. Or does none of it apply because $session->{prefs}->getTopicPreferencesValue() is not part of the officially published API? If that method is simply going away without an alternative method, does LocalTimePlugin get a pass on the backward compatibility requirement and can I just rip the thing out?

I think the code is really, really old user prefs code that has probably been obsolete for years. It seems to have been used to fetch the value of a TIMEZONE preference from a user's user topic. The preferences system has long since superseded any need for that capability within a plugin itself. Of course, it is also possible the use case is something else all together. I just can't think of a situation where you would need to pull a TIMEZONE preference from an arbitrary topic.

-- BryanThale - 24 Mar 2010

Bryan

Looking briefly at the plugin without thinking about how it works, only API compliance I see a few code lines that are a problem

        my( $web, $topic ) = $session->normalizeWebTopicName( $theWeb, $fromtopic );
        my $zone = $session->{prefs}->getTopicPreferencesValue('TIMEZONE', $web, $topic);

The $session->normalizeWebTopicName should be changed to Foswiki::Func::normalizeWebTopicName which is the official API way to do the same

The $session->{prefs}->getTopicPreferencesValue('TIMEZONE', $web, $topic) calls core code that is not part of official API and in this case the syntax has changed from 1.0.9 to what will become 1.1 so the plugin will break in 1.1 unless you make the code conditional. So here you should change to either use one of the official API calls documented in DevelopingPlugins, or write your own little piece of code in case none of the API calls work for you.

Looking at the function.

First thing to note is that the plugin has the policy "CoordinateWithAuthor". This means that you must ask the author for permission to change the behavour of the plugin. Most authors are perfectly OK with anyone fixing bugs. Getting a plugin API compliant is normally considered bug fixing. Removing a function required authors acceptance. The TIMEZONE is a documented feature so I would not remove it, anyway. It is pretty easy to change the code to pick a preference value from a topic.

Looking quickly at the code and the feature.

I would keep the feature as it is working "in spirit" but change the feature slightly. I would change it so the TIMEZONE is picked up as a preference value with the scope seen as the given topic. If no topic is given assume current topic. This would mean that the TIMEZONE preference would be picked from the topic given. If it is not defined on that topic it would pick the setting using the normal mechanism for preferences. This also means that the admin can define the TIMEZONE in SitePreferences (non-finalized) and users can change this by setting the TIMEZONE in their own topic in Main. Groups could define the setting in their group topic and specify the group topic.

The code would then become (not tested)

    if (defined($fromtopic)) {
        my( $web, $topic ) = Foswiki::Func::normalizeWebTopicName( $theWeb, $fromtopic );
        Foswiki::Func::pushTopicContext($web, $topic);
        my $zone = Foswiki::Func::getPreferencesValue( 'TIMEZONE' );
        Foswiki::Func::popTopicContext();
        $tz = $zone if defined($zone);
    }

That should give a good function that makes sense and make the plugin API compliant. The urgent status has only symbolic meaning for non-default extensions. For core code and default extensions it means release blocker.

Current author of the plugin is SvenDowideit.

-- KennethLavrsen - 24 Mar 2010

LocalTimePlugin 1.1 build 7009 (2010-03-31) released which solves this issue for it.

-- BryanThale - 03 Apr 2010

As I read it, both plugins have been cured.

-- CrawfordCurrie - 13 Apr 2013