commit 61f8a2c3c5097f1469142d32d06c28b2eb4aa129
parent 36371630b56b2ce79b6587b57d9f64b42ffc6537
Author: Dan Stillman <dstillman@zotero.org>
Date: Wed, 26 Jul 2017 05:35:58 -0400
Fix various problems with fulltextContent searches
Including finding items in the wrong library and not finding any items
when paired with the checkboxes in ANY mode
Diffstat:
2 files changed, 114 insertions(+), 33 deletions(-)
diff --git a/chrome/content/zotero/xpcom/data/search.js b/chrome/content/zotero/xpcom/data/search.js
@@ -649,16 +649,21 @@ Zotero.Search.prototype.search = Zotero.Promise.coroutine(function* (asTempTable
};
// Regexp mode -- don't use fulltext word index
- if (condition.mode && condition.mode.indexOf('regexp') == 0) {
- // In an ANY search, only bother scanning items that
- // haven't already been found by the main search
- if (joinMode == 'any') {
+ if (condition.mode && condition.mode.startsWith('regexp')) {
+ // In an ANY search with other conditions that alter the results, only bother
+ // scanning items that haven't already been found by the main search, as long as
+ // they're in the right library
+ if (joinMode == 'any' && this._hasPrimaryConditions) {
if (!tmpTable) {
tmpTable = yield Zotero.Search.idsToTempTable(ids);
}
var sql = "SELECT GROUP_CONCAT(itemID) FROM items WHERE "
+ "itemID NOT IN (SELECT itemID FROM " + tmpTable + ")";
+ if (this.libraryID) {
+ sql += " AND libraryID=" + parseInt(this.libraryID);
+ }
+
var res = yield Zotero.DB.valueQueryAsync(sql);
var scopeIDs = res ? res.split(",").map(id => parseInt(id)) : [];
}
@@ -672,6 +677,9 @@ Zotero.Search.prototype.search = Zotero.Promise.coroutine(function* (asTempTable
else {
Zotero.debug('Running subsearch against fulltext word index');
var s = new Zotero.Search();
+ if (this.libraryID) {
+ s.libraryID = this.libraryID;
+ }
// Add any necessary conditions to the fulltext word search --
// those that are required in an ANY search and any outside the
@@ -748,8 +756,7 @@ Zotero.Search.prototype.search = Zotero.Promise.coroutine(function* (asTempTable
//
// We only do this if there are primary conditions that alter the
// main search, since otherwise all items will match
- if (this._hasPrimaryConditions &&
- (joinMode == 'any' || hasQuicksearch) && ids) {
+ if (this._hasPrimaryConditions && (joinMode == 'any' || hasQuicksearch)) {
//Zotero.debug("Adding filtered IDs to main set");
for (let i=0; i<filteredIDs.length; i++) {
let id = filteredIDs[i];
@@ -900,13 +907,21 @@ Zotero.Search.idsToTempTable = Zotero.Promise.coroutine(function* (ids) {
var tmpTable = "tmpSearchResults_" + Zotero.randomString(8);
Zotero.debug(`Creating ${tmpTable} with ${ids.length} item${ids.length != 1 ? 's' : ''}`);
- var sql = "CREATE TEMPORARY TABLE " + tmpTable + " AS "
+ var sql = "CREATE TEMPORARY TABLE " + tmpTable;
+ if (ids.length) {
+ sql += " AS "
+ "WITH cte(itemID) AS ("
+ "VALUES " + ids.map(id => "(" + parseInt(id) + ")").join(',')
+ ") "
+ "SELECT * FROM cte";
+ }
+ else {
+ sql += " (itemID INTEGER PRIMARY KEY)";
+ }
yield Zotero.DB.queryAsync(sql, false, { debug: false });
- yield Zotero.DB.queryAsync(`CREATE UNIQUE INDEX ${tmpTable}_itemID ON ${tmpTable}(itemID)`);
+ if (ids.length) {
+ yield Zotero.DB.queryAsync(`CREATE UNIQUE INDEX ${tmpTable}_itemID ON ${tmpTable}(itemID)`);
+ }
return tmpTable;
});
diff --git a/test/tests/searchTest.js b/test/tests/searchTest.js
@@ -116,9 +116,12 @@ describe("Zotero.Search", function() {
});
describe("#search()", function () {
- let win;
- let fooItem;
- let foobarItem;
+ var win;
+ var userLibraryID;
+ var fooItem;
+ var foobarItem;
+ var fooItemGroup;
+ var foobarItemGroup;
before(function* () {
// Hidden browser, which requires a browser window, needed for charset detection
@@ -126,6 +129,11 @@ describe("Zotero.Search", function() {
win = yield loadBrowserWindow();
fooItem = yield importFileAttachment("search/foo.html");
foobarItem = yield importFileAttachment("search/foobar.html");
+ userLibraryID = fooItem.libraryID;
+
+ let group = yield getGroup();
+ fooItemGroup = yield importFileAttachment("search/foo.html", { libraryID: group.libraryID });
+ foobarItemGroup = yield importFileAttachment("search/foobar.html", { libraryID: group.libraryID });
});
after(function* () {
@@ -134,31 +142,11 @@ describe("Zotero.Search", function() {
}
yield fooItem.eraseTx();
yield foobarItem.eraseTx();
+ yield fooItemGroup.eraseTx();
+ yield foobarItemGroup.eraseTx();
});
describe("Conditions", function () {
- describe("attachmentContent", function () {
- it("should find text in HTML files", function* () {
- var s = new Zotero.Search();
- s.libraryID = foobarItem.libraryID;
- s.addCondition('fulltextContent', 'contains', 'foo bar');
- var matches = yield s.search();
- assert.sameMembers(matches, [foobarItem.id]);
- });
-
- it("should work in subsearch", function* () {
- var s = new Zotero.Search();
- s.libraryID = foobarItem.libraryID;
- s.addCondition('fulltextContent', 'contains', 'foo bar');
-
- var s2 = new Zotero.Search();
- s2.setScope(s);
- s2.addCondition('title', 'contains', 'foobar');
- var matches = yield s2.search();
- assert.sameMembers(matches, [foobarItem.id]);
- });
- });
-
describe("collection", function () {
it("should find item in collection", function* () {
var col = yield createDataObject('collection');
@@ -210,15 +198,89 @@ describe("Zotero.Search", function() {
describe("fileTypeID", function () {
it("should search by attachment file type", function* () {
let s = new Zotero.Search();
+ s.libraryID = userLibraryID;
s.addCondition('fileTypeID', 'is', Zotero.FileTypes.getID('webpage'));
let matches = yield s.search();
assert.sameMembers(matches, [fooItem.id, foobarItem.id]);
});
});
+ describe("fulltextContent", function () {
+ it("should find text in HTML files", function* () {
+ var s = new Zotero.Search();
+ s.libraryID = userLibraryID;
+ s.addCondition('fulltextContent', 'contains', 'foo bar');
+ var matches = yield s.search();
+ assert.sameMembers(matches, [foobarItem.id]);
+ });
+
+ it("should work in subsearch", function* () {
+ var s = new Zotero.Search();
+ s.libraryID = userLibraryID;
+ s.addCondition('fulltextContent', 'contains', 'foo bar');
+
+ var s2 = new Zotero.Search();
+ s2.setScope(s);
+ s2.addCondition('title', 'contains', 'foobar');
+ var matches = yield s2.search();
+ assert.sameMembers(matches, [foobarItem.id]);
+ });
+
+ it("should find matching item with joinMode=ANY and non-matching other condition", function* () {
+ var s = new Zotero.Search();
+ s.libraryID = userLibraryID;
+ s.addCondition('joinMode', 'any');
+ s.addCondition('fulltextContent', 'contains', 'foo bar');
+ s.addCondition('title', 'contains', 'nomatch');
+ var matches = yield s.search();
+ assert.sameMembers(matches, [foobarItem.id]);
+ });
+
+ it("should find matching items in regexp mode with joinMode=ANY with matching other condition", function* () {
+ var s = new Zotero.Search();
+ s.libraryID = userLibraryID;
+ s.addCondition('joinMode', 'any');
+ s.addCondition('fulltextContent/regexp', 'contains', 'foo.+bar');
+ s.addCondition('title', 'is', fooItem.getField('title'));
+ var matches = yield s.search();
+ assert.sameMembers(matches, [fooItem.id, foobarItem.id]);
+ });
+
+ it("should find matching item in regexp mode with joinMode=ANY and non-matching other condition", function* () {
+ var s = new Zotero.Search();
+ s.libraryID = userLibraryID;
+ s.addCondition('joinMode', 'any');
+ s.addCondition('fulltextContent/regexp', 'contains', 'foo.+bar');
+ s.addCondition('title', 'contains', 'nomatch');
+ var matches = yield s.search();
+ assert.sameMembers(matches, [foobarItem.id]);
+ });
+
+ it("should find item matching other condition in regexp mode when joinMode=ANY", function* () {
+ var s = new Zotero.Search();
+ s.libraryID = userLibraryID;
+ s.addCondition('joinMode', 'any');
+ s.addCondition('fulltextContent/regexp', 'contains', 'nomatch');
+ s.addCondition('title', 'is', foobarItem.getField('title'));
+ var matches = yield s.search();
+ assert.sameMembers(matches, [foobarItem.id]);
+ });
+
+ it("should find matching item in regexp mode with joinMode=ANY and recursive mode flag", function* () {
+ var s = new Zotero.Search();
+ s.libraryID = userLibraryID;
+ s.addCondition('joinMode', 'any');
+ s.addCondition('fulltextContent/regexp', 'contains', 'foo.+bar');
+ s.addCondition('recursive', 'true');
+ var matches = yield s.search();
+ assert.sameMembers(matches, [foobarItem.id]);
+ });
+ });
+
describe("fulltextWord", function () {
it("should return matches with full-text conditions", function* () {
let s = new Zotero.Search();
+ s.libraryID = userLibraryID;
s.addCondition('fulltextWord', 'contains', 'foo');
let matches = yield s.search();
assert.lengthOf(matches, 2);
@@ -227,6 +289,7 @@ describe("Zotero.Search", function() {
it("should not return non-matches with full-text conditions", function* () {
let s = new Zotero.Search();
+ s.libraryID = userLibraryID;
s.addCondition('fulltextWord', 'contains', 'baz');
let matches = yield s.search();
assert.lengthOf(matches, 0);
@@ -234,6 +297,7 @@ describe("Zotero.Search", function() {
it("should return matches for full-text conditions in ALL mode", function* () {
let s = new Zotero.Search();
+ s.libraryID = userLibraryID;
s.addCondition('joinMode', 'all');
s.addCondition('fulltextWord', 'contains', 'foo');
s.addCondition('fulltextWord', 'contains', 'bar');
@@ -243,6 +307,7 @@ describe("Zotero.Search", function() {
it("should not return non-matches for full-text conditions in ALL mode", function* () {
let s = new Zotero.Search();
+ s.libraryID = userLibraryID;
s.addCondition('joinMode', 'all');
s.addCondition('fulltextWord', 'contains', 'mjktkiuewf');
s.addCondition('fulltextWord', 'contains', 'zijajkvudk');
@@ -252,6 +317,7 @@ describe("Zotero.Search", function() {
it("should return a match that satisfies only one of two full-text condition in ANY mode", function* () {
let s = new Zotero.Search();
+ s.libraryID = userLibraryID;
s.addCondition('joinMode', 'any');
s.addCondition('fulltextWord', 'contains', 'bar');
s.addCondition('fulltextWord', 'contains', 'baz');
