Fix for index ordering issue

Fixes #4082

This version removes selective updating of the tag index, instead completely clearing the index on each update. I'm investigating restoring that optimisation.

boot/boot.js

@@ -1063,18 +1063,35 @@ $tw.Wiki = function(options) {
 		if(tiddler) {
 			var title = tiddler.fields.title;
 			if(title) {
-				var oldTiddler = this.getTiddler(title);
 // Uncomment the following line for detailed logs of all tiddler writes
 // console.log("Adding",title,tiddler)
+				// Record the old tiddler state
+				var updateDescriptor = {
+					old: {
+						tiddler: this.getTiddler(title),
+						shadow: this.isShadowTiddler(title),
+						exists: this.tiddlerExists(title)
+					}
+				}
+				// Save the new tiddler
 				tiddlers[title] = tiddler;
+				// Check we've got it's title
 				if(tiddlerTitles && tiddlerTitles.indexOf(title) === -1) {
 					tiddlerTitles.push(title);
 				}
+				// Record the new tiddler state
+				updateDescriptor["new"] = {
+					tiddler: tiddler,
+					shadow: this.isShadowTiddler(title),
+					exists: this.tiddlerExists(title)
+				}
+				// Update indexes
 				this.clearCache(title);
 				this.clearGlobalCache();
 				$tw.utils.each(indexers,function(indexer) {
-					indexer.update(oldTiddler,tiddler);
+					indexer.update(updateDescriptor);
 				});
+				// Queue a change event
 				this.enqueueTiddlerEvent(title);
 			}
 		}
@@ -1085,20 +1102,36 @@ $tw.Wiki = function(options) {
 // Uncomment the following line for detailed logs of all tiddler deletions
 // console.log("Deleting",title)
 		if($tw.utils.hop(tiddlers,title)) {
-			var oldTiddler = this.getTiddler(title);
+			// Record the old tiddler state
+			var updateDescriptor = {
+				old: {
+					tiddler: this.getTiddler(title),
+					shadow: this.isShadowTiddler(title),
+					exists: this.tiddlerExists(title)
+				}
+			}
+			// Delete the tiddler
 			delete tiddlers[title];
+			// Delete it from the list of titles
 			if(tiddlerTitles) {
 				var index = tiddlerTitles.indexOf(title);
 				if(index !== -1) {
 					tiddlerTitles.splice(index,1);
 				}				
 			}
+			// Record the new tiddler state
+			updateDescriptor["new"] = {
+				tiddler: this.getTiddler(title),
+				shadow: this.isShadowTiddler(title),
+				exists: this.tiddlerExists(title)
+			}
+			// Update indexes
 			this.clearCache(title);
 			this.clearGlobalCache();
-			var newTiddler = this.getTiddler(title);
 			$tw.utils.each(indexers,function(indexer) {
-				indexer.update(oldTiddler,newTiddler);
+				indexer.update(updateDescriptor);
 			});
+			// Queue a change event
 			this.enqueueTiddlerEvent(title,true);
 		}
 	};

core/modules/indexers/field-indexer.js

@@ -88,23 +88,22 @@ FieldIndexer.prototype.buildIndexForField = function(name) {
 
 /*
 Update the index in the light of a tiddler value changing; note that the title must be identical. (Renames are handled as a separate delete and create)
-oldTiddler: old tiddler value, or null for creation
+updateDescriptor: {old: {tiddler: <tiddler>, shadow: <boolean>, exists: <boolean>},new: {tiddler: <tiddler>, shadow: <boolean>, exists: <boolean>}}
-newTiddler: new tiddler value, or null for deletion
 */
-FieldIndexer.prototype.update = function(oldTiddler,newTiddler) {
+FieldIndexer.prototype.update = function(updateDescriptor) {
 	var self = this;
 	// Don't do anything if the index hasn't been built yet
 	if(this.index === null) {
 		return;
 	}
 	// Remove the old tiddler from the index
-	if(oldTiddler) {
+	if(updateDescriptor.old.tiddler) {
 		$tw.utils.each(this.index,function(indexEntry,name) {
-			if(name in oldTiddler.fields) {
+			if(name in updateDescriptor.old.tiddler.fields) {
-				var value = oldTiddler.getFieldString(name),
+				var value = updateDescriptor.old.tiddler.getFieldString(name),
 					tiddlerList = indexEntry[value];
 				if(tiddlerList) {
-					var index = tiddlerList.indexOf(oldTiddler.fields.title);
+					var index = tiddlerList.indexOf(updateDescriptor.old.tiddler.fields.title);
 					if(index !== -1) {
 						tiddlerList.splice(index,1);
 					}
@@ -113,13 +112,13 @@ FieldIndexer.prototype.update = function(oldTiddler,newTiddler) {
 		});
 	}
 	// Add the new tiddler to the index
-	if(newTiddler) {
+	if(updateDescriptor["new"].tiddler) {
 		$tw.utils.each(this.index,function(indexEntry,name) {
-			if(name in newTiddler.fields) {
+			if(name in updateDescriptor["new"].tiddler.fields) {
-				var value = newTiddler.getFieldString(name);
+				var value = updateDescriptor["new"].tiddler.getFieldString(name);
 				if(value.length < self.maxIndexedValueLength) {
 					indexEntry[value] = indexEntry[value] || [];
-					indexEntry[value].push(newTiddler.fields.title);
+					indexEntry[value].push(updateDescriptor["new"].tiddler.fields.title);
 				}
 			}
 		});		

core/modules/indexers/tag-indexer.js

@@ -17,41 +17,48 @@ function TagIndexer(wiki) {
 }
 
 TagIndexer.prototype.init = function() {
-	this.index = null; // Hashmap of tag title to {isSorted: bool, titles: [array]}
+	this.subIndexers = [
-	this.addIndexMethods();
+		new TagSubIndexer(this,"each"),
+		new TagSubIndexer(this,"eachShadow"),
+		new TagSubIndexer(this,"eachTiddlerPlusShadows"),
+		new TagSubIndexer(this,"eachShadowPlusTiddlers")
+	];
+	$tw.utils.each(this.subIndexers,function(subIndexer) {
+		subIndexer.addIndexMethod();
+	});
+};
+
+TagIndexer.prototype.rebuild = function() {
+	$tw.utils.each(this.subIndexers,function(subIndexer) {
+		subIndexer.rebuild();
+	});
+};
+
+TagIndexer.prototype.update = function(updateDescriptor) {
+	$tw.utils.each(this.subIndexers,function(subIndexer) {
+		subIndexer.update(updateDescriptor);
+	});
+};
+
+function TagSubIndexer(indexer,iteratorMethod) {
+	this.indexer = indexer;
+	this.iteratorMethod = iteratorMethod;
+	this.index = null; // Hashmap of tag title to {isSorted: bool, titles: [array]} or null if not yet initialised
 }
 
-TagIndexer.prototype.addIndexMethods = function() {
+TagSubIndexer.prototype.addIndexMethod = function() {
 	var self = this;
-	this.wiki.each.byTag = function(tag) {
+	this.indexer.wiki[this.iteratorMethod].byTag = function(tag) {
-		var titles = self.wiki.allTitles();
-		return self.lookup(tag).filter(function(title) {
-			return titles.indexOf(title) !== -1;
-		});
-	};
-	this.wiki.eachShadow.byTag = function(tag) {
-		var titles = self.wiki.allShadowTitles();
-		return self.lookup(tag).filter(function(title) {
-			return titles.indexOf(title) !== -1;
-		});
-	};
-	this.wiki.eachTiddlerPlusShadows.byTag = function(tag) {
-		return self.lookup(tag).slice(0);
-	};
-	this.wiki.eachShadowPlusTiddlers.byTag = function(tag) {
 		return self.lookup(tag).slice(0);
 	};
 };
 
-/*
+TagSubIndexer.prototype.rebuild = function() {
-Tear down and then rebuild the index as if all tiddlers have changed
-*/
-TagIndexer.prototype.rebuild = function() {
 	var self = this;
 	// Hashmap by tag of array of {isSorted:, titles:[]}
 	this.index = Object.create(null);
 	// Add all the tags
-	this.wiki.eachTiddlerPlusShadows(function(tiddler,title) {
+	this.indexer.wiki[this.iteratorMethod](function(tiddler,title) {
 		$tw.utils.each(tiddler.fields.tags,function(tag) {
 			if(!self.index[tag]) {
 				self.index[tag] = {isSorted: false, titles: [title]};
@@ -62,55 +69,11 @@ TagIndexer.prototype.rebuild = function() {
 	});
 };
 
-/*
+TagSubIndexer.prototype.update = function(updateDescriptor) {
-Update the index in the light of a tiddler value changing; note that the title must be identical. (Renames are handled as a separate delete and create)
+	this.index = null;
-oldTiddler: old tiddler value, or null for creation
-newTiddler: new tiddler value, or null for deletion
-*/
-TagIndexer.prototype.update = function(oldTiddler,newTiddler) {
-	// Don't update the index if it has yet to be built
-	if(this.index === null) {
-		return;
-	}
-	var self = this,
-		title = oldTiddler ? oldTiddler.fields.title : newTiddler.fields.title;
-	// Handle changes to the tags
-	var oldTiddlerTags = (oldTiddler ? (oldTiddler.fields.tags || []) : []),
-		newTiddlerTags = (newTiddler ? (newTiddler.fields.tags || []) : []);
-	$tw.utils.each(oldTiddlerTags,function(oldTag) {
-		if(newTiddlerTags.indexOf(oldTag) === -1) {
-			// Deleted tag
-			var indexRecord = self.index[oldTag],
-				pos = indexRecord.titles.indexOf(title);
-			if(pos !== -1) {
-				indexRecord.titles.splice(pos,1);
-			}
-		}
-	});
-	$tw.utils.each(newTiddlerTags,function(newTag) {
-		if(oldTiddlerTags.indexOf(newTag) === -1) {
-			// New tag
-			var indexRecord = self.index[newTag];
-			if(!indexRecord) {
-				self.index[newTag] = {isSorted: false, titles: [title]};
-			} else {
-				indexRecord.titles.push(title);
-				indexRecord.isSorted = false;
-			}
-		}
-	});
-	// Handle changes to the list field of tags
-	var oldTiddlerList = (oldTiddler ? (oldTiddler.fields.list || []) : []),
-		newTiddlerList = (newTiddler ? (newTiddler.fields.list || []) : []);
-	if(!$tw.utils.isArrayEqual(oldTiddlerList,newTiddlerList)) {
-		if(self.index[title]) {
-			self.index[title].isSorted = false;			
-		}
-	}
 };
 
-// Lookup the given tag returning an ordered list of tiddler titles
+TagSubIndexer.prototype.lookup = function(tag) {
-TagIndexer.prototype.lookup = function(tag) {
 	// Update the index if it has yet to be built
 	if(this.index === null) {
 		this.rebuild();
@@ -118,8 +81,8 @@ TagIndexer.prototype.lookup = function(tag) {
 	var indexRecord = this.index[tag];
 	if(indexRecord) {
 		if(!indexRecord.isSorted) {
-			if(this.wiki.sortByList) {
+			if(this.indexer.wiki.sortByList) {
-				indexRecord.titles = this.wiki.sortByList(indexRecord.titles,tag);
+				indexRecord.titles = this.indexer.wiki.sortByList(indexRecord.titles,tag);
 			}			
 			indexRecord.isSorted = true;
 		}
@@ -129,6 +92,7 @@ TagIndexer.prototype.lookup = function(tag) {
 	}
 };
 
+
 exports.TagIndexer = TagIndexer;
 
 })();

core/modules/wiki.js

@@ -491,7 +491,7 @@ exports.getTiddlersWithTag = function(tag) {
 	// Try to use the indexer
 	var self = this,
 		tagIndexer = this.getIndexer("TagIndexer"),
-		results = tagIndexer && tagIndexer.lookup(tag);
+		results = tagIndexer && tagIndexer.subIndexers[3].lookup(tag);
 	if(!results) {
 		// If not available, perform a manual scan
 		results = this.getGlobalCache("taglist-" + tag,function() {

editions/test/tiddlers/tests/test-filters.js

@@ -224,6 +224,15 @@ function runTests(wiki) {
 		expect(wiki.filterTiddlers("[all[shadows]tag[two]sort[title]]").join(",")).toBe("$:/TiddlerFive");
 	});
 
+	it("should handle the all operator with field, has and tag operators", function() {
+		expect(wiki.filterTiddlers("[all[shadows]tag[two]]").join(",")).toBe("$:/TiddlerFive");
+		expect(wiki.filterTiddlers("[all[shadows+tiddlers]tag[two]]").join(",")).toBe("$:/TiddlerFive,$:/TiddlerTwo,Tiddler Three");
+		expect(wiki.filterTiddlers("[all[tiddlers+shadows]tag[two]]").join(",")).toBe("$:/TiddlerTwo,Tiddler Three,$:/TiddlerFive");
+		expect(wiki.filterTiddlers("[all[shadows+tiddlers]]").join(",")).toBe("$:/TiddlerFive,TiddlerSix,TiddlerSeventh,Tiddler8,$:/ShadowPlugin,TiddlerOne,$:/TiddlerTwo,Tiddler Three,a fourth tiddler,one");
+		expect(wiki.filterTiddlers("[all[tiddlers+shadows]]").join(",")).toBe("$:/ShadowPlugin,TiddlerOne,$:/TiddlerTwo,Tiddler Three,a fourth tiddler,one,$:/TiddlerFive,TiddlerSix,TiddlerSeventh,Tiddler8");
+		expect(wiki.filterTiddlers("[all[tiddlers]tag[two]]").join(",")).toBe("$:/TiddlerTwo,Tiddler Three");
+	});
+
 	it("should handle the tags operator", function() {
 		expect(wiki.filterTiddlers("[tags[]sort[title]]").join(",")).toBe("one,two");
 		expect(wiki.filterTiddlers("[[TiddlerOne]tags[]sort[title]]").join(",")).toBe("one");