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.
-
It sucks that prefs can't be queried. Horrible regex concoctions make users cry; the most baroque QuerySearch merely gives a blank stare
-
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:
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:
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
- Changes to
Foswiki::Prefs
- 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
- 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
- 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 ...)
- Provide a way for
Foswiki::Meta::syncPreferences()
to detect if there have been any pref inserts since the initial topic parse - some flag somewhere
- Changes to
Foswiki::Meta
- 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).
- 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.
- 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()
- Change
Foswiki::Meta::save()
to call syncPreferences()
prior to the call to saveAs()
- 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).
- 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
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:
-
META:
prefs override * Set
statements silently, which is a major source of confusion
- The interface for editing
META:
prefs - when you realise they exist - is clunky and horrible
- 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:
- 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
- Extracting prefs from a topic (e.g. using a SEARCH or QUERY) is hard.
- 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
- 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
-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:
- Enhance the store API to detect this condition (working copy is different to latest revision)
- 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