Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion src/SIL.LCModel.Core/Text/StringSearcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Text;
using Icu;
Expand All @@ -30,7 +31,11 @@ public enum SearchType
/// <summary>
/// Matches any words in a string.
/// </summary>
FullText
FullText,
/// <summary>
/// Matches any portion within a string.
/// </summary>
Substring
}

/// <summary>
Expand Down Expand Up @@ -120,6 +125,7 @@ public IEnumerable<T> GetItems(byte[] lower, byte[] upper)
#endregion SortKeyIndex class

private readonly Dictionary<Tuple<int, int>, SortKeyIndex> m_indices = new Dictionary<Tuple<int, int>, SortKeyIndex>();
private readonly Dictionary<Tuple<int, int>, List<KeyValuePair<T, string>>> m_rawIndices = new Dictionary<Tuple<int, int>, List<KeyValuePair<T, string>>>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish I had a suggestion, but this type is really ugly lol. I might replace KeyValuePair with a custom struct which does the same thing but is more understandable what's going on when you look at it.

Another idea (bigger tweak) is to create a custom struct (maybe named SearchBucket or IndexKey) which is a new struct that replaces all the uses of Tuple<int, int> (which is quite meaning less IMO), you would just need to match the compare and equality rules of the Tuple. This more falls in line with leaving code better than when you found it.

private readonly SearchType m_type;
private readonly Func<int, string, byte[]> m_sortKeySelector;
private readonly Func<int, string, IEnumerable<string>> m_tokenizer;
Expand Down Expand Up @@ -195,6 +201,10 @@ public void Add(T item, int indexId, int wsId, string text)
foreach (string token in RemoveWhitespaceAndPunctTokens(m_tokenizer(wsId, text)))
index.Add(m_sortKeySelector(wsId, token), item);
break;

case SearchType.Substring:
GetRawIndex(indexId, wsId).Add(new KeyValuePair<T, string>(item, text ?? string.Empty));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're handling the case of a null text argument here by using an empty string, but searching for empty strings is explicitly banned. Should we just not add a value to the index if the text is null or empty? We could probably safely do this on the first line of this method. Taking a quick look at the EntryGoSearchEngine and others it looks like they never call Add with an empty string so that's probably safe to change.

break;
}
}

Expand Down Expand Up @@ -268,6 +278,16 @@ public IEnumerable<T> Search(int indexId, int wsId, string text)
results = results == null ? items : results.Intersect(items);
}
return results;

case SearchType.Substring:
{
List<KeyValuePair<T, string>> raw;
if (!m_rawIndices.TryGetValue(Tuple.Create(indexId, wsId), out raw))
return Enumerable.Empty<T>();
CompareInfo ci = CultureInfo.InvariantCulture.CompareInfo;
return raw.Where(kv => ci.IndexOf(kv.Value, text,
CompareOptions.IgnoreCase | CompareOptions.IgnoreNonSpace) >= 0).Select(kv => kv.Key);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might need to consider if this will work correctly for us, or if we need to create a manged wrapper in icu-dotnet for string search so we can use what ICU provides for this use case.

}
}

return Enumerable.Empty<T>();
Expand All @@ -284,6 +304,7 @@ private static IEnumerable<string> RemoveWhitespaceAndPunctTokens(IEnumerable<st
public void Clear()
{
m_indices.Clear();
m_rawIndices.Clear();
}

private SortKeyIndex GetIndex(int indexId, int ws)
Expand All @@ -299,6 +320,18 @@ private SortKeyIndex GetIndex(int indexId, int ws)
return index;
}

private List<KeyValuePair<T, string>> GetRawIndex(int indexId, int ws)
{
var key = Tuple.Create(indexId, ws);
List<KeyValuePair<T, string>> list;
if (!m_rawIndices.TryGetValue(key, out list))
{
list = new List<KeyValuePair<T, string>>();
m_rawIndices[key] = list;
}
return list;
}

private static IEnumerable<Tuple<int, string>> GetWsStrings(ITsString tss)
{
var sb = new StringBuilder();
Expand Down
117 changes: 113 additions & 4 deletions tests/SIL.LCModel.Core.Tests/Text/StringSearcherTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,13 @@ public void PrefixSearchTest()
}

/// <summary>
/// Tests prefix matching.
/// Builds the shared multi-writing-system corpus used by both <see cref="FullTextSearchTest"/>
/// and <see cref="SubstringResultsIncludeAllFullTextResults"/>. Item 2 deliberately mixes a
/// French run and an English run.
/// </summary>
[Test]
public void FullTextSearchTest()
private StringSearcher<int> BuildMultiRunCorpus(SearchType type)
{
var searcher = new StringSearcher<int>(SearchType.FullText, m_wsManager);
var searcher = new StringSearcher<int>(type, m_wsManager);
searcher.Add(0, 0, TsStringUtils.MakeString("test", m_enWs));
searcher.Add(1, 0, TsStringUtils.MakeString("c'est une phrase", m_frWs));
ITsIncStrBldr tisb = TsStringUtils.MakeIncStrBldr();
Expand All @@ -100,11 +101,119 @@ public void FullTextSearchTest()
tisb.Append("We use it for testing purposes.");
searcher.Add(2, 0, tisb.GetString());
searcher.Add(3, 0, TsStringUtils.MakeString("Hello, how are you doing? I am doing fine. That is good to know.", m_enWs));
return searcher;
}

