Feature Proposal: In-line * Set preferences should always match META:PREFERENCE, and vice versa

Motivation

  • The inconsistency hurts usability. Imagine the user's confusion upgrading to the new groups UI, where s/he discovers that changing the * Set GROUP line has no effect. PreferenceSettings doesn't help them; META:PREFERENCE is not discussed in the settings hierarchy - but, if altering the Set line updated META:PREFERENCEs and vice-versa, users would be happy.
  • NEW It sucks that prefs can't be queried. Horrible regex concoctions make users cry; the most baroque QuerySearch merely gives a blank stare
  • NEW We can very easily do nifty AJAX prefs editor (for example) by using RestPlugin's PATCH verb on an individual META:PREFERENCE. Such an editor could target the META:PREF, whereas right now you'd have to do a rest handler (or download the topic text?!) and mangle the inline * Set statements yourself (not to mention do the synchronisation logic)
  • My testing with DBIStoreContrib and MongoDBPlugin seem to indicate that the main bottleneck with SEARCH is still mainly centred in Foswiki::Meta::setEmbeddedStoreForm, which among other things parses out * Set statements to determine if ACLs allow the hit to be included in the result. If the ACL check could be done without setEmbeddedStoreForm, things would be much quicker. In order to do this, we need to serialise in-line * Set statements out of the body text, and I think META:PREFERENCE is a reasonable place to put them.

Description and Documentation

Functional description

Foswiki allows PreferenceSettings to be set using in-line Set and Local statements, as well as META:PREFERENCE MetaData. Both methods are functionally equivalent. In situations where a preference is set in-line in the topic text and again with the same name in MetaData, the MetaData setting is used and the in-line setting ignored.

Prior to Foswiki 2.0, the behaviour when loading in-line vs MetaData PreferenceSettings was as follows:
On-disk
 
Loaded as (and saved as)
 
 
In-line setting Metadata Setting In-line text Metadata Setting Expanded value
Not exist
Exist
No setting
Metadata value
Metadata value
Exist
Not exist
In-line value
Not set
In-line value
Exist
Exist
In-line value
Metadata value
Metadata value

Foswiki 2.0 introduces new functionality during topic loading and saving to make PreferenceSettings searchable with QuerySearch, and keep in-line settings synchronised with MetaData settings (and vice-vesra), The behaviour is as follows:
On-disk
 
Loaded as (and saved as)
 
 
In-line setting Metadata Setting In-line text Metadata Setting Expanded value
Not exist
Exist
No setting
Metadata value
Metadata value
Exist
Not exist
In-line value
In-line value
In-line value
Exist
Exist
Metadata value
Metadata value
Metadata value

Code change summary

  • At topic load, (actually at lazy-load-prefs time) change Foswiki::Prefs::Parser::parse() to allow CPU efficient sync of * Set & META:PREF
  • During the request, plugin authors use Foswiki::Meta::setPreference() in symmetry with getPreference(). If a PREF that is known to exist inline is 'set', mark the prefs as "out-of-sync"
  • Foswiki::Meta::getEmbeddedStoreForm() checks to see if prefs are marked as "in sync". If not, it calls Foswiki::Meta::synchronisePrefs(). Plugin authors need to be aware that if they have changed the text or manipulated META:PREF using put() and friends, it's their responsibility to forcibly call Foswiki::Meta::synchronisePrefs() before calling Foswiki::Meta::getEmbeddedStoreForm() Do we offer an Foswiki::Meta::invalidatePrefSyncState()?
  • At topic save, call Foswiki::Meta::synchronisePrefs(). This involves re-running Foswiki::Prefs::Parser::parse(), so we'll need to study the performance impact of this.

