diff --git a/YKanBan.Tests/Search/SearchQueryParserTests.cs b/YKanBan.Tests/Search/SearchQueryParserTests.cs index d17ce04..87fa18f 100644 --- a/YKanBan.Tests/Search/SearchQueryParserTests.cs +++ b/YKanBan.Tests/Search/SearchQueryParserTests.cs @@ -147,9 +147,10 @@ public class SearchQueryParserTests } [TestMethod] - public void QualifierValueMayContainColons() + public void QuotedQualifierValueMayContainColonsAndOperators() { - AssertParsesTo(new SearchNode.TagName("a:b"), "tag:a:b"); + AssertParsesTo(new SearchNode.TagName("a:b"), "tag:\"a:b\""); + AssertParsesTo(new SearchNode.TagName("AND"), "tag:\"AND\""); } [TestMethod] @@ -169,13 +170,20 @@ public class SearchQueryParserTests public void IdQualifierParsesNumber() { AssertParsesTo(new SearchNode.CardId(42), "id:42"); - AssertParsesTo(new SearchNode.CardId(12), "id:\"12\""); + AssertParsesTo(new SearchNode.CardId(61), "id:#61"); } [TestMethod] - public void ColonWithoutKeyPrefixIsFreeText() + public void QuotedTextWithColonIsFreeText() { - AssertParsesTo(new SearchNode.FreeText(":foo"), ":foo"); + AssertParsesTo(new SearchNode.FreeText("http://x"), "\"http://x\""); + } + + [TestMethod] + public void WordMayContainHashAndPunctuation() + { + AssertParsesTo(new SearchNode.FreeText("#61"), "#61"); + AssertParsesTo(new SearchNode.FreeText("c#/.net"), "c#/.net"); } #endregion @@ -262,9 +270,58 @@ public class SearchQueryParserTests } [TestMethod] - public void NonNumericIdIsAnError() + [DataRow("id:12x")] + [DataRow("id:abc")] + [DataRow("id:0")] + [DataRow("id:-1")] + [DataRow("id:+1")] + [DataRow("id:#")] + [DataRow("id:##1")] + [DataRow("id:1.5")] + [DataRow("id:٣")] + [DataRow("id:99999999999999999999")] + public void InvalidIdValueIsAnError(string text) { - AssertSyntaxError("id:12x"); + AssertSyntaxError(text); + } + + [TestMethod] + public void QuotedIdValueIsAnError() + { + AssertSyntaxError("id:\"61\""); + } + + [TestMethod] + [DataRow("tag:AND")] + [DataRow("tag:OR")] + [DataRow("title:AND")] + public void BareOperatorAsQualifierValueIsAnError(string text) + { + AssertSyntaxError(text); + } + + [TestMethod] + public void UnquotedQualifierValueWithColonIsAnError() + { + AssertSyntaxError("tag:a:b"); + } + + [TestMethod] + [DataRow(":foo")] + [DataRow("http://x")] + [DataRow("a:")] + public void ColonAfterNonKeyIsAnError(string text) + { + AssertSyntaxError(text); + } + + [TestMethod] + [DataRow("a\\b")] + [DataRow("\\")] + [DataRow("tag:a\\b")] + public void BackslashInBareWordIsAnError(string text) + { + AssertSyntaxError(text); } [TestMethod] diff --git a/YKanBan.Tests/Search/SearchSqlCompilerTests.cs b/YKanBan.Tests/Search/SearchSqlCompilerTests.cs index 836b4f5..9a81a1d 100644 --- a/YKanBan.Tests/Search/SearchSqlCompilerTests.cs +++ b/YKanBan.Tests/Search/SearchSqlCompilerTests.cs @@ -169,10 +169,25 @@ public class SearchSqlCompilerTests { CompiledSearch compiled = SearchSqlCompiler.Compile(new SearchNode.FreeText("x")); + // One folded parameter shared by the title and content tests. StringAssert.Contains(compiled.Predicate, "$p0"); - StringAssert.Contains(compiled.Predicate, "$p1"); - Assert.AreEqual(2, compiled.Parameters.Count); - Assert.AreEqual("%x%", compiled.Parameters[0].Value); - Assert.AreEqual("%x%", compiled.Parameters[1].Value); + Assert.AreEqual(1, compiled.Parameters.Count); + Assert.AreEqual("x", compiled.Parameters[0].Value); + } + + [TestMethod] + public void MatchingFoldsNonAsciiCase() + { + using var fixture = new SearchFixture(); + SqliteTestHelper.Exec(fixture.Connection, "INSERT INTO columns (title, description, created_at, updated_at) VALUES ('Érable', '', 1, 1);"); + SqliteTestHelper.Exec(fixture.Connection, "INSERT INTO cards (column_id, title, content, created_at, updated_at) VALUES (3, 'Ärger', 'Straße ΣΟΦΙΑ', 1, 1);"); + SqliteTestHelper.Exec(fixture.Connection, "INSERT INTO tags (name, color, description) VALUES ('Äpfel', '#123456', '');"); + SqliteTestHelper.Exec(fixture.Connection, "INSERT INTO card_tags (card_id, tag_id) VALUES (5, 4);"); + + CollectionAssert.AreEqual(new[] { 5L }, fixture.Search("ärger")); + CollectionAssert.AreEqual(new[] { 5L }, fixture.Search("title:ÄRGER")); + CollectionAssert.AreEqual(new[] { 5L }, fixture.Search("content:σοφια")); + CollectionAssert.AreEqual(new[] { 5L }, fixture.Search("tag:äpfel")); + CollectionAssert.AreEqual(new[] { 5L }, fixture.Search("column:érable")); } } diff --git a/YKanBan/Search/SearchLexer.cs b/YKanBan/Search/SearchLexer.cs index 0da560b..cce642d 100644 --- a/YKanBan/Search/SearchLexer.cs +++ b/YKanBan/Search/SearchLexer.cs @@ -35,8 +35,8 @@ public abstract record SearchToken /// quoted phrases and parentheses; whitespace only separates tokens. /// /// The lexer is strict: inside a phrase only \" and \\ are valid -/// escapes, a bare backslash or an unknown escape is rejected, and an -/// unterminated phrase is rejected. Any input not covered by the token rules +/// escapes, a bare backslash or an unknown escape is rejected, a backslash in a +/// bare word is rejected, and an unterminated phrase is rejected. Any input not covered by the token rules /// is rejected rather than silently ignored. /// public static class SearchLexer @@ -62,9 +62,11 @@ public static class SearchLexer private static readonly Parser CloseParenParser = Parser.Char(')').Select(_ => (SearchToken)new SearchToken.CloseParen()); + // ':' stays inside the word token (the parser splits qualifiers); a backslash + // is not a word character, so a bare one is left over and rejected by End. private static readonly Parser WordParser = Parser.Token(character => - !char.IsWhiteSpace(character) && character != '(' && character != ')' && character != '"') + !char.IsWhiteSpace(character) && character != '(' && character != ')' && character != '"' && character != '\\') .AtLeastOnce() .Select(chars => (SearchToken)new SearchToken.Word(new string(chars.ToArray()))); diff --git a/YKanBan/Search/SearchQueryParser.cs b/YKanBan/Search/SearchQueryParser.cs index 47b8054..b020c72 100644 --- a/YKanBan/Search/SearchQueryParser.cs +++ b/YKanBan/Search/SearchQueryParser.cs @@ -6,8 +6,10 @@ namespace YKanBan.Search; /// /// Parses the search grammar with Pidgin, strictly: any malformed input /// (unbalanced parentheses, dangling or consecutive operators, unterminated -/// phrases, invalid escapes, qualifier keys without a value or with an -/// unknown/non-numeric id value) raises . +/// phrases, invalid escapes, a backslash in a bare word, unknown qualifier +/// keys, qualifiers without a value, unquoted values containing ':' or equal +/// to AND/OR, and id values that are not an unquoted positive integer with an +/// optional '#') raises . /// /// Only uppercase AND/OR are operators (lowercase ones are plain words); AND — /// explicit or by adjacency — binds tighter than OR; parentheses group. Blank @@ -131,55 +133,86 @@ public static class SearchQueryParser private static Parser QualifierFrom(string word) { int colonIndex = word.IndexOf(':'); - - // Only a leading run of letters/digits/underscores before ':' can be a qualifier key. - if (colonIndex <= 0 || !IsQualifierKeyCandidate(word.AsSpan(0, colonIndex))) + if (colonIndex < 0) { return Parser.Return(new SearchNode.FreeText(word)); } + // Any ':' in a bare word makes it a qualifier, so the text before it must be a known key. string key = word[..colonIndex]; string value = word[(colonIndex + 1)..]; if (!TryLookupQualifier(key, out QualifierKind kind)) { - return Parser.Fail($"Unknown qualifier key '{key}'."); + return Parser.Fail($"Unknown qualifier key '{key}'; quote the text to search for it literally."); } // "key:" without an inline value takes the value from the following phrase, e.g. tag:"a b". - return value.Length == 0 - ? PhraseText.Bind(text => AtomParser(kind, text)) - : AtomParser(kind, value); + if (value.Length == 0) + { + return kind == QualifierKind.Id + ? Parser.Fail("The id qualifier expects an unquoted positive integer.") + : PhraseText.Select(text => TextAtom(kind, text)); + } + + if (value.Contains(':')) + { + return Parser.Fail($"Unquoted value '{value}' of qualifier '{key}' contains ':'; quote the value."); + } + + if (value is "AND" or "OR") + { + return Parser.Fail($"Operator {value} cannot be a qualifier value; quote it to search for it literally."); + } + + if (kind == QualifierKind.Id) + { + return TryParseCardId(value, out long id) + ? Parser.Return(new SearchNode.CardId(id)) + : Parser.Fail($"The id qualifier expects a positive integer, got '{value}'."); + } + + return Parser.Return(TextAtom(kind, value)); } /// - /// Builds the atom parser for a qualifier value, failing strictly on a non-numeric id. + /// Builds the atom for a textual qualifier value. /// - /// The qualifier kind. + /// The qualifier kind; never . /// The qualifier value. - /// The parser producing the atom node. - private static Parser AtomParser(QualifierKind kind, string value) => - AtomFrom(kind, value) is { } atom - ? Parser.Return(atom) - : Parser.Fail($"The id qualifier expects an integer, got '{value}'."); - - /// - /// Builds the atom for a qualifier value; null when an id value is not an integer. - /// - /// The qualifier kind. - /// The qualifier value. - /// The atom node, or for an invalid id value. - private static SearchNode? AtomFrom(QualifierKind kind, string value) => kind switch + /// The atom node. + private static SearchNode TextAtom(QualifierKind kind, string value) => kind switch { QualifierKind.Tag => new SearchNode.TagName(value), QualifierKind.Title => new SearchNode.FieldText(SearchField.Title, value), QualifierKind.Content => new SearchNode.FieldText(SearchField.Content, value), QualifierKind.Column => new SearchNode.ColumnTitle(value), - QualifierKind.Id when long.TryParse(value, NumberStyles.Integer, CultureInfo.InvariantCulture, out long id) => - new SearchNode.CardId(id), - _ => null, + _ => throw new InvalidOperationException($"Qualifier kind {kind} has no textual value."), }; + /// + /// Parses an id qualifier value: decimal digits with an optional leading + /// #, denoting a positive integer. + /// + /// The inline qualifier value. + /// The parsed card id. + /// when the value is a valid positive card id. + private static bool TryParseCardId(string value, out long id) + { + ReadOnlySpan digits = value.AsSpan(); + if (digits.StartsWith("#")) + { + digits = digits[1..]; + } + + // Digits only: the span check rules out signs, whitespace and other NumberStyles leniency. + id = 0; + return digits.Length > 0 + && !digits.ContainsAnyExceptInRange('0', '9') + && long.TryParse(digits, NumberStyles.None, CultureInfo.InvariantCulture, out id) + && id > 0; + } + /// /// Looks up a qualifier key in the extension table with ordinal (case-sensitive) matching. /// @@ -201,25 +234,6 @@ public static class SearchQueryParser return false; } - /// - /// Returns whether the text looks like a qualifier key: a non-empty run of - /// letters, digits or underscores. - /// - /// The candidate key text. - /// when every character is a word character. - private static bool IsQualifierKeyCandidate(ReadOnlySpan text) - { - foreach (char character in text) - { - if (!(char.IsLetterOrDigit(character) || character == '_')) - { - return false; - } - } - - return true; - } - /// /// Collapses an AND chain into a single node. /// diff --git a/YKanBan/Search/SearchSqlCompiler.cs b/YKanBan/Search/SearchSqlCompiler.cs index 95530fe..ac26c46 100644 --- a/YKanBan/Search/SearchSqlCompiler.cs +++ b/YKanBan/Search/SearchSqlCompiler.cs @@ -1,3 +1,5 @@ +using YKanBan.Storage; + namespace YKanBan.Search; /// @@ -10,10 +12,11 @@ public sealed record CompiledSearch(string Predicate, IReadOnlyList<(string Name /// /// Compiles a parsed search AST into a parameterized SQL predicate. Matching is -/// case-insensitive with SQLite's built-in semantics: LIKE's default case -/// folding for substring matches plus COLLATE NOCASE for exact matches. LIKE -/// wildcards and the escape character in user text are escaped so they match -/// literally. +/// Unicode case-insensitive: both sides are folded with +/// (the column side through the per-connection +/// function) and then compared +/// ordinally — instr for substrings, = for exact matches. LIKE is +/// deliberately not used, so % and _ carry no wildcard meaning. /// public static class SearchSqlCompiler { @@ -48,29 +51,28 @@ public static class SearchSqlCompiler case SearchNode.FreeText freeText: { // A bare word matches a substring of the title OR the content. - string titleParam = AddParameter(parameters, "%" + EscapeLike(freeText.Text) + "%"); - string contentParam = AddParameter(parameters, "%" + EscapeLike(freeText.Text) + "%"); - return $"(c.title LIKE {titleParam} ESCAPE '\\' OR c.content LIKE {contentParam} ESCAPE '\\')"; + string param = AddParameter(parameters, SqliteDatabase.Fold(freeText.Text)); + return $"({Contains("c.title", param)} OR {Contains("c.content", param)})"; } case SearchNode.FieldText field: { string column = field.Field == SearchField.Title ? "c.title" : "c.content"; - string param = AddParameter(parameters, "%" + EscapeLike(field.Text) + "%"); - return $"{column} LIKE {param} ESCAPE '\\'"; + string param = AddParameter(parameters, SqliteDatabase.Fold(field.Text)); + return Contains(column, param); } case SearchNode.TagName tag: { - string param = AddParameter(parameters, tag.Name); + string param = AddParameter(parameters, SqliteDatabase.Fold(tag.Name)); return "EXISTS (SELECT 1 FROM card_tags ct JOIN tags t ON t.id = ct.tag_id " + - $"WHERE ct.card_id = c.id AND t.name COLLATE NOCASE = {param})"; + $"WHERE ct.card_id = c.id AND {Folded("t.name")} = {param})"; } case SearchNode.ColumnTitle column: { - string param = AddParameter(parameters, column.Title); - return $"c.column_id IN (SELECT id FROM columns WHERE title COLLATE NOCASE = {param})"; + string param = AddParameter(parameters, SqliteDatabase.Fold(column.Title)); + return $"c.column_id IN (SELECT id FROM columns WHERE {Folded("title")} = {param})"; } case SearchNode.CardId id: @@ -98,10 +100,17 @@ public static class SearchSqlCompiler } /// - /// Escapes LIKE wildcards and the escape character inside user text. + /// Wraps a column expression in the case-folding SQL function. /// - /// The raw user text. - /// The text with \, % and _ escaped. - private static string EscapeLike(string text) => - text.Replace("\\", "\\\\").Replace("%", "\\%").Replace("_", "\\_"); + /// The column expression. + /// The folded column expression. + private static string Folded(string column) => $"{SqliteDatabase.FoldFunctionName}({column})"; + + /// + /// Builds an ordinal substring test of a folded column against an already folded parameter. + /// + /// The column expression. + /// The placeholder of the folded search text. + /// The SQL boolean expression. + private static string Contains(string column, string param) => $"instr({Folded(column)}, {param}) > 0"; } diff --git a/YKanBan/Storage/SqliteDatabase.cs b/YKanBan/Storage/SqliteDatabase.cs index 0b5d973..f9ae236 100644 --- a/YKanBan/Storage/SqliteDatabase.cs +++ b/YKanBan/Storage/SqliteDatabase.cs @@ -5,10 +5,25 @@ namespace YKanBan.Storage; /// /// Shared SQLite plumbing for workspace databases: opens a connection with /// the mandated PRAGMA configuration (WAL journal, NORMAL synchronous mode, -/// foreign keys on) and applies incremental user_version migrations. +/// foreign keys on), registers the custom SQL functions and applies +/// incremental user_version migrations. /// public static class SqliteDatabase { + /// + /// Name of the per-connection SQL function that applies ; + /// search uses it for Unicode case-insensitive matching. + /// + public const string FoldFunctionName = "ykb_fold"; + + /// + /// Unicode case folding shared by the SQL function and the search + /// parameters it is compared against; comparisons after folding are ordinal. + /// + /// The text to fold. + /// The invariant lowercase form. + public static string Fold(string text) => text.ToLowerInvariant(); + /// /// Opens (creating if needed) a database file, applies the mandatory /// PRAGMAs and runs any pending migrations. @@ -31,6 +46,7 @@ public static class SqliteDatabase try { ApplyPragmas(connection); + RegisterFunctions(connection); ApplyMigrations(connection, migrations); return connection; } @@ -67,6 +83,18 @@ public static class SqliteDatabase Execute(connection, "PRAGMA foreign_keys=ON;"); } + /// + /// Registers the custom SQL functions; like the PRAGMAs they live on the + /// connection and must be registered on every one. + /// + /// An open database connection. + internal static void RegisterFunctions(SqliteConnection connection) + { + // NULL never reaches it from the NOT NULL text columns, but stays NULL-safe regardless. + connection.CreateFunction( + FoldFunctionName, text => text is null ? null : Fold(text), isDeterministic: true); + } + /// /// Runs every migration newer than the stored user_version, each in /// its own transaction together with its version bump.