/// <summary>
/// The queries exercised by <see cref="FullTextSearchTest"/>, so the substring-superset test
/// covers exactly the same scenarios. These are all single tokens or contiguous, in-order
/// phrases, and that is deliberate: substring is a superset of full-text ONLY for those shapes
/// (full-text ANDs word tokens regardless of order, while substring needs the whole query to
/// appear contiguously). Adding an out-of-order multi-word query here would make
/// <see cref="SubstringResultsIncludeAllFullTextResults"/> fail; that boundary is demonstrated
/// by <see cref="Substring_isNotASupersetForOutOfOrderMultiWordQueries"/>.
/// </summary>
private ITsString[] FullTextQueries()
{
return new[]
{
TsStringUtils.MakeString("test", m_enWs),
TsStringUtils.MakeString("c'est une", m_frWs),
TsStringUtils.MakeString("t", m_enWs),
TsStringUtils.MakeString("testing purpose", m_enWs)
};
}

/// <summary>
/// Tests full-text (word/prefix) matching.
/// </summary>
[Test]
public void FullTextSearchTest()
{
var searcher = BuildMultiRunCorpus(SearchType.FullText);

CheckSearch(searcher, TsStringUtils.MakeString("test", m_enWs), new[] {0, 2});
CheckSearch(searcher, TsStringUtils.MakeString("c'est une", m_frWs), new[] {1, 2});
CheckSearch(searcher, TsStringUtils.MakeString("t", m_enWs), new[] {0, 2, 3});
CheckSearch(searcher, TsStringUtils.MakeString("testing purpose", m_enWs), new[] {2});
}

/// <summary>
/// Tests substring (match-anywhere) matching, including infix, case- and diacritic-insensitivity.
/// </summary>
[Test]
public void SubstringSearchTest()
{
var searcher = new StringSearcher<int>(SearchType.Substring, m_wsManager);
searcher.Add(0, 0, TsStringUtils.MakeString("language", m_enWs));
searcher.Add(1, 0, TsStringUtils.MakeString("gauge", m_enWs));
searcher.Add(2, 0, TsStringUtils.MakeString("résumé", m_frWs));
searcher.Add(3, 0, TsStringUtils.MakeString("zebra", m_enWs));

// infix match: "uage" is not a prefix of "language" but is a substring (fails under Prefix/FullText).
CheckSearch(searcher, TsStringUtils.MakeString("uage", m_enWs), new[] {0});
// interior substring
CheckSearch(searcher, TsStringUtils.MakeString("gua", m_enWs), new[] {0});
CheckSearch(searcher, TsStringUtils.MakeString("aug", m_enWs), new[] {1});
// case-insensitive
CheckSearch(searcher, TsStringUtils.MakeString("LANG", m_enWs), new[] {0});
// diacritic-insensitive
CheckSearch(searcher, TsStringUtils.MakeString("resume", m_frWs), new[] {2});
// whole-string still matches
CheckSearch(searcher, TsStringUtils.MakeString("zebra", m_enWs), new[] {3});
// no match anywhere
CheckNoResultsSearch(searcher, TsStringUtils.MakeString("xyz", m_enWs));
}

/// <summary>
/// Substring search must not miss anything a full-text search would find on the same corpus and
/// queries: its result set is a near superset
/// (see <see cref="Substring_isNotASupersetForOutOfOrderMultiWordQueries"/>)
/// of the full-text result set. This guards the promise that switching Find Lexical Entry to
/// substring never drops a result that used to appear.
/// (This is a superset, not equality: substring also returns extra infix matches.)
/// </summary>
[Test]
public void SubstringResultsIncludeAllFullTextResults()
{
var fullText = BuildMultiRunCorpus(SearchType.FullText);
var substring = BuildMultiRunCorpus(SearchType.Substring);

foreach (ITsString query in FullTextQueries())
{
// StringSearcher.Search can return the same item several times (once per matching word);
// the real consumer (SearchEngine) dedupes via a HashSet, so compare as sets here too.
int[] fullTextResults = fullText.Search(0, query).Distinct().ToArray();
Assert.That(fullTextResults, Is.Not.Empty,
"query '" + query.Text + "' should match something under full-text (otherwise the check is vacuous)");
Assert.That(substring.Search(0, query).Distinct(), Is.SupersetOf(fullTextResults),
"substring dropped a full-text match for query '" + query.Text + "'");
}
}

/// <summary>
/// Pins the boundary of the superset guarantee: it holds only for single-token or contiguous,
/// in-order queries. A multi-word query whose words appear OUT OF ORDER matches under full-text
/// (which ANDs the word tokens regardless of order) but NOT under substring (which needs the
/// whole query to appear contiguously). This is the concrete case behind the scoping note on
/// <see cref="FullTextQueries"/>.
/// </summary>
[Test]
public void Substring_isNotASupersetForOutOfOrderMultiWordQueries()
{
var fullText = new StringSearcher<int>(SearchType.FullText, m_wsManager);
var substring = new StringSearcher<int>(SearchType.Substring, m_wsManager);
ITsString text = TsStringUtils.MakeString("alpha beta gamma", m_enWs);
fullText.Add(0, 0, text);
substring.Add(0, 0, text);

// Words present but in a different order than the text.
ITsString outOfOrder = TsStringUtils.MakeString("gamma alpha", m_enWs);

Assert.That(fullText.Search(0, outOfOrder), Does.Contain(0),
"full-text ANDs the word tokens, so it matches the words in any order");
Assert.That(substring.Search(0, outOfOrder), Does.Not.Contain(0),
"substring needs the query contiguous, so out-of-order words do not match");
}
}
}
Loading