Code change detail

  1. Changes to Foswiki::Prefs
    1. Remove already deprecated, non-documented 'S' formfield attribute that allowed setting of preferences via formfields. I've never heard of this feature, I assume it's pre-Foswiki 1.0. The reason I want to do this now, is to offset some additional overheads that this feature introduces. Add deprecation advice to upgrade guide/release notes
    2. Change Foswiki::Prefs::BaseBackend and its implementations, to not only set up & destroy $prefs->{local} vs $prefs->{value} (Set) prefs but also track inline-only Sets/Locals
    3. Change Foswiki::Prefs::Parser::parse() which blindly overwrites prefs set with * Set statements coming from META:PREFERENCES of the same name. Instead, remember the * Set value in two places, rather than just $prefs->insert( $type ...)
    4. Provide a way for Foswiki::Meta::syncPreferences() to detect if there have been any pref inserts since the initial topic parse - some flag somewhere
  2. Changes to Foswiki::Meta
    1. Add a new method: Foswiki::Meta::syncPreferences(). Add to release notes
      • Returns a boolean indicating that preferences were already synchronised (no change). Remember this state in some sort of $meta->{_prefsInSync}.
      • This method at some point calls Foswiki::Prefs::Parser::parse() (unless no pref changes have occurred since initial parse) with an empty $prefs stack. Afterwards, the temporary $prefs are compared against the original $prefs (and inline prefs checked against final prefs for consistency) - where there are differences, it checks to see if the inline-set is more current than the META version, or vice-versa; and updates the opposite (this might mean deleting or adding META:PREFERENCEs).
    2. Add a new method: Foswiki::Meta::setPreference(), as mentioned in comments for getEmbeddedStoreForm() should we we get the ability to sync in-line and META prefs. Add to release notes
      • Use the prefs stack to see if an inline pref of the same name exists. If yes, then set $meta->{_prefsInSync} false.
    3. Change Foswiki::Meta::getEmbeddedStoreForm() to check $meta->{_prefsInSync} - call Foswiki::Meta::syncPreferences() if false.
      • Doesn't help you if $meta->{_text} was modified to include new/deleted/modified in-line * Set statements since initial prefs parse - add advice in method doc: plugin authors should use setPreference() and/or call syncPreferences() themselves before calling getEmbeddedStoreForm()
    4. Change Foswiki::Meta::save() to call syncPreferences() prior to the call to saveAs()
    5. Change Foswiki::Meta::getPreference() to call Foswiki::Meta::syncPreferences() after the initial lazy-load/loadPreferences() call.
      • Apart from save, this is the only mandatory call to syncPreferences(). We can avoid unnecessary parsing if the topic's prefs stack hasn't been touched since the initial load (useful when topics are just having their ACLs scanned). Given that haveAccess() must always use getPreference() anyway, "lazy load" is a bit misleading (we always load prefs unless you are admin and the prefs are only being scraped for ACLs).
  3. Ship a script resthandler that reports and/or fixes topics that have inconsistent inline vs meta prefs. While not mandatory for most RCS users, I think MongoDB + DBIStore users will need it to transition to a prefs back-end which doesn't need to use the Foswiki::Prefs::Parser::parse() and/or Foswiki::Meta::setEmbeddedStore()

Examples

Impact

  • Save performance (will include a test)
  • Direct filesystem modification of topics could leave the prefs in an inconsistent state, but that's what we have already 

%WHATDOESITAFFECT%
edit

Implementation

-- Contributors: PaulHarvey - 15 Dec 2010

Discussion

In principle I agree BUT I do not understand the spec that you actually want to implement.

Can you elaborate a little more on that?

-- KennethLavrsen - 15 Dec 2010

Added some implementation thoughts. Reset clock. Please comment. This part of Foswiki isn't very familiar to me.

-- PaulHarvey - 15 Dec 2010

I'm concerned that the motivation for this is not clear. Let's just worry for a moment about how the inconsistency affects usability, without getting too technical. There are a number of problems here:
  1. META: prefs override * Set statements silently, which is a major source of confusion
  2. The interface for editing META: prefs - when you realise they exist - is clunky and horrible
  3. The inline-ness of * Set prefs adversely affects performance
Strictly aligning the * Set prefs with the META: prefs is one way to address these points, but it is not the only way. Another way is for the user interface to be clear where (1) happens and for (2) to be done more sweetly. (3) is a separate issue, which can be solved without changing the user experience in any way, by caching the * Set= in META: on the fly during saves.

So, let's just concentrate on (1) and (2). The simplest possible interface for editing prefs is to have them inline in the text. There are a number of ongoing issues with this:
  1. Prefs are ordered in the topic - only the last * Set BLAH actually sets BLAH.
    • My intention is that all * Sets of the same scope+name would be updated to the same value - PH
  2. Extracting prefs from a topic (e.g. using a SEARCH or QUERY) is hard.
    • That's beyond the scope of this proposal, but
      %SEARCH{"preferences[name='WEBFORMS' AND value~'*MyForm*']" type="query" topic="WebPreferences" web="all"}%
      is still far more readable than any regex concoction - PH
  3. The syntax for line continuation is pretty horrible; it's hard to assign a value to a pref that is anything more complex than a short text string.
    • That's beyond the scope of this proposal, I don't try to fix every Pref problem here. - PH
  4. Finding prefs settings in the middle of a big topic can be a PITA.
    • This concern is so vague I cannot even formulate a response stick out tongue -PH
I don't see this proposal as addressing any of these points.

-- CrawfordCurrie - 16 Dec 2010

"Remove already deprecated, non-documented 'S' formfield attribute that allowed setting of preferences via formfields. I've never heard of this feature, I assume it's pre-Foswiki 1.0. The reason I want to do this now, is to offset some additional overheads that this feature introduces. Add deprecation advice to upgrade guide/release notes"

This feature was deprecated from (tm)wiki some time around 2006. It is not documented anywhere anymore. It is only in the code as the normal deprecation action. So we should not in any way revive it in the documentation. Question is if it is now the right time to remove it from the code. Question is if anyone comming from (tm)wiki still have webs that depend on this.

The last release in which it was documented was Cairo

http://merlin.lavrsen.dk/cairo/bin/view/TWiki/TWikiDocumentation#Using_Forms_For_Settings

I believe it was removed in (tm)wiki 4.0.0 (Dakar). At the same time we added PreferencesPlugin as a replacement for the deprecated feature. You can find the story in the Codev web in the old project.

In short I think we are ready to take the code out in 2.0. Would like to hear other views.

-- KennethLavrsen - 16 Dec 2010

Crawford, added some notes. I don't have time to fix everything that is wrong with prefs all at once. Synchronising in-line and meta is a narrow focused problem that I can hope to get a result for. I figure that if we can agree on a robust public API, and get a good list of unit tests going - that would be a valuable contribution to the project even if you dislike the implementation (hopefully hidden behind the API, not locking us into any limitations)

-- PaulHarvey - 16 Dec 2010

Added clearer motivations, and a brief summary of the implementation detail under "Description & Documentation"

-- PaulHarvey - 16 Dec 2010

Maybe by reading the code description I can have the answer but I prefer a clear verbal statement.

Do you suggest that when setting a "Edit Preferences" type preference and the same exists in the topic itself, the line in the topic should be altered? That sounds like something that could go very wrong if what you wanted to do was setting a preference in meta first and then plan to rename the in-topic pref to something else right after.

Or is it only when defining an in topic Set/Local that you want it stored in meta also?

I am not clear yet on your spec.

-- KennethLavrsen - 16 Dec 2010

Oops - as I think Crawford was trying to tell me, I needed to write the user documentation. Kenneth, I started a verbal description but I figured I've already used up my quota. So I've done some sort of truth table - does that help?

-- PaulHarvey - 16 Dec 2010

Crawford, please let me know how I can address your remaining concerns.

Personally, my own concern is performance impact. There is still a possibility that there will be an unacceptable overhead to keep everything synchronised. I plan to do load/save timing tests over every topic we ship and compare before/after.

-- PaulHarvey - 16 Dec 2010

Yes the truth table helped.

And my only concern is the case where an already defined Set in the topic gets rewritten when setting a preference with the same name.

I can imagine that some users will find it surprising. But that could be mitigated by a small confirmation UI, so when you save the preferences a warning dialog can tell the user that there are settings in the topic with the same name and ask for confirmation if the user wants them overwritten or kept as is. This way the user can choose to keep the setting and right after edit the topic and maybe change the setting so it sets another macro name.

I like the idea that we have a sync mechanism when saving topic or preferences. The current behavior creates problems for people. Especially with the Group topics. But also with other preferences like access right preferences.

-- KennethLavrsen - 17 Dec 2010

If we are validating Meta vs. Inline pref's we might also want to detect when a user has a duplicated Set statement in the topic. The unexpected behavior is that the last Set is always used. It can be confusing for the new user if they expect a more linear behavior.

-- GeorgeClark - 17 Dec 2010

I'm very worried about CPU overhead. My intention is that if there are multiple occurrences of an in-line set, then if inconsistencies are detect, the sync routine would fix all occurrences to be the same value. This reflects how Foswiki actually works, so I assumed that exposing it like this would be a good idea. Anyway, these inconsistencies will only occur on Foswikis that don't have this feature; going forward, the synchronisation should keep things consistent.

I guess my initial implementation I had in mind was going to ignore changes to all but the last occurrence of duplicate in-line set statements (as I'd be re-using the existing prefs parser), but there should be a cheap way to detect inconsistent duplicates as we scan them in (and fire the "synchronisePreferences" method accordingly).

I'm just worried about firing the sync method "too often" - if it proves expensive, we're going to have to encourage users to actively scan for and fix their topics rather than rely on the dynamic fixup.

I have in mind a prefs editor (actually a generic META datum editor) from some other work I'm doing, but I don't want to include that into this proposal.

Kenneth's idea sounds reasonable, I'll have to think about the best way to do it (honestly the oops/redirect mechanism bothers me, but that's what we use).

-- PaulHarvey - 18 Dec 2010

I think my concerns have been addressed above, with one exception, and that's that I'm not clear how this works from a user-interface perspective. The code changes above don't mention UI, yet my gut tells me there is some sort of interaction required to tell users why their meta-pref overrides their set-pref, and that interaction is much more important than the internal code changes.

Looking at all this from a "DB cache" perspective, I had hoped to move as much as possible of the prefs code behind the store interface so that it can be short-circuited by a cache (or looked up more efficiently in a DB store). I'm not currently seeing anything above that makes that impossible, but it needs to be checked.

-- CrawfordCurrie - 18 Dec 2010

Would it be possible to render a warning note for each * Set if that is overridden by meta? (possible extend this for * Sets that are redefined). It could be displayed in a topic info 'box' at the bottom of the page.

-- ArthurClemens - 21 Jan 2011

The only case this would be necessary is if both meta and Set are changed in the same save request. In all other situations, this proposal is to make an implementation that can decide which one of the conflicting prefs is the newest, and then synchronise them.

-- PaulHarvey - 21 Jan 2011

Then it would be necessary to show/explain what happened to the older * Set pref that is now overridden by a newer (invisible) meta.

-- ArthurClemens - 22 Jan 2011

But the * Set wouldn't be overridden... it'd be the same as the META:PREFERENCE

-- PaulHarvey - 22 Jan 2011

Sorry, I don't understand the issue then. Could you describe in a scenario what happens to a * Set statement?

-- ArthurClemens - 22 Jan 2011

this would be a very neat and good fix for Tasks.Item2386

-- SvenDowideit - 17 Mar 2011

I added Sven as a concern raiser, for the use-case where external processes are updating in-line * Set statements on the .txt files directly.

As I see it, here are the problems we are trying to solve:
  • In-line set statements are not queryable
  • In-line set statements should never contradict their META:PREFERENCE equivalent, if it exists, and vice versa.

The current proposal has a problem. It automatically adds META:PREFERENCE for every in-line set statement. But this means an external process updating the .txt file directly, if the in-line set statement is changed, that change will be ignored (META:PREFERENCE has priority in the prefs hierarchy).

So I think there are two ways forward:
  1. Enhance the store API to detect this condition (working copy is different to latest revision)
  2. Where only an in-line Set statement exists, add a new cached="1" key on the META:PREFERENCE datum to indicate that it should NOT be included in the prefs hierarchy

-- PaulHarvey - 08 Jun 2011

Of course, this adds some complexity when you want to promote the META:PREFERENCE to become equal citizen to the in-line pref. I think this promotion should happen whenever the pref is changed via its META version.

-- PaulHarvey - 08 Jun 2011

Perhaps keeping the two in sync is not the way the go. I am just thinking loud now.

How about a UI indication that the topic contains a Set statement which sets the same macro as the META pref?

  • Would such a UI indication be unwanted because some authors do want a mismatch in some cases?
  • Would an indication mean that the end user would learn and react by either removing the in-topic or the hidden setting in meta?

This could be considered instead. It is simpler to do. The indication could be limited to when you have just saved the topic and when you start editing a topic. This way it will not disturb the readers that should not care.

-- KennethLavrsen - 08 Jun 2011

Yeah, after a chat with Sven today, I think 80% of this proposal is a waste of time.

Specifically, we want to avoid redundant/duplicatation of in-line prefs into META.

We discussed other ways of presenting a consistent "TOM model" for preferences, maybe adjusting get/setEmbeddedStorForm (well, that idea is broken, more thinking needed there), or something.

I plan on re-writing this proposal at some point, once I've discussed with Sven & Crawford how to better architect a solution (which means separate proposals for some of the aspects outlined here).

-- PaulHarvey - 08 Jun 2011

Item10160 had this marked for 1.2. Deferring to 2.0

-- GeorgeClark - 02 Jun 2014
 
Topic revision: r23 - 02 Jun 2014, GeorgeClark - This page was cached on 16 Jan 2020 - 14:01.

The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy